[PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Justin Chen]

When the revised suspend/resume sequence was introduced this led to an edge
case where the TX is left disabled in the following sequence.

1. phy link is down, so UMAC is held in reset and then network interface
   is WoL enabled
2. Enter suspend, bcmgenet_wol_power_down_cfg() enables UMAC_RX since MAC
   is in SW_RESET
4. Enter resume, UMAC_RX is left enabled. Since we only enable UMAC_TX
   and UMAC_RX in SW_RESET. The UMAC_TX is never enabled again on link up.

Fixes: 254f3239dd07 ("net: bcmgenet: revise suspend/resume")
Signed-off-by: Justin Chen <justin.chen@broadcom.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
---
v2
	- Dropped fixes tag that did not introduce the bug

 drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
index 96d5d4f7f51f..984432952963 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet_wol.c
@@ -253,6 +253,17 @@ int bcmgenet_wol_power_up_cfg(struct bcmgenet_priv *priv,
 	reg = bcmgenet_umac_readl(priv, UMAC_CMD);
 	reg &= ~CMD_CRC_FWD;
 	bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+
+	/*
+	 * Mirror wol_power_down_cfg(). If only UMAC_RX
+	 * is enabled, then we must place the UMAC back
+	 * into SW_RESET.
+	 */
+	reg = bcmgenet_umac_readl(priv, UMAC_CMD);
+	if ((reg & CMD_RX_EN) && !(reg & CMD_TX_EN)) {
+		reg |= CMD_SW_RESET;
+		bcmgenet_umac_writel(priv, reg, UMAC_CMD);
+	}
 	spin_unlock_bh(&priv->reg_lock);
 
 	/* Resume link status tracking */
-- 
2.34.1

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Jakub Kicinski]

On Wed, 30 Sep 2026 23:08:43 +0000 netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
> 
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
> 
>  - What hardware the change was tested on. For driver fixes please
>    mention the device (and if relevant firmware version) used for
>    testing, or say that the change was not tested on real hardware.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

please respond to this

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Justin Chen]

On 10/1/2026 6:41 PM, Jakub Kicinski wrote:
> On Wed, 30 Sep 2026 23:08:43 +0000 netdev-bot+sinfo@kernel.org wrote:
>> Hi!
>>
>> This is an automated message. This series looks like a fix, but its
>> commit messages seem to be missing some information:
>>
>>   - How the issue was discovered, e.g. hit in production, hit during
>>     development, syzbot report, manual code inspection, LLM or static
>>     analysis tool scan.
>>
>>   - Whether the issue was actually triggered, or is only theoretical
>>     (e.g. found by code inspection). If it was triggered please include
>>     the symptoms, like the stack trace or error messages.
>>
>>   - What hardware the change was tested on. For driver fixes please
>>     mention the device (and if relevant firmware version) used for
>>     testing, or say that the change was not tested on real hardware.
>>
>> Please do not repost the series just to address the above. Instead,
>> reply to this email with the missing information, so that reviewers
>> can take it into account. If the series needs another revision for
>> other reasons, please include the information in the commit messages
>> then.
>>
>> The evaluation is done by an LLM so it may be wrong, if you think
>> that is the case please reply and explain.
> 
> please respond to this

The issue was hit in production and I verified the sequence keeps the TX 
disabled in my standalone test. I tested on an internal development 
board with bcm77122, but should be reproducible on any genet HW that 
supports power management.

Thanks,
Justin

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Jakub Kicinski]

On Fri, 2 Oct 2026 09:16:15 -0700 Justin Chen wrote:
> On 10/1/2026 6:41 PM, Jakub Kicinski wrote:
> > On Wed, 30 Sep 2026 23:08:43 +0000 netdev-bot+sinfo@kernel.org wrote:  
> >> Hi!
> >>
> >> This is an automated message. This series looks like a fix, but its
> >> commit messages seem to be missing some information:
> >>
> >>   - How the issue was discovered, e.g. hit in production, hit during
> >>     development, syzbot report, manual code inspection, LLM or static
> >>     analysis tool scan.
> >>
> >>   - Whether the issue was actually triggered, or is only theoretical
> >>     (e.g. found by code inspection). If it was triggered please include
> >>     the symptoms, like the stack trace or error messages.
> >>
> >>   - What hardware the change was tested on. For driver fixes please
> >>     mention the device (and if relevant firmware version) used for
> >>     testing, or say that the change was not tested on real hardware.
> >>
> >> Please do not repost the series just to address the above. Instead,
> >> reply to this email with the missing information, so that reviewers
> >> can take it into account. If the series needs another revision for
> >> other reasons, please include the information in the commit messages
> >> then.
> >>
> >> The evaluation is done by an LLM so it may be wrong, if you think
> >> that is the case please reply and explain.  
> > 
> > please respond to this  
> 
> The issue was hit in production and I verified the sequence keeps the TX 
> disabled in my standalone test. I tested on an internal development 
> board with bcm77122, but should be reproducible on any genet HW that 
> supports power management.

I obviously meant that you should respond to all the fix info requests.
Not just this one in particular.

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Justin Chen]

