Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for some SST flashes

List overview All Threads
Download

newer

older

Change in coreboot[master]:...

Change in ...coreboot[master]:...

Arthur Heymans (Code Review)

23 Sep 2019 23 Sep '19

11:50 a.m.

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for some SST flashes ......................................................................

sb/intel/spi: Use different SPIOPS for some SST flashes

Some SST flashes use the AAI OP to write.

UNTESTED.

Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/southbridge/intel/common/spi.c 1 file changed, 43 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/35537/1

diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 6569351..9f245c1 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -1054,12 +1054,26 @@ return 0; }

+const char *(aai_write_flash[]) = { + "SST25VF040B", + "SST25VF080B", + "SST25VF080", + "SST25VF016B", + "SST25VF032B", + "SST25WF512", + "SST25WF010", + "SST25WF020", + "SST25WF040", + "SST25WF080", + "SST25WF080B" +}; + void spi_finalize_ops(void) { struct ich_spi_controller *cntlr = &g_cntlr; u16 spi_opprefix; u16 optype = 0; - struct intel_swseq_spi_config spi_config = { + struct intel_swseq_spi_config spi_config_default = { {0x06, 0x50}, /* OPPREFIXES: EWSR and WREN */ { /* OPTYPE and OPCODE */ {0x01, WRITE_NO_ADDR}, /* WRSR: Write Status Register */ @@ -1072,19 +1086,41 @@ {0x0b, READ_WITH_ADDR}, /* FAST: Fast Read */ } }; + struct intel_swseq_spi_config spi_config_aai_write = { + {0x06, 0x50}, /* OPPREFIXES: EWSR and WREN */ + { + {0x01, WRITE_NO_ADDR}, /* WRSR: Write Status Register */ + {0x02, WRITE_WITH_ADDR}, /* BYPR: Byte Program */ + {0x03, READ_WITH_ADDR}, /* READ: Read Data */ + {0x05, READ_NO_ADDR}, /* RDSR: Read Status Register */ + {0x20, WRITE_WITH_ADDR}, /* SE20: Sector Erase 0x20 */ + {0x9f, READ_NO_ADDR}, /* RDID: Read ID */ + {0xad, WRITE_NO_ADDR}, /* Auto Address Increment Word Program */ + {0x04, WRITE_NO_ADDR} /* Write Disable */ + } + }; + const struct spi_flash *flash = boot_device_spi_flash(); + struct intel_swseq_spi_config *spi_config = &spi_config_default; int i;

+ for (i = 0; i < ARRAY_SIZE(aai_write_flash); i++) { + if (strcmp(flash->name, aai_write_flash[i])) { + spi_config = &spi_config_aai_write; + break; + } + } + if (spi_locked()) return;

- intel_southbridge_override_spi(&spi_config); + intel_southbridge_override_spi(spi_config);

- spi_opprefix = spi_config.opprefixes[0] - | (spi_config.opprefixes[1] << 8); + spi_opprefix = spi_config->opprefixes[0] + | (spi_config->opprefixes[1] << 8); writew_(spi_opprefix, cntlr->preop); - for (i = 0; i < ARRAY_SIZE(spi_config.ops); i++) { - optype |= (spi_config.ops[i].type & 3) << (i * 2); - writeb_(spi_config.ops[i].op, &cntlr->opmenu[i]); + for (i = 0; i < ARRAY_SIZE(spi_config->ops); i++) { + optype |= (spi_config->ops[i].type & 3) << (i * 2); + writeb_(spi_config->ops[i].op, &cntlr->opmenu[i]); } writew_(optype, cntlr->optype); }

Attachments:

attachment.htm (text/html — 7.3 KB)

Show replies by date

Arthur Heymans (Code Review)

24 Sep 24 Sep

1:33 p.m.

Hello Patrick Rudolph, Angel Pons, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35537

to look at the new patch set (#2).

Change subject: sb/intel/spi: Use different SPIOPS for some SST flashes ......................................................................

sb/intel/spi: Use different SPIOPS for some SST flashes

Some SST flashes use the AAI OP to write.

TESTED on Thinkpad X60 with SST25VF016B, flashrom can use AAI_WRITE op with locked down SPIOPS.

Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/southbridge/intel/common/spi.c 1 file changed, 43 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/35537/2

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Arthur Heymans (Code Review)

2:06 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for some SST flashes ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... PS2, Line 1056: : const char *(aai_write_flash[]) = { : "SST25VF040B", : "SST25VF080B", : "SST25VF080", : "SST25VF016B", : "SST25VF032B", : "SST25WF512", : "SST25WF010", : "SST25WF020", : "SST25WF040", : "SST25WF080", : "SST25WF080B" : }; Except id_code 0x4b all others use AAI_WRITE. Should that be checked for? amd/soc does it like that.

Nico Huber (Code Review)

28 Sep 28 Sep

4:48 p.m.

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for some SST flashes ......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

...

Except id_code 0x4b all others use AAI_WRITE. Should that be checked for? amd/soc does it like that.

flashchips.c also mentions a lot that use 0xaf (and don't support 0xad I guess). Those seem to be older versions, though.

Generally, I wouldn't mind a simpler check. As most of SST seems to be AAI, it would even be better to assume AAI for SST and make 0x02 the exception.

https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... PS2, Line 1102: const struct spi_flash *flash = boot_device_spi_flash(); can it return NULL?

Arthur Heymans (Code Review)

4:59 p.m.

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for some SST flashes ......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

...

flashchips.c also mentions a lot that use 0xaf (and don't support 0xad I guess). Those seem to be older versions, though.

Generally, I wouldn't mind a simpler check. As most of SST seems to be AAI, it would even be better to assume AAI for SST and make 0x02 the exception.

Flashrom seems to use 0x02 on those flashes with 0xaf AAI, so no harm seems to be done to use a simpler check for SST with just the 0x4d exception.

Arthur Heymans (Code Review)

5:10 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Hello Patrick Rudolph, Angel Pons, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35537

to look at the new patch set (#3).

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

sb/intel/spi: Use different SPIOPS for most SST flashes

Many supported SST flashes use the AAI OP (0xad) to write.

TESTED on Thinkpad X60 with SST25VF016B, flashrom can use AAI_WRITE op with locked down SPIOPS.

Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/southbridge/intel/common/spi.c 1 file changed, 31 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/35537/3

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Nico Huber (Code Review)

5:26 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35537/3/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

https://review.coreboot.org/c/coreboot/+/35537/3/src/southbridge/intel/commo... PS3, Line 1098: == != ?

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 28 Sep 2019 15:26:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans (Code Review)

5:27 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Hello Patrick Rudolph, Angel Pons, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35537

to look at the new patch set (#4).

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

sb/intel/spi: Use different SPIOPS for most SST flashes

Many supported SST flashes use the AAI OP (0xad) to write.

TESTED on Thinkpad X60 with SST25VF016B, flashrom can use AAI_WRITE op with locked down SPIOPS.

Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/southbridge/intel/common/spi.c 1 file changed, 31 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/35537/4

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

Arthur Heymans (Code Review)

5:27 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35537/3/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

https://review.coreboot.org/c/coreboot/+/35537/3/src/southbridge/intel/commo... PS3, Line 1098: ==

...

!= ?

wups

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 28 Sep 2019 15:27:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Nico Huber (Code Review)

5:42 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

Patch Set 4: Code-Review+2

(4 comments)

https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

...

...
flashchips.c also mentions a lot that use 0xaf (and don't support 0xad I guess). […]

Ack

https://review.coreboot.org/c/coreboot/+/35537/2/src/southbridge/intel/commo... PS2, Line 1102: const struct spi_flash *flash = boot_device_spi_flash();

...

can it return NULL?

Done

https://review.coreboot.org/c/coreboot/+/35537/3/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

https://review.coreboot.org/c/coreboot/+/35537/3/src/southbridge/intel/commo... PS3, Line 1098: ==

...

wups

Done

https://review.coreboot.org/c/coreboot/+/35537/4/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

https://review.coreboot.org/c/coreboot/+/35537/4/src/southbridge/intel/commo... PS4, Line 1078: {0x01, WRITE_NO_ADDR}, /* WRSR: Write Status Register */ : {0x02, WRITE_WITH_ADDR}, /* BYPR: Byte Program */ : {0x03, READ_WITH_ADDR}, /* READ: Read Data */ : {0x05, READ_NO_ADDR}, /* RDSR: Read Status Register */ : {0x20, WRITE_WITH_ADDR}, /* SE20: Sector Erase 0x20 */ : {0x9f, READ_NO_ADDR}, /* RDID: Read ID */ : {0xad, WRITE_NO_ADDR}, /* Auto Address Increment Word Program */ : {0x04, WRITE_NO_ADDR} /* Write Disable */ Maybe align the comments like above?

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 28 Sep 2019 15:42:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Arthur Heymans arthur@aheymans.xyz Comment-In-Reply-To: Nico Huber nico.h@gmx.de Gerrit-MessageType: comment

Arthur Heymans (Code Review)

6:35 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Hello Patrick Rudolph, Angel Pons, build bot (Jenkins), Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/35537

to look at the new patch set (#5).

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

sb/intel/spi: Use different SPIOPS for most SST flashes

Many supported SST flashes use the AAI OP (0xad) to write.

TESTED on Thinkpad X60 with SST25VF016B, flashrom can use AAI_WRITE op with locked down SPIOPS.

Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/southbridge/intel/common/spi.c 1 file changed, 32 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/35537/5

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: newpatchset

build bot (Jenkins) (Code Review)

6:35 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35537/5/src/southbridge/intel/commo... File src/southbridge/intel/common/spi.c:

https://review.coreboot.org/c/coreboot/+/35537/5/src/southbridge/intel/commo... PS5, Line 1084: {0xad, WRITE_NO_ADDR}, /* Auto Address Increment Word Program */ line over 96 characters

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sat, 28 Sep 2019 16:35:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Nico Huber (Code Review)

29 Sep 29 Sep

3:48 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

Patch Set 5: Code-Review+2

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 5 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-Comment-Date: Sun, 29 Sep 2019 13:48:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Patrick Georgi (Code Review)

30 Sep 30 Sep

2:02 p.m.

New subject: Change in coreboot[master]: sb/intel/spi: Use different SPIOPS for most SST flashes

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35537 )

Change subject: sb/intel/spi: Use different SPIOPS for most SST flashes ......................................................................

sb/intel/spi: Use different SPIOPS for most SST flashes

Many supported SST flashes use the AAI OP (0xad) to write.

TESTED on Thinkpad X60 with SST25VF016B, flashrom can use AAI_WRITE op with locked down SPIOPS.

Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/35537 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/southbridge/intel/common/spi.c 1 file changed, 32 insertions(+), 8 deletions(-)

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved

diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c index 268030b..73181cf 100644 --- a/src/southbridge/intel/common/spi.c +++ b/src/southbridge/intel/common/spi.c @@ -1060,9 +1060,9 @@ struct ich_spi_controller *cntlr = car_get_var_ptr(&g_cntlr); u16 spi_opprefix; u16 optype = 0; - struct intel_swseq_spi_config spi_config = { + struct intel_swseq_spi_config spi_config_default = { {0x06, 0x50}, /* OPPREFIXES: EWSR and WREN */ - { /* OPTYPE and OPCODE */ + { /* OPCODE and OPTYPE */ {0x01, WRITE_NO_ADDR}, /* WRSR: Write Status Register */ {0x02, WRITE_WITH_ADDR}, /* BYPR: Byte Program */ {0x03, READ_WITH_ADDR}, /* READ: Read Data */ @@ -1073,19 +1073,43 @@ {0x0b, READ_WITH_ADDR}, /* FAST: Fast Read */ } }; + struct intel_swseq_spi_config spi_config_aai_write = { + {0x06, 0x50}, /* OPPREFIXES: EWSR and WREN */ + { /* OPCODE and OPTYPE */ + {0x01, WRITE_NO_ADDR}, /* WRSR: Write Status Register */ + {0x02, WRITE_WITH_ADDR}, /* BYPR: Byte Program */ + {0x03, READ_WITH_ADDR}, /* READ: Read Data */ + {0x05, READ_NO_ADDR}, /* RDSR: Read Status Register */ + {0x20, WRITE_WITH_ADDR}, /* SE20: Sector Erase 0x20 */ + {0x9f, READ_NO_ADDR}, /* RDID: Read ID */ + {0xad, WRITE_NO_ADDR}, /* Auto Address Increment Word Program */ + {0x04, WRITE_NO_ADDR} /* Write Disable */ + } + }; + const struct spi_flash *flash = boot_device_spi_flash(); + struct intel_swseq_spi_config *spi_config = &spi_config_default; int i;

+ /* + * Some older SST SPI flashes support AAI write but use 0xaf opcde for + * that. Flashrom uses the byte program opcode to write those flashes, + * so this configuration is fine too. SST25VF064C (id = 0x4b) is an + * exception. + */ + if (flash && flash->vendor == VENDOR_ID_SST && (flash->model & 0x00ff) != 0x4b) + spi_config = &spi_config_aai_write; + if (spi_locked()) return;

- intel_southbridge_override_spi(&spi_config); + intel_southbridge_override_spi(spi_config);

-- To view, visit https://review.coreboot.org/c/coreboot/+/35537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ica72eda04a8d9f4e563987871b1640565c6e7e12 Gerrit-Change-Number: 35537 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Angel Pons th3fanbus@gmail.com Gerrit-Reviewer: Arthur Heymans arthur@aheymans.xyz Gerrit-Reviewer: Nico Huber nico.h@gmx.de Gerrit-Reviewer: Patrick Georgi pgeorgi@google.com Gerrit-Reviewer: Patrick Rudolph siro@das-labor.org Gerrit-Reviewer: build bot (Jenkins) no-reply@coreboot.org Gerrit-MessageType: merged

1820

days inactive

1827

days old

coreboot-gerrit@coreboot.org

13 comments

4 participants

tags (0)

participants (4)

Arthur Heymans (Code Review)
build bot (Jenkins) (Code Review)
Nico Huber (Code Review)
Patrick Georgi (Code Review)