diff mbox series

[v3,5/5] wifi: brcmfmac: Add support for AP6275P

Message ID 20240630073605.2164346-6-jacobe.zang@wesion.com (mailing list archive)
State New
Headers show
Series Add AP6275P wireless support | expand

Commit Message

Jacobe Zang June 30, 2024, 7:36 a.m. UTC 
This module features BCM43752A2 chipset. The firmware requires
randomness seeding, so enabled it.

Co-developed-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
Link: https://megous.com/git/linux/commit/?h=ap6275p-6.10&id=1a99573bc8ed412e60e1969c0b29d53a0e5782e0
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c      | 5 ++++-
 .../net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h    | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

Comments

Stefan Wahren June 30, 2024, 8:34 a.m. UTC | #1 
Am 30.06.24 um 09:36 schrieb Jacobe Zang:
> This module features BCM43752A2 chipset. The firmware requires
> randomness seeding, so enabled it.
>
> Co-developed-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
> Link: https://megous.com/git/linux/commit/?h=ap6275p-6.10&id=1a99573bc8ed412e60e1969c0b29d53a0e5782e0
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c      | 5 ++++-
>   .../net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h    | 2 ++
>   2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index e84f562fc91b8..f427d664cf3a5 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -71,6 +71,7 @@ BRCMF_FW_CLM_DEF(4377B3, "brcmfmac4377b3-pcie");
>   BRCMF_FW_CLM_DEF(4378B1, "brcmfmac4378b1-pcie");
>   BRCMF_FW_CLM_DEF(4378B3, "brcmfmac4378b3-pcie");
>   BRCMF_FW_CLM_DEF(4387C2, "brcmfmac4387c2-pcie");
> +BRCMF_FW_CLM_DEF(43752, "brcmfmac43752-pcie");
>
>   /* firmware config files */
>   MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.txt");
> @@ -105,6 +106,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
>   	BRCMF_FW_ENTRY(BRCM_CC_43664_CHIP_ID, 0xFFFFFFF0, 4366C),
>   	BRCMF_FW_ENTRY(BRCM_CC_43666_CHIP_ID, 0xFFFFFFF0, 4366C),
>   	BRCMF_FW_ENTRY(BRCM_CC_4371_CHIP_ID, 0xFFFFFFFF, 4371),
> +	BRCMF_FW_ENTRY(BRCM_CC_43752_CHIP_ID, 0xFFFFFFFF, 43752),
>   	BRCMF_FW_ENTRY(BRCM_CC_4377_CHIP_ID, 0xFFFFFFFF, 4377B3), /* revision ID 4 */
>   	BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0x0000000F, 4378B1), /* revision ID 3 */
>   	BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0xFFFFFFE0, 4378B3), /* revision ID 5 */
> @@ -1721,7 +1723,7 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
>   		memcpy_toio(devinfo->tcm + address, nvram, nvram_len);
>   		brcmf_fw_nvram_free(nvram);
>
> -		if (devinfo->otp.valid) {
> +		if (devinfo->otp.valid || devinfo->ci->chip == BRCM_CC_43752_CHIP_ID) {
As a reviewer, i would expect an explanation in the commit message or in
the code for this quirk.
>   			size_t rand_len = BRCMF_RANDOM_SEED_LENGTH;
>   			struct brcmf_random_seed_footer footer = {
>   				.length = cpu_to_le32(rand_len),
> @@ -2710,6 +2712,7 @@ static const struct pci_device_id brcmf_pcie_devid_table[] = {
>   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4366_5G_DEVICE_ID, BCA),
>   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4371_DEVICE_ID, WCC),
>   	BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_DEVICE_ID, CYW),
> +	BRCMF_PCIE_DEVICE(BRCM_PCIE_43752_DEVICE_ID, WCC),
>   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4377_DEVICE_ID, WCC),
>   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4378_DEVICE_ID, WCC),
>   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4387_DEVICE_ID, WCC),
> 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 44684bf1b9acc..c1e22c589d85e 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
> @@ -52,6 +52,7 @@
>   #define BRCM_CC_43664_CHIP_ID		43664
>   #define BRCM_CC_43666_CHIP_ID		43666
>   #define BRCM_CC_4371_CHIP_ID		0x4371
> +#define BRCM_CC_43752_CHIP_ID		43752
I think this should be converted to hexadecimal to be consistent.
>   #define BRCM_CC_4377_CHIP_ID		0x4377
>   #define BRCM_CC_4378_CHIP_ID		0x4378
>   #define BRCM_CC_4387_CHIP_ID		0x4387
> @@ -94,6 +95,7 @@
>   #define BRCM_PCIE_4366_5G_DEVICE_ID	0x43c5
>   #define BRCM_PCIE_4371_DEVICE_ID	0x440d
>   #define BRCM_PCIE_43596_DEVICE_ID	0x4415
> +#define BRCM_PCIE_43752_DEVICE_ID	0x449d
>   #define BRCM_PCIE_4377_DEVICE_ID	0x4488
>   #define BRCM_PCIE_4378_DEVICE_ID	0x4425
>   #define BRCM_PCIE_4387_DEVICE_ID	0x4433
Ondřej Jirman June 30, 2024, 8:36 a.m. UTC | #2 
On Sun, Jun 30, 2024 at 10:34:39AM GMT, Stefan Wahren wrote:
> Am 30.06.24 um 09:36 schrieb Jacobe Zang:
> > This module features BCM43752A2 chipset. The firmware requires
> > randomness seeding, so enabled it.
> > 
> > Co-developed-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Ondrej Jirman <megi@xff.cz>
> > Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
> > Link: https://megous.com/git/linux/commit/?h=ap6275p-6.10&id=1a99573bc8ed412e60e1969c0b29d53a0e5782e0
> > ---
> >   drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c      | 5 ++++-
> >   .../net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h    | 2 ++
> >   2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> > index e84f562fc91b8..f427d664cf3a5 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> > @@ -71,6 +71,7 @@ BRCMF_FW_CLM_DEF(4377B3, "brcmfmac4377b3-pcie");
> >   BRCMF_FW_CLM_DEF(4378B1, "brcmfmac4378b1-pcie");
> >   BRCMF_FW_CLM_DEF(4378B3, "brcmfmac4378b3-pcie");
> >   BRCMF_FW_CLM_DEF(4387C2, "brcmfmac4387c2-pcie");
> > +BRCMF_FW_CLM_DEF(43752, "brcmfmac43752-pcie");
> > 
> >   /* firmware config files */
> >   MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.txt");
> > @@ -105,6 +106,7 @@ static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
> >   	BRCMF_FW_ENTRY(BRCM_CC_43664_CHIP_ID, 0xFFFFFFF0, 4366C),
> >   	BRCMF_FW_ENTRY(BRCM_CC_43666_CHIP_ID, 0xFFFFFFF0, 4366C),
> >   	BRCMF_FW_ENTRY(BRCM_CC_4371_CHIP_ID, 0xFFFFFFFF, 4371),
> > +	BRCMF_FW_ENTRY(BRCM_CC_43752_CHIP_ID, 0xFFFFFFFF, 43752),
> >   	BRCMF_FW_ENTRY(BRCM_CC_4377_CHIP_ID, 0xFFFFFFFF, 4377B3), /* revision ID 4 */
> >   	BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0x0000000F, 4378B1), /* revision ID 3 */
> >   	BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0xFFFFFFE0, 4378B3), /* revision ID 5 */
> > @@ -1721,7 +1723,7 @@ static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
> >   		memcpy_toio(devinfo->tcm + address, nvram, nvram_len);
> >   		brcmf_fw_nvram_free(nvram);
> > 
> > -		if (devinfo->otp.valid) {
> > +		if (devinfo->otp.valid || devinfo->ci->chip == BRCM_CC_43752_CHIP_ID) {
> As a reviewer, i would expect an explanation in the commit message or in
> the code for this quirk.

That's the "The firmware requires randomness seeding, so enabled it." part of
the commit message.

> >   			size_t rand_len = BRCMF_RANDOM_SEED_LENGTH;
> >   			struct brcmf_random_seed_footer footer = {
> >   				.length = cpu_to_le32(rand_len),
> > @@ -2710,6 +2712,7 @@ static const struct pci_device_id brcmf_pcie_devid_table[] = {
> >   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4366_5G_DEVICE_ID, BCA),
> >   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4371_DEVICE_ID, WCC),
> >   	BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_DEVICE_ID, CYW),
> > +	BRCMF_PCIE_DEVICE(BRCM_PCIE_43752_DEVICE_ID, WCC),
> >   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4377_DEVICE_ID, WCC),
> >   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4378_DEVICE_ID, WCC),
> >   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4387_DEVICE_ID, WCC),
> > 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 44684bf1b9acc..c1e22c589d85e 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
> > +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
> > @@ -52,6 +52,7 @@
> >   #define BRCM_CC_43664_CHIP_ID		43664
> >   #define BRCM_CC_43666_CHIP_ID		43666
> >   #define BRCM_CC_4371_CHIP_ID		0x4371
> > +#define BRCM_CC_43752_CHIP_ID		43752
> I think this should be converted to hexadecimal to be consistent.
> >   #define BRCM_CC_4377_CHIP_ID		0x4377
> >   #define BRCM_CC_4378_CHIP_ID		0x4378
> >   #define BRCM_CC_4387_CHIP_ID		0x4387
> > @@ -94,6 +95,7 @@
> >   #define BRCM_PCIE_4366_5G_DEVICE_ID	0x43c5
> >   #define BRCM_PCIE_4371_DEVICE_ID	0x440d
> >   #define BRCM_PCIE_43596_DEVICE_ID	0x4415
> > +#define BRCM_PCIE_43752_DEVICE_ID	0x449d
> >   #define BRCM_PCIE_4377_DEVICE_ID	0x4488
> >   #define BRCM_PCIE_4378_DEVICE_ID	0x4425
> >   #define BRCM_PCIE_4387_DEVICE_ID	0x4433
>
Stefan Wahren June 30, 2024, 8:37 a.m. UTC | #3 
Am 30.06.24 um 10:34 schrieb Stefan Wahren:
> Am 30.06.24 um 09:36 schrieb Jacobe Zang:
>> 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 44684bf1b9acc..c1e22c589d85e 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
>> +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
>> @@ -52,6 +52,7 @@
>>   #define BRCM_CC_43664_CHIP_ID        43664
>>   #define BRCM_CC_43666_CHIP_ID        43666
>>   #define BRCM_CC_4371_CHIP_ID        0x4371
>> +#define BRCM_CC_43752_CHIP_ID        43752
> I think this should be converted to hexadecimal to be consistent.
Sorry, please ignore this comment
>>   #define BRCM_CC_4377_CHIP_ID 0x4377
>>   #define BRCM_CC_4378_CHIP_ID        0x4378
>>   #define BRCM_CC_4387_CHIP_ID        0x4387
>> @@ -94,6 +95,7 @@
>>   #define BRCM_PCIE_4366_5G_DEVICE_ID    0x43c5
>>   #define BRCM_PCIE_4371_DEVICE_ID    0x440d
>>   #define BRCM_PCIE_43596_DEVICE_ID    0x4415
>> +#define BRCM_PCIE_43752_DEVICE_ID    0x449d
>>   #define BRCM_PCIE_4377_DEVICE_ID    0x4488
>>   #define BRCM_PCIE_4378_DEVICE_ID    0x4425
>>   #define BRCM_PCIE_4387_DEVICE_ID    0x4433
>
Arend Van Spriel June 30, 2024, 11:17 a.m. UTC | #4 
On June 30, 2024 9:36:40 AM Jacobe Zang <jacobe.zang@wesion.com> wrote:

