[PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

Add support for BCM43596 dual-band AC chip, found in
SONY Xperia X Performance, XZ and XZs smartphones (and
*possibly* other devices from other manufacturers).
The chip doesn't require any special handling and seems to work
just fine OOTB.

PCIe IDs taken from: https://github.com/sonyxperiadev/kernel/commit/9e43fefbac8e43c3d7792e73ca52a052dd86d7e3.patch

Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
---
Changes since v1:
- rebased the patch against -next

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c       | 2 ++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c       | 4 ++++
 drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 4 ++++
 3 files changed, 10 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
index 3026166a56c1..6234e7475a1a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
@@ -727,6 +727,7 @@ static u32 brcmf_chip_tcm_rambase(struct brcmf_chip_priv *ci)
 	case BRCM_CC_43666_CHIP_ID:
 		return 0x200000;
 	case BRCM_CC_4359_CHIP_ID:
+	case BRCM_CC_43596_CHIP_ID:
 		return (ci->pub.chiprev < 9) ? 0x180000 : 0x160000;
 	case BRCM_CC_4364_CHIP_ID:
 	case CY_CC_4373_CHIP_ID:
@@ -1430,6 +1431,7 @@ bool brcmf_chip_sr_capable(struct brcmf_chip *pub)
 		reg = chip->ops->read32(chip->ctx, addr);
 		return (reg & CC_SR_CTL0_ENABLE_MASK) != 0;
 	case BRCM_CC_4359_CHIP_ID:
+	case BRCM_CC_43596_CHIP_ID:
 	case CY_CC_43752_CHIP_ID:
 	case CY_CC_43012_CHIP_ID:
 		addr = CORE_CC_REG(pmu->base, retention_ctl);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index f98641bb1528..2e7fc66adf31 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -81,6 +81,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
 	BRCMF_FW_ENTRY(BRCM_CC_43570_CHIP_ID, 0xFFFFFFFF, 43570),
 	BRCMF_FW_ENTRY(BRCM_CC_4358_CHIP_ID, 0xFFFFFFFF, 4358),
 	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
+	BRCMF_FW_ENTRY(BRCM_CC_43596_CHIP_ID, 0xFFFFFFFF, 4359),
 	BRCMF_FW_ENTRY(BRCM_CC_4364_CHIP_ID, 0xFFFFFFFF, 4364),
 	BRCMF_FW_ENTRY(BRCM_CC_4365_CHIP_ID, 0x0000000F, 4365B),
 	BRCMF_FW_ENTRY(BRCM_CC_4365_CHIP_ID, 0xFFFFFFF0, 4365C),
@@ -2451,6 +2452,9 @@ static const struct pci_device_id brcmf_pcie_devid_table[] = {
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_43570_RAW_DEVICE_ID),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4358_DEVICE_ID),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4359_DEVICE_ID),
+	BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_DEVICE_ID),
+	BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_2G_DEVICE_ID),
+	BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_5G_DEVICE_ID),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_43602_DEVICE_ID),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_43602_2G_DEVICE_ID),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_43602_5G_DEVICE_ID),
diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
index 1003f123ec25..c9c8701039c5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
@@ -44,6 +44,7 @@
 #define BRCM_CC_43570_CHIP_ID		43570
 #define BRCM_CC_4358_CHIP_ID		0x4358
 #define BRCM_CC_4359_CHIP_ID		0x4359
+#define BRCM_CC_43596_CHIP_ID		43596
 #define BRCM_CC_43602_CHIP_ID		43602
 #define BRCM_CC_4364_CHIP_ID		0x4364
 #define BRCM_CC_4365_CHIP_ID		0x4365
@@ -77,6 +78,9 @@
 #define BRCM_PCIE_43570_RAW_DEVICE_ID	0xaa31
 #define BRCM_PCIE_4358_DEVICE_ID	0x43e9
 #define BRCM_PCIE_4359_DEVICE_ID	0x43ef
+#define BRCM_PCIE_43596_DEVICE_ID	0x4415
+#define BRCM_PCIE_43596_2G_DEVICE_ID	0x4416
+#define BRCM_PCIE_43596_5G_DEVICE_ID	0x4417
 #define BRCM_PCIE_43602_DEVICE_ID	0x43ba
 #define BRCM_PCIE_43602_2G_DEVICE_ID	0x43bb
 #define BRCM_PCIE_43602_5G_DEVICE_ID	0x43bc
-- 
2.37.3

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Hector Martin]

On 21/09/2022 09.16, Konrad Dybcio wrote:
> Add support for BCM43596 dual-band AC chip, found in
> SONY Xperia X Performance, XZ and XZs smartphones (and
> *possibly* other devices from other manufacturers).
> The chip doesn't require any special handling and seems to work
> just fine OOTB.
> 
> PCIe IDs taken from: https://github.com/sonyxperiadev/kernel/commit/9e43fefbac8e43c3d7792e73ca52a052dd86d7e3.patch
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
> ---
> Changes since v1:
> - rebased the patch against -next
> 
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c       | 2 ++
>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c       | 4 ++++
>  drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 4 ++++
>  3 files changed, 10 insertions(+)
> 
[...]
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index f98641bb1528..2e7fc66adf31 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -81,6 +81,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
>  	BRCMF_FW_ENTRY(BRCM_CC_43570_CHIP_ID, 0xFFFFFFFF, 43570),
>  	BRCMF_FW_ENTRY(BRCM_CC_4358_CHIP_ID, 0xFFFFFFFF, 4358),
>  	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
> +	BRCMF_FW_ENTRY(BRCM_CC_43596_CHIP_ID, 0xFFFFFFFF, 4359),

So this works with the same firmware as 4359? That sounds a bit off. Is
that really the case?

brcmfmac4359-pcie isn't in linux-firmware, but presumably there is
*some* semi-canonical firmware you can find for that chip that other
people are already using. If that works on 43596 *and* you plan on using
that firmware or some other firmware marked 4359, then this is fine. If
you are using separate firmware that shipped with a 43596 device and
isn't itself marked 4359, please make it a separate firmware entry. We
can always symlink the firmwares if it later turns out there is no
reason to have different ones for each chip.

- Hector

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 21.09.2022 06:37, Hector Martin wrote:
> On 21/09/2022 09.16, Konrad Dybcio wrote:
>> Add support for BCM43596 dual-band AC chip, found in
>> SONY Xperia X Performance, XZ and XZs smartphones (and
>> *possibly* other devices from other manufacturers).
>> The chip doesn't require any special handling and seems to work
>> just fine OOTB.
>>
>> PCIe IDs taken from: https://github.com/sonyxperiadev/kernel/commit/9e43fefbac8e43c3d7792e73ca52a052dd86d7e3.patch
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>> ---
>> Changes since v1:
>> - rebased the patch against -next
>>
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c       | 2 ++
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c       | 4 ++++
>>  drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 4 ++++
>>  3 files changed, 10 insertions(+)
>>
> [...]
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> index f98641bb1528..2e7fc66adf31 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> @@ -81,6 +81,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
>>  	BRCMF_FW_ENTRY(BRCM_CC_43570_CHIP_ID, 0xFFFFFFFF, 43570),
>>  	BRCMF_FW_ENTRY(BRCM_CC_4358_CHIP_ID, 0xFFFFFFFF, 4358),
>>  	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
>> +	BRCMF_FW_ENTRY(BRCM_CC_43596_CHIP_ID, 0xFFFFFFFF, 4359),
> 
> So this works with the same firmware as 4359? That sounds a bit off. Is
> that really the case?
> 
> brcmfmac4359-pcie isn't in linux-firmware, but presumably there is
> *some* semi-canonical firmware you can find for that chip that other
> people are already using. If that works on 43596 *and* you plan on using
> that firmware or some other firmware marked 4359, then this is fine. If
> you are using separate firmware that shipped with a 43596 device and
> isn't itself marked 4359, please make it a separate firmware entry. We
> can always symlink the firmwares if it later turns out there is no
> reason to have different ones for each chip.
The firmware that SONY originally gave us for the devices that we know use
this chip seems to be marked 4359 [1]. That said, I have no other info
about the relation between the two models.

[1] https://github.com/sonyxperiadev/device-sony-kagura/tree/q-mr1/rootdir/vendor/firmware

Konrad
> 
> - Hector

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Hector Martin]

On 22/09/2022 06.26, Konrad Dybcio wrote:
> 
> 
> On 21.09.2022 06:37, Hector Martin wrote:
>> On 21/09/2022 09.16, Konrad Dybcio wrote:
>>> Add support for BCM43596 dual-band AC chip, found in
>>> SONY Xperia X Performance, XZ and XZs smartphones (and
>>> *possibly* other devices from other manufacturers).
>>> The chip doesn't require any special handling and seems to work
>>> just fine OOTB.
>>>
>>> PCIe IDs taken from: https://github.com/sonyxperiadev/kernel/commit/9e43fefbac8e43c3d7792e73ca52a052dd86d7e3.patch
>>>
>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>>> ---
>>> Changes since v1:
>>> - rebased the patch against -next
>>>
>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c       | 2 ++
>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c       | 4 ++++
>>>  drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 4 ++++
>>>  3 files changed, 10 insertions(+)
>>>
>> [...]
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>> index f98641bb1528..2e7fc66adf31 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>> @@ -81,6 +81,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
>>>  	BRCMF_FW_ENTRY(BRCM_CC_43570_CHIP_ID, 0xFFFFFFFF, 43570),
>>>  	BRCMF_FW_ENTRY(BRCM_CC_4358_CHIP_ID, 0xFFFFFFFF, 4358),
>>>  	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
>>> +	BRCMF_FW_ENTRY(BRCM_CC_43596_CHIP_ID, 0xFFFFFFFF, 4359),
>>
>> So this works with the same firmware as 4359? That sounds a bit off. Is
>> that really the case?
>>
>> brcmfmac4359-pcie isn't in linux-firmware, but presumably there is
>> *some* semi-canonical firmware you can find for that chip that other
>> people are already using. If that works on 43596 *and* you plan on using
>> that firmware or some other firmware marked 4359, then this is fine. If
>> you are using separate firmware that shipped with a 43596 device and
>> isn't itself marked 4359, please make it a separate firmware entry. We
>> can always symlink the firmwares if it later turns out there is no
>> reason to have different ones for each chip.
> The firmware that SONY originally gave us for the devices that we know use
> this chip seems to be marked 4359 [1]. That said, I have no other info
> about the relation between the two models.
> 
> [1] https://github.com/sonyxperiadev/device-sony-kagura/tree/q-mr1/rootdir/vendor/firmware

That link seems to have the nvram file and Bluetooth firmware, but not
WLAN firmware. I think if you run `strings` on the WLAN firmware you'll
get a build ID with the chip name in it, that might be a good indication
of what the firmware name should be.

I would suggest trying to find some other 4359 firmware and testing your
device with it. If it works, then it's definitely fine to use the same
firmware name. If it doesn't, then you might need different firmware
names, or it might just be a case for board-specific firmwares.

