[PATCH] remove dead cyrix/centaur mtrr init code

[PATCH] remove dead cyrix/centaur mtrr init code

[Andries Brouwer]

There are several cases where __init function pointers are
stored in a general purpose struct. For example, a SCSI
template may contain a __init detect function.
Have not yet thought of an elegant way to avoid this.

One such case is the mtrr code, where struct mtrr_ops has an
init field pointing at __init functions. Unless I overlook
something, this case may be easy to settle, since the .init
field is never used.

The patch below comments out the declaration and initialisation
of the .init field of struct mtrr_ops, and puts #if 0 ... #endif
around the centaur_mcr_init() and cyrix_arr_init() code.

Simultaneously a number of variables are made static.

Andries

-----

diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/centaur.c b/arch/i386/kernel/cpu/mtrr/centaur.c
--- a/arch/i386/kernel/cpu/mtrr/centaur.c	2003-12-18 03:58:38.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/centaur.c	2005-02-28 20:21:05.000000000 +0100
@@ -86,6 +86,8 @@ static void centaur_set_mcr(unsigned int
 	centaur_mcr[reg].low = low;
 	wrmsr(MSR_IDT_MCR0 + reg, low, high);
 }
+
+#if 0
 /*
  *	Initialise the later (saner) Winchip MCR variant. In this version
  *	the BIOS can pass us the registers it has used (but not their values)
@@ -183,6 +185,7 @@ centaur_mcr_init(void)
 
 	set_mtrr_done(&ctxt);
 }
+#endif
 
 static int centaur_validate_add_page(unsigned long base, 
 				     unsigned long size, unsigned int type)
@@ -203,7 +206,7 @@ static int centaur_validate_add_page(uns
 
 static struct mtrr_ops centaur_mtrr_ops = {
 	.vendor            = X86_VENDOR_CENTAUR,
-	.init              = centaur_mcr_init,
+//	.init              = centaur_mcr_init,
 	.set               = centaur_set_mcr,
 	.get               = centaur_get_mcr,
 	.get_free_region   = centaur_get_free_region,
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/cyrix.c b/arch/i386/kernel/cpu/mtrr/cyrix.c
--- a/arch/i386/kernel/cpu/mtrr/cyrix.c	2003-12-18 03:58:56.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/cyrix.c	2005-02-28 20:19:25.000000000 +0100
@@ -218,12 +218,12 @@ typedef struct {
 	mtrr_type type;
 } arr_state_t;
 
-arr_state_t arr_state[8] __initdata = {
+static arr_state_t arr_state[8] __initdata = {
 	{0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL},
 	{0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}
 };
 
-unsigned char ccr_state[7] __initdata = { 0, 0, 0, 0, 0, 0, 0 };
+static unsigned char ccr_state[7] __initdata = { 0, 0, 0, 0, 0, 0, 0 };
 
 static void cyrix_set_all(void)
 {
@@ -243,6 +243,7 @@ static void cyrix_set_all(void)
 	post_set();
 }
 
+#if 0
 /*
  * On Cyrix 6x86(MX) and M II the ARR3 is special: it has connection
  * with the SMM (System Management Mode) mode. So we need the following:
@@ -341,10 +342,11 @@ cyrix_arr_init(void)
 	if (ccrc[6])
 		printk(KERN_INFO "mtrr: ARR3 was write protected, unprotected\n");
 }
+#endif
 
 static struct mtrr_ops cyrix_mtrr_ops = {
 	.vendor            = X86_VENDOR_CYRIX,
-	.init              = cyrix_arr_init,
+//	.init              = cyrix_arr_init,
 	.set_all	   = cyrix_set_all,
 	.set               = cyrix_set_arr,
 	.get               = cyrix_get_arr,
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/generic.c b/arch/i386/kernel/cpu/mtrr/generic.c
--- a/arch/i386/kernel/cpu/mtrr/generic.c	2005-02-26 12:13:28.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/generic.c	2005-02-28 20:19:25.000000000 +0100
@@ -19,8 +19,7 @@ struct mtrr_state {
 };
 
 static unsigned long smp_changes_mask;
-struct mtrr_state mtrr_state = {};
-
+static struct mtrr_state mtrr_state = {};
 
 /*  Get the MSR pair relating to a var range  */
 static void __init
