[PATCH net-next 0/3] sh_eth: PM-related cleanups

[PATCH net-next 0/3] sh_eth: PM-related cleanups

[Geert Uytterhoeven]

	Hi all,

This patch series contains various cleanups related to power management
for the Renesas SH Ethernet driver, as used on Renesas SH, ARM32, and
ARM64 platforms.

This has been tested on various SoCs (R-Mobile A1, RZ/A1H, RZ/A2M, R-Car
H1, R-Car M2-W).

Thanks for your comments!

Geert Uytterhoeven (3):
  net: sh_eth: Remove dummy Runtime PM callbacks
  net: sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
  net: sh_eth: Use async pm_runtime_put()

 drivers/net/ethernet/renesas/sh_eth.c | 30 ++++-----------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

-- 
2.43.0

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH net-next 1/3] sh_eth: Remove dummy Runtime PM callbacks

[Geert Uytterhoeven]

Since commit 63d00be69348fda4 ("PM: runtime: Allow unassigned
->runtime_suspend|resume callbacks"), unassigned
.runtime_{suspend,resume}() callbacks are treated the same as dummy
callbacks that just return zero.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/net/ethernet/renesas/sh_eth.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 5fc8027c92c7c95b..0516db9dc946fcdb 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3529,21 +3529,8 @@ static int sh_eth_resume(struct device *dev)
 }
 #endif
 
-static int sh_eth_runtime_nop(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.
-	 */
-	return 0;
-}
-
 static const struct dev_pm_ops sh_eth_dev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(sh_eth_suspend, sh_eth_resume)
-	SET_RUNTIME_PM_OPS(sh_eth_runtime_nop, sh_eth_runtime_nop, NULL)
 };
 #define SH_ETH_PM_OPS (&sh_eth_dev_pm_ops)
 #else
-- 
2.43.0

[PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Geert Uytterhoeven]

Convert the Renesas SuperH Ethernet driver from an open-coded dev_pm_ops
structure to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr().  This lets
us drop the checks for CONFIG_PM and CONFIG_PM_SLEEP without impacting
code size, while increasing build coverage.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/net/ethernet/renesas/sh_eth.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 0516db9dc946fcdb..f16a350736e1aa04 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3443,8 +3443,6 @@ static void sh_eth_drv_remove(struct platform_device *pdev)
 	free_netdev(ndev);
 }
 
-#ifdef CONFIG_PM
-#ifdef CONFIG_PM_SLEEP
 static int sh_eth_wol_setup(struct net_device *ndev)
 {
 	struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -3527,15 +3525,8 @@ static int sh_eth_resume(struct device *dev)
 
 	return ret;
 }
-#endif
 
-static const struct dev_pm_ops sh_eth_dev_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(sh_eth_suspend, sh_eth_resume)
-};
-#define SH_ETH_PM_OPS (&sh_eth_dev_pm_ops)
-#else
-#define SH_ETH_PM_OPS NULL
-#endif
+static DEFINE_SIMPLE_DEV_PM_OPS(sh_eth_dev_pm_ops, sh_eth_suspend, sh_eth_resume);
 
 static const struct platform_device_id sh_eth_id_table[] = {
 	{ "sh7619-ether", (kernel_ulong_t)&sh7619_data },
@@ -3555,7 +3546,7 @@ static struct platform_driver sh_eth_driver = {
 	.id_table = sh_eth_id_table,
 	.driver = {
 		   .name = CARDNAME,
-		   .pm = SH_ETH_PM_OPS,
+		   .pm = pm_sleep_ptr(&sh_eth_dev_pm_ops),
 		   .of_match_table = of_match_ptr(sh_eth_match_table),
 	},
 };
-- 
2.43.0

[PATCH net-next 3/3] sh_eth: Use async pm_runtime_put()

[Geert Uytterhoeven]

There is no stringent need to power down the device immediately after a
register read, or after a failed open.  Relax power down handling by
replacing calls to synchronous pm_runtime_put_sync() by calls to
asynchronous pm_runtime_put().

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/net/ethernet/renesas/sh_eth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index f16a350736e1aa04..5a367c5523bb860f 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -2233,7 +2233,7 @@ static void sh_eth_get_regs(struct net_device *ndev, struct ethtool_regs *regs,
 
 	pm_runtime_get_sync(&mdp->pdev->dev);
 	__sh_eth_get_regs(ndev, buf);
-	pm_runtime_put_sync(&mdp->pdev->dev);
+	pm_runtime_put(&mdp->pdev->dev);
 }
 
 static u32 sh_eth_get_msglevel(struct net_device *ndev)
@@ -2447,7 +2447,7 @@ static int sh_eth_open(struct net_device *ndev)
 	free_irq(ndev->irq, ndev);
 out_napi_off:
 	napi_disable(&mdp->napi);
-	pm_runtime_put_sync(&mdp->pdev->dev);
+	pm_runtime_put(&mdp->pdev->dev);
 	return ret;
 }
 
