[patch] flashrom: resending my patch about SPI/LPC conflicts

[Bao, Zheng]

Signed-off-by: Zheng Bao <[e-mail]>

This patch is about flashrom running on dbm690t. It was sent several
months ago and hasn't got any response yet. Now it deals with 3
problems.
1. Fix the bug that the flashrom would hang if there is not SPI chip. A
timeout detection was added.
2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM if we
boot via SPI ROM. Doing crossly is not allowed. Anybody has better idea?
3. When we read/write SPI, we use memory read/write instead of sending
SPI command.

Joe

[patch] flashrom: resending my patch about SPI/LPCconflicts

[Bao, Zheng]

Does anybody give some comments?

Joe

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Bao, Zheng
Sent: Monday, February 02, 2009 12:12 PM
To: Coreboot
Subject: [coreboot] [patch] flashrom: resending my patch about
SPI/LPCconflicts

Signed-off-by: Zheng Bao <[e-mail]>

This patch is about flashrom running on dbm690t. It was sent several
months ago and hasn't got any response yet. Now it deals with 3
problems.
1. Fix the bug that the flashrom would hang if there is not SPI chip. A
timeout detection was added.
2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM if we
boot via SPI ROM. Doing crossly is not allowed. Anybody has better idea?
3. When we read/write SPI, we use memory read/write instead of sending
SPI command.

Joe

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Carl-Daniel Hailfinger]

Hi Zheng,

On 02.02.2009 05:12, Bao, Zheng wrote:
> Signed-off-by: Zheng Bao <[e-mail]>
>
> This patch is about flashrom running on dbm690t. It was sent several
> months ago and hasn't got any response yet. Now it deals with 3
> problems.
> 1. Fix the bug that the flashrom would hang if there is not SPI chip. A
> timeout detection was added.
> 2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM if we
> boot via SPI ROM. Doing crossly is not allowed. Anybody has better idea?
> 3. When we read/write SPI, we use memory read/write instead of sending
> SPI command.

I had no access to my hardware for a few weeks. I will test now.

Regards,
Carl-Daniel

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

flashrom SB600 LPC vs SPI, ROM straps

[Peter Stuge]

Hello,

Bao, Zheng wrote:
> This patch is about flashrom running on dbm690t.
> It was sent several months ago and hasn't got any response yet.

Thanks for sending it, and thank you very much for the reminder!
I sometimes forget about patches on the mailing list. Sorry for
the delay.
> Now it deals with 3 problems.
> 1. Fix the bug that the flashrom would hang if there is not SPI
>    chip. A timeout detection was added.

Carl-Daniel confirmed that flashrom hangs while trying to probe for
SPI flash chips, if the system is not using SPI.
> 2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM
>    if we boot via SPI ROM. Doing crossly is not allowed. Anybody
>    has better idea?

This is a good idea, but at the moment enable_flash_sb600() in
chipset_enable.c assumes that SB600 is always using SPI.

The best fix for 2. (and I believe it will also fix 1. without the
need for a timeout, but the timeout can be a good idea anyway) is to
make sure that enable_flash_sb600() sets

flashbus = BUS_TYPE_SB600_SPI;

only when SPI is actually used. In that case flashrom will not call
any SB600 SPI functions.

If the enable functions do not set flashbus then flashrom falls back
to assume direct CPU access, which will work correctly.

The SB600 RRG says that the BypassRom bits in FakeAsrEn (0xcd6 0x8f)
will only _override_ the strap setting, so I think that they can not
be used by software to read the ROM straps.

We need another method to find out what was used to boot the system.
The only way to more information that I can find is to read the LPC
ROM Address Range registers in the LPC/ISA bridge, register 0x68 and
test for the special value meaning that the LPC strap is disabled.
RRG page 256.

However: LPC being disabled does not mean that SPI is enabled. The
straps can also be FWH and PCI. How can we read the ROM straps?
> 3. When we read/write SPI, we use memory read/write instead of
>    sending SPI command.

Yes, this is a really nice feature of the SB600! :)


//Peter

flashrom SB600 LPC vs SPI, ROM straps

[Bao, Zheng]

Hi,
About 2, we don't have to know where the board boots. If we assume the
cross-burning is not allowed, the flashrom can not detect the existence
of a SPI chip. Then it will not do any SPI action. Right?