@@ -383,7 +382,7 @@ int generic_validate_add_page(unsigned l
 }
 
 
-int generic_have_wrcomb(void)
+static int generic_have_wrcomb(void)
 {
 	unsigned long config, dummy;
 	rdmsr(MTRRcap_MSR, config, dummy);
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/main.c b/arch/i386/kernel/cpu/mtrr/main.c
--- a/arch/i386/kernel/cpu/mtrr/main.c	2004-12-29 03:39:42.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/main.c	2005-02-28 20:19:25.000000000 +0100
@@ -57,10 +57,6 @@ static struct mtrr_ops * mtrr_ops[X86_VE
 
 struct mtrr_ops * mtrr_if = NULL;
 
-__initdata char *mtrr_if_name[] = {
-    "none", "Intel", "AMD K6", "Cyrix ARR", "Centaur MCR"
-};
-
 static void set_mtrr(unsigned int reg, unsigned long base,
 		     unsigned long size, mtrr_type type);
 
@@ -100,7 +96,7 @@ static int have_wrcomb(void)
 }
 
 /*  This function returns the number of variable MTRRs  */
-void __init set_num_var_ranges(void)
+static void __init set_num_var_ranges(void)
 {
 	unsigned long config = 0, dummy;
 
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/mtrr.h b/arch/i386/kernel/cpu/mtrr/mtrr.h
--- a/arch/i386/kernel/cpu/mtrr/mtrr.h	2004-10-30 21:43:59.000000000 +0200
+++ b/arch/i386/kernel/cpu/mtrr/mtrr.h	2005-02-28 20:19:25.000000000 +0100
@@ -37,7 +37,7 @@ typedef u8 mtrr_type;
 struct mtrr_ops {
 	u32	vendor;
 	u32	use_intel_if;
-	void	(*init)(void);
+//	void	(*init)(void);
 	void	(*set)(unsigned int reg, unsigned long base,
 		       unsigned long size, mtrr_type type);
 	void	(*set_all)(void);
@@ -57,7 +57,6 @@ extern int generic_validate_add_page(uns
 
 extern struct mtrr_ops generic_mtrr_ops;
 
-extern int generic_have_wrcomb(void);
 extern int positive_have_wrcomb(void);
 
 /* library functions for processor-specific routines */
@@ -96,4 +95,3 @@ void finalize_mtrr_state(void);
 void mtrr_state_warn(void);
 char *mtrr_attrib_to_str(int x);
 
-extern char * mtrr_if_name[];

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Adrian Bunk]

Hi Andries,

your patch has many overlappings with a patch of mine aleady in -mm 
(both none of the two patches is a subset of the other one).

Nowadays, working against -mm often avoids duplicate work.

cu
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: [PATCH] remove dead cyrix/centaur mtrr init code

[Andries Brouwer]

On Mon, Feb 28, 2005 at 08:35:29PM +0100, Adrian Bunk wrote:
> Hi Andries,
> 
> your patch has many overlappings with a patch of mine aleady in -mm 
> (both none of the two patches is a subset of the other one).
> 
> Nowadays, working against -mm often avoids duplicate work.
> 
> cu
> Adrian

As far as I am concerned this doesnt matter so much.

This is very trivial work, and only little time is wasted
in case of duplication. It is unavoidable.

(For example, looking at the detect functions of scsi drivers
I happened to come across pas16.c and did a largish cleanup.
Hesitate a bit submitting the results, since the driver showed
some signs of bitrot - maybe nobody has used it for years -
if someone uses pas16, please tell me - let me cc linux-scsi -
but today you submit a little patch on pas16.)

Andries

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Alan Cox]

On Llu, 2005-02-28 at 19:20, Andries Brouwer wrote:
> One such case is the mtrr code, where struct mtrr_ops has an
> init field pointing at __init functions. Unless I overlook
> something, this case may be easy to settle, since the .init
> field is never used.

The failure to invoke the ->init operator appears to be the bug.

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Andries Brouwer]

On Tue, Mar 01, 2005 at 11:52:44PM +0000, Alan Cox wrote:
> On Llu, 2005-02-28 at 19:20, Andries Brouwer wrote:
> > One such case is the mtrr code, where struct mtrr_ops has an
> > init field pointing at __init functions. Unless I overlook
> > something, this case may be easy to settle, since the .init
> > field is never used.
> 
> The failure to invoke the ->init operator appears to be the bug.
> The centaur code definitely wants the mcr init function to be called.

Yes, I expected that to be the answer. Therefore #if 0 instead of deleting.
But if calling ->init() is needed, and it has not been done the past
three years, the question arises whether there are any users.

Andries

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Dave Jones]

