lkml.org 
[lkml]   [2022]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v2 20/35] brcmfmac: pcie: Perform correct BCM4364 firmware selection
From
On 1/10/2022 12:20 PM, Hector Martin wrote:
> On 2022/01/10 18:12, Arend van Spriel wrote:
>> On 1/4/2022 8:26 AM, Hector Martin wrote:
>>> This chip exists in two revisions (B2=r3 and B3=r4) on different
>>> platforms, and was added without regard to doing proper firmware
>>> selection or differentiating between them. Fix this to have proper
>>> per-revision firmwares and support Apple NVRAM selection.
>>>
>>> Revision B2 is present on at least these Apple T2 Macs:
>>>
>>> kauai: MacBook Pro 15" (Touch/2018-2019)
>>> maui: MacBook Pro 13" (Touch/2018-2019)
>>> lanai: Mac mini (Late 2018)
>>> ekans: iMac Pro 27" (5K, Late 2017)
>>>
>>> And these non-T2 Macs:
>>>
>>> nihau: iMac 27" (5K, 2019)
>>>
>>> Revision B3 is present on at least these Apple T2 Macs:
>>>
>>> bali: MacBook Pro 16" (2019)
>>> trinidad: MacBook Pro 13" (2020, 4 TB3)
>>> borneo: MacBook Pro 16" (2019, 5600M)
>>> kahana: Mac Pro (2019)
>>> kahana: Mac Pro (2019, Rack)
>>> hanauma: iMac 27" (5K, 2020)
>>> kure: iMac 27" (5K, 2020, 5700/XT)
>>>
>>> Fixes: 24f0bd136264 ("brcmfmac: add the BRCM 4364 found in MacBook Pro 15,2")
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>> ---
>>> .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 11 +++++++++--
>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>> index 87daabb15cd0..e4f2aff3c0d5 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>> @@ -54,7 +54,8 @@ BRCMF_FW_CLM_DEF(4356, "brcmfmac4356-pcie");
>>> BRCMF_FW_CLM_DEF(43570, "brcmfmac43570-pcie");
>>> BRCMF_FW_DEF(4358, "brcmfmac4358-pcie");
>>> BRCMF_FW_DEF(4359, "brcmfmac4359-pcie");
>>> -BRCMF_FW_DEF(4364, "brcmfmac4364-pcie");
>>> +BRCMF_FW_CLM_DEF(4364B2, "brcmfmac4364b2-pcie");
>>> +BRCMF_FW_CLM_DEF(4364B3, "brcmfmac4364b3-pcie");
>>
>> would this break things for people. Maybe better to keep the old name
>> for the B2 variant.
>
> Or the B3 variant... people have been using random copied firmwares with
> the same name, I guess. Probably even the wrong NVRAMs in some cases.
> And then I'd have to add a special case to the firmware extraction
> script to rename one of these two to not include the revision...
>
> Plus, newer firmwares require the random blob, so this only ever worked
> with old, obsolete firmwares... which I think have security
> vulnerabilities (there was an AWDL exploit recently IIRC).
>
> Honestly though, there are probably rather few people using upstream
> kernels on T2s. Certainly on the MacBooks, since the keyboard/touchpad
> aren't supported upstream yet... plus given that there was never any
> "official" firmware distributed under the revision-less name, none of
> this would work out of the box with upstream kernels anyway.
>
> FWIW, I've been in contact with the t2linux folks and users have been
> testing this patchset (that's how I got it tested on all the chips), so
> at least some people are already aware of the story and how to get the
> firmware named properly :-)

Ok. When there is no brcmfmac4364-pcie.bin in linux-firmware repo we can
safely rename.

>>> - BRCMF_FW_ENTRY(BRCM_CC_4364_CHIP_ID, 0xFFFFFFFF, 4364),
>>> + BRCMF_FW_ENTRY(BRCM_CC_4364_CHIP_ID, 0x0000000F, 4364B2), /* 3 */
>>> + BRCMF_FW_ENTRY(BRCM_CC_4364_CHIP_ID, 0xFFFFFFF0, 4364B3), /* 4 */
>>
>> okay. so it is the numerical chip revision. If so, please drop that comment.
>>
>
> I figured it would be useful to document this somewhere, since the
> alphanumeric code -> rev number mapping doesn't seem to be consistent
> from chip to chip, and we might have to add a new revision in the future
> for an existing chip (which would require knowing the rev for the old
> one). Do you have any ideas?

Indeed the alphanumeric code differs from chip to chip depending on how
much respins are necessary and what type of respin. We start a 'a0' aka
numeric rev 0. For minor fixes we increase the digit, but for major
fixes or new functionality we move to the next letter whereas the
numeric revision simply increases.
[unhandled content-type:application/pkcs7-signature]
\
 
 \ /
  Last update: 2022-01-10 13:03    [W:0.123 / U:0.936 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site