Zheng

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Peter Stuge
Sent: Wednesday, February 04, 2009 11:40 AM
To: [e-mail]
Subject: Re: [coreboot] flashrom SB600 LPC vs SPI, ROM straps

Hello,

Bao, Zheng wrote:
> This patch is about flashrom running on dbm690t.
> It was sent several months ago and hasn't got any response yet.

Thanks for sending it, and thank you very much for the reminder!
I sometimes forget about patches on the mailing list. Sorry for
the delay.
> Now it deals with 3 problems.
> 1. Fix the bug that the flashrom would hang if there is not SPI
>    chip. A timeout detection was added.

Carl-Daniel confirmed that flashrom hangs while trying to probe for
SPI flash chips, if the system is not using SPI.
> 2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM
>    if we boot via SPI ROM. Doing crossly is not allowed. Anybody
>    has better idea?

This is a good idea, but at the moment enable_flash_sb600() in
chipset_enable.c assumes that SB600 is always using SPI.

The best fix for 2. (and I believe it will also fix 1. without the
need for a timeout, but the timeout can be a good idea anyway) is to
make sure that enable_flash_sb600() sets

flashbus = BUS_TYPE_SB600_SPI;

only when SPI is actually used. In that case flashrom will not call
any SB600 SPI functions.

If the enable functions do not set flashbus then flashrom falls back
to assume direct CPU access, which will work correctly.

The SB600 RRG says that the BypassRom bits in FakeAsrEn (0xcd6 0x8f)
will only _override_ the strap setting, so I think that they can not
be used by software to read the ROM straps.

We need another method to find out what was used to boot the system.
The only way to more information that I can find is to read the LPC
ROM Address Range registers in the LPC/ISA bridge, register 0x68 and
test for the special value meaning that the LPC strap is disabled.
RRG page 256.

However: LPC being disabled does not mean that SPI is enabled. The
straps can also be FWH and PCI. How can we read the ROM straps?
> 3. When we read/write SPI, we use memory read/write instead of
>    sending SPI command.

Yes, this is a really nice feature of the SB600! :)


//Peter

--
coreboot mailing list: [e-mail]
http://www.coreboot.org/mailman/listinfo/coreboot

flashrom SB600 LPC vs SPI, ROM straps

[Peter Stuge]

Bao, Zheng wrote:
> About 2, we don't have to know where the board boots.

I'm afraid we do.
> If we assume the cross-burning is not allowed, the flashrom can not
> detect the existence of a SPI chip. Then it will not do any SPI
> action. Right?

The flashbus variable is used to control if and how SPI commands are
performed by flashrom.

Existing chipset enable code for SB600 unconditionally enables SPI in
the PM registers, and sets flashbus = BUS_TYPE_SB600_SPI. This does
not work, as can be seen on SB600 boards with LPC flash. Leaving the
PM registers untouched and setting flashbus = BUS_TYPE_SB600_SPI also
does not work, because erase and probe commands will be sent via SPI.

flashrom must detect what the system is actually using so that the
flashbus variable is set correctly.


//Peter

flashrom SB600 LPC vs SPI, ROM straps

[Bao, Zheng]

That is why I send the patch which removes code that "unconditionally
enables SPI in the PM registers". If the SPI commands failed, it will go
on probing other chips. And then it will find the LPC flashchip which it
should work with.

Zheng

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Peter Stuge
Sent: Wednesday, February 04, 2009 12:28 PM
To: [e-mail]
Cc: Perley, Tim
Subject: Re: [coreboot] flashrom SB600 LPC vs SPI, ROM straps

Bao, Zheng wrote:
> About 2, we don't have to know where the board boots.

I'm afraid we do.
> If we assume the cross-burning is not allowed, the flashrom can not
> detect the existence of a SPI chip. Then it will not do any SPI
> action. Right?

The flashbus variable is used to control if and how SPI commands are
performed by flashrom.

Existing chipset enable code for SB600 unconditionally enables SPI in
the PM registers, and sets flashbus = BUS_TYPE_SB600_SPI. This does
not work, as can be seen on SB600 boards with LPC flash. Leaving the
PM registers untouched and setting flashbus = BUS_TYPE_SB600_SPI also
does not work, because erase and probe commands will be sent via SPI.

flashrom must detect what the system is actually using so that the
flashbus variable is set correctly.


//Peter

--
coreboot mailing list: [e-mail]
http://www.coreboot.org/mailman/listinfo/coreboot

