Attention is currently required from: Robert Zieba, Paul Menzel, Jon Murphy, Edward O'Callaghan, Karthik Ramasubramanian.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63422 )
Change subject: sb600spi.c: Use SPI100 bit mappings
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/63422/comment/f066a9cd_aef12d9c
PS1, Line 8:
: Flashrom has been using legacy spi100 bit mappings when programming the
: opcode and triggering the command. These legacy bit mappings are
: deprecated in upcoming generation of AMD SoCs.
> Added more details relevant to this CL. […]
spi_ctrlr_xfer in soc/amd/common/block/spi/fch_spi_ctrl.c in coreboot might clarify a bit how this spi100 engine is used
https://review.coreboot.org/c/flashrom/+/63422/comment/3bf8fcf4_796507e9
PS1, Line 11: Stop using the legacy
: spi100 registers and use the correct ones.
> Done
give me a few hours to check with the docs that this won't break things on older platforms before landing this patch
--
To view, visit https://review.coreboot.org/c/flashrom/+/63422
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If42130757331f4294b5a42c848557d3287f24fc3
Gerrit-Change-Number: 63422
Gerrit-PatchSet: 2
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Robert Zieba <robertzieba(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Comment-Date: Thu, 07 Apr 2022 18:42:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Robert Zieba <robertzieba(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Paul Menzel, Edward O'Callaghan, Felix Held.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63423 )
Change subject: sb600spi.c: Add Promontory chipset rev 0x71
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/63423/comment/648b2b7d_66bb938b
PS1, Line 7: Add rev 0x71 as promontory
> Maybe: […]
Done
File sb600spi.c:
https://review.coreboot.org/c/flashrom/+/63423/comment/cdc08a05_7dfa54aa
PS1, Line 133: * outputs are as follows: 0x4b, 0x59 & 0x61.
> Does this list need to be updated? Or can you remove it in a follow-up?
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/63423
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2408959fbf1c105508f0a12f38418c9606280ab9
Gerrit-Change-Number: 63423
Gerrit-PatchSet: 2
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 07 Apr 2022 18:17:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment
Attention is currently required from: Robert Zieba, Paul Menzel, Jon Murphy, Edward O'Callaghan, Felix Held.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63422 )
Change subject: sb600spi.c: Use SPI100 bit mappings
......................................................................
Patch Set 2:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/63422/comment/bd80d4d2_ce0ed620
PS1, Line 11: generation
> generations?
Done
https://review.coreboot.org/c/flashrom/+/63422/comment/6537841e_fcef8cfc
PS1, Line 8:
: Flashrom has been using legacy spi100 bit mappings when programming the
: opcode and triggering the command. These legacy bit mappings are
: deprecated in upcoming generation of AMD SoCs.
> The knowledge of how SPI100 works completely is spare, can you provide more precise details here?
Added more details relevant to this CL. Also added the reference document number (for Cezanne SoC) which has some details about SPI100.
https://review.coreboot.org/c/flashrom/+/63422/comment/edc5ae1e_43074ca6
PS1, Line 11: Stop using the legacy
: spi100 registers and use the correct ones.
> Ran on Cezanne and everything looks good […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/63422
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If42130757331f4294b5a42c848557d3287f24fc3
Gerrit-Change-Number: 63422
Gerrit-PatchSet: 2
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Robert Zieba <robertzieba(a)google.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 07 Apr 2022 18:13:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Robert Zieba <robertzieba(a)google.com>
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com>
Comment-In-Reply-To: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Karthik Ramasubramanian, Felix Held.
Hello build bot (Jenkins), Robert Zieba, Raul Rangel, Jon Murphy, Edward O'Callaghan, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/63423
to look at the new patch set (#2).
Change subject: sb600spi.c: Add Promontory chipset rev 0x71
......................................................................
sb600spi.c: Add Promontory chipset rev 0x71
Sabrina SoC uses SMBUS revision code 0x71 which behaves exactly as the
promontory chip. Hence add 0x71 as promontory.
BUG=b:228238107
TEST=Build and deploy flashrom in Skyrim. Ensure that flashrom is able
to detect the SPI ROM chip, read from it and write to it successfully.
Ran flashrom_tester on Skyrim (Sabrina SoC) successfully and ensured
that all the tests passed.
Change-Id: I2408959fbf1c105508f0a12f38418c9606280ab9
Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com>
---
M sb600spi.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/63423/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/63423
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2408959fbf1c105508f0a12f38418c9606280ab9
Gerrit-Change-Number: 63423
Gerrit-PatchSet: 2
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Jon Murphy, Edward O'Callaghan, Karthik Ramasubramanian, Felix Held.
Hello build bot (Jenkins), Robert Zieba, Raul Rangel, Jon Murphy, Edward O'Callaghan, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/63422
to look at the new patch set (#2).
Change subject: sb600spi.c: Use SPI100 bit mappings
......................................................................
sb600spi.c: Use SPI100 bit mappings
On AMD SoCs that use SPI100 engine, flashrom has been using legacy
spi100 register and bit mappings when programming the engine -
specifically when programming the opcode and triggering their execution.
---------------------------------------------------------------------
| Register Name | Legacy SPI100 mapping | Updated SPI100 mapping |
|---------------|------------------------|--------------------------|
| Opcode | Offset 0 from SPI BAR | Offset 0x45 from SPI BAR |
| | Bits 0:7 | Bits 0:7 |
|---------------|------------------------|--------------------------|
| Execute Cmd | Offset 2 from SPI BAR | Offset 0x47 from SPI BAR |
| | Bit 1 | Bit 7 |
---------------------------------------------------------------------
These legacy register mappings are deprecated in upcoming generations of
AMD SoCs. Stop using the legacy spi100 registers. For more details about
SPI100 refer to document: 56569-A1 Rev 3.01
BUG=b:228238107
TEST=Build and deploy flashrom in Skyrim. Ensure that flashrom is able
to detect the SPI ROM chip, read from it and write to it successfully.
Ran flashrom_tester on Dewatt (Cezanne SoC), Dalboz (Picasso SoC)
successfully and ensured that all the tests passed.
Change-Id: If42130757331f4294b5a42c848557d3287f24fc3
Signed-off-by: Karthikeyan Ramasubramanian <kramasub(a)google.com>
---
M sb600spi.c
1 file changed, 16 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/63422/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/63422
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If42130757331f4294b5a42c848557d3287f24fc3
Gerrit-Change-Number: 63422
Gerrit-PatchSet: 2
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Karthik Ramasubramanian, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63423 )
Change subject: sb600spi.c: Add rev 0x71 as promontory
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/63423
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2408959fbf1c105508f0a12f38418c9606280ab9
Gerrit-Change-Number: 63423
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 07 Apr 2022 17:46:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jon Murphy, Edward O'Callaghan, Karthik Ramasubramanian, Felix Held.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63422 )
Change subject: sb600spi.c: Use SPI100 bit mappings
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/63422
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If42130757331f4294b5a42c848557d3287f24fc3
Gerrit-Change-Number: 63422
Gerrit-PatchSet: 1
Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-Reviewer: Robert Zieba <robertzieba(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthikeyan Ramasubramanian <kramasub(a)chromium.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Thu, 07 Apr 2022 17:45:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment