Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1642117 > unrolled thread

Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-16 01:20 +0200
    Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Arend Van Spriel <arend.vanspriel@broadcom.com> - 2017-05-16 10:50 +0200
      Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-17 02:00 +0200
    Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Johannes Berg <johannes@sipsolutions.net> - 2017-05-17 14:10 +0200
      Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-05-17 15:00 +0200
        Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Johannes Berg <johannes@sipsolutions.net> - 2017-05-17 15:10 +0200
          Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Pali Rohár <pali.rohar@gmail.com> - 2017-05-17 15:30 +0200
            Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Johannes Berg <johannes@sipsolutions.net> - 2017-05-17 15:30 +0200
        Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for  loading NVS calibration data Bjorn Andersson <bjorn.andersson@linaro.org> - 2017-05-18 02:10 +0200

#1642117 — Re: [PATCH 2/6] wl1251: Use request_firmware_prefer_user() for loading NVS calibration data

On Fri, May 12, 2017 at 11:02:26PM +0200, Arend Van Spriel wrote:
> try again.. replacing email address from Michał
> On 12-5-2017 22:55, Arend Van Spriel wrote:
> > Let me explain the idea to refresh your memory (and mine). It started
> > when we were working on adding driver support for OpenWrt in brcmfmac.
> > The driver requests for firmware calibration data, but on routers it is
> > stored in flash. So after failing on the firmware request we now call a
> > platform specific API. That was my itch, but it was not bad enough to go
> > and scratch. Now for N900 case there is a similar scenario alhtough it
> > has additional requirement to go to user-space due to need to use a
> > proprietary library to obtain the NVS calibration data. My thought: Why
> > should firmware_class care?

Agreed.

> > So the idea is that firmware_class provides
> > a registry for modules that can produce a certain firmware "file". Those
> > modules can do whatever is needed. If they need to use umh so be it.
> > They would only register themselves with firmware_class on platforms
> > that need them. It would basically be replacing the fallback mechanism
> > and only be effective on certain platforms.

Sure, so it sounds like the work that Daniel Wagner and Tom Gundersen worked
[0] on which provides a firmwared with two modes: best-effort, and final-mode,
would address what you are looking for but without requiring any upstream
changes, *and* it also helps solve the rootfs race remote-proc folks had
concerns over.

The other added gain over this solution is if folks need their own proprietary
concoction they can just fork firmwared and have that do whatever it needs
for the specific device on the specific rootfs. That is, firmwared can be the
upstream solution if folks need it, but if folks need something custom they can
just mimic the implementation: best-effort, and and final-mode.

Yet another added gain over this solution we can do *not* support the
custom fallback mechanism as its not needed, the udev event should suffice
to let userspace do what it needs.

Lastly, if we did not want to deal with timeouts for the way the driver data
API implements it I think we might be able to do away with them for for async
requests if we assume there will be a daemon that spawns in final-mode eventually,
and since it *knows* when the rootfs is ready it should be able to do a final
lookup, if it returns -ENOENT; then indeed we know we can give up. Now, perhaps
how and if we want to deal with timeouts when using the driver data API for
the fallback mechanism is worth considering given it does not have a fallback
mechanism support yet. If we *add* them it would seem this would also put an
implicit race against userspace finishing initialization and running firmwared
in final-mode.

Johannes, do you recall the corner cases we spoke about regarding timeouts?
Does this match what we spoke about?

> > Let me know if this idea is still of interest and I will rebase what I
> > have for an RFC round.

Since no upstream delta is needed for firmwared I'd like to first encourage
evaluating the above. While distributions don't carry it yet that may be seen as
an issue but since what we are looking for are corner cases, only folks needing
to deploy a specific solution would need it or a custom proprietary solution.

[0] https://github.com/teg/firmwared.git

PS.

Note that firmware signing will require an additional file, the detached
signature. The driver data API does not currently support the fallback
mechanism so we would not have to worry about that yet but once we add
fallback support we'd need to consider this.

  Luis