- Hector

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 22.09.2022 08:40, Hector Martin wrote:
> On 22/09/2022 06.26, Konrad Dybcio wrote:
>>
>>
>> On 21.09.2022 06:37, Hector Martin wrote:
>>> On 21/09/2022 09.16, Konrad Dybcio wrote:
>>>> Add support for BCM43596 dual-band AC chip, found in
>>>> SONY Xperia X Performance, XZ and XZs smartphones (and
>>>> *possibly* other devices from other manufacturers).
>>>> The chip doesn't require any special handling and seems to work
>>>> just fine OOTB.
>>>>
>>>> PCIe IDs taken from: https://github.com/sonyxperiadev/kernel/commit/9e43fefbac8e43c3d7792e73ca52a052dd86d7e3.patch
>>>>
>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@somainline.org>
>>>> ---
>>>> Changes since v1:
>>>> - rebased the patch against -next
>>>>
>>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c       | 2 ++
>>>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c       | 4 ++++
>>>>  drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 4 ++++
>>>>  3 files changed, 10 insertions(+)
>>>>
>>> [...]
>>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>>> index f98641bb1528..2e7fc66adf31 100644
>>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>>>> @@ -81,6 +81,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
>>>>  	BRCMF_FW_ENTRY(BRCM_CC_43570_CHIP_ID, 0xFFFFFFFF, 43570),
>>>>  	BRCMF_FW_ENTRY(BRCM_CC_4358_CHIP_ID, 0xFFFFFFFF, 4358),
>>>>  	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
>>>> +	BRCMF_FW_ENTRY(BRCM_CC_43596_CHIP_ID, 0xFFFFFFFF, 4359),
>>>
>>> So this works with the same firmware as 4359? That sounds a bit off. Is
>>> that really the case?
>>>
>>> brcmfmac4359-pcie isn't in linux-firmware, but presumably there is
>>> *some* semi-canonical firmware you can find for that chip that other
>>> people are already using. If that works on 43596 *and* you plan on using
>>> that firmware or some other firmware marked 4359, then this is fine. If
>>> you are using separate firmware that shipped with a 43596 device and
>>> isn't itself marked 4359, please make it a separate firmware entry. We
>>> can always symlink the firmwares if it later turns out there is no
>>> reason to have different ones for each chip.
>> The firmware that SONY originally gave us for the devices that we know use
>> this chip seems to be marked 4359 [1]. That said, I have no other info
>> about the relation between the two models.
>>
>> [1] https://github.com/sonyxperiadev/device-sony-kagura/tree/q-mr1/rootdir/vendor/firmware
> 
> That link seems to have the nvram file and Bluetooth firmware, but not
> WLAN firmware. I think if you run `strings` on the WLAN firmware you'll
> get a build ID with the chip name in it, that might be a good indication
> of what the firmware name should be.
Riight, sorry about that.. This is the correct link [1]

Running strings on fw_bcmdhd.bin (which I renamed to brcmfmac4359-pcie.bin for
mainline firwmare name matching purposes) outputs:

43596a0-roml/pcie-wl11u-ve-mfp-tdls-sr-die3-wepso-wnm-pfn-olpc-mobfd-rcc-ccx-noccxaka-clm_43xx_somc_mimo-fmc-phyflags-rscanf-murx-ltecx-rpi-txpwrctrls-dpo-proxd-hs20sta-linkstat-gscan-rmon-pfnanqpo-dpm Version: 9.75.119.15 (r691661) CRC: 46ae3900 Date: Fri 2017-03-24 13:22:28 KST Ucode Ver: 1060.20542 FWID: 01-a0d2ee7a

However, as you can see, the directory is named bcm4359 and not bcm43596, so my
best guess is that it's probably part of the same chip family with minor
differences.

Also worth noting is the 'somc' bit, meaning there are probably *some* SONY
customizations, but that's also just a guess.


I did also find this [2] fw for 4359 which contains:

4359b1-roml/pcie-wl11u-mfp-nvramadj-sr-die3-pwrofs-wnm-pfn-pwrstats-olpc-mobfd-txbf-mimopscan-slna-wapi-nsslimit-lpc-wepso-pspretend-apbs-apcs-spurcan-clm_min-obss-fbt-idsup-idauth Version: 9.40.109 (r710128 CY) CRC: e4b3d019 Date: Wed 2019-02-20 18:09:51 PST Ucode Ver: 1043.20426 FWID 01-848095d2

So I suppose I will respin this series to make 43596 use its own fw, since - while
probably similar - the two are in fact distinct.

Konrad

[1] https://github.com/sonyxperiadev/vendor-broadcom-wlan/tree/master/bcmdhd/firmware/bcm4359
[2] https://github.com/NXP/imx-firmware/tree/master/cyw-wifi-bt/1FD_CYW4359
> 
> I would suggest trying to find some other 4359 firmware and testing your
> device with it. If it works, then it's definitely fine to use the same
> firmware name. If it doesn't, then you might need different firmware
> names, or it might just be a case for board-specific firmwares.
> 
> - Hector

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Linus Walleij]

On Thu, Sep 22, 2022 at 12:21 PM Konrad Dybcio
<konrad.dybcio@somainline.org> wrote:

> Also worth noting is the 'somc' bit, meaning there are probably *some* SONY
> customizations, but that's also just a guess.

What I have seen from BRCM customizations on Samsung phones is that
the per-device customization of firmware seems to involve the set-up of
some GPIO and power management pins. For example if integrated with
an SoC that has autonomous system resume, or if some GPIO line has
to be pulled to enable an external regulator or PA.

To the best of my knowledge that customization is done by consultants
from Broadcom when working with the device manufacturer, and
eventually they roll a unique firmware for the device. Probably because
the firmware can only be signed for execution by Broadcom?

Yours,
Linus Walleij

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Hector Martin]

On 22/09/2022 22.02, Linus Walleij wrote:
> On Thu, Sep 22, 2022 at 12:21 PM Konrad Dybcio
> <konrad.dybcio@somainline.org> wrote:
> 
>> Also worth noting is the 'somc' bit, meaning there are probably *some* SONY
>> customizations, but that's also just a guess.
> 
> What I have seen from BRCM customizations on Samsung phones is that
> the per-device customization of firmware seems to involve the set-up of
> some GPIO and power management pins. For example if integrated with
> an SoC that has autonomous system resume, or if some GPIO line has
> to be pulled to enable an external regulator or PA.
> 
> To the best of my knowledge that customization is done by consultants
> from Broadcom when working with the device manufacturer, and
> eventually they roll a unique firmware for the device. Probably because
> the firmware can only be signed for execution by Broadcom?

I don't think the firmware is signed, they probably just don't want to
share the source code with most customers? (Except Apple maybe, but
Apple gets custom silicon too...).

- Hector

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Alvin Šipraga]

On Thu, Sep 22, 2022 at 03:02:12PM +0200, Linus Walleij wrote:
> On Thu, Sep 22, 2022 at 12:21 PM Konrad Dybcio
> <konrad.dybcio@somainline.org> wrote:
> 
> > Also worth noting is the 'somc' bit, meaning there are probably *some* SONY
> > customizations, but that's also just a guess.
> 
> What I have seen from BRCM customizations on Samsung phones is that
> the per-device customization of firmware seems to involve the set-up of
> some GPIO and power management pins. For example if integrated with
> an SoC that has autonomous system resume, or if some GPIO line has
> to be pulled to enable an external regulator or PA.

At least with Infineon (formerly Cypress), as a customer you might get a
private firmware and this will be maintained internally by them on a
separate customer branch. Any subsequent bugfixes or feature requests
will usually be applied to that customer branch and a new firmware built
from it. I think their internal "mainline" branch might get merged into
the customer branches from time to time, but this seems to be done on an
ad-hoc basis. This is our experience at least.

I would also point out that the BCM4359 is equivalent to the
CYW88359/CYW89359 chipset, which we are using in some of our
products. Note that this is a Cypress chipset (identifiable by the
Version: ... (... CY) tag in the version string). But the FW Konrad is
linking appears to be for a Broadcom chipset.

FYI, here's a publicly available set of firmware files for the '4359:

https://github.com/NXP/imx-firmware/tree/master/cyw-wifi-bt/1FD_CYW4359

Anyway, I would second Hector's suggestion and make this a separate FW.

> 
> To the best of my knowledge that customization is done by consultants
> from Broadcom when working with the device manufacturer, and
> eventually they roll a unique firmware for the device. Probably because
> the firmware can only be signed for execution by Broadcom?

Kind regards,
Alvin

> 
> Yours,
> Linus Walleij

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Linus Walleij]

On Thu, Sep 22, 2022 at 3:31 PM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:

> I would also point out that the BCM4359 is equivalent to the
> CYW88359/CYW89359 chipset, which we are using in some of our
> products. Note that this is a Cypress chipset (identifiable by the
> Version: ... (... CY) tag in the version string). But the FW Konrad is
> linking appears to be for a Broadcom chipset.

This just makes me think about Peter Robinsons seminar at
LPC last week...
"All types of wireless in Linux are terrible and why the vendors
should feel bad"
https://lpc.events/event/16/contributions/1278/attachments/1120/2153/wireless-issues.pdf

Yours,
Linus Walleij

Stockholm syndrome with Linux wireless?

[Kalle Valo]

(changing the subject as this has nothing to do with brcmfmac)

Linus Walleij <linus.walleij@linaro.org> writes:

> On Thu, Sep 22, 2022 at 3:31 PM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
>
>> I would also point out that the BCM4359 is equivalent to the
>> CYW88359/CYW89359 chipset, which we are using in some of our
>> products. Note that this is a Cypress chipset (identifiable by the
>> Version: ... (... CY) tag in the version string). But the FW Konrad is
>> linking appears to be for a Broadcom chipset.
>
> This just makes me think about Peter Robinsons seminar at
> LPC last week...
> "All types of wireless in Linux are terrible and why the vendors
> should feel bad"
> https://lpc.events/event/16/contributions/1278/attachments/1120/2153/wireless-issues.pdf

Thanks, this was a good read! I'm always interested about user and
downstream feedback, both good and bad :) But I didn't get the Stockholm
syndrome comment in the end, what does he mean with that?

BTW we have a wireless workshop in netdevconf 0x16, it would be great to
have there a this kind of session discussing user pain points:

https://netdevconf.info/0x16/session.html?Wireless-Workshop

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Stockholm syndrome with Linux wireless?

[Russell King (Oracle)]

On Mon, Sep 26, 2022 at 11:20:18AM +0300, Kalle Valo wrote:
> (changing the subject as this has nothing to do with brcmfmac)
> 
> Linus Walleij <linus.walleij@linaro.org> writes:
> 
> > On Thu, Sep 22, 2022 at 3:31 PM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
> >
> >> I would also point out that the BCM4359 is equivalent to the
> >> CYW88359/CYW89359 chipset, which we are using in some of our
> >> products. Note that this is a Cypress chipset (identifiable by the
> >> Version: ... (... CY) tag in the version string). But the FW Konrad is
> >> linking appears to be for a Broadcom chipset.
> >
> > This just makes me think about Peter Robinsons seminar at
> > LPC last week...
> > "All types of wireless in Linux are terrible and why the vendors
> > should feel bad"
> > https://lpc.events/event/16/contributions/1278/attachments/1120/2153/wireless-issues.pdf
> 
> Thanks, this was a good read! I'm always interested about user and
> downstream feedback, both good and bad :) But I didn't get the Stockholm
> syndrome comment in the end, what does he mean with that?
> 
> BTW we have a wireless workshop in netdevconf 0x16, it would be great to
> have there a this kind of session discussing user pain points:
> 
> https://netdevconf.info/0x16/session.html?Wireless-Workshop

