[PATCH 0/4] emaclite bug fixes and code cleanup

[PATCH 0/4] emaclite bug fixes and code cleanup

[Radhey Shyam Pandey]

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(-)

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

[Radhey Shyam Pandey]

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 <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 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

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

[Radhey Shyam Pandey]

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 <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 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

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

[Radhey Shyam Pandey]

Remove unused 'has_mdio' flag.

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 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

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

[Radhey Shyam Pandey]

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

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
 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

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

[Andrew Lunn]

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 <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

[Andrew Lunn]

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 <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

[Andrew Lunn]

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 <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

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

[Andrew Lunn]

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 <radhey.shyam.pandey@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>  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 <andrew@lunn.ch>

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

    Andrew

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

[Radhey Shyam Pandey]

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Wednesday, June 13, 2018 12:59 PM
> To: Radhey Shyam Pandey <radheys@xilinx.com>
> Cc: davem@davemloft.net; michal.simek@xilinx.com;
> netdev@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> 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 <radhey.shyam.pandey@xilinx.com>
> > Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > ---
> >  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 <andrew@lunn.ch>
> 
> 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

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

[David Miller]

From: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
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.