[v2] net: ravb: Add suspend to RAM and runtime PM support for RZ/G3S

[PATCH net-next v2 19/21] net: ravb: Do not set promiscuous mode if the interface is down

Posted by Claudiu 2 years ago

From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Do not allow setting promiscuous mode if the interface is down. In case
runtime PM is enabled, and while interface is down, the IP will be in reset
mode (as for some platforms disabling/enabling the clocks will switch the
IP to standby mode which will lead to losing registers' content).

Commit prepares for the addition of runtime PM.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes in v2:
- none; this patch is new

 drivers/net/ethernet/renesas/ravb_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 1995cf7ff084..633346b6cd7c 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2164,6 +2164,9 @@ static void ravb_set_rx_mode(struct net_device *ndev)
 	struct ravb_private *priv = netdev_priv(ndev);
 	unsigned long flags;
 
+	if (!netif_running(ndev))
+		return;
+
 	spin_lock_irqsave(&priv->lock, flags);
 	ravb_modify(ndev, ECMR, ECMR_PRM,
 		    ndev->flags & IFF_PROMISC ? ECMR_PRM : 0);
-- 
2.39.2

Re: [PATCH net-next v2 19/21] net: ravb: Do not set promiscuous mode if the interface is down

Posted by Sergey Shtylyov 2 years ago

On 12/14/23 2:45 PM, Claudiu wrote:

> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Do not allow setting promiscuous mode if the interface is down. In case
> runtime PM is enabled, and while interface is down, the IP will be in reset
> mode (as for some platforms disabling/enabling the clocks will switch the
> IP to standby mode which will lead to losing registers' content).

   Register.
   Have this issue actually occurred for you?

> Commit prepares for the addition of runtime PM.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
[...]

> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 1995cf7ff084..633346b6cd7c 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2164,6 +2164,9 @@ static void ravb_set_rx_mode(struct net_device *ndev)
>  	struct ravb_private *priv = netdev_priv(ndev);
>  	unsigned long flags;
>  
> +	if (!netif_running(ndev))

   Seems racy as well...

> +		return;

   Hm, sh_eth.c doesn't have such check -- perhaps should be fixed
as well...

[...]

MBR, Sergey

Re: [PATCH net-next v2 19/21] net: ravb: Do not set promiscuous mode if the interface is down

Posted by claudiu beznea 2 years ago


On 16.12.2023 22:16, Sergey Shtylyov wrote:
> On 12/14/23 2:45 PM, Claudiu wrote:
> 
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> Do not allow setting promiscuous mode if the interface is down. In case
>> runtime PM is enabled, and while interface is down, the IP will be in reset
>> mode (as for some platforms disabling/enabling the clocks will switch the
>> IP to standby mode which will lead to losing registers' content).
> 
>    Register.
>    Have this issue actually occurred for you?
> 
>> Commit prepares for the addition of runtime PM.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> [...]
> 
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index 1995cf7ff084..633346b6cd7c 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -2164,6 +2164,9 @@ static void ravb_set_rx_mode(struct net_device *ndev)
>>  	struct ravb_private *priv = netdev_priv(ndev);
>>  	unsigned long flags;
>>  
>> +	if (!netif_running(ndev))
> 
>    Seems racy as well...

It is also called with rtnl_mutex locked at least though __dev_change_flags().

> 
>> +		return;
> 
>    Hm, sh_eth.c doesn't have such check -- perhaps should be fixed
> as well...
> 
> [...]
> 
> MBR, Sergey

Re: [PATCH net-next v2 19/21] net: ravb: Do not set promiscuous mode if the interface is down

Posted by Sergey Shtylyov 2 years ago

On 12/17/23 5:02 PM, claudiu beznea wrote:

[...]
>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>
>>> Do not allow setting promiscuous mode if the interface is down. In case
>>> runtime PM is enabled, and while interface is down, the IP will be in reset
>>> mode (as for some platforms disabling/enabling the clocks will switch the
>>> IP to standby mode which will lead to losing registers' content).
>>
>>    Register.
>>    Have this issue actually occurred for you?
>>
>>> Commit prepares for the addition of runtime PM.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>> [...]
>>
>>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>>> index 1995cf7ff084..633346b6cd7c 100644
>>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>>> @@ -2164,6 +2164,9 @@ static void ravb_set_rx_mode(struct net_device *ndev)
>>>  	struct ravb_private *priv = netdev_priv(ndev);
>>>  	unsigned long flags;
>>>  
>>> +	if (!netif_running(ndev))
>>
>>    Seems racy as well...
> 
> It is also called with rtnl_mutex locked at least though __dev_change_flags().

   I'm tired of chasing the call tree for you. :-)
   Since I'm hoping we'll agree on the ndo_get_stats() case, it would
seem safer to use an is_opened flag here as well, like sh_eth.c does.

[...]

MBR, Sergey