2018-06-13 06:38:57

by Radhey Shyam Pandey

[permalink] [raw]
Subject: [PATCH 0/4] emaclite bug fixes and code cleanup

This patch series fixes bug in emaclite remove and mdio_setup routines.
It does minor code cleanup.

Radhey Shyam Pandey (4):
net: emaclite: Fix position of lp->mii_bus assignment
net: emaclite: Fix MDIO bus unregister bug
net: emaclite: Remove unused 'has_mdio' flag.
net: emaclite: Remove xemaclite_mdio_setup return check

drivers/net/ethernet/xilinx/xilinx_emaclite.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)



2018-06-13 06:36:55

by Radhey Shyam Pandey

[permalink] [raw]
Subject: [PATCH 1/4] net: emaclite: Fix position of lp->mii_bus assignment

To ensure MDIO bus is not double freed in remove() path
assign lp->mii_bus after MDIO bus registration.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 69e31ce..37989ce 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -863,14 +863,14 @@ static int xemaclite_mdio_setup(struct net_local *lp, struct device *dev)
bus->write = xemaclite_mdio_write;
bus->parent = dev;

- lp->mii_bus = bus;
-
rc = of_mdiobus_register(bus, np);
if (rc) {
dev_err(dev, "Failed to register mdio bus.\n");
goto err_register;
}

+ lp->mii_bus = bus;
+
return 0;

err_register:
--
1.7.1


2018-06-13 06:37:18

by Radhey Shyam Pandey

[permalink] [raw]
Subject: [PATCH 4/4] net: emaclite: Remove xemaclite_mdio_setup return check

Errors are already reported in xemaclite_mdio_setup so avoid
reporting it again.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index ec4608e..2a0c06e 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -1143,9 +1143,7 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
xemaclite_update_address(lp, ndev->dev_addr);

lp->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
- rc = xemaclite_mdio_setup(lp, &ofdev->dev);
- if (rc)
- dev_warn(&ofdev->dev, "error registering MDIO bus\n");
+ xemaclite_mdio_setup(lp, &ofdev->dev);

dev_info(dev, "MAC address is now %pM\n", ndev->dev_addr);

--
1.7.1


2018-06-13 06:39:16

by Radhey Shyam Pandey

[permalink] [raw]
Subject: [PATCH 2/4] net: emaclite: Fix MDIO bus unregister bug

Since 'has_mdio' flag is not used,sequence insmod->rmmod-> insmod
leads to failure as MDIO unregister doesn't happen in .remove().
Fix it by checking MII bus pointer instead.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 37989ce..06eb6c8 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -1191,7 +1191,7 @@ static int xemaclite_of_remove(struct platform_device *of_dev)
struct net_local *lp = netdev_priv(ndev);

/* Un-register the mii_bus, if configured */
- if (lp->has_mdio) {
+ if (lp->mii_bus) {
mdiobus_unregister(lp->mii_bus);
mdiobus_free(lp->mii_bus);
lp->mii_bus = NULL;
--
1.7.1


2018-06-13 06:39:31

by Radhey Shyam Pandey

[permalink] [raw]
Subject: [PATCH 3/4] net: emaclite: Remove unused 'has_mdio' flag.

Remove unused 'has_mdio' flag.

Signed-off-by: Radhey Shyam Pandey <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
---
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index 06eb6c8..ec4608e 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -123,7 +123,6 @@
* @phy_node: pointer to the PHY device node
* @mii_bus: pointer to the MII bus
* @last_link: last link status
- * @has_mdio: indicates whether MDIO is included in the HW
*/
struct net_local {

@@ -144,7 +143,6 @@ struct net_local {
struct mii_bus *mii_bus;

int last_link;
- bool has_mdio;
};


--
1.7.1


2018-06-13 07:22:14

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 1/4] net: emaclite: Fix position of lp->mii_bus assignment

On Wed, Jun 13, 2018 at 12:05:16PM +0530, Radhey Shyam Pandey wrote:
> To ensure MDIO bus is not double freed in remove() path
> assign lp->mii_bus after MDIO bus registration.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2018-06-13 07:24:11

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 2/4] net: emaclite: Fix MDIO bus unregister bug

On Wed, Jun 13, 2018 at 12:05:17PM +0530, Radhey Shyam Pandey wrote:
> Since 'has_mdio' flag is not used,sequence insmod->rmmod-> insmod
> leads to failure as MDIO unregister doesn't happen in .remove().
> Fix it by checking MII bus pointer instead.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2018-06-13 07:26:02

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 3/4] net: emaclite: Remove unused 'has_mdio' flag.

