Re: [coreboot] v3 CS5536 SMBus bug

[Carl-Daniel Hailfinger]

On 20.08.2008 16:37, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
> > v3 can't use global variables in stage1 or initram. Same applies to
> > static local variables. See the bug below.
> >
> > Ideas for fixes? The generic variable infrastructure would be one option.
>
> Just call smbus_init() prior to calling smbus_read_byte() the first
> time. The variable infrastructure might be a nice idea for some things,
> but I think in cases as simple as this, we should not rely on it.
>
> Is there a method to change variables in your "variable infrastructure"
> across cpus? If so, it could be useful for semaphores / locking. But I
> don't think that's possible since the stuff is in cache, right?

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);
 }