[toc] | [next] | [standalone]

#1642314

On 16-5-2017 1:13, Luis R. Rodriguez wrote:
> On Fri, May 12, 2017 at 11:02:26PM +0200, Arend Van Spriel wrote:
>> try again.. replacing email address from Michał
>> On 12-5-2017 22:55, Arend Van Spriel wrote:
>>> Let me explain the idea to refresh your memory (and mine). It started
>>> when we were working on adding driver support for OpenWrt in brcmfmac.
>>> The driver requests for firmware calibration data, but on routers it is
>>> stored in flash. So after failing on the firmware request we now call a
>>> platform specific API. That was my itch, but it was not bad enough to go
>>> and scratch. Now for N900 case there is a similar scenario alhtough it
>>> has additional requirement to go to user-space due to need to use a
>>> proprietary library to obtain the NVS calibration data. My thought: Why
>>> should firmware_class care?
> 
> Agreed.
> 
>>> So the idea is that firmware_class provides
>>> a registry for modules that can produce a certain firmware "file". Those
>>> modules can do whatever is needed. If they need to use umh so be it.
>>> They would only register themselves with firmware_class on platforms
>>> that need them. It would basically be replacing the fallback mechanism
>>> and only be effective on certain platforms.
> 
> Sure, so it sounds like the work that Daniel Wagner and Tom Gundersen worked
> [0] on which provides a firmwared with two modes: best-effort, and final-mode,
> would address what you are looking for but without requiring any upstream
> changes, *and* it also helps solve the rootfs race remote-proc folks had
> concerns over.
> 
> The other added gain over this solution is if folks need their own proprietary
> concoction they can just fork firmwared and have that do whatever it needs
> for the specific device on the specific rootfs. That is, firmwared can be the
> upstream solution if folks need it, but if folks need something custom they can
> just mimic the implementation: best-effort, and and final-mode.
> 
> Yet another added gain over this solution we can do *not* support the
> custom fallback mechanism as its not needed, the udev event should suffice
> to let userspace do what it needs.
> 
> Lastly, if we did not want to deal with timeouts for the way the driver data
> API implements it I think we might be able to do away with them for for async
> requests if we assume there will be a daemon that spawns in final-mode eventually,
> and since it *knows* when the rootfs is ready it should be able to do a final
> lookup, if it returns -ENOENT; then indeed we know we can give up. Now, perhaps
> how and if we want to deal with timeouts when using the driver data API for
> the fallback mechanism is worth considering given it does not have a fallback
> mechanism support yet. If we *add* them it would seem this would also put an
> implicit race against userspace finishing initialization and running firmwared
> in final-mode.

Just to be clear. When you are saying "rootfs" in this story, you mean
any (mounted) file-system which may hold the firmware. At least that was
one of the arguments. In kernel space we can not know how the system is
setup in terms of mount points, let alone on which mounted file-system
the firmware resides.

> Johannes, do you recall the corner cases we spoke about regarding timeouts?
> Does this match what we spoke about?
> 
>>> Let me know if this idea is still of interest and I will rebase what I
>>> have for an RFC round.
> 
> Since no upstream delta is needed for firmwared I'd like to first encourage
> evaluating the above. While distributions don't carry it yet that may be seen as
> an issue but since what we are looking for are corner cases, only folks needing
> to deploy a specific solution would need it or a custom proprietary solution.

Ok. I will go try and run firmwared in OpenWrt on a router platform.
Have to steal one from a colleague :-p Will study firmwared.

> [0] https://github.com/teg/firmwared.git
> 
> PS.
> 
> Note that firmware signing will require an additional file, the detached
> signature. The driver data API does not currently support the fallback
> mechanism so we would not have to worry about that yet but once we add
> fallback support we'd need to consider this.

Do you have references to the firmware signing design. Is the idea to
have one signature and all "firmware files" need to be signed with it?

