[09/10] net: ethernet: ixp4xx: Get port ID from base address

Message ID 20191021000824.531-10-linus.walleij@linaro.org
State Superseded
Headers show
Series
  • IXP4xx networking cleanups
Related show

Commit Message

Linus Walleij Oct. 21, 2019, 12:08 a.m.
The port->id was picked from the platform device .id field,
but this is not supposed to be used for passing around
random numbers in hardware. Identify the port ID number
from the base address instead.

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

---
 drivers/net/ethernet/xscale/ixp4xx_eth.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.21.0

Comments

Arnd Bergmann Oct. 21, 2019, 10:30 a.m. | #1
On Mon, Oct 21, 2019 at 2:10 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>

> The port->id was picked from the platform device .id field,

> but this is not supposed to be used for passing around

> random numbers in hardware. Identify the port ID number

> from the base address instead.

>

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


I'm not sure this is better, as now the driver hardcodes the physical
address, and the port->id value is still the same as the pdev->id
value that all boards still pass.

Is this just meant to avoid setting the port id explicitly in DT?

       Arnd
Linus Walleij Oct. 31, 2019, 11:17 p.m. | #2
On Mon, Oct 21, 2019 at 12:30 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Oct 21, 2019 at 2:10 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> >

> > The port->id was picked from the platform device .id field,

> > but this is not supposed to be used for passing around

> > random numbers in hardware. Identify the port ID number

> > from the base address instead.

> >

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

>

> I'm not sure this is better, as now the driver hardcodes the physical

> address, and the port->id value is still the same as the pdev->id

> value that all boards still pass.

>

> Is this just meant to avoid setting the port id explicitly in DT?


Yes, because the DT bindings people would not like us to encode
that in DT, as it is a Linuxism.

To DT these are just three networking engines (NPEs) that the
OS can choose to use however it likes.

That they behave differently and that the driver has to cope
with that is due to different firmware being loaded into the
different NPE:s. DT doesn't care about that.

I will mention it in the commit message.

Yours,
Linus Walleij

Patch

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index 60cff1bce113..0996046bd046 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -1385,7 +1385,6 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 	SET_NETDEV_DEV(ndev, dev);
 	port = netdev_priv(ndev);
 	port->netdev = ndev;
-	port->id = pdev->id;
 
 	/* Get the port resource and remap */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1394,13 +1393,15 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 	regs_phys = res->start;
 	port->regs = devm_ioremap_resource(dev, res);
 
-	switch (port->id) {
-	case IXP4XX_ETH_NPEA:
+	switch (res->start) {
+	case 0xc800c000:
+		port->id = IXP4XX_ETH_NPEA;
 		/* If the MDIO bus is not up yet, defer probe */
 		if (!mdio_bus)
 			return -EPROBE_DEFER;
 		break;
-	case IXP4XX_ETH_NPEB:
+	case 0xc8009000:
+		port->id = 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
@@ -1417,7 +1418,8 @@  static int ixp4xx_eth_probe(struct platform_device *pdev)
 		if (!mdio_bus)
 			return -EPROBE_DEFER;
 		break;
-	case IXP4XX_ETH_NPEC:
+	case 0xc800a000:
+		port->id = 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.