| Message ID | 20190107113401.6824-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | brcmfmac: Use request_firmware_direct for the clm_blob | expand |
Hans de Goede <hdegoede@redhat.com> writes: > The linux-firmware brcmfmac firmware files contain an embedded table with > per country allowed channels and strength info. > > These versions of the firmware are specially build for linux-firmware, > the firmware files directly available from Broadcom / Cypress rely on > a separate clm_blob file for this info. > > For some unknown reason Broadcom / Cypress refuse to provide the standard > firmware files + clm_blob files it uses elsewhere for inclusion into > linux-firmware, instead relying on these special builds with the clm_blob > info embedded. This means that the linux-firmware firmware versions often > lag behind, but I digress. > > The brcmfmac driver does support the separate clm_blob file and always > tries to load this. Currently we use request_firmware for this. This means > that on any standard install, using the standard combo of linux-kernel + > linux-firmware, we will get a warning: > "Direct firmware load for ... failed with error -2" > > On top of this, brcmfmac itself prints: "no clm_blob available (err=-2), > device may have limited channels available" and we will get a slow > fallback to the userspace firmware loading mechanism. > > This commit fixes both almost any brcmfmac device logging the warning > (leaving the brcmfmac info message in pace), as well as the slow and > unnecesary fallback by switching to request_firmware_direct for > the clm_blob. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c > index 0bb16bf574e3..e0a5e78ee437 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c > @@ -149,7 +149,7 @@ static int brcmf_c_process_clm_blob(struct brcmf_if *ifp) > return err; > } > > - err = request_firmware(&clm, clm_name, bus->dev); > + err = request_firmware_direct(&clm, clm_name, bus->dev); If you just want to avoid the warning shouldn't this be firmware_request_nowarn()? On ath10k we also did the workaround using _direct() variant but that caused problems with openwrt.
On 1/7/2019 12:34 PM, Hans de Goede wrote: > The linux-firmware brcmfmac firmware files contain an embedded table with > per country allowed channels and strength info. > > These versions of the firmware are specially build for linux-firmware, > the firmware files directly available from Broadcom / Cypress rely on > a separate clm_blob file for this info. Hi Hans, It is a bit more subtle than how you put it here. It is more of an historical thing. The table used to be embedded in firmware only. Much later the clm_blob loading functionality was added so customers could get an updated blob file while using the same firmware. In our router business we still provide firmwares with embedded table. Cypress decided to move to a model in which the firmware contains a null table and needs clm_blob to get things going. > For some unknown reason Broadcom / Cypress refuse to provide the standard > firmware files + clm_blob files it uses elsewhere for inclusion into > linux-firmware, instead relying on these special builds with the clm_blob > info embedded. This means that the linux-firmware firmware versions often > lag behind, but I digress. Most of them are not special builds and provided to AOSP as well. > The brcmfmac driver does support the separate clm_blob file and always > tries to load this. Currently we use request_firmware for this. This means > that on any standard install, using the standard combo of linux-kernel + > linux-firmware, we will get a warning: > "Direct firmware load for ... failed with error -2" > > On top of this, brcmfmac itself prints: "no clm_blob available (err=-2), > device may have limited channels available" and we will get a slow > fallback to the userspace firmware loading mechanism. > > This commit fixes both almost any brcmfmac device logging the warning > (leaving the brcmfmac info message in pace), as well as the slow and 'pace' should probably be 'place' here. > unnecesary fallback by switching to request_firmware_direct for > the clm_blob. As Kalle mentioned it is probably better to use the 'nowarn' api. Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)
Hi, On 07-01-19 12:58, Arend Van Spriel wrote: > On 1/7/2019 12:34 PM, Hans de Goede wrote: >> The linux-firmware brcmfmac firmware files contain an embedded table with >> per country allowed channels and strength info. >> >> These versions of the firmware are specially build for linux-firmware, >> the firmware files directly available from Broadcom / Cypress rely on >> a separate clm_blob file for this info. > > Hi Hans, > > It is a bit more subtle than how you put it here. It is more of an historical thing. I know this started as a historical thing and for some of the older firmwares, the info being embedded is indeed simply a legacy thing. > The table used to be embedded in firmware only. Much later the clm_blob loading functionality was added so customers could get an updated blob file while using the same firmware. In our router business we still provide firmwares with embedded table. Cypress decided to move to a model in which the firmware contains a null table and needs clm_blob to get things going. Cypress is actually doing both the old (embedded info) and the new (separate clm_blob) model for the firmwares which they are now maintaining, they are still providing updates *with the embedded info, see e.g.: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/brcm?id=253a573936ee2078d206527f3ae845b4dc681269 Where as in their own firmware-distribution: https://community.cypress.com/docs/DOC-15932 "Cypress Linux WiFi Driver Release (FMAC) [2018-09-28]" Cypress does the null-table + separate clm_blob file thing and for reasons which they have never explained they insist on the linux-firmware files and their own files being different. The zip file from DOC-15932 link contains a "cypress-firmware-v4.14.52-2018_0928.tar.gz" which contains LICENCE.cypress from before this commit: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit?id=ba51e861f4444f51e7e83f778575a8146dc514d which means it has a troublesome termination clause and a README which says: " * This LICENCE is different from the LICENCE.Cypress on linux-firmware.git. * The files under this licence are not intended for linux-firmware.git upstreaming. " This also means that linux-firmware is always lagging in firmware version, missing out on bug and SECURITY fixes, e.g. Looking at the included brcmfmac4356-pcie.bin file, linux-firmware has version 7.35.180.176. Where as cypress-firmware-v4.14.52-2018_0928.tar.gz made available from Cypress directly has a newer version, quoting from their changelog: * 4356-pcie * --- 7.35.180.190 --- * Firmware crash fix * MFP bug fix A "Firmware crash fix" to me sounds like a potential security issue, yet Cypress is deliberately blocking us from distributing the new version. Another example is the firmware for the wifi on the Raspberry Pi 3B+ which is *years* older in linux-firmware. I must say these whole shenanigans with the firmware causing linux-firmware to have way too old firmware versions makes me very unhappy. I'm at the point where I'm telling friends to not buy any hardware with Cypress wifi in there because of this and because of the *complete* lack of bluetooth firmware. >> For some unknown reason Broadcom / Cypress refuse to provide the standard >> firmware files + clm_blob files it uses elsewhere for inclusion into >> linux-firmware, instead relying on these special builds with the clm_blob >> info embedded. This means that the linux-firmware firmware versions often >> lag behind, but I digress. > > Most of them are not special builds and provided to AOSP as well. They are not the same builds as Cypress is distributing in their SDK and they are generally anywhere between a bit and a lot older, so to me these seem to be special builds and not the preferred form of firmware, otherwise Cypress would be distributing the same builds in their SDK. Anyways except for the legacy cases seems to all be about how Cypress is distributing the firmware, so I will drop the Broadcom reference in this paragraph for v2 of this patch. >> The brcmfmac driver does support the separate clm_blob file and always >> tries to load this. Currently we use request_firmware for this. This means >> that on any standard install, using the standard combo of linux-kernel + >> linux-firmware, we will get a warning: >> "Direct firmware load for ... failed with error -2" >> >> On top of this, brcmfmac itself prints: "no clm_blob available (err=-2), >> device may have limited channels available" and we will get a slow >> fallback to the userspace firmware loading mechanism. >> >> This commit fixes both almost any brcmfmac device logging the warning >> (leaving the brcmfmac info message in pace), as well as the slow and > > 'pace' should probably be 'place' here. Right, will fix. >> unnecesary fallback by switching to request_firmware_direct for >> the clm_blob. > > As Kalle mentioned it is probably better to use the 'nowarn' api. Ok will also fix for v2. Regards, Hans > > Acked-by: Arend van Spriel <arend.vanspriel@broadcom.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-)
(+John) On 01/07/2019 9:05, Hans de Goede wrote: > Hi, > > On 07-01-19 12:58, Arend Van Spriel wrote: >> On 1/7/2019 12:34 PM, Hans de Goede wrote: > > Cypress is actually doing both the old (embedded info) and the new > (separate clm_blob) model for the firmwares which they are now maintaining, > they are still providing updates *with the embedded info, see e.g.: > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/brcm?id=253a573936ee2078d206527f3ae845b4dc681269 Hi Hans, 43362 is a very old chip that it doesn't support the clm_blob model. For newer chips (some in newer builds) we use clm_blob by default. The clm_blob should be an optional file and the desired behavior is to fall through even when the file is not there. As Kalle and Arend mentioned, using request_firmware_nowarn() is more appropriate. Thanks for posting this enhancement. > Cypress does the null-table + separate clm_blob file thing and for reasons > which they have never explained they insist on the linux-firmware files and > their own files being different. The binary files in linux-firmware are taken from the release packages. The latest binary with embedded clm (historical mode) were upstreamed. Will explain more about the clm_blobs in the V2 comment. > A "Firmware crash fix" to me sounds like a potential security issue, yet It is a functional bug that causes wifi to stop working correctly. Not a security hole. We did upstream firmware for security issues. > Another example is the firmware for the wifi on the Raspberry Pi 3B+ which > is *years* older in linux-firmware. Could you please specify the source (link) of the firmware you're referring to? > > I must say these whole shenanigans with the firmware causing > linux-firmware to > have way too old firmware versions makes me very unhappy. I'm at the point > where I'm telling friends to not buy any hardware with Cypress wifi in > there > because of this and because of the *complete* lack of bluetooth firmware. We did work with HTC to upstream their bluetooth firmware to linux-firmware, although I admit that most of the bluetooth firmware are only available through hardware vendors. Bluetooth firmware files are specific to vendor board designs. When there is an update, the hardware (device/board) vendor qualifies the firmware and release through their preferred channel (in HTC's case they chose to upstream it). Regards, Chi-hsien Lin
Hi, On 10-01-19 09:16, Chi-Hsien Lin wrote: >> A "Firmware crash fix" to me sounds like a potential security issue, yet > > It is a functional bug that causes wifi to stop working correctly. Not a > security hole. We did upstream firmware for security issues. Even if this is just a functional bug it would be still nice to also have bugfixes like this available to the millions of users who get their firmware through linux-firmware. Also if this bug us remotely triggerable it allows a denial-of-service attack and thus still is a security issue. >> Another example is the firmware for the wifi on the Raspberry Pi 3B+ which >> is *years* older in linux-firmware. > > Could you please specify the source (link) of the firmware you're > referring to? linux-firmware has: [hans@shalem ~]$ strings /lib/firmware/brcm/brcmfmac43455-sdio.bin | tail -n1 43455c0-roml/sdio-ag-p2p-pno-aoe-pktfilter-keepalive-mchan-pktctx-proptxstatus-ampduhostreorder-lpc-pwropt-txbf-wnm-okc-ccx-ltecx-wfds-wl11u-mfp-tdls-ve Version: 7.45.18.0 CRC: d7226371 Date: Sun 2015-03-01 07:31:57 PST Ucode Ver: 1026.2 FWID: 01-6a2c8ad4 Where as: https://community.cypress.com/docs/DOC-15932 has: [hans@shalem Download]$ strings firmware/brcmfmac43455-sdio.bin | tail -n2 43455c0-roml/43455_sdio-pno-aoe-pktfilter-pktctx-wfds-mfp-dfsradar-wowlpf-idsup-idauth-noclminc-clm_min-obss-obssdump-swdiv-gtkoe-roamprof-txbf-ve-sae-sr Version: 7.45.173 (r707987 CY) CRC: 7beaf14c Date: Fri 2018-09-21 04:12:04 PDT Ucode Ver: 1043.2116 FWID 01-d2799ea2 So the linux firmware version is *three and a half* years out of date! >> I must say these whole shenanigans with the firmware causing >> linux-firmware to >> have way too old firmware versions makes me very unhappy. I'm at the point >> where I'm telling friends to not buy any hardware with Cypress wifi in >> there >> because of this and because of the *complete* lack of bluetooth firmware. > > We did work with HTC to upstream their bluetooth firmware to > linux-firmware, although I admit that most of the bluetooth firmware are > only available through hardware vendors. > > Bluetooth firmware files are specific to vendor board designs. When > there is an update, the hardware (device/board) vendor qualifies the > firmware and release through their preferred channel (in HTC's case they > chose to upstream it). I'm aware that the bluetooth patchram files may have board specific elements too them (most are generic but some contain board specific workarounds), this is also why for USB bluetooth adapters the kernel includes the vendorid-productid in the filename, as can be seen with for example the HTC vive bluetooth firmware. A community member has made bluetooth patchram files from the Windows drivers with the filenames expected by Linux available for Linux users here: https://github.com/winterheart/broadcom-bt-firmware/tree/master/ These files come with/under the following license: https://github.com/winterheart/broadcom-bt-firmware/blob/master/LICENSE.broadcom_bcm20702 Which indicates that even though these files are distributed through vendor channels for Windows, Broadcom and now Cypress holds the rights on these files. So it is within Cypress power to give permission to distribute these files under the https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.cypress license. That is all we are asking for. We are not asking you to do special Linux builds, Linux can use the per board Windows files just fine, all we need is permission to distribute this and then Linux users will run their boards with the same qualified firmware files as are being used under Windows. Actually by not allowing us to provide these files you run the risk of users grabbing a random file from somewhere and renaming it, potentially having it not match the board the user is using. Regards, Hans
On 01/10/2019 4:50, Hans de Goede wrote: > Hi, > > On 10-01-19 09:16, Chi-Hsien Lin wrote: > So the linux firmware version is *three and a half* years out of date! Thanks a lot for the reference. This discussion could be moved to the V2 thread. > We are not asking you to do special Linux builds, Linux can use the per > board Windows files just fine, all we need is permission to distribute this > and then Linux users will run their boards with the same qualified firmware > files as are being used under Windows. I remember that this has been brought up a while back and I did check with the Cypress bluetooth team about it. The answer I got is that the files were committed before the Cypress acquisition and our bluetooth team couldn't even locate these files in our server. So, unfortunately we cannot modify the license for firmware files that are not released by us. > > Actually by not allowing us to provide these files you run the risk of > users > grabbing a random file from somewhere and renaming it, potentially > having it > not match the board the user is using. > > Regards, > > Hans > > . >
Hi, On 10-01-19 10:45, Chi-Hsien Lin wrote: > > > On 01/10/2019 4:50, Hans de Goede wrote: >> Hi, >> >> On 10-01-19 09:16, Chi-Hsien Lin wrote: >> So the linux firmware version is *three and a half* years out of date! > > Thanks a lot for the reference. This discussion could be moved to the V2 > thread. > > >> We are not asking you to do special Linux builds, Linux can use the per >> board Windows files just fine, all we need is permission to distribute this >> and then Linux users will run their boards with the same qualified firmware >> files as are being used under Windows. > > I remember that this has been brought up a while back and I did check > with the Cypress bluetooth team about it. The answer I got is that the > files were committed before the Cypress acquisition and our bluetooth > team couldn't even locate these files in our server. So, unfortunately > we cannot modify the license for firmware files that are not released by us. AFAICT Cypress owns the copyright on these files, IANAL but it is not necessary for Cypress to (re)distribute these files under the new license. As copyright holder Cypress can simply give permission to distribute these files under the: https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.cypress license. Regards, Hans
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c index 0bb16bf574e3..e0a5e78ee437 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c @@ -149,7 +149,7 @@ static int brcmf_c_process_clm_blob(struct brcmf_if *ifp) return err; } - err = request_firmware(&clm, clm_name, bus->dev); + err = request_firmware_direct(&clm, clm_name, bus->dev); if (err) { brcmf_info("no clm_blob available (err=%d), device may have limited channels available\n", err);
The linux-firmware brcmfmac firmware files contain an embedded table with per country allowed channels and strength info. These versions of the firmware are specially build for linux-firmware, the firmware files directly available from Broadcom / Cypress rely on a separate clm_blob file for this info. For some unknown reason Broadcom / Cypress refuse to provide the standard firmware files + clm_blob files it uses elsewhere for inclusion into linux-firmware, instead relying on these special builds with the clm_blob info embedded. This means that the linux-firmware firmware versions often lag behind, but I digress. The brcmfmac driver does support the separate clm_blob file and always tries to load this. Currently we use request_firmware for this. This means that on any standard install, using the standard combo of linux-kernel + linux-firmware, we will get a warning: "Direct firmware load for ... failed with error -2" On top of this, brcmfmac itself prints: "no clm_blob available (err=-2), device may have limited channels available" and we will get a slow fallback to the userspace firmware loading mechanism. This commit fixes both almost any brcmfmac device logging the warning (leaving the brcmfmac info message in pace), as well as the slow and unnecesary fallback by switching to request_firmware_direct for the clm_blob. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)