diff mbox series

[05/10] net: ethernet: ixp4xx: Standard module init

Message ID 20191021000824.531-6-linus.walleij@linaro.org
State New
Headers show
Series IXP4xx networking cleanups | expand

Commit Message

Linus Walleij Oct. 21, 2019, 12:08 a.m. UTC
The IXP4xx driver was initializing the MDIO bus before even
probing, in the callbacks supposed to be used for setting up
the module itself, and with the side effect of trying to
register the MDIO bus as soon as this module was loaded or
compiled into the kernel whether the device was discovered
or not.

This does not work with multiplatform environments.

To get rid of this: set up the MDIO bus from the probe()
callback and remove it in the remove() callback. Rename
the probe() and remove() calls to reflect the most common
conventions.

Since there is a bit of checking for the ethernet feature
to be present in the MDIO registering function, making the
whole module not even be registered if we can't find an
MDIO bus, we need something similar: register the MDIO
bus when the corresponding ethernet is probed, and
return -EPROBE_DEFER on the other interfaces until this
happens. If no MDIO bus is present on any of the
registered interfaces we will eventually bail out.

None of the platforms I've seen has e.g. MDIO on EthB
and only uses EthC, there is always a Ethernet hardware
on the NPE (B, C) that has the MDIO bus, we just might
have to wait for it.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

---
 drivers/net/ethernet/xscale/ixp4xx_eth.c | 82 ++++++++++++------------
 1 file changed, 40 insertions(+), 42 deletions(-)

-- 
2.21.0

Comments

Jakub Kicinski Oct. 22, 2019, 1:19 a.m. UTC | #1
On Mon, 21 Oct 2019 02:08:19 +0200, Linus Walleij wrote:
> @@ -1376,7 +1365,7 @@ static const struct net_device_ops ixp4xx_netdev_ops = {

>  	.ndo_validate_addr = eth_validate_addr,

>  };

>  

> -static int eth_init_one(struct platform_device *pdev)

> +static int ixp4xx_eth_probe(struct platform_device *pdev)

