[PATCH net v2 0/5] net: bcmgenet: Collection of bug fixes

[PATCH net v2 0/5] net: bcmgenet: Collection of bug fixes

[Florian Fainelli]

This patch series contains a collection of bug fixes found during a
LLM-assisted coding session.

Changes in v2:

- corrected Fixes commit for "net: bcmgenet: do not skip WoL power up on
  GENET V1"
- dropped unnecessary patches

Florian Fainelli (5):
  net: bcmgenet: fix 64-bit RTNL stats reading in ethtool on 32-bit
    systems
  net: bcmgenet: initialize u64 stats seq counter for all queues
  net: bcmgenet: do not skip WoL power up on GENET V1
  net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr
  net: bcmgenet: mask DMA_TIMEOUT_MASK when reading DMA_RING0_TIMEOUT

 .../net/ethernet/broadcom/genet/bcmgenet.c    | 28 +++++++++++--------
 1 file changed, 17 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH net v2 1/5] net: bcmgenet: fix 64-bit RTNL stats reading in ethtool on 32-bit systems

[Florian Fainelli]

When bcmgenet was converted to 64-bit statistics, STAT_RTNL members were
switched to point into struct rtnl_link_stats64, whose fields are 64-bit
(__u64) regardless of architecture.

However, bcmgenet_get_ethtool_stats() retained a legacy check:
  if (sizeof(unsigned long) != sizeof(u32) &&
      s->stat_sizeof == sizeof(unsigned long))

On 32-bit systems, sizeof(unsigned long) == sizeof(u32), causing this
condition to evaluate to false. As a result, 64-bit RTNL stats fields were
read via *(u32 *)p. On 32-bit Big-Endian systems (such as MIPS BE), this
reads the high 32 bits and returns 0 until the counter exceeds 4GB; on
32-bit Little-Endian systems (such as 32-bit ARM), the value is truncated
to 32 bits.

Fix this by checking if s->stat_sizeof == sizeof(u64) so 64-bit fields are
always read as 64-bit values.

Fixes: 59aa6e3072aa ("net: bcmgenet: switch to use 64bit statistics")
Assisted-by: LLM
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index b916080f4ff1..7b089de9484e 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1346,9 +1346,8 @@ static void bcmgenet_get_ethtool_stats(struct net_device *dev,
 				p = (char *)&stats64;
 
 			p += s->stat_offset;
-			if (sizeof(unsigned long) != sizeof(u32) &&
-				s->stat_sizeof == sizeof(unsigned long))
-				data[i] = *(unsigned long *)p;
+			if (s->stat_sizeof == sizeof(u64))
+				data[i] = *(u64 *)p;
 			else
 				data[i] = *(u32 *)p;
 		}
-- 
2.34.1

[PATCH net v2 2/5] net: bcmgenet: initialize u64 stats seq counter for all queues

[Florian Fainelli]

bcmgenet_gstrings_stats statically defines ethtool statistics for queues
0 through GENET_MAX_MQ_CNT (4). However, bcmgenet_probe() only initialized
the u64_stats_sync seq counter up to priv->hw_params->rx_queues and
priv->hw_params->tx_queues.

Since priv->hw_params->rx_queues is 0 across all hardware versions (and
priv->hw_params->tx_queues is 0 on GENET V1), rings 1..4 have uninitialized
u64_stats_sync structures. When ethtool -S is run on 32-bit kernels,
bcmgenet_get_ethtool_stats() reads stats from rx_rings[1..4], causing
lockdep warnings due to the uninitialized sequence counters.

Initialize the sequence counters for all GENET_MAX_MQ_CNT + 1 queues.

Fixes: ffc2c8c4a714 ("net: bcmgenet: Initialize u64 stats seq counter")
Assisted-by: LLM
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 7b089de9484e..055e1362173b 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -4134,10 +4134,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
 		priv->rx_rings[i].rx_max_coalesced_frames = 1;
 
 	/* Initialize u64 stats seq counter for 32bit machines */
-	for (i = 0; i <= priv->hw_params->rx_queues; i++)
+	for (i = 0; i <= GENET_MAX_MQ_CNT; i++) {
 		u64_stats_init(&priv->rx_rings[i].stats64.syncp);
-	for (i = 0; i <= priv->hw_params->tx_queues; i++)
 		u64_stats_init(&priv->tx_rings[i].stats64.syncp);
+	}
 
 	/* libphy will determine the link state */
 	netif_carrier_off(dev);
-- 
2.34.1

[PATCH net v2 3/5] net: bcmgenet: do not skip WoL power up on GENET V1

[Florian Fainelli]

bcmgenet_power_up() had an early check for bcmgenet_has_ext(priv) before
dispatching by power mode. GENET V1 does not have the EXT block (unlike
GENET V2+), which causes bcmgenet_power_up() to immediately return 0.

