[LinuxBIOS] [PATCH] vt8237r fixes/improvements (LBv2)

[Stefan Reinauer]

* Uwe Hermann <uwe at hermann-uwe.de> [071102 16:56]:
> >  	loops = 0;
> >  	/* Yes, this is a mess, but it's the easiest way to do it. */
> > -	while ((inb(SMBHSTSTAT) & 1) == 1 && loops <= SMBUS_TIMEOUT)
> > +	while ((inb(SMBHSTSTAT) & 1) == 1 && loops < SMBUS_TIMEOUT)
> 
> Rationale? Does it make a big difference?

Yes, the one is correct, the other is not because it counts
SMBUS_TIMEOUT+1 loops. 

> > -u8 smbus_read_byte(u32 dimm, u32 offset)
> > +/**
> > + * Read a byte from the smbus
> > + *
> > + * @param dimm The address location of the dimm on the smbus
> > + * @param offset The offset the data is located at
> > + */
> 
> > +u8 smbus_read_byte(u8 dimm, u8 offset)
> 
> I'm still not entirely sure they're always only 8 bit. Do you have a
> pointer to a datasheet or spec or standard where it's explicitly
> defined as 8 bit? Yes, it _is_ 8 bits in most cases, but can we be sure
> that it'll be 8 bit in _all_ of them? On all chipsets and controllers?

smbus address space is 8 bits. Let's reflect this in the code. We can
think about this when we find the first machine where it is not.

> > +inline u8 spd_read_byte(u32 address, u8 offset)
> > +{
> > +	return smbus_read_byte(address, offset);
> > +}
> 
> Hm, this is usually done in auto.c per-mainboard, I think. Either
> you make spd_read_byte() a wrapper for smbus_read_byte(), or you
> use the "fake spd" method to return hard-coded settings if there's
> no real SPD data to be read.
 
Yes, this is a mainboard specific abstraction. It might have to
circumvent smbus switches on the bus.

> Not sure this function makes sense in vt8237r_early_smbus.c, because of
> the above and also because it's not SMBus-related per se. I'd say drop it.
> 
> Also, address is 32bit here but 8bit in smbus_read_byte()?

That's fine. The i2c has 8 (actually 7) bits address space. spd might
not necessarily be behind the i2c bus. Or it might have a number of
switches. So you might have an address of 0xaabbccdd saying that aa, bb,
cc, dd are the settings of 3 i2c switches, while dd is the actual
address of the spd on the bus _after_ doing the other settings. This is
done on the agámi aruma in a similar way.

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866