Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752593AbaFEPMh (ORCPT ); Thu, 5 Jun 2014 11:12:37 -0400 Received: from cantor2.suse.de ([195.135.220.15]:34995 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751776AbaFEPMg (ORCPT ); Thu, 5 Jun 2014 11:12:36 -0400 Date: Thu, 05 Jun 2014 17:12:32 +0200 Message-ID: From: Takashi Iwai To: Ming Lei Cc: Tom Gundersen , LKML , Greg KH , Stefan Roese , Arnd Bergmann , Abhay Salunke , Kay Sievers Subject: Re: [PATCH v2] firmware loader: allow disabling of udev as firmware loader In-Reply-To: References: <1401896895-14262-1-git-send-email-tiwai@suse.de> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Thu, 5 Jun 2014 22:54:33 +0800, Ming Lei wrote: > > On Thu, Jun 5, 2014 at 10:32 PM, Tom Gundersen wrote: > > On Thu, Jun 5, 2014 at 4:24 PM, Ming Lei wrote: > >> On Thu, Jun 5, 2014 at 10:05 PM, Takashi Iwai wrote: > >>> At Thu, 5 Jun 2014 21:59:52 +0800, > >>> Ming Lei wrote: > >>>> > >>>> On Thu, Jun 5, 2014 at 9:47 PM, Takashi Iwai wrote: > >>>> > At Thu, 5 Jun 2014 21:31:56 +0800, > >>>> > Ming Lei wrote: > >>>> >> > >>>> >> On Thu, Jun 5, 2014 at 8:25 PM, Tom Gundersen wrote: > >>>> >> > > >>>> >> > On 5 Jun 2014 14:18, "Ming Lei" wrote: > >>>> >> >> > >>>> >> >> On Wed, Jun 4, 2014 at 11:48 PM, Takashi Iwai wrote: > >>>> >> >> > [The patch was originally proposed by Tom Gundersen, and rewritten > >>>> >> >> > afterwards by me; most of changelogs below borrowed from Tom's > >>>> >> >> > original patch -- tiwai] > >>>> >> >> > > >>>> >> >> > Currently (at least) the dell-rbu driver selects FW_LOADER_USER_HELPER, > >>>> >> >> > which means that distros can't really stop loading firmware through > >>>> >> >> > udev without breaking other users (though some have). > >>>> >> >> > > >>>> >> >> > Ideally we would remove/disable the udev firmware helper in both the > >>>> >> >> > kernel and in udev, but if we were to disable it in udev and not the > >>>> >> >> > kernel, the result would be (seemingly) hung kernels as no one would > >>>> >> >> > be around to cancel firmware requests. > >>>> >> >> > > >>>> >> >> > This patch allows udev firmware loading to be disabled while still > >>>> >> >> > allowing non-udev firmware loading, as done by the dell-rbu driver, to > >>>> >> >> > continue working. This is achieved by only using the fallback > >>>> >> >> > mechanism when the uevent is suppressed. > >>>> >> >> > > >>>> >> >> > The patch renames the user-selectable Kconfig from FW_LOADER_USER_HELPER > >>>> >> >> > to FW_LOADER_USER_HELPER_FALLBACK, and the former is reverse-selected > >>>> >> >> > by the latter or the drivers that need userhelper like dell-rbu. > >>>> >> >> > > >>>> >> >> > Also, the "default y" is removed together with this change, since it's > >>>> >> >> > been deprecated in udev upstream, thus rather better to disable it > >>>> >> >> > nowadays. > >>>> >> >> > > >>>> >> >> > Tested with > >>>> >> >> > FW_LOADER_USER_HELPER=n > >>>> >> >> > LATTICE_ECP3_CONFIG=y > >>>> >> >> > DELL_RBU=y > >>>> >> >> > and udev without the firmware loading support, but I don't have the > >>>> >> >> > hardware to test the lattice/dell drivers, so additional testing would > >>>> >> >> > be appreciated. > >>>> >> >> > > >>>> >> >> > Reviewed-by: Tom Gundersen > >>>> >> >> > Cc: Ming Lei > >>>> >> >> > Cc: Greg Kroah-Hartman > >>>> >> >> > Cc: Abhay Salunke > >>>> >> >> > Cc: Stefan Roese > >>>> >> >> > Cc: Arnd Bergmann > >>>> >> >> > Cc: Kay Sievers > >>>> >> >> > Signed-off-by: Takashi Iwai > >>>> >> >> > --- > >>>> >> >> > drivers/base/Kconfig | 10 ++++++++-- > >>>> >> >> > drivers/base/firmware_class.c | 15 ++++++++++----- > >>>> >> >> > include/linux/firmware.h | 2 +- > >>>> >> >> > 3 files changed, 19 insertions(+), 8 deletions(-) > >>>> >> >> > > >>>> >> >> > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig > >>>> >> >> > index 8fa8deab6449..d0bb32e4c416 100644 > >>>> >> >> > --- a/drivers/base/Kconfig > >>>> >> >> > +++ b/drivers/base/Kconfig > >>>> >> >> > @@ -144,15 +144,21 @@ config EXTRA_FIRMWARE_DIR > >>>> >> >> > some other directory containing the firmware files. > >>>> >> >> > > >>>> >> >> > config FW_LOADER_USER_HELPER > >>>> >> >> > + bool > >>>> >> >> > + > >>>> >> >> > +config FW_LOADER_USER_HELPER_FALLBACK > >>>> >> >> > bool "Fallback user-helper invocation for firmware loading" > >>>> >> >> > depends on FW_LOADER > >>>> >> >> > - default y > >>>> >> >> > + select FW_LOADER_USER_HELPER > >>>> >> >> > help > >>>> >> >> > This option enables / disables the invocation of user-helper > >>>> >> >> > (e.g. udev) for loading firmware files as a fallback after the > >>>> >> >> > direct file loading in kernel fails. The user-mode helper is > >>>> >> >> > no longer required unless you have a special firmware file > >>>> >> >> > that > >>>> >> >> > - resides in a non-standard path. > >>>> >> >> > + resides in a non-standard path. Moreover, the udev support has > >>>> >> >> > + been deprecated upstream. > >>>> >> >> > + > >>>> >> >> > + If you are unsure about this, say N here. > >>>> >> >> > >>>> >> >> It may be safer to say Y here for fallback if not sure. > >>>> >> > > >>>> >> > Saying Y here will be actively harmful if the firmware loader is disabled in > >>>> >> > udev (which I guess most distros will do as soon as they can), so I think we > >>>> >> > >>>> >> If fallback is triggered, it means that the firmware can't be found > >>>> >> in default direct rootfs path, so it is better to continue to look for it > >>>> >> from userspace. > >>>> >> > >>>> >> Also it won't a big problem for hanging the request user context > >>>> >> for some while(60sec at default) if udev is disabled, will it? > >>>> >> > >>>> >> BTW, are you sure most distros will do that in the near future? > >>>> >> > >>>> >> > should advice people to say N unless they really know what they are doing > >>>> >> > and that their userspace can cope with it. > >>>> >> > >>>> >> That is why I suggest to say Y if someone isn't sure. > >>>> > > >>>> > For the time being, having default this Y causes more troubles. > >>>> > >>>> I am wondering why default Y may cause more troubles, as we > >>>> know, it has been default Y for long long time. > >>> > >>> More trouble = more bug reports. At least, a handful distros suffer. > >>> I don't know the situation with Ubuntu, though. > >> > >> Looks recently not see related report, :-) > > > > Ubuntu currently enables the firmware loader in both the kernel and in > > udev, so would not yet have a problem here at the moment. However, I > > spoke with Martin Pitt and he told me that both Debian and Ubuntu > > would like to switch this off in udev once it is possible (i.e., once > > this patch has been merged I guess). Fedora already did, and speaking > > for Arch we definitely would like to do the same. I did not check with > > other distros, but I'm pretty sure "everyone" will disable this in > > udev by the next cycle. At which point having it enabled in the kernel > > will cause real problems and bug reports. > > That won't cover the case of old distributions with new kernel, do > you want to break/confuse these users? Why it breaks? They can just select it. > > For distro kernels that's not a problem, as they know what to do, but > > I guess for random kernel users we should give them the correct > > recommendation. > > Also I remember that android users put firmware under their > special path. And, they are sure what Kconfig options to take. > >>>> It only falls back if the request fw is _not_ found from direct loading, > >>>> so it is reasonable to try to continue to look for it from user space. > >> > >>> Some drivers fall back to different firmware (e.g. different revision > >>> suffix) if the requested file isn't found. So, this happens in > >>> reality. > >> > >> So do you think the fallback is better or worse? For CPU microcode > >> case, maybe it is fine, but for other devices, maybe it is better to > >> get a firmware for working at least. > > > > What usecase do you have in mind here? For people using stock udev, > > enabling the fallback will not give any benefit, but it will soon > > Again, we have old distributions, also it should make sense to fall back > to userspace for non-exist firmwares under default path. > > > start giving real problems... > > If there isn't firmwares at default path for devices, the device may > not work, and that is the real problem. Ming, we're discussing about the help text for people who aren't sure what to select. Which chance would you bet as such a target? A newbie, or a bearded techy sticking with old distros? For people who are using the old distros *and* with non-standard firmware paths, they must be sure what they're doing and what they must select. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/