Hi Claudiu,
> -----Original Message-----
> From: Claudiu <claudiu.beznea@xxxxxxxxx>
> 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@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> ---
>
> 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
