[coreboot] v3 smbus device structure

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sat Feb 23 01:15:12 CET 2008


On 23.02.2008 00:34, Marc Jones wrote:
>
>
> Carl-Daniel Hailfinger wrote:
>> On 22.02.2008 22:02, Marc Jones wrote:
>>> We need to discuss v3 smbus operations. Someone has done a lot of 
>>> work to make smbus_ops.c and smbus.h. The code treats smbus as a 
>>> bus, like the pci bus, with a device structure and all.
>>>
>>> This approach seems nice and maybe the right way to do it, but it is 
>>> somewhat overkill. I think that the complexity is one reason why the 
>>> structure is in place in v2 but never used. Instead, simpler 
>>> chipset/mainboard specific functions are used. The other reason is 
>>> that the smbus is accessed in ROMCC/CAR code and not in the main 
>>> coreboot bus enumeration code. My observation is that the SPD is the 
>>> only device on smbus used by most mainboards in coreboot.
>>>
>>> So, what do we want to do for v3? If we go with the bus/device 
>>> structure every mainboard in v2 will need to have the smbus 
>>> functions ported. Also, someone will have to figure out how to 
>>> describe the smbus devices in the dts and the entire thing might 
>>> need to use a simpler bus/device structure. Or, we can do as was 
>>> done in v2 and leave it to the chipset/mainboard code.
>>>   
>>
>> This is a bit more complicated than visible at first glance. We have 
>> two different smbus_read_byte() functions in v3:
>> int smbus_read_byte(u16 device, u8 address);
>> int smbus_read_byte(struct device *dev, u8 addr);
>>
>> The confusion arises because these two clearly different functions 
>> have the same name. I'd suggest to rename the first smbus_read_byte 
>> to smbus_read_byte_early to make it clear that it does not care about 
>> the device tree. All  initram stuff just uses the first function.
>
> That would work, but why have the device tree version if it is never 
> used?

That may change in the future. We keep lots of code in v3 which is not 
used yet. AGP, PCIE, CardBus etc.


Rename the non-devicetree version of smbus_read_byte() to
smbus_read_byte_early() to resolve a naming conflict.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: LinuxBIOSv3-smbusreadbyte-rename/southbridge/amd/cs5536/smbus_initram.c
===================================================================
--- LinuxBIOSv3-smbusreadbyte-rename/southbridge/amd/cs5536/smbus_initram.c	(Revision 616)
+++ LinuxBIOSv3-smbusreadbyte-rename/southbridge/amd/cs5536/smbus_initram.c	(Arbeitskopie)
@@ -319,7 +319,7 @@
  * @param address The address.
  * @return The data from the SMBus packet area or an error of 0xff (i.e. -1).
  */
-int smbus_read_byte(u16 device, u8 address)
+int smbus_read_byte_early(u16 device, u8 address)
 {
 	static int first_time = 1;
 
@@ -335,7 +335,7 @@
  * Read a byte from the SPD. 
  *
  * For this chip, that is really just saying 'read a byte from SMBus'.
- * So we use smbus_read_byte(). Nota Bene: leave this here as a function 
+ * So we use smbus_read_byte_early(). Nota Bene: leave this here as a function 
  * rather than a #define in an obscure location. This function is called 
  * only a few dozen times, and it's not performance critical. 
  *
@@ -345,5 +345,5 @@
  */
 u8 spd_read_byte(u16 device, u8 address)
 {
-	return smbus_read_byte(device, address);
+	return smbus_read_byte_early(device, address);
 }



-- 
http://www.hailfinger.org/





More information about the coreboot mailing list