Re: [coreboot] v3 CS5536 SMBus bug

[Stefan Reinauer]

ron minnich wrote:
> On Wed, Aug 20, 2008 at 9:30 AM, Carl-Daniel Hailfinger
>  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
> > 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