Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Greg KH]

On Thu, Jul 10, 2014 at 08:10:01PM +0530, B_B_Singh@DELL.com wrote:
> Resending the mail..
> 
> Hi All,
> 
> As I communicated earlier the test never passed with older BIOS DUPs.

That test?  What patch?  Please be explicit, I deal with thousands of
patches and have no short term memory...

> Again today I have tested with 3.15.5 kernel with below patch applied,
> & I don't see the BIOS getting updated. The BIOS update always failed
> with below patch.  
> 
> I'm not sure why the patch is getting committed even upon failure.

Because in that long and winding email thread I thought you said this
patch _worked_.  Obviously I was wrong, sorry, please suggest an
alternative.

greg k-h

RE: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[B_B_Singh]

Sorry Greg,

Here is the patch which I was referring to https://lkml.org/lkml/2014/6/4/327

I was doing the BIOS update test using the DELL BIOS-DUP's with the above patch.

Will provide you more technical analysis on why it is failing by tomorrow.

Regards
Balaji Singh

-----Original Message-----
From: Greg KH [mailto:gregkh@linuxfoundation.org]
Sent: Thursday, July 10, 2014 8:24 PM
To: Singh, B B
Cc: tiwai@suse.de; Abhay_Salunke@dell.com; arnd@arndb.de; kay@vrfy.org; ming.lei@canonical.com; sr@denx.de; teg@jklm.no; Hayes, Stuart; Gowda, Srinivas G; linux-kernel@vger.kernel.org
Subject: Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

On Thu, Jul 10, 2014 at 08:10:01PM +0530, B_B_Singh@DELL.com wrote:
> Resending the mail..
> 
> Hi All,
> 
> As I communicated earlier the test never passed with older BIOS DUPs.

That test? What patch? Please be explicit, I deal with thousands of patches and have no short term memory...

> Again today I have tested with 3.15.5 kernel with below patch applied, 
> & I don't see the BIOS getting updated. The BIOS update always failed 
> with below patch.
> 
> I'm not sure why the patch is getting committed even upon failure.

Because in that long and winding email thread I thought you said this patch _worked_. Obviously I was wrong, sorry, please suggest an alternative.

greg k-h

RE: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[B_B_Singh]

Hi Takashi,

Sorry for the late response,  I tried with latest stable kernel 3.15.8.  surprisingly the BIOS update works even without applying the patch in both the cases.

Case I:
BIOS update is always successful with or without applying the patch https://lkml.org/lkml/2014/6/4/327, I don't see any issue related to firmware caching as seen in 3.15.5.

 1. echo 4096 > /sys/devices/platform/dell_rbu/packet_size.
 2. echo packet > /sys/devices/platform/dell_rbu/image_type.
 3. echo init > /sys/devices/platform/dell_rbu/image_type.
 4. echo 1 > /sys/class/firmware/dell_rbu/loading.
 5. cat /home/payload > /sys/class/firmware/dell_rbu/data.
 6. echo 0 > /sys/class/firmware/dell_rbu/loading.
 7. smbios-token-ctl --activate -i 0x005c