flashrom SB600 LPC vs SPI, ROM straps

[Peter Stuge]

Hi,

Bao, Zheng wrote:
> That is why I send the patch which removes code that
> "unconditionally enables SPI in the PM registers". If the SPI
> commands failed, it will go on probing other chips. And then it
> will find the LPC flashchip which it should work with.

SPI commands will hang if there is no SPI flash chip so they must
not be used blindly. Do you know what state the SPI hardware will
be in if an SPI command is executed without an SPI flash chip?
> > Leaving the PM registers untouched and setting
> > flashbus = BUS_TYPE_SB600_SPI also does not work, because
> > erase and probe commands will be sent via SPI.

Please consider this again. By "leave untouched" I mean that flashrom
does not set these registers, as in your suggested patch. If flashbus
is set to BUS_TYPE_SB600_SPI, sb600_spi_ functions will be called.
The way flashrom is written, each function can not fall back to
direct CPU access. The flashbus variable was introduced to control
this exact thing and it needs to be set to reflect the hardware.
> > flashrom must detect what the system is actually using so that the
> > flashbus variable is set correctly.

Unfortunately we can not get around this. For flashrom to do the
right thing, we must check (or worst case, guess) what the system is
using.


SPI command timeout is not as good indication as if we can find out
the value of the ROM strap through other means. Maybe there is even
risk that issuing a command that hangs has a negative effect on the
LPC/SPI state. In particular, I am worried about the SPI/LPC
translation that SB600 can do which transparently allows the MAC
access to an LPC flash chip via SPI.


//Peter

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Ward Vandewege]

Hi Joe,

Thanks for this patch. It fixes flashrom on the asus m2a-vm - without it
flashrom just hangs indefinitely.

Thanks,
Ward.

On Mon, Feb 02, 2009 at 12:12:17PM +0800, Bao, Zheng wrote:
> Signed-off-by: Zheng Bao <[e-mail]>
>
> This patch is about flashrom running on dbm690t. It was sent several
> months ago and hasn't got any response yet. Now it deals with 3
> problems.
> 1. Fix the bug that the flashrom would hang if there is not SPI chip. A
> timeout detection was added.
> 2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM if we
> boot via SPI ROM. Doing crossly is not allowed. Anybody has better idea?
> 3. When we read/write SPI, we use memory read/write instead of sending
> SPI command.
>
> Joe
>
> !DSPAM:498672cf103771030414324!
> !DSPAM:498672cf103771030414324!

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Peter Stuge]

Ward Vandewege wrote:
> Thanks for this patch. It fixes flashrom on the asus m2a-vm -
> without it flashrom just hangs indefinitely.

It's not a very generic fix. It's basically a compile time decision
whether to use SPI or LPC on SB600 chipsets.

Is there no way we can detect what the system booted from?


//Peter

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Ward Vandewege]

On Fri, Feb 20, 2009 at 06:35:11PM +0100, Peter Stuge wrote:
> Ward Vandewege wrote:
> > Thanks for this patch. It fixes flashrom on the asus m2a-vm -
> > without it flashrom just hangs indefinitely.
>
> It's not a very generic fix. It's basically a compile time decision
> whether to use SPI or LPC on SB600 chipsets.

I understand. It works around the hang for me.
> Is there no way we can detect what the system booted from?

Is that what should decide which rom to flash? Are there scenarios where that
would cause problems?

I suppose the 'easiest' way to fix this would be adding a command line option
that forces LPC/SPI?

Or, perhaps make the decision based on which board is detected? That brings
us back to the age-old problem of identifying boards based on PCI ids though.

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior Systems Administrator

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Peter Stuge]

Ward Vandewege wrote:
> > It's not a very generic fix. It's basically a compile time decision
> > whether to use SPI or LPC on SB600 chipsets.
>
> I understand. It works around the hang for me.

But it breaks all SB600 boards with SPI flash.
> > Is there no way we can detect what the system booted from?
>
> Is that what should decide which rom to flash?

I think that is the best we can do at this point.
> Are there scenarios where that would cause problems?

I can't think of one. Do you have something in mind?
> I suppose the 'easiest' way to fix this would be adding a command
> line option that forces LPC/SPI?

It just makes me really sad and annoyed to require manual control
like that when the SB600 for all other flash chip purposes is totally
excellent - nice and simple! I just don't want to believe that noone
thought of mirroring the boot straps in some register.
> Or, perhaps make the decision based on which board is detected?

