On Fri, Nov 02, 2007 at 05:44:03PM +0100, Stefan Reinauer wrote:
-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
ACK.
think about this when we find the first machine where it is not.
I don't know if there are any, I was just speculating. If SMBus address space is specified as 8 bits u8 is the way to go.
+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.
OK, should be dropped here then.
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.
Thanks for the clarifications! For this patch that's not relevant though as spd_read_byte() should be dropped anyway then as it's board specific.
But maybe we should have this info somewhere in the wiki? http://linuxbios.org/Developer_Manual ?
Uwe.