Thanks,
Arend

[toc] | [prev] | [next] | [standalone]

#1642897

On Tue, May 16, 2017 at 10:41:08AM +0200, Arend Van Spriel wrote:
> On 16-5-2017 1:13, Luis R. Rodriguez wrote:
> > On Fri, May 12, 2017 at 11:02:26PM +0200, Arend Van Spriel wrote:
> >> try again.. replacing email address from Michał
> >> On 12-5-2017 22:55, Arend Van Spriel wrote:
> >>> Let me explain the idea to refresh your memory (and mine). It started
> >>> when we were working on adding driver support for OpenWrt in brcmfmac.
> >>> The driver requests for firmware calibration data, but on routers it is
> >>> stored in flash. So after failing on the firmware request we now call a
> >>> platform specific API. That was my itch, but it was not bad enough to go
> >>> and scratch. Now for N900 case there is a similar scenario alhtough it
> >>> has additional requirement to go to user-space due to need to use a
> >>> proprietary library to obtain the NVS calibration data. My thought: Why
> >>> should firmware_class care?
> > 
> > Agreed.
> > 
> >>> So the idea is that firmware_class provides
> >>> a registry for modules that can produce a certain firmware "file". Those
> >>> modules can do whatever is needed. If they need to use umh so be it.
> >>> They would only register themselves with firmware_class on platforms
> >>> that need them. It would basically be replacing the fallback mechanism
> >>> and only be effective on certain platforms.
> > 
> > Sure, so it sounds like the work that Daniel Wagner and Tom Gundersen worked
> > [0] on which provides a firmwared with two modes: best-effort, and final-mode,
> > would address what you are looking for but without requiring any upstream
> > changes, *and* it also helps solve the rootfs race remote-proc folks had
> > concerns over.
> > 
> > The other added gain over this solution is if folks need their own proprietary
> > concoction they can just fork firmwared and have that do whatever it needs
> > for the specific device on the specific rootfs. That is, firmwared can be the
> > upstream solution if folks need it, but if folks need something custom they can
> > just mimic the implementation: best-effort, and and final-mode.
> > 
> > Yet another added gain over this solution we can do *not* support the
> > custom fallback mechanism as its not needed, the udev event should suffice
> > to let userspace do what it needs.
> > 
> > Lastly, if we did not want to deal with timeouts for the way the driver data
> > API implements it I think we might be able to do away with them for for async
> > requests if we assume there will be a daemon that spawns in final-mode eventually,
> > and since it *knows* when the rootfs is ready it should be able to do a final
> > lookup, if it returns -ENOENT; then indeed we know we can give up. Now, perhaps
> > how and if we want to deal with timeouts when using the driver data API for
> > the fallback mechanism is worth considering given it does not have a fallback
> > mechanism support yet. If we *add* them it would seem this would also put an
> > implicit race against userspace finishing initialization and running firmwared
> > in final-mode.
> 
> Just to be clear. When you are saying "rootfs" in this story, you mean
> any (mounted) file-system which may hold the firmware. At least that was
> one of the arguments. In kernel space we can not know how the system is
> setup in terms of mount points, let alone on which mounted file-system
> the firmware resides.

Right, wherever the hell that thing is on, which could be on a crypic fuse
drive waiting for some bits to be decrypted from Elon Musk on a spaceship on his
way to Mars, and only userspace knows how to decrypt this thing through some
evil proprietary thing, way way after a full bootup.

> > Johannes, do you recall the corner cases we spoke about regarding timeouts?
> > Does this match what we spoke about?
> > 
> >>> Let me know if this idea is still of interest and I will rebase what I
> >>> have for an RFC round.
> > 
> > Since no upstream delta is needed for firmwared I'd like to first encourage
> > evaluating the above. While distributions don't carry it yet that may be seen as
> > an issue but since what we are looking for are corner cases, only folks needing
> > to deploy a specific solution would need it or a custom proprietary solution.
> 
> Ok. I will go try and run firmwared in OpenWrt on a router platform.
> Have to steal one from a colleague :-p Will study firmwared.