> This module features BCM43752A2 chipset. The firmware requires
> randomness seeding, so enabled it.
>
> Co-developed-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
> Link: 
> https://megous.com/git/linux/commit/?h=ap6275p-6.10&id=1a99573bc8ed412e60e1969c0b29d53a0e5782e0
> ---
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c      | 5 ++++-
> .../net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h    | 2 ++
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c 
> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index e84f562fc91b8..f427d664cf3a5 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c

[...]

> @@ -1721,7 +1723,7 @@ static int brcmf_pcie_download_fw_nvram(struct 
> brcmf_pciedev_info *devinfo,
>  memcpy_toio(devinfo->tcm + address, nvram, nvram_len);
>  brcmf_fw_nvram_free(nvram);
>
> - if (devinfo->otp.valid) {
> + if (devinfo->otp.valid || devinfo->ci->chip

So the problem here is that otp validity is not unambiguous. The seed 
requirement is mandated by firmware and was introduced for new WCC chips 
moving forward.

I would suggest to have a need_seed flag in the driver_data of the device 
table (below). It currently is used only for vendor identification.

Regards,
Arend

> == BRCM_CC_43752_CHIP_ID) {
>  size_t rand_len = BRCMF_RANDOM_SEED_LENGTH;
>  struct brcmf_random_seed_footer footer = {
>  .length = cpu_to_le32(rand_len),
> @@ -2710,6 +2712,7 @@ static const struct pci_device_id 
> brcmf_pcie_devid_table[] = {
>  BRCMF_PCIE_DEVICE(BRCM_PCIE_4366_5G_DEVICE_ID, BCA),
>  BRCMF_PCIE_DEVICE(BRCM_PCIE_4371_DEVICE_ID, WCC),
>  BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_DEVICE_ID, CYW),
> + BRCMF_PCIE_DEVICE(BRCM_PCIE_43752_DEVICE_ID, WCC),
>  BRCMF_PCIE_DEVICE(BRCM_PCIE_4377_DEVICE_ID, WCC),
>  BRCMF_PCIE_DEVICE(BRCM_PCIE_4378_DEVICE_ID, WCC),
>  BRCMF_PCIE_DEVICE(BRCM_PCIE_4387_DEVICE_ID, WCC),
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index e84f562fc91b8..f427d664cf3a5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -71,6 +71,7 @@  BRCMF_FW_CLM_DEF(4377B3, "brcmfmac4377b3-pcie");
 BRCMF_FW_CLM_DEF(4378B1, "brcmfmac4378b1-pcie");
 BRCMF_FW_CLM_DEF(4378B3, "brcmfmac4378b3-pcie");
 BRCMF_FW_CLM_DEF(4387C2, "brcmfmac4387c2-pcie");
+BRCMF_FW_CLM_DEF(43752, "brcmfmac43752-pcie");
 
 /* firmware config files */
 MODULE_FIRMWARE(BRCMF_FW_DEFAULT_PATH "brcmfmac*-pcie.txt");
@@ -105,6 +106,7 @@  static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
 	BRCMF_FW_ENTRY(BRCM_CC_43664_CHIP_ID, 0xFFFFFFF0, 4366C),
 	BRCMF_FW_ENTRY(BRCM_CC_43666_CHIP_ID, 0xFFFFFFF0, 4366C),
 	BRCMF_FW_ENTRY(BRCM_CC_4371_CHIP_ID, 0xFFFFFFFF, 4371),
+	BRCMF_FW_ENTRY(BRCM_CC_43752_CHIP_ID, 0xFFFFFFFF, 43752),
 	BRCMF_FW_ENTRY(BRCM_CC_4377_CHIP_ID, 0xFFFFFFFF, 4377B3), /* revision ID 4 */
 	BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0x0000000F, 4378B1), /* revision ID 3 */
 	BRCMF_FW_ENTRY(BRCM_CC_4378_CHIP_ID, 0xFFFFFFE0, 4378B3), /* revision ID 5 */
@@ -1721,7 +1723,7 @@  static int brcmf_pcie_download_fw_nvram(struct brcmf_pciedev_info *devinfo,
 		memcpy_toio(devinfo->tcm + address, nvram, nvram_len);
 		brcmf_fw_nvram_free(nvram);
 
-		if (devinfo->otp.valid) {
+		if (devinfo->otp.valid || devinfo->ci->chip == BRCM_CC_43752_CHIP_ID) {
 			size_t rand_len = BRCMF_RANDOM_SEED_LENGTH;
 			struct brcmf_random_seed_footer footer = {
 				.length = cpu_to_le32(rand_len),
@@ -2710,6 +2712,7 @@  static const struct pci_device_id brcmf_pcie_devid_table[] = {
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4366_5G_DEVICE_ID, BCA),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4371_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_43596_DEVICE_ID, CYW),
+	BRCMF_PCIE_DEVICE(BRCM_PCIE_43752_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4377_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4378_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4387_DEVICE_ID, WCC),
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 44684bf1b9acc..c1e22c589d85e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
@@ -52,6 +52,7 @@ 
 #define BRCM_CC_43664_CHIP_ID		43664
 #define BRCM_CC_43666_CHIP_ID		43666
 #define BRCM_CC_4371_CHIP_ID		0x4371
+#define BRCM_CC_43752_CHIP_ID		43752
 #define BRCM_CC_4377_CHIP_ID		0x4377
 #define BRCM_CC_4378_CHIP_ID		0x4378
 #define BRCM_CC_4387_CHIP_ID		0x4387
@@ -94,6 +95,7 @@ 
 #define BRCM_PCIE_4366_5G_DEVICE_ID	0x43c5
 #define BRCM_PCIE_4371_DEVICE_ID	0x440d
 #define BRCM_PCIE_43596_DEVICE_ID	0x4415
+#define BRCM_PCIE_43752_DEVICE_ID	0x449d
 #define BRCM_PCIE_4377_DEVICE_ID	0x4488
 #define BRCM_PCIE_4378_DEVICE_ID	0x4425
 #define BRCM_PCIE_4387_DEVICE_ID	0x4433