-- 
2.43.0

Re: [PATCH net-next 1/3] sh_eth: Remove dummy Runtime PM callbacks

[Andrew Lunn]

On Thu, Sep 04, 2025 at 05:18:56PM +0200, Geert Uytterhoeven wrote:
> Since commit 63d00be69348fda4 ("PM: runtime: Allow unassigned
> ->runtime_suspend|resume callbacks"), unassigned
> .runtime_{suspend,resume}() callbacks are treated the same as dummy
> callbacks that just return zero.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Andrew Lunn]

On Thu, Sep 04, 2025 at 05:18:57PM +0200, Geert Uytterhoeven wrote:
> Convert the Renesas SuperH Ethernet driver from an open-coded dev_pm_ops
> structure to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr().  This lets
> us drop the checks for CONFIG_PM and CONFIG_PM_SLEEP without impacting
> code size, while increasing build coverage.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/net/ethernet/renesas/sh_eth.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 0516db9dc946fcdb..f16a350736e1aa04 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -3443,8 +3443,6 @@ static void sh_eth_drv_remove(struct platform_device *pdev)
>  	free_netdev(ndev);
>  }
>  
> -#ifdef CONFIG_PM
> -#ifdef CONFIG_PM_SLEEP
>  static int sh_eth_wol_setup(struct net_device *ndev)

Humm, sh_eth_wol_setup() went from conditional to always available.

Can WoL work without PM? Is the current get_wol() and set_wol()
broken?

Maybe you can take a look at this, do some testing?

It is however not strictly related to this patch, so:

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 3/3] sh_eth: Use async pm_runtime_put()

[Andrew Lunn]

On Thu, Sep 04, 2025 at 05:18:58PM +0200, Geert Uytterhoeven wrote:
> There is no stringent need to power down the device immediately after a
> register read, or after a failed open.  Relax power down handling by
> replacing calls to synchronous pm_runtime_put_sync() by calls to
> asynchronous pm_runtime_put().
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH net-next 0/3] sh_eth: PM-related cleanups

[Niklas Söderlund]

Hi Geert,

Thanks for your work.

On 2025-09-04 17:18:55 +0200, Geert Uytterhoeven wrote:
> 	Hi all,
> 
> This patch series contains various cleanups related to power management
> for the Renesas SH Ethernet driver, as used on Renesas SH, ARM32, and
> ARM64 platforms.
> 
> This has been tested on various SoCs (R-Mobile A1, RZ/A1H, RZ/A2M, R-Car
> H1, R-Car M2-W).

For all three patches,

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

And I tested it on R-Car M2 so feel free to add,

Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> 
> Thanks for your comments!
> 
> Geert Uytterhoeven (3):
>   net: sh_eth: Remove dummy Runtime PM callbacks
>   net: sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
>   net: sh_eth: Use async pm_runtime_put()
> 
>  drivers/net/ethernet/renesas/sh_eth.c | 30 ++++-----------------------
>  1 file changed, 4 insertions(+), 26 deletions(-)
> 
> -- 
> 2.43.0
> 
> Gr{oetje,eeting}s,
> 
> 						Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> 							    -- Linus Torvalds

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Geert Uytterhoeven]

Hi Andrew,

Thanks for your comments!