On Wed, Mar 02, 2005 at 08:50:38AM +0100, Andries Brouwer wrote:
 > On Tue, Mar 01, 2005 at 11:52:44PM +0000, Alan Cox wrote:
 > > On Llu, 2005-02-28 at 19:20, Andries Brouwer wrote:
 > > > One such case is the mtrr code, where struct mtrr_ops has an
 > > > init field pointing at __init functions. Unless I overlook
 > > > something, this case may be easy to settle, since the .init
 > > > field is never used.
 > > 
 > > The failure to invoke the ->init operator appears to be the bug.
 > > The centaur code definitely wants the mcr init function to be called.
 > 
 > Yes, I expected that to be the answer. Therefore #if 0 instead of deleting.
 > But if calling ->init() is needed, and it has not been done the past
 > three years, the question arises whether there are any users.

The Winchips never really sold that well, and stopped being produced
when IDT sold off Centaur.  It was a niche processor in 1997. In 2005,
I'll be surprised if there are that many of them still working.
Mine lost its magic smoke for no reason around ~2002.

If there are any of them still being used out there, I'd be even
more surprised if they're running 2.6.  Then again, there are
probably loonies out there running it on 386/486's. 8-)

		Dave

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Alan Cox]

On Mer, 2005-03-02 at 08:02, Dave Jones wrote:
> If there are any of them still being used out there, I'd be even
> more surprised if they're running 2.6.  Then again, there are
> probably loonies out there running it on 386/486's. 8-)

I have one here running 2.4 still. I can test a 2.6 fix for the mtrr
init happily enough.

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Andries Brouwer]

On Wed, Mar 02, 2005 at 01:45:43PM +0000, Alan Cox wrote:
> On Mer, 2005-03-02 at 08:02, Dave Jones wrote:
> > If there are any of them still being used out there, I'd be even
> > more surprised if they're running 2.6.  Then again, there are
> > probably loonies out there running it on 386/486's. 8-)
> 
> I have one here running 2.4 still. I can test a 2.6 fix for the mtrr
> init happily enough.

Good. If I understand things correctly - you or davej or someone will
correct me otherwise - failing to initialise mtrr does not break anything,
it would just mean slower access to certain kinds of memory for certain
kinds of access patterns. (Would you test using an X benchmark?)

Below roughly speaking the same patch as before, but with calls
to the cyrix and centaur mtrr init routines inserted.

Andries

-----

diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/centaur.c b/arch/i386/kernel/cpu/mtrr/centaur.c
--- a/arch/i386/kernel/cpu/mtrr/centaur.c	2003-12-18 03:58:38.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/centaur.c	2005-03-02 23:22:10.000000000 +0100
@@ -86,6 +86,7 @@ static void centaur_set_mcr(unsigned int
 	centaur_mcr[reg].low = low;
 	wrmsr(MSR_IDT_MCR0 + reg, low, high);
 }
+
 /*
  *	Initialise the later (saner) Winchip MCR variant. In this version
  *	the BIOS can pass us the registers it has used (but not their values)
@@ -168,7 +169,7 @@ centaur_mcr0_init(void)
  *	Initialise Winchip series MCR registers
  */
 
