Re: [PATCH wireless-next v2 7/8] firmware: bcm47xx_sprom: read boardflags3 and AvVmid_c0..2 from NVRAM

[vit9696]

I think my previous message regarding 6/8 also relates to the change here in 7/8.

You are right to observe that NVRAM overrides are generally used to set not just GPIO leds but also PHY properties. Some of these variables are actually present in the open-source driver[1].

Whether or not you need to support them generally depends on the hardware. I personally do not remember seeing AvVmid_cX being set on BCM4360, I think it is generally set only for BCM4354. The proprietary driver might observe these variables regardless of the general configuration presence perhaps for unification purposes, but I am not sure this is needed for any BCM4360 in the wild.

Do you have an NVRAM dump with AvVmid_cX set for any BCM4360 hardware? If not, the AvVmid part is probably a no-op and I would not rush adding that. The amount of overrides supported by Broadcom drivers in general is quite large, dozens of vars, and it is probably best to add them based on NVRAM dumps to reduce the amount of potential format parsing bugs.

In general defaultsromvars_4360 array[2] is a good reference of which variables are generally found in the NVRAM. You can see e.g. boardflags3 there.

Best,
Vitaly

[1] https://github.com/RMerl/asuswrt-merlin.ng/blob/ca72a61f8bc5ef920e29d63d53d9864262208f0c/release/src-rt-5.04behnd.4916/bcmdrivers/broadcom/net/wl/impl103/main/src/shared/bcmsrom.c#L2823
[2] https://github.com/RMerl/asuswrt-merlin.ng/blob/ca72a61f8bc5ef920e29d63d53d9864262208f0c/release/src-rt-5.02axhnd/bcmdrivers/broadcom/net/wl/impl51/main/src/shared/bcmsrom.c#L598


> On 27 Oct 2026, at 02:53, Alessio Ferri <alessio.ferri@xxxxxxxxxxx> wrote:
> 
> Read them along with ledbh4..ledbh15. AvVmid_cN holds the Av/Vmid pairs
> of chain N's power detector per band; avvmid_valid marks the chains
> whose list parsed. A boardflags3 in NVRAM overrides the SPROM word.
> 
> Signed-off-by: Alessio Ferri <alessio.ferri@xxxxxxxxxxx>
> ---
> drivers/firmware/broadcom/bcm47xx_sprom.c | 32 +++++++++++++++++++++++++++++++
> include/linux/ssb/ssb.h                   |  3 +++
> 2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/firmware/broadcom/bcm47xx_sprom.c b/drivers/firmware/broadcom/bcm47xx_sprom.c
> index 75604cd70..581dd867d 100644
> --- a/drivers/firmware/broadcom/bcm47xx_sprom.c
> +++ b/drivers/firmware/broadcom/bcm47xx_sprom.c
> @@ -173,6 +173,32 @@ static void nvram_read_alpha2(const char *prefix, const char *name,
> memcpy(val, buf, 2);
> }
> 
> +/*
> + * AvVmid_c<core>: Av,Vmid of the chain's power detector for 2g, 5gl, 5gml,
> + * 5gmu and 5gh. The chain is marked valid only if all ten values parse.
> + */
> +static void nvram_read_avvmid(const char *prefix, int core,
> +      struct ssb_sprom *sprom, bool fallback)
> +{
> + char name[sizeof("AvVmid_c0")];
> + char buf[64], *p = buf, *tok;
> + u8 v[10];
> + int n = 0;
> +
> + snprintf(name, sizeof(name), "AvVmid_c%d", core);
> + if (get_nvram_var(prefix, NULL, name, buf, sizeof(buf), fallback) < 0)
> + return;
> + while (n < ARRAY_SIZE(v) && (tok = strsep(&p, ",")) != NULL) {
> + if (kstrtou8(strim(tok), 0, &v[n]))
> + return;
> + n++;
> + }
> + if (n != ARRAY_SIZE(v))
> + return;
> + memcpy(sprom->avvmid[core], v, sizeof(v));
> + sprom->avvmid_valid |= BIT(core);
> +}
> +
> /* Variables read from NVRAM whether or not the SPROM itself comes from it. */
> static void bcm47xx_sprom_fill_nvram_only(struct ssb_sprom *sprom,
>  const char *prefix, bool fallback)
> @@ -185,6 +211,12 @@ static void bcm47xx_sprom_fill_nvram_only(struct ssb_sprom *sprom,
> nvram_read_u8(prefix, NULL, name, &sprom->gpio_ext[i], 0xff,
>      fallback);
> }
> +
> + nvram_read_u32(prefix, NULL, "boardflags3", &sprom->boardflags3, 0,
> +       fallback);
> + sprom->avvmid_valid = 0;
> + for (i = 0; i < ARRAY_SIZE(sprom->avvmid); i++)
> + nvram_read_avvmid(prefix, i, sprom, fallback);
> }
> 
> /* This is one-function-only macro, it uses local "sprom" variable! */
> diff --git a/include/linux/ssb/ssb.h b/include/linux/ssb/ssb.h
> index 02e6c9054..5a4daa76d 100644
> --- a/include/linux/ssb/ssb.h
> +++ b/include/linux/ssb/ssb.h
> @@ -77,6 +77,9 @@ struct ssb_sprom {
> u8 gpio2; /* GPIO pin 2 */
> u8 gpio3; /* GPIO pin 3 */
> u8 gpio_ext[12]; /* GPIO pins 4-15, NVRAM ledbh4-15 only */
> + /* NVRAM AvVmid_c0..2: {Av, Vmid} for 2g, 5gl, 5gml, 5gmu, 5gh */
> + u8 avvmid[3][5][2];
> + u8 avvmid_valid; /* bit per chain whose AvVmid_cN parsed */
> u8 maxpwr_bg; /* 2.4GHz Amplifier Max Power (in dBm Q5.2) */
> u8 maxpwr_al; /* 5.2GHz Amplifier Max Power (in dBm Q5.2) */
> u8 maxpwr_a; /* 5.3GHz Amplifier Max Power (in dBm Q5.2) */
> 
> --
> 2.34.1
>

Attachment: signature.asc
Description: OpenPGP digital signature