linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Arend Van Spriel <arend.vanspriel@broadcom.com>,
	Franky Lin <franky.lin@broadcom.com>,
	Hante Meuleman <hante.meuleman@broadcom.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
	Wright Feng <wright.feng@cypress.com>
Cc: linux-wireless@vger.kernel.org, brcm80211-dev-list.pdl@broadcom.com
Subject: Re: [PATCH] brcmfmac: Use request_firmware_direct for the clm_blob
Date: Mon, 7 Jan 2019 14:05:47 +0100	[thread overview]
Message-ID: <5b365baf-99b3-2033-1f8e-6ca38d6daf42@redhat.com> (raw)
In-Reply-To: <d6dafb19-4e88-c2c7-9547-2ebd5b4bec2e@broadcom.com>

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

  reply	other threads:[~2019-01-07 13:05 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-07 11:34 [PATCH] brcmfmac: Use request_firmware_direct for the clm_blob Hans de Goede
2019-01-07 11:52 ` Kalle Valo
2019-01-07 11:58 ` Arend Van Spriel
2019-01-07 13:05   ` Hans de Goede [this message]
2019-01-10  8:16     ` Chi-Hsien Lin
2019-01-10  8:50       ` Hans de Goede
2019-01-10  9:45         ` Chi-Hsien Lin
2019-01-10 10:12           ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5b365baf-99b3-2033-1f8e-6ca38d6daf42@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=arend.vanspriel@broadcom.com \
    --cc=brcm80211-dev-list.pdl@broadcom.com \
    --cc=chi-hsien.lin@cypress.com \
    --cc=franky.lin@broadcom.com \
    --cc=hante.meuleman@broadcom.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=wright.feng@cypress.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).