driver: net: smsc911x: set NOCARRIER flag in driver initialization
Subsystems (3)
PATCH 1/1 driver: net: smsc911x: set NOCARRIER flag in driver initialization
As smsc driver supports carrier detection, it should unset NOCARRIER flag only after carrier state determination. By default that flag is off so driver should set it before starting auto-negotiation
--- drivers/net/ethernet/smsc/smsc911x.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/net/ethernet/smsc/smsc911x.c +2 -0
@@ -2525,6 +2525,8 @@ static int smsc911x_drv_probe(struct platform_device *pdev)netdev_info (dev,"MAC Address: %pM\n" , dev->dev_addr); +netif_carrier_off (dev); +return 0 ; out_unregister_netdev_5:
Signature · 1 line
1.8.3.2
David Miller
maintainerFrom: Balakumaran Kannan <[email protected]>
Date: Sun, 25 May 2014 00:24:46 +0530
drivers/net/ethernet/smsc/smsc911x.c static int smsc911x_drv_probe(struct platform_device *pdev) · line 2531
Quoted 8 lines
drivers/net/ethernet/smsc/smsc911x.c +2
-0@@ -2525,6 +2525,8 @@ static int smsc911x_drv_probe(struct platform_device *pdev)netdev_info (dev,"MAC Address: %pM\n" , dev->dev_addr); +netif_carrier_off (dev); +return 0 ;
You should do this before the network device is registered.
Hi David,
Quoted 5 lines · David Miller
From: Balakumaran Kannan <[email protected]>
Date: Sun, 25 May 2014 00:24:46 +0530Quoted 2 lines
(quoted diff) +0
-0@@ -2525,6 +2525,8 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
Quoted 8 lines
drivers/net/ethernet/smsc/smsc911x.c static int smsc911x_drv_probe(struct platform_device *pdev) · line 2531
Quoted 6 lines
drivers/net/ethernet/smsc/smsc911x.c +2
-0netdev_info (dev,"MAC Address: %pM\n" , dev->dev_addr); +netif_carrier_off (dev); +return 0 ;You should do this before the network device is registered.
IMHO calling netif_carrier_off after registering device will not affect functionality. Because,
* Device starts carrier detection once driver's open function
calls phy_start.
* Kernel starts network layer initialization only after __dev_open
function sets IFF_UP bit of dev->flags.
And __dev_open sets this bit only after driver's open function returns.
* So we could place netif_carrier_off call anywhere beforephy_start in driver's open function gets called.
Also I have referred some other drivers.
* Intel i40e driver calls netif_carrier_off at the beginning of
driver's open (i40e_open) function.
* e1000 driver places netif_carrier_off at the end of driver probe
(e1000_probe) function.
* realtek r8169 driver calls netif_carrier_off at the end of
driver init (rtl_init_one) function.
All these drivers call netif_carrier_off function after registering the network device using register_netdev.
This explanation is based on my understanding. Please explain to me if I'm wrong or I miss something. In such case I'll modify the patch accordingly.
Thank you.
Regards,
K.Balakumaran
David Miller
maintainerFrom: Balakumaran Kannan <[email protected]>
Date: Fri, 30 May 2014 18:35:43 +0530
Quoted 2 lines
All these drivers call
netif_carrier_offfunction after registering the network device usingregister_netdev.
I can quote more counter-examples than your list, starting with the Broadcom tg3 driver.
Quoted 8 lines · David Miller
From: Balakumaran Kannan <[email protected]>
Date: Fri, 30 May 2014 18:35:43 +0530Quoted 2 lines
All these drivers call
netif_carrier_offfunction after registering the network device usingregister_netdev.I can quote more counter-examples than your list, starting with the Broadcom tg3 driver.
Thank you David for this information. I'll send updated patch as version-2.
Still I couldn't understand the reason behind this. I should be grateful if you would explain me little bit.
Regards,
K.Balakumaran
2014-06-03 7:15 GMT-07:00 Balakumaran Kannan <[email protected]>:
Quoted 7 lines · David Miller
Quoted 6 lines
From: Balakumaran Kannan <[email protected]>
Date: Fri, 30 May 2014 18:35:43 +0530Quoted 2 lines
All these drivers call
netif_carrier_offfunction after registering the network device usingregister_netdev.
Quoted 7 lines
Quoted 2 lines
I can quote more counter-examples than your list, starting with the Broadcom tg3 driver.
Thank you David for this information. I'll send updated patch as version-2.
Still I couldn't understand the reason behind this. I should be grateful if you would explain me little bit.
I believe one reason is that as soon as you call register_netdev(), the network device notifiers run as part of call_netdevice_notifiers() and it is valid for a notified callback to open your network device. This means that you can call the driver's ndo_open() before you returned from register_netdev(). If netif_carrier_off() is called after register_netdev(), there is a risk for ndo_open() not to get a consistent carrier state, and the actual call to netif_carrier_off() to mess things up.
Signature · 1 line
Florian
David Miller
maintainerFrom: Balakumaran Kannan <[email protected]>
Date: Tue, 3 Jun 2014 19:45:39 +0530
Quoted 7 lines · David Miller
Quoted 6 lines
From: Balakumaran Kannan <[email protected]>
Date: Fri, 30 May 2014 18:35:43 +0530Quoted 2 lines
All these drivers call
netif_carrier_offfunction after registering the network device usingregister_netdev.
Quoted 7 lines
Quoted 2 lines
I can quote more counter-examples than your list, starting with the Broadcom tg3 driver.
Thank you David for this information. I'll send updated patch as version-2.
Still I couldn't understand the reason behind this. I should be grateful if you would explain me little bit.
At the moment you call register_netdev() the device is visible, notifications are sent to userspace, and userland tools can try to bring the interface up and see the incorrect link state, before you do the netif_carrier_off().
Said another way, between the register_netdev() and netif_carrier_off() call, userspace can see the device in an inconsistent state.
Thank you very much Florian and David for great explanation.
Regards,
K.Balakumaran
Quoted 24 lines · David Miller
From: Balakumaran Kannan <[email protected]>
Date: Tue, 3 Jun 2014 19:45:39 +0530Quoted 7 lines · David Miller
Quoted 6 lines
From: Balakumaran Kannan <[email protected]>
Date: Fri, 30 May 2014 18:35:43 +0530Quoted 2 lines
All these drivers call
netif_carrier_offfunction after registering the network device usingregister_netdev.Quoted 7 lines
Quoted 2 lines
I can quote more counter-examples than your list, starting with the Broadcom tg3 driver.
Thank you David for this information. I'll send updated patch as version-2.
Still I couldn't understand the reason behind this. I should be grateful if you would explain me little bit.
At the moment you call
register_netdev()the device is visible, notifications are sent to userspace, and userland tools can try to bring the interface up and see the incorrect link state, before you do thenetif_carrier_off().Said another way, between the
register_netdev()andnetif_carrier_off()call, userspace can see the device in an inconsistent state.