From: Claudiu Beznea <[email protected]>
Hi,
This series adds suspend to RAM and runtime PM support for Ethernet
IP available on the RZ/G3S (R9A08G045) SoC.
As there are IP versions that switch to module standby when disabling
the clocks, and because of module standby IP switches to reset and
the register content is lost, to be able to have runtime PM supported
for all IP variants, the configuration operations were moved all to
ravb_open()/ravb_close() letting the ravb_probe() and ravb_remove()
to deal with resource parsing and allocation/free.
The ethtool and IOCTL APIs that could have been run asyncronously
were adapted to return if the interface is down. As explained in
each individual commits description, this should be harmless.
Along with it, the series contains preparatory cleanups.
The series has been tested on the boards with the following device trees:
- r8a7742-iwg21d-q7.dts
- r8a774a1-hihope-rzg2m-ex.dts
- r9a07g043u11-smarc-rzg2ul.dts
- r9a07g054l2-smarc-rzv2l.dts
- r9a07g044l2-smarc-rzg2l.dts
Patches are based on series at [1].
Thank you,
Claudiu Beznea
Changes in v2:
- rework the driver (mainly, ravb_open() contains now only resource
allocation and parsing leaving the settings to ravb_open(); ravb_remove()
has been adapted accordingly) to be able to use runtime PM for all
IP variants; due to this number of patches increased
- adjust previous series to review comments
- collected tags
- populated driver's own runtime PM ops with enable/disable of reference
clock
[1] https://lore.kernel.org/all/[email protected]/
Claudiu Beznea (21):
net: ravb: Let IP-specific receive function to interrogate descriptors
net: ravb: Rely on PM domain to enable gptp_clk
net: ravb: Make reset controller support mandatory
net: ravb: Switch to SYSTEM_SLEEP_PM_OPS()/RUNTIME_PM_OPS() and
pm_ptr()
net: ravb: Use tabs instead of spaces
net: ravb: Assert/de-assert reset on suspend/resume
net: ravb: Move reference clock enable/disable on runtime PM APIs
net: ravb: Move the IRQs get and request in the probe function
net: ravb: Split GTI computation and set operations
net: ravb: Move delay mode set in the driver's ndo_open API
net: ravb: Move DBAT configuration to the driver's ndo_open API
net: ravb: Move ptp initialization in the driver's ndo_open API for
ccc_gac platorms
net: ravb: Set config mode in ndo_open and reset mode in ndo_close
net: ravb: Simplify ravb_suspend()
net: ravb: Simplify ravb_resume()
net: ravb: Keep the reverse order of operations in ravb_close()
net: ravb: Keep clock request operations grouped together
net: ravb: Return cached statistics if the interface is down
net: ravb: Do not set promiscuous mode if the interface is down
net: ravb: Do not apply RX CSUM settings to hardware if the interface
is down
net: ravb: Add runtime PM support
drivers/net/ethernet/renesas/ravb.h | 2 +
drivers/net/ethernet/renesas/ravb_main.c | 783 ++++++++++++-----------
2 files changed, 417 insertions(+), 368 deletions(-)
--
2.39.2
From: Claudiu Beznea <[email protected]>
DBAT setup was done in the driver's probe API. As some IP variants switch
to reset mode (and thus registers' content is lost) when setting clocks
(due to module standby functionality) to be able to implement runtime PM
move the DBAT configuration in the driver's ndo_open API.
This commit prepares the code for the addition of runtime PM.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- none; this patch is new
drivers/net/ethernet/renesas/ravb_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 04eaa1967651..6b8ca08be35e 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1822,6 +1822,7 @@ static int ravb_open(struct net_device *ndev)
napi_enable(&priv->napi[RAVB_NC]);
ravb_set_delay_mode(ndev);
+ ravb_write(ndev, priv->desc_bat_dma, DBAT);
/* Device init */
error = ravb_dmac_init(ndev);
@@ -2841,7 +2842,6 @@ static int ravb_probe(struct platform_device *pdev)
}
for (q = RAVB_BE; q < DBAT_ENTRY_NUM; q++)
priv->desc_bat[q].die_dt = DT_EOS;
- ravb_write(ndev, priv->desc_bat_dma, DBAT);
/* Initialise HW timestamp list */
INIT_LIST_HEAD(&priv->ts_skb_list);
--
2.39.2
From: Claudiu Beznea <[email protected]>
The initialization sequence for PTP is the same for platforms with ccc_gac
and gptp (according to chapter "Figure 50.71 Flow of gPTP Initialization
(Normal, Common to All Modes)" of the R-Car Series, 3rd generation hardware
manual and chapter "Figure 37A.53 Flow of gPTP Initialization (Normal,
Common to All Modes)" of the RZ/G Series hardware manual).
As some IP variants switch to reset mode (and thus registers' content is
lost) when setting clocks (due to module standby functionality) to be able
to implement runtime PM, move the PTP initialization to the driver's
ndo_open API.
This commit prepares the code for the addition of runtime PM.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- none; this patch is new
drivers/net/ethernet/renesas/ravb_main.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 6b8ca08be35e..db9222fc57c2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1835,7 +1835,7 @@ static int ravb_open(struct net_device *ndev)
goto out_dma_stop;
/* Initialise PTP Clock driver */
- if (info->gptp)
+ if (info->gptp || info->ccc_gac)
ravb_ptp_init(ndev, priv->pdev);
/* PHY control start */
@@ -1849,7 +1849,7 @@ static int ravb_open(struct net_device *ndev)
out_ptp_stop:
/* Stop PTP Clock driver */
- if (info->gptp)
+ if (info->gptp || info->ccc_gac)
ravb_ptp_stop(ndev);
out_dma_stop:
ravb_stop_dma(ndev);
@@ -2151,7 +2151,7 @@ static int ravb_close(struct net_device *ndev)
ravb_write(ndev, 0, TIC);
/* Stop PTP Clock driver */
- if (info->gptp)
+ if (info->gptp || info->ccc_gac)
ravb_ptp_stop(ndev);
/* Set the config mode to stop the AVB-DMAC's processes */
@@ -2846,10 +2846,6 @@ static int ravb_probe(struct platform_device *pdev)
/* Initialise HW timestamp list */
INIT_LIST_HEAD(&priv->ts_skb_list);
- /* Initialise PTP Clock driver */
- if (info->ccc_gac)
- ravb_ptp_init(ndev, pdev);
-
/* Debug message level */
priv->msg_enable = RAVB_DEF_MSG_ENABLE;
@@ -2894,10 +2890,6 @@ static int ravb_probe(struct platform_device *pdev)
out_dma_free:
dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
priv->desc_bat_dma);
-
- /* Stop PTP Clock driver */
- if (info->ccc_gac)
- ravb_ptp_stop(ndev);
out_rpm_put:
pm_runtime_put(&pdev->dev);
out_rpm_disable:
@@ -2924,10 +2916,6 @@ static void ravb_remove(struct platform_device *pdev)
ravb_mdio_release(priv);
- /* Stop PTP Clock driver */
- if (info->ccc_gac)
- ravb_ptp_stop(ndev);
-
dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat,
priv->desc_bat_dma);
--
2.39.2
On Fri, 15 Dec 2023 11:44:32 +0200 claudiu beznea wrote:
> > You have to wait for fixes to land, we marge the trees every week.
>
> The intention was to let the reviewers know they should apply [1] (if any)
> for reviewing this series.
If there's a dependency please post the "later" thing as RFC.
We can't apply it, and it saves us clicking it away in patchwork.
From: Claudiu Beznea <[email protected]>
RZ/G3S can go to deep sleep states where power to most of the SoC parts is
off. When resuming from such a state, the Ethernet controller needs to be
reinitialized. De-asserting the reset signal for it should also be done.
Thus, add reset assert/de-assert on suspend/resume functions.
On the resume function, the de-assert was not reverted in case of failures
to give the user a chance to restore the interface (e.g., bringing down/up
the interface) in case suspend/resume failed.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- fixed typos in patch description and subject
- on ravb_suspend() assert the reset signal in case interface is down;
due to this the Sergey's Rb tag was left aside in this version
drivers/net/ethernet/renesas/ravb_main.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 5a57383762e7..9a618d8dbfcd 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2985,7 +2985,7 @@ static int ravb_suspend(struct device *dev)
int ret;
if (!netif_running(ndev))
- return 0;
+ goto reset_assert;
netif_device_detach(ndev);
@@ -2997,7 +2997,11 @@ static int ravb_suspend(struct device *dev)
if (priv->info->ccc_gac)
ravb_ptp_stop(ndev);
- return ret;
+ if (priv->wol_enabled)
+ return ret;
+
+reset_assert:
+ return reset_control_assert(priv->rstc);
}
static int ravb_resume(struct device *dev)
@@ -3005,7 +3009,11 @@ static int ravb_resume(struct device *dev)
struct net_device *ndev = dev_get_drvdata(dev);
struct ravb_private *priv = netdev_priv(ndev);
const struct ravb_hw_info *info = priv->info;
- int ret = 0;
+ int ret;
+
+ ret = reset_control_deassert(priv->rstc);
+ if (ret)
+ return ret;
/* If WoL is enabled set reset mode to rearm the WoL logic */
if (priv->wol_enabled) {
--
2.39.2
From: Claudiu Beznea <[email protected]>
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 <[email protected]>
---
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
From: Claudiu Beznea <[email protected]>
Remove explicit calls to functions that are called ravb_open(). There is
no need to have them doubled now that the ravb_open() already contains
what is needed for the interface configuration. Along with it, PTP needed
configurations were moved to ravb_wol_restore(). With this, code in
ravb_resume() is simpler.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- none; this patch is new
drivers/net/ethernet/renesas/ravb_main.c | 60 ++++++++++--------------
1 file changed, 24 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 16450bf241cd..b581666e341f 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2978,6 +2978,20 @@ static int ravb_wol_restore(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
const struct ravb_hw_info *info = priv->info;
+ int error;
+
+ /* Set reset mode to rearm the WoL logic. */
+ error = ravb_set_reset_mode(ndev);
+ if (error)
+ return error;
+
+ /* Set AVB config mode. */
+ error = ravb_set_config_mode(ndev);
+ if (error)
+ return error;
+
+ if (priv->info->ccc_gac)
+ ravb_ptp_init(ndev, priv->pdev);
if (info->nc_queues)
napi_enable(&priv->napi[RAVB_NC]);
@@ -3017,55 +3031,29 @@ static int ravb_resume(struct device *dev)
{
struct net_device *ndev = dev_get_drvdata(dev);
struct ravb_private *priv = netdev_priv(ndev);
- const struct ravb_hw_info *info = priv->info;
int ret;
ret = reset_control_deassert(priv->rstc);
if (ret)
return ret;
- /* If WoL is enabled set reset mode to rearm the WoL logic */
+ if (!netif_running(ndev))
+ return 0;
+
+ /* If WoL is enabled restore the interface. */
if (priv->wol_enabled) {
- ret = ravb_set_reset_mode(ndev);
+ ret = ravb_wol_restore(ndev);
if (ret)
return ret;
}
- /* All register have been reset to default values.
- * Restore all registers which where setup at probe time and
- * reopen device if it was running before system suspended.
- */
-
- /* Set AVB config mode */
- ret = ravb_set_config_mode(ndev);
- if (ret)
+ /* Reopening the interface will restore the device to the working state. */
+ ret = ravb_open(ndev);
+ if (ret < 0)
return ret;
- ret = ravb_set_gti(ndev);
- if (ret)
- return ret;
-
- if (info->internal_delay)
- ravb_set_delay_mode(ndev);
-
- /* Restore descriptor base address table */
- ravb_write(ndev, priv->desc_bat_dma, DBAT);
-
- if (priv->info->ccc_gac)
- ravb_ptp_init(ndev, priv->pdev);
-
- if (netif_running(ndev)) {
- if (priv->wol_enabled) {
- ret = ravb_wol_restore(ndev);
- if (ret)
- return ret;
- }
- ret = ravb_open(ndev);
- if (ret < 0)
- return ret;
- ravb_set_rx_mode(ndev);
- netif_device_attach(ndev);
- }
+ ravb_set_rx_mode(ndev);
+ netif_device_attach(ndev);
return ret;
}
--
2.39.2
From: Claudiu Beznea <[email protected]>
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).
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- keep RPM support for all platforms
drivers/net/ethernet/renesas/ravb_main.c | 47 ++++++++++++++++++++++--
1 file changed, 44 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 9ff943dff522..0733b63ff910 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1839,16 +1839,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);
@@ -1894,6 +1899,9 @@ static int ravb_open(struct net_device *ndev)
ravb_stop_dma(ndev);
out_set_reset:
ravb_set_reset_mode(ndev);
+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]);
@@ -2189,6 +2197,7 @@ 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;
netif_tx_stop_all_queues(ndev);
@@ -2237,6 +2246,9 @@ static int ravb_close(struct net_device *ndev)
/* Set reset mode. */
ravb_set_reset_mode(ndev);
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_put_autosuspend(dev);
+
return 0;
}
@@ -2808,6 +2820,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)
@@ -2916,6 +2930,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_netdev_unregister:
@@ -2934,6 +2951,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);
@@ -2947,6 +2965,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)
@@ -2958,8 +2982,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);
@@ -3041,6 +3066,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);
}
@@ -3063,16 +3092,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;
}
--
2.39.2
On 12/14/23 2:45 PM, Claudiu wrote:
> From: Claudiu Beznea <[email protected]>
>
> The initialization sequence for PTP is the same for platforms with ccc_gac
> and gptp (according to chapter "Figure 50.71 Flow of gPTP Initialization
> (Normal, Common to All Modes)" of the R-Car Series, 3rd generation hardware
Figure is hardly a chapter. :-)
> manual and chapter "Figure 37A.53 Flow of gPTP Initialization (Normal,
Here as well...
> Common to All Modes)" of the RZ/G Series hardware manual).
>
> As some IP variants switch to reset mode (and thus registers' content is
The register content.
> lost) when setting clocks (due to module standby functionality) to be able
> to implement runtime PM, move the PTP initialization to the driver's
> ndo_open API.
>
> This commit prepares the code for the addition of runtime PM.
>
> Signed-off-by: Claudiu Beznea <[email protected]>
Reviewed-by: Sergey Shtylyov <[email protected]>
[...]
MBR, Sergey
On 12/14/23 2:45 PM, Claudiu wrote:
> From: Claudiu Beznea <[email protected]>
>
> Remove explicit calls to functions that are called ravb_open(). There is
^ by?
> no need to have them doubled now that the ravb_open() already contains
> what is needed for the interface configuration. Along with it, PTP needed
> configurations were moved to ravb_wol_restore(). With this, code in
Configurations needed by PTP?
> ravb_resume() is simpler.
s/is/becomes/?
> Signed-off-by: Claudiu Beznea <[email protected]>
Reviewed-by: Sergey Shtylyov <[email protected]>
[...]
MBR, Sergey
On 12/14/23 2:45 PM, Claudiu wrote:
> From: Claudiu Beznea <[email protected]>
>
> 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 <[email protected]>
[...]
> 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
On 12/14/23 2:46 PM, Claudiu wrote:
> From: Claudiu Beznea <[email protected]>
>
> 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
The register contents.
> 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).
>
> Signed-off-by: Claudiu Beznea <[email protected]>
Reviewed-by: Sergey Shtylyov <[email protected]>
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 9ff943dff522..0733b63ff910 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1839,16 +1839,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;
> +
Note that sh_eth.c does this before enabling NAPI...
[...]
MBR, Sergey
From: Claudiu Beznea <[email protected]>
Keep the reverse order of operations in ravb_close() when comparing with
ravb_open(). This is the recommended configuration sequence.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- none; this patch is new
drivers/net/ethernet/renesas/ravb_main.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index b581666e341f..38999ef1ea85 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2182,6 +2182,14 @@ static int ravb_close(struct net_device *ndev)
ravb_write(ndev, 0, RIC2);
ravb_write(ndev, 0, TIC);
+ /* PHY disconnect */
+ if (ndev->phydev) {
+ phy_stop(ndev->phydev);
+ phy_disconnect(ndev->phydev);
+ if (of_phy_is_fixed_link(np))
+ of_phy_deregister_fixed_link(np);
+ }
+
/* Stop PTP Clock driver */
if (info->gptp || info->ccc_gac)
ravb_ptp_stop(ndev);
@@ -2200,14 +2208,6 @@ static int ravb_close(struct net_device *ndev)
}
}
- /* PHY disconnect */
- if (ndev->phydev) {
- phy_stop(ndev->phydev);
- phy_disconnect(ndev->phydev);
- if (of_phy_is_fixed_link(np))
- of_phy_deregister_fixed_link(np);
- }
-
cancel_work_sync(&priv->work);
if (info->nc_queues)
--
2.39.2
From: Claudiu Beznea <[email protected]>
Move the IRQs get and request in the driver's probe function. As some IP
variants switches to reset operation mode as a result of setting module
standby through clock enable/disable APIs, to implement runtime PM the
resource parsing and requests are moved in the probe function and IP
settings are moved in the open functions. This is a preparatory change to
add runtime PM support for all IP variants.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- none; this patch is new
drivers/net/ethernet/renesas/ravb_main.c | 274 +++++++++++------------
1 file changed, 132 insertions(+), 142 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 83691a0f0cc2..d7f6e8ea8e79 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1731,7 +1731,7 @@ static inline int ravb_hook_irq(unsigned int irq, irq_handler_t handler,
name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s", ndev->name, ch);
if (!name)
return -ENOMEM;
- error = request_irq(irq, handler, 0, name, ndev);
+ error = devm_request_irq(dev, irq, handler, 0, name, ndev);
if (error)
netdev_err(ndev, "cannot request IRQ %s\n", name);
@@ -1755,63 +1755,16 @@ static int ravb_open(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
const struct ravb_hw_info *info = priv->info;
- struct platform_device *pdev = priv->pdev;
- struct device *dev = &pdev->dev;
int error;
napi_enable(&priv->napi[RAVB_BE]);
if (info->nc_queues)
napi_enable(&priv->napi[RAVB_NC]);
- if (!info->multi_irqs) {
- error = request_irq(ndev->irq, ravb_interrupt, IRQF_SHARED,
- ndev->name, ndev);
- if (error) {
- netdev_err(ndev, "cannot request IRQ\n");
- goto out_napi_off;
- }
- } else {
- error = ravb_hook_irq(ndev->irq, ravb_multi_interrupt, ndev,
- dev, "ch22:multi");
- if (error)
- goto out_napi_off;
- error = ravb_hook_irq(priv->emac_irq, ravb_emac_interrupt, ndev,
- dev, "ch24:emac");
- if (error)
- goto out_free_irq;
- error = ravb_hook_irq(priv->rx_irqs[RAVB_BE], ravb_be_interrupt,
- ndev, dev, "ch0:rx_be");
- if (error)
- goto out_free_irq_emac;
- error = ravb_hook_irq(priv->tx_irqs[RAVB_BE], ravb_be_interrupt,
- ndev, dev, "ch18:tx_be");
- if (error)
- goto out_free_irq_be_rx;
- error = ravb_hook_irq(priv->rx_irqs[RAVB_NC], ravb_nc_interrupt,
- ndev, dev, "ch1:rx_nc");
- if (error)
- goto out_free_irq_be_tx;
- error = ravb_hook_irq(priv->tx_irqs[RAVB_NC], ravb_nc_interrupt,
- ndev, dev, "ch19:tx_nc");
- if (error)
- goto out_free_irq_nc_rx;
-
- if (info->err_mgmt_irqs) {
- error = ravb_hook_irq(priv->erra_irq, ravb_multi_interrupt,
- ndev, dev, "err_a");
- if (error)
- goto out_free_irq_nc_tx;
- error = ravb_hook_irq(priv->mgmta_irq, ravb_multi_interrupt,
- ndev, dev, "mgmt_a");
- if (error)
- goto out_free_irq_erra;
- }
- }
-
/* Device init */
error = ravb_dmac_init(ndev);
if (error)
- goto out_free_irq_mgmta;
+ goto out_napi_off;
ravb_emac_init(ndev);
/* Initialise PTP Clock driver */
@@ -1832,26 +1785,6 @@ static int ravb_open(struct net_device *ndev)
if (info->gptp)
ravb_ptp_stop(ndev);
ravb_stop_dma(ndev);
-out_free_irq_mgmta:
- if (!info->multi_irqs)
- goto out_free_irq;
- if (info->err_mgmt_irqs)
- free_irq(priv->mgmta_irq, ndev);
-out_free_irq_erra:
- if (info->err_mgmt_irqs)
- free_irq(priv->erra_irq, ndev);
-out_free_irq_nc_tx:
- free_irq(priv->tx_irqs[RAVB_NC], ndev);
-out_free_irq_nc_rx:
- free_irq(priv->rx_irqs[RAVB_NC], ndev);
-out_free_irq_be_tx:
- free_irq(priv->tx_irqs[RAVB_BE], ndev);
-out_free_irq_be_rx:
- free_irq(priv->rx_irqs[RAVB_BE], ndev);
-out_free_irq_emac:
- free_irq(priv->emac_irq, ndev);
-out_free_irq:
- free_irq(ndev->irq, ndev);
out_napi_off:
if (info->nc_queues)
napi_disable(&priv->napi[RAVB_NC]);
@@ -2177,19 +2110,6 @@ static int ravb_close(struct net_device *ndev)
cancel_work_sync(&priv->work);
- if (info->multi_irqs) {
- free_irq(priv->tx_irqs[RAVB_NC], ndev);
- free_irq(priv->rx_irqs[RAVB_NC], ndev);
- free_irq(priv->tx_irqs[RAVB_BE], ndev);
- free_irq(priv->rx_irqs[RAVB_BE], ndev);
- free_irq(priv->emac_irq, ndev);
- if (info->err_mgmt_irqs) {
- free_irq(priv->erra_irq, ndev);
- free_irq(priv->mgmta_irq, ndev);
- }
- }
- free_irq(ndev->irq, ndev);
-
if (info->nc_queues)
napi_disable(&priv->napi[RAVB_NC]);
napi_disable(&priv->napi[RAVB_BE]);
@@ -2616,6 +2536,127 @@ static void ravb_parse_delay_mode(struct device_node *np, struct net_device *nde
}
}
+static int ravb_get_irqs(struct ravb_private *priv)
+{
+ const char *err_a_irq_name = NULL, *mgmt_a_irq_name = NULL;
+ const struct ravb_hw_info *info = priv->info;
+ struct platform_device *pdev = priv->pdev;
+ struct net_device *ndev = priv->ndev;
+ const char *irq_name, *emac_irq_name;
+ int i, irq;
+
+ if (!info->multi_irqs) {
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0)
+ return irq;
+
+ ndev->irq = irq;
+ return 0;
+ }
+
+ if (info->err_mgmt_irqs) {
+ irq_name = "dia";
+ emac_irq_name = "line3";
+ err_a_irq_name = "err_a";
+ mgmt_a_irq_name = "mgmt_a";
+ } else {
+ irq_name = "ch22";
+ emac_irq_name = "ch24";
+ }
+
+ irq = platform_get_irq_byname(pdev, irq_name);
+ if (irq < 0)
+ return irq;
+ ndev->irq = irq;
+
+ irq = platform_get_irq_byname(pdev, emac_irq_name);
+ if (irq < 0)
+ return irq;
+ priv->emac_irq = irq;
+
+ if (err_a_irq_name) {
+ irq = platform_get_irq_byname(pdev, "err_a");
+ if (irq < 0)
+ return irq;
+ priv->erra_irq = irq;
+ }
+
+ if (mgmt_a_irq_name) {
+ irq = platform_get_irq_byname(pdev, "mgmt_a");
+ if (irq < 0)
+ return irq;
+ priv->mgmta_irq = irq;
+ }
+
+ for (i = 0; i < NUM_RX_QUEUE; i++) {
+ irq = platform_get_irq_byname(pdev, ravb_rx_irqs[i]);
+ if (irq < 0)
+ return irq;
+ priv->rx_irqs[i] = irq;
+ }
+ for (i = 0; i < NUM_TX_QUEUE; i++) {
+ irq = platform_get_irq_byname(pdev, ravb_tx_irqs[i]);
+ if (irq < 0)
+ return irq;
+ priv->tx_irqs[i] = irq;
+ }
+
+ return 0;
+}
+
+static int ravb_request_irqs(struct ravb_private *priv)
+{
+ const struct ravb_hw_info *info = priv->info;
+ struct net_device *ndev = priv->ndev;
+ struct device *dev = &priv->pdev->dev;
+ int error;
+
+ if (!info->multi_irqs) {
+ error = devm_request_irq(dev, ndev->irq, ravb_interrupt, IRQF_SHARED,
+ ndev->name, ndev);
+ if (error)
+ netdev_err(ndev, "cannot request IRQ\n");
+
+ return error;
+ }
+
+ error = ravb_hook_irq(ndev->irq, ravb_multi_interrupt, ndev,
+ dev, "ch22:multi");
+ if (error)
+ return error;
+ error = ravb_hook_irq(priv->emac_irq, ravb_emac_interrupt, ndev,
+ dev, "ch24:emac");
+ if (error)
+ return error;
+ error = ravb_hook_irq(priv->rx_irqs[RAVB_BE], ravb_be_interrupt,
+ ndev, dev, "ch0:rx_be");
+ if (error)
+ return error;
+ error = ravb_hook_irq(priv->tx_irqs[RAVB_BE], ravb_be_interrupt,
+ ndev, dev, "ch18:tx_be");
+ if (error)
+ return error;
+ error = ravb_hook_irq(priv->rx_irqs[RAVB_NC], ravb_nc_interrupt,
+ ndev, dev, "ch1:rx_nc");
+ if (error)
+ return error;
+ error = ravb_hook_irq(priv->tx_irqs[RAVB_NC], ravb_nc_interrupt,
+ ndev, dev, "ch19:tx_nc");
+ if (error)
+ return error;
+
+ if (!info->err_mgmt_irqs)
+ return 0;
+
+ error = ravb_hook_irq(priv->erra_irq, ravb_multi_interrupt,
+ ndev, dev, "err_a");
+ if (error)
+ return error;
+
+ return ravb_hook_irq(priv->mgmta_irq, ravb_multi_interrupt,
+ ndev, dev, "mgmt_a");
+}
+
static void ravb_set_delay_mode(struct net_device *ndev)
{
struct ravb_private *priv = netdev_priv(ndev);
@@ -2635,9 +2676,8 @@ static int ravb_probe(struct platform_device *pdev)
struct reset_control *rstc;
struct ravb_private *priv;
struct net_device *ndev;
- int error, irq, q;
struct resource *res;
- int i;
+ int error, q;
if (!np) {
dev_err(&pdev->dev,
@@ -2664,20 +2704,6 @@ static int ravb_probe(struct platform_device *pdev)
if (error)
goto out_free_netdev;
- if (info->multi_irqs) {
- if (info->err_mgmt_irqs)
- irq = platform_get_irq_byname(pdev, "dia");
- else
- irq = platform_get_irq_byname(pdev, "ch22");
- } else {
- irq = platform_get_irq(pdev, 0);
- }
- if (irq < 0) {
- error = irq;
- goto out_reset_assert;
- }
- ndev->irq = irq;
-
SET_NETDEV_DEV(ndev, &pdev->dev);
priv = netdev_priv(ndev);
@@ -2692,6 +2718,14 @@ static int ravb_probe(struct platform_device *pdev)
priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
}
+ error = ravb_get_irqs(priv);
+ if (error)
+ goto out_reset_assert;
+
+ error = ravb_request_irqs(priv);
+ if (error)
+ goto out_reset_assert;
+
priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
if (IS_ERR(priv->refclk)) {
error = PTR_ERR(priv->refclk);
@@ -2725,50 +2759,6 @@ static int ravb_probe(struct platform_device *pdev)
priv->avb_link_active_low =
of_property_read_bool(np, "renesas,ether-link-active-low");
- if (info->multi_irqs) {
- if (info->err_mgmt_irqs)
- irq = platform_get_irq_byname(pdev, "line3");
- else
- irq = platform_get_irq_byname(pdev, "ch24");
- if (irq < 0) {
- error = irq;
- goto out_rpm_put;
- }
- priv->emac_irq = irq;
- for (i = 0; i < NUM_RX_QUEUE; i++) {
- irq = platform_get_irq_byname(pdev, ravb_rx_irqs[i]);
- if (irq < 0) {
- error = irq;
- goto out_rpm_put;
- }
- priv->rx_irqs[i] = irq;
- }
- for (i = 0; i < NUM_TX_QUEUE; i++) {
- irq = platform_get_irq_byname(pdev, ravb_tx_irqs[i]);
- if (irq < 0) {
- error = irq;
- goto out_rpm_put;
- }
- priv->tx_irqs[i] = irq;
- }
-
- if (info->err_mgmt_irqs) {
- irq = platform_get_irq_byname(pdev, "err_a");
- if (irq < 0) {
- error = irq;
- goto out_rpm_put;
- }
- priv->erra_irq = irq;
-
- irq = platform_get_irq_byname(pdev, "mgmt_a");
- if (irq < 0) {
- error = irq;
- goto out_rpm_put;
- }
- priv->mgmta_irq = irq;
- }
- }
-
priv->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(priv->clk)) {
error = PTR_ERR(priv->clk);
--
2.39.2
From: Claudiu Beznea <[email protected]>
Reference clock could be or not part of the power domain. If it is part of
the power domain, the power domain takes care of propertly setting it. In
case it is not part of the power domain and full runtime PM support is
available in driver the clock will not be propertly disabled/enabled at
runtime. For this, keep the prepare/unprepare operations in the driver's
probe()/remove() functions and move the enable/disable in runtime PM
functions.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- this patch is new and follows the recommendations proposed in the
discussion of patch 08/13 ("net: ravb: Rely on PM domain to enable refclk")
from v2
drivers/net/ethernet/renesas/ravb_main.c | 88 +++++++++++++-----------
1 file changed, 46 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 9a618d8dbfcd..83691a0f0cc2 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -2664,11 +2664,6 @@ static int ravb_probe(struct platform_device *pdev)
if (error)
goto out_free_netdev;
- pm_runtime_enable(&pdev->dev);
- error = pm_runtime_resume_and_get(&pdev->dev);
- if (error < 0)
- goto out_rpm_disable;
-
if (info->multi_irqs) {
if (info->err_mgmt_irqs)
irq = platform_get_irq_byname(pdev, "dia");
@@ -2679,7 +2674,7 @@ static int ravb_probe(struct platform_device *pdev)
}
if (irq < 0) {
error = irq;
- goto out_release;
+ goto out_reset_assert;
}
ndev->irq = irq;
@@ -2697,10 +2692,23 @@ static int ravb_probe(struct platform_device *pdev)
priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
}
+ priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
+ if (IS_ERR(priv->refclk)) {
+ error = PTR_ERR(priv->refclk);
+ goto out_reset_assert;
+ }
+ clk_prepare(priv->refclk);
+
+ platform_set_drvdata(pdev, ndev);
+ pm_runtime_enable(&pdev->dev);
+ error = pm_runtime_resume_and_get(&pdev->dev);
+ if (error < 0)
+ goto out_rpm_disable;
+
priv->addr = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
if (IS_ERR(priv->addr)) {
error = PTR_ERR(priv->addr);
- goto out_release;
+ goto out_rpm_put;
}
/* The Ether-specific entries in the device structure. */
@@ -2711,7 +2719,7 @@ static int ravb_probe(struct platform_device *pdev)
error = of_get_phy_mode(np, &priv->phy_interface);
if (error && error != -ENODEV)
- goto out_release;
+ goto out_rpm_put;
priv->no_avb_link = of_property_read_bool(np, "renesas,no-ether-link");
priv->avb_link_active_low =
@@ -2724,14 +2732,14 @@ static int ravb_probe(struct platform_device *pdev)
irq = platform_get_irq_byname(pdev, "ch24");
if (irq < 0) {
error = irq;
- goto out_release;
+ goto out_rpm_put;
}
priv->emac_irq = irq;
for (i = 0; i < NUM_RX_QUEUE; i++) {
irq = platform_get_irq_byname(pdev, ravb_rx_irqs[i]);
if (irq < 0) {
error = irq;
- goto out_release;
+ goto out_rpm_put;
}
priv->rx_irqs[i] = irq;
}
@@ -2739,7 +2747,7 @@ static int ravb_probe(struct platform_device *pdev)
irq = platform_get_irq_byname(pdev, ravb_tx_irqs[i]);
if (irq < 0) {
error = irq;
- goto out_release;
+ goto out_rpm_put;
}
priv->tx_irqs[i] = irq;
}
@@ -2748,14 +2756,14 @@ static int ravb_probe(struct platform_device *pdev)
irq = platform_get_irq_byname(pdev, "err_a");
if (irq < 0) {
error = irq;
- goto out_release;
+ goto out_rpm_put;
}
priv->erra_irq = irq;
irq = platform_get_irq_byname(pdev, "mgmt_a");
if (irq < 0) {
error = irq;
- goto out_release;
+ goto out_rpm_put;
}
priv->mgmta_irq = irq;
}
@@ -2764,21 +2772,14 @@ static int ravb_probe(struct platform_device *pdev)
priv->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(priv->clk)) {
error = PTR_ERR(priv->clk);
- goto out_release;
+ goto out_rpm_put;
}
- priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
- if (IS_ERR(priv->refclk)) {
- error = PTR_ERR(priv->refclk);
- goto out_release;
- }
- clk_prepare_enable(priv->refclk);
-
if (info->gptp_ref_clk) {
priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
if (IS_ERR(priv->gptp_clk)) {
error = PTR_ERR(priv->gptp_clk);
- goto out_disable_refclk;
+ goto out_rpm_put;
}
}
@@ -2799,20 +2800,20 @@ static int ravb_probe(struct platform_device *pdev)
/* Set AVB config mode */
error = ravb_set_config_mode(ndev);
if (error)
- goto out_disable_refclk;
+ goto out_rpm_put;
if (info->gptp || info->ccc_gac) {
/* Set GTI value */
error = ravb_set_gti(ndev);
if (error)
- goto out_disable_refclk;
+ goto out_rpm_put;
/* Request GTI loading */
ravb_modify(ndev, GCCR, GCCR_LTI, GCCR_LTI);
/* Check completion status. */
error = ravb_wait(ndev, GCCR, GCCR_LTI, 0);
if (error)
- goto out_disable_refclk;
+ goto out_rpm_put;
}
if (info->internal_delay) {
@@ -2829,7 +2830,7 @@ static int ravb_probe(struct platform_device *pdev)
"Cannot allocate desc base address table (size %d bytes)\n",
priv->desc_bat_size);
error = -ENOMEM;
- goto out_disable_refclk;
+ goto out_rpm_put;
}
for (q = RAVB_BE; q < DBAT_ENTRY_NUM; q++)
priv->desc_bat[q].die_dt = DT_EOS;
@@ -2875,8 +2876,6 @@ 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);
- platform_set_drvdata(pdev, ndev);
-
return 0;
out_napi_del:
@@ -2892,12 +2891,12 @@ static int ravb_probe(struct platform_device *pdev)
/* Stop PTP Clock driver */
if (info->ccc_gac)
ravb_ptp_stop(ndev);
-out_disable_refclk:
- clk_disable_unprepare(priv->refclk);
-out_release:
+out_rpm_put:
pm_runtime_put(&pdev->dev);
out_rpm_disable:
pm_runtime_disable(&pdev->dev);
+ clk_unprepare(priv->refclk);
+out_reset_assert:
reset_control_assert(rstc);
out_free_netdev:
free_netdev(ndev);
@@ -2929,10 +2928,9 @@ static void ravb_remove(struct platform_device *pdev)
if (error)
netdev_err(ndev, "Failed to reset ndev\n");
- clk_disable_unprepare(priv->refclk);
-
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
+ clk_unprepare(priv->refclk);
reset_control_assert(priv->rstc);
free_netdev(ndev);
platform_set_drvdata(pdev, NULL);
@@ -3071,21 +3069,27 @@ static int ravb_resume(struct device *dev)
return ret;
}
-static int ravb_runtime_nop(struct device *dev)
+static int ravb_runtime_suspend(struct device *dev)
{
- /* Runtime PM callback shared between ->runtime_suspend()
- * and ->runtime_resume(). Simply returns success.
- *
- * This driver re-initializes all registers after
- * pm_runtime_get_sync() anyway so there is no need
- * to save and restore registers here.
- */
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct ravb_private *priv = netdev_priv(ndev);
+
+ clk_disable(priv->refclk);
+
return 0;
}
+static int ravb_runtime_resume(struct device *dev)
+{
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct ravb_private *priv = netdev_priv(ndev);
+
+ return clk_enable(priv->refclk);
+}
+
static const struct dev_pm_ops ravb_dev_pm_ops = {
SYSTEM_SLEEP_PM_OPS(ravb_suspend, ravb_resume)
- RUNTIME_PM_OPS(ravb_runtime_nop, ravb_runtime_nop, NULL)
+ RUNTIME_PM_OPS(ravb_runtime_suspend, ravb_runtime_resume, NULL)
};
static struct platform_driver ravb_driver = {
--
2.39.2
From: Claudiu Beznea <[email protected]>
ravb_poll() initial code used to interrogate the first descriptor of the
RX queue in case gPTP is false to determine if ravb_rx() should be called.
This is done for non-gPTP IPs. For gPTP IPs the driver PTP-specific
information was used to determine if receive function should be called. As
every IP has its own receive function that interrogates the RX descriptors
list in the same way the ravb_poll() was doing there is no need to double
check this in ravb_poll(). Removing the code from ravb_poll() leads to a
cleaner code.
Signed-off-by: Claudiu Beznea <[email protected]>
---
Changes in v2:
- addressed review comments and keep stale code out of this patch
drivers/net/ethernet/renesas/ravb_main.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index 1c253403a297..8e67a18b2924 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -1282,25 +1282,16 @@ static int ravb_poll(struct napi_struct *napi, int budget)
struct net_device *ndev = napi->dev;
struct ravb_private *priv = netdev_priv(ndev);
const struct ravb_hw_info *info = priv->info;
- bool gptp = info->gptp || info->ccc_gac;
- struct ravb_rx_desc *desc;
unsigned long flags;
int q = napi - priv->napi;
int mask = BIT(q);
int quota = budget;
- unsigned int entry;
- if (!gptp) {
- entry = priv->cur_rx[q] % priv->num_rx_ring[q];
- desc = &priv->gbeth_rx_ring[entry];
- }
/* Processing RX Descriptor Ring */
/* Clear RX interrupt */
ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
- if (gptp || desc->die_dt != DT_FEMPTY) {
- if (ravb_rx(ndev, "a, q))
- goto out;
- }
+ if (ravb_rx(ndev, "a, q))
+ goto out;
/* Processing TX Descriptor Ring */
spin_lock_irqsave(&priv->lock, flags);
--
2.39.2
On 16.12.2023 22:16, Sergey Shtylyov wrote:
> On 12/14/23 2:45 PM, Claudiu wrote:
>
>> From: Claudiu Beznea <[email protected]>
>>
>> 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 <[email protected]>
> [...]
>
>> 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
On 12/17/23 5:02 PM, claudiu beznea wrote:
[...]
>>> From: Claudiu Beznea <[email protected]>
>>>
>>> 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 <[email protected]>
>> [...]
>>
>>> 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