You asked. :)

It's probably outside of your control as it's probably firmware issues
is when using Broadcom or TI WiFi in AP mode, and the resulting
instability.

Over the years, SolidRun have used Broadcom 4329 and 4330 chipsets on
their iMX6 products, and then switched to using a TI WL18xx chipset.
I forget what the issues were with the Broadcom chipsets, as I'm
currently using the TI variants.

In order to keep the WiFi network stable, I implemented a userspace
program that polls the WL18xx statistics in debugfs every 100ms, and
when it seems the adapter has got stuck, it takes the interface down
and brings it back up again to reset stuff. This seems to improve the
overall stability, but it's still far from perfect as one regularly
sees latency go through the roof.

I recently noticed (earlier this month) a bigger problem with it -
I had one laptop running zoom, another laptop running interactive
stuff, and while zoom was running, the other laptop basically lost
network access - which came back when zoom was stopped. I'm not
sure what was going on there, because if you don't have the ability
to do interactive stuff it's pretty hard to debug what's going on
at the machine with the AP.

I've just looked at that machine, which has been mostly idle (as in
no clients connected) and I see:

[271559.346460] wlcore: ERROR Tx stuck (in FW) for 5000 ms. Starting recovery
[271559.353395] WARNING: CPU: 1 PID: 6395 at drivers/net/wireless/ti/wlcore/main.c:803 wl12xx_queue_recovery_work.part.0+0x50/0x54 [wlcore]

with the resutling entirely useless backtrace - that's 3 days, 3 hours
and 25 minutes ago, which would make it Friday 6:25am when nothing
was connected to the wifi network.

I've turned off all the runtime PM for the hardware path for wifi
conenctivity (every single power/control file is forced to "on") so
it isn't being triggered by some runtime PM behaviour.

Like I think many, I've come to the conclusion that WiFi is just a
completely unstable medium, and wired networking is just far superior,
even though it comes with the nusience of needing wires.

I don't think this is the fault of the Linux WiFi core code, I think
this is down to vendors, and vendors just do not want to know about
problems.

That said, running this stuff in AP mode isn't vendors primary goal,
since that isn't what most users want to do, so it's probably
understandable that AP mode tends to be flakey.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Kalle Valo]

Alvin Šipraga <ALSI@bang-olufsen.dk> writes:

> On Thu, Sep 22, 2022 at 03:02:12PM +0200, Linus Walleij wrote:
>> On Thu, Sep 22, 2022 at 12:21 PM Konrad Dybcio
>> <konrad.dybcio@somainline.org> wrote:
>> 
>> > Also worth noting is the 'somc' bit, meaning there are probably *some* SONY
>> > customizations, but that's also just a guess.
>> 
>> What I have seen from BRCM customizations on Samsung phones is that
>> the per-device customization of firmware seems to involve the set-up of
>> some GPIO and power management pins. For example if integrated with
>> an SoC that has autonomous system resume, or if some GPIO line has
>> to be pulled to enable an external regulator or PA.
>
> At least with Infineon (formerly Cypress), as a customer you might get a
> private firmware and this will be maintained internally by them on a
> separate customer branch. Any subsequent bugfixes or feature requests
> will usually be applied to that customer branch and a new firmware built
> from it. I think their internal "mainline" branch might get merged into
> the customer branches from time to time, but this seems to be done on an
> ad-hoc basis. This is our experience at least.
>
> I would also point out that the BCM4359 is equivalent to the
> CYW88359/CYW89359 chipset, which we are using in some of our
> products. Note that this is a Cypress chipset (identifiable by the
> Version: ... (... CY) tag in the version string). But the FW Konrad is
> linking appears to be for a Broadcom chipset.
>
> FYI, here's a publicly available set of firmware files for the '4359:
>
> https://github.com/NXP/imx-firmware/tree/master/cyw-wifi-bt/1FD_CYW4359
>
> Anyway, I would second Hector's suggestion and make this a separate FW.

I also recommend having a separate firmware filename. Like Hector said,
it's easy to have a symlink in userspace if same binary can be used.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: Stockholm syndrome with Linux wireless?

[Linus Walleij]

On Mon, Sep 26, 2022 at 10:20 AM Kalle Valo <kvalo@kernel.org> wrote:

> Thanks, this was a good read! I'm always interested about user and
> downstream feedback, both good and bad :) But I didn't get the Stockholm
> syndrome comment in the end, what does he mean with that?

That refers to this:
https://en.wikipedia.org/wiki/Stockholm_syndrome

What I think Peter means is we can't have a situation where the
community is held hostage by low quality wireless vendors and
even start sympathize with their development process and level
of support.

> BTW we have a wireless workshop in netdevconf 0x16, it would be great to
> have there a this kind of session discussing user pain points:
>
> https://netdevconf.info/0x16/session.html?Wireless-Workshop

I added Peter to the thread.

Yours,
Linus Walleij

Re: Stockholm syndrome with Linux wireless?

[Linus Walleij]

On Mon, Sep 26, 2022 at 10:20 AM Kalle Valo <kvalo@kernel.org> wrote:
> Linus Walleij <linus.walleij@linaro.org> writes:
> > On Thu, Sep 22, 2022 at 3:31 PM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
> >
> >> I would also point out that the BCM4359 is equivalent to the
> >> CYW88359/CYW89359 chipset, which we are using in some of our
> >> products. Note that this is a Cypress chipset (identifiable by the
> >> Version: ... (... CY) tag in the version string). But the FW Konrad is
> >> linking appears to be for a Broadcom chipset.
> >
> > This just makes me think about Peter Robinsons seminar at
> > LPC last week...
> > "All types of wireless in Linux are terrible and why the vendors
> > should feel bad"
> > https://lpc.events/event/16/contributions/1278/attachments/1120/2153/wireless-issues.pdf
>
> Thanks, this was a good read! I'm always interested about user and
> downstream feedback, both good and bad :) But I didn't get the Stockholm
> syndrome comment in the end, what does he mean with that?
>
> BTW we have a wireless workshop in netdevconf 0x16, it would be great to
> have there a this kind of session discussing user pain points:

I can't go to Lisbon, but my personal pain points are all this:
https://openwrt.org/meta/infobox/broadcom_wifi
and I think I'm not alone, but I can't speak for OpenWrt.

The lack of support in b43 for modern phys such as AC, i.e. the gap
between b43 and brcmfmac, is extremely annoying and turning perfectly
fine aftermarket devices into paperweights because there isn't even
a way to make Broadcoms old proprietary blob work with contemporary
kernels.

If Broadcom could be convinced to either add support for the late b43
variants or at least release documentation for the aftermarket that
would be great.

I suppose they might be coming to the conference so give them my best
regards with a "please fix" tag attached.

Yours,
Linus Walleij

Re: Stockholm syndrome with Linux wireless?

[Dave Taht]

On Thu, Oct 20, 2022 at 7:34 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Sep 26, 2022 at 10:20 AM Kalle Valo <kvalo@kernel.org> wrote:
> > Linus Walleij <linus.walleij@linaro.org> writes:
> > > On Thu, Sep 22, 2022 at 3:31 PM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
> > >
> > >> I would also point out that the BCM4359 is equivalent to the
> > >> CYW88359/CYW89359 chipset, which we are using in some of our
> > >> products. Note that this is a Cypress chipset (identifiable by the
> > >> Version: ... (... CY) tag in the version string). But the FW Konrad is
> > >> linking appears to be for a Broadcom chipset.
> > >
> > > This just makes me think about Peter Robinsons seminar at
> > > LPC last week...
> > > "All types of wireless in Linux are terrible and why the vendors
> > > should feel bad"
> > > https://lpc.events/event/16/contributions/1278/attachments/1120/2153/wireless-issues.pdf
> >
> > Thanks, this was a good read! I'm always interested about user and
> > downstream feedback, both good and bad :) But I didn't get the Stockholm
> > syndrome comment in the end, what does he mean with that?
> >
> > BTW we have a wireless workshop in netdevconf 0x16, it would be great to
> > have there a this kind of session discussing user pain points:
>
> I can't go to Lisbon, but my personal pain points are all this:
> https://openwrt.org/meta/infobox/broadcom_wifi
> and I think I'm not alone, but I can't speak for OpenWrt.
>
> The lack of support in b43 for modern phys such as AC, i.e. the gap
> between b43 and brcmfmac, is extremely annoying and turning perfectly
> fine aftermarket devices into paperweights because there isn't even
> a way to make Broadcoms old proprietary blob work with contemporary
> kernels.

+10. I'm a big believer in coping with the present day supply problems with
modern software on perfectly good old routers. To heck with planned
obsolescence.

There are 5.2 billion cellphones turning into e-waste this year, also. The wifi
situation there is also a mess.

>
> If Broadcom could be convinced to either add support for the late b43
> variants or at least release documentation for the aftermarket that
> would be great.
>
> I suppose they might be coming to the conference so give them my best
> regards with a "please fix" tag attached.

Please! A symbol of a rotting raspberry, on their badges, or some
other gentle poke, might work wonders.

I too cannot make it to lisbon. I just burned 9 months of my life
(unpaid, mostly, but a huge thanks to NLNET for covering
half my costs) on helping fix a ton of regressions in the ath10k,
mt76, and ath9k, instead of making forward progress with new stuff.

I can call out the ax210 chips as being especially terrible still.
It's hard to test APs when the client chips
are as bad as that.

I'm very tempted to just buy a one-way ticket to lisbon, join this
wireless emotional support group there, and then find a beach to
retire on, and never think about this part of our field again.

It seems far saner to quit working on wifi and seek out something more
rewarding. Maybe there's some bright light on the horizon for less
binary blobs in wifi7?

>
> Yours,
> Linus Walleij



-- 
This song goes out to all the folk that thought Stadia would work:
https://www.linkedin.com/posts/dtaht_the-mushroom-song-activity-6981366665607352320-FXtz
Dave Täht CEO, TekLibre, LLC

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 26/09/2022 11:27, Kalle Valo wrote:
> Alvin Šipraga <ALSI@bang-olufsen.dk> writes:
> 
>> On Thu, Sep 22, 2022 at 03:02:12PM +0200, Linus Walleij wrote:
>>> On Thu, Sep 22, 2022 at 12:21 PM Konrad Dybcio
>>> <konrad.dybcio@somainline.org> wrote:
>>>
>>>> Also worth noting is the 'somc' bit, meaning there are probably *some* SONY
>>>> customizations, but that's also just a guess.
>>>
>>> What I have seen from BRCM customizations on Samsung phones is that
>>> the per-device customization of firmware seems to involve the set-up of
>>> some GPIO and power management pins. For example if integrated with
>>> an SoC that has autonomous system resume, or if some GPIO line has
>>> to be pulled to enable an external regulator or PA.
>>
>> At least with Infineon (formerly Cypress), as a customer you might get a
>> private firmware and this will be maintained internally by them on a
>> separate customer branch. Any subsequent bugfixes or feature requests
>> will usually be applied to that customer branch and a new firmware built
>> from it. I think their internal "mainline" branch might get merged into
>> the customer branches from time to time, but this seems to be done on an
>> ad-hoc basis. This is our experience at least.
>>
>> I would also point out that the BCM4359 is equivalent to the
>> CYW88359/CYW89359 chipset, which we are using in some of our
>> products. Note that this is a Cypress chipset (identifiable by the
>> Version: ... (... CY) tag in the version string). But the FW Konrad is
>> linking appears to be for a Broadcom chipset.
>>
>> FYI, here's a publicly available set of firmware files for the '4359:
>>
>> https://github.com/NXP/imx-firmware/tree/master/cyw-wifi-bt/1FD_CYW4359
>>
>> Anyway, I would second Hector's suggestion and make this a separate FW.
> 
> I also recommend having a separate firmware filename. Like Hector said,
> it's easy to have a symlink in userspace if same binary can be used.
So, I dusted off this patch and tried to change the fw name, only to 
discover that the BRCM_PCIE_43596_DEVICE_ID is equal to 
CY_PCIE_89459_DEVICE_ID, (which btw uses 4355/89459 fw), but then it 
makes the driver expect 4359/9 based on rev matching, so... the 43596 
chip ID may be a lie? Or at least for the one used in my particular 
device? I'm beyond confused now..