The finale-mode is the trick.

> > [0] https://github.com/teg/firmwared.git
> > 
> > PS.
> > 
> > Note that firmware signing will require an additional file, the detached
> > signature. The driver data API does not currently support the fallback
> > mechanism so we would not have to worry about that yet but once we add
> > fallback support we'd need to consider this.
> 
> Do you have references to the firmware signing design. Is the idea to
> have one signature and all "firmware files" need to be signed with it?

Nope, I'm afraid a lot has been discussed so best we can do is wait.

 Luis

[toc] | [prev] | [next] | [standalone]

#1643301

On Tue, 2017-05-16 at 01:13 +0200, Luis R. Rodriguez wrote:
> > > Now for N900 case there is a similar scenario
> > > alhtough it has additional requirement to go to user-space due to
> > > need to use a proprietary library to obtain the NVS calibration
> > > data. My thought: Why should firmware_class care?
> 
> Agreed.

In fact, why should the *driver* care either? IOW - why should
"request_firmware_prefer_user()" even exist?

> > > So the idea is that firmware_class provides
> > > a registry for modules that can produce a certain firmware
> > > "file". Those
> > > modules can do whatever is needed. If they need to use umh so be
> > > it.
> > > They would only register themselves with firmware_class on
> > > platforms
> > > that need them. It would basically be replacing the fallback
> > > mechanism
> > > and only be effective on certain platforms.
> 
> Sure, so it sounds like the work that Daniel Wagner and Tom Gundersen
> worked [0] on which provides a firmwared with two modes: best-effort, 
> and final-mode, would address what you are looking for but without
> requiring any upstream changes, *and* it also helps solve the rootfs
> race remote-proc folks had concerns over.

Right.

> The other added gain over this solution is if folks need their own
> proprietary concoction they can just fork firmwared

Or just reimplement it to the same kernel API, no need to even use the
same code base.

> Lastly, if we did not want to deal with timeouts for the way the
> driver data API implements it I think we might be able to do away
> with them for for async requests if we assume there will be a daemon
> that spawns in final-mode eventually, and since it *knows* when the
> rootfs is ready it should be able to do a final lookup, if it returns
> -ENOENT; then indeed we know we can give up. Now, perhaps how and if
> we want to deal with timeouts when using the driver data API for the
> fallback mechanism is worth considering given it does not have a
> fallback mechanism support yet. If we *add* them it would seem this
> would also put an implicit race against userspace finishing
> initialization and running firmwared in final-mode.
> 
> Johannes, do you recall the corner cases we spoke about regarding
> timeouts? Does this match what we spoke about?

I think we have to protect against userspace code crashing, not
existing, etc. - so I think we do need a timeout anyway.

However, I don't recall any (other) corner cases we might have spoken
about.

> Note that firmware signing will require an additional file, the
> detached signature.

Is anything like that happening finally? :)

johannes
> 

[toc] | [prev] | [next] | [standalone]

#1643342

On Wednesday 17 May 2017 14:06:06 Johannes Berg wrote:
> On Tue, 2017-05-16 at 01:13 +0200, Luis R. Rodriguez wrote:
> > > > Now for N900 case there is a similar scenario
> > > > alhtough it has additional requirement to go to user-space due to
> > > > need to use a proprietary library to obtain the NVS calibration
> > > > data. My thought: Why should firmware_class care?
> > 
> > Agreed.
> 
> In fact, why should the *driver* care either? IOW - why should
> "request_firmware_prefer_user()" even exist?

There are default/example NVS data, which are stored in /lib/firmware
and installed by linux-firmware package. Those example calibration data
should not be used for real usage, but Pavel told us that on N900 they
are enough for working WIFI connection. They does not contain valid MAC
address, so kernel should generate some (random?).

