diff mbox series

[v1,2/4] brcmfmac: pcie: Add IDs/properties for BCM4355

Message ID 20230104100116.729-3-marcan@marcan.st
State New
Headers
Series BCM4355/4364/4377 support & identification fixes |

Commit Message

Hector Martin Jan. 4, 2023, 10:01 a.m. UTC 
  This chip is present on at least these Apple T2 Macs:

* hawaii: MacBook Air 13" (Late 2018)
* hawaii: MacBook Air 13" (True Tone, 2019)

Users report seeing PCI revision ID 12 for this chip, which Arend
reports should be revision C2, but Apple has the firmware tagged as
revision C1. Assume the right cutoff point for firmware versions is
revision ID 11 then, and leave older revisions using the non-versioned
firmware filename (Apple only uses C1 firmware builds).

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c    | 10 +++++++++-
 .../wireless/broadcom/brcm80211/include/brcm_hw_ids.h  |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

Comments

Arend van Spriel Jan. 4, 2023, 1:35 p.m. UTC | #1 
On 1/4/2023 11:01 AM, 'Hector Martin' via BRCM80211-DEV-LIST,PDL wrote:
> This chip is present on at least these Apple T2 Macs:
> 
> * hawaii: MacBook Air 13" (Late 2018)
> * hawaii: MacBook Air 13" (True Tone, 2019)
> 
> Users report seeing PCI revision ID 12 for this chip, which Arend
> reports should be revision C2, but Apple has the firmware tagged as
> revision C1. Assume the right cutoff point for firmware versions is
> revision ID 11 then, and leave older revisions using the non-versioned
> firmware filename (Apple only uses C1 firmware builds).

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c    | 10 +++++++++-
>   .../wireless/broadcom/brcm80211/include/brcm_hw_ids.h  |  1 +
>   2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 3264be485e20..bb4faea0f0b6 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c

[...]

