| Message ID | 1401836138-13253-1-git-send-email-festevam@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
2014-06-03 15:55 GMT-07:00 Fabio Estevam <festevam@gmail.com>: > From: Fabio Estevam <fabio.estevam@freescale.com> > > Quoting David Miller: > "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." > > So call netif_carrier_off() prior to register_netdev(). Potentially all drivers implementing libphy correctly should be fixed to start with a carrier off until the phy library has determined otherwise. > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> > --- > This is untested > > drivers/net/ethernet/freescale/gianfar.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c > index a7fe6a7..861e0c9 100644 > --- a/drivers/net/ethernet/freescale/gianfar.c > +++ b/drivers/net/ethernet/freescale/gianfar.c > @@ -1384,6 +1384,9 @@ static int gfar_probe(struct platform_device *ofdev) > > gfar_hw_init(priv); > > + /* Carrier starts down, phylib will bring it up */ > + netif_carrier_off(dev); > + > err = register_netdev(dev); > > if (err) { > @@ -1391,9 +1394,6 @@ static int gfar_probe(struct platform_device *ofdev) > goto register_fail; > } > > - /* Carrier starts down, phylib will bring it up */ > - netif_carrier_off(dev); > - > device_init_wakeup(&dev->dev, > priv->device_flags & > FSL_GIANFAR_DEV_HAS_MAGIC_PACKET); > -- > 1.8.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Florian Fainelli <f.fainelli@gmail.com> Date: Tue, 3 Jun 2014 16:04:49 -0700 > 2014-06-03 15:55 GMT-07:00 Fabio Estevam <festevam@gmail.com>: >> From: Fabio Estevam <fabio.estevam@freescale.com> >> >> Quoting David Miller: >> "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." >> >> So call netif_carrier_off() prior to register_netdev(). > > Potentially all drivers implementing libphy correctly should be fixed > to start with a carrier off until the phy library has determined > otherwise. Are you saying we should hold off on this gianfar patch? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le 03/06/2014 19:05, David Miller a écrit : > From: Florian Fainelli <f.fainelli@gmail.com> > Date: Tue, 3 Jun 2014 16:04:49 -0700 > >> 2014-06-03 15:55 GMT-07:00 Fabio Estevam <festevam@gmail.com>: >>> From: Fabio Estevam <fabio.estevam@freescale.com> >>> >>> Quoting David Miller: >>> "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." >>> >>> So call netif_carrier_off() prior to register_netdev(). >> >> Potentially all drivers implementing libphy correctly should be fixed >> to start with a carrier off until the phy library has determined >> otherwise. > > Are you saying we should hold off on this gianfar patch? Not at all, I think this patch is fine. If Fabio wants/can audit the other Freescale drivers I think this should be beneficial. -- Florian -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 3, 2014 at 11:42 PM, Florian Fainelli <f.fainelli@gmail.com> wrote: > Not at all, I think this patch is fine. If Fabio wants/can audit the other > Freescale drivers I think this should be beneficial. Yes, I have checked the other Freescale net drivers and the only one with this particular problem was gianfar. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Fabio Estevam <festevam@gmail.com> Date: Tue, 3 Jun 2014 19:55:38 -0300 > From: Fabio Estevam <fabio.estevam@freescale.com> > > Quoting David Miller: > "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." > > So call netif_carrier_off() prior to register_netdev(). > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com> Applied, thank you. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c index a7fe6a7..861e0c9 100644 --- a/drivers/net/ethernet/freescale/gianfar.c +++ b/drivers/net/ethernet/freescale/gianfar.c @@ -1384,6 +1384,9 @@ static int gfar_probe(struct platform_device *ofdev) gfar_hw_init(priv); + /* Carrier starts down, phylib will bring it up */ + netif_carrier_off(dev); + err = register_netdev(dev); if (err) { @@ -1391,9 +1394,6 @@ static int gfar_probe(struct platform_device *ofdev) goto register_fail; } - /* Carrier starts down, phylib will bring it up */ - netif_carrier_off(dev); - device_init_wakeup(&dev->dev, priv->device_flags & FSL_GIANFAR_DEV_HAS_MAGIC_PACKET);