On 10/5/26 6:27 PM, Jakub Kicinski wrote:
> On Fri, 2 Oct 2026 09:16:15 -0700 Justin Chen wrote:
>> On 10/1/2026 6:41 PM, Jakub Kicinski wrote:
>>> On Wed, 30 Sep 2026 23:08:43 +0000 netdev-bot+sinfo@kernel.org wrote:
>>>> Hi!
>>>>
>>>> This is an automated message. This series looks like a fix, but its
>>>> commit messages seem to be missing some information:
>>>>
>>>>    - How the issue was discovered, e.g. hit in production, hit during
>>>>      development, syzbot report, manual code inspection, LLM or static
>>>>      analysis tool scan.
>>>>
>>>>    - Whether the issue was actually triggered, or is only theoretical
>>>>      (e.g. found by code inspection). If it was triggered please include
>>>>      the symptoms, like the stack trace or error messages.
>>>>
>>>>    - What hardware the change was tested on. For driver fixes please
>>>>      mention the device (and if relevant firmware version) used for
>>>>      testing, or say that the change was not tested on real hardware.
>>>>
>>>> Please do not repost the series just to address the above. Instead,
>>>> reply to this email with the missing information, so that reviewers
>>>> can take it into account. If the series needs another revision for
>>>> other reasons, please include the information in the commit messages
>>>> then.
>>>>
>>>> The evaluation is done by an LLM so it may be wrong, if you think
>>>> that is the case please reply and explain.
>>>
>>> please respond to this
>>
>> The issue was hit in production and I verified the sequence keeps the TX
>> disabled in my standalone test. I tested on an internal development
>> board with bcm77122, but should be reproducible on any genet HW that
>> supports power management.
> 
> I obviously meant that you should respond to all the fix info requests.
> Not just this one in particular.

Sorry Jakub, I am not understanding what you are asking for. I believe I 
answered the questions above? Or am I missing something else?

Thanks,
Justin

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Jakub Kicinski]

On Tue, 6 Oct 2026 15:29:01 -0700 Justin Chen wrote:
> On 10/5/26 6:27 PM, Jakub Kicinski wrote:
> > On Fri, 2 Oct 2026 09:16:15 -0700 Justin Chen wrote:  
> >> The issue was hit in production and I verified the sequence keeps the TX
> >> disabled in my standalone test. I tested on an internal development
> >> board with bcm77122, but should be reproducible on any genet HW that
> >> supports power management.  
> > 
> > I obviously meant that you should respond to all the fix info requests.
> > Not just this one in particular.  
> 
> Sorry Jakub, I am not understanding what you are asking for. I believe I 
> answered the questions above? Or am I missing something else?

I mean other threads, immediate example:

https://lore.kernel.org/all/20260929203400.1588868-1-justin.chen@broadcom.com/

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Justin Chen]

On 10/6/26 3:44 PM, Jakub Kicinski wrote:
> On Tue, 6 Oct 2026 15:29:01 -0700 Justin Chen wrote:
>> On 10/5/26 6:27 PM, Jakub Kicinski wrote:
>>> On Fri, 2 Oct 2026 09:16:15 -0700 Justin Chen wrote:
>>>> The issue was hit in production and I verified the sequence keeps the TX
>>>> disabled in my standalone test. I tested on an internal development
>>>> board with bcm77122, but should be reproducible on any genet HW that
>>>> supports power management.
>>>
>>> I obviously meant that you should respond to all the fix info requests.
>>> Not just this one in particular.
>>
>> Sorry Jakub, I am not understanding what you are asking for. I believe I
>> answered the questions above? Or am I missing something else?
> 
> I mean other threads, immediate example:
> 
> https://lore.kernel.org/all/20260929203400.1588868-1-justin.chen@broadcom.com/

Hmmmm. Looks like those missing info emails didn't hit my inbox. They 
were sent as I can see them on kernel lore... Let me see what happened 
there. Florian submitted more patches, but these should be the only two 
inflight from me. But yes, I will answer on the bcmasp patch as well.

Thanks,
Justin

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[Jakub Kicinski]

On Tue, 6 Oct 2026 16:05:48 -0700 Justin Chen wrote:
> >> Sorry Jakub, I am not understanding what you are asking for. I believe I
> >> answered the questions above? Or am I missing something else?  
> > 
> > I mean other threads, immediate example:
> > 
> > https://lore.kernel.org/all/20260929203400.1588868-1-justin.chen@broadcom.com/  
> 
> Hmmmm. Looks like those missing info emails didn't hit my inbox. They 
> were sent as I can see them on kernel lore... Let me see what happened 
> there. Florian submitted more patches, but these should be the only two 
> inflight from me. But yes, I will answer on the bcmasp patch as well.

Thanks. I wonder if it's something about Broadcom email filters or we're
running afoul of gmail(?) with the rate of these automatic requests :(

Re: [PATCH net v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET

[patchwork-bot+netdevbpf]

Hello:

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

On Wed, 30 Sep 2026 16:06:34 -0700 you wrote:
> When the revised suspend/resume sequence was introduced this led to an edge
> case where the TX is left disabled in the following sequence.
> 
> 1. phy link is down, so UMAC is held in reset and then network interface
>    is WoL enabled
> 2. Enter suspend, bcmgenet_wol_power_down_cfg() enables UMAC_RX since MAC
>    is in SW_RESET
> 4. Enter resume, UMAC_RX is left enabled. Since we only enable UMAC_TX
>    and UMAC_RX in SW_RESET. The UMAC_TX is never enabled again on link up.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: bcmgenet: if UMAC was suspended in SW_RESET, restore it to SW_RESET
    https://git.kernel.org/netdev/net/c/5857e5a196b0

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