> @@ -1994,6 +1996,11 @@ static int brcmf_pcie_read_otp(struct brcmf_pciedev_info *devinfo)
>   	int ret;
>   
>   	switch (devinfo->ci->chip) {
> +	case BRCM_CC_4355_CHIP_ID:
> +		coreid = BCMA_CORE_CHIPCOMMON;
> +		base = 0x8c0;
> +		words = 0xb2;
> +		break;
>   	case BRCM_CC_4378_CHIP_ID:
>   		coreid = BCMA_CORE_GCI;
>   		base = 0x1120;

This bit is not described in the commit message. Can you remind me why 
the driver needs to read OTP?
Hector Martin Jan. 4, 2023, 4:27 p.m. UTC | #2 
On 04/01/2023 22.35, Arend van Spriel wrote:
> On 1/4/2023 11:01 AM, 'Hector Martin' via BRCM80211-DEV-LIST,PDL wrote:
>> This chip is present on at least these Apple T2 Macs:
>>
>> * hawaii: MacBook Air 13" (Late 2018)
>> * hawaii: MacBook Air 13" (True Tone, 2019)
>>
>> Users report seeing PCI revision ID 12 for this chip, which Arend
>> reports should be revision C2, but Apple has the firmware tagged as
>> revision C1. Assume the right cutoff point for firmware versions is
>> revision ID 11 then, and leave older revisions using the non-versioned
>> firmware filename (Apple only uses C1 firmware builds).
> 
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>> Signed-off-by: Hector Martin <marcan@marcan.st>
>> ---
>>   .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c    | 10 +++++++++-
>>   .../wireless/broadcom/brcm80211/include/brcm_hw_ids.h  |  1 +
>>   2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> index 3264be485e20..bb4faea0f0b6 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> 
> [...]
> 
>> @@ -1994,6 +1996,11 @@ static int brcmf_pcie_read_otp(struct brcmf_pciedev_info *devinfo)
>>   	int ret;
>>   
>>   	switch (devinfo->ci->chip) {
>> +	case BRCM_CC_4355_CHIP_ID:
>> +		coreid = BCMA_CORE_CHIPCOMMON;
>> +		base = 0x8c0;
>> +		words = 0xb2;
>> +		break;
>>   	case BRCM_CC_4378_CHIP_ID:
>>   		coreid = BCMA_CORE_GCI;
>>   		base = 0x1120;
> 
> This bit is not described in the commit message. Can you remind me why 
> the driver needs to read OTP?

Apple platforms use a vendor-specific OTP area to store identification
information used to select the right firmware/txcap/clm/nvram blobs. See
6bad3eeab6d3d (already upstream) and the immediately preceding commits
for how this all works.

In principle this should just return gracefully if that part of the OTP
is empty, though when I originally wrote this code we only knew of Apple
platforms using these chips anyway. If you think this might possibly
introduce issues to other platforms using the same chips (e.g. if
reading the OTP fails outright for some reason), we could gate it on the
presence of a valid devinfo->settings->antenna_sku, which would indicate
an Apple platform (since that property is specific to the Apple
firmware-selection process and only populated on those platforms).

- Hector
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 3264be485e20..bb4faea0f0b6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -52,6 +52,7 @@  BRCMF_FW_DEF(43602, "brcmfmac43602-pcie");
 BRCMF_FW_DEF(4350, "brcmfmac4350-pcie");
 BRCMF_FW_DEF(4350C, "brcmfmac4350c2-pcie");
 BRCMF_FW_CLM_DEF(4355, "brcmfmac4355-pcie");
+BRCMF_FW_CLM_DEF(4355C1, "brcmfmac4355c1-pcie");
 BRCMF_FW_CLM_DEF(4356, "brcmfmac4356-pcie");
 BRCMF_FW_CLM_DEF(43570, "brcmfmac43570-pcie");
 BRCMF_FW_DEF(4358, "brcmfmac4358-pcie");
@@ -78,7 +79,8 @@  static const struct brcmf_firmware_mapping brcmf_pcie_fwnames[] = {
 	BRCMF_FW_ENTRY(BRCM_CC_4350_CHIP_ID, 0x000000FF, 4350C),
 	BRCMF_FW_ENTRY(BRCM_CC_4350_CHIP_ID, 0xFFFFFF00, 4350),
 	BRCMF_FW_ENTRY(BRCM_CC_43525_CHIP_ID, 0xFFFFFFF0, 4365C),
-	BRCMF_FW_ENTRY(BRCM_CC_4355_CHIP_ID, 0xFFFFFFFF, 4355),
+	BRCMF_FW_ENTRY(BRCM_CC_4355_CHIP_ID, 0x000007FF, 4355),
+	BRCMF_FW_ENTRY(BRCM_CC_4355_CHIP_ID, 0xFFFFF800, 4355C1), /* rev ID 12/C2 seen */
 	BRCMF_FW_ENTRY(BRCM_CC_4356_CHIP_ID, 0xFFFFFFFF, 4356),
 	BRCMF_FW_ENTRY(BRCM_CC_43567_CHIP_ID, 0xFFFFFFFF, 43570),
 	BRCMF_FW_ENTRY(BRCM_CC_43569_CHIP_ID, 0xFFFFFFFF, 43570),
@@ -1994,6 +1996,11 @@  static int brcmf_pcie_read_otp(struct brcmf_pciedev_info *devinfo)
 	int ret;
 
 	switch (devinfo->ci->chip) {
+	case BRCM_CC_4355_CHIP_ID:
+		coreid = BCMA_CORE_CHIPCOMMON;
+		base = 0x8c0;
+		words = 0xb2;
+		break;
 	case BRCM_CC_4378_CHIP_ID:
 		coreid = BCMA_CORE_GCI;
 		base = 0x1120;
@@ -2591,6 +2598,7 @@  static const struct pci_device_id brcmf_pcie_devid_table[] = {
 	BRCMF_PCIE_DEVICE_SUB(0x4355, BRCM_PCIE_VENDOR_ID_BROADCOM, 0x4355, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4354_RAW_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4355_RAW_DEVICE_ID, WCC),
+	BRCMF_PCIE_DEVICE(BRCM_PCIE_4355_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_4356_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_43567_DEVICE_ID, WCC),
 	BRCMF_PCIE_DEVICE(BRCM_PCIE_43570_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 cacc43db86eb..a722c37d7399 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
@@ -73,6 +73,7 @@ 
 #define BRCM_PCIE_4354_DEVICE_ID	0x43df
 #define BRCM_PCIE_4354_RAW_DEVICE_ID	0x4354
 #define BRCM_PCIE_4355_RAW_DEVICE_ID	0x4355
+#define BRCM_PCIE_4355_DEVICE_ID	0x43dc
 #define BRCM_PCIE_4356_DEVICE_ID	0x43ec
 #define BRCM_PCIE_43567_DEVICE_ID	0x43d3
 #define BRCM_PCIE_43570_DEVICE_ID	0x43d9