AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
(they are there in 2.6.17.7 too), to fix module initialization
problems appearing with at least some newer Celerons and
Pentium III.
Signed-off-by: Jukka Partanen <[email protected]>
--- drivers/isdn/avmb1/c4.c.orig 2006-08-03 19:32:17.000000000 +0000
+++ drivers/isdn/avmb1/c4.c 2006-08-03 19:32:55.000000000 +0000
@@ -151,6 +151,7 @@
while (c4inmeml(card->mbase+DOORBELL) != 0xffffffff) {
if (!time_before(jiffies, stop))
return -1;
+ mb();
}
return 0;
}
@@ -305,6 +306,7 @@
if (!time_before(jiffies, stop))
return;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
+ mb();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
@@ -328,6 +330,7 @@
if (!time_before(jiffies, stop))
return 2;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
+ mb();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
Yes, that should be go in.
AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
(they are there in 2.6.17.7 too), to fix module initialization
problems appearing with at least some newer Celerons and
Pentium III.
Acked-by: Karsten Keil <[email protected]>
Signed-off-by: Jukka Partanen <[email protected]>
--- drivers/isdn/avmb1/c4.c.orig 2006-08-03 19:32:17.000000000 +0000
+++ drivers/isdn/avmb1/c4.c 2006-08-03 19:32:55.000000000 +0000
@@ -151,6 +151,7 @@
while (c4inmeml(card->mbase+DOORBELL) != 0xffffffff) {
if (!time_before(jiffies, stop))
return -1;
+ mb();
}
return 0;
}
@@ -305,6 +306,7 @@
if (!time_before(jiffies, stop))
return;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
+ mb();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
@@ -328,6 +330,7 @@
if (!time_before(jiffies, stop))
return 2;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
+ mb();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
--
Karsten Keil
SuSE Labs
ISDN development
Ar Iau, 2006-08-03 am 19:53 +0300, ysgrifennodd Jukka Partanen:
> AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
> (they are there in 2.6.17.7 too), to fix module initialization
> problems appearing with at least some newer Celerons and
> Pentium III.
Should be using cpu_relax() I think. Its a polled busy loop so you want
other CPU threads to run if possible. Otherwise the code seems quite
logical depending how c4inmeml is defined.
Alan
On Thu, Aug 03, 2006 at 06:56:15PM +0100, Alan Cox wrote:
> Ar Iau, 2006-08-03 am 19:53 +0300, ysgrifennodd Jukka Partanen:
> > AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
> > (they are there in 2.6.17.7 too), to fix module initialization
> > problems appearing with at least some newer Celerons and
> > Pentium III.
>
> Should be using cpu_relax() I think. Its a polled busy loop so you want
> other CPU threads to run if possible.
Agreed, but I think it should be a second patch since 2.6 would need it
too.
> Otherwise the code seems quite logical depending how c4inmeml is defined.
OK, I'm queuing it for 2.4.34.
> Alan
Willy
On Thu, Aug 03, 2006 at 06:56:15PM +0100, Alan Cox wrote:
> Ar Iau, 2006-08-03 am 19:53 +0300, ysgrifennodd Jukka Partanen:
> > AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
> > (they are there in 2.6.17.7 too), to fix module initialization
> > problems appearing with at least some newer Celerons and
> > Pentium III.
>
> Should be using cpu_relax() I think. Its a polled busy loop so you want
> other CPU threads to run if possible.
You mean like this ? Here's the patch for 2.6, I'll queue the same for 2.4
if it's alright.
> Alan
Regards,
Willy
>From 512d12bd7ce9c0a15dfd91a6f7c2970c92b3abdd Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Fri, 4 Aug 2006 08:50:10 +0200
Subject: [PATCH] AVM C4 ISDN card : use cpu_relax() in busy loops
As suggested by Alan, use cpu_relax() in 3 busy loops : "It's a
polled busy loop so you want other CPU threads to run if possible".
Signed-off-by: Willy Tarreau <[email protected]>
---
drivers/isdn/hardware/avm/c4.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/isdn/hardware/avm/c4.c b/drivers/isdn/hardware/avm/c4.c
index f7253b2..aee278e 100644
--- a/drivers/isdn/hardware/avm/c4.c
+++ b/drivers/isdn/hardware/avm/c4.c
@@ -22,6 +22,7 @@ #include <linux/capi.h>
#include <linux/kernelcapi.h>
#include <linux/init.h>
#include <asm/io.h>
+#include <asm/processor.h>
#include <asm/uaccess.h>
#include <linux/netdevice.h>
#include <linux/isdn/capicmd.h>
@@ -150,6 +151,7 @@ static inline int wait_for_doorbell(avmc
if (!time_before(jiffies, stop))
return -1;
mb();
+ cpu_relax();
}
return 0;
}
@@ -303,6 +305,7 @@ static void c4_reset(avmcard *card)
return;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
mb();
+ cpu_relax();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
@@ -327,6 +330,7 @@ static int c4_detect(avmcard *card)
return 2;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
mb();
+ cpu_relax();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
--
1.4.1
Ar Iau, 2006-08-03 am 19:31 +0200, ysgrifennodd Karsten Keil:
> Yes, that should be go in.
>
> AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
> (they are there in 2.6.17.7 too), to fix module initialization
> problems appearing with at least some newer Celerons and
> Pentium III.
>
> Acked-by: Karsten Keil <[email protected]>
> Signed-off-by: Jukka Partanen <[email protected]>
NAK: Alan Cox <[email protected]>
Two reasons
#1 You should use cpu_relax in such loops
#2 The readl (which c4inmeml is a pointless #define of) is defined to be
a volatile reference itself
That means that the real bug would appear to be different and either you
have a gcc bug which is possible or you have something stranger going
on, such as the continued polling busying the microcontroller the other
end, in which case you need a delay not the lucky chance that mb() is
slowish on some x86 systems.
So you either want
cpu_relax + other fixes
or udelay(something)
Alan
On Friday 04 August 2006 08:56, Willy Tarreau wrote:
> On Thu, Aug 03, 2006 at 06:56:15PM +0100, Alan Cox wrote:
> > Ar Iau, 2006-08-03 am 19:53 +0300, ysgrifennodd Jukka Partanen:
> > > AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
> > > (they are there in 2.6.17.7 too), to fix module initialization
> > > problems appearing with at least some newer Celerons and
> > > Pentium III.
> >
> > Should be using cpu_relax() I think. Its a polled busy loop so you want
> > other CPU threads to run if possible.
>
> You mean like this ? Here's the patch for 2.6, I'll queue the same for 2.4
> if it's alright.
>
> > Alan
>
> Regards,
> Willy
>
> From 512d12bd7ce9c0a15dfd91a6f7c2970c92b3abdd Mon Sep 17 00:00:00 2001
> From: Willy Tarreau <[email protected]>
> Date: Fri, 4 Aug 2006 08:50:10 +0200
> Subject: [PATCH] AVM C4 ISDN card : use cpu_relax() in busy loops
>
> As suggested by Alan, use cpu_relax() in 3 busy loops : "It's a
> polled busy loop so you want other CPU threads to run if possible".
>
> Signed-off-by: Willy Tarreau <[email protected]>
> ---
> drivers/isdn/hardware/avm/c4.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/isdn/hardware/avm/c4.c b/drivers/isdn/hardware/avm/c4.c
> index f7253b2..aee278e 100644
> --- a/drivers/isdn/hardware/avm/c4.c
> +++ b/drivers/isdn/hardware/avm/c4.c
> @@ -22,6 +22,7 @@ #include <linux/capi.h>
> #include <linux/kernelcapi.h>
> #include <linux/init.h>
> #include <asm/io.h>
> +#include <asm/processor.h>
> #include <asm/uaccess.h>
> #include <linux/netdevice.h>
> #include <linux/isdn/capicmd.h>
> @@ -150,6 +151,7 @@ static inline int wait_for_doorbell(avmc
> if (!time_before(jiffies, stop))
> return -1;
> mb();
> + cpu_relax();
cpu_relax() implies a memory barrier.
--
Greetings Michael.
On Fri, Aug 04, 2006 at 12:39:12PM +0200, Michael Buesch wrote:
> On Friday 04 August 2006 08:56, Willy Tarreau wrote:
> > On Thu, Aug 03, 2006 at 06:56:15PM +0100, Alan Cox wrote:
> > > Ar Iau, 2006-08-03 am 19:53 +0300, ysgrifennodd Jukka Partanen:
> > > > AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
> > > > (they are there in 2.6.17.7 too), to fix module initialization
> > > > problems appearing with at least some newer Celerons and
> > > > Pentium III.
> > >
> > > Should be using cpu_relax() I think. Its a polled busy loop so you want
> > > other CPU threads to run if possible.
> >
> > You mean like this ? Here's the patch for 2.6, I'll queue the same for 2.4
> > if it's alright.
> >
> > > Alan
> >
> > Regards,
> > Willy
> >
> > From 512d12bd7ce9c0a15dfd91a6f7c2970c92b3abdd Mon Sep 17 00:00:00 2001
> > From: Willy Tarreau <[email protected]>
> > Date: Fri, 4 Aug 2006 08:50:10 +0200
> > Subject: [PATCH] AVM C4 ISDN card : use cpu_relax() in busy loops
> >
> > As suggested by Alan, use cpu_relax() in 3 busy loops : "It's a
> > polled busy loop so you want other CPU threads to run if possible".
> >
> > Signed-off-by: Willy Tarreau <[email protected]>
> > ---
> > drivers/isdn/hardware/avm/c4.c | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/isdn/hardware/avm/c4.c b/drivers/isdn/hardware/avm/c4.c
> > index f7253b2..aee278e 100644
> > --- a/drivers/isdn/hardware/avm/c4.c
> > +++ b/drivers/isdn/hardware/avm/c4.c
> > @@ -22,6 +22,7 @@ #include <linux/capi.h>
> > #include <linux/kernelcapi.h>
> > #include <linux/init.h>
> > #include <asm/io.h>
> > +#include <asm/processor.h>
> > #include <asm/uaccess.h>
> > #include <linux/netdevice.h>
> > #include <linux/isdn/capicmd.h>
> > @@ -150,6 +151,7 @@ static inline int wait_for_doorbell(avmc
> > if (!time_before(jiffies, stop))
> > return -1;
> > mb();
> > + cpu_relax();
>
> cpu_relax() implies a memory barrier.
OK fine. Reading from various asm-xxx/processor.h files, I was not sure about this.
I will repost it fixed and queue it for next 2.4 too.
> Greetings Michael.
Thanks,
Willy
On Fri, Aug 04, 2006 at 12:39:12PM +0200, Michael Buesch wrote:
> On Friday 04 August 2006 08:56, Willy Tarreau wrote:
> > On Thu, Aug 03, 2006 at 06:56:15PM +0100, Alan Cox wrote:
> > > Ar Iau, 2006-08-03 am 19:53 +0300, ysgrifennodd Jukka Partanen:
> > > > AVM C4 ISDN NIC: Add three memory barriers, taken from 2.6.7,
> > > > (they are there in 2.6.17.7 too), to fix module initialization
> > > > problems appearing with at least some newer Celerons and
> > > > Pentium III.
> > >
> > > Should be using cpu_relax() I think. Its a polled busy loop so you want
> > > other CPU threads to run if possible.
(...)
> cpu_relax() implies a memory barrier.
Ok, here's the updated patch for 2.6 using cpu_relax() instead of
mb(), as suggested by Alan and Michael.
Regards,
Willy
> Greetings Michael.
>From b76136dc1ac989081933d28ec958ecdd32d368e4 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Fri, 4 Aug 2006 22:11:23 +0200
Subject: [PATCH] AVM C4 ISDN card : use cpu_relax() in busy loops
As suggested by Alan, use cpu_relax() in 3 busy loops : "It's a
polled busy loop so you want other CPU threads to run if possible".
Signed-off-by: Willy Tarreau <[email protected]>
---
drivers/isdn/hardware/avm/c4.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/isdn/hardware/avm/c4.c b/drivers/isdn/hardware/avm/c4.c
index f7253b2..8149dce 100644
--- a/drivers/isdn/hardware/avm/c4.c
+++ b/drivers/isdn/hardware/avm/c4.c
@@ -22,6 +22,7 @@ #include <linux/capi.h>
#include <linux/kernelcapi.h>
#include <linux/init.h>
#include <asm/io.h>
+#include <asm/processor.h>
#include <asm/uaccess.h>
#include <linux/netdevice.h>
#include <linux/isdn/capicmd.h>
@@ -149,7 +150,7 @@ static inline int wait_for_doorbell(avmc
while (c4inmeml(card->mbase+DOORBELL) != 0xffffffff) {
if (!time_before(jiffies, stop))
return -1;
- mb();
+ cpu_relax();
}
return 0;
}
@@ -302,7 +303,7 @@ static void c4_reset(avmcard *card)
if (!time_before(jiffies, stop))
return;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
- mb();
+ cpu_relax();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
@@ -326,7 +327,7 @@ static int c4_detect(avmcard *card)
if (!time_before(jiffies, stop))
return 2;
c4outmeml(card->mbase+DOORBELL, DBELL_ADDR);
- mb();
+ cpu_relax();
}
c4_poke(card, DC21285_ARMCSR_BASE + CHAN_1_CONTROL, 0);
--
1.4.1