Messages in this thread | | | Subject | Re: [PATCH v9 1/5] firmware: add extensible driver data params | From | Rafał Miłecki <> | Date | Tue, 13 Jun 2017 16:12:17 +0200 |
| |
On 06/13/2017 03:17 PM, Greg KH wrote: > On Tue, Jun 13, 2017 at 12:31:04PM +0200, Rafał Miłecki wrote: >> On 2017-06-13 11:05, Greg KH wrote: >>> On Mon, Jun 05, 2017 at 02:39:33PM -0700, Luis R. Rodriguez wrote: >>>> As the firmware API evolves we keep extending functions with more >>>> arguments. >>>> Stop this nonsense by proving an extensible data structure which can >>>> be used >>>> to represent both user parameters and private internal parameters. >>> >>> Let's take a simple C function interface and make it a more complex >>> data-driven interface that is impossible to understand and obviously >>> understand how it is to be used and works! >>> >>> :( >>> >>> Seriously, why? Why are we extending any of this at all? This series >>> adds a ton of new "features" and complexity, but for absolutely no gain. >>> >>> Oh, I take it back, you removed 29 lines from the iwlwifi driver. >>> >>> That's still not worth it at all, you have yet to sell me on this whole >>> complex beast. I can't see why we need it, and if I, one of the few >>> people who thinks they actually understand this kernel interface, can't >>> see it, how can you sell it to someone else? >>> >>> Sorry, but no, I'm still not going to take this series until you show >>> some _REAL_ benefit for it. >> >> FWIW I saw (or maybe still see?) a need to extend request_firmware* API to >> allow silencing a warning if firmware file is missing. >> >> I even sent a trivial patch adding support for this: >> [PATCH V4 1/2] firmware: add more flexible request_firmware_async function >> https://patchwork.kernel.org/patch/9588787/ >> (I think it still applies) but it got rejected due to Luis's big rework. > > Can you resend this series if it still does apply?
Sure, if you think it's worth trying, I'll do that!
> And what exact warning is this silencing? Normally we want the warning > there, as that implies that something is wrong if the firmware file that > a driver is asking for is not present. That way the user can know to go > fix it up, right?
It's because brcmfmac looks for NVRAM in two places: /lib/firmware/ and platform NVRAM. It's supposed to silence [ 10.801506] brcmfmac 0000:01:00.0: Direct firmware load for brcm/brcmfmac43602-pcie.txt failed with error -2 in case there is platform NVRAM present.
For more details please take a look at: [PATCH V4 2/2] brcmfmac: don't warn user about NVRAM if fallback to platform one succeeds https://patchwork.kernel.org/patch/9588791/
| |