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

[Justin Chen <justin.chen@broadcom.com>]

On 9/29/26 2:09 PM, Nicolai Buchwitz wrote:
> Hi Justin
> 
> On 29.9.2026 21:22, Justin Chen 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.
>>
>> Fixes: 254f3239dd07 ("net: bcmgenet: revise suspend/resume")
>> Fixes: 88f6c8bf1aae ("net: bcmgenet: keep MAC in reset until PHY is up")
> 
> AFAIU before 254f3239dd07 resume always went through init_umac(), so older
> kernels shouldn't be affected. If we drop the tag for, it would save some
> backports to the older LTS kernels.
> 

That's a good point. I will drop the 88f6c8bf1aae Fixes tag in v2.

Thanks,
Justin

>> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
>> ---
>>  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 */
> 
> Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
> 
> Thanks,
> Nicolai