diff mbox series

brcmfmac: Use request_firmware_direct for the clm_blob

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

Commit Message

Hans de Goede Jan. 7, 2019, 11:34 a.m. UTC
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(-)

Comments

Kalle Valo Jan. 7, 2019, 11:52 a.m. UTC | #1
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.
Arend van Spriel Jan. 7, 2019, 11:58 a.m. UTC | #2
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(-)
Hans de Goede Jan. 7, 2019, 1:05 p.m. UTC | #3
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(-)
Chi-Hsien Lin Jan. 10, 2019, 8:16 a.m. UTC | #4
(+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
Hans de Goede Jan. 10, 2019, 8:50 a.m. UTC | #5
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
Chi-Hsien Lin Jan. 10, 2019, 9:45 a.m. UTC | #6
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
> 
> .
>
Hans de Goede Jan. 10, 2019, 10:12 a.m. UTC | #7
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 mbox series

Patch

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);