I can think of a couple of hacky ways to force use of 43596 fw, but I 
don't think any would be really upstreamable..

Any thoughts?

Konrad

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Linus Walleij]

On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:

> I can think of a couple of hacky ways to force use of 43596 fw, but I
> don't think any would be really upstreamable..

If it is only known to affect the Sony Xperias mentioned then
a thing such as:

if (of_machine_is_compatible("sony,xyz") ||
    of_machine_is_compatible("sony,zzz")... ) {
   // Enforce FW version
}

would be completely acceptable in my book. It hammers the
problem from the top instead of trying to figure out itsy witsy
details about firmware revisions.

Yours,
Linus Walleij

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 21.11.2022 14:56, Linus Walleij wrote:
> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> 
>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>> don't think any would be really upstreamable..
> 
> If it is only known to affect the Sony Xperias mentioned then
> a thing such as:
> 
> if (of_machine_is_compatible("sony,xyz") ||
>     of_machine_is_compatible("sony,zzz")... ) {
>    // Enforce FW version
> }
> 
> would be completely acceptable in my book. It hammers the
> problem from the top instead of trying to figure out itsy witsy
> details about firmware revisions.
> 
> Yours,
> Linus Walleij
Actually, I think I came up with a better approach by pulling a page
out of Asahi folks' book - please take a look and tell me what you
think about this:

[1] https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
[2] https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7


Konrad

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Kalle Valo]

Konrad Dybcio <konrad.dybcio@linaro.org> writes:

> On 21.11.2022 14:56, Linus Walleij wrote:
>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>> 
>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>> don't think any would be really upstreamable..
>> 
>> If it is only known to affect the Sony Xperias mentioned then
>> a thing such as:
>> 
>> if (of_machine_is_compatible("sony,xyz") ||
>>     of_machine_is_compatible("sony,zzz")... ) {
>>    // Enforce FW version
>> }
>> 
>> would be completely acceptable in my book. It hammers the
>> problem from the top instead of trying to figure out itsy witsy
>> details about firmware revisions.
>> 
>> Yours,
>> Linus Walleij
>
> Actually, I think I came up with a better approach by pulling a page
> out of Asahi folks' book - please take a look and tell me what you
> think about this:
>
> [1]
> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
> [2]
> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7

Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
have directory names in Device Tree.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 25.11.2022 12:53, Kalle Valo wrote:
> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
> 
>> On 21.11.2022 14:56, Linus Walleij wrote:
>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>
>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>>> don't think any would be really upstreamable..
>>>
>>> If it is only known to affect the Sony Xperias mentioned then
>>> a thing such as:
>>>
>>> if (of_machine_is_compatible("sony,xyz") ||
>>>     of_machine_is_compatible("sony,zzz")... ) {
>>>    // Enforce FW version
>>> }
>>>
>>> would be completely acceptable in my book. It hammers the
>>> problem from the top instead of trying to figure out itsy witsy
>>> details about firmware revisions.
>>>
>>> Yours,
>>> Linus Walleij
>>
>> Actually, I think I came up with a better approach by pulling a page
>> out of Asahi folks' book - please take a look and tell me what you
>> think about this:
>>
>> [1]
>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>> [2]
>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
> 
> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
> have directory names in Device Tree.
I think it's common practice to include a full $FIRMWARE_DIR-relative
path when specifying firmware in DT, though here I left out the board
name bit as that's assigned dynamically anyway. That said, if you don't
like it, I can change it.

Konrad
> 

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Kalle Valo]

Konrad Dybcio <konrad.dybcio@linaro.org> writes:

> On 25.11.2022 12:53, Kalle Valo wrote:
>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>> 
>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>
>>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>>>> don't think any would be really upstreamable..
>>>>
>>>> If it is only known to affect the Sony Xperias mentioned then
>>>> a thing such as:
>>>>
>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>     of_machine_is_compatible("sony,zzz")... ) {
>>>>    // Enforce FW version
>>>> }
>>>>
>>>> would be completely acceptable in my book. It hammers the
>>>> problem from the top instead of trying to figure out itsy witsy
>>>> details about firmware revisions.
>>>>
>>>> Yours,
>>>> Linus Walleij
>>>
>>> Actually, I think I came up with a better approach by pulling a page
>>> out of Asahi folks' book - please take a look and tell me what you
>>> think about this:
>>>
>>> [1]
>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>> [2]
>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>> 
>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
>> have directory names in Device Tree.
>
> I think it's common practice to include a full $FIRMWARE_DIR-relative
> path when specifying firmware in DT, though here I left out the board
> name bit as that's assigned dynamically anyway. That said, if you don't
> like it, I can change it.

It's just that I have understood that Device Tree is supposed to
describe hardware and to me a firmware directory "brcm/" is a software
property, not a hardware property. But this is really for the Device
Tree maintainers to decide, they know this best :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Linus Walleij]

On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>
> > On 25.11.2022 12:53, Kalle Valo wrote:
> >> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
> >>
> >>> On 21.11.2022 14:56, Linus Walleij wrote:
> >>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>>>
> >>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
> >>>>> don't think any would be really upstreamable..
> >>>>
> >>>> If it is only known to affect the Sony Xperias mentioned then
> >>>> a thing such as:
> >>>>
> >>>> if (of_machine_is_compatible("sony,xyz") ||
> >>>>     of_machine_is_compatible("sony,zzz")... ) {
> >>>>    // Enforce FW version
> >>>> }
> >>>>
> >>>> would be completely acceptable in my book. It hammers the
> >>>> problem from the top instead of trying to figure out itsy witsy
> >>>> details about firmware revisions.
> >>>>
> >>>> Yours,
> >>>> Linus Walleij
> >>>
> >>> Actually, I think I came up with a better approach by pulling a page
> >>> out of Asahi folks' book - please take a look and tell me what you
> >>> think about this:
> >>>
> >>> [1]
> >>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
> >>> [2]
> >>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7

Something in this direction works too.

The upside is that it tells all operating systems how to deal
with the firmware for this hardware.

> >> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
> >> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
> >> have directory names in Device Tree.
> >
> > I think it's common practice to include a full $FIRMWARE_DIR-relative
> > path when specifying firmware in DT, though here I left out the board
> > name bit as that's assigned dynamically anyway. That said, if you don't
> > like it, I can change it.
>
> It's just that I have understood that Device Tree is supposed to
> describe hardware and to me a firmware directory "brcm/" is a software
> property, not a hardware property. But this is really for the Device
> Tree maintainers to decide, they know this best :)

I would personally just minimize the amount of information
put into the device tree to be exactly what is needed to find
the right firmware.

brcm,firmware-compatible = "43596";

since the code already knows how to conjure the rest of the string.

But check with Rob/Krzysztof.

Yours,
Linus Walleij

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Kalle Valo]

Linus Walleij <linus.walleij@linaro.org> writes:

>> >> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
>> >> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
>> >> have directory names in Device Tree.
>> >
>> > I think it's common practice to include a full $FIRMWARE_DIR-relative
>> > path when specifying firmware in DT, though here I left out the board
>> > name bit as that's assigned dynamically anyway. That said, if you don't
>> > like it, I can change it.
>>
>> It's just that I have understood that Device Tree is supposed to
>> describe hardware and to me a firmware directory "brcm/" is a software
>> property, not a hardware property. But this is really for the Device
>> Tree maintainers to decide, they know this best :)
>
> I would personally just minimize the amount of information
> put into the device tree to be exactly what is needed to find
> the right firmware.
>
> brcm,firmware-compatible = "43596";
>
> since the code already knows how to conjure the rest of the string.

FWIW I like this.

> But check with Rob/Krzysztof.

Indeed, they are the experts here.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 26.11.2022 22:45, Linus Walleij wrote:
> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>
>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>
>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>>>
>>>>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>>>>>> don't think any would be really upstreamable..
>>>>>>
>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>> a thing such as:
>>>>>>
>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>     of_machine_is_compatible("sony,zzz")... ) {
>>>>>>    // Enforce FW version
>>>>>> }
>>>>>>
>>>>>> would be completely acceptable in my book. It hammers the
>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>> details about firmware revisions.
>>>>>>
>>>>>> Yours,
>>>>>> Linus Walleij
>>>>>
>>>>> Actually, I think I came up with a better approach by pulling a page
>>>>> out of Asahi folks' book - please take a look and tell me what you
>>>>> think about this:
>>>>>
>>>>> [1]
>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>> [2]
>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
> 
> Something in this direction works too.
> 
> The upside is that it tells all operating systems how to deal
> with the firmware for this hardware.
> 
>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
>>>> have directory names in Device Tree.
>>>
>>> I think it's common practice to include a full $FIRMWARE_DIR-relative
>>> path when specifying firmware in DT, though here I left out the board
>>> name bit as that's assigned dynamically anyway. That said, if you don't
>>> like it, I can change it.
>>
>> It's just that I have understood that Device Tree is supposed to
>> describe hardware and to me a firmware directory "brcm/" is a software
>> property, not a hardware property. But this is really for the Device
>> Tree maintainers to decide, they know this best :)
> 
> I would personally just minimize the amount of information
> put into the device tree to be exactly what is needed to find
> the right firmware.
> 
> brcm,firmware-compatible = "43596";
> 
> since the code already knows how to conjure the rest of the string.
> 
> But check with Rob/Krzysztof.
> 
> Yours,
> Linus Walleij

Krzysztof, Rob [added to CC] - can I have your opinions?

Konrad

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Krzysztof Kozlowski]