On Thu, 4 Sept 2025 at 20:37, Andrew Lunn <andrew@lunn.ch> wrote:
> On Thu, Sep 04, 2025 at 05:18:57PM +0200, Geert Uytterhoeven wrote:
> > Convert the Renesas SuperH Ethernet driver from an open-coded dev_pm_ops
> > structure to DEFINE_SIMPLE_DEV_PM_OPS() and pm_sleep_ptr().  This lets
> > us drop the checks for CONFIG_PM and CONFIG_PM_SLEEP without impacting
> > code size, while increasing build coverage.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  drivers/net/ethernet/renesas/sh_eth.c | 13 ++-----------
> >  1 file changed, 2 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> > index 0516db9dc946fcdb..f16a350736e1aa04 100644
> > --- a/drivers/net/ethernet/renesas/sh_eth.c
> > +++ b/drivers/net/ethernet/renesas/sh_eth.c
> > @@ -3443,8 +3443,6 @@ static void sh_eth_drv_remove(struct platform_device *pdev)
> >       free_netdev(ndev);
> >  }
> >
> > -#ifdef CONFIG_PM
> > -#ifdef CONFIG_PM_SLEEP
> >  static int sh_eth_wol_setup(struct net_device *ndev)
>
> Humm, sh_eth_wol_setup() went from conditional to always available.

Its sole caller is sh_eth_suspend().  If the compiler optimizes away
the latter, it should optimize away the former, too (verified with an
m68k allmodconfig build).

> Can WoL work without PM? Is the current get_wol() and set_wol()
> broken?
>
> Maybe you can take a look at this, do some testing?

You cannot enter system sleep without CONFIG_PM_SLEEP, so enabling
WoL would be pointless.

> It is however not strictly related to this patch, so:
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Andrew Lunn]

> You cannot enter system sleep without CONFIG_PM_SLEEP, so enabling
> WoL would be pointless.

Yet get_wol will return WoL can be used, and set_wol will allow you to
configure it. It seems like EOPNOTSUPP would be better.

	  Andrew

Re: [PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Niklas Söderlund]

On 2025-09-05 13:57:05 +0200, Andrew Lunn wrote:
> > You cannot enter system sleep without CONFIG_PM_SLEEP, so enabling
> > WoL would be pointless.
> 
> Yet get_wol will return WoL can be used, and set_wol will allow you to
> configure it. It seems like EOPNOTSUPP would be better.

Out of curiosity. Are you suggesting a compile time check/construct for 
CONFIG_PM_SLEEP be added in the driver itself, or in ethtool_set_wol() 
and ethtool_get_wol() in net/ethtool/ioctl.c to complement the 

    if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol)
        return -EOPNOTSUPP;

checks already there? To always return EOPNOTSUPP if PM_SLEEP is not 
selected?

> 
> 	  Andrew

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH net-next 0/3] sh_eth: PM-related cleanups

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  4 Sep 2025 17:18:55 +0200 you wrote:
> Hi all,
> 
> This patch series contains various cleanups related to power management
> for the Renesas SH Ethernet driver, as used on Renesas SH, ARM32, and
> ARM64 platforms.
> 
> This has been tested on various SoCs (R-Mobile A1, RZ/A1H, RZ/A2M, R-Car
> H1, R-Car M2-W).
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] sh_eth: Remove dummy Runtime PM callbacks
    https://git.kernel.org/netdev/net-next/c/86e6257192c8
  - [net-next,2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
    https://git.kernel.org/netdev/net-next/c/3406114a303e
  - [net-next,3/3] sh_eth: Use async pm_runtime_put()
    https://git.kernel.org/netdev/net-next/c/ae52c3e846e1

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Geert Uytterhoeven]

Hi Niklas,

On Fri, 5 Sept 2025 at 20:41, Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
> On 2025-09-05 13:57:05 +0200, Andrew Lunn wrote:
> > > You cannot enter system sleep without CONFIG_PM_SLEEP, so enabling
> > > WoL would be pointless.
> >
> > Yet get_wol will return WoL can be used, and set_wol will allow you to
> > configure it. It seems like EOPNOTSUPP would be better.
>
> Out of curiosity. Are you suggesting a compile time check/construct for
> CONFIG_PM_SLEEP be added in the driver itself, or in ethtool_set_wol()
> and ethtool_get_wol() in net/ethtool/ioctl.c to complement the
>
>     if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol)
>         return -EOPNOTSUPP;
>
> checks already there? To always return EOPNOTSUPP if PM_SLEEP is not
> selected?

