[coreboot] v3 CS5536 SMBus bug

[Stefan Reinauer]

ron minnich wrote:
> On Wed, Aug 20, 2008 at 9:30 AM, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>   
>> Thanks to Marc for answering a few questions about that code. Proposed
>> patch follows.
>> Please note that this patch will break compilation because stage1 code
>> tries to call initram code. SMBus init functions have to be moved from
>> initram to stage1.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>
>> Index: southbridge/amd/cs5536/stage1.c
>> ===================================================================
>> --- southbridge/amd/cs5536/stage1.c     (Revision 790)
>> +++ southbridge/amd/cs5536/stage1.c     (Arbeitskopie)
>> @@ -309,5 +309,5 @@
>>        cs5536_usb_swapsif();
>>        cs5536_setup_iobase();
>>        cs5536_setup_smbus_gpio();
>> -       /* cs5536_enable_smbus(); -- Leave this out for now. */
>> +       cs5536_enable_smbus();
>>  }
>> Index: southbridge/amd/cs5536/smbus_initram.c
>> ===================================================================
>> --- southbridge/amd/cs5536/smbus_initram.c      (Revision 790)
>> +++ southbridge/amd/cs5536/smbus_initram.c      (Arbeitskopie)
>> @@ -50,7 +50,7 @@
>>  * controller address. Code can call this more than once, but the effect of
>>  * doing so is uncertain due to SMBus address set.
>>  */
>> -static void smbus_init(void)
>> +static void cs5536_enable_smbus(void)
>>  {
>>        smbus_enable();
>>
>> @@ -321,13 +321,6 @@
>>  */
>>  int smbus_read_byte(u16 device, u8 address)
>>  {
>> -       static int first_time = 1;
>> -
>> -       if (first_time) {
>> -               smbus_init();
>> -               first_time = 0;
>> -       }
>> -
>>        return do_smbus_read_byte(SMBUS_IO_BASE, device, address);
>>  }
>>
>>
>>     
>
> I think we don't want to do it this way, since we can not guarantee that
> - all platforms have/need smbus (many do not)
>   
bringing the smbus controller in a known state may not be a bad idea.

> - those platforms that have/need smbus use the cs5536 smbus.
>   
on a system equipped with a CS5536? Why would anyone build another smbus
controller on the mainboard if the southbridge already has one?

> I will try another patch later today. I see no reason that initram
> code can't just call smbus_init. It's a simple fix that should solve
> the problem.
>
> Thanks
>
> ron
>
> --
> coreboot mailing list: coreboot at coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
>
>   


-- 
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