Please revert "fix typo in geode_configre()@cyrix.c"

Please revert "fix typo in geode_configre()@cyrix.c"

[Adrian Bunk]

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

Re: Please revert "fix typo in geode_configre()@cyrix.c"

[Andrew Morton]

On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk <bunk@stusta.de> 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.

Re: Please revert "fix typo in geode_configre()@cyrix.c"

[Lennart Sorensen]

On Fri, Feb 02, 2007 at 12:05:43AM -0800, Andrew Morton wrote:
> On Fri, 2 Feb 2007 07:29:41 +0100 Adrian Bunk <bunk@stusta.de> 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

Re: Please revert "fix typo in geode_configre()@cyrix.c"

[Andrew Morton]

On Fri, 2 Feb 2007 10:12:36 -0500
lsorense@csclub.uwaterloo.ca (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 <bunk@stusta.de> 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.

Re: Please revert "fix typo in geode_configre()@cyrix.c"

[TAKADA Yoshihito]

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.000000000 +0900
+++ linux-2.6.19/arch/i386/kernel/cpu/cyrix.c	2007-02-03 14:57:35.000000000 +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 <akpm@linux-foundation.org> wrote:

> On Fri, 2 Feb 2007 10:12:36 -0500
> lsorense@csclub.uwaterloo.ca (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 <bunk@stusta.de> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
TAKADA <takada@mbf.nifty.com>

Re: Please revert "fix typo in geode_configre()@cyrix.c"

[Juergen Beisert]

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

Re: Please revert "fix typo in geode_configre()@cyrix.c"

[Lennart Sorensen]

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