Case II:
Instead of using /sys/class/firmware path, I tried using the /lib/firmware/   path for the payload (i.e, direct load method). The BIOS update happens successfully every time even with or without applying patch. ( https://lkml.org/lkml/2014/6/4/327)

1. cat payload  > /lib/firmware/dell_rbu (copy the whole packetized data file to /lib/firmware/dell_rbu)
2. echo 4096  > /sys/devices/platform/dell_rbu/packet_size.
3. echo packet > /sys/devices/platform/dell_rbu/image_type.
4. echo init > /sys/devices/platform/dell_rbu/image_type.
5. smbios-token-ctl --activate -i 0x005c


Regards
Balaji Singh




Regards
Balaji Singh
-----Original Message-----
From: Takashi Iwai [mailto:tiwai@suse.de]
Sent: Friday, July 18, 2014 12:32 AM
To: Singh, B B
Cc: gregkh@linuxfoundation.org; Abhay_Salunke@dell.com; arnd@arndb.de; kay@vrfy.org; ming.lei@canonical.com; sr@denx.de; teg@jklm.no; Hayes, Stuart; Gowda, Srinivas G
Subject: Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

At Thu, 17 Jul 2014 11:30:09 -0700,
wrote:
>
> Dell - Internal Use - Confidential
> Hi Takashi,
>
> I made a fresh install on a new machine, then all the below steps passed on the first attempt.

Did you test with the patch or without? Please clarify.

>
> 1. echo 4096 > /sys/devices/platform/dell_rbu/packet_size.
> 2. echo packet > /sys/devices/platform/dell_rbu/image_type.
> 3. echo init > /sys/devices/platform/dell_rbu/image_type.
> 4. echo 1 > /sys/class/firmware/dell_rbu/loading.
> 5. cat /home/payload > /sys/class/firmware/dell_rbu/data.
> 6. echo 0 > /sys/class/firmware/dell_rbu/loading.
>
> from second attempt I faced the same issue again ie., /sys/class/firmware/dell_rbu is missing. I'm working it will provide you the more details in couple of days.

I guess this is a side-effect of the firmware caching.
If so, it's been so over years.


Takashi

>
> Regards
> Balaji Singh
>
>
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, July 15, 2014 3:09 PM
> To: Singh, B B
> Cc: gregkh@linuxfoundation.org; Abhay_Salunke@dell.com; arnd@arndb.de;
> kay@vrfy.org; ming.lei@canonical.com; sr@denx.de; teg@jklm.no; Hayes,
> Stuart; Gowda, Srinivas G
> Subject: Re: patch "firmware loader: allow disabling of udev as
> firmware loader" added to driver-core tree
>
> At Mon, 14 Jul 2014 10:05:00 -0700,
> wrote:
> >
> > >> Did the kernel really work before patching...?
> >
> > What do you mean by "Did really kernel work"?
>
> Well, I thought you mentioned that my patch breaks the stuff.
> So I asked whether the f/w loader stuff really worked with vanilla 3.15.x kernel as is before you patch.
>
> > I was able to boot successfully with 3.15.5 kernel. But the network interfaces was never up. My guess is due to the same issue its unable to update the .fw file.
>
> But, judging from this, 3.15.5 vanilla is already broken?
> Then you'd better to bisect.
>
>
> Takashi
>
> > >>The codepath should go through _request_firmware() ->
> > >>fw_load_from_user_helper() -> _request_firmware_load(), and the sysfs file is created at this point.
> >
> > I will check this & let you know.
> >
> > Regards
> > Balaji Singh
> >
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Friday, July 11, 2014 9:12 PM
> > To: Singh, B B
> > Cc: gregkh@linuxfoundation.org; Abhay_Salunke@dell.com;
> > arnd@arndb.de; kay@vrfy.org; ming.lei@canonical.com; sr@denx.de;
> > teg@jklm.no; Hayes, Stuart; Gowda, Srinivas G
> > Subject: Re: patch "firmware loader: allow disabling of udev as
> > firmware loader" added to driver-core tree
> >
> > At Fri, 11 Jul 2014 21:03:19 +0530,
> > wrote:
> > >
> > > Hi All,
> > >
> > > Today I have tested with below config with Dell DUP R910_BIOS_GRWR5_LN_2.9.0.BIN The test failed.
> > >
> > > FW_LOADER_USER_HELPER=y,
> > > FW_LOADER_USER_HELPER_FALLBACK=n , DELL_RBU=y.
> > >
> > > To further investigate I removed DUP dependency & come up with
> > > simple steps Here are simple steps that I followed for the BIOS update. (Internally the .BIN does the same).
> > >
> > > 1. echo 4096 > /sys/devices/platform/dell_rbu/packet_size.
> > > 2. echo packet > /sys/devices/platform/dell_rbu/image_type.
> > > 3. echo init > /sys/devices/platform/dell_rbu/image_type.
> > > 4. echo 1 > /sys/class/firmware/dell_rbu/loading.
> > > 5. cat /home/payload > /sys/class/firmware/dell_rbu/data.
> > > 6. echo 0 > /sys/class/firmware/dell_rbu/loading.
> > >
> > > The failure is causing at step 4 because the entries(loading &
> > > data
> > > ) are not created for dell_rbu under /sys/class/firmware. (I mean
> > > the dell_rbu folder is not created under /sys/class/firmware/ )
> > >
> > > The return value of request_firmware_nowait is "0" & the fw_work->opt_flags value was "6" as expected.
> > >
> > > Does anyone have idea about "why entries are not created under /sys/class/firmware?"
> >
> > Did the kernel really work before patching...?
> > The codepath should go through _request_firmware() ->
> > fw_load_from_user_helper() -> _request_firmware_load(), and the sysfs file is created at this point.
> >
> >
> > Takashi
> >
> > >
> > > Regards
> > > Balaji Singh
> > >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Thursday, July 10, 2014 9:16 PM
> > > To: Singh, B B
> > > Cc: gregkh@linuxfoundation.org; Abhay_Salunke@dell.com;
> > > arnd@arndb.de; kay@vrfy.org; ming.lei@canonical.com; sr@denx.de;
> > > teg@jklm.no; Hayes, Stuart; Gowda, Srinivas G
> > > Subject: Re: patch "firmware loader: allow disabling of udev as
> > > firmware loader" added to driver-core tree
> > >
> > > At Thu, 10 Jul 2014 20:50:35 +0530,
> > > wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > Just now I made test with below config.
> > > >
> > > > FW_LOADER_USER_HELPER=y,
> > > > FW_LOADER_USER_HELPER_FALLBACK=n , DELL_RBU=y.
> > > >
> > > > But still the test fails.
> > >
> > > Hrm, that's bad.
> > >
> > > > Will provide you more details tomorrow on why it is failing.
> > >
> > > OK, thanks. Try to put a debug print for opt_flags in _request_firmware(). This must be 6 ( = FW_OPT_NOWAIT | FW_OPT_USERHELPER).
> > >
> > >
> > > Takashi
> > >
> > > >
> > > > Regards
> > > > Balaji Singh
> > > >
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Thursday, July 10, 2014 8:27 PM
> > > > To: Singh, B B
> > > > Cc: gregkh@linuxfoundation.org; Abhay_Salunke@dell.com;
> > > > arnd@arndb.de; kay@vrfy.org; ming.lei@canonical.com; sr@denx.de;
> > > > teg@jklm.no; Hayes, Stuart; Gowda, Srinivas G
> > > > Subject: Re: patch "firmware loader: allow disabling of udev as
> > > > firmware loader" added to driver-core tree
> > > >
> > > > At Thu, 10 Jul 2014 20:20:39 +0530,
> > > > wrote:
> > > > >
> > > > > Hi Takashi/Tom,
> > > > >
> > > > > I have tested using the below config as mentioned in the patch & It didn't work.
> > > > >
> > > > > FW_LOADER_USER_HELPER=n
> > > > > DELL_RBU=y
> > > >
> > > > Oh, no, you chose the wrong one. The new Kconfig is FW_LOADER_USER_HELPER_FALLBACK. FW_LOADER_USER_HELPER is reverse-selected by DELL_RBU, so it must be y.
> > > > If you run "make oldconfig" once, the option should have been set automatically.
> > > >
> > > >
> > > > Takashi
> > > >
> > > > > Will try the combinations of CONFIG_FW_LOADER_USER_HELPER_FALLBACK & FW_LOADER_USER_HELPER by tomorrow & let you know the results. & also will provide the details where exactly it is failing.
> > > > >
> > > > > If you have any specific scenario to try let me know, I'll test & provide you the results.
> > > > >
> > > > > Regards
> > > > > Balaji Singh
> > > > >
> > > > > -----Original Message-----
> > > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > > Sent: Thursday, July 10, 2014 8:10 PM
> > > > > To: Singh, B B
> > > > > Cc: gregkh@linuxfoundation.org; Abhay_Salunke@dell.com;
> > > > > arnd@arndb.de; kay@vrfy.org; ming.lei@canonical.com;
> > > > > sr@denx.de; teg@jklm.no; Hayes, Stuart; Gowda, Srinivas G
> > > > > Subject: Re: patch "firmware loader: allow disabling of udev
> > > > > as firmware loader" added to driver-core tree
> > > > >
> > > > > At Thu, 10 Jul 2014 19:51:40 +0530,
> > > > > wrote:
> > > > > >
> > > > > > Dell - Internal Use - Confidential
> > > > > >
> > > > > > Hi All,
> > > > > >
> > > > > >
> > > > > > As I communicated earlier the test never passed with older BIOS DUPs.
> > > > >
> > > > > I didn't get any mail from you until now.
> > > > >
> > > > > > Again today I have tested with 3.15.5 kernel with below patch applied, & I don't see the BIOS getting updated. The BIOS update always failed with below patch.
> > > > >
> > > > > Double-check whether you're testing correctly. Which Kconfig option did you set? It fails no matter whether you set CONFIG_FW_LOADER_USER_HELPER_FALLBACK or not?
> > > > >
> > > > > > I'm not sure why the patch is getting committed even upon failure.
> > > > >
> > > > > It's just a miscommunication. You should have put more people involved.
> > > > >
> > > > >
> > > > > thanks,
> > > > >
> > > > > Takashi
> > > > >
> > > > > > Regards
> > > > > > Balaji Singh
> > > > > >
> > > > > >
> > > > > > -----Original Message-----
> > > > > > From: gregkh@linuxfoundation.org
> > > > > > [mailto:gregkh@linuxfoundation.org]
> > > > > > Sent: Wednesday, July 09, 2014 3:55 AM
> > > > > > To: tiwai@suse.de; Abhay_Salunke@dell.com; Singh, B B;
> > > > > > arnd@arndb.de; gregkh@linuxfoundation.org; kay@vrfy.org;
> > > > > > ming.lei@canonical.com; sr@denx.de; teg@jklm.no
> > > > > > Subject: patch "firmware loader: allow disabling of udev as
> > > > > > firmware loader" added to driver-core tree
> > > > > >
> > > > > >
> > > > > > This is a note to let you know that I've just added the
> > > > > > patch titled
> > > > > >
> > > > > > firmware loader: allow disabling of udev as firmware loader
> > > > > >
> > > > > > to my driver-core git tree which can be found at
> > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-
> > > > > > co
> > > > > > re
> > > > > > .g
> > > > > > it
> > > > > > in the driver-core-next branch.
> > > > > >
> > > > > > The patch will show up in the next release of the linux-next
> > > > > > tree (usually sometime within the next 24 hours during the
> > > > > > week.)
> > > > > >
> > > > > > The patch will also be merged in the next major kernel release during the merge window.
> > > > > >
> > > > > > If you have any questions about this process, please let me know.
> > > > > >
> > > > > >
> > > > > > >From 5a1379e8748a5cfa3eb068f812d61bde849ef76c Mon Sep 17
> > > > > > >00:00:00
> > > > > > >2001
> > > > > > From: Takashi Iwai
> > > > > > Date: Wed, 4 Jun 2014 17:48:15 +0200
> > > > > > Subject: firmware loader: allow disabling of udev as
> > > > > > firmware loader
> > > > > >
> > > > > > [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: Abhay Salunke
> > > > > > Cc: Stefan Roese
> > > > > > Cc: Arnd Bergmann
> > > > > > Cc: Kay Sievers
> > > > > > Tested-by: Balaji Singh
> > > > > > Signed-off-by: Takashi Iwai
> > > > > > Signed-off-by: Greg Kroah-Hartman
> > > > > > ---
> > > > > > 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 00e13ce5cbbd..88500fed3c7a 100644
> > > > > > --- a/drivers/base/Kconfig
> > > > > > +++ b/drivers/base/Kconfig
> > > > > > @@ -149,15 +149,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.
> > > > > >
> > > > > > config DEBUG_DRIVER
> > > > > > bool "Driver Core verbose debug messages"
> > > > > > diff --git a/drivers/base/firmware_class.c
> > > > > > b/drivers/base/firmware_class.c index
> > > > > > d276e33880be..46ea5f4c3bb5
> > > > > > 100644
> > > > > > --- a/drivers/base/firmware_class.c
> > > > > > +++ b/drivers/base/firmware_class.c
> > > > > > @@ -100,9 +100,14 @@ static inline long
> > > > > > firmware_loading_timeout(void) #define FW_OPT_UEVENT (1U <<
> > > > > > 0) #define FW_OPT_NOWAIT (1U << 1) #ifdef
> > > > > > CONFIG_FW_LOADER_USER_HELPER -#define FW_OPT_FALLBACK (1U <<
> > > > > > 2)
> > > > > > +#define FW_OPT_USERHELPER (1U << 2)
> > > > > > #else
> > > > > > -#define FW_OPT_FALLBACK 0
> > > > > > +#define FW_OPT_USERHELPER 0 #endif #ifdef
> > > > > > +CONFIG_FW_LOADER_USER_HELPER_FALLBACK
> > > > > > +#define FW_OPT_FALLBACK FW_OPT_USERHELPER #else #define
> > > > > > +FW_OPT_FALLBACK
> > > > > > +0
> > > > > > #endif
> > > > > >
> > > > > > struct firmware_cache {
> > > > > > @@ -1111,7 +1116,7 @@ _request_firmware(const struct
> > > > > > firmware **firmware_p, const char *name,
> > > > > >
> > > > > > ret = fw_get_filesystem_firmware(device, fw->priv); if (ret)
> > > > > > {
> > > > > > - if (opt_flags & FW_OPT_FALLBACK) {
> > > > > > + if (opt_flags & FW_OPT_USERHELPER) {
> > > > > > dev_warn(device,
> > > > > > "Direct firmware load failed with error %d\n", ret); @@
> > > > > > -1171,7
> > > > > > +1176,7 @@ request_firmware(const struct firmware
> > > > > > +**firmware_p, const
> > > > > > char *name, } EXPORT_SYMBOL(request_firmware);
> > > > > >
> > > > > > -#ifdef CONFIG_FW_LOADER_USER_HELPER
> > > > > > +#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK
> > > > > > /**
> > > > > > * request_firmware: - load firmware directly without
> > > > > > usermode helper
> > > > > > * @firmware_p: pointer to firmware image @@ -1277,7 +1282,7
> > > > > > @@ request_firmware_nowait( fw_work->context = context;
> > > > > > fw_work->cont = cont; fw_work->opt_flags = FW_OPT_NOWAIT |
> > > > > > FW_OPT_FALLBACK |
> > > > > > - (uevent ? FW_OPT_UEVENT : 0);
> > > > > > + (uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
> > > > > >
> > > > > > if (!try_module_get(module)) { kfree(fw_work); diff --git
> > > > > > a/include/linux/firmware.h b/include/linux/firmware.h index
> > > > > > 59529330efd6..67e5b801af0c 100644
> > > > > > --- a/include/linux/firmware.h
> > > > > > +++ b/include/linux/firmware.h
> > > > > > @@ -68,7 +68,7 @@ static inline void release_firmware(const
> > > > > > struct firmware *fw)
> > > > > >
> > > > > > #endif
> > > > > >
> > > > > > -#ifdef CONFIG_FW_LOADER_USER_HELPER
> > > > > > +#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK
> > > > > > int request_firmware_direct(const struct firmware **fw,
> > > > > > const char *name, struct device *device); #else
> > > > > > --
> > > > > > 2.0.0.254.g50f84e3
> > > > > >
> > > > >
> > > >
> > >
> > [2 ]
> >

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Jean-Michel Hautbois]

2014-08-01 22:01 GMT+02:00  <B_B_Singh@dell.com>:
> Hi Takashi,
>
> Sorry for the late response,  I tried with latest stable kernel 3.15.8.  surprisingly the BIOS update works even without applying the patch in both the cases.

Hi,I'm sorry for my late answer too, but I didn't try this patch recently...

I have a Lattice FPGA on my board, and it seems that your patch breaks
the firmware loading.
I have the following config :
CONFIG_FW_LOADER=y
CONFIG_FW_LOADER_USER_HELPER=n
FW_LOADER_USER_HELPER_FALLBACK=n
LATTICE_ECP3_CONFIG=y


And my kernel panics :
[    1.180677] lattice-ecp3 spi2.3: FPGA bitstream configuration
driver registered
[    1.188285] lattice-ecp3 spi2.3: Direct firmware load for
lattice-ecp3.bit failed with error -2
[    1.197063] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[    1.203616] spi_imx 2010000.ecspi: probed
[    1.215947] pgd = 80004000
[    1.218671] [00000000] *pgd=00000000
[    1.222284] Internal error: Oops: 5 [#1] SMP ARM
[    1.226914] Modules linked in:
[    1.230007] CPU: 1 PID: 47 Comm: kworker/1:1 Not tainted
3.16.0+yocto+g403c2fc #1
[    1.237516] Workqueue: events request_firmware_work_func
[    1.242861] task: be1d2700 ti: be20e000 task.ti: be20e000
[    1.248281] PC is at firmware_load+0x10/0x47c
[    1.252652] LR is at request_firmware_work_func+0x38/0x58
[    1.258064] pc : [<80390490>]    lr : [<803834c8>]    psr: 20000113
[    1.258064] sp : be20fda0  ip : be20fe68  fp : be20fe64
[    1.269552] r10: be20e008  r9 : 00000001  r8 : be7c5a00
[    1.274788] r7 : be20fea8  r6 : be7c2500  r5 : be3f95c0  r4 : be3f95c0
[    1.281326] r3 : 80390480  r2 : be3f9600  r1 : bd829800  r0 : 00000000
[    1.287866] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
[    1.295185] Control: 10c5387d  Table: 1000404a  DAC: 00000015
[    1.300941] Process kworker/1:1 (pid: 47, stack limit = 0xbe20e240)
[    1.307220] Stack: (0xbe20fda0 to 0xbe210000)
[    1.311595] fda0: be20fdd4 be20fdb0 803825b4 800e44f8 00000001
00000000 80382614 be3f9600
[    1.319787] fdc0: 811ccae8 be3f9600 806e3bd4 be3f9600 fffffffe
fffffffe 00000001 be0eb000
[    1.327980] fde0: be20fe04 be20fdf0 803826c8 800e44f8 be1d2700
80870aa8 be20fe64 be20fe08
[    1.336172] fe00: 80382d18 80382668 be3fa348 be20e000 be1d2700
be078000 80870aa8 be3f9600
[    1.344364] fe20: be3fa348 be20fe6c bd829800 00000003 be3fa300
803834b8 00000001 be3f95c0
[    1.352556] fe40: be3f95c0 be7c2500 be20fea8 be7c5a00 00000001
be20e008 be20fe84 be20fe68
[    1.360748] fe60: 803834c8 8039048c be0c2800 00000000 be7c2500
be0c2800 be20fee4 be20fe88
[    1.368939] fe80: 8004245c 8038349c 00000001 00000000 800423e8
be7c2500 80042e54 00000000
[    1.377131] fea0: 00000000 00000000 811ccb7c 80b0bd70 00000000
8086f89c 806acc48 be7c2500
[    1.385322] fec0: be7c2530 be20e028 be20e000 be0c2818 00000008
be0c2800 be20ff24 be20fee8
[    1.393514] fee0: 80042f30 800422bc 806acd4c be0c5280 00000000
be0c2800 80042de8 be0c5280
[    1.401706] ff00: 00000000 be0c2800 80042de8 00000000 00000000
00000000 be20ffac be20ff28
[    1.409898] ff20: 80048ed0 80042df4 800675d0 00000000 be20ff54
be0c2800 00000000 00000000
[    1.418089] ff40: dead4ead ffffffff ffffffff 809caae0 00000000
00000000 80825c5c be20ff5c
[    1.426280] ff60: be20ff5c 00000000 00000000 dead4ead ffffffff
ffffffff 809caae0 00000000
[    1.434472] ff80: 00000000 80825c5c be20ff88 be20ff88 be0c5280
80048df4 00000000 00000000
[    1.442663] ffa0: 00000000 be20ffb0 8000f748 80048e00 00000000
00000000 00000000 00000000
[    1.450853] ffc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[    1.459044] ffe0: 00000000 00000000 00000000 00000000 00000013
00000000 40030210 12290009
[    1.467229] Backtrace:
[    1.469719] [<80390480>] (firmware_load) from [<803834c8>]
(request_firmware_work_func+0x38/0x58)
[    1.478600]  r10:be20e008 r9:00000001 r8:be7c5a00 r7:be20fea8
r6:be7c2500 r5:be3f95c0
[    1.486548]  r4:be3f95c0
[    1.489131] [<80383490>] (request_firmware_work_func) from
[<8004245c>] (process_one_work+0x1ac/0x414)
[    1.498446]  r4:be0c2800
[    1.501019] [<800422b0>] (process_one_work) from [<80042f30>]
(worker_thread+0x148/0x4e4)
[    1.509206]  r10:be0c2800 r9:00000008 r8:be0c2818 r7:be20e000
r6:be20e028 r5:be7c2530
[    1.517152]  r4:be7c2500
[    1.519729] [<80042de8>] (worker_thread) from [<80048ed0>]
(kthread+0xdc/0xf8)
[    1.526960]  r10:00000000 r9:00000000 r8:00000000 r7:80042de8
r6:be0c2800 r5:00000000
[    1.534906]  r4:be0c5280
[    1.537483] [<80048df4>] (kthread) from [<8000f748>]
(ret_from_fork+0x14/0x2c)
[    1.544715]  r7:00000000 r6:00000000 r5:80048df4 r4:be0c5280
[    1.550474] Code: e1a0c00d e92ddff0 e24cb004 e24dd09c (e5903000)
[    1.556642] ---[ end trace d18e8b19760cc1e3 ]---

JM

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Takashi Iwai]

At Tue, 5 Aug 2014 17:43:17 +0200,
Jean-Michel Hautbois wrote:
> 
> 2014-08-01 22:01 GMT+02:00  <B_B_Singh@dell.com>:
> > Hi Takashi,
> >
> > Sorry for the late response,  I tried with latest stable kernel 3.15.8.  surprisingly the BIOS update works even without applying the patch in both the cases.
> 
> Hi,I'm sorry for my late answer too, but I didn't try this patch recently...
> 
> I have a Lattice FPGA on my board, and it seems that your patch breaks
> the firmware loading.
> I have the following config :
> CONFIG_FW_LOADER=y
> CONFIG_FW_LOADER_USER_HELPER=n
> FW_LOADER_USER_HELPER_FALLBACK=n
> LATTICE_ECP3_CONFIG=y

It worked before applying my patch properly...?

The kernel Oops itself is a bug in lattice-ecp3-config driver.
It blindly assumes that the returned firmware is non-NULL.
firmware_load() must have a NULL check of fw at the beginning.


thanks,

Takashi

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Jean-Michel Hautbois]

2014-08-05 17:53 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> At Tue, 5 Aug 2014 17:43:17 +0200,
> Jean-Michel Hautbois wrote:
>>
>> 2014-08-01 22:01 GMT+02:00  <B_B_Singh@dell.com>:
>> > Hi Takashi,
>> >
>> > Sorry for the late response,  I tried with latest stable kernel 3.15.8.  surprisingly the BIOS update works even without applying the patch in both the cases.
>>
>> Hi,I'm sorry for my late answer too, but I didn't try this patch recently...
>>
>> I have a Lattice FPGA on my board, and it seems that your patch breaks
>> the firmware loading.
>> I have the following config :
>> CONFIG_FW_LOADER=y
>> CONFIG_FW_LOADER_USER_HELPER=n
>> FW_LOADER_USER_HELPER_FALLBACK=n
>> LATTICE_ECP3_CONFIG=y
>
> It worked before applying my patch properly...?
>
> The kernel Oops itself is a bug in lattice-ecp3-config driver.
> It blindly assumes that the returned firmware is non-NULL.
> firmware_load() must have a NULL check of fw at the beginning.
>
>
> thanks,
>
> Takashi

Yes, it worked, if the firmware was in the /lib/firmware directory.
You are right, it lacks a NULL check, I will fix that.

Thanks,
JM

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Takashi Iwai]

At Tue, 5 Aug 2014 17:55:40 +0200,
Jean-Michel Hautbois wrote:
> 
> 2014-08-05 17:53 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> > At Tue, 5 Aug 2014 17:43:17 +0200,
> > Jean-Michel Hautbois wrote:
> >>
> >> 2014-08-01 22:01 GMT+02:00  <B_B_Singh@dell.com>:
> >> > Hi Takashi,
> >> >
> >> > Sorry for the late response,  I tried with latest stable kernel 3.15.8.  surprisingly the BIOS update works even without applying the patch in both the cases.
> >>
> >> Hi,I'm sorry for my late answer too, but I didn't try this patch recently...
> >>
> >> I have a Lattice FPGA on my board, and it seems that your patch breaks
> >> the firmware loading.
> >> I have the following config :
> >> CONFIG_FW_LOADER=y
> >> CONFIG_FW_LOADER_USER_HELPER=n
> >> FW_LOADER_USER_HELPER_FALLBACK=n
> >> LATTICE_ECP3_CONFIG=y
> >
> > It worked before applying my patch properly...?
> >
> > The kernel Oops itself is a bug in lattice-ecp3-config driver.
> > It blindly assumes that the returned firmware is non-NULL.
> > firmware_load() must have a NULL check of fw at the beginning.
> >
> >
> > thanks,
> >
> > Takashi
> 
> Yes, it worked, if the firmware was in the /lib/firmware directory.

Then double-check whether it's really my patch who broke.
In your kernel log:

[    1.188285] lattice-ecp3 spi2.3: Direct firmware load for lattice-ecp3.bit failed with error -2

which indicates that the direct f/w loading from /lib/firmware failed.
Since this code path is called no matter which kernel option is set,
the failure is very unlikely by my patch.


Takashi

> You are right, it lacks a NULL check, I will fix that.
> 
> Thanks,
> JM
> 

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Shuah Khan]

On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> Yes, it worked, if the firmware was in the /lib/firmware directory.
>

ok it works when the firmware is in /lib/firmware. It sounds to me the
reason load fails when the firmware is under /sys/class/firmware is
fw_load_from_user_helper() returns -ENOENT when
CONFIG_FW_LOADER_USER_HELPER is disabled.

It would be nice to see the entire dmesg with debug enabled though.

-- Shuah

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Jean-Michel Hautbois]

2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
> On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
>>> Yes, it worked, if the firmware was in the /lib/firmware directory.
>>
>
> ok it works when the firmware is in /lib/firmware. It sounds to me the
> reason load fails when the firmware is under /sys/class/firmware is
> fw_load_from_user_helper() returns -ENOENT when
> CONFIG_FW_LOADER_USER_HELPER is disabled.
>
> It would be nice to see the entire dmesg with debug enabled though.
>
> -- Shuah

It does not work with the 3.16 kernel even when firmware is in
/lib/firmware, it worked before this patch is applied.
Here is what I got in my dmesg :
[    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
[    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
lattice-ecp3.bit failed with error -2
[    0.308078] __fw_free_buf: fw-lattice-ecp3.bit buf=bd81a480 data=
(null) size=0
[    0.308094] lattice-ecp3 spi2.3: Cannot load firmware, aborting

If you want specific messages, please tell me.

Thanks,
JM

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Takashi Iwai]

At Wed, 6 Aug 2014 11:10:27 +0200,
Jean-Michel Hautbois wrote:
> 
> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
> >>
> >
> > ok it works when the firmware is in /lib/firmware. It sounds to me the
> > reason load fails when the firmware is under /sys/class/firmware is
> > fw_load_from_user_helper() returns -ENOENT when
> > CONFIG_FW_LOADER_USER_HELPER is disabled.
> >
> > It would be nice to see the entire dmesg with debug enabled though.
> >
> > -- Shuah
> 
> It does not work with the 3.16 kernel even when firmware is in
> /lib/firmware, it worked before this patch is applied.

Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.

> Here is what I got in my dmesg :
> [    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
> [    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
> lattice-ecp3.bit failed with error -2

It's -ENOENT.  Are you sure that you really have
 /lib/firmware/lattice-ecp3.bit file?


Takashi

> [    0.308078] __fw_free_buf: fw-lattice-ecp3.bit buf=bd81a480 data=
> (null) size=0
> [    0.308094] lattice-ecp3 spi2.3: Cannot load firmware, aborting
> 
> If you want specific messages, please tell me.
> 
> Thanks,
> JM
> 

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Jean-Michel Hautbois]

2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> At Wed, 6 Aug 2014 11:10:27 +0200,
> Jean-Michel Hautbois wrote:
>>
>> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
>> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
>> >>
>> >
>> > ok it works when the firmware is in /lib/firmware. It sounds to me the
>> > reason load fails when the firmware is under /sys/class/firmware is
>> > fw_load_from_user_helper() returns -ENOENT when
>> > CONFIG_FW_LOADER_USER_HELPER is disabled.
>> >
>> > It would be nice to see the entire dmesg with debug enabled though.
>> >
>> > -- Shuah
>>
>> It does not work with the 3.16 kernel even when firmware is in
>> /lib/firmware, it worked before this patch is applied.
>
> Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.

Oh, you are right of course, I am on upstream kernel and I have your
patch. I don't mean your match is causing the issue though ;-).

>> Here is what I got in my dmesg :
>> [    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
>> [    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
>> lattice-ecp3.bit failed with error -2
>
> It's -ENOENT.  Are you sure that you really have
>  /lib/firmware/lattice-ecp3.bit file?

Yes :
ls -al /lib/firmware/lattice-ecp3.bit
-rwxr-xr-x 1 root root 897753 Aug  5 15:04 /lib/firmware/lattice-ecp3.bit

Thanks,
JM

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Takashi Iwai]

At Wed, 6 Aug 2014 11:44:14 +0200,
Jean-Michel Hautbois wrote:
> 
> 2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> > At Wed, 6 Aug 2014 11:10:27 +0200,
> > Jean-Michel Hautbois wrote:
> >>
> >> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
> >> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
> >> >>
> >> >
> >> > ok it works when the firmware is in /lib/firmware. It sounds to me the
> >> > reason load fails when the firmware is under /sys/class/firmware is
> >> > fw_load_from_user_helper() returns -ENOENT when
> >> > CONFIG_FW_LOADER_USER_HELPER is disabled.
> >> >
> >> > It would be nice to see the entire dmesg with debug enabled though.
> >> >
> >> > -- Shuah
> >>
> >> It does not work with the 3.16 kernel even when firmware is in
> >> /lib/firmware, it worked before this patch is applied.
> >
> > Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
> 
> Oh, you are right of course, I am on upstream kernel and I have your
> patch. I don't mean your match is causing the issue though ;-).

I see.

> >> Here is what I got in my dmesg :
> >> [    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
> >> [    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
> >> lattice-ecp3.bit failed with error -2
> >
> > It's -ENOENT.  Are you sure that you really have
> >  /lib/firmware/lattice-ecp3.bit file?
> 
> Yes :
> ls -al /lib/firmware/lattice-ecp3.bit
> -rwxr-xr-x 1 root root 897753 Aug  5 15:04 /lib/firmware/lattice-ecp3.bit

Then you'd better bisect...
Or, at least, put a debug code to see which file is opened in the loop
in fw_get_filesystem_firmware().


Takashi

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Jean-Michel Hautbois]

2014-08-06 12:21 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> At Wed, 6 Aug 2014 11:44:14 +0200,
> Jean-Michel Hautbois wrote:
>>
>> 2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
>> > At Wed, 6 Aug 2014 11:10:27 +0200,
>> > Jean-Michel Hautbois wrote:
>> >>
>> >> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
>> >> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> >> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
>> >> >>
>> >> >
>> >> > ok it works when the firmware is in /lib/firmware. It sounds to me the
>> >> > reason load fails when the firmware is under /sys/class/firmware is
>> >> > fw_load_from_user_helper() returns -ENOENT when
>> >> > CONFIG_FW_LOADER_USER_HELPER is disabled.
>> >> >
>> >> > It would be nice to see the entire dmesg with debug enabled though.
>> >> >
>> >> > -- Shuah
>> >>
>> >> It does not work with the 3.16 kernel even when firmware is in
>> >> /lib/firmware, it worked before this patch is applied.
>> >
>> > Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
>>
>> Oh, you are right of course, I am on upstream kernel and I have your
>> patch. I don't mean your match is causing the issue though ;-).
>
> I see.
>
>> >> Here is what I got in my dmesg :
>> >> [    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
>> >> [    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
>> >> lattice-ecp3.bit failed with error -2
>> >
>> > It's -ENOENT.  Are you sure that you really have
>> >  /lib/firmware/lattice-ecp3.bit file?
>>
>> Yes :
>> ls -al /lib/firmware/lattice-ecp3.bit
>> -rwxr-xr-x 1 root root 897753 Aug  5 15:04 /lib/firmware/lattice-ecp3.bit
>
> Then you'd better bisect...
> Or, at least, put a debug code to see which file is opened in the loop
> in fw_get_filesystem_firmware().

Well, when this function is called, my rootfs is not yet mounted (I
have a dedicated partition on a SDCard)...
So I probably missed a step in order to get /lib/firmware before / is
mounted... ?

JM

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Takashi Iwai]

At Wed, 6 Aug 2014 12:50:28 +0200,
Jean-Michel Hautbois wrote:
> 
> 2014-08-06 12:21 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> > At Wed, 6 Aug 2014 11:44:14 +0200,
> > Jean-Michel Hautbois wrote:
> >>
> >> 2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> >> > At Wed, 6 Aug 2014 11:10:27 +0200,
> >> > Jean-Michel Hautbois wrote:
> >> >>
> >> >> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
> >> >> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >> >> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
> >> >> >>
> >> >> >
> >> >> > ok it works when the firmware is in /lib/firmware. It sounds to me the
> >> >> > reason load fails when the firmware is under /sys/class/firmware is
> >> >> > fw_load_from_user_helper() returns -ENOENT when
> >> >> > CONFIG_FW_LOADER_USER_HELPER is disabled.
> >> >> >
> >> >> > It would be nice to see the entire dmesg with debug enabled though.
> >> >> >
> >> >> > -- Shuah
> >> >>
> >> >> It does not work with the 3.16 kernel even when firmware is in
> >> >> /lib/firmware, it worked before this patch is applied.
> >> >
> >> > Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
> >>
> >> Oh, you are right of course, I am on upstream kernel and I have your
> >> patch. I don't mean your match is causing the issue though ;-).
> >
> > I see.
> >
> >> >> Here is what I got in my dmesg :
> >> >> [    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
> >> >> [    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
> >> >> lattice-ecp3.bit failed with error -2
> >> >
> >> > It's -ENOENT.  Are you sure that you really have
> >> >  /lib/firmware/lattice-ecp3.bit file?
> >>
> >> Yes :
> >> ls -al /lib/firmware/lattice-ecp3.bit
> >> -rwxr-xr-x 1 root root 897753 Aug  5 15:04 /lib/firmware/lattice-ecp3.bit
> >
> > Then you'd better bisect...
> > Or, at least, put a debug code to see which file is opened in the loop
> > in fw_get_filesystem_firmware().
> 
> Well, when this function is called, my rootfs is not yet mounted (I
> have a dedicated partition on a SDCard)...
> So I probably missed a step in order to get /lib/firmware before / is
> mounted... ?

Yeah, possibly :)  Maybe you did put /lib/firmware/* into initrd in
the earlier tests?


Takashi

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Jean-Michel Hautbois]

2014-08-06 12:52 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> At Wed, 6 Aug 2014 12:50:28 +0200,
> Jean-Michel Hautbois wrote:
>>
>> 2014-08-06 12:21 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
>> > At Wed, 6 Aug 2014 11:44:14 +0200,
>> > Jean-Michel Hautbois wrote:
>> >>
>> >> 2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
>> >> > At Wed, 6 Aug 2014 11:10:27 +0200,
>> >> > Jean-Michel Hautbois wrote:
>> >> >>
>> >> >> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
>> >> >> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> >> >> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
>> >> >> >>
>> >> >> >
>> >> >> > ok it works when the firmware is in /lib/firmware. It sounds to me the
>> >> >> > reason load fails when the firmware is under /sys/class/firmware is
>> >> >> > fw_load_from_user_helper() returns -ENOENT when
>> >> >> > CONFIG_FW_LOADER_USER_HELPER is disabled.
>> >> >> >
>> >> >> > It would be nice to see the entire dmesg with debug enabled though.
>> >> >> >
>> >> >> > -- Shuah
>> >> >>
>> >> >> It does not work with the 3.16 kernel even when firmware is in
>> >> >> /lib/firmware, it worked before this patch is applied.
>> >> >
>> >> > Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
>> >>
>> >> Oh, you are right of course, I am on upstream kernel and I have your
>> >> patch. I don't mean your match is causing the issue though ;-).
>> >
>> > I see.
>> >
>> >> >> Here is what I got in my dmesg :
>> >> >> [    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
>> >> >> [    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
>> >> >> lattice-ecp3.bit failed with error -2
>> >> >
>> >> > It's -ENOENT.  Are you sure that you really have
>> >> >  /lib/firmware/lattice-ecp3.bit file?
>> >>
>> >> Yes :
>> >> ls -al /lib/firmware/lattice-ecp3.bit
>> >> -rwxr-xr-x 1 root root 897753 Aug  5 15:04 /lib/firmware/lattice-ecp3.bit
>> >
>> > Then you'd better bisect...
>> > Or, at least, put a debug code to see which file is opened in the loop
>> > in fw_get_filesystem_firmware().
>>
>> Well, when this function is called, my rootfs is not yet mounted (I
>> have a dedicated partition on a SDCard)...
>> So I probably missed a step in order to get /lib/firmware before / is
>> mounted... ?
>
> Yeah, possibly :)  Maybe you did put /lib/firmware/* into initrd in
> the earlier tests?

Nope, it failed and when / was mounted, it was called again and used
the firmware thanks to udev.
This is the CONFIG_FW_LOADER_USER_HELPER config which makes that now, right ?
But how can I have /lib/firmware in my initrd, that is the question I
need to answer now :).

JM

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Takashi Iwai]

At Wed, 6 Aug 2014 13:24:37 +0200,
Jean-Michel Hautbois wrote:
> 
> 2014-08-06 12:52 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> > At Wed, 6 Aug 2014 12:50:28 +0200,
> > Jean-Michel Hautbois wrote:
> >>
> >> 2014-08-06 12:21 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> >> > At Wed, 6 Aug 2014 11:44:14 +0200,
> >> > Jean-Michel Hautbois wrote:
> >> >>
> >> >> 2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> >> >> > At Wed, 6 Aug 2014 11:10:27 +0200,
> >> >> > Jean-Michel Hautbois wrote:
> >> >> >>
> >> >> >> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
> >> >> >> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >> >> >> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
> >> >> >> >>
> >> >> >> >
> >> >> >> > ok it works when the firmware is in /lib/firmware. It sounds to me the
> >> >> >> > reason load fails when the firmware is under /sys/class/firmware is
> >> >> >> > fw_load_from_user_helper() returns -ENOENT when
> >> >> >> > CONFIG_FW_LOADER_USER_HELPER is disabled.
> >> >> >> >
> >> >> >> > It would be nice to see the entire dmesg with debug enabled though.
> >> >> >> >
> >> >> >> > -- Shuah
> >> >> >>
> >> >> >> It does not work with the 3.16 kernel even when firmware is in
> >> >> >> /lib/firmware, it worked before this patch is applied.
> >> >> >
> >> >> > Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
> >> >>
> >> >> Oh, you are right of course, I am on upstream kernel and I have your
> >> >> patch. I don't mean your match is causing the issue though ;-).
> >> >
> >> > I see.
> >> >
> >> >> >> Here is what I got in my dmesg :
> >> >> >> [    0.307856] __allocate_fw_buf: fw-lattice-ecp3.bit buf=bd81a480
> >> >> >> [    0.308029] lattice-ecp3 spi2.3: Direct firmware load for
> >> >> >> lattice-ecp3.bit failed with error -2
> >> >> >
> >> >> > It's -ENOENT.  Are you sure that you really have
> >> >> >  /lib/firmware/lattice-ecp3.bit file?
> >> >>
> >> >> Yes :
> >> >> ls -al /lib/firmware/lattice-ecp3.bit
> >> >> -rwxr-xr-x 1 root root 897753 Aug  5 15:04 /lib/firmware/lattice-ecp3.bit
> >> >
> >> > Then you'd better bisect...
> >> > Or, at least, put a debug code to see which file is opened in the loop
> >> > in fw_get_filesystem_firmware().
> >>
> >> Well, when this function is called, my rootfs is not yet mounted (I
> >> have a dedicated partition on a SDCard)...
> >> So I probably missed a step in order to get /lib/firmware before / is
> >> mounted... ?
> >
> > Yeah, possibly :)  Maybe you did put /lib/firmware/* into initrd in
> > the earlier tests?
> 
> Nope, it failed and when / was mounted, it was called again and used
> the firmware thanks to udev.
> This is the CONFIG_FW_LOADER_USER_HELPER config which makes that now, right ?

Not really.  The user helper is invoked when
CONFIG_FW_LOADER_USER_HELPER_FALLBACK is set.  The exception is the
case of non-udev f/w handling like dell-rbu (which passes
FW_ACTION_NOHOTPLUG).  Then the user mode helper is used as fallback
even if CONFIG_FW_LOADER_USER_HELPER_FALLBACK isn't set.

> But how can I have /lib/firmware in my initrd, that is the question I
> need to answer now :).

Pretty depends on how to create initrd.  But most programs look up the
MODULE_FIRMWARE() entries of the modules to be loaded in initrd and
put them automatically.  And it's missing in lattice-ecp3-config
driver...


Takashi

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Shuah Khan]

On Wed, Aug 6, 2014 at 3:44 AM, Jean-Michel Hautbois
<jhautbois@gmail.com> wrote:
> 2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
>> At Wed, 6 Aug 2014 11:10:27 +0200,
>> Jean-Michel Hautbois wrote:
>>>
>>> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
>>> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
>>> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
>>> >>
>>> >
>>> > ok it works when the firmware is in /lib/firmware. It sounds to me the
>>> > reason load fails when the firmware is under /sys/class/firmware is
>>> > fw_load_from_user_helper() returns -ENOENT when
>>> > CONFIG_FW_LOADER_USER_HELPER is disabled.
>>> >
>>> > It would be nice to see the entire dmesg with debug enabled though.
>>> >
>>> > -- Shuah
>>>
>>> It does not work with the 3.16 kernel even when firmware is in
>>> /lib/firmware, it worked before this patch is applied.
>>
>> Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
>
> Oh, you are right of course, I am on upstream kernel and I have your
> patch. I don't mean your match is causing the issue though ;-).
>

I think this is what is going on and this patch is the cause:
fw_load_from_user_helper() is a stub that returns -ENOENT with this
patch. As a result, in _request_firmware() after fw_get_filesystem_firmware()
fails to find the file, fw_load_from_user_helper() gets called and it
returns right
away with -ENOENT.

In some cases if rootfs mount is in progress, fw_load_from_user_helper()
steps into load the firmware.

A quick way to test is by adding a dev_info() in the fw_load_from_user_helper()
and see if that is path it is taking.

-- Shuah

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Takashi Iwai]

At Wed, 6 Aug 2014 13:00:24 -0600,
Shuah Khan wrote:
> 
> On Wed, Aug 6, 2014 at 3:44 AM, Jean-Michel Hautbois
> <jhautbois@gmail.com> wrote:
> > 2014-08-06 11:24 GMT+02:00 Takashi Iwai <tiwai@suse.de>:
> >> At Wed, 6 Aug 2014 11:10:27 +0200,
> >> Jean-Michel Hautbois wrote:
> >>>
> >>> 2014-08-05 21:22 GMT+02:00 Shuah Khan <shuahkhan@gmail.com>:
> >>> > On Tue, Aug 5, 2014 at 10:01 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >>> >>> Yes, it worked, if the firmware was in the /lib/firmware directory.
> >>> >>
> >>> >
> >>> > ok it works when the firmware is in /lib/firmware. It sounds to me the
> >>> > reason load fails when the firmware is under /sys/class/firmware is
> >>> > fw_load_from_user_helper() returns -ENOENT when
> >>> > CONFIG_FW_LOADER_USER_HELPER is disabled.
> >>> >
> >>> > It would be nice to see the entire dmesg with debug enabled though.
> >>> >
> >>> > -- Shuah
> >>>
> >>> It does not work with the 3.16 kernel even when firmware is in
> >>> /lib/firmware, it worked before this patch is applied.
> >>
> >> Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
> >
> > Oh, you are right of course, I am on upstream kernel and I have your
> > patch. I don't mean your match is causing the issue though ;-).
> >
> 
> I think this is what is going on and this patch is the cause:
> fw_load_from_user_helper() is a stub that returns -ENOENT with this
> patch.

... unless specifying the new Kconfig
CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.  This is exactly the purpose
of the commit.

> As a result, in _request_firmware() after fw_get_filesystem_firmware()
> fails to find the file, fw_load_from_user_helper() gets called and it
> returns right
> away with -ENOENT.
> 
> In some cases if rootfs mount is in progress, fw_load_from_user_helper()
> steps into load the firmware.

When a module in initrd requires a firmware, it should be put in
initrd, too.


thanks,

Takashi

Re: patch "firmware loader: allow disabling of udev as firmware loader" added to driver-core tree

[Shuah Khan]

On Thu, Aug 7, 2014 at 1:57 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> >> Hm?  3.16 doesn't contain my patch yet.  It's merged for 3.17-rc1.
>> >
>> > Oh, you are right of course, I am on upstream kernel and I have your
>> > patch. I don't mean your match is causing the issue though ;-).
>> >
>>
>> I think this is what is going on and this patch is the cause:
>> fw_load_from_user_helper() is a stub that returns -ENOENT with this
>> patch.
>
> ... unless specifying the new Kconfig
> CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.  This is exactly the purpose
> of the commit.

Right - I figured that is the intent.

>
>> As a result, in _request_firmware() after fw_get_filesystem_firmware()
>> fails to find the file, fw_load_from_user_helper() gets called and it
>> returns right
>> away with -ENOENT.
>>
>> In some cases if rootfs mount is in progress, fw_load_from_user_helper()
>> steps into load the firmware.
>
> When a module in initrd requires a firmware, it should be put in
> initrd, too.

Right. This is something drivers have to watch out for with this change.

-- Shuah