On 28/11/2022 15:40, Konrad Dybcio wrote:
> 
> 
> On 26.11.2022 22:45, Linus Walleij wrote:
>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>
>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>
>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>>>>
>>>>>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>>>>>>> don't think any would be really upstreamable..
>>>>>>>
>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>> a thing such as:
>>>>>>>
>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>     of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>    // Enforce FW version
>>>>>>> }
>>>>>>>
>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>>> details about firmware revisions.
>>>>>>>
>>>>>>> Yours,
>>>>>>> Linus Walleij
>>>>>>
>>>>>> Actually, I think I came up with a better approach by pulling a page
>>>>>> out of Asahi folks' book - please take a look and tell me what you
>>>>>> think about this:
>>>>>>
>>>>>> [1]
>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>> [2]
>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>
>> Something in this direction works too.
>>
>> The upside is that it tells all operating systems how to deal
>> with the firmware for this hardware.
>>
>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
>>>>> have directory names in Device Tree.
>>>>
>>>> I think it's common practice to include a full $FIRMWARE_DIR-relative
>>>> path when specifying firmware in DT, though here I left out the board
>>>> name bit as that's assigned dynamically anyway. That said, if you don't
>>>> like it, I can change it.
>>>
>>> It's just that I have understood that Device Tree is supposed to
>>> describe hardware and to me a firmware directory "brcm/" is a software
>>> property, not a hardware property. But this is really for the Device
>>> Tree maintainers to decide, they know this best :)
>>
>> I would personally just minimize the amount of information
>> put into the device tree to be exactly what is needed to find
>> the right firmware.
>>
>> brcm,firmware-compatible = "43596";
>>
>> since the code already knows how to conjure the rest of the string.
>>
>> But check with Rob/Krzysztof.
>>
>> Yours,
>> Linus Walleij
> 
> Krzysztof, Rob [added to CC] - can I have your opinions?

I just got here bunch of quotes and no original message, so my response
probably won't be complete.

Devicetree also describes the system integration properties because we
need to know how the things are glued together.

We have firmware-name property which is a form of path. The
"directories" in the firmware-name are actually good because they allow
to create a hierarchy based on a vendor/SoC/model/board. I don't think
it is worth adding new properties replacing firmware-name.


Best regards,
Krzysztof

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 3059 bytes --]

On 11/28/2022 3:40 PM, Konrad Dybcio wrote:
> 
> 
> On 26.11.2022 22:45, Linus Walleij wrote:
>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>
>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>
>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>>>>
>>>>>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>>>>>>> don't think any would be really upstreamable..
>>>>>>>
>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>> a thing such as:
>>>>>>>
>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>      of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>     // Enforce FW version
>>>>>>> }
>>>>>>>
>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>>> details about firmware revisions.
>>>>>>>
>>>>>>> Yours,
>>>>>>> Linus Walleij
>>>>>>
>>>>>> Actually, I think I came up with a better approach by pulling a page
>>>>>> out of Asahi folks' book - please take a look and tell me what you
>>>>>> think about this:
>>>>>>
>>>>>> [1]
>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>> [2]
>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>
>> Something in this direction works too.
>>
>> The upside is that it tells all operating systems how to deal
>> with the firmware for this hardware.
>>
>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
>>>>> have directory names in Device Tree.
>>>>
>>>> I think it's common practice to include a full $FIRMWARE_DIR-relative
>>>> path when specifying firmware in DT, though here I left out the board
>>>> name bit as that's assigned dynamically anyway. That said, if you don't
>>>> like it, I can change it.
>>>
>>> It's just that I have understood that Device Tree is supposed to
>>> describe hardware and to me a firmware directory "brcm/" is a software
>>> property, not a hardware property. But this is really for the Device
>>> Tree maintainers to decide, they know this best :)
>>
>> I would personally just minimize the amount of information
>> put into the device tree to be exactly what is needed to find
>> the right firmware.
>>
>> brcm,firmware-compatible = "43596";
>>
>> since the code already knows how to conjure the rest of the string.
>>
>> But check with Rob/Krzysztof.
>>
>> Yours,
>> Linus Walleij
> 
> Krzysztof, Rob [added to CC] - can I have your opinions?

I tried catching up on this thread. Reading it I am not sure what the 
issue is, but I am happy to dive in. If you can provide a boot log with 
brcmfmac loaded with module parameter 'debug=0x1416' I can try and make 
sense of the chipid/devid confusion.

Regards,
Arend

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 1.12.2022 12:31, Arend van Spriel wrote:
> On 11/28/2022 3:40 PM, Konrad Dybcio wrote:
>>
>>
>> On 26.11.2022 22:45, Linus Walleij wrote:
>>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>
>>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>
>>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>>>>>
>>>>>>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>>>>>>>> don't think any would be really upstreamable..
>>>>>>>>
>>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>>> a thing such as:
>>>>>>>>
>>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>>      of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>>     // Enforce FW version
>>>>>>>> }
>>>>>>>>
>>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>>>> details about firmware revisions.
>>>>>>>>
>>>>>>>> Yours,
>>>>>>>> Linus Walleij
>>>>>>>
>>>>>>> Actually, I think I came up with a better approach by pulling a page
>>>>>>> out of Asahi folks' book - please take a look and tell me what you
>>>>>>> think about this:
>>>>>>>
>>>>>>> [1]
>>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>>> [2]
>>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>>
>>> Something in this direction works too.
>>>
>>> The upside is that it tells all operating systems how to deal
>>> with the firmware for this hardware.
>>>
>>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
>>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
>>>>>> have directory names in Device Tree.
>>>>>
>>>>> I think it's common practice to include a full $FIRMWARE_DIR-relative
>>>>> path when specifying firmware in DT, though here I left out the board
>>>>> name bit as that's assigned dynamically anyway. That said, if you don't
>>>>> like it, I can change it.
>>>>
>>>> It's just that I have understood that Device Tree is supposed to
>>>> describe hardware and to me a firmware directory "brcm/" is a software
>>>> property, not a hardware property. But this is really for the Device
>>>> Tree maintainers to decide, they know this best :)
>>>
>>> I would personally just minimize the amount of information
>>> put into the device tree to be exactly what is needed to find
>>> the right firmware.
>>>
>>> brcm,firmware-compatible = "43596";
>>>
>>> since the code already knows how to conjure the rest of the string.
>>>
>>> But check with Rob/Krzysztof.
>>>
>>> Yours,
>>> Linus Walleij
>>
>> Krzysztof, Rob [added to CC] - can I have your opinions?
> 
> I tried catching up on this thread. Reading it I am not sure what the issue is, but I am happy to dive in. If you can provide a boot log with brcmfmac loaded with module parameter 'debug=0x1416' I can try and make sense of the chipid/devid confusion.

Hope this helps, thanks! https://hastebin.com/xidagekuge.yaml

Konrad
> 
> Regards,
> Arend

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 3975 bytes --]

On 12/2/2022 11:33 AM, Konrad Dybcio wrote:
> 
> 
> On 1.12.2022 12:31, Arend van Spriel wrote:
>> On 11/28/2022 3:40 PM, Konrad Dybcio wrote:
>>>
>>>
>>> On 26.11.2022 22:45, Linus Walleij wrote:
>>>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>
>>>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>
>>>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>>>>>>
>>>>>>>>>> I can think of a couple of hacky ways to force use of 43596 fw, but I
>>>>>>>>>> don't think any would be really upstreamable..
>>>>>>>>>
>>>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>>>> a thing such as:
>>>>>>>>>
>>>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>>>       of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>>>      // Enforce FW version
>>>>>>>>> }
>>>>>>>>>
>>>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>>>>> details about firmware revisions.
>>>>>>>>>
>>>>>>>>> Yours,
>>>>>>>>> Linus Walleij
>>>>>>>>
>>>>>>>> Actually, I think I came up with a better approach by pulling a page
>>>>>>>> out of Asahi folks' book - please take a look and tell me what you
>>>>>>>> think about this:
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>>>> [2]
>>>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>>>
>>>> Something in this direction works too.
>>>>
>>>> The upside is that it tells all operating systems how to deal
>>>> with the firmware for this hardware.
>>>>
>>>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not provide
>>>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's unnecessary to
>>>>>>> have directory names in Device Tree.
>>>>>>
>>>>>> I think it's common practice to include a full $FIRMWARE_DIR-relative
>>>>>> path when specifying firmware in DT, though here I left out the board
>>>>>> name bit as that's assigned dynamically anyway. That said, if you don't
>>>>>> like it, I can change it.
>>>>>
>>>>> It's just that I have understood that Device Tree is supposed to
>>>>> describe hardware and to me a firmware directory "brcm/" is a software
>>>>> property, not a hardware property. But this is really for the Device
>>>>> Tree maintainers to decide, they know this best :)
>>>>
>>>> I would personally just minimize the amount of information
>>>> put into the device tree to be exactly what is needed to find
>>>> the right firmware.
>>>>
>>>> brcm,firmware-compatible = "43596";
>>>>
>>>> since the code already knows how to conjure the rest of the string.
>>>>
>>>> But check with Rob/Krzysztof.
>>>>
>>>> Yours,
>>>> Linus Walleij
>>>
>>> Krzysztof, Rob [added to CC] - can I have your opinions?
>>
>> I tried catching up on this thread. Reading it I am not sure what the issue is, but I am happy to dive in. If you can provide a boot log with brcmfmac loaded with module parameter 'debug=0x1416' I can try and make sense of the chipid/devid confusion.
> 
> Hope this helps, thanks! https://hastebin.com/xidagekuge.yaml

It does to some extent. It is basically a 4359 revision 9:

[   25.898782] brcmfmac: brcmf_chip_recognition found AXI chip: BCM4359/9

The 4359 entry in pcie.c is applicable for revision 0 and higher 
(doubtful but that is in the code):

	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),

We need to change the mask above to 0x000001FF and add a new entry with 
mask 0xFFFFFE00. All we need is come up with a reasonable firmware 
filename. So can you run the strings command on the firmware you use:

$ strings fw.bin | tail -1

and let me know the output.

Regards,
Arend
So for rev 9 we need a new entry

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Arend Van Spriel]