Could do - would require every SB600 board to have an explicit
board_enable entry..
> That brings us back to the age-old problem of identifying boards
> based on PCI ids though.

..and quite likely instead require the use of -m for every SB600
board.


//Peter

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Carl-Daniel Hailfinger]

On 20.02.2009 18:53, Peter Stuge wrote:
> Ward Vandewege wrote:
> > > It's not a very generic fix. It's basically a compile time decision
> > > whether to use SPI or LPC on SB600 chipsets.
> >
> > I understand. It works around the hang for me.
>
> But it breaks all SB600 boards with SPI flash.

I thought the patch just didn't enable SPI unconditionally any more. Do
you have reports of breakage with that patch on SB600 SPI boards?

Regards,
Carl-Daniel

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

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Ward Vandewege]

On Fri, Feb 20, 2009 at 06:53:24PM +0100, Peter Stuge wrote:
> Ward Vandewege wrote:
> > > It's not a very generic fix. It's basically a compile time decision
> > > whether to use SPI or LPC on SB600 chipsets.
> >
> > I understand. It works around the hang for me.
>
> But it breaks all SB600 boards with SPI flash.

Ah, I didn't realize that.
> > > Is there no way we can detect what the system booted from?
> >
> > Is that what should decide which rom to flash?
>
> I think that is the best we can do at this point.
> > Are there scenarios where that would cause problems?
>
> I can't think of one. Do you have something in mind?

I was wondering if there would be a case where you boot from an SPI rom and
wanted to flash PLCC or something like that. But let's ignore that for now.
> > I suppose the 'easiest' way to fix this would be adding a command
> > line option that forces LPC/SPI?
>
> It just makes me really sad and annoyed to require manual control
> like that when the SB600 for all other flash chip purposes is totally
> excellent - nice and simple! I just don't want to believe that noone
> thought of mirroring the boot straps in some register.

Oh, that would be very sad indeed.
> > Or, perhaps make the decision based on which board is detected?
>
> Could do - would require every SB600 board to have an explicit
> board_enable entry..

Or maybe just the ones that we know have LPC? That's what I was thinking.

Thanks,
Ward.

-- 
Ward Vandewege <[e-mail]>
Free Software Foundation - Senior Systems Administrator

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Carl-Daniel Hailfinger]

On 20.02.2009 18:53, Peter Stuge wrote:
> It just makes me really sad and annoyed to require manual control
> like that when the SB600 for all other flash chip purposes is totally
> excellent - nice and simple! I just don't want to believe that noone
> thought of mirroring the boot straps in some register.

IIRC the LPC/SPI/FWH (yes, also FWH) straps were stored in some register
on SB600. At least that's the impression I got from reading the data
sheets back then.

Regards,
Carl-Daniel

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

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Peter Stuge]

Carl-Daniel Hailfinger wrote:
> > I just don't want to believe that noone thought of mirroring the
> > boot straps in some register.
>
> IIRC the LPC/SPI/FWH (yes, also FWH) straps were stored in some
> register on SB600. At least that's the impression I got from
> reading the data sheets back then.

I looked but was unable to find that information.


There's a strap override in PM_Reg:8Fh on page 169 of the RRG.

And the default of the LPC ROM Address Range registers (page 256)
changes depending on the Lpc_Rom strap, but the value at flashrom run
time says nothing really about the reset value of the register.


//Peter

[patch] flashrom: resending my patch aboutSPI/LPC conflicts

[Bao, Zheng]

If we can't find the best solution to fix everything, could we just stop
the board from hanging currently?

Zheng

-----Original Message-----
From: [e-mail]
[mailto:[e-mail]] On Behalf Of Peter Stuge
Sent: Saturday, February 21, 2009 2:18 AM
To: [e-mail]
Subject: Re: [coreboot] [patch] flashrom: resending my patch
aboutSPI/LPC conflicts

Carl-Daniel Hailfinger wrote:
> > I just don't want to believe that noone thought of mirroring the
> > boot straps in some register.
>
> IIRC the LPC/SPI/FWH (yes, also FWH) straps were stored in some
> register on SB600. At least that's the impression I got from
> reading the data sheets back then.

I looked but was unable to find that information.


There's a strap override in PM_Reg:8Fh on page 169 of the RRG.

And the default of the LPC ROM Address Range registers (page 256)
changes depending on the Lpc_Rom strap, but the value at flashrom run
time says nothing really about the reset value of the register.


