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

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

[Meng Li]

From: Meng Li <meng.li@windriver.com>

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 <Meng.Li@windriver.com>
---
 .../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

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

[Denis Kirjanov]

11/11/21 4:56 PM, Meng Li пишет:
> From: Meng Li <meng.li@windriver.com>
> 
> 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 <Meng.Li@windriver.com>
> ---
>   .../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,
> 

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

[Jakub Kicinski]

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

static

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

[Li, Meng]

> -----Original Message-----
> From: Denis Kirjanov <dkirjanov@suse.de>
> Sent: Thursday, November 11, 2021 10:02 PM
> To: Li, Meng <Meng.Li@windriver.com>; peppe.cavallaro@st.com;
> alexandre.torgue@foss.st.com; joabreu@synopsys.com;
> davem@davemloft.net; kuba@kernel.org; mcoquelin.stm32@gmail.com
> Cc: netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> 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 <meng.li@windriver.com>
> >
> > 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 <Meng.Li@windriver.com>
> > ---
> >   .../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,
> >

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

[Denis Kirjanov]

11/11/21 5:16 PM, Li, Meng пишет:
> 
> 
>> -----Original Message-----
>> From: Denis Kirjanov <dkirjanov@suse.de>
>> Sent: Thursday, November 11, 2021 10:02 PM
>> To: Li, Meng <Meng.Li@windriver.com>; peppe.cavallaro@st.com;
>> alexandre.torgue@foss.st.com; joabreu@synopsys.com;
>> davem@davemloft.net; kuba@kernel.org; mcoquelin.stm32@gmail.com
>> Cc: netdev@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com;
>> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> 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 <meng.li@windriver.com>
>>>
>>> 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 <Meng.Li@windriver.com>
>>> ---
>>>    .../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,
>>>