linux-kernel - Re: [PATCH v1 1/4] wifi: brcmfmac: Rename Cypress 89459 to BCM4355

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6517b791-1700-970d-ac0a-847f60a6fa6f@broadcom.com>
Date:   Wed, 4 Jan 2023 14:29:44 +0100
From:   Arend van Spriel <arend.vanspriel@...adcom.com>
To:     Hector Martin <marcan@...can.st>,
        Arend van Spriel <aspriel@...il.com>,
        Franky Lin <franky.lin@...adcom.com>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        Kalle Valo <kvalo@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
Cc:     Alexander Prutskov <alep@...ress.com>,
        Chi-Hsien Lin <chi-hsien.lin@...ress.com>,
        Wright Feng <wright.feng@...ress.com>,
        Ian Lin <ian.lin@...ineon.com>,
        Soontak Lee <soontak.lee@...ress.com>,
        Joseph chuang <jiac@...ress.com>,
        Sven Peter <sven@...npeter.dev>,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        asahi@...ts.linux.dev, linux-wireless@...r.kernel.org,
        brcm80211-dev-list.pdl@...adcom.com,
        SHA-cyfmac-dev-list@...ineon.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/4] wifi: brcmfmac: Rename Cypress 89459 to BCM4355

On 1/4/2023 11:01 AM, 'Hector Martin' via BRCM80211-DEV-LIST,PDL wrote:
> The commit that introduced support for this chip incorrectly claimed it
> is a Cypress-specific part, while in actuality it is just a variant of
> BCM4355 silicon (as evidenced by the chip ID).
> 
> The relationship between Cypress products and Broadcom products isn't
> entirely clear, but given what little information is available and prior
> art in the driver, it seems the convention should be that originally
> Broadcom parts should retain the Broadcom name.
> 
> Thus, rename the relevant constants and firmware file. Also rename the
> specific 89459 PCIe ID to BCM43596, which seems to be the original
> subvariant name for this PCI ID (as defined in the out-of-tree bcmdhd
> driver). Also declare the firmware as CLM-capable, since it is.
> 
> Fixes: dce45ded7619 ("brcmfmac: Support 89459 pcie")
> Signed-off-by: Hector Martin <marcan@...can.st>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c   | 5 ++---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c   | 8 ++++----
>   .../net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h | 6 +++---
>   3 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> index 121893bbaa1d..3e42c2bd0d9a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/chip.c

[...]

> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index ae57a9a3ab05..3264be485e20 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c

[...]

> @@ -2590,6 +2590,7 @@ static const struct pci_device_id brcmf_pcie_devid_table[] = {
>   	BRCMF_PCIE_DEVICE(BRCM_PCIE_4350_DEVICE_ID, WCC),
>   	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),

A bit of a problem here. If Cypress want to support this device, 
regardless how they branded it, they will provide its firmware. Given 
that they initially added it (as 89459) I suppose we should mark it with 
CYW and not WCC. Actually, see my comment below on RAW dev ids.

>   	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 f4939cf62767..cacc43db86eb 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/include/brcm_hw_ids.h

[...]

> @@ -72,6 +72,7 @@
>   #define BRCM_PCIE_4350_DEVICE_ID	0x43a3
>   #define BRCM_PCIE_4354_DEVICE_ID	0x43df
>   #define BRCM_PCIE_4354_RAW_DEVICE_ID	0x4354
> +#define BRCM_PCIE_4355_RAW_DEVICE_ID	0x4355

I would remove all RAW device ids. These should not be observed outside 
chip vendor walls.


Download attachment "smime.p7s" of type "application/pkcs7-signature" (4219 bytes)