Flashrom problems on SB600

[Tiago Marques]

Hi all,

I tried to run flashrom on an ECS AMD690GM-M2 motherboard and it detects the
SB600 southbridge and enables flash writing but hangs before trying to find
the flash/EEPROM chip. The system itself doesn't hang, just flashrom that
stays stalled apparently waiting for something.

Any ideas?

Best regards,

Tiago Marques

[Ward Vandewege]

Hi there,

On Sun, Mar 15, 2009 at 08:32:01PM +0000, Tiago Marques wrote:
> I tried to run flashrom on an ECS AMD690GM-M2 motherboard and it detects the
> SB600 southbridge and enables flash writing but hangs before trying to find
> the flash/EEPROM chip. The system itself doesn't hang, just flashrom that
> stays stalled apparently waiting for something.

Yeah. LPC rom chip? If so, try this patch:

http://www.coreboot.org/pipermail/coreboot/2009-February/044573.html

Thanks,
Ward.

[Tiago Marques]

Thanks!

Will do and report back, as soon as I can.

Why hasn't this patch been merged yet, it seems pretty strightforward.

Best regards,

Tiago Marques

On Sun, Mar 15, 2009 at 10:47 PM, Ward Vandewege  wrote:
> Hi there,
>
> On Sun, Mar 15, 2009 at 08:32:01PM +0000, Tiago Marques wrote:
> > I tried to run flashrom on an ECS AMD690GM-M2 motherboard and it detects
>
> the
> > SB600 southbridge and enables flash writing but hangs before trying to
>
> find
> > the flash/EEPROM chip. The system itself doesn't hang, just flashrom that
> > stays stalled apparently waiting for something.
>
> Yeah. LPC rom chip? If so, try this patch:
>
> http://www.coreboot.org/pipermail/coreboot/2009-February/044573.html
>
> Thanks,
> Ward.

[Peter Stuge]

Tiago Marques wrote:
> Why hasn't this patch been merged yet, it seems pretty
> strightforward.

The heuristic nature worries me. I would much prefer to learn that I
suck because I couldn't find the SB600 register which says whether
system booted from SPI or LPC flash, but I doubt it'll happen.

Sorry for holding it up. I completely agree that flashrom should
just work<tm> also on SB600, but I also want to solve problems as
neatly as possible and this was a bit tricky because it is new.

I am thinking that maybe SB600 SPI support could use the flashrom
plugin interface (which does not exist yet) and require explicit
enabling. We discussed explicit options back when the patch was
created, and I do want to add an option to select plugin when the
plugin interface is available. I think it would fit SB600 SPI well.

Any thoughts?

//Peter

[Carl-Daniel Hailfinger]

On 17.03.2009 00:02, Peter Stuge wrote:
> Tiago Marques wrote:
> > Why hasn't this patch been merged yet, it seems pretty
> > strightforward.
>
> The heuristic nature worries me. I would much prefer to learn that I
> suck because I couldn't find the SB600 register which says whether
> system booted from SPI or LPC flash, but I doubt it'll happen.
>
> Sorry for holding it up. I completely agree that flashrom should
> just work<tm> also on SB600, but I also want to solve problems as
> neatly as possible and this was a bit tricky because it is new.
>
> I am thinking that maybe SB600 SPI support could use the flashrom
> plugin interface (which does not exist yet) and require explicit
> enabling. We discussed explicit options back when the patch was
> created, and I do want to add an option to select plugin when the
> plugin interface is available. I think it would fit SB600 SPI well.

Can't we just rely on the SPI/LPC selection register to be unchanged
after boot? If someone deliberately changes that register, he/she sure
is capable of adding code to handle it in flashrom. The average user
shouldn't have an undue burden just because some hardcode hardware
hacker _may_ mess with some registers.

Regards,
Carl-Daniel

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> Can't we just rely on the SPI/LPC selection register to be
> unchanged after boot?

That would be perfect.
> If someone deliberately changes that register,

Which register? I couldn't find one. :\

//Peter

[Carl-Daniel Hailfinger]

On 17.03.2009 00:24, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > Can't we just rely on the SPI/LPC selection register to be
> > unchanged after boot?
>
> That would be perfect.

This hunk does it, it even explains what is being done.

--- chipset_enable.c	(revision 312)
+++ chipset_enable.c	(working copy)
@@ -680,8 +680,16 @@
    flashbus = BUS_TYPE_SB600_SPI;

/* Enable SPI ROM in SB600 PM register. */
+	/* If we enable SPI ROM here, we have to disable it after we leave.
+	 * But how can we know which ROM we are going to handle? So we have
+	 * to trade off. We only access LPC ROM if we boot via LPC ROM. And
+	 * only SPI ROM if we boot via SPI ROM. If you want to do it crossly,
+	 * you have to use the code below.
+	 */
+	/*
    OUTB(0x8f, 0xcd6);
    OUTB(0x0e, 0xcd7);
+	*/

return 0;
 }
> > If someone deliberately changes that register,
>
> Which register? I couldn't find one. :\

See the patch hunk above. Look for CD6 in the SB600 RRG. Notice CD6 is
PM_INDEX. CD7 is PM_DATA. Look at section 2.3.3.2 Power Management (PM)
Registers. 8F is FakeAsrEn.
FakeAsrEn has two interesting fields: UseBypassRom and BypassRomSel.

