2019-11-06 08:03:40

by Chuhong Yuan

[permalink] [raw]
Subject: [PATCH] net: fec: add a check for CONFIG_PM to avoid clock count mis-match

If CONFIG_PM is enabled, runtime pm will work and call runtime_suspend
automatically to disable clks.
Therefore, remove only needs to disable clks when CONFIG_PM is disabled.
Add this check to avoid clock count mis-match caused by double-disable.

This patch depends on patch
("net: fec: add missed clk_disable_unprepare in remove").

Signed-off-by: Chuhong Yuan <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index a9c386b63581..696550f4972f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3645,8 +3645,10 @@ fec_drv_remove(struct platform_device *pdev)
regulator_disable(fep->reg_phy);
pm_runtime_put(&pdev->dev);
pm_runtime_disable(&pdev->dev);
+#ifndef CONFIG_PM
clk_disable_unprepare(fep->clk_ahb);
clk_disable_unprepare(fep->clk_ipg);
+#endif
if (of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
of_node_put(fep->phy_node);
--
2.23.0


2019-11-06 08:15:29

by Andy Duan

[permalink] [raw]
Subject: RE: [EXT] [PATCH] net: fec: add a check for CONFIG_PM to avoid clock count mis-match

From: Chuhong Yuan <[email protected]> Sent: Wednesday, November 6, 2019 4:01 PM
> If CONFIG_PM is enabled, runtime pm will work and call runtime_suspend
> automatically to disable clks.
> Therefore, remove only needs to disable clks when CONFIG_PM is disabled.
> Add this check to avoid clock count mis-match caused by double-disable.
>
> This patch depends on patch
> ("net: fec: add missed clk_disable_unprepare in remove").
>
Please add Fixes tag here.

Andy
> Signed-off-by: Chuhong Yuan <[email protected]>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c
> b/drivers/net/ethernet/freescale/fec_main.c
> index a9c386b63581..696550f4972f 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3645,8 +3645,10 @@ fec_drv_remove(struct platform_device *pdev)
> regulator_disable(fep->reg_phy);
> pm_runtime_put(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> +#ifndef CONFIG_PM
> clk_disable_unprepare(fep->clk_ahb);
> clk_disable_unprepare(fep->clk_ipg);
> +#endif
> if (of_phy_is_fixed_link(np))
> of_phy_deregister_fixed_link(np);
> of_node_put(fep->phy_node);
> --
> 2.23.0

2019-11-06 08:31:22

by Chuhong Yuan

[permalink] [raw]
Subject: Re: [EXT] [PATCH] net: fec: add a check for CONFIG_PM to avoid clock count mis-match

On Wed, Nov 6, 2019 at 4:13 PM Andy Duan <[email protected]> wrote:
>
> From: Chuhong Yuan <[email protected]> Sent: Wednesday, November 6, 2019 4:01 PM
> > If CONFIG_PM is enabled, runtime pm will work and call runtime_suspend
> > automatically to disable clks.
> > Therefore, remove only needs to disable clks when CONFIG_PM is disabled.
> > Add this check to avoid clock count mis-match caused by double-disable.
> >
> > This patch depends on patch
> > ("net: fec: add missed clk_disable_unprepare in remove").
> >
> Please add Fixes tag here.
>

The previous patch has not been merged to linux, so I do not know
which commit ID
should be used.

> Andy
> > Signed-off-by: Chuhong Yuan <[email protected]>
> > ---
> > drivers/net/ethernet/freescale/fec_main.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index a9c386b63581..696550f4972f 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -3645,8 +3645,10 @@ fec_drv_remove(struct platform_device *pdev)
> > regulator_disable(fep->reg_phy);
> > pm_runtime_put(&pdev->dev);
> > pm_runtime_disable(&pdev->dev);
> > +#ifndef CONFIG_PM
> > clk_disable_unprepare(fep->clk_ahb);
> > clk_disable_unprepare(fep->clk_ipg);
> > +#endif
> > if (of_phy_is_fixed_link(np))
> > of_phy_deregister_fixed_link(np);
> > of_node_put(fep->phy_node);
> > --
> > 2.23.0
>

2019-11-06 10:21:08

by Andy Duan

[permalink] [raw]
Subject: RE: [EXT] [PATCH] net: fec: add a check for CONFIG_PM to avoid clock count mis-match

From: Chuhong Yuan <[email protected]> Sent: Wednesday, November 6, 2019 4:29 PM
> On Wed, Nov 6, 2019 at 4:13 PM Andy Duan <[email protected]> wrote:
> >
> > From: Chuhong Yuan <[email protected]> Sent: Wednesday, November
> 6,
> > 2019 4:01 PM
> > > If CONFIG_PM is enabled, runtime pm will work and call
> > > runtime_suspend automatically to disable clks.
> > > Therefore, remove only needs to disable clks when CONFIG_PM is
> disabled.
> > > Add this check to avoid clock count mis-match caused by double-disable.
> > >
> > > This patch depends on patch
> > > ("net: fec: add missed clk_disable_unprepare in remove").
> > >
> > Please add Fixes tag here.
> >
>
> The previous patch has not been merged to linux, so I do not know which
> commit ID should be used.

It should be merged into net-next tree.

Andy
>
> > Andy
> > > Signed-off-by: Chuhong Yuan <[email protected]>
> > > ---
> > > drivers/net/ethernet/freescale/fec_main.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > > b/drivers/net/ethernet/freescale/fec_main.c
> > > index a9c386b63581..696550f4972f 100644
> > > --- a/drivers/net/ethernet/freescale/fec_main.c
> > > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > > @@ -3645,8 +3645,10 @@ fec_drv_remove(struct platform_device
> *pdev)
> > > regulator_disable(fep->reg_phy);
> > > pm_runtime_put(&pdev->dev);
> > > pm_runtime_disable(&pdev->dev);
> > > +#ifndef CONFIG_PM
> > > clk_disable_unprepare(fep->clk_ahb);
> > > clk_disable_unprepare(fep->clk_ipg);
> > > +#endif
> > > if (of_phy_is_fixed_link(np))
> > > of_phy_deregister_fixed_link(np);
> > > of_node_put(fep->phy_node);
> > > --
> > > 2.23.0
> >

2019-11-07 01:20:36

by Chuhong Yuan

[permalink] [raw]
Subject: Re: [EXT] [PATCH] net: fec: add a check for CONFIG_PM to avoid clock count mis-match

On Wed, Nov 6, 2019 at 6:17 PM Andy Duan <[email protected]> wrote:
>
> From: Chuhong Yuan <[email protected]> Sent: Wednesday, November 6, 2019 4:29 PM
> > On Wed, Nov 6, 2019 at 4:13 PM Andy Duan <[email protected]> wrote:
> > >
> > > From: Chuhong Yuan <[email protected]> Sent: Wednesday, November
> > 6,
> > > 2019 4:01 PM
> > > > If CONFIG_PM is enabled, runtime pm will work and call
> > > > runtime_suspend automatically to disable clks.
> > > > Therefore, remove only needs to disable clks when CONFIG_PM is
> > disabled.
> > > > Add this check to avoid clock count mis-match caused by double-disable.
> > > >
> > > > This patch depends on patch
> > > > ("net: fec: add missed clk_disable_unprepare in remove").
> > > >
> > > Please add Fixes tag here.
> > >
> >
> > The previous patch has not been merged to linux, so I do not know which
> > commit ID should be used.
>
> It should be merged into net-next tree.
>

I have searched in net-next but did not find it.

> Andy
> >
> > > Andy
> > > > Signed-off-by: Chuhong Yuan <[email protected]>
> > > > ---
> > > > drivers/net/ethernet/freescale/fec_main.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > > > b/drivers/net/ethernet/freescale/fec_main.c
> > > > index a9c386b63581..696550f4972f 100644
> > > > --- a/drivers/net/ethernet/freescale/fec_main.c
> > > > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > > > @@ -3645,8 +3645,10 @@ fec_drv_remove(struct platform_device
> > *pdev)
> > > > regulator_disable(fep->reg_phy);
> > > > pm_runtime_put(&pdev->dev);
> > > > pm_runtime_disable(&pdev->dev);
> > > > +#ifndef CONFIG_PM
> > > > clk_disable_unprepare(fep->clk_ahb);
> > > > clk_disable_unprepare(fep->clk_ipg);
> > > > +#endif
> > > > if (of_phy_is_fixed_link(np))
> > > > of_phy_deregister_fixed_link(np);
> > > > of_node_put(fep->phy_node);
> > > > --
> > > > 2.23.0
> > >

2019-11-07 01:48:13

by Andy Duan

[permalink] [raw]
Subject: RE: [EXT] [PATCH] net: fec: add a check for CONFIG_PM to avoid clock count mis-match

From: Chuhong Yuan <[email protected]> Sent: Thursday, November 7, 2019 9:19 AM
> On Wed, Nov 6, 2019 at 6:17 PM Andy Duan <[email protected]> wrote:
> >
> > From: Chuhong Yuan <[email protected]> Sent: Wednesday, November
> 6,
> > 2019 4:29 PM
> > > On Wed, Nov 6, 2019 at 4:13 PM Andy Duan <[email protected]>
> wrote:
> > > >
> > > > From: Chuhong Yuan <[email protected]> Sent: Wednesday,
> > > > November
> > > 6,
> > > > 2019 4:01 PM
> > > > > If CONFIG_PM is enabled, runtime pm will work and call
> > > > > runtime_suspend automatically to disable clks.
> > > > > Therefore, remove only needs to disable clks when CONFIG_PM is
> > > disabled.
> > > > > Add this check to avoid clock count mis-match caused by
> double-disable.
> > > > >
> > > > > This patch depends on patch
> > > > > ("net: fec: add missed clk_disable_unprepare in remove").
> > > > >
> > > > Please add Fixes tag here.
> > > >
> > >
> > > The previous patch has not been merged to linux, so I do not know
> > > which commit ID should be used.
> >
> > It should be merged into net-next tree.
> >
>
> I have searched in net-next but did not find it.

David, please give the comment. Thanks.

Regards,
Andy
>
> > Andy
> > >
> > > > Andy
> > > > > Signed-off-by: Chuhong Yuan <[email protected]>
> > > > > ---
> > > > > drivers/net/ethernet/freescale/fec_main.c | 2 ++
> > > > > 1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > > > > b/drivers/net/ethernet/freescale/fec_main.c
> > > > > index a9c386b63581..696550f4972f 100644
> > > > > --- a/drivers/net/ethernet/freescale/fec_main.c
> > > > > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > > > > @@ -3645,8 +3645,10 @@ fec_drv_remove(struct platform_device
> > > *pdev)
> > > > > regulator_disable(fep->reg_phy);
> > > > > pm_runtime_put(&pdev->dev);
> > > > > pm_runtime_disable(&pdev->dev);
> > > > > +#ifndef CONFIG_PM
> > > > > clk_disable_unprepare(fep->clk_ahb);
> > > > > clk_disable_unprepare(fep->clk_ipg);
> > > > > +#endif
> > > > > if (of_phy_is_fixed_link(np))
> > > > > of_phy_deregister_fixed_link(np);
> > > > > of_node_put(fep->phy_node);
> > > > > --
> > > > > 2.23.0
> > > >

2019-11-12 09:16:53

by Simon Horman

[permalink] [raw]
Subject: Re: [EXT] [PATCH] net: fec: add a check for CONFIG_PM to avoid clock count mis-match

On Thu, Nov 07, 2019 at 01:44:11AM +0000, Andy Duan wrote:
> From: Chuhong Yuan <[email protected]> Sent: Thursday, November 7, 2019 9:19 AM
> > On Wed, Nov 6, 2019 at 6:17 PM Andy Duan <[email protected]> wrote:
> > >
> > > From: Chuhong Yuan <[email protected]> Sent: Wednesday, November
> > 6,
> > > 2019 4:29 PM
> > > > On Wed, Nov 6, 2019 at 4:13 PM Andy Duan <[email protected]>
> > wrote:
> > > > >
> > > > > From: Chuhong Yuan <[email protected]> Sent: Wednesday,
> > > > > November
> > > > 6,
> > > > > 2019 4:01 PM
> > > > > > If CONFIG_PM is enabled, runtime pm will work and call
> > > > > > runtime_suspend automatically to disable clks.
> > > > > > Therefore, remove only needs to disable clks when CONFIG_PM is
> > > > disabled.
> > > > > > Add this check to avoid clock count mis-match caused by
> > double-disable.
> > > > > >
> > > > > > This patch depends on patch
> > > > > > ("net: fec: add missed clk_disable_unprepare in remove").
> > > > > >
> > > > > Please add Fixes tag here.
> > > > >
> > > >
> > > > The previous patch has not been merged to linux, so I do not know
> > > > which commit ID should be used.
> > >
> > > It should be merged into net-next tree.
> > >
> >
> > I have searched in net-next but did not find it.

Commit ids are stable, so if there is an id in Linus's tree
it will be same in net-next (when the patch appears there).

So you want:

Fixes: c43eab3eddb4 ("net: fec: add missed clk_disable_unprepare in remove")

Also, it is unclear from the patch subject if this patch is targeted at
'net' or 'net-next'. But as c43eab3eddb4 is in Linus's tree I think
it should be for 'net'. So the correct patch subject would be:

[PATCH net] net: fec: add a check for CONFIG_PM to avoid clock

> David, please give the comment. Thanks.
>
> Regards,
> Andy
> >
> > > Andy
> > > >
> > > > > Andy
> > > > > > Signed-off-by: Chuhong Yuan <[email protected]>
> > > > > > ---
> > > > > > drivers/net/ethernet/freescale/fec_main.c | 2 ++
> > > > > > 1 file changed, 2 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > > > > > b/drivers/net/ethernet/freescale/fec_main.c
> > > > > > index a9c386b63581..696550f4972f 100644
> > > > > > --- a/drivers/net/ethernet/freescale/fec_main.c
> > > > > > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > > > > > @@ -3645,8 +3645,10 @@ fec_drv_remove(struct platform_device
> > > > *pdev)
> > > > > > regulator_disable(fep->reg_phy);
> > > > > > pm_runtime_put(&pdev->dev);
> > > > > > pm_runtime_disable(&pdev->dev);
> > > > > > +#ifndef CONFIG_PM
> > > > > > clk_disable_unprepare(fep->clk_ahb);
> > > > > > clk_disable_unprepare(fep->clk_ipg);
> > > > > > +#endif

FWIIW, I am surprised this is the cleanest way to resolve this problem,
though I confess that I have no specific alternative in mind.

> > > > > > if (of_phy_is_fixed_link(np))
> > > > > > of_phy_deregister_fixed_link(np);
> > > > > > of_node_put(fep->phy_node);
> > > > > > --
> > > > > > 2.23.0
> > > > >