Iff we want to go that route, I'd vote for handling it in common code.
Still, there is no guarantee that WoL will actually work, as on
some systems it may depend on the firmware, too.  E.g. on ARM
systems with PSCI, the SoC may be powered down during s2ram, so
there is no guarantee that any of the wake-up sources shown in
/sys/kernel/debug/wakeup_sources can actually wake up the system.
I tried having a mechanism to describe that in DT, but it was rejected.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Andrew Lunn]

On Mon, Sep 08, 2025 at 04:36:29PM +0200, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Fri, 5 Sept 2025 at 20:41, Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> > On 2025-09-05 13:57:05 +0200, Andrew Lunn wrote:
> > > > You cannot enter system sleep without CONFIG_PM_SLEEP, so enabling
> > > > WoL would be pointless.
> > >
> > > Yet get_wol will return WoL can be used, and set_wol will allow you to
> > > configure it. It seems like EOPNOTSUPP would be better.
> >
> > Out of curiosity. Are you suggesting a compile time check/construct for
> > CONFIG_PM_SLEEP be added in the driver itself, or in ethtool_set_wol()
> > and ethtool_get_wol() in net/ethtool/ioctl.c to complement the
> >
> >     if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol)
> >         return -EOPNOTSUPP;
> >
> > checks already there? To always return EOPNOTSUPP if PM_SLEEP is not
> > selected?
> 
> Iff we want to go that route, I'd vote for handling it in common code.
> Still, there is no guarantee that WoL will actually work, as on
> some systems it may depend on the firmware, too.  E.g. on ARM
> systems with PSCI, the SoC may be powered down during s2ram, so
> there is no guarantee that any of the wake-up sources shown in
> /sys/kernel/debug/wakeup_sources can actually wake up the system.
> I tried having a mechanism to describe that in DT, but it was rejected.

WoL is a bit of a mess. Russell has done a little cleanup for when the
PHY does WoL, not the MAC.

I don't think we can check for PM_SLEEP in the core. There are some
PHYs which are powered by the standby voltage, so not effected by
power off. And their WoL output is connected directly to the PMIC. So
they have the ability to wake the system from off, not just suspend.

For the hardware you are dealing with, the MAC does WoL. You know it
cannot work without PM_SLEEP, so i think it needs to be the MAC which
returns EOPNOTSUP.

	Andrew

Re: [PATCH net-next 2/3] sh_eth: Convert to DEFINE_SIMPLE_DEV_PM_OPS()

[Geert Uytterhoeven]

On Mon, 8 Sept 2025 at 16:36, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Fri, 5 Sept 2025 at 20:41, Niklas Söderlund
> <niklas.soderlund@ragnatech.se> wrote:
> > On 2025-09-05 13:57:05 +0200, Andrew Lunn wrote:
> > > > You cannot enter system sleep without CONFIG_PM_SLEEP, so enabling
> > > > WoL would be pointless.
> > >
> > > Yet get_wol will return WoL can be used, and set_wol will allow you to
> > > configure it. It seems like EOPNOTSUPP would be better.
> >
> > Out of curiosity. Are you suggesting a compile time check/construct for
> > CONFIG_PM_SLEEP be added in the driver itself, or in ethtool_set_wol()
> > and ethtool_get_wol() in net/ethtool/ioctl.c to complement the
> >
> >     if (!dev->ethtool_ops->get_wol || !dev->ethtool_ops->set_wol)
> >         return -EOPNOTSUPP;
> >
> > checks already there? To always return EOPNOTSUPP if PM_SLEEP is not
> > selected?
>
> Iff we want to go that route, I'd vote for handling it in common code.
> Still, there is no guarantee that WoL will actually work, as on
> some systems it may depend on the firmware, too.  E.g. on ARM
> systems with PSCI, the SoC may be powered down during s2ram, so
> there is no guarantee that any of the wake-up sources shown in
> /sys/kernel/debug/wakeup_sources can actually wake up the system.
> I tried having a mechanism to describe that in DT, but it was rejected.

(oops, forgot to press "send" in an old draft)

Discovering commit af8dbf9c6aa8972f ("schemas: wakeup-source:
Possibility for system states") in dt-schema.git, there seems to
be hope!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds