[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220915153438.zt7hs3yuates7na7@bang-olufsen.dk>
Date: Thu, 15 Sep 2022 15:34:40 +0000
From: Alvin Šipraga <ALSI@...g-olufsen.dk>
To: Russell King <rmk+kernel@...linux.org.uk>
CC: Arend van Spriel <aspriel@...il.com>,
Franky Lin <franky.lin@...adcom.com>,
Hante Meuleman <hante.meuleman@...adcom.com>,
Alyssa Rosenzweig <alyssa@...enzweig.io>,
"asahi@...ts.linux.dev" <asahi@...ts.linux.dev>,
"brcm80211-dev-list.pdl@...adcom.com"
<brcm80211-dev-list.pdl@...adcom.com>,
"David S. Miller" <davem@...emloft.net>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Hector Martin <marcan@...can.st>,
Jakub Kicinski <kuba@...nel.org>,
Kalle Valo <kvalo@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Paolo Abeni <pabeni@...hat.com>,
Rafa__ Mi__ecki <zajec5@...il.com>,
Rob Herring <robh+dt@...nel.org>,
"SHA-cyfmac-dev-list@...ineon.com" <SHA-cyfmac-dev-list@...ineon.com>,
Sven Peter <sven@...npeter.dev>
Subject: Re: [PATCH wireless-next v2 08/12] brcmfmac: firmware: Allow platform
to override macaddr
On Mon, Sep 12, 2022 at 10:53:17AM +0100, Russell King wrote:
> From: Hector Martin <marcan@...can.st>
>
> On Device Tree platforms, it is customary to be able to set the MAC
> address via the Device Tree, as it is often stored in system firmware.
> This is particularly relevant for Apple ARM64 platforms, where this
> information comes from system configuration and passed through by the
> bootloader into the DT.
>
> Implement support for this by fetching the platform MAC address and
> adding or replacing the macaddr= property in nvram. This becomes the
> dongle's default MAC address.
>
> On platforms with an SROM MAC address, this overrides it. On platforms
> without one, such as Apple ARM64 devices, this is required for the
> firmware to boot (it will fail if it does not have a valid MAC at all).
>
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> Signed-off-by: Hector Martin <marcan@...can.st>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> ---
> .../broadcom/brcm80211/brcmfmac/firmware.c | 31 +++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> index 371c086d1f48..c109e20fc5c6 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c
> @@ -21,6 +21,8 @@
> #define BRCMF_FW_NVRAM_DEVPATH_LEN 19 /* devpath0=pcie/1/4/ */
> #define BRCMF_FW_NVRAM_PCIEDEV_LEN 10 /* pcie/1/4/ + \0 */
> #define BRCMF_FW_DEFAULT_BOARDREV "boardrev=0xff"
> +#define BRCMF_FW_MACADDR_FMT "macaddr=%pM"
> +#define BRCMF_FW_MACADDR_LEN (7 + ETH_ALEN * 3)
>
> enum nvram_parser_state {
> IDLE,
> @@ -57,6 +59,7 @@ struct nvram_parser {
> bool multi_dev_v1;
> bool multi_dev_v2;
> bool boardrev_found;
> + bool strip_mac;
CC [M] drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.o
drivers/net/wireless/broadcom/brcm80211/brcmfmac/firmware.c:63: warning: Function parameter or member 'strip_mac' not described in 'nvram_parser'
Otherwise looks OK and you can add
Reviewed-by: Alvin Šipraga <alsi@...g-olufsen.dk>
> };
>
> /*
> @@ -121,6 +124,10 @@ static enum nvram_parser_state brcmf_nvram_handle_key(struct nvram_parser *nvp)
> nvp->multi_dev_v2 = true;
> if (strncmp(&nvp->data[nvp->entry], "boardrev", 8) == 0)
> nvp->boardrev_found = true;
> + /* strip macaddr if platform MAC overrides */
> + if (nvp->strip_mac &&
> + strncmp(&nvp->data[nvp->entry], "macaddr", 7) == 0)
> + st = COMMENT;
> } else if (!is_nvram_char(c) || c == ' ') {
> brcmf_dbg(INFO, "warning: ln=%d:col=%d: '=' expected, skip invalid key entry\n",
> nvp->line, nvp->column);
> @@ -209,6 +216,7 @@ static int brcmf_init_nvram_parser(struct nvram_parser *nvp,
> size = data_len;
> /* Add space for properties we may add */
> size += strlen(BRCMF_FW_DEFAULT_BOARDREV) + 1;
> + size += BRCMF_FW_MACADDR_LEN + 1;
> /* Alloc for extra 0 byte + roundup by 4 + length field */
> size += 1 + 3 + sizeof(u32);
> nvp->nvram = kzalloc(size, GFP_KERNEL);
> @@ -368,22 +376,37 @@ static void brcmf_fw_add_defaults(struct nvram_parser *nvp)
> nvp->nvram_len++;
> }
>
> +static void brcmf_fw_add_macaddr(struct nvram_parser *nvp, u8 *mac)
> +{
> + int len;
> +
> + len = scnprintf(&nvp->nvram[nvp->nvram_len], BRCMF_FW_MACADDR_LEN + 1,
> + BRCMF_FW_MACADDR_FMT, mac);
> + WARN_ON(len != BRCMF_FW_MACADDR_LEN);
> + nvp->nvram_len += len + 1;
> +}
> +
> /* brcmf_nvram_strip :Takes a buffer of "<var>=<value>\n" lines read from a fil
> * and ending in a NUL. Removes carriage returns, empty lines, comment lines,
> * and converts newlines to NULs. Shortens buffer as needed and pads with NULs.
> * End of buffer is completed with token identifying length of buffer.
> */
> static void *brcmf_fw_nvram_strip(const u8 *data, size_t data_len,
> - u32 *new_length, u16 domain_nr, u16 bus_nr)
> + u32 *new_length, u16 domain_nr, u16 bus_nr,
> + struct device *dev)
> {
> struct nvram_parser nvp;
> u32 pad;
> u32 token;
> __le32 token_le;
> + u8 mac[ETH_ALEN];
>
> if (brcmf_init_nvram_parser(&nvp, data, data_len) < 0)
> return NULL;
>
> + if (eth_platform_get_mac_address(dev, mac) == 0)
> + nvp.strip_mac = true;
> +
> while (nvp.pos < data_len) {
> nvp.state = nv_parser_states[nvp.state](&nvp);
> if (nvp.state == END)
> @@ -404,6 +427,9 @@ static void *brcmf_fw_nvram_strip(const u8 *data, size_t data_len,
>
> brcmf_fw_add_defaults(&nvp);
>
> + if (nvp.strip_mac)
> + brcmf_fw_add_macaddr(&nvp, mac);
> +
> pad = nvp.nvram_len;
> *new_length = roundup(nvp.nvram_len + 1, 4);
> while (pad != *new_length) {
> @@ -538,7 +564,8 @@ static int brcmf_fw_request_nvram_done(const struct firmware *fw, void *ctx)
> if (data)
> nvram = brcmf_fw_nvram_strip(data, data_len, &nvram_length,
> fwctx->req->domain_nr,
> - fwctx->req->bus_nr);
> + fwctx->req->bus_nr,
> + fwctx->dev);
>
> if (free_bcm47xx_nvram)
> bcm47xx_nvram_release_contents(data);
> --
> 2.30.2
>
Powered by blists - more mailing lists