From: Christian Hemp <[email protected]>
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 <[email protected]>
Signed-off-by: Stefan Riedmueller <[email protected]>
---
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
On Wed, Sep 23, 2020 at 04:25:28PM +0200, Stefan Riedmueller wrote:
> From: Christian Hemp <[email protected]>
>
> 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
From: Stefan Riedmueller <[email protected]>
Date: Wed, 23 Sep 2020 16:25:28 +0200
> From: Christian Hemp <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Stefan Riedmueller <[email protected]>
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.
From: David Miller <[email protected]> Sent: Thursday, September 24, 2020 4:32 AM
> From: Stefan Riedmueller <[email protected]>
> Date: Wed, 23 Sep 2020 16:25:28 +0200
>
> > From: Christian Hemp <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Stefan Riedmueller <[email protected]>
>
> 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
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 <[email protected]> Sent: Thursday, September 24, 2020 4:32 AM
>> From: Stefan Riedmueller <[email protected]>
>> Date: Wed, 23 Sep 2020 16:25:28 +0200
>>
>>> From: Christian Hemp <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Signed-off-by: Stefan Riedmueller <[email protected]>
>>
>> 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
>