Patchwork am79c961a: fix spin_lock usage

login
register
mail settings
Submitter Uwe Kleine-König
Date Jan. 22, 2009, 10:29 p.m.
Message ID <1232663382-10503-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/19909/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Uwe Kleine-König - Jan. 22, 2009, 10:29 p.m.
spin_lock functions take a pointer to the lock, not the lock itself.
This error was noticed by compiling ebsa110_defconfig for linux-rt where
the locking functions obviously are more picky about their arguments.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Roel Kluin <12o3l@tiscali.nl>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Steven Rostedt <srostedt@redhat.com>
Cc: netdev@vger.kernel.org
---
Hello,

unrelated to that, with arm-linux-gnu-gcc 4.3.2 I get some warnings when
compiling am79c961a.c:

/tmp/ccAfRLG9.s: Assembler messages:
/tmp/ccAfRLG9.s:77: Warning: register range not in ascending order
/tmp/ccAfRLG9.s:615: Warning: register range not in ascending order
/tmp/ccAfRLG9.s:1242: Warning: register range not in ascending order

These correspond to lines 107[1] and 144 in am79c961a.c where inline
assembly is used.  E.g

	stm%?ia        %1!, {%2, %3}

so the order depends on the compilers choice for %2 and %3.

Best regards
Uwe

[1] 77 and 615 in the .s file both correspond to 107 in am79c961a.c.

 drivers/net/arm/am79c961a.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)
Russell King - ARM Linux - Jan. 22, 2009, 10:34 p.m.
On Thu, Jan 22, 2009 at 11:29:42PM +0100, Uwe Kleine-König wrote:
> unrelated to that, with arm-linux-gnu-gcc 4.3.2 I get some warnings when
> compiling am79c961a.c:

Don't care about gcc 4.3.2 - it's broken for the ARM architecture.
--
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
Uwe Kleine-König - Jan. 22, 2009, 10:44 p.m.
On Thu, Jan 22, 2009 at 10:34:12PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 22, 2009 at 11:29:42PM +0100, Uwe Kleine-König wrote:
> > unrelated to that, with arm-linux-gnu-gcc 4.3.2 I get some warnings when
> > compiling am79c961a.c:
> 
> Don't care about gcc 4.3.2 - it's broken for the ARM architecture.
OK.  I just read that on linux-arm-kernel (up to now I didn't have
problems).  Nevertheless the warnings are valid and the patch fixes a
real (and different) problem.

Best regards
Uwe
David Miller - Jan. 22, 2009, 10:56 p.m.
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Date: Thu, 22 Jan 2009 23:29:42 +0100

> spin_lock functions take a pointer to the lock, not the lock itself.
> This error was noticed by compiling ebsa110_defconfig for linux-rt where
> the locking functions obviously are more picky about their arguments.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Russell, this fix is real.  Want me to take it or will you?

BTW, I took linux-arm-kernel out of the CC because all of my postings
there bounce simply because I lack reverse DNS :-/
--
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
Russell King - Jan. 22, 2009, 11:40 p.m.
On Thu, Jan 22, 2009 at 02:56:15PM -0800, David Miller wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Date: Thu, 22 Jan 2009 23:29:42 +0100
> 
> > spin_lock functions take a pointer to the lock, not the lock itself.
> > This error was noticed by compiling ebsa110_defconfig for linux-rt where
> > the locking functions obviously are more picky about their arguments.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> Russell, this fix is real.  Want me to take it or will you?

I'll take it.

> BTW, I took linux-arm-kernel out of the CC because all of my postings
> there bounce simply because I lack reverse DNS :-/

No need; in theory I've whitelisted your IP.  However, I've dropped
12o3l@tiscali.nl from the CC because that address bounces.
David Miller - Jan. 23, 2009, 12:16 a.m.
From: Russell King <rmk@arm.linux.org.uk>
Date: Thu, 22 Jan 2009 23:40:36 +0000

> On Thu, Jan 22, 2009 at 02:56:15PM -0800, David Miller wrote:
> > From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Date: Thu, 22 Jan 2009 23:29:42 +0100
> > 
> > > spin_lock functions take a pointer to the lock, not the lock itself.
> > > This error was noticed by compiling ebsa110_defconfig for linux-rt where
> > > the locking functions obviously are more picky about their arguments.
> > > 
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 
> > Russell, this fix is real.  Want me to take it or will you?
> 
> I'll take it.

Great.

> > BTW, I took linux-arm-kernel out of the CC because all of my postings
> > there bounce simply because I lack reverse DNS :-/
> 
> No need; in theory I've whitelisted your IP.  However, I've dropped
> 12o3l@tiscali.nl from the CC because that address bounces.

Thanks, I noticed him bounce too.
--
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
Robert Schwebel - Jan. 23, 2009, 8:43 p.m.
On Thu, Jan 22, 2009 at 10:34:12PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 22, 2009 at 11:29:42PM +0100, Uwe Kleine-König wrote:
> > unrelated to that, with arm-linux-gnu-gcc 4.3.2 I get some warnings when
> > compiling am79c961a.c:
>
> Don't care about gcc 4.3.2 - it's broken for the ARM architecture.