Basically, the patch hunk above does simply remove the forcing of SPI
access and relies on the default boot-up method. Back when you NAKed
that change, I wondered why but I assumed you wanted some more design to
go into it.

Other parts of the patch introduce timeout handling to the SB600 SPI
functions and change the programming interface a bit.

I think the patch is committable, but it should be split into multiple
pieces with one conceptual change each.

Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> > > If someone deliberately changes that register,
> >
> > Which register? I couldn't find one. :\
>
> See the patch hunk above. Look for CD6 in the SB600 RRG. Notice CD6
> is PM_INDEX. CD7 is PM_DATA. Look at section 2.3.3.2 Power
> Management (PM) Registers. 8F is FakeAsrEn.
> FakeAsrEn has two interesting fields: UseBypassRom and BypassRomSel.

Fake is the key part of those names. They are overrides only.
> Basically, the patch hunk above does simply remove the forcing of
> SPI access and relies on the default boot-up method. Back when you
> NAKed that change, I wondered why but I assumed you wanted some
> more design to go into it.

The register can not be read to determine what the system booted
from as far as I understand.

Thus it is not possible to determine whether to use the SB600 SPI
driver or use physical address reads and writes.

For some operations it does not matter, but probe and erase for SPI
chips does not work with only physical address accesses.
> I think the patch is committable,

Please reconsider?

//Peter

[Carl-Daniel Hailfinger]

On 17.03.2009 01:05, Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > > > If someone deliberately changes that register,
> > >
> > > Which register? I couldn't find one. :\
> >
> > See the patch hunk above. Look for CD6 in the SB600 RRG. Notice CD6
> > is PM_INDEX. CD7 is PM_DATA. Look at section 2.3.3.2 Power
> > Management (PM) Registers. 8F is FakeAsrEn.
> > FakeAsrEn has two interesting fields: UseBypassRom and BypassRomSel.
>
> Fake is the key part of those names. They are overrides only.

Yes. So we leave the overrides alone. That's part I of the solution.
> > Basically, the patch hunk above does simply remove the forcing of
> > SPI access and relies on the default boot-up method. Back when you
> > NAKed that change, I wondered why but I assumed you wanted some
> > more design to go into it.
>
> The register can not be read to determine what the system booted
> from as far as I understand.

Correct.
> Thus it is not possible to determine whether to use the SB600 SPI
> driver or use physical address reads and writes.

You can use the half dozen other registers depending indirectly on the
ROM strap for a good heuristic. Search for "LPC" and "FWH" in the RRG if
you really want that.
> For some operations it does not matter, but probe and erase for SPI
> chips does not work with only physical address accesses.

So what? The timeout part of the patch makes sure SPI accesses won't
hang forever. Enable both by default and flashrom may be slow, but it
won't hang.
> > I think the patch is committable,
>
> Please reconsider?

Which part of "the patch works for both LPC and SPI boards" is
objectionable?

Anyway, the forced ROM strap override must go. (Post 1.0 we can still
enable it again if the user forces SB600 SPI.)
The timeout is needed as well. Not having a timeout is waiting for a
disaster to happen. Other drivers have timeouts as well.

After that, no controversial changes remain. Sure, conceptually we want
to be able to determine the ROM straps, but as long as flashrom is just
slow during probe, people won't lose sleep over it.

AFAIK the patch has three Acks or so. This is more than what most other
flashrom patches have.

Regards,
Carl-Daniel

[FENG Yu Ning]

Peter Stuge wrote:
> Carl-Daniel Hailfinger wrote:
> > Can't we just rely on the SPI/LPC selection register to be
> > unchanged after boot?
>
> Which register? I couldn't find one. :\

BDG Rev.3.00 Section 3.14 says:

``Note: ... There is no register status to reflect whether the current ROM
      interface is LPC or SPI."

Is that the (sad) answer?

yu ning

[Tiago Marques]

Thanks! Got a change to try it out yesterday and it worked perfectly. No
delays checking for SPI, worked as expected. Merge, please?

Best regards,

Tiago Marques

On Sun, Mar 15, 2009 at 10:47 PM, Ward Vandewege  wrote:
> Hi there,
>
> On Sun, Mar 15, 2009 at 08:32:01PM +0000, Tiago Marques wrote:
> > I tried to run flashrom on an ECS AMD690GM-M2 motherboard and it detects
>
> the
> > SB600 southbridge and enables flash writing but hangs before trying to
>
> find
> > the flash/EEPROM chip. The system itself doesn't hang, just flashrom that
> > stays stalled apparently waiting for something.
>
> Yeah. LPC rom chip? If so, try this patch:
>
> http://www.coreboot.org/pipermail/coreboot/2009-February/044573.html
>
> Thanks,
> Ward.

[Carl-Daniel Hailfinger]

On 23.03.2009 19:24, Tiago Marques wrote:
> Thanks! Got a change to try it out yesterday and it worked perfectly. No
> delays checking for SPI, worked as expected. Merge, please?

An alternative fix has been merged.

Regards,
Carl-Daniel

[Tiago Marques]

Tks!

Best regards,

Tiago Marques

On Thu, May 14, 2009 at 10:02 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> On 23.03.2009 19:24, Tiago Marques wrote:
> > Thanks! Got a change to try it out yesterday and it worked perfectly. No
> > delays checking for SPI, worked as expected. Merge, please?
>
> An alternative fix has been merged.
>
> Regards,
> Carl-Daniel