diff mbox

[net,11/16] net: ethernet: marvell: mvneta: fix fixed-link phydev leaks

Message ID 1480357509-28074-12-git-send-email-johan@kernel.org (mailing list archive)
State Awaiting Upstream
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Johan Hovold Nov. 28, 2016, 6:25 p.m. UTC
Make sure to deregister and free any fixed-link PHY registered using
of_phy_register_fixed_link() on probe errors and on driver unbind.

Fixes: 83895bedeee6 ("net: mvneta: add support for fixed links")
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/net/ethernet/marvell/mvneta.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Thomas Petazzoni Nov. 28, 2016, 8:10 p.m. UTC | #1
Hello,

On Mon, 28 Nov 2016 19:25:04 +0100, Johan Hovold wrote:
> Make sure to deregister and free any fixed-link PHY registered using
> of_phy_register_fixed_link() on probe errors and on driver unbind.
> 
> Fixes: 83895bedeee6 ("net: mvneta: add support for fixed links")
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 5 +++++
>  1 file changed, 5 insertions(+)

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Thanks!

Thomas
Naresh Kamboju May 6, 2020, 6:57 p.m. UTC | #2
On Tue, 29 Nov 2016 at 00:00, Johan Hovold <johan@kernel.org> wrote:
>
> Make sure to deregister and free any fixed-link PHY registered using
> of_phy_register_fixed_link() on probe errors and on driver unbind.
>
> Fixes: 83895bedeee6 ("net: mvneta: add support for fixed links")
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/net/ethernet/marvell/mvneta.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> index 0c0a45af950f..707bc4680b9b 100644
> --- a/drivers/net/ethernet/marvell/mvneta.c
> +++ b/drivers/net/ethernet/marvell/mvneta.c
> @@ -4191,6 +4191,8 @@ static int mvneta_probe(struct platform_device *pdev)
>         clk_disable_unprepare(pp->clk);
>  err_put_phy_node:
>         of_node_put(phy_node);
> +       if (of_phy_is_fixed_link(dn))
> +               of_phy_deregister_fixed_link(dn);

While building kernel Image for arm architecture on stable-rc 4.4 branch
the following build error found.

drivers/net/ethernet/marvell/mvneta.c:3442:3: error: implicit
declaration of function 'of_phy_deregister_fixed_link'; did you mean
'of_phy_register_fixed_link'? [-Werror=implicit-function-declaration]
|    of_phy_deregister_fixed_link(dn);
|    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
|    of_phy_register_fixed_link

ref:
https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/541374729

- Naresh
Johan Hovold May 7, 2020, 6:44 a.m. UTC | #3
On Thu, May 07, 2020 at 12:27:53AM +0530, Naresh Kamboju wrote:
> On Tue, 29 Nov 2016 at 00:00, Johan Hovold <johan@kernel.org> wrote:
> >
> > Make sure to deregister and free any fixed-link PHY registered using
> > of_phy_register_fixed_link() on probe errors and on driver unbind.
> >
> > Fixes: 83895bedeee6 ("net: mvneta: add support for fixed links")
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > ---
> >  drivers/net/ethernet/marvell/mvneta.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index 0c0a45af950f..707bc4680b9b 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -4191,6 +4191,8 @@ static int mvneta_probe(struct platform_device *pdev)
> >         clk_disable_unprepare(pp->clk);
> >  err_put_phy_node:
> >         of_node_put(phy_node);
> > +       if (of_phy_is_fixed_link(dn))
> > +               of_phy_deregister_fixed_link(dn);
> 
> While building kernel Image for arm architecture on stable-rc 4.4 branch
> the following build error found.
> 
> drivers/net/ethernet/marvell/mvneta.c:3442:3: error: implicit
> declaration of function 'of_phy_deregister_fixed_link'; did you mean
> 'of_phy_register_fixed_link'? [-Werror=implicit-function-declaration]
> |    of_phy_deregister_fixed_link(dn);
> |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> |    of_phy_register_fixed_link
> 
> ref:
> https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/541374729