>  {

>  	struct port *port;

>  	struct net_device *dev;

> @@ -1396,14 +1385,46 @@ static int eth_init_one(struct platform_device *pdev)

>  

>  	switch (port->id) {

>  	case IXP4XX_ETH_NPEA:

> +		/* If the MDIO bus is not up yet, defer probe */

> +		if (!mdio_bus)

> +			return -EPROBE_DEFER;


There's an allocation at the top of this function. All the rest of the
code does goto err_xyz. I don't think you can just return directly here,
or anywhere below.

>  		port->regs = (struct eth_regs __iomem *)IXP4XX_EthA_BASE_VIRT;

>  		regs_phys  = IXP4XX_EthA_BASE_PHYS;

>  		break;

>  	case IXP4XX_ETH_NPEB:

> +		/*

> +		 * On all except IXP43x, NPE-B is used for the MDIO bus.

> +		 * If there is no NPE-B in the feature set, bail out, else

> +		 * register the MDIO bus.

> +		 */

> +		if (!cpu_is_ixp43x()) {

> +			if (!(ixp4xx_read_feature_bits() &

> +			      IXP4XX_FEATURE_NPEB_ETH0))

> +				return -ENODEV;

> +			/* Else register the MDIO bus on NPE-B */

> +			if ((err = ixp4xx_mdio_register(IXP4XX_EthC_BASE_VIRT)))

> +				return err;

> +		}

> +		if (!mdio_bus)

> +			return -EPROBE_DEFER;

>  		port->regs = (struct eth_regs __iomem *)IXP4XX_EthB_BASE_VIRT;

>  		regs_phys  = IXP4XX_EthB_BASE_PHYS;

>  		break;

>  	case IXP4XX_ETH_NPEC:

> +		/*

> +		 * IXP43x lacks NPE-B and uses NPE-C for the MDIO bus access,

> +		 * of there is no NPE-C, no bus, nothing works, so bail out.

> +		 */

> +		if (cpu_is_ixp43x()) {

> +			if (!(ixp4xx_read_feature_bits() &

> +			      IXP4XX_FEATURE_NPEC_ETH))

> +				return -ENODEV;

> +			/* Else register the MDIO bus on NPE-C */

> +			if ((err = ixp4xx_mdio_register(IXP4XX_EthC_BASE_VIRT)))

> +				return err;

> +		}

> +		if (!mdio_bus)

> +			return -EPROBE_DEFER;

>  		port->regs = (struct eth_regs __iomem *)IXP4XX_EthC_BASE_VIRT;

>  		regs_phys  = IXP4XX_EthC_BASE_PHYS;

>  		break;
diff mbox series

Patch

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index e811bf0d23cb..26da84402cfd 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -519,25 +519,14 @@  static int ixp4xx_mdio_write(struct mii_bus *bus, int phy_id, int location,
 	return ret;
 }
 
-static int ixp4xx_mdio_register(void)
+static int ixp4xx_mdio_register(struct eth_regs __iomem *regs)
 {
 	int err;
 
 	if (!(mdio_bus = mdiobus_alloc()))
 		return -ENOMEM;
 
-	if (cpu_is_ixp43x()) {
-		/* IXP43x lacks NPE-B and uses NPE-C for MII PHY access */
-		if (!(ixp4xx_read_feature_bits() & IXP4XX_FEATURE_NPEC_ETH))
-			return -ENODEV;
-		mdio_regs = (struct eth_regs __iomem *)IXP4XX_EthC_BASE_VIRT;
-	} else {
-		/* All MII PHY accesses use NPE-B Ethernet registers */
-		if (!(ixp4xx_read_feature_bits() & IXP4XX_FEATURE_NPEB_ETH0))
-			return -ENODEV;
-		mdio_regs = (struct eth_regs __iomem *)IXP4XX_EthB_BASE_VIRT;
-	}
-
+	mdio_regs = regs;
 	__raw_writel(DEFAULT_CORE_CNTRL, &mdio_regs->core_control);
 	spin_lock_init(&mdio_lock);
 	mdio_bus->name = "IXP4xx MII Bus";
@@ -1376,7 +1365,7 @@  static const struct net_device_ops ixp4xx_netdev_ops = {
 	.ndo_validate_addr = eth_validate_addr,
 };
 
-static int eth_init_one(struct platform_device *pdev)
+static int ixp4xx_eth_probe(struct platform_device *pdev)
 {
 	struct port *port;
 	struct net_device *dev;
@@ -1396,14 +1385,46 @@  static int eth_init_one(struct platform_device *pdev)
 
 	switch (port->id) {
 	case IXP4XX_ETH_NPEA:
+		/* If the MDIO bus is not up yet, defer probe */
+		if (!mdio_bus)
+			return -EPROBE_DEFER;
 		port->regs = (struct eth_regs __iomem *)IXP4XX_EthA_BASE_VIRT;
 		regs_phys  = IXP4XX_EthA_BASE_PHYS;
 		break;
 	case IXP4XX_ETH_NPEB:
+		/*
+		 * On all except IXP43x, NPE-B is used for the MDIO bus.
+		 * If there is no NPE-B in the feature set, bail out, else
+		 * register the MDIO bus.
+		 */
+		if (!cpu_is_ixp43x()) {
+			if (!(ixp4xx_read_feature_bits() &
+			      IXP4XX_FEATURE_NPEB_ETH0))
+				return -ENODEV;
+			/* Else register the MDIO bus on NPE-B */
+			if ((err = ixp4xx_mdio_register(IXP4XX_EthC_BASE_VIRT)))
+				return err;
+		}
+		if (!mdio_bus)
+			return -EPROBE_DEFER;
 		port->regs = (struct eth_regs __iomem *)IXP4XX_EthB_BASE_VIRT;
 		regs_phys  = IXP4XX_EthB_BASE_PHYS;
 		break;
 	case IXP4XX_ETH_NPEC:
+		/*
+		 * IXP43x lacks NPE-B and uses NPE-C for the MDIO bus access,
+		 * of there is no NPE-C, no bus, nothing works, so bail out.
+		 */
+		if (cpu_is_ixp43x()) {
+			if (!(ixp4xx_read_feature_bits() &
+			      IXP4XX_FEATURE_NPEC_ETH))
+				return -ENODEV;
+			/* Else register the MDIO bus on NPE-C */
+			if ((err = ixp4xx_mdio_register(IXP4XX_EthC_BASE_VIRT)))
+				return err;
+		}
+		if (!mdio_bus)
+			return -EPROBE_DEFER;
 		port->regs = (struct eth_regs __iomem *)IXP4XX_EthC_BASE_VIRT;
 		regs_phys  = IXP4XX_EthC_BASE_PHYS;
 		break;
@@ -1472,7 +1493,7 @@  static int eth_init_one(struct platform_device *pdev)
 	return err;
 }
 
-static int eth_remove_one(struct platform_device *pdev)
+static int ixp4xx_eth_remove(struct platform_device *pdev)
 {
 	struct net_device *dev = platform_get_drvdata(pdev);
 	struct phy_device *phydev = dev->phydev;
@@ -1480,6 +1501,7 @@  static int eth_remove_one(struct platform_device *pdev)
 
 	unregister_netdev(dev);
 	phy_disconnect(phydev);
+	ixp4xx_mdio_remove();
 	npe_port_tab[NPE_ID(port->id)] = NULL;
 	npe_release(port->npe);
 	release_resource(port->mem_res);
@@ -1489,36 +1511,12 @@  static int eth_remove_one(struct platform_device *pdev)
 
 static struct platform_driver ixp4xx_eth_driver = {
 	.driver.name	= DRV_NAME,
-	.probe		= eth_init_one,
-	.remove		= eth_remove_one,
+	.probe		= ixp4xx_eth_probe,
+	.remove		= ixp4xx_eth_remove,
 };
-
-static int __init eth_init_module(void)
-{
-	int err;
-
-	/*
-	 * FIXME: we bail out on device tree boot but this really needs
-	 * to be fixed in a nicer way: this registers the MDIO bus before
-	 * even matching the driver infrastructure, we should only probe
-	 * detected hardware.
-	 */
-	if (of_have_populated_dt())
-		return -ENODEV;
-	if ((err = ixp4xx_mdio_register()))
-		return err;
-	return platform_driver_register(&ixp4xx_eth_driver);
-}
-
-static void __exit eth_cleanup_module(void)
-{
-	platform_driver_unregister(&ixp4xx_eth_driver);
-	ixp4xx_mdio_remove();
-}
+module_platform_driver(ixp4xx_eth_driver);
 
 MODULE_AUTHOR("Krzysztof Halasa");
 MODULE_DESCRIPTION("Intel IXP4xx Ethernet driver");
 MODULE_LICENSE("GPL v2");
 MODULE_ALIAS("platform:ixp4xx_eth");
-module_init(eth_init_module);
-module_exit(eth_cleanup_module);