* Uwe Hermann uwe@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