[net-next,09/10,v2] net: ethernet: ixp4xx: Get port ID from base address

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

Commit Message

Linus Walleij Nov. 1, 2019, 1:02 p.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.

This is necessary for Device Tree conversion: to DT
these are just three networking engines (NPEs) that the OS
can choose to use however it likes. When we move to DT we
cannot get these port numbers from the device tree.

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. The firmware
can theoretically be changed, but the DT bindings can not.

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

---
ChangeLog v1->v2:
- Rebased on the rest of the series.
---
 drivers/net/ethernet/xscale/ixp4xx_eth.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.21.0

Comments

David Miller Nov. 4, 2019, 1:43 a.m. | #1
From: Linus Walleij <linus.walleij@linaro.org>

Date: Fri,  1 Nov 2019 14:02:23 +0100

> @@ -1388,13 +1387,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:


This is extremely non-portable.

The resource values are %100 opaque architecture specific values.

On sparc64 for example, it is absolutely not the bus address but rather
the physical address that the cpu needs to use to perform MMIO's to what
is behind that resource.

I'm not applying this, sorry.
Linus Walleij Nov. 5, 2019, 10:27 a.m. | #2
On Mon, Nov 4, 2019 at 2:43 AM David Miller <davem@davemloft.net> wrote:

> From: Linus Walleij <linus.walleij@linaro.org>

> Date: Fri,  1 Nov 2019 14:02:23 +0100

>

> > @@ -1388,13 +1387,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:

>

> This is extremely non-portable.

>

> The resource values are %100 opaque architecture specific values.

>

> On sparc64 for example, it is absolutely not the bus address but rather

> the physical address that the cpu needs to use to perform MMIO's to what

> is behind that resource.

>

> I'm not applying this, sorry.


No problem, I'll think of something better, like adapting
Arnd's approach.

Could you apply patches 1-8?

Yours,
Linus Walleij
David Miller Nov. 5, 2019, 5:18 p.m. | #3
From: Linus Walleij <linus.walleij@linaro.org>

Date: Tue, 5 Nov 2019 11:27:36 +0100

> Could you apply patches 1-8?


Sorry, that's not how patch submissions work.

Resubmit your series as you want me to apply it.  Make sure you update
your cover letter appropriately.

Thanks.

Patch

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index ee45215c4ba4..c5835a2fb965 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -1379,7 +1379,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);
@@ -1388,13 +1387,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
@@ -1411,7 +1412,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.