Re: Please revert "fix typo in geode_configre()@cyrix.c"
Hi. I'm late.
I'll to resend the patch against 2.6.19.
original code doesn't write back to CCR4 register. this patch reflects a
value of a register.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-02-03 14:57:35.0
+0900
@@ -161,19 +161,19 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
+ u8 ccr3;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* enable MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x38);
+ setCx86(CX86_CCR3, ccr3); /* disable MAPEN */
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +415,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO "Enabling CPUID on Cyrix
processor.\n");
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
On Fri, 2 Feb 2007 13:18:54 -0800
Andrew Morton <[EMAIL PROTECTED]> wrote:
> On Fri, 2 Feb 2007 10:12:36 -0500
> [EMAIL PROTECTED] (Lennart Sorensen) wrote:
>
> > On Fri, Feb 02, 2007 at 12:05:43AM -0800, Andrew Morton wrote:
> > > On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk <[EMAIL PROTECTED]> wrote:
> > >
> > > > Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba
> > > >
> > >
> > > Yup.
> > >
> > > That discussion seems to have died. The 2.6.19 code looks rather silly,
> > > but
> > > presumably it passed someone's testing at some stage.
> >
> > The discussion ended because the last patch seemed to be correct to
> > everyone involved in the discussion. At least that is my understanding.
> > Of course I am just one of the users affected by the patch.
>
> The discussion ended with me asking for someone to send a patch. That
> hasn't happened yet. I don't want to have to troll through 20-30 messages
> and try to work out what patch we ended up with - that's the way in which
> mistakes occur.
>
> Linus has now reverted e4f0ae0ea63caceff37a13f281a72652b7ea71ba. Now,
> please, could someone send a patch against either current -git or against
> 2.6.19? One which includes a descriptin of what it does, and why.
>
> Thanks.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
TAKADA <[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert "fix typo in geode_configre()@cyrix.c"
On Fri, 2 Feb 2007 10:12:36 -0500 [EMAIL PROTECTED] (Lennart Sorensen) wrote: > On Fri, Feb 02, 2007 at 12:05:43AM -0800, Andrew Morton wrote: > > On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > > > Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba > > > > > > > Yup. > > > > That discussion seems to have died. The 2.6.19 code looks rather silly, but > > presumably it passed someone's testing at some stage. > > The discussion ended because the last patch seemed to be correct to > everyone involved in the discussion. At least that is my understanding. > Of course I am just one of the users affected by the patch. The discussion ended with me asking for someone to send a patch. That hasn't happened yet. I don't want to have to troll through 20-30 messages and try to work out what patch we ended up with - that's the way in which mistakes occur. Linus has now reverted e4f0ae0ea63caceff37a13f281a72652b7ea71ba. Now, please, could someone send a patch against either current -git or against 2.6.19? One which includes a descriptin of what it does, and why. Thanks. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert "fix typo in geode_configre()@cyrix.c"
On Fri, Feb 02, 2007 at 12:05:43AM -0800, Andrew Morton wrote: > On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba > > > > Yup. > > That discussion seems to have died. The 2.6.19 code looks rather silly, but > presumably it passed someone's testing at some stage. The discussion ended because the last patch seemed to be correct to everyone involved in the discussion. At least that is my understanding. Of course I am just one of the users affected by the patch. -- Len Sorensen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert "fix typo in geode_configre()@cyrix.c"
On Fri, Feb 02, 2007 at 07:29:41AM +0100, Adrian Bunk wrote: > Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba > > Rationale: > > The thread starting with http://lkml.org/lkml/2007/1/9/63 contains > a discussion about what might be a correct patch. > > Somehow the first patch of this discussion made it into 2.6.20-rc, but > the ones considered more correct are later in the thread. > > Since there is AFAIK no actually observed problem fixed by this, it > should be safe to simply revert the patch for 2.6.20. True, the bits that are not being enabled are all performance related. Getting the correct patch it should theoretically make systems with this cpu series run faster. -- Len Sorensen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert "fix typo in geode_configre()@cyrix.c"
On Friday 02 February 2007 07:29, Adrian Bunk wrote: > Since there is AFAIK no actually observed problem fixed by this, it > should be safe to simply revert the patch for 2.6.20. The most important fix was: Index: linux-2.6.19/arch/i386/kernel/cpu/cyrix.c === --- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c @@ -173,6 +173,7 @@ static void __cpuinit geode_configure(vo ccr4 = getCx86(CX86_CCR4); ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ + setCx86(CX86_CCR4, ccr4); setCx86(CX86_CCR3, ccr3); set_cx86_memwb(); to write back the modified register content. But yes, it works also without this fix (but faster with it). Juergen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert "fix typo in geode_configre()@cyrix.c"
On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk <[EMAIL PROTECTED]> wrote: > Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba > Yup. That discussion seems to have died. The 2.6.19 code looks rather silly, but presumably it passed someone's testing at some stage. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert fix typo in geode_configre()@cyrix.c
On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk [EMAIL PROTECTED] wrote: Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba Yup. That discussion seems to have died. The 2.6.19 code looks rather silly, but presumably it passed someone's testing at some stage. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert fix typo in geode_configre()@cyrix.c
On Friday 02 February 2007 07:29, Adrian Bunk wrote: Since there is AFAIK no actually observed problem fixed by this, it should be safe to simply revert the patch for 2.6.20. The most important fix was: Index: linux-2.6.19/arch/i386/kernel/cpu/cyrix.c === --- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c @@ -173,6 +173,7 @@ static void __cpuinit geode_configure(vo ccr4 = getCx86(CX86_CCR4); ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ + setCx86(CX86_CCR4, ccr4); setCx86(CX86_CCR3, ccr3); set_cx86_memwb(); to write back the modified register content. But yes, it works also without this fix (but faster with it). Juergen - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert fix typo in geode_configre()@cyrix.c
On Fri, Feb 02, 2007 at 07:29:41AM +0100, Adrian Bunk wrote: Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba Rationale: The thread starting with http://lkml.org/lkml/2007/1/9/63 contains a discussion about what might be a correct patch. Somehow the first patch of this discussion made it into 2.6.20-rc, but the ones considered more correct are later in the thread. Since there is AFAIK no actually observed problem fixed by this, it should be safe to simply revert the patch for 2.6.20. True, the bits that are not being enabled are all performance related. Getting the correct patch it should theoretically make systems with this cpu series run faster. -- Len Sorensen - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert fix typo in geode_configre()@cyrix.c
On Fri, Feb 02, 2007 at 12:05:43AM -0800, Andrew Morton wrote: On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk [EMAIL PROTECTED] wrote: Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba Yup. That discussion seems to have died. The 2.6.19 code looks rather silly, but presumably it passed someone's testing at some stage. The discussion ended because the last patch seemed to be correct to everyone involved in the discussion. At least that is my understanding. Of course I am just one of the users affected by the patch. -- Len Sorensen - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert fix typo in geode_configre()@cyrix.c
On Fri, 2 Feb 2007 10:12:36 -0500 [EMAIL PROTECTED] (Lennart Sorensen) wrote: On Fri, Feb 02, 2007 at 12:05:43AM -0800, Andrew Morton wrote: On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk [EMAIL PROTECTED] wrote: Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba Yup. That discussion seems to have died. The 2.6.19 code looks rather silly, but presumably it passed someone's testing at some stage. The discussion ended because the last patch seemed to be correct to everyone involved in the discussion. At least that is my understanding. Of course I am just one of the users affected by the patch. The discussion ended with me asking for someone to send a patch. That hasn't happened yet. I don't want to have to troll through 20-30 messages and try to work out what patch we ended up with - that's the way in which mistakes occur. Linus has now reverted e4f0ae0ea63caceff37a13f281a72652b7ea71ba. Now, please, could someone send a patch against either current -git or against 2.6.19? One which includes a descriptin of what it does, and why. Thanks. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Please revert fix typo in geode_configre()@cyrix.c
Hi. I'm late.
I'll to resend the patch against 2.6.19.
original code doesn't write back to CCR4 register. this patch reflects a
value of a register.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-02-03 14:57:35.0
+0900
@@ -161,19 +161,19 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
+ u8 ccr3;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* enable MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x38);
+ setCx86(CX86_CCR3, ccr3); /* disable MAPEN */
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +415,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO Enabling CPUID on Cyrix
processor.\n);
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
On Fri, 2 Feb 2007 13:18:54 -0800
Andrew Morton [EMAIL PROTECTED] wrote:
On Fri, 2 Feb 2007 10:12:36 -0500
[EMAIL PROTECTED] (Lennart Sorensen) wrote:
On Fri, Feb 02, 2007 at 12:05:43AM -0800, Andrew Morton wrote:
On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk [EMAIL PROTECTED] wrote:
Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba
Yup.
That discussion seems to have died. The 2.6.19 code looks rather silly,
but
presumably it passed someone's testing at some stage.
The discussion ended because the last patch seemed to be correct to
everyone involved in the discussion. At least that is my understanding.
Of course I am just one of the users affected by the patch.
The discussion ended with me asking for someone to send a patch. That
hasn't happened yet. I don't want to have to troll through 20-30 messages
and try to work out what patch we ended up with - that's the way in which
mistakes occur.
Linus has now reverted e4f0ae0ea63caceff37a13f281a72652b7ea71ba. Now,
please, could someone send a patch against either current -git or against
2.6.19? One which includes a descriptin of what it does, and why.
Thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
TAKADA [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Please revert "fix typo in geode_configre()@cyrix.c"
Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba Rationale: The thread starting with http://lkml.org/lkml/2007/1/9/63 contains a discussion about what might be a correct patch. Somehow the first patch of this discussion made it into 2.6.20-rc, but the ones considered more correct are later in the thread. Since there is AFAIK no actually observed problem fixed by this, it should be safe to simply revert the patch for 2.6.20. TIA Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Please revert fix typo in geode_configre()@cyrix.c
Linus, please revert commit e4f0ae0ea63caceff37a13f281a72652b7ea71ba Rationale: The thread starting with http://lkml.org/lkml/2007/1/9/63 contains a discussion about what might be a correct patch. Somehow the first patch of this discussion made it into 2.6.20-rc, but the ones considered more correct are later in the thread. Since there is AFAIK no actually observed problem fixed by this, it should be safe to simply revert the patch for 2.6.20. TIA Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Wed, 17 Jan 2007 02:12:42 +0900 (JST)
takada <[EMAIL PROTECTED]> wrote:
> From: [EMAIL PROTECTED] (Lennart Sorensen)
> Subject: Re: fix typo in geode_configre()@cyrix.c
> Date: Tue, 16 Jan 2007 11:50:07 -0500
>
> > On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
> > > You are right. I agree to your comment. These variables are needless.
> > > I made a patch again.
> >
> > Of course there are also lots of "magic numbers" around, but I must
> > admit I don't personally really feel like going through the data sheet
> > and naming all of them.
>
> Me too.
>
> >
> > > diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
> > > linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
> > > --- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c2006-11-30
> > > 06:57:37.0 +0900
> > > +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16
> > > 19:55:05.0 +0900
> > > @@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
> > > static void __cpuinit geode_configure(void)
> > > {
> > > unsigned long flags;
> > > - u8 ccr3, ccr4;
> > > local_irq_save(flags);
> > >
> > > /* Suspend on halt power saving and enable #SUSP pin */
> > > setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
> > >
> > > - ccr3 = getCx86(CX86_CCR3);
> > > - setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */
> > > + setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10); /* Enable */
> > >
> > > - ccr4 = getCx86(CX86_CCR4);
> > > - ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
> > > -
> > > - setCx86(CX86_CCR3, ccr3);
> > > + /* FPU fast, DTE cache, Mem bypass */
> > > + setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
> >
> > Why did that change from 0x38 to 0x30?
>
> ah... I made mistake. My eye is tired.
> Correctry, 0x38.
>
I'm lost. Could someone please prepare a complete new patch against the
latest -linus tree?
Thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Wed, 17 Jan 2007 02:12:42 +0900 (JST)
takada [EMAIL PROTECTED] wrote:
From: [EMAIL PROTECTED] (Lennart Sorensen)
Subject: Re: fix typo in geode_configre()@cyrix.c
Date: Tue, 16 Jan 2007 11:50:07 -0500
On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
You are right. I agree to your comment. These variables are needless.
I made a patch again.
Of course there are also lots of magic numbers around, but I must
admit I don't personally really feel like going through the data sheet
and naming all of them.
Me too.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16
19:55:05.0 +0900
@@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
- ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10); /* Enable */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
Why did that change from 0x38 to 0x30?
ah... I made mistake. My eye is tired.
Correctry, 0x38.
I'm lost. Could someone please prepare a complete new patch against the
latest -linus tree?
Thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
Hi. Thanks Sorensen and Juergen.
I overlooked the restored to CCR3.
The bit4(0x10, MAPEN) of CCR3 is necessary to access advanced configuration
registers.
I'll repost patch.
- restore CCR3
- fix use 0x30 instead of 0x38.
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-17 16:15:40.0
+0900
@@ -161,19 +161,18 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
+ u8 ccr3;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* enable MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x38);
+ setCx86(CX86_CCR3, ccr3); /* disable MAPEN */
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +414,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO "Enabling CPUID on Cyrix
processor.\n");
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
Hi. Thanks Sorensen and Juergen.
I overlooked the restored to CCR3.
The bit4(0x10, MAPEN) of CCR3 is necessary to access advanced configuration
registers.
I'll repost patch.
- restore CCR3
- fix use 0x30 instead of 0x38.
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-17 16:15:40.0
+0900
@@ -161,19 +161,18 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
+ u8 ccr3;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* enable MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x38);
+ setCx86(CX86_CCR3, ccr3); /* disable MAPEN */
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +414,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO Enabling CPUID on Cyrix
processor.\n);
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tuesday 09 January 2007 18:33, Lennart Sorensen wrote: > Then for the next one it does: > ccr3 = GetCx86(CX86_CCR3); > setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); > > Couldn't that have been: > setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10); > > No temp variable, and again it clearly does not intend to restore the > value again later (even though the bug actually did cause the value to > be restored by accident). No, ccr3 should be restored to protect some registers (or at least bit 4 should be cleared in ccr3). BTW: In function set_cx86_inc() [...] /* PCR1 -- Performance Control */ /* Incrementor on, whatever that is */ setCx86(CX86_PCR1, getCx86(CX86_PCR1) | 0x02); /* PCR0 -- Performance Control */ /* Incrementor Margin 10 */ setCx86(CX86_PCR0, getCx86(CX86_PCR0) | 0x04); [...] This setting is only valid for 200MHz...266MHz CPUs, for 300MHz and 333MHz CPUs the Incrementor Margin should be 1-1. There is an AppNote about this setting: AMD Geode GX1 Processor Memory Timings for Maximum Performance. Juergen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tuesday 09 January 2007 16:43, Lennart Sorensen wrote: > On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote: > > In kernel 2.6, write back wrong register when configure Geode processor. > > Instead of storing to CCR4, it stores to CCR3. > > > > --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig 2007-01-09 > > 16:45:21.0 +0900 +++ > > linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0 > > +0900 @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo > > ccr4 = getCx86(CX86_CCR4); > > ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ > > > > - setCx86(CX86_CCR3, ccr3); > > + setCx86(CX86_CCR4, ccr4); > > > > set_cx86_memwb(); > > set_cx86_reorder(); > > Any idea what the consequence of this would be? Any chance that while > fixing this file anyhow, adding a missing variant could be done? Writing back of ccr4 should be intented here, but also writing back the ccr3 to disable the MAPEN again. So both are required. But the ccr4 first: setCx86(CX86_CCR4, ccr4); setCx86(CX86_CCR3, ccr3); Juergen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
> You are right. I agree to your comment. These variables are needless.
> I made a patch again.
>
> diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
> linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
> --- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c2006-11-30
> 06:57:37.0 +0900
> +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16 19:55:05.0
> +0900
> @@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
> static void __cpuinit geode_configure(void)
> {
> unsigned long flags;
> - u8 ccr3, ccr4;
> local_irq_save(flags);
>
> /* Suspend on halt power saving and enable #SUSP pin */
> setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
>
> - ccr3 = getCx86(CX86_CCR3);
> - setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */
> + setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10); /* Enable */
>
> - ccr4 = getCx86(CX86_CCR4);
> - ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
> -
> - setCx86(CX86_CCR3, ccr3);
> + /* FPU fast, DTE cache, Mem bypass */
> + setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
Actually is it possible that the original intent was:
ccr3 = getCx86(CX86_CCR3);
setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */ /* enable advanced
register access? */
ccr4 = getCx86(CX86_CCR4);
ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
setCx86(CX86_CCR4, ccr4);
setCx86(CX86_CCR3, ccr3); /* restore ccr3 register */
Seems something similar with ccr3 was taking place elsewhere in the
function.
--
Len Sorensen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
From: [EMAIL PROTECTED] (Lennart Sorensen)
Subject: Re: fix typo in geode_configre()@cyrix.c
Date: Tue, 16 Jan 2007 11:50:07 -0500
> On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
> > You are right. I agree to your comment. These variables are needless.
> > I made a patch again.
>
> Of course there are also lots of "magic numbers" around, but I must
> admit I don't personally really feel like going through the data sheet
> and naming all of them.
Me too.
>
> > diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
> > linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
> > --- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
> > 06:57:37.0 +0900
> > +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16
> > 19:55:05.0 +0900
> > @@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
> > static void __cpuinit geode_configure(void)
> > {
> > unsigned long flags;
> > - u8 ccr3, ccr4;
> > local_irq_save(flags);
> >
> > /* Suspend on halt power saving and enable #SUSP pin */
> > setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
> >
> > - ccr3 = getCx86(CX86_CCR3);
> > - setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */
> > + setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10); /* Enable */
> >
> > - ccr4 = getCx86(CX86_CCR4);
> > - ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
> > -
> > - setCx86(CX86_CCR3, ccr3);
> > + /* FPU fast, DTE cache, Mem bypass */
> > + setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
>
> Why did that change from 0x38 to 0x30?
ah... I made mistake. My eye is tired.
Correctry, 0x38.
>
> > set_cx86_memwb();
> > set_cx86_reorder();
> > @@ -415,15 +411,14 @@ static void __cpuinit cyrix_identify(str
> >
> > if (dir0 == 5 || dir0 == 3)
> > {
> > - unsigned char ccr3, ccr4;
> > + unsigned char ccr3;
> > unsigned long flags;
> > printk(KERN_INFO "Enabling CPUID on Cyrix
> > processor.\n");
> > local_irq_save(flags);
> > ccr3 = getCx86(CX86_CCR3);
> > - setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* enable
> > MAPEN */
> > - ccr4 = getCx86(CX86_CCR4);
> > - setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
> > cpuid */
> > - setCx86(CX86_CCR3, ccr3); /* disable
> > MAPEN */
> > + setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /*
> > enable MAPEN */
> > + setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
> > enable cpuid */
> > + setCx86(CX86_CCR3, ccr3); /*
> > disable MAPEN */
> > local_irq_restore(flags);
> > }
> > }
>
> --
> Len Sorensen
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
> You are right. I agree to your comment. These variables are needless.
> I made a patch again.
Of course there are also lots of "magic numbers" around, but I must
admit I don't personally really feel like going through the data sheet
and naming all of them.
> diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
> linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
> --- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c2006-11-30
> 06:57:37.0 +0900
> +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16 19:55:05.0
> +0900
> @@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
> static void __cpuinit geode_configure(void)
> {
> unsigned long flags;
> - u8 ccr3, ccr4;
> local_irq_save(flags);
>
> /* Suspend on halt power saving and enable #SUSP pin */
> setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
>
> - ccr3 = getCx86(CX86_CCR3);
> - setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */
> + setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10); /* Enable */
>
> - ccr4 = getCx86(CX86_CCR4);
> - ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
> -
> - setCx86(CX86_CCR3, ccr3);
> + /* FPU fast, DTE cache, Mem bypass */
> + setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
Why did that change from 0x38 to 0x30?
> set_cx86_memwb();
> set_cx86_reorder();
> @@ -415,15 +411,14 @@ static void __cpuinit cyrix_identify(str
>
> if (dir0 == 5 || dir0 == 3)
> {
> - unsigned char ccr3, ccr4;
> + unsigned char ccr3;
> unsigned long flags;
> printk(KERN_INFO "Enabling CPUID on Cyrix
> processor.\n");
> local_irq_save(flags);
> ccr3 = getCx86(CX86_CCR3);
> - setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* enable
> MAPEN */
> - ccr4 = getCx86(CX86_CCR4);
> - setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
> cpuid */
> - setCx86(CX86_CCR3, ccr3); /* disable
> MAPEN */
> + setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /*
> enable MAPEN */
> + setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
> enable cpuid */
> + setCx86(CX86_CCR3, ccr3); /*
> disable MAPEN */
> local_irq_restore(flags);
> }
> }
--
Len Sorensen
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
From: [EMAIL PROTECTED] (Lennart Sorensen)
Subject: Re: fix typo in geode_configre()@cyrix.c
Date: Tue, 9 Jan 2007 12:33:48 -0500
Thank you for comments.
> On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote:
> > In kernel 2.6, write back wrong register when configure Geode processor.
> > Instead of storing to CCR4, it stores to CCR3.
> >
> > --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig 2007-01-09
> > 16:45:21.0 +0900
> > +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09
> > 17:10:13.0 +0900
> > @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo
> > ccr4 = getCx86(CX86_CCR4);
> > ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
> >
> > - setCx86(CX86_CCR3, ccr3);
> > + setCx86(CX86_CCR4, ccr4);
> >
> > set_cx86_memwb();
> > set_cx86_reorder();
> > -
>
> One more comment on this:
>
> Why is this function using 3 different styles to do the same thing to 3
> different registers?
>
> First it does CCR2 by doing:
> setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
>
> Nice, no temp values, and it is obvious this is a permanent change.
>
> Then for the next one it does:
> ccr3 = GetCx86(CX86_CCR3);
> setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10);
>
> Couldn't that have been:
> setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10);
>
> No temp variable, and again it clearly does not intend to restore the
> value again later (even though the bug actually did cause the value to
> be restored by accident).
>
> Then the last case does:
> ccr4 = GetCx86(CX86_CCR4);
> ccr4 |= 0x38;
> setCx86(CX86_CCR4, ccr4); (after fixing the typo you found of course).
>
> Why did this one have to modify the variable first before using it?
> Maybe the ccr3 and ccr4 cases would have made the line too long and
> needed wrapping, but at least those two cases should be done the same
> way. It makes it look like it was written by 3 different people who
> didn't look at the lines around the changes they were making.
You are right. I agree to your comment. These variables are needless.
I made a patch again.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16 19:55:05.0
+0900
@@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
- ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10); /* Enable */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +411,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO "Enabling CPUID on Cyrix
processor.\n");
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
>
> --
> Len Sorensen
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
From: [EMAIL PROTECTED] (Lennart Sorensen)
Subject: Re: fix typo in geode_configre()@cyrix.c
Date: Tue, 9 Jan 2007 12:33:48 -0500
Thank you for comments.
On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote:
In kernel 2.6, write back wrong register when configure Geode processor.
Instead of storing to CCR4, it stores to CCR3.
--- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig 2007-01-09
16:45:21.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09
17:10:13.0 +0900
@@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo
ccr4 = getCx86(CX86_CCR4);
ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
- setCx86(CX86_CCR3, ccr3);
+ setCx86(CX86_CCR4, ccr4);
set_cx86_memwb();
set_cx86_reorder();
-
One more comment on this:
Why is this function using 3 different styles to do the same thing to 3
different registers?
First it does CCR2 by doing:
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
Nice, no temp values, and it is obvious this is a permanent change.
Then for the next one it does:
ccr3 = GetCx86(CX86_CCR3);
setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10);
Couldn't that have been:
setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10);
No temp variable, and again it clearly does not intend to restore the
value again later (even though the bug actually did cause the value to
be restored by accident).
Then the last case does:
ccr4 = GetCx86(CX86_CCR4);
ccr4 |= 0x38;
setCx86(CX86_CCR4, ccr4); (after fixing the typo you found of course).
Why did this one have to modify the variable first before using it?
Maybe the ccr3 and ccr4 cases would have made the line too long and
needed wrapping, but at least those two cases should be done the same
way. It makes it look like it was written by 3 different people who
didn't look at the lines around the changes they were making.
You are right. I agree to your comment. These variables are needless.
I made a patch again.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16 19:55:05.0
+0900
@@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
- ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10); /* Enable */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +411,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO Enabling CPUID on Cyrix
processor.\n);
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
--
Len Sorensen
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
You are right. I agree to your comment. These variables are needless.
I made a patch again.
Of course there are also lots of magic numbers around, but I must
admit I don't personally really feel like going through the data sheet
and naming all of them.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16 19:55:05.0
+0900
@@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
- ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10); /* Enable */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
Why did that change from 0x38 to 0x30?
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +411,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO Enabling CPUID on Cyrix
processor.\n);
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
--
Len Sorensen
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
From: [EMAIL PROTECTED] (Lennart Sorensen)
Subject: Re: fix typo in geode_configre()@cyrix.c
Date: Tue, 16 Jan 2007 11:50:07 -0500
On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
You are right. I agree to your comment. These variables are needless.
I made a patch again.
Of course there are also lots of magic numbers around, but I must
admit I don't personally really feel like going through the data sheet
and naming all of them.
Me too.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c 2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16
19:55:05.0 +0900
@@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
- ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10); /* Enable */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
Why did that change from 0x38 to 0x30?
ah... I made mistake. My eye is tired.
Correctry, 0x38.
set_cx86_memwb();
set_cx86_reorder();
@@ -415,15 +411,14 @@ static void __cpuinit cyrix_identify(str
if (dir0 == 5 || dir0 == 3)
{
- unsigned char ccr3, ccr4;
+ unsigned char ccr3;
unsigned long flags;
printk(KERN_INFO Enabling CPUID on Cyrix
processor.\n);
local_irq_save(flags);
ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* enable
MAPEN */
- ccr4 = getCx86(CX86_CCR4);
- setCx86(CX86_CCR4, ccr4 | 0x80); /* enable
cpuid */
- setCx86(CX86_CCR3, ccr3); /* disable
MAPEN */
+ setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /*
enable MAPEN */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x80); /*
enable cpuid */
+ setCx86(CX86_CCR3, ccr3); /*
disable MAPEN */
local_irq_restore(flags);
}
}
--
Len Sorensen
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Wed, Jan 17, 2007 at 01:38:35AM +0900, takada wrote:
You are right. I agree to your comment. These variables are needless.
I made a patch again.
diff -Narup linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c
linux-2.6.19/arch/i386/kernel/cpu/cyrix.c
--- linux-2.6.19.orig/arch/i386/kernel/cpu/cyrix.c2006-11-30
06:57:37.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-16 19:55:05.0
+0900
@@ -161,19 +161,15 @@ static void __cpuinit set_cx86_inc(void)
static void __cpuinit geode_configure(void)
{
unsigned long flags;
- u8 ccr3, ccr4;
local_irq_save(flags);
/* Suspend on halt power saving and enable #SUSP pin */
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88);
- ccr3 = getCx86(CX86_CCR3);
- setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */
+ setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10); /* Enable */
- ccr4 = getCx86(CX86_CCR4);
- ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
-
- setCx86(CX86_CCR3, ccr3);
+ /* FPU fast, DTE cache, Mem bypass */
+ setCx86(CX86_CCR4, getCx86(CX86_CCR4) | 0x30);
Actually is it possible that the original intent was:
ccr3 = getCx86(CX86_CCR3);
setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); /* Enable */ /* enable advanced
register access? */
ccr4 = getCx86(CX86_CCR4);
ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
setCx86(CX86_CCR4, ccr4);
setCx86(CX86_CCR3, ccr3); /* restore ccr3 register */
Seems something similar with ccr3 was taking place elsewhere in the
function.
--
Len Sorensen
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tuesday 09 January 2007 16:43, Lennart Sorensen wrote: On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote: In kernel 2.6, write back wrong register when configure Geode processor. Instead of storing to CCR4, it stores to CCR3. --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig 2007-01-09 16:45:21.0 +0900 +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0 +0900 @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo ccr4 = getCx86(CX86_CCR4); ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ - setCx86(CX86_CCR3, ccr3); + setCx86(CX86_CCR4, ccr4); set_cx86_memwb(); set_cx86_reorder(); Any idea what the consequence of this would be? Any chance that while fixing this file anyhow, adding a missing variant could be done? Writing back of ccr4 should be intented here, but also writing back the ccr3 to disable the MAPEN again. So both are required. But the ccr4 first: setCx86(CX86_CCR4, ccr4); setCx86(CX86_CCR3, ccr3); Juergen - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tuesday 09 January 2007 18:33, Lennart Sorensen wrote: Then for the next one it does: ccr3 = GetCx86(CX86_CCR3); setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); Couldn't that have been: setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10); No temp variable, and again it clearly does not intend to restore the value again later (even though the bug actually did cause the value to be restored by accident). No, ccr3 should be restored to protect some registers (or at least bit 4 should be cleared in ccr3). BTW: In function set_cx86_inc() [...] /* PCR1 -- Performance Control */ /* Incrementor on, whatever that is */ setCx86(CX86_PCR1, getCx86(CX86_PCR1) | 0x02); /* PCR0 -- Performance Control */ /* Incrementor Margin 10 */ setCx86(CX86_PCR0, getCx86(CX86_PCR0) | 0x04); [...] This setting is only valid for 200MHz...266MHz CPUs, for 300MHz and 333MHz CPUs the Incrementor Margin should be 1-1. There is an AppNote about this setting: AMD Geode GX1 Processor Memory Timings for Maximum Performance. Juergen - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote: > In kernel 2.6, write back wrong register when configure Geode processor. > Instead of storing to CCR4, it stores to CCR3. > > --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig2007-01-09 > 16:45:21.0 +0900 > +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0 > +0900 > @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo > ccr4 = getCx86(CX86_CCR4); > ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ > > - setCx86(CX86_CCR3, ccr3); > + setCx86(CX86_CCR4, ccr4); > > set_cx86_memwb(); > set_cx86_reorder(); > - One more comment on this: Why is this function using 3 different styles to do the same thing to 3 different registers? First it does CCR2 by doing: setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88); Nice, no temp values, and it is obvious this is a permanent change. Then for the next one it does: ccr3 = GetCx86(CX86_CCR3); setCx86(CX86_CCR3, (ccr3 & 0x0f) | 0x10); Couldn't that have been: setCx86(CX86_CCR3, (getCx86(CX86_CCR3) & 0x0f) | 0x10); No temp variable, and again it clearly does not intend to restore the value again later (even though the bug actually did cause the value to be restored by accident). Then the last case does: ccr4 = GetCx86(CX86_CCR4); ccr4 |= 0x38; setCx86(CX86_CCR4, ccr4); (after fixing the typo you found of course). Why did this one have to modify the variable first before using it? Maybe the ccr3 and ccr4 cases would have made the line too long and needed wrapping, but at least those two cases should be done the same way. It makes it look like it was written by 3 different people who didn't look at the lines around the changes they were making. -- Len Sorensen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote: > In kernel 2.6, write back wrong register when configure Geode processor. > Instead of storing to CCR4, it stores to CCR3. > > --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig2007-01-09 > 16:45:21.0 +0900 > +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0 > +0900 > @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo > ccr4 = getCx86(CX86_CCR4); > ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ > > - setCx86(CX86_CCR3, ccr3); > + setCx86(CX86_CCR4, ccr4); > > set_cx86_memwb(); > set_cx86_reorder(); Any idea what the consequence of this would be? Any chance that while fixing this file anyhow, adding a missing variant could be done? The patch I currently run with is this (To cyrix.c): @@ -257,6 +257,8 @@ break; case 4: /* MediaGX/GXm or Geode GXM/GXLV/GX1 */ + case 11: /* SC1200 seems to return 11 or something but it is a geode gx1 too as far as I know */ + dir0_msn = 4; #ifdef CONFIG_PCI /* It isn't really a PCI quirk directly, but the cure is the same. The MediaGX has deep magic SMM stuff that handles the After the patch I get this: # cat /proc/cpuinfo processor : 0 vendor_id : CyrixInstead cpu family : 5 model : 9 model name : Geode(TM) Integrated Processor by National Semi stepping: 1 cpu MHz : 266.759 cache size : 16 KB fdiv_bug: no hlt_bug : no f00f_bug: no coma_bug: no fpu : yes fpu_exception : yes cpuid level : 2 wp : yes flags : fpu tsc msr cx8 cmov mmx cxmmx bogomips: 535.09 Before that the cpu type was unknown, the cache size wasn't there, and a number of fixups were not being applied as far as I could tell. I know I have had to fix the jsm serial driver which uses memcpy_toio, and I got characters out of order on transmit on the geode, while on a pentium 4 or athlon it works perfectly fine with the same card and driver. My workaround (which of course isn't good for efficiency) has been: - memcpy_toio(>ch_neo_uart->txrxburst, ch->ch_wqueue + tail, s); + for(i=0;ich_neo_uart->txrxburst[i]), ch->ch_wqueue + tail + i, 1); + } + /*memcpy_toio(>ch_neo_uart->txrxburst, ch->ch_wqueue + tail, s);*/ Could the wrong register being saved have anything to do with that? Without my fix a burst transfer of the data 0123456789ABCDEF results in the other end receiving: 123056749AB8DEFC -- Len Sorensen - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
fix typo in geode_configre()@cyrix.c
In kernel 2.6, write back wrong register when configure Geode processor. Instead of storing to CCR4, it stores to CCR3. --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig 2007-01-09 16:45:21.0 +0900 +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0 +0900 @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo ccr4 = getCx86(CX86_CCR4); ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ - setCx86(CX86_CCR3, ccr3); + setCx86(CX86_CCR4, ccr4); set_cx86_memwb(); set_cx86_reorder(); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
fix typo in geode_configre()@cyrix.c
In kernel 2.6, write back wrong register when configure Geode processor. Instead of storing to CCR4, it stores to CCR3. --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig 2007-01-09 16:45:21.0 +0900 +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0 +0900 @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo ccr4 = getCx86(CX86_CCR4); ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ - setCx86(CX86_CCR3, ccr3); + setCx86(CX86_CCR4, ccr4); set_cx86_memwb(); set_cx86_reorder(); - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote:
In kernel 2.6, write back wrong register when configure Geode processor.
Instead of storing to CCR4, it stores to CCR3.
--- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig2007-01-09
16:45:21.0 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0
+0900
@@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo
ccr4 = getCx86(CX86_CCR4);
ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */
- setCx86(CX86_CCR3, ccr3);
+ setCx86(CX86_CCR4, ccr4);
set_cx86_memwb();
set_cx86_reorder();
Any idea what the consequence of this would be? Any chance that while
fixing this file anyhow, adding a missing variant could be done?
The patch I currently run with is this (To cyrix.c):
@@ -257,6 +257,8 @@
break;
case 4: /* MediaGX/GXm or Geode GXM/GXLV/GX1 */
+ case 11: /* SC1200 seems to return 11 or something but it is a geode
gx1 too as far as I know */
+ dir0_msn = 4;
#ifdef CONFIG_PCI
/* It isn't really a PCI quirk directly, but the cure is the
same. The MediaGX has deep magic SMM stuff that handles the
After the patch I get this:
# cat /proc/cpuinfo
processor : 0
vendor_id : CyrixInstead
cpu family : 5
model : 9
model name : Geode(TM) Integrated Processor by National Semi
stepping: 1
cpu MHz : 266.759
cache size : 16 KB
fdiv_bug: no
hlt_bug : no
f00f_bug: no
coma_bug: no
fpu : yes
fpu_exception : yes
cpuid level : 2
wp : yes
flags : fpu tsc msr cx8 cmov mmx cxmmx
bogomips: 535.09
Before that the cpu type was unknown, the cache size wasn't there,
and a number of fixups were not being applied as far as I could tell.
I know I have had to fix the jsm serial driver which uses memcpy_toio,
and I got characters out of order on transmit on the geode, while on a
pentium 4 or athlon it works perfectly fine with the same card and driver.
My workaround (which of course isn't good for efficiency) has been:
- memcpy_toio(ch-ch_neo_uart-txrxburst, ch-ch_wqueue + tail,
s);
+ for(i=0;is;i++) {
+ memcpy_toio((ch-ch_neo_uart-txrxburst[i]),
ch-ch_wqueue + tail + i, 1);
+ }
+ /*memcpy_toio(ch-ch_neo_uart-txrxburst, ch-ch_wqueue +
tail, s);*/
Could the wrong register being saved have anything to do with that?
Without my fix a burst transfer of the data 0123456789ABCDEF results in
the other end receiving: 123056749AB8DEFC
--
Len Sorensen
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: fix typo in geode_configre()@cyrix.c
On Tue, Jan 09, 2007 at 06:41:56PM +0900, takada wrote: In kernel 2.6, write back wrong register when configure Geode processor. Instead of storing to CCR4, it stores to CCR3. --- linux-2.6.19/arch/i386/kernel/cpu/cyrix.c.orig2007-01-09 16:45:21.0 +0900 +++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c 2007-01-09 17:10:13.0 +0900 @@ -173,7 +173,7 @@ static void __cpuinit geode_configure(vo ccr4 = getCx86(CX86_CCR4); ccr4 |= 0x38; /* FPU fast, DTE cache, Mem bypass */ - setCx86(CX86_CCR3, ccr3); + setCx86(CX86_CCR4, ccr4); set_cx86_memwb(); set_cx86_reorder(); - One more comment on this: Why is this function using 3 different styles to do the same thing to 3 different registers? First it does CCR2 by doing: setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x88); Nice, no temp values, and it is obvious this is a permanent change. Then for the next one it does: ccr3 = GetCx86(CX86_CCR3); setCx86(CX86_CCR3, (ccr3 0x0f) | 0x10); Couldn't that have been: setCx86(CX86_CCR3, (getCx86(CX86_CCR3) 0x0f) | 0x10); No temp variable, and again it clearly does not intend to restore the value again later (even though the bug actually did cause the value to be restored by accident). Then the last case does: ccr4 = GetCx86(CX86_CCR4); ccr4 |= 0x38; setCx86(CX86_CCR4, ccr4); (after fixing the typo you found of course). Why did this one have to modify the variable first before using it? Maybe the ccr3 and ccr4 cases would have made the line too long and needed wrapping, but at least those two cases should be done the same way. It makes it look like it was written by 3 different people who didn't look at the lines around the changes they were making. -- Len Sorensen - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
