[PATCH] Some more DIMM0 related cleanups and deduplication

[Uwe Hermann]

See patch.

Abuild-tested.

Uwe.

[PATCH] Some more DIMM0 related cleanups and deduplication

[Patrick Georgi]

Am 20.11.2010 21:00, schrieb Uwe Hermann:
> Some more DIMM0 related cleanups and deduplication.
>
> VIA VT8235: Do the shift in smbus_read_byte() as all other chipsets do.
>
> spd.h: Move RC00-RC63 #defines here, they were duplicated in lots of
> romstage.c files and lots of spd_addr.h files. Don't even bother for
> those spd_addr.h which aren't even actually used, drop them right away.
>
> Replace various 0x50 hardcoded numbers with DIMM0, 0x51 with DIMM1,
> and 0xa0 with (DIMM0 << 1) where appropriate.
>
> Various debug.c files: Replace SMBUS_MEM_DEVICE_START with DIMM0,
> SMBUS_MEM_DEVICE_END with DIMM7, and drop useless SMBUS_MEM_DEVICE_INC.
>
> VIA VX800: Drop unused SMBUS_ADDR_CH* #defines.
>
> VIA VT8623: Do the shift in smbus_read_byte() as all other chipsets do.
> Then, replace 0xa0 (which now becomes 0x50) with DIMM0.
>
> alix1c/romstage.c, alix2d/romstage.c: Adapt to recent bit shift changes.
>
> Various files: Drop DIMM_SPD_BASE and/or replace it with DIMM0.
>
> Signed-off-by: Uwe Hermann

Acked-by: Patrick Georgi

I really like where this is heading (and it also fixes a couple of bugs
my patch introduced *blush*)

Patrick

[PATCH] Some more DIMM0 related cleanups and deduplication

[Uwe Hermann]

On Sat, Nov 20, 2010 at 09:07:05PM +0100, Patrick Georgi wrote:
> Acked-by: Patrick Georgi

Thanks, r6100.

Uwe.

[PATCH] Some more DIMM0 related cleanups and deduplication

[Peter Stuge]

Uwe Hermann wrote:
> spd.h: Move RC00-RC63 #defines here
> +++ src/include/spd.h	(Arbeitskopie)
> @@ -166,5 +166,70 @@
>  #define DIMM6                            0x56
>  #define DIMM7                            0x57
>
> -#endif /* _SPD_H_ */
> +#define RC00 0
> +#define RC01 1
> +#define RC02 2
> +#define RC03 3
> +#define RC04 4
> +#define RC05 5
> +#define RC06 6
> +#define RC07 7
> +#define RC08 8
> +#define RC09 9
> +#define RC10 10
> +#define RC11 11
> +#define RC12 12
> +#define RC13 13
> +#define RC14 14
> +#define RC15 15
> +#define RC16 16
> +#define RC17 17
> +#define RC18 18
> +#define RC19 19
> +#define RC20 20
> +#define RC21 21
> +#define RC22 22
> +#define RC23 23
> +#define RC24 24
> +#define RC25 25
> +#define RC26 26
> +#define RC27 27
> +#define RC28 28
> +#define RC29 29
> +#define RC30 30
> +#define RC31 31
>
> +#define RC32 32
> +#define RC33 33
> +#define RC34 34
> +#define RC35 35
> +#define RC36 36
> +#define RC37 37
> +#define RC38 38
> +#define RC39 39
> +#define RC40 40
> +#define RC41 41
> +#define RC42 42
> +#define RC43 43
> +#define RC44 44
> +#define RC45 45
> +#define RC46 46
> +#define RC47 47
> +#define RC48 48
> +#define RC49 49
> +#define RC50 50
> +#define RC51 51
> +#define RC52 52
> +#define RC53 53
> +#define RC54 54
> +#define RC55 55
> +#define RC56 56
> +#define RC57 57
> +#define RC58 58
> +#define RC59 59
> +#define RC60 60
> +#define RC61 61
> +#define RC62 62
> +#define RC63 63

Seriously? Please just remove these completely.
> +++ src/mainboard/supermicro/x6dai_g/debug.c	(Arbeitskopie)

..
>    device = DIMM0;
>    while(device <= DIMM7) {
>            int status = 0;
>            int i;
>    	print_debug("\n");
>
> @@ -296,7 +294,7 @@
>    		print_debug_hex8(status);
>    		print_debug_char(' ');
>    	}
> device += SMBUS_MEM_DEVICE_INC;
> device++;
> print_debug("\n");
> }

The above is usually written with for().
> static const u8 spd_addr[] = {
>    //first node
>    RC00, DIMM0, DIMM2, 0, 0, DIMM1, DIMM3, 0, 0,

Is this the only use of RC00? Great student code. The arrays seem to
be all the same, and replaceable with a completely trivial algorithm.
Or no?

//Peter

[PATCH] Some more DIMM0 related cleanups and deduplication

[Uwe Hermann]

On Sun, Nov 21, 2010 at 09:27:41PM +0100, Peter Stuge wrote:
> > +#define RC63 63
>
> Seriously? Please just remove these completely.

I tend to agree, but I'll have to look into what exactly these values
mean and what "RC" stands for first.
> > +++ src/mainboard/supermicro/x6dai_g/debug.c	(Arbeitskopie)
>
> ..
> >    device = DIMM0;
> >    while(device <= DIMM7) {
> >            int status = 0;
> >            int i;
> >    	print_debug("\n");
> >
> > @@ -296,7 +294,7 @@
> >    		print_debug_hex8(status);
> >    		print_debug_char(' ');
> >    	}
> > device += SMBUS_MEM_DEVICE_INC;
> > device++;
> > print_debug("\n");
> > }
>
> The above is usually written with for().

Yes. And they should also not be duplicated in a bazillion files. It's
on my TODO list.
> > static const u8 spd_addr[] = {
> >    //first node
> >    RC00, DIMM0, DIMM2, 0, 0, DIMM1, DIMM3, 0, 0,
>
> Is this the only use of RC00? Great student code. The arrays seem to
> be all the same, and replaceable with a completely trivial algorithm.
> Or no?

Maybe, will check. Fun thing is there are also some RC0, RC1 (note: only
one digit) left in the romstage.c files, but these _seem_ to be a little
different at least from a quick glance, but might also be yet another
way to express the same things. Will look into this.

Uwe.