[PATCH] net: fec: Keep device numbering consistent with datasheet

[PATCH] net: fec: Keep device numbering consistent with datasheet

[Stefan Riedmueller]

From: Christian Hemp <c.hemp@phytec.de>

Make use of device tree alias for device enumeration to keep the device
order consistent with the naming in the datasheet.

Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as eth1
and ENET2 as eth0.

Signed-off-by: Christian Hemp <c.hemp@phytec.de>
Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
---
 drivers/net/ethernet/freescale/fec_main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index fb37816a74db..97dd41bed70a 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3504,6 +3504,7 @@ fec_probe(struct platform_device *pdev)
 	char irq_name[8];
 	int irq_cnt;
 	struct fec_devinfo *dev_info;
+	int eth_id;
 
 	fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
 
@@ -3691,6 +3692,10 @@ fec_probe(struct platform_device *pdev)
 
 	ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN;
 
+	eth_id = of_alias_get_id(pdev->dev.of_node, "ethernet");
+	if (eth_id >= 0)
+		sprintf(ndev->name, "eth%d", eth_id);
+
 	ret = register_netdev(ndev);
 	if (ret)
 		goto failed_register;
-- 
2.25.1

Re: [PATCH] net: fec: Keep device numbering consistent with datasheet

[Andrew Lunn]

On Wed, Sep 23, 2020 at 04:25:28PM +0200, Stefan Riedmueller wrote:
> From: Christian Hemp <c.hemp@phytec.de>
> 
> Make use of device tree alias for device enumeration to keep the device
> order consistent with the naming in the datasheet.
> 
> Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as eth1
> and ENET2 as eth0.

What is wrong with that?  Ethernet interface names have never been
guaranteed to be stable.

You have to be careful here. Have you reviewed all the DTS files to
make sure none already have aliases? You could be breaking boards
which currently 'work' because the alias is ignored.

udev is actually a better solution for giving the interfaces
dependable names.

      Andrew

Re: [PATCH] net: fec: Keep device numbering consistent with datasheet

[David Miller]

From: Stefan Riedmueller <s.riedmueller@phytec.de>
Date: Wed, 23 Sep 2020 16:25:28 +0200

> From: Christian Hemp <c.hemp@phytec.de>
> 
> Make use of device tree alias for device enumeration to keep the device
> order consistent with the naming in the datasheet.
> 
> Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as eth1
> and ENET2 as eth0.
> 
> Signed-off-by: Christian Hemp <c.hemp@phytec.de>
> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>

Device naming and ordering for networking devices was never, ever,
guaranteed.

Use udev or similar.

> @@ -3691,6 +3692,10 @@ fec_probe(struct platform_device *pdev)
>  
>  	ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN;
>  
> +	eth_id = of_alias_get_id(pdev->dev.of_node, "ethernet");
> +	if (eth_id >= 0)
> +		sprintf(ndev->name, "eth%d", eth_id);

You can't ever just write into ndev->name, what if another networking
device is already using that name?

This change is incorrect on many levels.

RE: [EXT] Re: [PATCH] net: fec: Keep device numbering consistent with datasheet

[Andy Duan]

From: David Miller <davem@davemloft.net> Sent: Thursday, September 24, 2020 4:32 AM
> From: Stefan Riedmueller <s.riedmueller@phytec.de>
> Date: Wed, 23 Sep 2020 16:25:28 +0200
> 
> > From: Christian Hemp <c.hemp@phytec.de>
> >
> > Make use of device tree alias for device enumeration to keep the
> > device order consistent with the naming in the datasheet.
> >
> > Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as
> > eth1 and ENET2 as eth0.
> >
> > Signed-off-by: Christian Hemp <c.hemp@phytec.de>
> > Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
> 
> Device naming and ordering for networking devices was never, ever,
> guaranteed.
> 
> Use udev or similar.
> 
> > @@ -3691,6 +3692,10 @@ fec_probe(struct platform_device *pdev)
> >
> >       ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN;
> >
> > +     eth_id = of_alias_get_id(pdev->dev.of_node, "ethernet");
> > +     if (eth_id >= 0)
> > +             sprintf(ndev->name, "eth%d", eth_id);
> 
> You can't ever just write into ndev->name, what if another networking device is
> already using that name?
> 
> This change is incorrect on many levels.

David is correct.

For example, imx8DXL has ethernet0 is EQOS TSN, ethernet1 is FEC.
EQOS TSN is andother driver and is registered early, the dev->name is eth0.
So the patch will bring conflict in such case.

Andy

Re: [EXT] Re: [PATCH] net: fec: Keep device numbering consistent with datasheet

[Stefan Riedmüller]

Hi Andy, David and Andrew,

first of all, thanks for your review. I really appreciate it!

On 24.09.20 08:36, Andy Duan wrote:
> From: David Miller <davem@davemloft.net> Sent: Thursday, September 24, 2020 4:32 AM
>> From: Stefan Riedmueller <s.riedmueller@phytec.de>
>> Date: Wed, 23 Sep 2020 16:25:28 +0200
>>
>>> From: Christian Hemp <c.hemp@phytec.de>
>>>
>>> Make use of device tree alias for device enumeration to keep the
>>> device order consistent with the naming in the datasheet.
>>>
>>> Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as
>>> eth1 and ENET2 as eth0.
>>>
>>> Signed-off-by: Christian Hemp <c.hemp@phytec.de>
>>> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
>>
>> Device naming and ordering for networking devices was never, ever,
>> guaranteed.
>>
>> Use udev or similar.
>>
>>> @@ -3691,6 +3692,10 @@ fec_probe(struct platform_device *pdev)
>>>
>>>        ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN;
>>>
>>> +     eth_id = of_alias_get_id(pdev->dev.of_node, "ethernet");
>>> +     if (eth_id >= 0)
>>> +             sprintf(ndev->name, "eth%d", eth_id);
>>
>> You can't ever just write into ndev->name, what if another networking device is
>> already using that name?
>>
>> This change is incorrect on many levels.
> 
> David is correct.
> 
> For example, imx8DXL has ethernet0 is EQOS TSN, ethernet1 is FEC.
> EQOS TSN is andother driver and is registered early, the dev->name is eth0.
> So the patch will bring conflict in such case.

I was not aware of that conflict, but now that you mention it it makes total 
sense.

I wanted to make life a little easier for myself but underestimated the 
global context. I will try to find a solution with udev or something similar.

So please drop this patch and sorry for the noise.

Stefan

> 
> Andy
>