Re: [PATCH] net: stmmac:fix system hang when setting up tag_8021q VLAN for DSA ports

[Jacob Keller]

On 4/13/2023 6:38 AM, Yan Wang wrote:
> The system hang because of dsa_tag_8021q_port_setup()->
> 				stmmac_vlan_rx_add_vid().
> 
> I found in stmmac_drv_probe() that cailing pm_runtime_put()
> disabled the clock.
> 
> First, when the kernel is compiled with CONFIG_PM=y,The stmmac's
> resume/suspend is active.
> 
> Secondly,stmmac as DSA master,the dsa_tag_8021q_port_setup() function
> will callback stmmac_vlan_rx_add_vid when DSA dirver starts. However,
> The system is hanged for the stmmac_vlan_rx_add_vid() accesses its
> registers after stmmac's clock is closed.
> 

And access to the registers basically just waits because its clock is
stopped?

> I would suggest adding the pm_runtime_resume_and_get() to the
> stmmac_vlan_rx_add_vid().This guarantees that resuming clock output
> while in use.
> 

It feels a bit odd to need this within the VLAN function, but these can
be entered at an odd time where its possible that the clock has been
closed... Ok.

> Signed-off-by: Yan Wang <rk.code@xxxxxxxxxxx>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>

>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index d7fcab057032..f9cd063f1fe3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6350,6 +6350,10 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
>  	bool is_double = false;
>  	int ret;
>  
> +	ret = pm_runtime_resume_and_get(priv->device);
> +	if (ret < 0)
> +		return ret;
> +
>  	if (be16_to_cpu(proto) == ETH_P_8021AD)
>  		is_double = true;
>  
> @@ -6357,16 +6361,18 @@ static int stmmac_vlan_rx_add_vid(struct net_device *ndev, __be16 proto, u16 vid
>  	ret = stmmac_vlan_update(priv, is_double);
>  	if (ret) {
>  		clear_bit(vid, priv->active_vlans);
> -		return ret;
> +		goto err_pm_put;
>  	}
>  
>  	if (priv->hw->num_vlan) {
>  		ret = stmmac_add_hw_vlan_rx_fltr(priv, ndev, priv->hw, proto, vid);
>  		if (ret)
> -			return ret;
> +			goto err_pm_put;
>  	}
> +err_pm_put:
> +	pm_runtime_put(priv->device);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int stmmac_vlan_rx_kill_vid(struct net_device *ndev, __be16 proto, u16 vid)