[PATCH] Don't take the mdio_lock in atl1e_probe

[PATCH] Don't take the mdio_lock in atl1e_probe

[Matthew Wilcox]

Lockdep warns about the mdio_lock taken with interrupts enabled then later
taken from interrupt context.  Initially, I considered changing these
to spin_lock_irq/spin_unlock_irq, but then I looked at atl1e_phy_init()
and saw that it calls msleep().  Sleeping while holding a spinlock is
not allowed either.

In the probe path, we haven't registered the interrupt handler, so
it can't poke at this card yet.  It's before we call register_netdev(),
so I don't think any other threads can reach this card either.  If I'm
right, we don't need a spinlock at all.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c
index 82d7be1..ba22a51 100644
--- a/drivers/net/atl1e/atl1e_main.c
+++ b/drivers/net/atl1e/atl1e_main.c
@@ -2389,9 +2389,7 @@ static int __devinit atl1e_probe(struct pci_dev *pdev,
 	}
 
 	/* Init GPHY as early as possible due to power saving issue  */
-	spin_lock(&adapter->mdio_lock);
 	atl1e_phy_init(&adapter->hw);
-	spin_unlock(&adapter->mdio_lock);
 	/* reset the controller to
 	 * put the device in a known good starting state */
 	err = atl1e_reset_hw(&adapter->hw);

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

RE: [PATCH] Don't take the mdio_lock in atl1e_probe

[Jie Yang]

 On Tuesday, August 12, 2008 9:13 PM
Matthew Wilcox <matthew@wil.cx> wrote
> Lockdep warns about the mdio_lock taken with interrupts
> enabled then later taken from interrupt context.  Initially,
> I considered changing these to spin_lock_irq/spin_unlock_irq,
> but then I looked at atl1e_phy_init() and saw that it calls
> msleep().  Sleeping while holding a spinlock is not allowed either.
>
> In the probe path, we haven't registered the interrupt
> handler, so it can't poke at this card yet.  It's before we
> call register_netdev(), so I don't think any other threads
> can reach this card either.  If I'm right, we don't need a
> spinlock at all.
>
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
>
> diff --git a/drivers/net/atl1e/atl1e_main.c
> b/drivers/net/atl1e/atl1e_main.c index 82d7be1..ba22a51 100644
> --- a/drivers/net/atl1e/atl1e_main.c
> +++ b/drivers/net/atl1e/atl1e_main.c
> @@ -2389,9 +2389,7 @@ static int __devinit atl1e_probe(struct
> pci_dev *pdev,
>         }
>
>         /* Init GPHY as early as possible due to power saving
> issue  */
> -       spin_lock(&adapter->mdio_lock);
>         atl1e_phy_init(&adapter->hw);
> -       spin_unlock(&adapter->mdio_lock);
>         /* reset the controller to
>          * put the device in a known good starting state */
>         err = atl1e_reset_hw(&adapter->hw);
>

Ok.
Thanks Matthew Wilcox

Acked-by: Jie Yang <jie.yang@atheros.com>

Re: [PATCH] Don't take the mdio_lock in atl1e_probe

[Jeff Garzik]

Matthew Wilcox wrote:
> Lockdep warns about the mdio_lock taken with interrupts enabled then later
> taken from interrupt context.  Initially, I considered changing these
> to spin_lock_irq/spin_unlock_irq, but then I looked at atl1e_phy_init()
> and saw that it calls msleep().  Sleeping while holding a spinlock is
> not allowed either.
> 
> In the probe path, we haven't registered the interrupt handler, so
> it can't poke at this card yet.  It's before we call register_netdev(),
> so I don't think any other threads can reach this card either.  If I'm
> right, we don't need a spinlock at all.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> 
> diff --git a/drivers/net/atl1e/atl1e_main.c b/drivers/net/atl1e/atl1e_main.c
> index 82d7be1..ba22a51 100644
> --- a/drivers/net/atl1e/atl1e_main.c
> +++ b/drivers/net/atl1e/atl1e_main.c
> @@ -2389,9 +2389,7 @@ static int __devinit atl1e_probe(struct pci_dev *pdev,
>  	}
>  
>  	/* Init GPHY as early as possible due to power saving issue  */
> -	spin_lock(&adapter->mdio_lock);
>  	atl1e_phy_init(&adapter->hw);
> -	spin_unlock(&adapter->mdio_lock);
>  	/* reset the controller to
>  	 * put the device in a known good starting state */

applied