[LinuxBIOS] patch for geodelx in arch/x86

[ron minnich]

add one debug print, move all smbus_read_byte to spd_read_byte.

attached.

ron

[Corey Osgood]

ron minnich wrote:
> add one debug print, move all smbus_read_byte to spd_read_byte.
>
> attached.
>
> ron

Acked-by: Corey Osgood

[Jordan Crouse]

On 27/11/07 08:28 -0800, ron minnich wrote:
> add one debug print, move all smbus_read_byte to spd_read_byte.

Sorry, I didn't catch this before.  I think this might be a bad move - 
smbus isn't just for reading spd anymore.  There are thermal sensors,
GPIO extenders and other things out there that we might need to get on
top of.  We've already seen this in v2 with OLPC - we needed SMBUS to
talk to the display.

I don't mind the name spd_read_byte, but I think it should be a 
derivative of a core smbus_read_byte function rather then something that
just lives in an early stage (initram, or whatever it is kids are calling
it today).

Jordan
> attached.
>
> ron
> Add a debug print. Change all smbus_read_byte to spd_read_byte.
>
> Signed-off-by: Ronald G. Minnich
> Index: arch/x86/geodelx/geodelx.c
>
> --- arch/x86/geodelx/geodelx.c	(revision 524)
> +++ arch/x86/geodelx/geodelx.c	(working copy)
> @@ -150,6 +150,7 @@
>  {
>    struct msr msr_glcp_sys_pll;	/* GeodeLink PLL control MSR */
>
> printk(BIOS_DEBUG, "pll_reset: read msr %#x\n", GLCP_SYS_RSTPLL);
> msr_glcp_sys_pll = rdmsr(GLCP_SYS_RSTPLL);
>
> printk(BIOS_DEBUG, "_MSR GLCP_SYS_RSTPLL (%08x) value is: %08x:%08x\n",
>
> @@ -346,20 +347,20 @@
>     * Note: We only support a module width of 64.
>     */
>    dimms = 0;
> spdbyte0 = smbus_read_byte(dimm0, SPD_PRIMARY_SDRAM_WIDTH);
> spdbyte0 = spd_read_byte(dimm0, SPD_PRIMARY_SDRAM_WIDTH);
> if (spdbyte0 != 0xFF) {
> dimms++;
> spdbyte0 = (u8)64 / spdbyte0 *
> 	   (u8)(smbus_read_byte(dimm0, SPD_NUM_DIMM_BANKS));
> 	   (u8)(spd_read_byte(dimm0, SPD_NUM_DIMM_BANKS));
> } else {
> spdbyte0 = 0;
> }
> spdbyte1 = smbus_read_byte(dimm1, SPD_PRIMARY_SDRAM_WIDTH);
> spdbyte1 = spd_read_byte(dimm1, SPD_PRIMARY_SDRAM_WIDTH);
> if (spdbyte1 != 0xFF) {
> dimms++;
> spdbyte1 = (u8)64 / spdbyte1 *
> 	   (u8)(smbus_read_byte(dimm1, SPD_NUM_DIMM_BANKS));
> 	   (u8)(spd_read_byte(dimm1, SPD_NUM_DIMM_BANKS));
> } else {
> spdbyte1 = 0;
> }
>

[ron minnich]

On Nov 27, 2007 8:51 AM, Jordan Crouse  wrote:
> On 27/11/07 08:28 -0800, ron minnich wrote:
> > add one debug print, move all smbus_read_byte to spd_read_byte.
>
> Sorry, I didn't catch this before.  I think this might be a bad move -
> smbus isn't just for reading spd anymore.  There are thermal sensors,
> GPIO extenders and other things out there that we might need to get on
> top of.  We've already seen this in v2 with OLPC - we needed SMBUS to
> talk to the display.

I'm not really removing smbus stuff at all. but the ram stuff should
use spd_read_byte, so you can fake it out as I am doing on alix1c and
as we have done on other platforms.

So if you are doing stuff that is spd-related, you really should use
spd_read_byte, since some systems don't even have an smbus.

If you are doing smbus stuff, by all means use smbus functions.

Make sense? Did I do the wrong file or something?

[note: IIRC, you're the guy who told me to use a fake spd function --
you or Marc :-)]

ron

[Jordan Crouse]

On 27/11/07 08:52 -0800, ron minnich wrote:
> On Nov 27, 2007 8:51 AM, Jordan Crouse  wrote:
> > On 27/11/07 08:28 -0800, ron minnich wrote:
> > > add one debug print, move all smbus_read_byte to spd_read_byte.
> >
> > Sorry, I didn't catch this before.  I think this might be a bad move -
> > smbus isn't just for reading spd anymore.  There are thermal sensors,
> > GPIO extenders and other things out there that we might need to get on
> > top of.  We've already seen this in v2 with OLPC - we needed SMBUS to
> > talk to the display.
>
> I'm not really removing smbus stuff at all. but the ram stuff should
> use spd_read_byte, so you can fake it out as I am doing on alix1c and
> as we have done on other platforms.

Okay, i just wanted to make sure that we don't make the mistake of assuming
that smbus is only good for SPD.

In yourcase, faking it out is the right move - and for those
systems that do have SPD, your spd_read_byte can just call into 
smbus_read_byte, perhaps through some clever #defines or other such thing.
> So if you are doing stuff that is spd-related, you really should use
> spd_read_byte, since some systems don't even have an smbus.

Agreed.
> [note: IIRC, you're the guy who told me to use a fake spd function --
> you or Marc :-)]

Probably Marc.  He is the smart one.

[ron minnich]

On Nov 27, 2007 9:00 AM, Jordan Crouse  wrote:
> In yourcase, faking it out is the right move - and for those
> systems that do have SPD, your spd_read_byte can just call into
> smbus_read_byte, perhaps through some clever #defines or other such thing.

dang, we're clever :

int spd_read_byte(u16 device, u8 address)
{
        return smbus_read_byte(device, address);
}
"southbridge/amd/cs5536/smbus_initram.c" 349L, 9276C

so we're good on that score.

ron

[Marc Jones]

ron minnich wrote:
> add one debug print, move all smbus_read_byte to spd_read_byte.
>
> attached.
>
> ron

Acked-by: Marc Jones

[Uwe Hermann]

On Tue, Nov 27, 2007 at 11:37:47AM -0500, Corey Osgood wrote:
> Acked-by: Corey Osgood

This is r527.

Uwe.