Greg, 3f65047c853a ("of_mdio: add helper to deregister fixed-link
PHYs") needs to be backported as well for these.

Original series can be found here:

	https://lkml.kernel.org/r/1480357509-28074-1-git-send-email-johan@kernel.org

Johan
Greg KH May 7, 2020, 6:47 a.m. UTC | #4
On Thu, May 07, 2020 at 08:44:12AM +0200, Johan Hovold wrote:
> On Thu, May 07, 2020 at 12:27:53AM +0530, Naresh Kamboju wrote:
> > On Tue, 29 Nov 2016 at 00:00, Johan Hovold <johan@kernel.org> wrote:
> > >
> > > Make sure to deregister and free any fixed-link PHY registered using
> > > of_phy_register_fixed_link() on probe errors and on driver unbind.
> > >
> > > Fixes: 83895bedeee6 ("net: mvneta: add support for fixed links")
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > ---
> > >  drivers/net/ethernet/marvell/mvneta.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > > index 0c0a45af950f..707bc4680b9b 100644
> > > --- a/drivers/net/ethernet/marvell/mvneta.c
> > > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > > @@ -4191,6 +4191,8 @@ static int mvneta_probe(struct platform_device *pdev)
> > >         clk_disable_unprepare(pp->clk);
> > >  err_put_phy_node:
> > >         of_node_put(phy_node);
> > > +       if (of_phy_is_fixed_link(dn))
> > > +               of_phy_deregister_fixed_link(dn);
> > 
> > While building kernel Image for arm architecture on stable-rc 4.4 branch
> > the following build error found.
> > 
> > drivers/net/ethernet/marvell/mvneta.c:3442:3: error: implicit
> > declaration of function 'of_phy_deregister_fixed_link'; did you mean
> > 'of_phy_register_fixed_link'? [-Werror=implicit-function-declaration]
> > |    of_phy_deregister_fixed_link(dn);
> > |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > |    of_phy_register_fixed_link
> > 
> > ref:
> > https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/541374729
> 
> Greg, 3f65047c853a ("of_mdio: add helper to deregister fixed-link
> PHYs") needs to be backported as well for these.
> 
> Original series can be found here:
> 
> 	https://lkml.kernel.org/r/1480357509-28074-1-git-send-email-johan@kernel.org

Ah, thanks for that, I thought I dropped all of the ones that caused
build errors, but missed the above one.  I'll go take the whole series
instead.

greg k-h
Greg KH May 7, 2020, 11:13 a.m. UTC | #5
On Thu, May 07, 2020 at 08:47:34AM +0200, Greg Kroah-Hartman wrote:
> On Thu, May 07, 2020 at 08:44:12AM +0200, Johan Hovold wrote:
> > On Thu, May 07, 2020 at 12:27:53AM +0530, Naresh Kamboju wrote:
> > > On Tue, 29 Nov 2016 at 00:00, Johan Hovold <johan@kernel.org> wrote:
> > > >
> > > > Make sure to deregister and free any fixed-link PHY registered using
> > > > of_phy_register_fixed_link() on probe errors and on driver unbind.
> > > >
> > > > Fixes: 83895bedeee6 ("net: mvneta: add support for fixed links")
> > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > ---
> > > >  drivers/net/ethernet/marvell/mvneta.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > > > index 0c0a45af950f..707bc4680b9b 100644
> > > > --- a/drivers/net/ethernet/marvell/mvneta.c
> > > > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > > > @@ -4191,6 +4191,8 @@ static int mvneta_probe(struct platform_device *pdev)
> > > >         clk_disable_unprepare(pp->clk);
> > > >  err_put_phy_node:
> > > >         of_node_put(phy_node);
> > > > +       if (of_phy_is_fixed_link(dn))
> > > > +               of_phy_deregister_fixed_link(dn);
> > > 
> > > While building kernel Image for arm architecture on stable-rc 4.4 branch
> > > the following build error found.
> > > 
> > > drivers/net/ethernet/marvell/mvneta.c:3442:3: error: implicit
> > > declaration of function 'of_phy_deregister_fixed_link'; did you mean
> > > 'of_phy_register_fixed_link'? [-Werror=implicit-function-declaration]
> > > |    of_phy_deregister_fixed_link(dn);
> > > |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > |    of_phy_register_fixed_link
> > > 
> > > ref:
> > > https://gitlab.com/Linaro/lkft/kernel-runs/-/jobs/541374729
> > 
> > Greg, 3f65047c853a ("of_mdio: add helper to deregister fixed-link
> > PHYs") needs to be backported as well for these.
> > 
> > Original series can be found here:
> > 
> > 	https://lkml.kernel.org/r/1480357509-28074-1-git-send-email-johan@kernel.org
> 
> Ah, thanks for that, I thought I dropped all of the ones that caused
> build errors, but missed the above one.  I'll go take the whole series
> instead.

This should now all be fixed up, thanks.

greg k-h
Naresh Kamboju May 7, 2020, 10:05 p.m. UTC | #6
On Thu, 7 May 2020 at 16:43, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
<trim>
> > >
> > > Greg, 3f65047c853a ("of_mdio: add helper to deregister fixed-link
> > > PHYs") needs to be backported as well for these.
> > >
> > > Original series can be found here:
> > >
> > >     https://lkml.kernel.org/r/1480357509-28074-1-git-send-email-johan@kernel.org
> >
> > Ah, thanks for that, I thought I dropped all of the ones that caused
> > build errors, but missed the above one.  I'll go take the whole series
> > instead.
>
> This should now all be fixed up, thanks.

While building kernel Image for arm architecture on stable-rc 4.4 branch
the following build error found.

of_mdio: add helper to deregister fixed-link PHYs
commit 3f65047c853a2a5abcd8ac1984af3452b5df4ada upstream.

Add helper to deregister fixed-link PHYs registered using
of_phy_register_fixed_link().

Convert the two drivers that care to deregister their fixed-link PHYs to
use the new helper, but note that most drivers currently fail to do so.

Signed-off-by: Johan Hovold <johan@kernel.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
[only take helper function for 4.4.y - gregkh]

 # make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arm
CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc CC="sccache
arm-linux-gnueabihf-gcc" O=build zImage
70 #
71 ../drivers/of/of_mdio.c: In function ‘of_phy_deregister_fixed_link’:
72 ../drivers/of/of_mdio.c:379:2: error: implicit declaration of
function ‘fixed_phy_unregister’; did you mean ‘fixed_phy_register’?
[-Werror=implicit-function-declaration]
73  379 | fixed_phy_unregister(phydev);
74  | ^~~~~~~~~~~~~~~~~~~~
75  | fixed_phy_register
76 ../drivers/of/of_mdio.c:381:22: error: ‘struct phy_device’ has no
member named ‘mdio’; did you mean ‘mdix’?
77  381 | put_device(&phydev->mdio.dev); /* of_phy_find_device() */
78  | ^~~~
79  | mdix

>
> greg k-h
Johan Hovold May 8, 2020, 6:21 a.m. UTC | #7
On Fri, May 08, 2020 at 03:35:02AM +0530, Naresh Kamboju wrote:
> On Thu, 7 May 2020 at 16:43, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> <trim>
> > > >
> > > > Greg, 3f65047c853a ("of_mdio: add helper to deregister fixed-link
> > > > PHYs") needs to be backported as well for these.
> > > >
> > > > Original series can be found here:
> > > >
> > > >     https://lkml.kernel.org/r/1480357509-28074-1-git-send-email-johan@kernel.org
> > >
> > > Ah, thanks for that, I thought I dropped all of the ones that caused
> > > build errors, but missed the above one.  I'll go take the whole series
> > > instead.
> >
> > This should now all be fixed up, thanks.
> 
> While building kernel Image for arm architecture on stable-rc 4.4 branch
> the following build error found.
> 
> of_mdio: add helper to deregister fixed-link PHYs
> commit 3f65047c853a2a5abcd8ac1984af3452b5df4ada upstream.
> 
> Add helper to deregister fixed-link PHYs registered using
> of_phy_register_fixed_link().
> 
> Convert the two drivers that care to deregister their fixed-link PHYs to
> use the new helper, but note that most drivers currently fail to do so.
> 
> Signed-off-by: Johan Hovold <johan@kernel.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> [only take helper function for 4.4.y - gregkh]
> 
>  # make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arm
> CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc CC="sccache
> arm-linux-gnueabihf-gcc" O=build zImage
> 70 #
> 71 ../drivers/of/of_mdio.c: In function ‘of_phy_deregister_fixed_link’:
> 72 ../drivers/of/of_mdio.c:379:2: error: implicit declaration of
> function ‘fixed_phy_unregister’; did you mean ‘fixed_phy_register’?
> [-Werror=implicit-function-declaration]
> 73  379 | fixed_phy_unregister(phydev);
> 74  | ^~~~~~~~~~~~~~~~~~~~
> 75  | fixed_phy_register
> 76 ../drivers/of/of_mdio.c:381:22: error: ‘struct phy_device’ has no
> member named ‘mdio’; did you mean ‘mdix’?
> 77  381 | put_device(&phydev->mdio.dev); /* of_phy_find_device() */
> 78  | ^~~~
> 79  | mdix

Another dependency: 5bcbe0f35fb1 ("phy: fixed: Fix removal of phys.")

Greg, these patches are from four years ago so can't really remember if
there are other dependencies or reasons against backporting them (the
missing stable tags are per Dave's preference), sorry.

The cover letter also mentions another dependency, but that may just
have been some context conflict.

Perhaps you better drop these unless you want to review them closer.

Johan
Greg KH May 8, 2020, 12:02 p.m. UTC | #8
On Fri, May 08, 2020 at 08:21:19AM +0200, Johan Hovold wrote:
> On Fri, May 08, 2020 at 03:35:02AM +0530, Naresh Kamboju wrote:
> > On Thu, 7 May 2020 at 16:43, Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > <trim>
> > > > >
> > > > > Greg, 3f65047c853a ("of_mdio: add helper to deregister fixed-link
> > > > > PHYs") needs to be backported as well for these.
> > > > >
> > > > > Original series can be found here:
> > > > >
> > > > >     https://lkml.kernel.org/r/1480357509-28074-1-git-send-email-johan@kernel.org
> > > >
> > > > Ah, thanks for that, I thought I dropped all of the ones that caused
> > > > build errors, but missed the above one.  I'll go take the whole series
> > > > instead.
> > >
> > > This should now all be fixed up, thanks.
> > 
> > While building kernel Image for arm architecture on stable-rc 4.4 branch
> > the following build error found.
> > 
> > of_mdio: add helper to deregister fixed-link PHYs
> > commit 3f65047c853a2a5abcd8ac1984af3452b5df4ada upstream.
> > 
> > Add helper to deregister fixed-link PHYs registered using
> > of_phy_register_fixed_link().
> > 
> > Convert the two drivers that care to deregister their fixed-link PHYs to
> > use the new helper, but note that most drivers currently fail to do so.
> > 
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > [only take helper function for 4.4.y - gregkh]
> > 
> >  # make -sk KBUILD_BUILD_USER=TuxBuild -C/linux -j16 ARCH=arm
> > CROSS_COMPILE=arm-linux-gnueabihf- HOSTCC=gcc CC="sccache
> > arm-linux-gnueabihf-gcc" O=build zImage
> > 70 #
> > 71 ../drivers/of/of_mdio.c: In function ‘of_phy_deregister_fixed_link’:
> > 72 ../drivers/of/of_mdio.c:379:2: error: implicit declaration of
> > function ‘fixed_phy_unregister’; did you mean ‘fixed_phy_register’?
> > [-Werror=implicit-function-declaration]
> > 73  379 | fixed_phy_unregister(phydev);
> > 74  | ^~~~~~~~~~~~~~~~~~~~
> > 75  | fixed_phy_register
> > 76 ../drivers/of/of_mdio.c:381:22: error: ‘struct phy_device’ has no
> > member named ‘mdio’; did you mean ‘mdix’?
> > 77  381 | put_device(&phydev->mdio.dev); /* of_phy_find_device() */
> > 78  | ^~~~
> > 79  | mdix
> 
> Another dependency: 5bcbe0f35fb1 ("phy: fixed: Fix removal of phys.")
> 
> Greg, these patches are from four years ago so can't really remember if
> there are other dependencies or reasons against backporting them (the
> missing stable tags are per Dave's preference), sorry.
> 
> The cover letter also mentions another dependency, but that may just
> have been some context conflict.
> 
> Perhaps you better drop these unless you want to review them closer.

Good idea, I've dropped them all for now, sorry for the noise.

greg k-h
diff mbox

Patch

diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index 0c0a45af950f..707bc4680b9b 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -4191,6 +4191,8 @@  static int mvneta_probe(struct platform_device *pdev)
 	clk_disable_unprepare(pp->clk);
 err_put_phy_node:
 	of_node_put(phy_node);
+	if (of_phy_is_fixed_link(dn))
+		of_phy_deregister_fixed_link(dn);
 err_free_irq:
 	irq_dispose_mapping(dev->irq);
 err_free_netdev:
@@ -4202,6 +4204,7 @@  static int mvneta_probe(struct platform_device *pdev)
 static int mvneta_remove(struct platform_device *pdev)
 {
 	struct net_device  *dev = platform_get_drvdata(pdev);
+	struct device_node *dn = pdev->dev.of_node;
 	struct mvneta_port *pp = netdev_priv(dev);
 
 	unregister_netdev(dev);
@@ -4209,6 +4212,8 @@  static int mvneta_remove(struct platform_device *pdev)
 	clk_disable_unprepare(pp->clk);
 	free_percpu(pp->ports);
 	free_percpu(pp->stats);
+	if (of_phy_is_fixed_link(dn))
+		of_phy_deregister_fixed_link(dn);
 	irq_dispose_mapping(dev->irq);
 	of_node_put(pp->phy_node);
 	free_netdev(dev);