Any pointer?

rsc
Uwe Kleine-König - Jan. 23, 2009, 9:15 p.m.
Hello Robert,

On Fri, Jan 23, 2009 at 09:43:18PM +0100, Robert Schwebel wrote:
> On Thu, Jan 22, 2009 at 10:34:12PM +0000, Russell King - ARM Linux wrote:
> > On Thu, Jan 22, 2009 at 11:29:42PM +0100, Uwe Kleine-König wrote:
> > > unrelated to that, with arm-linux-gnu-gcc 4.3.2 I get some warnings when
> > > compiling am79c961a.c:
> >
> > Don't care about gcc 4.3.2 - it's broken for the ARM architecture.
> 
> Any pointer?
I recently saw:

http://thread.gmane.org/gmane.linux.ports.arm.omap/15540/focus=15557
http://thread.gmane.org/gmane.linux.ports.arm.kernel/47539/focus=51934

Best regards
Uwe

Patch

diff --git a/drivers/net/arm/am79c961a.c b/drivers/net/arm/am79c961a.c
index 0c628a9..c2d012f 100644
--- a/drivers/net/arm/am79c961a.c
+++ b/drivers/net/arm/am79c961a.c
@@ -208,9 +208,9 @@  am79c961_init_for_open(struct net_device *dev)
 	/*
 	 * Stop the chip.
 	 */
-	spin_lock_irqsave(priv->chip_lock, flags);
+	spin_lock_irqsave(&priv->chip_lock, flags);
 	write_rreg (dev->base_addr, CSR0, CSR0_BABL|CSR0_CERR|CSR0_MISS|CSR0_MERR|CSR0_TINT|CSR0_RINT|CSR0_STOP);
-	spin_unlock_irqrestore(priv->chip_lock, flags);
+	spin_unlock_irqrestore(&priv->chip_lock, flags);
 
 	write_ireg (dev->base_addr, 5, 0x00a0); /* Receive address LED */
 	write_ireg (dev->base_addr, 6, 0x0081); /* Collision LED */
@@ -332,10 +332,10 @@  am79c961_close(struct net_device *dev)
 	netif_stop_queue(dev);
 	netif_carrier_off(dev);
 
-	spin_lock_irqsave(priv->chip_lock, flags);
+	spin_lock_irqsave(&priv->chip_lock, flags);
 	write_rreg (dev->base_addr, CSR0, CSR0_STOP);
 	write_rreg (dev->base_addr, CSR3, CSR3_MASKALL);
-	spin_unlock_irqrestore(priv->chip_lock, flags);
+	spin_unlock_irqrestore(&priv->chip_lock, flags);
 
 	free_irq (dev->irq, dev);
 
@@ -391,7 +391,7 @@  static void am79c961_setmulticastlist (struct net_device *dev)
 			am79c961_mc_hash(dmi, multi_hash);
 	}
 
-	spin_lock_irqsave(priv->chip_lock, flags);
+	spin_lock_irqsave(&priv->chip_lock, flags);
 
 	stopped = read_rreg(dev->base_addr, CSR0) & CSR0_STOP;
 
@@ -405,9 +405,9 @@  static void am79c961_setmulticastlist (struct net_device *dev)
 		 * Spin waiting for chip to report suspend mode
 		 */
 		while ((read_rreg(dev->base_addr, CTRL1) & CTRL1_SPND) == 0) {
-			spin_unlock_irqrestore(priv->chip_lock, flags);
+			spin_unlock_irqrestore(&priv->chip_lock, flags);
 			nop();
-			spin_lock_irqsave(priv->chip_lock, flags);
+			spin_lock_irqsave(&priv->chip_lock, flags);
 		}
 	}
 
@@ -429,7 +429,7 @@  static void am79c961_setmulticastlist (struct net_device *dev)
 		write_rreg(dev->base_addr, CTRL1, 0);
 	}
 
-	spin_unlock_irqrestore(priv->chip_lock, flags);
+	spin_unlock_irqrestore(&priv->chip_lock, flags);
 }
 
 static void am79c961_timeout(struct net_device *dev)
@@ -467,10 +467,10 @@  am79c961_sendpacket(struct sk_buff *skb, struct net_device *dev)
 	am_writeword (dev, hdraddr + 2, TMD_OWN|TMD_STP|TMD_ENP);
 	priv->txhead = head;
 
-	spin_lock_irqsave(priv->chip_lock, flags);
+	spin_lock_irqsave(&priv->chip_lock, flags);
 	write_rreg (dev->base_addr, CSR0, CSR0_TDMD|CSR0_IENA);
 	dev->trans_start = jiffies;
-	spin_unlock_irqrestore(priv->chip_lock, flags);
+	spin_unlock_irqrestore(&priv->chip_lock, flags);
 
 	/*
 	 * If the next packet is owned by the ethernet device,