On 12/2/2022 4:26 PM, Arend van Spriel wrote:
> On 12/2/2022 11:33 AM, Konrad Dybcio wrote:
>>
>>
>> On 1.12.2022 12:31, Arend van Spriel wrote:
>>> On 11/28/2022 3:40 PM, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 26.11.2022 22:45, Linus Walleij wrote:
>>>>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>
>>>>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>>
>>>>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio 
>>>>>>>>>> <konrad.dybcio@linaro.org> wrote:
>>>>>>>>>>
>>>>>>>>>>> I can think of a couple of hacky ways to force use of 43596 
>>>>>>>>>>> fw, but I
>>>>>>>>>>> don't think any would be really upstreamable..
>>>>>>>>>>
>>>>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>>>>> a thing such as:
>>>>>>>>>>
>>>>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>>>>       of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>>>>      // Enforce FW version
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>>>>>> details about firmware revisions.
>>>>>>>>>>
>>>>>>>>>> Yours,
>>>>>>>>>> Linus Walleij
>>>>>>>>>
>>>>>>>>> Actually, I think I came up with a better approach by pulling a 
>>>>>>>>> page
>>>>>>>>> out of Asahi folks' book - please take a look and tell me what you
>>>>>>>>> think about this:
>>>>>>>>>
>>>>>>>>> [1]
>>>>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>>>>> [2]
>>>>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>>>>
>>>>> Something in this direction works too.
>>>>>
>>>>> The upside is that it tells all operating systems how to deal
>>>>> with the firmware for this hardware.
>>>>>
>>>>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not 
>>>>>>>> provide
>>>>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's 
>>>>>>>> unnecessary to
>>>>>>>> have directory names in Device Tree.
>>>>>>>
>>>>>>> I think it's common practice to include a full 
>>>>>>> $FIRMWARE_DIR-relative
>>>>>>> path when specifying firmware in DT, though here I left out the 
>>>>>>> board
>>>>>>> name bit as that's assigned dynamically anyway. That said, if you 
>>>>>>> don't
>>>>>>> like it, I can change it.
>>>>>>
>>>>>> It's just that I have understood that Device Tree is supposed to
>>>>>> describe hardware and to me a firmware directory "brcm/" is a 
>>>>>> software
>>>>>> property, not a hardware property. But this is really for the Device
>>>>>> Tree maintainers to decide, they know this best :)
>>>>>
>>>>> I would personally just minimize the amount of information
>>>>> put into the device tree to be exactly what is needed to find
>>>>> the right firmware.
>>>>>
>>>>> brcm,firmware-compatible = "43596";
>>>>>
>>>>> since the code already knows how to conjure the rest of the string.
>>>>>
>>>>> But check with Rob/Krzysztof.
>>>>>
>>>>> Yours,
>>>>> Linus Walleij
>>>>
>>>> Krzysztof, Rob [added to CC] - can I have your opinions?
>>>
>>> I tried catching up on this thread. Reading it I am not sure what the 
>>> issue is, but I am happy to dive in. If you can provide a boot log 
>>> with brcmfmac loaded with module parameter 'debug=0x1416' I can try 
>>> and make sense of the chipid/devid confusion.
>>
>> Hope this helps, thanks! https://hastebin.com/xidagekuge.yaml
> 
> It does to some extent. It is basically a 4359 revision 9:
> 
> [   25.898782] brcmfmac: brcmf_chip_recognition found AXI chip: BCM4359/9
> 
> The 4359 entry in pcie.c is applicable for revision 0 and higher 
> (doubtful but that is in the code):
> 
>      BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
> 
> We need to change the mask above to 0x000001FF and add a new entry with 
> mask 0xFFFFFE00. All we need is come up with a reasonable firmware 
> filename. So can you run the strings command on the firmware you use:
> 
> $ strings fw.bin | tail -1
> 
> and let me know the output.

Actually realized you already provided a URL to the repo containing the 
firmware you used. So I had a look and it shows:

43596a0-roml/pcie-ag-apcs-pktctx-proptxstatus-ampduhostreorder-lpc-die3-olpc-pspretend-mfp-ltecx-clm_43xx_somc_mimo-phyflags-txpwrctrls-dpo 
Version: 9.75.119.15 (r691661) CRC: a6cf427b Date: Fri 2017-03-24 
13:24:25 KST Ucode Ver: 1060.20542 FWID: 01-e4abc35c

However, from firmware perspective this is equivalent to 4359c0 so I 
would suggest the change below.

Let me know if that works.

Regards,
Arend
---
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index cf564adc612a..b59cf0f2939c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -55,6 +55,7 @@ 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");
+BCRMF_FW_DEF(4359C, "brcmfmac4359c-pcie");
  BRCMF_FW_DEF(4364, "brcmfmac4364-pcie");
  BRCMF_FW_DEF(4365B, "brcmfmac4365b-pcie");
  BRCMF_FW_DEF(4365C, "brcmfmac4365c-pcie");
@@ -83,7 +84,8 @@ static const struct brcmf_firmware_mapping 
brcmf_pcie_fwnames[] = {
  	BRCMF_FW_ENTRY(BRCM_CC_43569_CHIP_ID, 0xFFFFFFFF, 43570),
  	BRCMF_FW_ENTRY(BRCM_CC_43570_CHIP_ID, 0xFFFFFFFF, 43570),
  	BRCMF_FW_ENTRY(BRCM_CC_4358_CHIP_ID, 0xFFFFFFFF, 4358),
-	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
+	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0x000001FF, 4359),
+	BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFE00, 4359C),
  	BRCMF_FW_ENTRY(BRCM_CC_4364_CHIP_ID, 0xFFFFFFFF, 4364),
  	BRCMF_FW_ENTRY(BRCM_CC_4365_CHIP_ID, 0x0000000F, 4365B),
  	BRCMF_FW_ENTRY(BRCM_CC_4365_CHIP_ID, 0xFFFFFFF0, 4365C),

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 02/12/2022 20:28, Arend Van Spriel wrote:
> 
> 
> On 12/2/2022 4:26 PM, Arend van Spriel wrote:
>> On 12/2/2022 11:33 AM, Konrad Dybcio wrote:
>>>
>>>
>>> On 1.12.2022 12:31, Arend van Spriel wrote:
>>>> On 11/28/2022 3:40 PM, Konrad Dybcio wrote:
>>>>>
>>>>>
>>>>> On 26.11.2022 22:45, Linus Walleij wrote:
>>>>>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>
>>>>>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>>>
>>>>>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio 
>>>>>>>>>>> <konrad.dybcio@linaro.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> I can think of a couple of hacky ways to force use of 43596 
>>>>>>>>>>>> fw, but I
>>>>>>>>>>>> don't think any would be really upstreamable..
>>>>>>>>>>>
>>>>>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>>>>>> a thing such as:
>>>>>>>>>>>
>>>>>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>>>>>       of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>>>>>      // Enforce FW version
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>>>>>>> details about firmware revisions.
>>>>>>>>>>>
>>>>>>>>>>> Yours,
>>>>>>>>>>> Linus Walleij
>>>>>>>>>>
>>>>>>>>>> Actually, I think I came up with a better approach by pulling 
>>>>>>>>>> a page
>>>>>>>>>> out of Asahi folks' book - please take a look and tell me what 
>>>>>>>>>> you
>>>>>>>>>> think about this:
>>>>>>>>>>
>>>>>>>>>> [1]
>>>>>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>>>>>> [2]
>>>>>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>>>>>
>>>>>> Something in this direction works too.
>>>>>>
>>>>>> The upside is that it tells all operating systems how to deal
>>>>>> with the firmware for this hardware.
>>>>>>
>>>>>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why not 
>>>>>>>>> provide
>>>>>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's 
>>>>>>>>> unnecessary to
>>>>>>>>> have directory names in Device Tree.
>>>>>>>>
>>>>>>>> I think it's common practice to include a full 
>>>>>>>> $FIRMWARE_DIR-relative
>>>>>>>> path when specifying firmware in DT, though here I left out the 
>>>>>>>> board
>>>>>>>> name bit as that's assigned dynamically anyway. That said, if 
>>>>>>>> you don't
>>>>>>>> like it, I can change it.
>>>>>>>
>>>>>>> It's just that I have understood that Device Tree is supposed to
>>>>>>> describe hardware and to me a firmware directory "brcm/" is a 
>>>>>>> software
>>>>>>> property, not a hardware property. But this is really for the Device
>>>>>>> Tree maintainers to decide, they know this best :)
>>>>>>
>>>>>> I would personally just minimize the amount of information
>>>>>> put into the device tree to be exactly what is needed to find
>>>>>> the right firmware.
>>>>>>
>>>>>> brcm,firmware-compatible = "43596";
>>>>>>
>>>>>> since the code already knows how to conjure the rest of the string.
>>>>>>
>>>>>> But check with Rob/Krzysztof.
>>>>>>
>>>>>> Yours,
>>>>>> Linus Walleij
>>>>>
>>>>> Krzysztof, Rob [added to CC] - can I have your opinions?
>>>>
>>>> I tried catching up on this thread. Reading it I am not sure what 
>>>> the issue is, but I am happy to dive in. If you can provide a boot 
>>>> log with brcmfmac loaded with module parameter 'debug=0x1416' I can 
>>>> try and make sense of the chipid/devid confusion.
>>>
>>> Hope this helps, thanks! https://hastebin.com/xidagekuge.yaml
>>
>> It does to some extent. It is basically a 4359 revision 9:
>>
>> [   25.898782] brcmfmac: brcmf_chip_recognition found AXI chip: BCM4359/9
>>
>> The 4359 entry in pcie.c is applicable for revision 0 and higher 
>> (doubtful but that is in the code):
>>
>>      BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
>>
>> We need to change the mask above to 0x000001FF and add a new entry 
>> with mask 0xFFFFFE00. All we need is come up with a reasonable 
>> firmware filename. So can you run the strings command on the firmware 
>> you use:
>>
>> $ strings fw.bin | tail -1
>>
>> and let me know the output.
> 
> Actually realized you already provided a URL to the repo containing the 
> firmware you used. So I had a look and it shows:
> 
> 43596a0-roml/pcie-ag-apcs-pktctx-proptxstatus-ampduhostreorder-lpc-die3-olpc-pspretend-mfp-ltecx-clm_43xx_somc_mimo-phyflags-txpwrctrls-dpo Version: 9.75.119.15 (r691661) CRC: a6cf427b Date: Fri 2017-03-24 13:24:25 KST Ucode Ver: 1060.20542 FWID: 01-e4abc35c
> 
> However, from firmware perspective this is equivalent to 4359c0 so I 
> would suggest the change below.
> 
> Let me know if that works.
Sorry for the late reply.

Yes, it does seem to work just fine! The kernel now looks for 
brcm/brcmfmac4359c-pcie.sony,kagura-row.bin as we would expect.

Could you submit this patch below to supersede my one?

Konrad
> 
> Regards,
> Arend
> ---
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index cf564adc612a..b59cf0f2939c 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -55,6 +55,7 @@ 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");
> +BCRMF_FW_DEF(4359C, "brcmfmac4359c-pcie");
>   BRCMF_FW_DEF(4364, "brcmfmac4364-pcie");
>   BRCMF_FW_DEF(4365B, "brcmfmac4365b-pcie");
>   BRCMF_FW_DEF(4365C, "brcmfmac4365c-pcie");
> @@ -83,7 +84,8 @@ static const struct brcmf_firmware_mapping 
> brcmf_pcie_fwnames[] = {
>       BRCMF_FW_ENTRY(BRCM_CC_43569_CHIP_ID, 0xFFFFFFFF, 43570),
>       BRCMF_FW_ENTRY(BRCM_CC_43570_CHIP_ID, 0xFFFFFFFF, 43570),
>       BRCMF_FW_ENTRY(BRCM_CC_4358_CHIP_ID, 0xFFFFFFFF, 4358),
> -    BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
> +    BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0x000001FF, 4359),
> +    BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFE00, 4359C),
>       BRCMF_FW_ENTRY(BRCM_CC_4364_CHIP_ID, 0xFFFFFFFF, 4364),
>       BRCMF_FW_ENTRY(BRCM_CC_4365_CHIP_ID, 0x0000000F, 4365B),
>       BRCMF_FW_ENTRY(BRCM_CC_4365_CHIP_ID, 0xFFFFFFF0, 4365C),
> 

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 5433 bytes --]