As a consequence, when waking up from GENET_POWER_WOL_MAGIC on GENET V1,
bcmgenet_wol_power_up_cfg() is never invoked to disable the WoL clock,
clear wake event masks, and restore normal PHY and MAC operations.

Move the bcmgenet_has_ext() checks to the GENET_POWER_PASSIVE and
GENET_POWER_CABLE_SENSE cases where the EXT registers are actually
accessed, allowing GENET_POWER_WOL_MAGIC cleanup to execute on all
hardware versions.

Fixes: c3ae64ae0c08 ("net: bcmgenet: handle GENET_POWER_WOL_MAGIC")
Assisted-by: LLM
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Siged-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 055e1362173b..bfb74dca55d6 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1762,13 +1762,12 @@ static int bcmgenet_power_up(struct bcmgenet_priv *priv,
 	int ret = 0;
 	u32 reg;
 
-	if (!bcmgenet_has_ext(priv))
-		return ret;
-
-	reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
-
 	switch (mode) {
 	case GENET_POWER_PASSIVE:
+		if (!bcmgenet_has_ext(priv))
+			break;
+
+		reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 		reg &= ~(EXT_PWR_DOWN_DLL | EXT_PWR_DOWN_BIAS |
 			 EXT_ENERGY_DET_MASK);
 		if (GENET_IS_V5(priv) && !bcmgenet_has_ephy_16nm(priv)) {
@@ -1792,8 +1791,12 @@ static int bcmgenet_power_up(struct bcmgenet_priv *priv,
 		break;
 
 	case GENET_POWER_CABLE_SENSE:
+		if (!bcmgenet_has_ext(priv))
+			break;
+
 		/* enable APD */
 		if (!GENET_IS_V5(priv)) {
+			reg = bcmgenet_ext_readl(priv, EXT_EXT_PWR_MGMT);
 			reg |= EXT_PWR_DN_EN_LD;
 			bcmgenet_ext_writel(priv, reg, EXT_EXT_PWR_MGMT);
 		}
-- 
2.34.1

[PATCH net v2 4/5] net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr

[Florian Fainelli]

bcmgenet_set_mac_addr() did not check whether the provided MAC address is a
valid Ethernet address before applying it. Userspace could configure an
invalid address (such as all zeroes or a multicast address) while the
interface is down.

Add a call to is_valid_ether_addr() and return -EADDRNOTAVAIL if the MAC
address is not valid.

Fixes: 1c1008c793fa ("net: bcmgenet: add main driver file")
Assisted-by: LLM
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index bfb74dca55d6..e9d9d3d7b636 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3634,6 +3634,9 @@ static int bcmgenet_set_mac_addr(struct net_device *dev, void *p)
 	if (netif_running(dev))
 		return -EBUSY;
 
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
 	eth_hw_addr_set(dev, addr->sa_data);
 
 	return 0;
-- 
2.34.1

[PATCH net v2 5/5] net: bcmgenet: mask DMA_TIMEOUT_MASK when reading DMA_RING0_TIMEOUT

[Florian Fainelli]

bcmgenet_get_coalesce() reads DMA_RING0_TIMEOUT to calculate
rx_coalesce_usecs without masking out bits outside DMA_TIMEOUT_MASK
(16 bits). If upper bits are non-zero or contain status/flags, the
computed value of rx_coalesce_usecs returned to userspace via ethtool
becomes corrupted.

Mask the register read with DMA_TIMEOUT_MASK before computing the
timeout in microseconds.

Fixes: 4a29645bfe6c ("net: bcmgenet: Implement RX coalescing control knobs")
Assisted-by: LLM
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
---
 drivers/net/ethernet/broadcom/genet/bcmgenet.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index e9d9d3d7b636..ef3b750630d3 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -852,7 +852,8 @@ static int bcmgenet_get_coalesce(struct net_device *dev,
 	ec->rx_max_coalesced_frames =
 		bcmgenet_rdma_ring_readl(priv, 0, DMA_MBUF_DONE_THRESH);
 	ec->rx_coalesce_usecs =
-		bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT) * 8192 / 1000;
+		(bcmgenet_rdma_readl(priv, DMA_RING0_TIMEOUT) &
+		 DMA_TIMEOUT_MASK) * 8192 / 1000;
 
 	for (i = 0; i <= priv->hw_params->rx_queues; i++) {
 		ring = &priv->rx_rings[i];
-- 
2.34.1

Re: [PATCH net v2 0/5] net: bcmgenet: Collection of bug fixes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 21 Sep 2026 15:00:16 -0700 you wrote:
> This patch series contains a collection of bug fixes found during a
> LLM-assisted coding session.
> 
> Changes in v2:
> 
> - corrected Fixes commit for "net: bcmgenet: do not skip WoL power up on
>   GENET V1"
> - dropped unnecessary patches
> 
> [...]

Here is the summary with links:
  - [net,v2,1/5] net: bcmgenet: fix 64-bit RTNL stats reading in ethtool on 32-bit systems
    https://git.kernel.org/netdev/net/c/0e2bec77ea62
  - [net,v2,2/5] net: bcmgenet: initialize u64 stats seq counter for all queues
    https://git.kernel.org/netdev/net/c/3aeaa609fda1
  - [net,v2,3/5] net: bcmgenet: do not skip WoL power up on GENET V1
    https://git.kernel.org/netdev/net/c/cbbc1aee7776
  - [net,v2,4/5] net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr
    https://git.kernel.org/netdev/net/c/273941c85fc2
  - [net,v2,5/5] net: bcmgenet: mask DMA_TIMEOUT_MASK when reading DMA_RING0_TIMEOUT
    https://git.kernel.org/netdev/net/c/d64e277b955b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net v2 3/5] net: bcmgenet: do not skip WoL power up on GENET V1

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message of "net: bcmgenet: do not skip WoL power up
  on GENET V1" (387c3a954a3a) ends with the malformed trailer `Siged-…

--

net: bcmgenet: do not skip WoL power up on GENET V1

This moves the bcmgenet_has_ext(priv) test out of the top of
bcmgenet_power_up() and into the GENET_POWER_PASSIVE and
GENET_POWER_CABLE_SENSE cases, where the EXT registers are actually
touched. That lets the GENET_POWER_WOL_MAGIC case reach
bcmgenet_wol_power_up_cfg() on GENET V1, which has no EXT block.

The only comment below is about the commit message trailers, so the diff
hunks are omitted.

> Fixes: c3ae64ae0c08 ("net: bcmgenet: handle GENET_POWER_WOL_MAGIC")
> Assisted-by: LLM
> Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
> Siged-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
  ^^^^^^^^^^^^

[Severity: Medium]
This isn't a code bug, but should this read Signed-off-by: instead of
Siged-off-by:?

As spelled, the trailer is not matched by checkpatch.pl or the patchwork
netdev checks, so the patch appears to have no author sign-off.

The other four patches in the same series use the correct spelling, so
this looks like an isolated typo in this patch only. Since a trailer can
only be corrected by amending this commit, would a resend be needed here?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921220021.281418-1-florian.fainelli%40broadcom.com

Re: [PATCH net v2 4/5] net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] bcmgenet_set_mac_addr()
  (drivers/net/ethernet/broadcom/genet/bcmgenet.c:3628-3644) now hand-…

--

net: bcmgenet: validate Ethernet address in bcmgenet_set_mac_addr

This adds an is_valid_ether_addr() check to bcmgenet_set_mac_addr() so
that an invalid MAC address (all zeroes, or a multicast address) is
rejected with -EADDRNOTAVAIL instead of being programmed while the
interface is down.

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index bfb74dca55d6f..e9d9d3d7b6369 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -3634,6 +3634,9 @@ static int bcmgenet_set_mac_addr(struct net_device *dev, void *p)
>  	if (netif_running(dev))
>  		return -EBUSY;
>  
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
>  	eth_hw_addr_set(dev, addr->sa_data);
>  
>  	return 0;

[Severity: Low]
This isn't a bug, but with this hunk bcmgenet_set_mac_addr() now open-codes
exactly what the core helpers already do.

net/ethernet/eth.c:

	int eth_prepare_mac_addr_change(struct net_device *dev, void *p)
	{
		struct sockaddr *addr = p;

		if (!(dev->priv_flags & IFF_LIVE_ADDR_CHANGE) && netif_running(dev))
			return -EBUSY;
		if (!is_valid_ether_addr(addr->sa_data))
			return -EADDRNOTAVAIL;
		return 0;
	}

	void eth_commit_mac_addr_change(struct net_device *dev, void *p)
	{
		struct sockaddr *addr = p;

		eth_hw_addr_set(dev, addr->sa_data);
	}

The resulting body of bcmgenet_set_mac_addr() is then equivalent to
eth_mac_addr(). Would calling eth_prepare_mac_addr_change() followed by
eth_commit_mac_addr_change() (keeping the driver-specific comment about the
UniMAC RX/TX enable bits) be preferable to duplicating the validation here?

One small difference: the driver copy tests netif_running() unconditionally
and so does not honour the IFF_LIVE_ADDR_CHANGE exemption that the core
helper checks. bcmgenet does not set that private flag today, so there is no
behavioural change now, but the duplicated checks will not track future
changes to the core validation rules.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921220021.281418-1-florian.fainelli%40broadcom.com