netdev - RE: [PATCH net-next v3 5/6] net: ravb: Do not apply features to hardware if the interface is down

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <TYCPR01MB112698DE07AAA9C535776805D864F2@TYCPR01MB11269.jpnprd01.prod.outlook.com>
Date: Tue, 13 Feb 2024 10:13:16 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Claudiu.Beznea <claudiu.beznea@...on.dev>, "s.shtylyov@....ru"
	<s.shtylyov@....ru>, "davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Claudiu.Beznea
	<claudiu.beznea@...on.dev>, Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: RE: [PATCH net-next v3 5/6] net: ravb: Do not apply features to
 hardware if the interface is down


Hi Claudiu,

> -----Original Message-----
> From: Claudiu <claudiu.beznea@...on.dev>
> Sent: Tuesday, February 13, 2024 9:41 AM
> Subject: [PATCH net-next v3 5/6] net: ravb: Do not apply features to
> hardware if the interface is down
> 
> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> 
> Do not apply features to hardware if the interface is down. In case
> runtime PM is enabled, and while the interface is down, the IP will be in
> reset mode (as for some platforms disabling the clocks will switch the IP
> to reset mode, which will lead to losing register contents) and applying
> settings in reset mode is not an option. Instead, cache the features and
> apply them in ravb_open() through ravb_emac_init().
> 
> To avoid accessing the hardware while the interface is down
> pm_runtime_active() check was introduced. Along with it the device runtime
> PM usage counter has been incremented to avoid disabling the device clocks
> while the check is in progress (if any).
> 
> Commit prepares for the addition of runtime PM.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> ---
> 
> Changes in v3:
> - updated patch title and description
> - updated patch content due to patch 4/6
> 
> Changes in v2:
> - fixed typo in patch description
> - adjusted ravb_set_features_gbeth(); didn't collect the Sergey's Rb
>   tag due to this
> 
> Changes since [2]:
> - use pm_runtime_get_noresume() and pm_runtime_active() and updated the
>   commit message to describe that
> - fixed typos
> - s/CSUM/checksum in patch title and description
> 
> Changes in v3 of [2]:
> - this was patch 20/21 in v2
> - fixed typos in patch description
> - removed code from ravb_open()
> - use ndev->flags & IFF_UP checks instead of netif_running()
> 
> Changes in v2 of [2]:
> - none; this patch is new
> 
> 
>  drivers/net/ethernet/renesas/ravb_main.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> b/drivers/net/ethernet/renesas/ravb_main.c
> index b3b91783bb7a..4dd0520dea90 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -2566,15 +2566,23 @@ static int ravb_set_features(struct net_device
> *ndev,  {
>  	struct ravb_private *priv = netdev_priv(ndev);
>  	const struct ravb_hw_info *info = priv->info;
> -	int ret;
> +	struct device *dev = &priv->pdev->dev;
> +	int ret = 0;
> +
> +	pm_runtime_get_noresume(dev);
> +
> +	if (!pm_runtime_active(dev))
> +		goto out_set_features;

This can be simplified, which avoids 1 goto statement and
Unnecessary ret initialization. I am leaving to you and Sergey.

	if (!pm_runtime_active(dev))
		ret = 0;
	else
		ret = info->set_feature(ndev, features);

	pm_runtime_put_noidle(dev);
	if (ret)
		goto err;

	ndev->features = features;

err:
	return ret;

Cheers,
Biju

> 
>  	ret = info->set_feature(ndev, features);
>  	if (ret)
> -		return ret;
> +		goto out_rpm_put;
> 
> +out_set_features:
>  	ndev->features = features;
> -
> -	return 0;
> +out_rpm_put:
> +	pm_runtime_put_noidle(dev);
> +	return ret;
>  }
> 
>  static const struct net_device_ops ravb_netdev_ops = {
> --
> 2.39.2