On 12/6/2022 10:58 AM, Konrad Dybcio wrote:
> 
> 
> On 02/12/2022 20:28, Arend Van Spriel wrote:
>>
>>
>> On 12/2/2022 4:26 PM, Arend van Spriel wrote:
>>> On 12/2/2022 11:33 AM, Konrad Dybcio wrote:
>>>>
>>>>
>>>> On 1.12.2022 12:31, Arend van Spriel wrote:
>>>>> On 11/28/2022 3:40 PM, Konrad Dybcio wrote:
>>>>>>
>>>>>>
>>>>>> On 26.11.2022 22:45, Linus Walleij wrote:
>>>>>>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> wrote:
>>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>>
>>>>>>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>>>>
>>>>>>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio 
>>>>>>>>>>>> <konrad.dybcio@linaro.org> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> I can think of a couple of hacky ways to force use of 43596 
>>>>>>>>>>>>> fw, but I
>>>>>>>>>>>>> don't think any would be really upstreamable..
>>>>>>>>>>>>
>>>>>>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>>>>>>> a thing such as:
>>>>>>>>>>>>
>>>>>>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>>>>>>       of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>>>>>>      // Enforce FW version
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>>>>>>> problem from the top instead of trying to figure out itsy witsy
>>>>>>>>>>>> details about firmware revisions.
>>>>>>>>>>>>
>>>>>>>>>>>> Yours,
>>>>>>>>>>>> Linus Walleij
>>>>>>>>>>>
>>>>>>>>>>> Actually, I think I came up with a better approach by pulling 
>>>>>>>>>>> a page
>>>>>>>>>>> out of Asahi folks' book - please take a look and tell me 
>>>>>>>>>>> what you
>>>>>>>>>>> think about this:
>>>>>>>>>>>
>>>>>>>>>>> [1]
>>>>>>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>>>>>>> [2]
>>>>>>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>>>>>>
>>>>>>> Something in this direction works too.
>>>>>>>
>>>>>>> The upside is that it tells all operating systems how to deal
>>>>>>> with the firmware for this hardware.
>>>>>>>
>>>>>>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why 
>>>>>>>>>> not provide
>>>>>>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's 
>>>>>>>>>> unnecessary to
>>>>>>>>>> have directory names in Device Tree.
>>>>>>>>>
>>>>>>>>> I think it's common practice to include a full 
>>>>>>>>> $FIRMWARE_DIR-relative
>>>>>>>>> path when specifying firmware in DT, though here I left out the 
>>>>>>>>> board
>>>>>>>>> name bit as that's assigned dynamically anyway. That said, if 
>>>>>>>>> you don't
>>>>>>>>> like it, I can change it.
>>>>>>>>
>>>>>>>> It's just that I have understood that Device Tree is supposed to
>>>>>>>> describe hardware and to me a firmware directory "brcm/" is a 
>>>>>>>> software
>>>>>>>> property, not a hardware property. But this is really for the 
>>>>>>>> Device
>>>>>>>> Tree maintainers to decide, they know this best :)
>>>>>>>
>>>>>>> I would personally just minimize the amount of information
>>>>>>> put into the device tree to be exactly what is needed to find
>>>>>>> the right firmware.
>>>>>>>
>>>>>>> brcm,firmware-compatible = "43596";
>>>>>>>
>>>>>>> since the code already knows how to conjure the rest of the string.
>>>>>>>
>>>>>>> But check with Rob/Krzysztof.
>>>>>>>
>>>>>>> Yours,
>>>>>>> Linus Walleij
>>>>>>
>>>>>> Krzysztof, Rob [added to CC] - can I have your opinions?
>>>>>
>>>>> I tried catching up on this thread. Reading it I am not sure what 
>>>>> the issue is, but I am happy to dive in. If you can provide a boot 
>>>>> log with brcmfmac loaded with module parameter 'debug=0x1416' I can 
>>>>> try and make sense of the chipid/devid confusion.
>>>>
>>>> Hope this helps, thanks! https://hastebin.com/xidagekuge.yaml
>>>
>>> It does to some extent. It is basically a 4359 revision 9:
>>>
>>> [   25.898782] brcmfmac: brcmf_chip_recognition found AXI chip: 
>>> BCM4359/9
>>>
>>> The 4359 entry in pcie.c is applicable for revision 0 and higher 
>>> (doubtful but that is in the code):
>>>
>>>      BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
>>>
>>> We need to change the mask above to 0x000001FF and add a new entry 
>>> with mask 0xFFFFFE00. All we need is come up with a reasonable 
>>> firmware filename. So can you run the strings command on the firmware 
>>> you use:
>>>
>>> $ strings fw.bin | tail -1
>>>
>>> and let me know the output.
>>
>> Actually realized you already provided a URL to the repo containing 
>> the firmware you used. So I had a look and it shows:
>>
>> 43596a0-roml/pcie-ag-apcs-pktctx-proptxstatus-ampduhostreorder-lpc-die3-olpc-pspretend-mfp-ltecx-clm_43xx_somc_mimo-phyflags-txpwrctrls-dpo Version: 9.75.119.15 (r691661) CRC: a6cf427b Date: Fri 2017-03-24 13:24:25 KST Ucode Ver: 1060.20542 FWID: 01-e4abc35c
>>
>> However, from firmware perspective this is equivalent to 4359c0 so I 
>> would suggest the change below.
>>
>> Let me know if that works.
> Sorry for the late reply.
> 
> Yes, it does seem to work just fine! The kernel now looks for 
> brcm/brcmfmac4359c-pcie.sony,kagura-row.bin as we would expect.
> 
> Could you submit this patch below to supersede my one?

I have no problem when you include this patch in yours and submit it to 
the linux-wireless list.

Regards,
Arend

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Linus Walleij]

On Tue, Dec 6, 2022 at 10:59 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:

> Yes, it does seem to work just fine! The kernel now looks for
> brcm/brcmfmac4359c-pcie.sony,kagura-row.bin as we would expect.

So the Sony kagura needs a special brcmfmac firmware like so many
other mobile phones. There are a few Samsungs with custom firmware
as well.

Arend: what is the legal situation with these custom firmwares?

I was under the impression that Broadcom actually made these,
so they could in theory be given permission for redistribution in
linux-firmware?

Some that I have are newer versions than what is in Linux-firmware,
e.g this from linux-firmware:

brcm/brcmfmac4330-sdio.bin
4330b2-roml/sdio-ag-pool-ccx-btamp-p2p-idsup-idauth-proptxstatus-pno-aoe-toe-pktfilter-keepalive
Version: 5.90.125.104 CRC: 2570e6a3 Date: Tue 2011-10-25 19:34:26 PDT

There is this found in Samsung Codina GT-I8160:
4330b2-roml/sdio-g-p2p-aoe-pktfilter-keepalive-pno-ccx-wepso Version:
5.99.10.0 CRC: 4f7fccf Date: Wed 2012-12-05 01:02:50 PST FWID
01-52653ba9

Or:
brcmfmac4334-sdio.bin
4334b1min-roml/sdio-ag-pno-p2p-ccx-extsup-proptxstatus-dmatxrc-rxov-pktfilter-keepalive-aoe-vsdb-wapi-wl11d
Version: 6.10.0.0 CRC: 31410dd4 Date: Tue 2012-06-26 11:33:07 PDT FWID
01-8ee3be86

There is this found in Samsung Golden GT-I8190N:
4334b1-roml/sdio-ag-p2p-extsup-aoe-pktfilter-keepalive-pno-dmatxrc-rxov-proptxstatus-vsdb-mchan-okc-rcc-fmc-wepso-txpwr-autoabn-sr
Version: 6.10.58.99 CRC: 828f9174 Date: Mon 2013-08-26 02:13:44 PDT
FWID 01-e39d4d77

So in some cases more than a year newer firmware versions
compared to linux-firmware, I guess also customized for the
phones, but I can't really tell because we don't have anything
of similar date in linux-firmware.

Yours,
Linus Walleij

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Arend Van Spriel]

[-- Attachment #1: Type: text/plain, Size: 2761 bytes --]

On December 7, 2022 12:38:10 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> On Tue, Dec 6, 2022 at 10:59 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
>> Yes, it does seem to work just fine! The kernel now looks for
>> brcm/brcmfmac4359c-pcie.sony,kagura-row.bin as we would expect.
>
> So the Sony kagura needs a special brcmfmac firmware like so many
> other mobile phones. There are a few Samsungs with custom firmware
> as well.

Hi Linus,

It is not so much the device requiring custom firmware, but the customer 
gets firmware with a set of feature that fulfills their functional 
requirements. The brcmfmac driver has feature detection so this doesn't 
necessarily pose any issues. When adding new device support we enable all 
revisions, current and future, until proven otherwise. When it doesn't we 
have to update the firmware mapping like what needs to done for sony kagura.

> Arend: what is the legal situation with these custom firmwares?
>
> I was under the impression that Broadcom actually made these,
> so they could in theory be given permission for redistribution in
> linux-firmware?

In theory we could. However, we can not ack/review firmware patches posted 
by the community. So we release firmware with a feature set that matches 
brcmfmac and verify it works with brcmfmac.

> Some that I have are newer versions than what is in Linux-firmware,
> e.g this from linux-firmware:
>
> brcm/brcmfmac4330-sdio.bin
> 4330b2-roml/sdio-ag-pool-ccx-btamp-p2p-idsup-idauth-proptxstatus-pno-aoe-toe-pktfilter-keepalive
> Version: 5.90.125.104 CRC: 2570e6a3 Date: Tue 2011-10-25 19:34:26 PDT
>
> There is this found in Samsung Codina GT-I8160:
> 4330b2-roml/sdio-g-p2p-aoe-pktfilter-keepalive-pno-ccx-wepso Version:
> 5.99.10.0 CRC: 4f7fccf Date: Wed 2012-12-05 01:02:50 PST FWID
> 01-52653ba9

Right. The firmware in linux-firmware seems to be dual and, ie. -ag- versus 
-g-.

> Or:
> brcmfmac4334-sdio.bin
> 4334b1min-roml/sdio-ag-pno-p2p-ccx-extsup-proptxstatus-dmatxrc-rxov-pktfilter-keepalive-aoe-vsdb-wapi-wl11d
> Version: 6.10.0.0 CRC: 31410dd4 Date: Tue 2012-06-26 11:33:07 PDT FWID
> 01-8ee3be86
>
> There is this found in Samsung Golden GT-I8190N:
> 4334b1-roml/sdio-ag-p2p-extsup-aoe-pktfilter-keepalive-pno-dmatxrc-rxov-proptxstatus-vsdb-mchan-okc-rcc-fmc-wepso-txpwr-autoabn-sr
> Version: 6.10.58.99 CRC: 828f9174 Date: Mon 2013-08-26 02:13:44 PDT
> FWID 01-e39d4d77
>
> So in some cases more than a year newer firmware versions
> compared to linux-firmware, I guess also customized for the
> phones, but I can't really tell because we don't have anything
> of similar date in linux-firmware.

I am actually preparing series of firmware patches. As most chips are EOL 
those are newer, but not recent.

Regards,
Arend



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 07/12/2022 00:37, Linus Walleij wrote:
> On Tue, Dec 6, 2022 at 10:59 AM Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> 
>> Yes, it does seem to work just fine! The kernel now looks for
>> brcm/brcmfmac4359c-pcie.sony,kagura-row.bin as we would expect.
> 
> So the Sony kagura needs a special brcmfmac firmware like so many
> other mobile phones. There are a few Samsungs with custom firmware
> as well.
FWIW, Sony did a great job and agreed to license calibration files under 
CC0-1.0 [1] and the firmwares itself [2] have a Broadcom license 
attached to it.

Konrad

[1] 
https://github.com/sonyxperiadev/device-sony-kagura/commit/1f633325a3890864503b5e19f581d1b6a538996c
[2] 
https://github.com/sonyxperiadev/vendor-broadcom-wlan/tree/master/bcmdhd/firmware
> 
> Arend: what is the legal situation with these custom firmwares?
> 
> I was under the impression that Broadcom actually made these,
> so they could in theory be given permission for redistribution in
> linux-firmware?
> 
> Some that I have are newer versions than what is in Linux-firmware,
> e.g this from linux-firmware:
> 
> brcm/brcmfmac4330-sdio.bin
> 4330b2-roml/sdio-ag-pool-ccx-btamp-p2p-idsup-idauth-proptxstatus-pno-aoe-toe-pktfilter-keepalive
> Version: 5.90.125.104 CRC: 2570e6a3 Date: Tue 2011-10-25 19:34:26 PDT
> 
> There is this found in Samsung Codina GT-I8160:
> 4330b2-roml/sdio-g-p2p-aoe-pktfilter-keepalive-pno-ccx-wepso Version:
> 5.99.10.0 CRC: 4f7fccf Date: Wed 2012-12-05 01:02:50 PST FWID
> 01-52653ba9
> 
> Or:
> brcmfmac4334-sdio.bin
> 4334b1min-roml/sdio-ag-pno-p2p-ccx-extsup-proptxstatus-dmatxrc-rxov-pktfilter-keepalive-aoe-vsdb-wapi-wl11d
> Version: 6.10.0.0 CRC: 31410dd4 Date: Tue 2012-06-26 11:33:07 PDT FWID
> 01-8ee3be86
> 
> There is this found in Samsung Golden GT-I8190N:
> 4334b1-roml/sdio-ag-p2p-extsup-aoe-pktfilter-keepalive-pno-dmatxrc-rxov-proptxstatus-vsdb-mchan-okc-rcc-fmc-wepso-txpwr-autoabn-sr
> Version: 6.10.58.99 CRC: 828f9174 Date: Mon 2013-08-26 02:13:44 PDT
> FWID 01-e39d4d77
> 
> So in some cases more than a year newer firmware versions
> compared to linux-firmware, I guess also customized for the
> phones, but I can't really tell because we don't have anything
> of similar date in linux-firmware.
> 
> Yours,
> Linus Walleij

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Konrad Dybcio]

