2021-11-11 13:57:16

by Li, Meng

[permalink] [raw]
Subject: [PATCH] net: stmmac: socfpga: add runtime suspend/resume callback for stratix10 platform

From: Meng Li <[email protected]>

According to upstream commit 5ec55823438e("net: stmmac:
add clocks management for gmac driver "), it improve clocks
management for stmmac driver. So, it is necessary to implement
the runtime callback in dwmac-socfpga driver because it doesn’t
use the common stmmac_pltfr_pm_ops instance. Otherwise, clocks
are not disabled when system enters suspend status.

Signed-off-by: Meng Li <[email protected]>
---
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 24 +++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 85208128f135..93abde467de4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -485,8 +485,28 @@ static int socfpga_dwmac_resume(struct device *dev)
}
#endif /* CONFIG_PM_SLEEP */

-static SIMPLE_DEV_PM_OPS(socfpga_dwmac_pm_ops, stmmac_suspend,
- socfpga_dwmac_resume);
+static int __maybe_unused socfpga_dwmac_runtime_suspend(struct device *dev)
+{
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+
+ stmmac_bus_clks_config(priv, false);
+
+ return 0;
+}
+
+static int __maybe_unused socfpga_dwmac_runtime_resume(struct device *dev)
+{
+ struct net_device *ndev = dev_get_drvdata(dev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+
+ return stmmac_bus_clks_config(priv, true);
+}
+
+const struct dev_pm_ops socfpga_dwmac_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(stmmac_suspend, socfpga_dwmac_resume)
+ SET_RUNTIME_PM_OPS(socfpga_dwmac_runtime_suspend, socfpga_dwmac_runtime_resume, NULL)
+};

static const struct socfpga_dwmac_ops socfpga_gen5_ops = {
.set_phy_mode = socfpga_gen5_set_phy_mode,
--
2.17.1



2021-11-11 14:01:58

by Denis Kirjanov

[permalink] [raw]
Subject: Re: [PATCH] net: stmmac: socfpga: add runtime suspend/resume callback for stratix10 platform



11/11/21 4:56 PM, Meng Li пишет:
> From: Meng Li <[email protected]>
>
> According to upstream commit 5ec55823438e("net: stmmac:
> add clocks management for gmac driver "), it improve clocks
> management for stmmac driver. So, it is necessary to implement
> the runtime callback in dwmac-socfpga driver because it doesn’t
> use the common stmmac_pltfr_pm_ops instance. Otherwise, clocks
> are not disabled when system enters suspend status.

Please add Fixes tag
>
> Signed-off-by: Meng Li <[email protected]>
> ---
> .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 24 +++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> index 85208128f135..93abde467de4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> @@ -485,8 +485,28 @@ static int socfpga_dwmac_resume(struct device *dev)
> }
> #endif /* CONFIG_PM_SLEEP */
>
> -static SIMPLE_DEV_PM_OPS(socfpga_dwmac_pm_ops, stmmac_suspend,
> - socfpga_dwmac_resume);
> +static int __maybe_unused socfpga_dwmac_runtime_suspend(struct device *dev)
> +{
> + struct net_device *ndev = dev_get_drvdata(dev);
> + struct stmmac_priv *priv = netdev_priv(ndev);
> +
> + stmmac_bus_clks_config(priv, false);
check the return value?
> +
> + return 0;
> +}
> +
> +static int __maybe_unused socfpga_dwmac_runtime_resume(struct device *dev)
> +{
> + struct net_device *ndev = dev_get_drvdata(dev);
> + struct stmmac_priv *priv = netdev_priv(ndev);
> +
> + return stmmac_bus_clks_config(priv, true);
> +}
> +
> +const struct dev_pm_ops socfpga_dwmac_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(stmmac_suspend, socfpga_dwmac_resume)
> + SET_RUNTIME_PM_OPS(socfpga_dwmac_runtime_suspend, socfpga_dwmac_runtime_resume, NULL)
> +};
>
> static const struct socfpga_dwmac_ops socfpga_gen5_ops = {
> .set_phy_mode = socfpga_gen5_set_phy_mode,
>

2021-11-11 14:09:32

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH] net: stmmac: socfpga: add runtime suspend/resume callback for stratix10 platform