-static void __init
+void __init
 centaur_mcr_init(void)
 {
 	struct set_mtrr_context ctxt;
@@ -203,7 +204,6 @@ static int centaur_validate_add_page(uns
 
 static struct mtrr_ops centaur_mtrr_ops = {
 	.vendor            = X86_VENDOR_CENTAUR,
-	.init              = centaur_mcr_init,
 	.set               = centaur_set_mcr,
 	.get               = centaur_get_mcr,
 	.get_free_region   = centaur_get_free_region,
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/cyrix.c b/arch/i386/kernel/cpu/mtrr/cyrix.c
--- a/arch/i386/kernel/cpu/mtrr/cyrix.c	2003-12-18 03:58:56.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/cyrix.c	2005-03-02 23:22:40.000000000 +0100
@@ -218,12 +218,12 @@ typedef struct {
 	mtrr_type type;
 } arr_state_t;
 
-arr_state_t arr_state[8] __initdata = {
+static arr_state_t arr_state[8] __initdata = {
 	{0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL},
 	{0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}, {0UL, 0UL, 0UL}
 };
 
-unsigned char ccr_state[7] __initdata = { 0, 0, 0, 0, 0, 0, 0 };
+static unsigned char ccr_state[7] __initdata = { 0, 0, 0, 0, 0, 0, 0 };
 
 static void cyrix_set_all(void)
 {
@@ -257,7 +257,7 @@ static void cyrix_set_all(void)
  *   - (maybe) disable ARR3
  * Just to be sure, we enable ARR usage by the processor (CCR5 bit 5 set)
  */
-static void __init
+void __init
 cyrix_arr_init(void)
 {
 	struct set_mtrr_context ctxt;
@@ -344,7 +344,6 @@ cyrix_arr_init(void)
 
 static struct mtrr_ops cyrix_mtrr_ops = {
 	.vendor            = X86_VENDOR_CYRIX,
-	.init              = cyrix_arr_init,
 	.set_all	   = cyrix_set_all,
 	.set               = cyrix_set_arr,
 	.get               = cyrix_get_arr,
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/generic.c b/arch/i386/kernel/cpu/mtrr/generic.c
--- a/arch/i386/kernel/cpu/mtrr/generic.c	2005-03-02 20:54:48.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/generic.c	2005-03-02 20:56:19.000000000 +0100
@@ -19,8 +19,7 @@ struct mtrr_state {
 };
 
 static unsigned long smp_changes_mask;
-struct mtrr_state mtrr_state = {};
-
+static struct mtrr_state mtrr_state = {};
 
 /*  Get the MSR pair relating to a var range  */
 static void __init
@@ -383,7 +382,7 @@ int generic_validate_add_page(unsigned l
 }
 
 
-int generic_have_wrcomb(void)
+static int generic_have_wrcomb(void)
 {
 	unsigned long config, dummy;
 	rdmsr(MTRRcap_MSR, config, dummy);
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/main.c b/arch/i386/kernel/cpu/mtrr/main.c
--- a/arch/i386/kernel/cpu/mtrr/main.c	2004-12-29 03:39:42.000000000 +0100
+++ b/arch/i386/kernel/cpu/mtrr/main.c	2005-03-02 23:21:46.000000000 +0100
@@ -57,10 +57,6 @@ static struct mtrr_ops * mtrr_ops[X86_VE
 
 struct mtrr_ops * mtrr_if = NULL;
 
-__initdata char *mtrr_if_name[] = {
-    "none", "Intel", "AMD K6", "Cyrix ARR", "Centaur MCR"
-};
-
 static void set_mtrr(unsigned int reg, unsigned long base,
 		     unsigned long size, mtrr_type type);
 
@@ -100,7 +96,7 @@ static int have_wrcomb(void)
 }
 
 /*  This function returns the number of variable MTRRs  */
-void __init set_num_var_ranges(void)
+static void __init set_num_var_ranges(void)
 {
 	unsigned long config = 0, dummy;
 
@@ -526,6 +522,8 @@ EXPORT_SYMBOL(mtrr_del);
  * These should be called implicitly, but we can't yet until all the initcall
  * stuff is done...
  */
+extern void centaur_mcr_init(void);
+extern void cyrix_arr_init(void);
 extern void amd_init_mtrr(void);
 extern void cyrix_init_mtrr(void);
 extern void centaur_init_mtrr(void);
@@ -665,6 +663,7 @@ static int __init mtrr_init(void)
 			break;
 		case X86_VENDOR_CENTAUR:
 			if (cpu_has_centaur_mcr) {
+				centaur_mcr_init();
 				mtrr_if = mtrr_ops[X86_VENDOR_CENTAUR];
 				size_or_mask = 0xfff00000;	/* 32 bits */
 				size_and_mask = 0;
@@ -672,6 +671,7 @@ static int __init mtrr_init(void)
 			break;
 		case X86_VENDOR_CYRIX:
 			if (cpu_has_cyrix_arr) {
+				cyrix_arr_init();
 				mtrr_if = mtrr_ops[X86_VENDOR_CYRIX];
 				size_or_mask = 0xfff00000;	/* 32 bits */
 				size_and_mask = 0;
diff -uprN -X /linux/dontdiff a/arch/i386/kernel/cpu/mtrr/mtrr.h b/arch/i386/kernel/cpu/mtrr/mtrr.h
--- a/arch/i386/kernel/cpu/mtrr/mtrr.h	2004-10-30 21:43:59.000000000 +0200
+++ b/arch/i386/kernel/cpu/mtrr/mtrr.h	2005-03-02 23:17:52.000000000 +0100
@@ -37,7 +37,6 @@ typedef u8 mtrr_type;
 struct mtrr_ops {
 	u32	vendor;
 	u32	use_intel_if;
-	void	(*init)(void);
 	void	(*set)(unsigned int reg, unsigned long base,
 		       unsigned long size, mtrr_type type);
 	void	(*set_all)(void);
@@ -57,7 +56,6 @@ extern int generic_validate_add_page(uns
 
 extern struct mtrr_ops generic_mtrr_ops;
 
-extern int generic_have_wrcomb(void);
 extern int positive_have_wrcomb(void);
 
 /* library functions for processor-specific routines */
@@ -96,4 +94,3 @@ void finalize_mtrr_state(void);
 void mtrr_state_warn(void);
 char *mtrr_attrib_to_str(int x);
 
-extern char * mtrr_if_name[];

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Dave Jones]

On Wed, Mar 02, 2005 at 11:21:06PM +0100, Andries Brouwer wrote:
 > On Wed, Mar 02, 2005 at 01:45:43PM +0000, Alan Cox wrote:
 > > On Mer, 2005-03-02 at 08:02, Dave Jones wrote:
 > > > If there are any of them still being used out there, I'd be even
 > > > more surprised if they're running 2.6.  Then again, there are
 > > > probably loonies out there running it on 386/486's. 8-)
 > > 
 > > I have one here running 2.4 still. I can test a 2.6 fix for the mtrr
 > > init happily enough.
 > 
 > Good. If I understand things correctly - you or davej or someone will
 > correct me otherwise - failing to initialise mtrr does not break anything,
 > it would just mean slower access to certain kinds of memory for certain
 > kinds of access patterns. (Would you test using an X benchmark?)

The winchips had a funky feature where you could mark system ram
writes as out-of-order. This led to something like a 25% speedup iirc
on benchmarks that did lots of memory copying. lmbench showed
significant wins iirc, but any results I had saved are long since
wiped out in hard disk failures/cruft removal over the years.

 > Below roughly speaking the same patch as before, but with calls
 > to the cyrix and centaur mtrr init routines inserted.

Looks ok on a quick eyeball.

		Dave

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Alan Cox]

On Mer, 2005-03-02 at 22:28, Dave Jones wrote:
> The winchips had a funky feature where you could mark system ram
> writes as out-of-order. This led to something like a 25% speedup iirc
> on benchmarks that did lots of memory copying. lmbench showed
> significant wins iirc, but any results I had saved are long since
> wiped out in hard disk failures/cruft removal over the years.

Yep - providing your kernel is built for it you get about 20-30% speed
up against a base kernel. It's the one freak case (in 2.4 anyway) where
kernel cpu options matter.

Alan

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Nuno Monteiro]

On 2005.03.02 08:02, Dave Jones wrote:
> 
> The Winchips never really sold that well, and stopped being produced
> when IDT sold off Centaur.  It was a niche processor in 1997. In 2005,
> I'll be surprised if there are that many of them still working.
> Mine lost its magic smoke for no reason around ~2002.
> 
> If there are any of them still being used out there, I'd be even
> more surprised if they're running 2.6.  Then again, there are
> probably loonies out there running it on 386/486's. 8-)
>

Heh. Let me brag about it a little:

$ cat /proc/cpuinfo
processor       : 0
vendor_id       : CentaurHauls
cpu family      : 5
model           : 4
model name      : WinChip C6
stepping        : 1
fdiv_bug        : no
hlt_bug         : no
f00f_bug        : no
coma_bug        : no
fpu             : yes
fpu_exception   : yes
cpuid level     : 1
wp              : yes
flags           : fpu de msr mce cx8 mmx centaur_mcr cid
bogomips        : 399.76

$ uname -a
Linux kawasaki 2.6.10-rc3 #3 Fri Dec 17 21:44:53 CET 2004 i586 unknown

$ uptime
 19:00:14 up 58 days,  1:21,  1 user,  load average: 0.37, 0.17, 0.11

Running 2.6 at least since 2.6.0-test11-wli-something, dated Dec 6 2003  
according to /boot (may have been running other 2.6-test before, but I  
dont have them around any longer):

$ ls -al /boot/vmlinuz-2.6.0-test11
-rw-r-----    1 root     root       840607 Dec  6  2003 /boot/vmlinuz- 
2.6.0-test11-wli


Not a single hiccup, so far *knock on wood*. Bragging even further, I  
also have a genuine 80386 DX 33 up and running, although that one is  
still on 2.0.37, iirc.

Oh, yes, I'm a loonie ;-)


Regards,


		Nuno

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Ondrej Zary]

Andries Brouwer wrote:
> On Tue, Mar 01, 2005 at 11:52:44PM +0000, Alan Cox wrote:
> 
>>On Llu, 2005-02-28 at 19:20, Andries Brouwer wrote:
>>
>>>One such case is the mtrr code, where struct mtrr_ops has an
>>>init field pointing at __init functions. Unless I overlook
>>>something, this case may be easy to settle, since the .init
>>>field is never used.
>>
>>The failure to invoke the ->init operator appears to be the bug.
>>The centaur code definitely wants the mcr init function to be called.
> 
> 
> Yes, I expected that to be the answer. Therefore #if 0 instead of deleting.
> But if calling ->init() is needed, and it has not been done the past
> three years, the question arises whether there are any users.

I'm running 2.6.10 on Cyrix MII PR333 and it works. Maybe the code is 
broken but I haven't noticed :-)


-- 
Ondrej Zary

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Dave Jones]

On Wed, Mar 02, 2005 at 03:59:00PM +0100, Ondrej Zary wrote:
 > >>The failure to invoke the ->init operator appears to be the bug.
 > >>The centaur code definitely wants the mcr init function to be called.
 > >
 > >Yes, I expected that to be the answer. Therefore #if 0 instead of deleting.
 > >But if calling ->init() is needed, and it has not been done the past
 > >three years, the question arises whether there are any users.
 > 
 > I'm running 2.6.10 on Cyrix MII PR333 and it works. Maybe the code is 
 > broken but I haven't noticed :-)

your /proc/mtrr is likely broken/suboptimal.

		Dave

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Ondrej Zary]

Dave Jones wrote:
> On Wed, Mar 02, 2005 at 03:59:00PM +0100, Ondrej Zary wrote:
>  > >>The failure to invoke the ->init operator appears to be the bug.
>  > >>The centaur code definitely wants the mcr init function to be called.
>  > >
>  > >Yes, I expected that to be the answer. Therefore #if 0 instead of deleting.
>  > >But if calling ->init() is needed, and it has not been done the past
>  > >three years, the question arises whether there are any users.
>  > 
>  > I'm running 2.6.10 on Cyrix MII PR333 and it works. Maybe the code is 
>  > broken but I haven't noticed :-)
> 
> your /proc/mtrr is likely broken/suboptimal.

It looks fine to me:

rainbow@pentium:~$ cat /proc/mtrr
reg01: base=0x000c0000 (   0MB), size= 256KB: uncachable, count=1
reg02: base=0xe1000000 (3600MB), size=   4MB: write-combining, count=2
reg07: base=0x00000000 (   0MB), size= 128MB: write-back, count=1

The machine has 128MB memory, there's 4MB Matrox Mystique card:

00:14.0 VGA compatible controller: Matrox Graphics, Inc. MGA 1064SG 
[Mystique] (rev 02) (prog-if 00 [VGA])
         Subsystem: Matrox Graphics, Inc. MGA-1084SG Mystique
         Flags: bus master, stepping, medium devsel, latency 64, IRQ 6
         Memory at e0000000 (32-bit, non-prefetchable) [size=16K]
         Memory at e1000000 (32-bit, prefetchable) [size=8M]
         Memory at e2000000 (32-bit, non-prefetchable) [size=8M]
         Expansion ROM at <unassigned> [disabled] [size=64K]


-- 
Ondrej Zary

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Alan Cox]

On Maw, 2005-03-08 at 17:40, Linux Kernel Mailing List wrote:
> ChangeSet 1.2094, 2005/03/08 09:40:59-08:00, Andries.Brouwer@cwi.nl
> 
> 	[PATCH] remove dead cyrix/centaur mtrr init code


This patch was discussed previously and declared incorrect. The ->init
method call is missing in the base mtrr code.

Should be reverted and/or fixed properly.

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Linus Torvalds]

On Wed, 9 Mar 2005, Alan Cox wrote:
> 
> This patch was discussed previously and declared incorrect.

Well, it was also declared as a "don't care" by Dave, I think, by virtue 
of nobody having ever complained.

		Linus

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Alan Cox]

On Mer, 2005-03-09 at 17:09, Linus Torvalds wrote:
> On Wed, 9 Mar 2005, Alan Cox wrote:
> > 
> > This patch was discussed previously and declared incorrect.
> 
> Well, it was also declared as a "don't care" by Dave, I think, by virtue 
> of nobody having ever complained.

And in further discussion people produced the relevant CPU's. Its a
performance hit (30% on winchip). 

Alan

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Andries Brouwer]

On Wed, Mar 09, 2005 at 04:55:27PM +0000, Alan Cox wrote:

> > 	[PATCH] remove dead cyrix/centaur mtrr init code
> 
> This patch was discussed previously and declared incorrect. The ->init
> method call is missing in the base mtrr code.
> 
> Should be reverted and/or fixed properly.

Hi Alan - a surprising reaction.

The patch is an improvement - it #ifdef's out some dead code.
I sent you a follow-up patch that activates the dead code,
since you said

  I have one here running 2.4 still. I can test a 2.6 fix
  for the mtrr init happily enough.

But so far you have not replied.
The moment you report that the follow-up patch is fine, we can
remove the #if 0 and insert the initcalls instead.

So, all is well today, and we are waiting for your report.

Andries

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Alan Cox]

On Mer, 2005-03-09 at 19:09, Andries Brouwer wrote:
> The moment you report that the follow-up patch is fine, we can
> remove the #if 0 and insert the initcalls instead.
> 
> So, all is well today, and we are waiting for your report.

Ok works for me. I'll let you know ASAP.

Re: [PATCH] remove dead cyrix/centaur mtrr init code

[Alan Cox]

On Mer, 2005-03-09 at 20:36, Alan Cox wrote:
> On Mer, 2005-03-09 at 19:09, Andries Brouwer wrote:
> > The moment you report that the follow-up patch is fine, we can
> > remove the #if 0 and insert the initcalls instead.
> > 
> > So, all is well today, and we are waiting for your report.
> 
> Ok works for me. I'll let you know ASAP.

Winchip works as well if you call the ->init, and it is much happier as
a result.