On Wed, Jun 13, 2018 at 12:05:18PM +0530, Radhey Shyam Pandey wrote:
> Remove unused 'has_mdio' flag.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>

Reviewed-by: Andrew Lunn <[email protected]>

Andrew

2018-06-13 07:30:09

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH 4/4] net: emaclite: Remove xemaclite_mdio_setup return check

On Wed, Jun 13, 2018 at 12:05:19PM +0530, Radhey Shyam Pandey wrote:
> Errors are already reported in xemaclite_mdio_setup so avoid
> reporting it again.
>
> Signed-off-by: Radhey Shyam Pandey <[email protected]>
> Signed-off-by: Michal Simek <[email protected]>
> ---
> drivers/net/ethernet/xilinx/xilinx_emaclite.c | 4 +---
> 1 files changed, 1 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> index ec4608e..2a0c06e 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> @@ -1143,9 +1143,7 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
> xemaclite_update_address(lp, ndev->dev_addr);
>
> lp->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-handle", 0);
> - rc = xemaclite_mdio_setup(lp, &ofdev->dev);
> - if (rc)
> - dev_warn(&ofdev->dev, "error registering MDIO bus\n");
> + xemaclite_mdio_setup(lp, &ofdev->dev);
>
> dev_info(dev, "MAC address is now %pM\n", ndev->dev_addr);

The patch itself is O.K.

Reviewed-by: Andrew Lunn <[email protected]>

However, do you want to keep going if the MDIO bus fails? Maybe you
should failed the probe?

Andrew

2018-06-13 16:03:43

by Radhey Shyam Pandey

[permalink] [raw]
Subject: RE: [PATCH 4/4] net: emaclite: Remove xemaclite_mdio_setup return check


> -----Original Message-----
> From: Andrew Lunn [mailto:[email protected]]
> Sent: Wednesday, June 13, 2018 12:59 PM
> To: Radhey Shyam Pandey <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH 4/4] net: emaclite: Remove xemaclite_mdio_setup return
> check
>
> On Wed, Jun 13, 2018 at 12:05:19PM +0530, Radhey Shyam Pandey wrote:
> > Errors are already reported in xemaclite_mdio_setup so avoid
> > reporting it again.
> >
> > Signed-off-by: Radhey Shyam Pandey <[email protected]>
> > Signed-off-by: Michal Simek <[email protected]>
> > ---
> > drivers/net/ethernet/xilinx/xilinx_emaclite.c | 4 +---
> > 1 files changed, 1 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > index ec4608e..2a0c06e 100644
> > --- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > +++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
> > @@ -1143,9 +1143,7 @@ static int xemaclite_of_probe(struct
> platform_device *ofdev)
> > xemaclite_update_address(lp, ndev->dev_addr);
> >
> > lp->phy_node = of_parse_phandle(ofdev->dev.of_node, "phy-
> handle", 0);
> > - rc = xemaclite_mdio_setup(lp, &ofdev->dev);
> > - if (rc)
> > - dev_warn(&ofdev->dev, "error registering MDIO bus\n");
> > + xemaclite_mdio_setup(lp, &ofdev->dev);
> >
> > dev_info(dev, "MAC address is now %pM\n", ndev->dev_addr);
>
> The patch itself is O.K.
>
> Reviewed-by: Andrew Lunn <[email protected]>
>
> However, do you want to keep going if the MDIO bus fails? Maybe you
> should failed the probe?
Thanks for the review. Yes, I will fix it in next series.

-Radhey
>
> Andrew

2018-06-15 00:09:00

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 0/4] emaclite bug fixes and code cleanup

From: Radhey Shyam Pandey <[email protected]>
Date: Wed, 13 Jun 2018 12:05:15 +0530

> This patch series fixes bug in emaclite remove and mdio_setup routines.
> It does minor code cleanup.

Series applied.