Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752035AbaFENsA (ORCPT ); Thu, 5 Jun 2014 09:48:00 -0400 Received: from cantor2.suse.de ([195.135.220.15]:32977 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751980AbaFENr6 (ORCPT ); Thu, 5 Jun 2014 09:47:58 -0400 Date: Thu, 05 Jun 2014 15:47:55 +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 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. In such a case, we should decide from utilitarianism POV, IMO. Anyone who isn't sure would likely have a recent system that cannot cope with user-space f/w loader. If anyone install to a non-standard path, they do something special, after all. (And, due to Kconfig rename, user has to review the Kconfig again. So they must see the change.) 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/