On 06/12/2022 12:22, Arend van Spriel wrote:
> On 12/6/2022 10:58 AM, Konrad Dybcio wrote:
>>
>>
>> On 02/12/2022 20:28, Arend Van Spriel wrote:
>>>
>>>
>>> On 12/2/2022 4:26 PM, Arend van Spriel wrote:
>>>> On 12/2/2022 11:33 AM, Konrad Dybcio wrote:
>>>>>
>>>>>
>>>>> On 1.12.2022 12:31, Arend van Spriel wrote:
>>>>>> On 11/28/2022 3:40 PM, Konrad Dybcio wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 26.11.2022 22:45, Linus Walleij wrote:
>>>>>>>> On Fri, Nov 25, 2022 at 1:25 PM Kalle Valo <kvalo@kernel.org> 
>>>>>>>> wrote:
>>>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>>>
>>>>>>>>>> On 25.11.2022 12:53, Kalle Valo wrote:
>>>>>>>>>>> Konrad Dybcio <konrad.dybcio@linaro.org> writes:
>>>>>>>>>>>
>>>>>>>>>>>> On 21.11.2022 14:56, Linus Walleij wrote:
>>>>>>>>>>>>> On Fri, Nov 18, 2022 at 5:47 PM Konrad Dybcio 
>>>>>>>>>>>>> <konrad.dybcio@linaro.org> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> I can think of a couple of hacky ways to force use of 
>>>>>>>>>>>>>> 43596 fw, but I
>>>>>>>>>>>>>> don't think any would be really upstreamable..
>>>>>>>>>>>>>
>>>>>>>>>>>>> If it is only known to affect the Sony Xperias mentioned then
>>>>>>>>>>>>> a thing such as:
>>>>>>>>>>>>>
>>>>>>>>>>>>> if (of_machine_is_compatible("sony,xyz") ||
>>>>>>>>>>>>>       of_machine_is_compatible("sony,zzz")... ) {
>>>>>>>>>>>>>      // Enforce FW version
>>>>>>>>>>>>> }
>>>>>>>>>>>>>
>>>>>>>>>>>>> would be completely acceptable in my book. It hammers the
>>>>>>>>>>>>> problem from the top instead of trying to figure out itsy 
>>>>>>>>>>>>> witsy
>>>>>>>>>>>>> details about firmware revisions.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yours,
>>>>>>>>>>>>> Linus Walleij
>>>>>>>>>>>>
>>>>>>>>>>>> Actually, I think I came up with a better approach by 
>>>>>>>>>>>> pulling a page
>>>>>>>>>>>> out of Asahi folks' book - please take a look and tell me 
>>>>>>>>>>>> what you
>>>>>>>>>>>> think about this:
>>>>>>>>>>>>
>>>>>>>>>>>> [1]
>>>>>>>>>>>> https://github.com/SoMainline/linux/commit/4b6fccc995cd79109b0dae4e4ab2e48db97695e7
>>>>>>>>>>>> [2]
>>>>>>>>>>>> https://github.com/SoMainline/linux/commit/e3ea1dc739634f734104f37fdbed046873921af7
>>>>>>>>
>>>>>>>> Something in this direction works too.
>>>>>>>>
>>>>>>>> The upside is that it tells all operating systems how to deal
>>>>>>>> with the firmware for this hardware.
>>>>>>>>
>>>>>>>>>>> Instead of a directory path ("brcm/brcmfmac43596-pcie") why 
>>>>>>>>>>> not provide
>>>>>>>>>>> just the chipset name ("brcmfmac43596-pcie")? IMHO it's 
>>>>>>>>>>> unnecessary to
>>>>>>>>>>> have directory names in Device Tree.
>>>>>>>>>>
>>>>>>>>>> I think it's common practice to include a full 
>>>>>>>>>> $FIRMWARE_DIR-relative
>>>>>>>>>> path when specifying firmware in DT, though here I left out 
>>>>>>>>>> the board
>>>>>>>>>> name bit as that's assigned dynamically anyway. That said, if 
>>>>>>>>>> you don't
>>>>>>>>>> like it, I can change it.
>>>>>>>>>
>>>>>>>>> It's just that I have understood that Device Tree is supposed to
>>>>>>>>> describe hardware and to me a firmware directory "brcm/" is a 
>>>>>>>>> software
>>>>>>>>> property, not a hardware property. But this is really for the 
>>>>>>>>> Device
>>>>>>>>> Tree maintainers to decide, they know this best :)
>>>>>>>>
>>>>>>>> I would personally just minimize the amount of information
>>>>>>>> put into the device tree to be exactly what is needed to find
>>>>>>>> the right firmware.
>>>>>>>>
>>>>>>>> brcm,firmware-compatible = "43596";
>>>>>>>>
>>>>>>>> since the code already knows how to conjure the rest of the string.
>>>>>>>>
>>>>>>>> But check with Rob/Krzysztof.
>>>>>>>>
>>>>>>>> Yours,
>>>>>>>> Linus Walleij
>>>>>>>
>>>>>>> Krzysztof, Rob [added to CC] - can I have your opinions?
>>>>>>
>>>>>> I tried catching up on this thread. Reading it I am not sure what 
>>>>>> the issue is, but I am happy to dive in. If you can provide a boot 
>>>>>> log with brcmfmac loaded with module parameter 'debug=0x1416' I 
>>>>>> can try and make sense of the chipid/devid confusion.
>>>>>
>>>>> Hope this helps, thanks! https://hastebin.com/xidagekuge.yaml
>>>>
>>>> It does to some extent. It is basically a 4359 revision 9:
>>>>
>>>> [   25.898782] brcmfmac: brcmf_chip_recognition found AXI chip: 
>>>> BCM4359/9
>>>>
>>>> The 4359 entry in pcie.c is applicable for revision 0 and higher 
>>>> (doubtful but that is in the code):
>>>>
>>>>      BRCMF_FW_ENTRY(BRCM_CC_4359_CHIP_ID, 0xFFFFFFFF, 4359),
>>>>
>>>> We need to change the mask above to 0x000001FF and add a new entry 
>>>> with mask 0xFFFFFE00. All we need is come up with a reasonable 
>>>> firmware filename. So can you run the strings command on the 
>>>> firmware you use:
>>>>
>>>> $ strings fw.bin | tail -1
>>>>
>>>> and let me know the output.
>>>
>>> Actually realized you already provided a URL to the repo containing 
>>> the firmware you used. So I had a look and it shows:
>>>
>>> 43596a0-roml/pcie-ag-apcs-pktctx-proptxstatus-ampduhostreorder-lpc-die3-olpc-pspretend-mfp-ltecx-clm_43xx_somc_mimo-phyflags-txpwrctrls-dpo Version: 9.75.119.15 (r691661) CRC: a6cf427b Date: Fri 2017-03-24 13:24:25 KST Ucode Ver: 1060.20542 FWID: 01-e4abc35c
>>>
>>> However, from firmware perspective this is equivalent to 4359c0 so I 
>>> would suggest the change below.
>>>
>>> Let me know if that works.
>> Sorry for the late reply.
>>
>> Yes, it does seem to work just fine! The kernel now looks for 
>> brcm/brcmfmac4359c-pcie.sony,kagura-row.bin as we would expect.
>>
>> Could you submit this patch below to supersede my one?
> 
> I have no problem when you include this patch in yours and submit it to 
> the linux-wireless list.
Thanks, I'll do that soon!

Konrad
> 
> Regards,
> Arend

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

On 12/7/2022 9:57 AM, Konrad Dybcio wrote:
> 
> 
> On 07/12/2022 00:37, Linus Walleij wrote:
>> On Tue, Dec 6, 2022 at 10:59 AM Konrad Dybcio 
>> <konrad.dybcio@linaro.org> wrote:
>>
>>> Yes, it does seem to work just fine! The kernel now looks for
>>> brcm/brcmfmac4359c-pcie.sony,kagura-row.bin as we would expect.
>>
>> So the Sony kagura needs a special brcmfmac firmware like so many
>> other mobile phones. There are a few Samsungs with custom firmware
>> as well.
> FWIW, Sony did a great job and agreed to license calibration files under 
> CC0-1.0 [1] and the firmwares itself [2] have a Broadcom license 
> attached to it.
The Broadcom license in [2] is probably not appropriate (have not 
checked it) and I would like to avoid having multiple license files in 
linux-firmware for Broadcom firmwares.

Regards,
Arend

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Linus Walleij]

On Wed, Dec 7, 2022 at 8:42 AM Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:

> I am actually preparing series of firmware patches. As most chips are EOL
> those are newer, but not recent.

Excellent, I'll test them on my specimen and see what happens!

Yours,
Linus Walleij

Re: [PATCH v2] brcmfmac: Add support for BCM43596 PCIe Wi-Fi

[Arend van Spriel]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

On 12/7/2022 2:42 PM, Linus Walleij wrote:
> On Wed, Dec 7, 2022 at 8:42 AM Arend Van Spriel
> <arend.vanspriel@broadcom.com> wrote:
> 
>> I am actually preparing series of firmware patches. As most chips are EOL
>> those are newer, but not recent.
> 
> Excellent, I'll test them on my specimen and see what happens!

I posted the patch but it bounced on the linux-wireless list. I think I 
will repost and split it up in individual patches.

Regards,
Arend

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]