On Thu, 11 Nov 2021 21:56:30 +0800 Meng Li wrote:
> +const struct dev_pm_ops socfpga_dwmac_pm_ops = {

static

2021-11-11 14:17:08

by Li, Meng

[permalink] [raw]
Subject: RE: [PATCH] net: stmmac: socfpga: add runtime suspend/resume callback for stratix10 platform



> -----Original Message-----
> From: Denis Kirjanov <[email protected]>
> Sent: Thursday, November 11, 2021 10:02 PM
> To: Li, Meng <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH] net: stmmac: socfpga: add runtime suspend/resume
> callback for stratix10 platform
>
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> 11/11/21 4:56 PM, Meng Li пишет:
> > From: Meng Li <[email protected]>
> >
> > According to upstream commit 5ec55823438e("net: stmmac:
> > add clocks management for gmac driver "), it improve clocks management
> > for stmmac driver. So, it is necessary to implement the runtime
> > callback in dwmac-socfpga driver because it doesn’t use the common
> > stmmac_pltfr_pm_ops instance. Otherwise, clocks are not disabled when
> > system enters suspend status.
>
> Please add Fixes tag

Thanks for suggest.
Yes! this patch is used to fix an clock operation issue in dwmac-socfpga driver,
But I am not sure which Fixing commit ID I should use.
Because 5ec55823438e breaks the original clock operation of dwmac-socfpga driver, but this commit 5ec55823438e is not a bug.
Moreover, if without 5ec55823438e dwmac-socfpga driver also works fine.

How about your suggest?

Thanks,
Limeng

> >
> > Signed-off-by: Meng Li <[email protected]>
> > ---
> > .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 24
> +++++++++++++++++--
> > 1 file changed, 22 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > index 85208128f135..93abde467de4 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> > @@ -485,8 +485,28 @@ static int socfpga_dwmac_resume(struct device
> *dev)
> > }
> > #endif /* CONFIG_PM_SLEEP */
> >
> > -static SIMPLE_DEV_PM_OPS(socfpga_dwmac_pm_ops, stmmac_suspend,
> > - socfpga_dwmac_resume);
> > +static int __maybe_unused socfpga_dwmac_runtime_suspend(struct
> device
> > +*dev) {
> > + struct net_device *ndev = dev_get_drvdata(dev);
> > + struct stmmac_priv *priv = netdev_priv(ndev);
> > +
> > + stmmac_bus_clks_config(priv, false);
> check the return value?
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused socfpga_dwmac_runtime_resume(struct
> device
> > +*dev) {
> > + struct net_device *ndev = dev_get_drvdata(dev);
> > + struct stmmac_priv *priv = netdev_priv(ndev);
> > +
> > + return stmmac_bus_clks_config(priv, true); }
> > +
> > +const struct dev_pm_ops socfpga_dwmac_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(stmmac_suspend,
> socfpga_dwmac_resume)
> > + SET_RUNTIME_PM_OPS(socfpga_dwmac_runtime_suspend,
> > +socfpga_dwmac_runtime_resume, NULL) };
> >
> > static const struct socfpga_dwmac_ops socfpga_gen5_ops = {
> > .set_phy_mode = socfpga_gen5_set_phy_mode,
> >

2021-11-11 15:02:34

by Denis Kirjanov

[permalink] [raw]
Subject: Re: [PATCH] net: stmmac: socfpga: add runtime suspend/resume callback for stratix10 platform



11/11/21 5:16 PM, Li, Meng пишет:
>
>
>> -----Original Message-----
>> From: Denis Kirjanov <[email protected]>
>> Sent: Thursday, November 11, 2021 10:02 PM
>> To: Li, Meng <[email protected]>; [email protected];
>> [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected]
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected]
>> Subject: Re: [PATCH] net: stmmac: socfpga: add runtime suspend/resume
>> callback for stratix10 platform
>>
>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>
>> 11/11/21 4:56 PM, Meng Li пишет:
>>> From: Meng Li <[email protected]>
>>>
>>> According to upstream commit 5ec55823438e("net: stmmac:
>>> add clocks management for gmac driver "), it improve clocks management
>>> for stmmac driver. So, it is necessary to implement the runtime
>>> callback in dwmac-socfpga driver because it doesn’t use the common
>>> stmmac_pltfr_pm_ops instance. Otherwise, clocks are not disabled when
>>> system enters suspend status.
>>
>> Please add Fixes tag
>
> Thanks for suggest.
> Yes! this patch is used to fix an clock operation issue in dwmac-socfpga driver,
> But I am not sure which Fixing commit ID I should use.
> Because 5ec55823438e breaks the original clock operation of dwmac-socfpga driver, but this commit 5ec55823438e is not a bug.
> Moreover, if without 5ec55823438e dwmac-socfpga driver also works fine.
Yes I see. I also checked the commit 5ec55823438e and it logically
relates to your change
>
> How about your suggest?
>
> Thanks,
> Limeng
>
>>>
>>> Signed-off-by: Meng Li <[email protected]>
>>> ---
>>> .../ethernet/stmicro/stmmac/dwmac-socfpga.c | 24
>> +++++++++++++++++--
>>> 1 file changed, 22 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>> index 85208128f135..93abde467de4 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
>>> @@ -485,8 +485,28 @@ static int socfpga_dwmac_resume(struct device
>> *dev)
>>> }
>>> #endif /* CONFIG_PM_SLEEP */
>>>
>>> -static SIMPLE_DEV_PM_OPS(socfpga_dwmac_pm_ops, stmmac_suspend,
>>> - socfpga_dwmac_resume);
>>> +static int __maybe_unused socfpga_dwmac_runtime_suspend(struct
>> device
>>> +*dev) {
>>> + struct net_device *ndev = dev_get_drvdata(dev);
>>> + struct stmmac_priv *priv = netdev_priv(ndev);
>>> +
>>> + stmmac_bus_clks_config(priv, false);
>> check the return value?
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __maybe_unused socfpga_dwmac_runtime_resume(struct
>> device
>>> +*dev) {
>>> + struct net_device *ndev = dev_get_drvdata(dev);
>>> + struct stmmac_priv *priv = netdev_priv(ndev);
>>> +
>>> + return stmmac_bus_clks_config(priv, true); }
>>> +
>>> +const struct dev_pm_ops socfpga_dwmac_pm_ops = {
>>> + SET_SYSTEM_SLEEP_PM_OPS(stmmac_suspend,
>> socfpga_dwmac_resume)
>>> + SET_RUNTIME_PM_OPS(socfpga_dwmac_runtime_suspend,
>>> +socfpga_dwmac_runtime_resume, NULL) };
>>>
>>> static const struct socfpga_dwmac_ops socfpga_gen5_ops = {
>>> .set_phy_mode = socfpga_gen5_set_phy_mode,
>>>