| Message ID | 20240105082339.1468817-20-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | net: ravb: Add suspend to RAM and runtime PM support for RZ/G3S | expand |
On 1/5/24 11:23 AM, Claudiu wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > Add runtime PM support for the ravb driver. As the driver is used by > different IP variants, with different behaviors, to be able to have the > runtime PM support available for all devices, the preparatory commits > moved all the resources parsing and allocations in the driver's probe > function and kept the settings for ravb_open(). This is due to the fact > that on some IP variants-platforms tuples disabling/enabling the clocks > will switch the IP to the reset operation mode where registers' content is > lost and reconfiguration needs to be done. For this the rabv_open() > function enables the clocks, switches the IP to configuration mode, applies > all the registers settings and switches the IP to the operational mode. At > the end of ravb_open() IP is ready to send/receive data. > > In ravb_close() necessary reverts are done (compared with ravb_open()), the > IP is switched to reset mode and clocks are disabled. > > The ethtool APIs or IOCTLs that might execute while the interface is down > are either cached (and applied in ravb_open()) or rejected (as at that time > the IP is in reset mode). Keeping the IP in the reset mode also increases > the power saved (according to the hardware manual). > > Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> > 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 e909960fbc30..e99351fe8d7f 100644 > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c [...] > @@ -2233,7 +2243,14 @@ static int ravb_close(struct net_device *ndev) > ravb_get_stats(ndev); > > /* Set reset mode. */ > - return ravb_set_opmode(ndev, CCC_OPC_RESET); > + error = ravb_set_opmode(ndev, CCC_OPC_RESET); > + if (error) > + return error; > + > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); BTW, doesn't make sense to check/return the result? > + > + return 0; > } > > static int ravb_hwtstamp_get(struct net_device *ndev, struct ifreq *req) [...] MBR, Sergey
On 09.01.2024 21:53, Sergey Shtylyov wrote: > On 1/5/24 11:23 AM, Claudiu wrote: > >> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> >> >> Add runtime PM support for the ravb driver. As the driver is used by >> different IP variants, with different behaviors, to be able to have the >> runtime PM support available for all devices, the preparatory commits >> moved all the resources parsing and allocations in the driver's probe >> function and kept the settings for ravb_open(). This is due to the fact >> that on some IP variants-platforms tuples disabling/enabling the clocks >> will switch the IP to the reset operation mode where registers' content is >> lost and reconfiguration needs to be done. For this the rabv_open() >> function enables the clocks, switches the IP to configuration mode, applies >> all the registers settings and switches the IP to the operational mode. At >> the end of ravb_open() IP is ready to send/receive data. >> >> In ravb_close() necessary reverts are done (compared with ravb_open()), the >> IP is switched to reset mode and clocks are disabled. >> >> The ethtool APIs or IOCTLs that might execute while the interface is down >> are either cached (and applied in ravb_open()) or rejected (as at that time >> the IP is in reset mode). Keeping the IP in the reset mode also increases >> the power saved (according to the hardware manual). >> >> Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> >> 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 e909960fbc30..e99351fe8d7f 100644 >> --- a/drivers/net/ethernet/renesas/ravb_main.c >> +++ b/drivers/net/ethernet/renesas/ravb_main.c > [...] >> @@ -2233,7 +2243,14 @@ static int ravb_close(struct net_device *ndev) >> ravb_get_stats(ndev); >> >> /* Set reset mode. */ >> - return ravb_set_opmode(ndev, CCC_OPC_RESET); >> + error = ravb_set_opmode(ndev, CCC_OPC_RESET); >> + if (error) >> + return error; >> + >> + pm_runtime_mark_last_busy(dev); >> + pm_runtime_put_autosuspend(dev); > > BTW, doesn't make sense to check/return the result? Most users of this are not checking return code, I followed this pattern, too. But, yes, would be better to check it, AFAICT. > >> + >> + return 0; >> } >> >> static int ravb_hwtstamp_get(struct net_device *ndev, struct ifreq *req) > [...] > > MBR, Sergey
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index e909960fbc30..e99351fe8d7f 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -1840,16 +1840,21 @@ static int ravb_open(struct net_device *ndev) { struct ravb_private *priv = netdev_priv(ndev); const struct ravb_hw_info *info = priv->info; + struct device *dev = &priv->pdev->dev; int error; napi_enable(&priv->napi[RAVB_BE]); if (info->nc_queues) napi_enable(&priv->napi[RAVB_NC]); + error = pm_runtime_resume_and_get(dev); + if (error < 0) + goto out_napi_off; + /* Set AVB config mode */ error = ravb_set_config_mode(ndev); if (error) - goto out_napi_off; + goto out_rpm_put; ravb_set_delay_mode(ndev); ravb_write(ndev, priv->desc_bat_dma, DBAT); @@ -1883,6 +1888,9 @@ static int ravb_open(struct net_device *ndev) ravb_stop_dma(ndev); out_set_reset: ravb_set_opmode(ndev, CCC_OPC_RESET); +out_rpm_put: + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); out_napi_off: if (info->nc_queues) napi_disable(&priv->napi[RAVB_NC]); @@ -2184,6 +2192,8 @@ static int ravb_close(struct net_device *ndev) struct ravb_private *priv = netdev_priv(ndev); const struct ravb_hw_info *info = priv->info; struct ravb_tstamp_skb *ts_skb, *ts_skb2; + struct device *dev = &priv->pdev->dev; + int error; netif_tx_stop_all_queues(ndev); @@ -2233,7 +2243,14 @@ static int ravb_close(struct net_device *ndev) ravb_get_stats(ndev); /* Set reset mode. */ - return ravb_set_opmode(ndev, CCC_OPC_RESET); + error = ravb_set_opmode(ndev, CCC_OPC_RESET); + if (error) + return error; + + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + + return 0; } static int ravb_hwtstamp_get(struct net_device *ndev, struct ifreq *req) @@ -2725,6 +2742,8 @@ static int ravb_probe(struct platform_device *pdev) clk_prepare(priv->refclk); platform_set_drvdata(pdev, ndev); + pm_runtime_set_autosuspend_delay(&pdev->dev, 100); + pm_runtime_use_autosuspend(&pdev->dev); pm_runtime_enable(&pdev->dev); error = pm_runtime_resume_and_get(&pdev->dev); if (error < 0) @@ -2830,6 +2849,9 @@ static int ravb_probe(struct platform_device *pdev) netdev_info(ndev, "Base address at %#x, %pM, IRQ %d.\n", (u32)ndev->base_addr, ndev->dev_addr, ndev->irq); + pm_runtime_mark_last_busy(&pdev->dev); + pm_runtime_put_autosuspend(&pdev->dev); + return 0; out_napi_del: @@ -2847,6 +2869,7 @@ static int ravb_probe(struct platform_device *pdev) pm_runtime_put(&pdev->dev); out_rpm_disable: pm_runtime_disable(&pdev->dev); + pm_runtime_dont_use_autosuspend(&pdev->dev); clk_unprepare(priv->refclk); out_reset_assert: reset_control_assert(rstc); @@ -2860,6 +2883,12 @@ static void ravb_remove(struct platform_device *pdev) struct net_device *ndev = platform_get_drvdata(pdev); struct ravb_private *priv = netdev_priv(ndev); const struct ravb_hw_info *info = priv->info; + struct device *dev = &priv->pdev->dev; + int error; + + error = pm_runtime_resume_and_get(dev); + if (error < 0) + return; unregister_netdev(ndev); if (info->nc_queues) @@ -2871,8 +2900,9 @@ static void ravb_remove(struct platform_device *pdev) dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat, priv->desc_bat_dma); - pm_runtime_put_sync(&pdev->dev); + pm_runtime_put_sync_suspend(&pdev->dev); pm_runtime_disable(&pdev->dev); + pm_runtime_dont_use_autosuspend(dev); clk_unprepare(priv->refclk); reset_control_assert(priv->rstc); free_netdev(ndev); @@ -2954,6 +2984,10 @@ static int ravb_suspend(struct device *dev) if (ret) return ret; + ret = pm_runtime_force_suspend(&priv->pdev->dev); + if (ret) + return ret; + reset_assert: return reset_control_assert(priv->rstc); } @@ -2976,16 +3010,28 @@ static int ravb_resume(struct device *dev) ret = ravb_wol_restore(ndev); if (ret) return ret; + } else { + ret = pm_runtime_force_resume(dev); + if (ret) + return ret; } /* Reopening the interface will restore the device to the working state. */ ret = ravb_open(ndev); if (ret < 0) - return ret; + goto out_rpm_put; ravb_set_rx_mode(ndev); netif_device_attach(ndev); + return 0; + +out_rpm_put: + if (!priv->wol_enabled) { + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + } + return ret; }