So kernel driver should get NVS calibration data from userspace (which
know how where to get or how to prepare them) and in case userspace do
not have it, then we can try fallback to those example data (as people
reported us they can be useful instead of non-working WIFI).

And that fallback is working by direct firmware loading from kernel.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1643348

On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote:

> > In fact, why should the *driver* care either? IOW - why should
> > "request_firmware_prefer_user()" even exist?
> 
> There are default/example NVS data, which are stored in /lib/firmware
> and installed by linux-firmware package.
[...]

Oh, so you're saying you want this to invert the order ... Ok, that
makes some sense.

I still hope that all other requests will eventually fall back to user
loading though, I think that's important to system integration in
general.

johannes

[toc] | [prev] | [next] | [standalone]

#1643360

On Wednesday 17 May 2017 15:04:50 Johannes Berg wrote:
> On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote:
> 
> > > In fact, why should the *driver* care either? IOW - why should
> > > "request_firmware_prefer_user()" even exist?
> > 
> > There are default/example NVS data, which are stored in /lib/firmware
> > and installed by linux-firmware package.
> [...]
> 
> Oh, so you're saying you want this to invert the order ... Ok, that
> makes some sense.

Yes! I thought that this fact can be understood from commit message. If
not, I can change it, but provide how to improve it.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1643361

On Wed, 2017-05-17 at 15:21 +0200, Pali Rohár wrote:
> On Wednesday 17 May 2017 15:04:50 Johannes Berg wrote:
> > On Wed, 2017-05-17 at 14:53 +0200, Pali Rohár wrote:
> > 
> > > > In fact, why should the *driver* care either? IOW - why should
> > > > "request_firmware_prefer_user()" even exist?
> > > 
> > > There are default/example NVS data, which are stored in
> > > /lib/firmware
> > > and installed by linux-firmware package.
> > 
> > [...]
> > 
> > Oh, so you're saying you want this to invert the order ... Ok, that
> > makes some sense.
> 
> Yes! I thought that this fact can be understood from commit message.
> If not, I can change it, but provide how to improve it.

It probably can, I was only Cc'ed later :)

Sorry for the noise.

johannes

[toc] | [prev] | [next] | [standalone]

#1643709

On Wed 17 May 05:53 PDT 2017, Pali Roh?r wrote:

> On Wednesday 17 May 2017 14:06:06 Johannes Berg wrote:
> > On Tue, 2017-05-16 at 01:13 +0200, Luis R. Rodriguez wrote:
> > > > > Now for N900 case there is a similar scenario
> > > > > alhtough it has additional requirement to go to user-space due to
> > > > > need to use a proprietary library to obtain the NVS calibration
> > > > > data. My thought: Why should firmware_class care?
> > > 
> > > Agreed.
> > 
> > In fact, why should the *driver* care either? IOW - why should
> > "request_firmware_prefer_user()" even exist?
> 
> There are default/example NVS data, which are stored in /lib/firmware
> and installed by linux-firmware package. Those example calibration data
> should not be used for real usage, but Pavel told us that on N900 they
> are enough for working WIFI connection. They does not contain valid MAC
> address, so kernel should generate some (random?).
> 
> So kernel driver should get NVS calibration data from userspace (which
> know how where to get or how to prepare them) and in case userspace do
> not have it, then we can try fallback to those example data (as people
> reported us they can be useful instead of non-working WIFI).
> 

We're going to see a similar case with the Qualcomm DB410c WiFi soon,
where there is default calibration for the chip (wcn3620) but specific
calibration data for the particular board or product using this chip.

As with your case we expect to have a "generic" calibration file
integrated in linux-firmware, but providing means to supporting
device-specific calibration is probably going to be requested shortly.


We have however altered the reference design of picking the MAC address
from the calibration data and have the bootloader pass it via DT - so
our calibration data doesn't need to be specific to each unit.

Regards,
Bjorn

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web