[PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Ivan Vecera]

Without calling of netif_carrier_off at the end of the probe the operstate
is unknown when the device is initially opened. By default the carrier is
on so when the device is opened and netif_carrier_on is called the link
watch event is not fired and operstate remains zero (unknown).

This patch fixes this behavior in forcedeth and r8169.

Signed-off-by: Ivan Vecera <ivecera@redhat.com>
---
 drivers/net/forcedeth.c |    2 ++
 drivers/net/r8169.c     |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index af09296..9c0b1ba 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -5645,6 +5645,8 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 		goto out_error;
 	}
 
+	netif_carrier_off(dev);
+
 	dev_info(&pci_dev->dev, "ifname %s, PHY OUI 0x%x @ %d, addr %pM\n",
 		 dev->name, np->phy_oui, np->phyaddr, dev->dev_addr);
 
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 59ccf0c..469ab0b 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3190,6 +3190,8 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (pci_dev_run_wake(pdev))
 		pm_runtime_put_noidle(&pdev->dev);
 
+	netif_carrier_off(dev);
+
 out:
 	return rc;
 
-- 
1.7.3.4

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Francois Romieu]

Ivan Vecera <ivecera@redhat.com> :
> Without calling of netif_carrier_off at the end of the probe the operstate
> is unknown when the device is initially opened. By default the carrier is
> on so when the device is opened and netif_carrier_on is called the link
> watch event is not fired and operstate remains zero (unknown).

Stated this way it sounds like a core dev layer issue.

I am not completely sure after reading some history. Namely:
- (37e8273cd30592d3a82bcb70cbb1bdc4eaeb6b71 ?)
- c276e098d3ee33059b4a1c747354226cec58487c 
- 22604c866889c4b2e12b73cbf1683bda1b72a313
- b47300168e770b60ab96c8924854c3b0eb4260eb

I am confused.

-- 
Ueimor

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Ben Hutchings]

On Tue, 2011-02-15 at 16:22 +0100, Francois Romieu wrote:
> Ivan Vecera <ivecera@redhat.com> :
> > Without calling of netif_carrier_off at the end of the probe the operstate
> > is unknown when the device is initially opened. By default the carrier is
> > on so when the device is opened and netif_carrier_on is called the link
> > watch event is not fired and operstate remains zero (unknown).
> 
> Stated this way it sounds like a core dev layer issue.

Due to hardware limitations, some network drivers cannot report the
carrier state and they never call netif_carrier_{on,off}().  Therefore
the initial operstate of 'unknown' is correct.

> I am not completely sure after reading some history. Namely:
> - (37e8273cd30592d3a82bcb70cbb1bdc4eaeb6b71 ?)
> - c276e098d3ee33059b4a1c747354226cec58487c 
> - 22604c866889c4b2e12b73cbf1683bda1b72a313
> - b47300168e770b60ab96c8924854c3b0eb4260eb
> 
> I am confused.

Drivers that can report carrier state should do so initially some time
between registering a device and bringing it up (either in the bus probe
function or the ndo_open function).  It generally seems to be safe to
assume that the link is down initially, and then to rely on
notifications from the hardware.  However, that does depend on the
behaviour of the hardware.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Ivan Vecera]

----- Original Message -----
> On Tue, 2011-02-15 at 16:22 +0100, Francois Romieu wrote:
> > Stated this way it sounds like a core dev layer issue.
> 
> ...
> > I am not completely sure after reading some history. Namely:
> > - (37e8273cd30592d3a82bcb70cbb1bdc4eaeb6b71 ?)
> > - c276e098d3ee33059b4a1c747354226cec58487c
> > - 22604c866889c4b2e12b73cbf1683bda1b72a313
> > - b47300168e770b60ab96c8924854c3b0eb4260eb
> >
> > I am confused.
> 
> Drivers that can report carrier state should do so initially some time
> between registering a device and bringing it up (either in the bus
> probe
> function or the ndo_open function). It generally seems to be safe to
> assume that the link is down initially, and then to rely on
> notifications from the hardware. However, that does depend on the
> behaviour of the hardware.
> 
Yes,that's true... forcedeth and r8169 are the drivers that detect link
state when device is opened and call netif_carrier_on(off) appropriately.

Ivan

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Ivan Vecera]

On Tue, 2011-02-15 at 13:01 -0500, Ivan Vecera wrote:
> ----- Original Message -----
> > On Tue, 2011-02-15 at 16:22 +0100, Francois Romieu wrote:
> > > Stated this way it sounds like a core dev layer issue.
> > 
> > ...
> > > I am not completely sure after reading some history. Namely:
> > > - (37e8273cd30592d3a82bcb70cbb1bdc4eaeb6b71 ?)
> > > - c276e098d3ee33059b4a1c747354226cec58487c
> > > - 22604c866889c4b2e12b73cbf1683bda1b72a313
> > > - b47300168e770b60ab96c8924854c3b0eb4260eb
> > >
> > > I am confused.
> > 
> > Drivers that can report carrier state should do so initially some time
> > between registering a device and bringing it up (either in the bus
> > probe
> > function or the ndo_open function). It generally seems to be safe to
> > assume that the link is down initially, and then to rely on
> > notifications from the hardware. However, that does depend on the
> > behaviour of the hardware.
> > 
> Yes,that's true... forcedeth and r8169 are the drivers that detect link
> state when device is opened and call netif_carrier_on(off) appropriately.
> 
> Ivan
Dave, Francois, is it acceptable?

Ivan

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Francois Romieu]

Ivan Vecera <ivecera@redhat.com> :
[...]
> Dave, Francois, is it acceptable?

Apparently both are acceptable. :o/

As a mildly convinced coward, I can not tell if it will help more applications
than it will break.

-- 
Ueimor

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Ivan Vecera]

On Thu, 2011-02-17 at 11:19 +0100, Francois Romieu wrote:
> Ivan Vecera <ivecera@redhat.com> :
> [...]
> > Dave, Francois, is it acceptable?
> 
> Apparently both are acceptable. :o/
> 
> As a mildly convinced coward, I can not tell if it will help more applications
> than it will break.
> 
It should not break any application as the patch only avoids the
operstate to be 'unknown' instead of 'up'.

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[Francois Romieu]

Ivan Vecera <ivecera@redhat.com> :
[...]
> It should not break any application as the patch only avoids the
> operstate to be 'unknown' instead of 'up'.

Apologies, I missed this point. It does not turn 'down' at boot into
'unknown'.

Acked-by: Francois Romieu <romieu@fr.zoreil.com>

-- 
Ueimor

Re: [PATCH] drivers/net: Call netif_carrier_off at the end of the probe

[David Miller]

From: Ivan Vecera <ivecera@redhat.com>
Date: Tue, 15 Feb 2011 13:08:39 +0100

> Without calling of netif_carrier_off at the end of the probe the operstate
> is unknown when the device is initially opened. By default the carrier is
> on so when the device is opened and netif_carrier_on is called the link
> watch event is not fired and operstate remains zero (unknown).
> 
> This patch fixes this behavior in forcedeth and r8169.
> 
> Signed-off-by: Ivan Vecera <ivecera@redhat.com>

Applied, thanks Ivan.