//Peter

--
coreboot mailing list: [e-mail]
http://www.coreboot.org/mailman/listinfo/coreboot

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Carl-Daniel Hailfinger]

On 02.02.2009 05:12, Bao, Zheng wrote:
> Signed-off-by: Zheng Bao <[e-mail]>
>
> This patch is about flashrom running on dbm690t. It was sent several
> months ago and hasn't got any response yet. Now it deals with 3
> problems.
> 1. Fix the bug that the flashrom would hang if there is not SPI chip. A
> timeout detection was added.
> 2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM if we
> boot via SPI ROM. Doing crossly is not allowed. Anybody has better idea?
> 3. When we read/write SPI, we use memory read/write instead of sending
> SPI command.

I acked and committed part 2 in r459.
Sorry for the delay.

The other parts need to be committed separately because they fix
separate issues.

Regards,
Carl-Daniel

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

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Carl-Daniel Hailfinger]

On 02.02.2009 05:12, Bao, Zheng wrote:
> Signed-off-by: Zheng Bao <[e-mail]>
>
> This patch is about flashrom running on dbm690t. It was sent several
> months ago and hasn't got any response yet. Now it deals with 3
> problems.
> 1. Fix the bug that the flashrom would hang if there is not SPI chip. A
> timeout detection was added.
> 2. We only access LPC ROM if we boot via LPC ROM. And only SPI ROM if we
> boot via SPI ROM. Doing crossly is not allowed. Anybody has better idea?
> 3. When we read/write SPI, we use memory read/write instead of sending
> SPI command.

I have simplified the SPI read memcpy() patch a bit. What do you think?

There is one possible problem with the patch: How will it interact with
ROM region read protection (PCI config reg 0x50, 0x54, 0x58, 0x5C of the
LPC bridge)? Does that protection only affect LPC or does it affect SPI
as well? If it affects SPI, we have to check for read protection before
using memcpy().

Signed-off-by: Carl-Daniel Hailfinger <[e-mail]>

Index: flashrom-sb600_spi_read_memcpy/sb600spi.c
===================================================================
--- flashrom-sb600_spi_read_memcpy/sb600spi.c	(Revision 531)
+++ flashrom-sb600_spi_read_memcpy/sb600spi.c	(Arbeitskopie)
@@ -41,14 +41,11 @@
 
 int sb600_spi_read(struct flashchip *flash, uint8_t *buf)
 {
-	int rc = 0, i;
 	int total_size = flash->total_size * 1024;
-	int page_size = 8;
 
-	for (i = 0; i < total_size / page_size; i++)
-		spi_nbyte_read(i * page_size, (void *)(buf + i * page_size),
-			       page_size);
-	return rc;
+	memcpy(buf, (void *)flash->virtual_memory, total_size);
+
+	return 0;
 }
 
 uint8_t sb600_read_status_register(void)


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


Index: flashrom-sb600_spi_read_memcpy/sb600spi.c
===================================================================
--- flashrom-sb600_spi_read_memcpy/sb600spi.c	(Revision 531)
+++ flashrom-sb600_spi_read_memcpy/sb600spi.c	(Arbeitskopie)
@@ -41,14 +41,11 @@
 
 int sb600_spi_read(struct flashchip *flash, uint8_t *buf)
 {
-	int rc = 0, i;
 	int total_size = flash->total_size * 1024;
-	int page_size = 8;
 
-	for (i = 0; i < total_size / page_size; i++)
-		spi_nbyte_read(i * page_size, (void *)(buf + i * page_size),
-			       page_size);
-	return rc;
+	memcpy(buf, (void *)flash->virtual_memory, total_size);
+
+	return 0;
 }
 
 uint8_t sb600_read_status_register(void)

[patch] flashrom: resending my patch about SPI/LPC conflicts

[Carl-Daniel Hailfinger]

On 02.02.2009 05:12, Bao, Zheng wrote:
> Signed-off-by: Zheng Bao <[e-mail]>
>
> 3. When we read/write SPI, we use memory read/write instead of sending
> SPI command.

We have to be careful with SPI write. Some chips only support
single-byte write, others allow 128-byte write and even others allow
256-byte write. I have a small patch which will allow using 8-byte
writes on selected chips with SPI commands on SB600 and will send that
patch soon.

Regards,
Carl-Daniel

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