| Message ID | 20231012121618.267315-1-yoshihiro.shimoda.uh@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [net-next] rswitch: Add PM ops | expand |
Hi Shimoda-san, On Thu, Oct 12, 2023 at 2:16 PM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > Add PM ops for Suspend to Idle. When the system suspended, > the Ethernet Serdes's clock will be stopped. So, this driver needs > to re-initialize the Ethernet Serdes by phy_init() in > renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on(). > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/rswitch.c > +++ b/drivers/net/ethernet/renesas/rswitch.c > @@ -17,6 +17,7 @@ > #include <linux/of_net.h> > #include <linux/phy/phy.h> > #include <linux/platform_device.h> > +#include <linux/pm.h> > #include <linux/pm_runtime.h> > #include <linux/rtnetlink.h> > #include <linux/slab.h> > @@ -1315,6 +1316,7 @@ static int rswitch_phy_device_init(struct rswitch_device *rdev) > if (!phydev) > goto out; > __set_bit(rdev->etha->phy_interface, phydev->host_interfaces); > + phydev->mac_managed_pm = true; > > phydev = of_phy_connect(rdev->ndev, phy, rswitch_adjust_link, 0, > rdev->etha->phy_interface); > @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev) > platform_set_drvdata(pdev, NULL); > } > > +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev) > +{ > + struct rswitch_private *priv = dev_get_drvdata(dev); > + struct net_device *ndev; > + int i; unsigned int (also below) > + > + rswitch_for_each_enabled_port(priv, i) { > + ndev = priv->rdev[i]->ndev; > + if (netif_running(ndev)) { > + netif_device_detach(ndev); > + rswitch_stop(ndev); > + } > + if (priv->rdev[i]->serdes->init_count) > + phy_exit(priv->rdev[i]->serdes); If !init_count, the PHY was not initialized before suspending? ... > + } > + > + return 0; > +} > + > +static int __maybe_unused renesas_eth_sw_resume(struct device *dev) > +{ > + struct rswitch_private *priv = dev_get_drvdata(dev); > + struct net_device *ndev; > + int i; > + > + rswitch_for_each_enabled_port(priv, i) { > + phy_init(priv->rdev[i]->serdes); ... while it is always initialized after resuming? Is that intentional, or should the pre-suspend state be preserved? > + ndev = priv->rdev[i]->ndev; > + if (netif_running(ndev)) { > + rswitch_open(ndev); > + netif_device_attach(ndev); > + } > + } > + > + return 0; > +} > + > +static SIMPLE_DEV_PM_OPS(renesas_eth_sw_pm_ops, renesas_eth_sw_suspend, > + renesas_eth_sw_resume); Please use DEFINE_SIMPLE_DEV_PM_OPS() instead, so you can drop the __maybe_unused tags from the callbacks. > + > static struct platform_driver renesas_eth_sw_driver_platform = { > .probe = renesas_eth_sw_probe, > .remove_new = renesas_eth_sw_remove, > .driver = { > .name = "renesas_eth_sw", > + .pm = &renesas_eth_sw_pm_ops, pm_sleep_ptr(...) > .of_match_table = renesas_eth_sw_of_table, > } > }; Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, October 12, 2023 9:35 PM > > Hi Shimoda-san, > > On Thu, Oct 12, 2023 at 2:16 PM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > Add PM ops for Suspend to Idle. When the system suspended, > > the Ethernet Serdes's clock will be stopped. So, this driver needs > > to re-initialize the Ethernet Serdes by phy_init() in > > renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on(). > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Thanks for your patch! Thank you for your review! > > --- a/drivers/net/ethernet/renesas/rswitch.c > > +++ b/drivers/net/ethernet/renesas/rswitch.c > > @@ -17,6 +17,7 @@ > > #include <linux/of_net.h> > > #include <linux/phy/phy.h> > > #include <linux/platform_device.h> > > +#include <linux/pm.h> > > #include <linux/pm_runtime.h> > > #include <linux/rtnetlink.h> > > #include <linux/slab.h> > > @@ -1315,6 +1316,7 @@ static int rswitch_phy_device_init(struct rswitch_device *rdev) > > if (!phydev) > > goto out; > > __set_bit(rdev->etha->phy_interface, phydev->host_interfaces); > > + phydev->mac_managed_pm = true; > > > > phydev = of_phy_connect(rdev->ndev, phy, rswitch_adjust_link, 0, > > rdev->etha->phy_interface); > > @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev) > > platform_set_drvdata(pdev, NULL); > > } > > > > +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev) > > +{ > > + struct rswitch_private *priv = dev_get_drvdata(dev); > > + struct net_device *ndev; > > + int i; > > unsigned int (also below) I don't know why unsigned int is needed. Other functions use rswitch_for_each_enabled_port{_continue_reverse}() with int. Especially, rswitch_for_each_enabled_port_continue_reverse() has the following code, unsigned int will not work correctly: --- #define rswitch_for_each_enabled_port_continue_reverse(priv, i) \ for (i--; i >= 0; i--) \ if (priv->rdev[i]->disabled) \ continue; \ else --- So, I would like to keep this for consistency with other functions' implementation. But, what do you think? > > + > > + rswitch_for_each_enabled_port(priv, i) { > > + ndev = priv->rdev[i]->ndev; > > + if (netif_running(ndev)) { > > + netif_device_detach(ndev); > > + rswitch_stop(ndev); > > + } > > + if (priv->rdev[i]->serdes->init_count) > > + phy_exit(priv->rdev[i]->serdes); > > If !init_count, the PHY was not initialized before suspending? ... Yes, it was possible if renesas_eth_sw_resume() called phy_init() and it failed. > > + } > > + > > + return 0; > > +} > > + > > +static int __maybe_unused renesas_eth_sw_resume(struct device *dev) > > +{ > > + struct rswitch_private *priv = dev_get_drvdata(dev); > > + struct net_device *ndev; > > + int i; > > + > > + rswitch_for_each_enabled_port(priv, i) { > > + phy_init(priv->rdev[i]->serdes); > > ... while it is always initialized after resuming? Is that intentional, > or should the pre-suspend state be preserved? Yes, that is intentional. After this driver was probed, the phy was always initialized. > > + ndev = priv->rdev[i]->ndev; > > + if (netif_running(ndev)) { > > + rswitch_open(ndev); > > + netif_device_attach(ndev); > > + } > > + } > > + > > + return 0; > > +} > > + > > +static SIMPLE_DEV_PM_OPS(renesas_eth_sw_pm_ops, renesas_eth_sw_suspend, > > + renesas_eth_sw_resume); > > Please use DEFINE_SIMPLE_DEV_PM_OPS() instead, so you can drop the > __maybe_unused tags from the callbacks. I got it. I'll fix them. > > + > > static struct platform_driver renesas_eth_sw_driver_platform = { > > .probe = renesas_eth_sw_probe, > > .remove_new = renesas_eth_sw_remove, > > .driver = { > > .name = "renesas_eth_sw", > > + .pm = &renesas_eth_sw_pm_ops, > > pm_sleep_ptr(...) I'll use it on v2. Best regards, Yoshihiro Shimoda > > .of_match_table = renesas_eth_sw_of_table, > > } > > }; > > 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
Hi Shimoda-san, On Fri, Oct 13, 2023 at 4:10 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Thursday, October 12, 2023 9:35 PM > > On Thu, Oct 12, 2023 at 2:16 PM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > Add PM ops for Suspend to Idle. When the system suspended, > > > the Ethernet Serdes's clock will be stopped. So, this driver needs > > > to re-initialize the Ethernet Serdes by phy_init() in > > > renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on(). > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > Thanks for your patch! > > Thank you for your review! > > > > --- a/drivers/net/ethernet/renesas/rswitch.c > > > +++ b/drivers/net/ethernet/renesas/rswitch.c > > > @@ -17,6 +17,7 @@ > > > #include <linux/of_net.h> > > > #include <linux/phy/phy.h> > > > #include <linux/platform_device.h> > > > +#include <linux/pm.h> > > > #include <linux/pm_runtime.h> > > > #include <linux/rtnetlink.h> > > > #include <linux/slab.h> > > > @@ -1315,6 +1316,7 @@ static int rswitch_phy_device_init(struct rswitch_device *rdev) > > > if (!phydev) > > > goto out; > > > __set_bit(rdev->etha->phy_interface, phydev->host_interfaces); > > > + phydev->mac_managed_pm = true; > > > > > > phydev = of_phy_connect(rdev->ndev, phy, rswitch_adjust_link, 0, > > > rdev->etha->phy_interface); > > > @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev) > > > platform_set_drvdata(pdev, NULL); > > > } > > > > > > +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev) > > > +{ > > > + struct rswitch_private *priv = dev_get_drvdata(dev); > > > + struct net_device *ndev; > > > + int i; > > > > unsigned int (also below) > > I don't know why unsigned int is needed. Other functions use There's this old mantra "prefer unsigned over signed in C", and a valid port array index here is always unsigned. > rswitch_for_each_enabled_port{_continue_reverse}() with int. > Especially, rswitch_for_each_enabled_port_continue_reverse() > has the following code, unsigned int will not work correctly: Oh, there is also a reverse variant, which indeed needs a signed iterator, currently... > --- > #define rswitch_for_each_enabled_port_continue_reverse(priv, i) \ > for (i--; i >= 0; i--) \ I think this can be made to work with an unsigned iterator using for (; i-- > 0; ) > if (priv->rdev[i]->disabled) \ > continue; \ > else > --- > > So, I would like to keep this for consistency with other functions' > implementation. But, what do you think? Consistency is good... Surprising readers (why is this signed?) is bad... It's hard to keep a good balance... BTW, perhaps it would make sense to use the reverse order in suspend? Although it probably doesn't matter, as rswitch_deinit() uses the non-reverse order, too. Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Friday, October 13, 2023 4:29 PM > > Hi Shimoda-san, > > On Fri, Oct 13, 2023 at 4:10 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > From: Geert Uytterhoeven, Sent: Thursday, October 12, 2023 9:35 PM > > > On Thu, Oct 12, 2023 at 2:16 PM Yoshihiro Shimoda > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > Add PM ops for Suspend to Idle. When the system suspended, > > > > the Ethernet Serdes's clock will be stopped. So, this driver needs > > > > to re-initialize the Ethernet Serdes by phy_init() in > > > > renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on(). > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > Thanks for your patch! > > > > Thank you for your review! > > > > > > --- a/drivers/net/ethernet/renesas/rswitch.c > > > > +++ b/drivers/net/ethernet/renesas/rswitch.c > > > > @@ -17,6 +17,7 @@ > > > > #include <linux/of_net.h> > > > > #include <linux/phy/phy.h> > > > > #include <linux/platform_device.h> > > > > +#include <linux/pm.h> > > > > #include <linux/pm_runtime.h> > > > > #include <linux/rtnetlink.h> > > > > #include <linux/slab.h> > > > > @@ -1315,6 +1316,7 @@ static int rswitch_phy_device_init(struct rswitch_device *rdev) > > > > if (!phydev) > > > > goto out; > > > > __set_bit(rdev->etha->phy_interface, phydev->host_interfaces); > > > > + phydev->mac_managed_pm = true; > > > > > > > > phydev = of_phy_connect(rdev->ndev, phy, rswitch_adjust_link, 0, > > > > rdev->etha->phy_interface); > > > > @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev) > > > > platform_set_drvdata(pdev, NULL); > > > > } > > > > > > > > +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev) > > > > +{ > > > > + struct rswitch_private *priv = dev_get_drvdata(dev); > > > > + struct net_device *ndev; > > > > + int i; > > > > > > unsigned int (also below) > > > > I don't know why unsigned int is needed. Other functions use > > There's this old mantra "prefer unsigned over signed in C", > and a valid port array index here is always unsigned. I understood it. > > rswitch_for_each_enabled_port{_continue_reverse}() with int. > > Especially, rswitch_for_each_enabled_port_continue_reverse() > > has the following code, unsigned int will not work correctly: > > Oh, there is also a reverse variant, which indeed needs a signed > iterator, currently... > > > --- > > #define rswitch_for_each_enabled_port_continue_reverse(priv, i) \ > > for (i--; i >= 0; i--) \ > > I think this can be made to work with an unsigned iterator using > > for (; i-- > 0; ) I think that this loop cannot access index 0 :) > > if (priv->rdev[i]->disabled) \ > > continue; \ > > else > > --- > > > > So, I would like to keep this for consistency with other functions' > > implementation. But, what do you think? > > Consistency is good... > Surprising readers (why is this signed?) is bad... > It's hard to keep a good balance... > > BTW, perhaps it would make sense to use the reverse order in suspend? > Although it probably doesn't matter, as rswitch_deinit() uses the > non-reverse order, too. Thank you for your suggestion. I also think that we can use the non-reverse order. So, 1) Change "_reverse()" macro to others somehow. 2) Change the type of i for "for_each" to unsigned int. 3) Add PM ops. Perhaps, can/should we merge the 1) and 2) to one patch? Best regards, Yoshihiro Shimoda > 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
Hi Shimoda-san, On Fri, Oct 13, 2023 at 10:55 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote: > > From: Geert Uytterhoeven, Sent: Friday, October 13, 2023 4:29 PM > > On Fri, Oct 13, 2023 at 4:10 AM Yoshihiro Shimoda > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > From: Geert Uytterhoeven, Sent: Thursday, October 12, 2023 9:35 PM > > > > On Thu, Oct 12, 2023 at 2:16 PM Yoshihiro Shimoda > > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > Add PM ops for Suspend to Idle. When the system suspended, > > > > > the Ethernet Serdes's clock will be stopped. So, this driver needs > > > > > to re-initialize the Ethernet Serdes by phy_init() in > > > > > renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on(). > > > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > --- a/drivers/net/ethernet/renesas/rswitch.c > > > > > +++ b/drivers/net/ethernet/renesas/rswitch.c > > > > > @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev) > > > > > platform_set_drvdata(pdev, NULL); > > > > > } > > > > > > > > > > +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev) > > > > > +{ > > > > > + struct rswitch_private *priv = dev_get_drvdata(dev); > > > > > + struct net_device *ndev; > > > > > + int i; > > > > > > > > unsigned int (also below) > > > > > > I don't know why unsigned int is needed. Other functions use > > > > There's this old mantra "prefer unsigned over signed in C", > > and a valid port array index here is always unsigned. > > I understood it. > > > > rswitch_for_each_enabled_port{_continue_reverse}() with int. > > > Especially, rswitch_for_each_enabled_port_continue_reverse() > > > has the following code, unsigned int will not work correctly: > > > > Oh, there is also a reverse variant, which indeed needs a signed > > iterator, currently... > > > > > --- > > > #define rswitch_for_each_enabled_port_continue_reverse(priv, i) \ > > > for (i--; i >= 0; i--) \ > > > > I think this can be made to work with an unsigned iterator using > > > > for (; i-- > 0; ) > > I think that this loop cannot access index 0 :) rswitch_for_each_enabled_port_continue_reverse() is meant to be used in the error path, to tear down all ports that were initialized succesfully? So i is always the index of the first port that failed to initialize, i.e. i > 0 on entry. See "git grep -Ww rswitch_for_each_enabled_port_continue_reverse". Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Friday, October 13, 2023 6:19 PM > > Hi Shimoda-san, > > On Fri, Oct 13, 2023 at 10:55 AM Yoshihiro Shimoda > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > From: Geert Uytterhoeven, Sent: Friday, October 13, 2023 4:29 PM > > > On Fri, Oct 13, 2023 at 4:10 AM Yoshihiro Shimoda > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > From: Geert Uytterhoeven, Sent: Thursday, October 12, 2023 9:35 PM > > > > > On Thu, Oct 12, 2023 at 2:16 PM Yoshihiro Shimoda > > > > > <yoshihiro.shimoda.uh@renesas.com> wrote: > > > > > > Add PM ops for Suspend to Idle. When the system suspended, > > > > > > the Ethernet Serdes's clock will be stopped. So, this driver needs > > > > > > to re-initialize the Ethernet Serdes by phy_init() in > > > > > > renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on(). > > > > > > > > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > > --- a/drivers/net/ethernet/renesas/rswitch.c > > > > > > +++ b/drivers/net/ethernet/renesas/rswitch.c > > > > > > @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev) > > > > > > platform_set_drvdata(pdev, NULL); > > > > > > } > > > > > > > > > > > > +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev) > > > > > > +{ > > > > > > + struct rswitch_private *priv = dev_get_drvdata(dev); > > > > > > + struct net_device *ndev; > > > > > > + int i; > > > > > > > > > > unsigned int (also below) > > > > > > > > I don't know why unsigned int is needed. Other functions use > > > > > > There's this old mantra "prefer unsigned over signed in C", > > > and a valid port array index here is always unsigned. > > > > I understood it. > > > > > > rswitch_for_each_enabled_port{_continue_reverse}() with int. > > > > Especially, rswitch_for_each_enabled_port_continue_reverse() > > > > has the following code, unsigned int will not work correctly: > > > > > > Oh, there is also a reverse variant, which indeed needs a signed > > > iterator, currently... > > > > > > > --- > > > > #define rswitch_for_each_enabled_port_continue_reverse(priv, i) \ > > > > for (i--; i >= 0; i--) \ > > > > > > I think this can be made to work with an unsigned iterator using > > > > > > for (; i-- > 0; ) > > > > I think that this loop cannot access index 0 :) > > rswitch_for_each_enabled_port_continue_reverse() is meant to be > used in the error path, to tear down all ports that were initialized > succesfully? So i is always the index of the first port that failed to > initialize, i.e. i > 0 on entry. > > See "git grep -Ww rswitch_for_each_enabled_port_continue_reverse". Thank you for your comment. And I'm sorry, I completely misunderstood about "for (; i-- > 0; )" behavior... I should have tested this before I send such a comment... Best regards, Yoshihiro Shimoda > 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
diff --git a/drivers/net/ethernet/renesas/rswitch.c b/drivers/net/ethernet/renesas/rswitch.c index 0023ff254915..301053db61ac 100644 --- a/drivers/net/ethernet/renesas/rswitch.c +++ b/drivers/net/ethernet/renesas/rswitch.c @@ -17,6 +17,7 @@ #include <linux/of_net.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> +#include <linux/pm.h> #include <linux/pm_runtime.h> #include <linux/rtnetlink.h> #include <linux/slab.h> @@ -1315,6 +1316,7 @@ static int rswitch_phy_device_init(struct rswitch_device *rdev) if (!phydev) goto out; __set_bit(rdev->etha->phy_interface, phydev->host_interfaces); + phydev->mac_managed_pm = true; phydev = of_phy_connect(rdev->ndev, phy, rswitch_adjust_link, 0, rdev->etha->phy_interface); @@ -1991,11 +1993,52 @@ static void renesas_eth_sw_remove(struct platform_device *pdev) platform_set_drvdata(pdev, NULL); } +static int __maybe_unused renesas_eth_sw_suspend(struct device *dev) +{ + struct rswitch_private *priv = dev_get_drvdata(dev); + struct net_device *ndev; + int i; + + rswitch_for_each_enabled_port(priv, i) { + ndev = priv->rdev[i]->ndev; + if (netif_running(ndev)) { + netif_device_detach(ndev); + rswitch_stop(ndev); + } + if (priv->rdev[i]->serdes->init_count) + phy_exit(priv->rdev[i]->serdes); + } + + return 0; +} + +static int __maybe_unused renesas_eth_sw_resume(struct device *dev) +{ + struct rswitch_private *priv = dev_get_drvdata(dev); + struct net_device *ndev; + int i; + + rswitch_for_each_enabled_port(priv, i) { + phy_init(priv->rdev[i]->serdes); + ndev = priv->rdev[i]->ndev; + if (netif_running(ndev)) { + rswitch_open(ndev); + netif_device_attach(ndev); + } + } + + return 0; +} + +static SIMPLE_DEV_PM_OPS(renesas_eth_sw_pm_ops, renesas_eth_sw_suspend, + renesas_eth_sw_resume); + static struct platform_driver renesas_eth_sw_driver_platform = { .probe = renesas_eth_sw_probe, .remove_new = renesas_eth_sw_remove, .driver = { .name = "renesas_eth_sw", + .pm = &renesas_eth_sw_pm_ops, .of_match_table = renesas_eth_sw_of_table, } };
Add PM ops for Suspend to Idle. When the system suspended, the Ethernet Serdes's clock will be stopped. So, this driver needs to re-initialize the Ethernet Serdes by phy_init() in renesas_eth_sw_resume(). Otherwise, timeout happened in phy_power_on(). Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- This patch is based on the latest net-next.git / next branch. After applied this patch with the following patches, the system can enter/exit Suspend to Idle without any error: https://lore.kernel.org/netdev/20230201131454.1928136-1-yoshihiro.shimoda.uh@renesas.com/ https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=510b18cf23b9bd8a982ef7f1fb19f3968a2bf787 https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=053f13f67be6d02781730c9ac71abde6e9140610 drivers/net/ethernet/renesas/rswitch.c | 43 ++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)