- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: flashrom: Use WP-based unlocking on opaque masters
by Sergii Dmytruk (Code Review) 04 May '23

04 May '23

Attention is currently required from: Nikolai Artemiev, Anastasia Klimchuk. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74930 ) Change subject: flashrom: Use WP-based unlocking on opaque masters ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/74930/comment/88a116c4_e917eacf PS2, Line 9: detectes nit: `detects` -- To view, visit https://review.coreboot.org/c/flashrom/+/74930 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1774ad64d82ae47cd085df6045e17e283855c01f Gerrit-Change-Number: 74930 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 04 May 2023 08:22:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom MAINTAINERS: Add Martin Roth as reviewer for AMD SPI
by Anastasia Klimchuk (Code Review) 04 May '23

04 May '23

Attention is currently required from: Martin L Roth. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74954 ) Change subject: flashrom MAINTAINERS: Add Martin Roth as reviewer for AMD SPI ...................................................................... Patch Set 1: Code-Review+2 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/74954/comment/a8dceb15_c68a22ca PS1, Line 7: as reviewer I think, drop "as reviewer", we don't use it in commit messages for this file. Patchset: PS1: Thank you Martin!! -- To view, visit https://review.coreboot.org/c/flashrom/+/74954 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a07be1549ef070ad72b8e657d72170c7e85620 Gerrit-Change-Number: 74954 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Comment-Date: Thu, 04 May 2023 02:45:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom: rename sb600spi.c to amd_spi.c
by Arthur Heymans (Code Review) 04 May '23

04 May '23

Attention is currently required from: Martin L Roth, Thomas Heijligen, Anastasia Klimchuk. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74952 ) Change subject: flashrom: rename sb600spi.c to amd_spi.c ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: https://review.coreboot.org/c/flashrom-stable/+/72577/4 adds a SPI100 driver for modern platforms, so the name sb600spi.c is not that bad? -- To view, visit https://review.coreboot.org/c/flashrom/+/74952 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13859de27e602cc6496684e6cb66b2dc2e21531a Gerrit-Change-Number: 74952 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 03 May 2023 19:46:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom MAINTAINERS: Add Martin Roth as reviewer for AMD SPI
by Martin L Roth (Code Review) 04 May '23

04 May '23

Attention is currently required from: Anastasia Klimchuk. Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74954 ) Change subject: flashrom MAINTAINERS: Add Martin Roth as reviewer for AMD SPI ...................................................................... flashrom MAINTAINERS: Add Martin Roth as reviewer for AMD SPI Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I27a07be1549ef070ad72b8e657d72170c7e85620 --- M MAINTAINERS 1 file changed, 11 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/74954/1 diff --git a/MAINTAINERS b/MAINTAINERS index 8ae44b1..5f1a9bc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -130,6 +130,7 @@ AMD SPI M: Arthur Heymans <arthur(a)aheymans.xyz> +R: Martin Roth <gaumless(a)tutanota.com> S: Maintained F: amd_spi.c -- To view, visit https://review.coreboot.org/c/flashrom/+/74954 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a07be1549ef070ad72b8e657d72170c7e85620 Gerrit-Change-Number: 74954 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: flashrom: Update 'sb600' references to 'amd'
by Martin L Roth (Code Review) 04 May '23

04 May '23

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74953 ) Change subject: flashrom: Update 'sb600' references to 'amd' ...................................................................... flashrom: Update 'sb600' references to 'amd' This goes along with renaming the sb600 file to better reflect the current situation with AMD chipsets and SoCs. Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I1200ef25b6765f809c754ae0adcdcfe680c202fd --- M amd_spi.c M chipset_enable.c M doc/classic_cli_manpage.rst M include/programmer.h 4 files changed, 103 insertions(+), 90 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/74953/1 diff --git a/amd_spi.c b/amd_spi.c index cec7e0a..8e55b4b 100644 --- a/amd_spi.c +++ b/amd_spi.c @@ -27,8 +27,8 @@ #include "spi.h" #include "platform/pci.h" -/* This struct is unused, but helps visualize the SB600 SPI BAR layout. - *struct sb600_spi_controller { +/* This struct is unused, but helps visualize the AMD SPI BAR layout. + *struct amd_spi_controller { * unsigned int spi_cntrl0; / * 00h * / * unsigned int restrictedcmd1; / * 04h * / * unsigned int restrictedcmd2; / * 08h * / @@ -58,7 +58,7 @@ #define SPI100_CMD_TRIGGER_REG 0x47 #define SPI100_EXECUTE_CMD (1 << 7) -struct sb600spi_data { +struct amdspi_data { struct flashctx *flash; uint8_t *spibar; }; @@ -153,18 +153,18 @@ return CHIPSET_AMD_UNKNOWN; } -static void reset_internal_fifo_pointer(uint8_t *sb600_spibar) +static void reset_internal_fifo_pointer(uint8_t *amd_spibar) { - mmio_writeb(mmio_readb(sb600_spibar + 2) | 0x10, sb600_spibar + 2); + mmio_writeb(mmio_readb(amd_spibar + 2) | 0x10, amd_spibar + 2); /* FIXME: This loop needs a timeout and a clearer message. */ - while (mmio_readb(sb600_spibar + 0xD) & 0x7) + while (mmio_readb(amd_spibar + 0xD) & 0x7) msg_pspew("reset\n"); } -static int compare_internal_fifo_pointer(uint8_t want, uint8_t *sb600_spibar) +static int compare_internal_fifo_pointer(uint8_t want, uint8_t *amd_spibar) { - uint8_t have = mmio_readb(sb600_spibar + 0xd) & 0x07; + uint8_t have = mmio_readb(amd_spibar + 0xd) & 0x07; want %= FIFO_SIZE_OLD; if (have != want) { msg_perr("AMD SPI FIFO pointer corruption! Pointer is %d, wanted %d\n", have, want); @@ -196,37 +196,37 @@ return 0; } -static void execute_command(uint8_t *sb600_spibar) +static void execute_command(uint8_t *amd_spibar) { msg_pspew("Executing... "); - mmio_writeb(mmio_readb(sb600_spibar + 2) | 1, sb600_spibar + 2); - while (mmio_readb(sb600_spibar + 2) & 1) + mmio_writeb(mmio_readb(amd_spibar + 2) | 1, amd_spibar + 2); + while (mmio_readb(amd_spibar + 2) & 1) ; msg_pspew("done\n"); } -static void execute_spi100_command(uint8_t *sb600_spibar) +static void execute_spi100_command(uint8_t *amd_spibar) { msg_pspew("Executing... "); - mmio_writeb(mmio_readb(sb600_spibar + SPI100_CMD_TRIGGER_REG) | SPI100_EXECUTE_CMD, - sb600_spibar + SPI100_CMD_TRIGGER_REG); - while (mmio_readb(sb600_spibar + SPI100_CMD_TRIGGER_REG) & SPI100_CMD_TRIGGER_REG) + mmio_writeb(mmio_readb(amd_spibar + SPI100_CMD_TRIGGER_REG) | SPI100_EXECUTE_CMD, + amd_spibar + SPI100_CMD_TRIGGER_REG); + while (mmio_readb(amd_spibar + SPI100_CMD_TRIGGER_REG) & SPI100_CMD_TRIGGER_REG) ; msg_pspew("done\n"); } -static int sb600_spi_send_command(const struct flashctx *flash, unsigned int writecnt, +static int amd_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { - struct sb600spi_data *sb600_data = flash->mst->spi.data; - uint8_t *sb600_spibar = sb600_data->spibar; + struct amdspi_data *amd_data = flash->mst->spi.data; + uint8_t *amd_spibar = amd_data->spibar; /* First byte is cmd which can not be sent through the FIFO. */ unsigned char cmd = *writearr++; writecnt--; msg_pspew("%s, cmd=0x%02x, writecnt=%d, readcnt=%d\n", __func__, cmd, writecnt, readcnt); - mmio_writeb(cmd, sb600_spibar + 0); + mmio_writeb(cmd, amd_spibar + 0); int ret = check_readwritecnt(flash, writecnt, readcnt); if (ret != 0) @@ -240,26 +240,26 @@ */ unsigned int readoffby1 = (writecnt > 0) ? 0 : 1; uint8_t readwrite = (readcnt + readoffby1) << 4 | (writecnt); - mmio_writeb(readwrite, sb600_spibar + 1); + mmio_writeb(readwrite, amd_spibar + 1); - reset_internal_fifo_pointer(sb600_spibar); + reset_internal_fifo_pointer(amd_spibar); msg_pspew("Filling FIFO: "); unsigned int count; for (count = 0; count < writecnt; count++) { msg_pspew("[%02x]", writearr[count]); - mmio_writeb(writearr[count], sb600_spibar + 0xC); + mmio_writeb(writearr[count], amd_spibar + 0xC); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt, sb600_spibar)) + if (compare_internal_fifo_pointer(writecnt, amd_spibar)) return SPI_PROGRAMMER_ERROR; /* * We should send the data in sequence, which means we need to reset * the FIFO pointer to the first byte we want to send. */ - reset_internal_fifo_pointer(sb600_spibar); - execute_command(sb600_spibar); - if (compare_internal_fifo_pointer(writecnt + readcnt, sb600_spibar)) + reset_internal_fifo_pointer(amd_spibar); + execute_command(amd_spibar); + if (compare_internal_fifo_pointer(writecnt + readcnt, amd_spibar)) return SPI_PROGRAMMER_ERROR; /* @@ -273,27 +273,27 @@ * the opcode, the FIFO already stores the response from the chip. * Usually, the chip will respond with 0x00 or 0xff. */ - reset_internal_fifo_pointer(sb600_spibar); + reset_internal_fifo_pointer(amd_spibar); /* Skip the bytes we sent. */ msg_pspew("Skipping: "); for (count = 0; count < writecnt; count++) { - msg_pspew("[%02x]", mmio_readb(sb600_spibar + 0xC)); + msg_pspew("[%02x]", mmio_readb(amd_spibar + 0xC)); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt, sb600_spibar)) + if (compare_internal_fifo_pointer(writecnt, amd_spibar)) return SPI_PROGRAMMER_ERROR; msg_pspew("Reading FIFO: "); for (count = 0; count < readcnt; count++) { - readarr[count] = mmio_readb(sb600_spibar + 0xC); + readarr[count] = mmio_readb(amd_spibar + 0xC); msg_pspew("[%02x]", readarr[count]); } msg_pspew("\n"); - if (compare_internal_fifo_pointer(writecnt+readcnt, sb600_spibar)) + if (compare_internal_fifo_pointer(writecnt+readcnt, amd_spibar)) return SPI_PROGRAMMER_ERROR; - if (mmio_readb(sb600_spibar + 1) != readwrite) { + if (mmio_readb(amd_spibar + 1) != readwrite) { msg_perr("Unexpected change in AMD SPI read/write count!\n"); msg_perr("Something else is accessing the flash chip and causes random corruption.\n" "Please stop all applications and drivers and IPMI which access the flash chip.\n"); @@ -308,35 +308,35 @@ const unsigned char *writearr, unsigned char *readarr) { - struct sb600spi_data *sb600_data = flash->mst->spi.data; - uint8_t *sb600_spibar = sb600_data->spibar; + struct amdspi_data *amd_data = flash->mst->spi.data; + uint8_t *amd_spibar = amd_data->spibar; /* First byte is cmd which can not be sent through the buffer. */ unsigned char cmd = *writearr++; writecnt--; msg_pspew("%s, cmd=0x%02x, writecnt=%d, readcnt=%d\n", __func__, cmd, writecnt, readcnt); - mmio_writeb(cmd, sb600_spibar + SPI100_CMD_CODE_REG); + mmio_writeb(cmd, amd_spibar + SPI100_CMD_CODE_REG); int ret = check_readwritecnt(flash, writecnt, readcnt); if (ret != 0) return ret; /* Use the extended TxByteCount and RxByteCount registers. */ - mmio_writeb(writecnt, sb600_spibar + 0x48); - mmio_writeb(readcnt, sb600_spibar + 0x4b); + mmio_writeb(writecnt, amd_spibar + 0x48); + mmio_writeb(readcnt, amd_spibar + 0x4b); msg_pspew("Filling buffer: "); unsigned int count; for (count = 0; count < writecnt; count++) { msg_pspew("[%02x]", writearr[count]); - mmio_writeb(writearr[count], sb600_spibar + 0x80 + count); + mmio_writeb(writearr[count], amd_spibar + 0x80 + count); } msg_pspew("\n"); - execute_spi100_command(sb600_spibar); + execute_spi100_command(amd_spibar); msg_pspew("Reading buffer: "); for (count = 0; count < readcnt; count++) { - readarr[count] = mmio_readb(sb600_spibar + 0x80 + (writecnt + count) % FIFO_SIZE_YANGTZE); + readarr[count] = mmio_readb(amd_spibar + 0x80 + (writecnt + count) % FIFO_SIZE_YANGTZE); msg_pspew("[%02x]", readarr[count]); } msg_pspew("\n"); @@ -371,19 +371,19 @@ "Fast Read", }; -static int set_speed(struct pci_dev *dev, enum amd_chipset amd_gen, uint8_t speed, uint8_t *sb600_spibar) +static int set_speed(struct pci_dev *dev, enum amd_chipset amd_gen, uint8_t speed, uint8_t *amd_spibar) { bool success = false; msg_pdbg("Setting SPI clock to %s (%i)... ", spispeeds[speed], speed); if (amd_gen >= CHIPSET_YANGTZE) { - rmmio_writew((speed << 12) | (speed << 8) | (speed << 4) | speed, sb600_spibar + 0x22); - uint16_t tmp = mmio_readw(sb600_spibar + 0x22); + rmmio_writew((speed << 12) | (speed << 8) | (speed << 4) | speed, amd_spibar + 0x22); + uint16_t tmp = mmio_readw(amd_spibar + 0x22); success = (((tmp >> 12) & 0xf) == speed && ((tmp >> 8) & 0xf) == speed && ((tmp >> 4) & 0xf) == speed && ((tmp >> 0) & 0xf) == speed); } else { - rmmio_writeb((mmio_readb(sb600_spibar + 0xd) & ~(0x3 << 4)) | (speed << 4), sb600_spibar + 0xd); - success = (speed == ((mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3)); + rmmio_writeb((mmio_readb(amd_spibar + 0xd) & ~(0x3 << 4)) | (speed << 4), amd_spibar + 0xd); + success = (speed == ((mmio_readb(amd_spibar + 0xd) >> 4) & 0x3)); } if (!success) { @@ -394,14 +394,14 @@ return 0; } -static int set_mode(struct pci_dev *dev, uint8_t mode, uint8_t *sb600_spibar) +static int set_mode(struct pci_dev *dev, uint8_t mode, uint8_t *amd_spibar) { msg_pdbg("Setting SPI read mode to %s (%i)... ", spireadmodes[mode], mode); - uint32_t tmp = mmio_readl(sb600_spibar + 0x00); + uint32_t tmp = mmio_readl(amd_spibar + 0x00); tmp &= ~(0x6 << 28 | 0x1 << 18); /* Clear mode bits */ tmp |= ((mode & 0x6) << 28) | ((mode & 0x1) << 18); - rmmio_writel(tmp, sb600_spibar + 0x00); - if (tmp != mmio_readl(sb600_spibar + 0x00)) { + rmmio_writel(tmp, amd_spibar + 0x00); + if (tmp != mmio_readl(amd_spibar + 0x00)) { msg_perr("FAILED!\n"); return 1; } @@ -410,7 +410,7 @@ } static int handle_speed(const struct programmer_cfg *cfg, - struct pci_dev *dev, enum amd_chipset amd_gen, uint8_t *sb600_spibar) + struct pci_dev *dev, enum amd_chipset amd_gen, uint8_t *amd_spibar) { uint32_t tmp; int16_t spispeed_idx = -1; @@ -469,7 +469,7 @@ */ if (amd_gen >= CHIPSET_BOLTON) { - tmp = mmio_readl(sb600_spibar + 0x00); + tmp = mmio_readl(amd_spibar + 0x00); uint8_t read_mode = ((tmp >> 28) & 0x6) | ((tmp >> 18) & 0x1); msg_pdbg("SPI read mode is %s (%i)\n", spireadmodes[read_mode], read_mode); @@ -477,16 +477,16 @@ msg_pdbg("spireadmode is not set, " "leaving SPI read mode unchanged.\n"); } - else if (set_mode(dev, spireadmode_idx, sb600_spibar) != 0) { + else if (set_mode(dev, spireadmode_idx, amd_spibar) != 0) { return 1; } if (amd_gen >= CHIPSET_YANGTZE) { - tmp = mmio_readb(sb600_spibar + 0x20); + tmp = mmio_readb(amd_spibar + 0x20); msg_pdbg("UseSpi100 is %sabled\n", (tmp & 0x1) ? "en" : "dis"); if ((tmp & 0x1) == 0) { - rmmio_writeb(tmp | 0x1, sb600_spibar + 0x20); - tmp = mmio_readb(sb600_spibar + 0x20) & 0x1; + rmmio_writeb(tmp | 0x1, amd_spibar + 0x20); + tmp = mmio_readb(amd_spibar + 0x20) & 0x1; if (tmp == 0) { msg_perr("Enabling Spi100 failed.\n"); return 1; @@ -494,7 +494,7 @@ msg_pdbg("Enabling Spi100 succeeded.\n"); } - tmp = mmio_readw(sb600_spibar + 0x22); /* SPI 100 Speed Config */ + tmp = mmio_readw(amd_spibar + 0x22); /* SPI 100 Speed Config */ msg_pdbg("NormSpeedNew is %s\n", spispeeds[(tmp >> 12) & 0xf]); msg_pdbg("FastSpeedNew is %s\n", spispeeds[(tmp >> 8) & 0xf]); msg_pdbg("AltSpeedNew is %s\n", spispeeds[(tmp >> 4) & 0xf]); @@ -502,15 +502,15 @@ } } else { if (amd_gen >= CHIPSET_SB89XX && amd_gen <= CHIPSET_HUDSON234) { - bool fast_read = (mmio_readl(sb600_spibar + 0x00) >> 18) & 0x1; + bool fast_read = (mmio_readl(amd_spibar + 0x00) >> 18) & 0x1; msg_pdbg("Fast Reads are %sabled\n", fast_read ? "en" : "dis"); if (fast_read) { msg_pdbg("Disabling them temporarily.\n"); - rmmio_writel(mmio_readl(sb600_spibar + 0x00) & ~(0x1 << 18), - sb600_spibar + 0x00); + rmmio_writel(mmio_readl(amd_spibar + 0x00) & ~(0x1 << 18), + amd_spibar + 0x00); } } - tmp = (mmio_readb(sb600_spibar + 0xd) >> 4) & 0x3; + tmp = (mmio_readb(amd_spibar + 0xd) >> 4) & 0x3; msg_pdbg("NormSpeed is %s\n", spispeeds[tmp]); if (spispeed_idx < 0) { spispeed_idx = 3; /* Default to 16.5 MHz */ @@ -520,7 +520,7 @@ msg_pdbg("spispeed is not set, leaving SPI speed unchanged.\n"); return 0; } - return set_speed(dev, amd_gen, spispeed_idx, sb600_spibar); + return set_speed(dev, amd_gen, spispeed_idx, amd_spibar); } static int handle_imc(const struct programmer_cfg *cfg, struct pci_dev *dev, enum amd_chipset amd_gen) @@ -575,7 +575,7 @@ static int promontory_read_memmapped(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { - struct sb600spi_data * data = (struct sb600spi_data *)flash->mst->spi.data; + struct amdspi_data * data = (struct amdspi_data *)flash->mst->spi.data; if (!data->flash) { map_flash(flash); data->flash = flash; /* keep a copy of flashctx for unmap() on tear-down. */ @@ -584,10 +584,10 @@ return 0; } -static int sb600spi_shutdown(void *data) +static int amdspi_shutdown(void *data) { - struct sb600spi_data *sb600_data = data; - struct flashctx *flash = sb600_data->flash; + struct amdspi_data *amd_data = data; + struct flashctx *flash = amd_data->flash; if (flash) finalize_flash_access(flash); @@ -595,15 +595,15 @@ return 0; } -static const struct spi_master spi_master_sb600 = { +static const struct spi_master spi_master_amd = { .max_data_read = FIFO_SIZE_OLD, .max_data_write = FIFO_SIZE_OLD - 3, - .command = sb600_spi_send_command, + .command = amd_spi_send_command, .map_flash_region = physmap, .unmap_flash_region = physunmap, .read = default_spi_read, .write_256 = default_spi_write_256, - .shutdown = sb600spi_shutdown, + .shutdown = amdspi_shutdown, }; static const struct spi_master spi_master_yangtze = { @@ -614,7 +614,7 @@ .unmap_flash_region = physunmap, .read = default_spi_read, .write_256 = default_spi_write_256, - .shutdown = sb600spi_shutdown, + .shutdown = amdspi_shutdown, }; static const struct spi_master spi_master_promontory = { @@ -625,15 +625,15 @@ .unmap_flash_region = physunmap, .read = promontory_read_memmapped, .write_256 = default_spi_write_256, - .shutdown = sb600spi_shutdown, + .shutdown = amdspi_shutdown, }; -int sb600_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev) +int amd_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev) { struct pci_dev *smbus_dev; uint32_t tmp; uint8_t reg; - uint8_t *sb600_spibar = NULL; + uint8_t *amd_spibar = NULL; /* Read SPI_BaseAddr */ tmp = pci_read_long(dev, 0xa0); @@ -645,14 +645,14 @@ return 0; /* Physical memory has to be mapped at page (4k) boundaries. */ - sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000); - if (sb600_spibar == ERROR_PTR) + amd_spibar = rphysmap("AMD SPI registers", tmp & 0xfffff000, 0x1000); + if (amd_spibar == ERROR_PTR) return ERROR_FLASHROM_FATAL; /* The low bits of the SPI base address are used as offset into * the mapped page. */ - sb600_spibar += tmp & 0xfff; + amd_spibar += tmp & 0xfff; enum amd_chipset amd_gen = determine_generation(dev); if (amd_gen == CHIPSET_AMD_UNKNOWN) @@ -713,7 +713,7 @@ * * <1> see handle_speed */ - tmp = mmio_readl(sb600_spibar + 0x00); + tmp = mmio_readl(amd_spibar + 0x00); msg_pdbg("(0x%08" PRIx32 ") SpiArbEnable=%"PRIi32"", tmp, (tmp >> 19) & 0x1); if (amd_gen >= CHIPSET_YANGTZE) msg_pdbg(", IllegalAccess=%"PRIi32"", (tmp >> 21) & 0x1); @@ -743,7 +743,7 @@ } if (amd_gen >= CHIPSET_SB89XX) { - tmp = mmio_readb(sb600_spibar + 0x1D); + tmp = mmio_readb(amd_spibar + 0x1D); msg_pdbg("Using SPI_CS%"PRId32"\n", tmp & 0x3); /* FIXME: Handle SpiProtect* configuration on Yangtze. */ } @@ -788,24 +788,24 @@ return 0; } - if (handle_speed(cfg, dev, amd_gen, sb600_spibar) != 0) + if (handle_speed(cfg, dev, amd_gen, amd_spibar) != 0) return ERROR_FLASHROM_FATAL; if (handle_imc(cfg, dev, amd_gen) != 0) return ERROR_FLASHROM_FATAL; - struct sb600spi_data *data = calloc(1, sizeof(*data)); + struct amdspi_data *data = calloc(1, sizeof(*data)); if (!data) { msg_perr("Unable to allocate space for extra SPI master data.\n"); return SPI_GENERIC_ERROR; } data->flash = NULL; - data->spibar = sb600_spibar; + data->spibar = amd_spibar; /* Starting with Yangtze the SPI controller got a different interface with a much bigger buffer. */ if (amd_gen < CHIPSET_YANGTZE) - register_spi_master(&spi_master_sb600, data); + register_spi_master(&spi_master_amd, data); else if (amd_gen == CHIPSET_YANGTZE) register_spi_master(&spi_master_yangtze, data); else diff --git a/chipset_enable.c b/chipset_enable.c index 9d3df5f..89e1eb8 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1375,7 +1375,7 @@ return enable_flash_amd_via(cfg, dev, name, 0xE0); } -static int enable_flash_sb600(const struct programmer_cfg *cfg, struct pci_dev *dev, const char *name) +static int enable_flash_amd(const struct programmer_cfg *cfg, struct pci_dev *dev, const char *name) { uint32_t prot; uint8_t reg; @@ -1408,7 +1408,7 @@ internal_buses_supported &= BUS_LPC | BUS_FWH; - ret = sb600_probe_spi(cfg, dev); + ret = amd_probe_spi(cfg, dev); /* Read ROM strap override register. */ OUTB(0x8f, 0xcd6); @@ -1433,7 +1433,7 @@ } msg_pdbg("\n"); - /* Force enable SPI ROM in SB600 PM register. + /* Force enable SPI ROM in the AMD 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 @@ -1763,16 +1763,16 @@ const struct penable chipset_enables[] = { #if defined(__i386__) || defined(__x86_64__) {0x1002, 0x4377, B_PFL, OK, "ATI", "SB400", enable_flash_sb400}, - {0x1002, 0x438d, B_FLS, OK, "AMD", "SB600", enable_flash_sb600}, - {0x1002, 0x439d, B_FLS, OK, "AMD", "SB7x0/SB8x0/SB9x0", enable_flash_sb600}, + {0x1002, 0x438d, B_FLS, OK, "AMD", "SB600", enable_flash_amd}, + {0x1002, 0x439d, B_FLS, OK, "AMD", "SB7x0/SB8x0/SB9x0", enable_flash_amd}, {0x100b, 0x0510, B_PFL, NT, "AMD", "SC1100", enable_flash_sc1100}, {0x1022, 0x2080, B_PFL, OK, "AMD", "CS5536", enable_flash_cs5536}, {0x1022, 0x2090, B_PFL, OK, "AMD", "CS5536", enable_flash_cs5536}, {0x1022, 0x3000, B_PFL, OK, "AMD", "Elan SC520", get_flashbase_sc520}, {0x1022, 0x7440, B_PFL, OK, "AMD", "AMD-768", enable_flash_amd_768_8111}, {0x1022, 0x7468, B_PFL, OK, "AMD", "AMD-8111", enable_flash_amd_768_8111}, - {0x1022, 0x780e, B_FLS, OK, "AMD", "FCH", enable_flash_sb600}, - {0x1022, 0x790e, B_FLS, OK, "AMD", "FP4", enable_flash_sb600}, + {0x1022, 0x780e, B_FLS, OK, "AMD", "FCH", enable_flash_amd}, + {0x1022, 0x790e, B_FLS, OK, "AMD", "FP4", enable_flash_amd}, {0x1039, 0x0406, B_PFL, NT, "SiS", "501/5101/5501", enable_flash_sis501}, {0x1039, 0x0496, B_PFL, NT, "SiS", "85C496+497", enable_flash_sis85c496}, {0x1039, 0x0530, B_PFL, OK, "SiS", "530", enable_flash_sis530}, diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst index 45ef4aa..613566e 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -424,7 +424,7 @@ Please report the outcome if you had to use this option to write a chip. An optional ``spispeed`` parameter specifies the frequency of the SPI bus where applicable - (i.e.SB600 or later with an SPI flash chip directly attached to the chipset). + (i.e. Recent AMD chipsets / SoCs with a SPI flash chip directly attached to the chipset). Syntax is:: flashrom -p internal:spispeed=frequency diff --git a/include/programmer.h b/include/programmer.h index 82bbd61..1eb4b61 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -396,7 +396,7 @@ void internal_par_init(enum chipbustype buses); /* amd_spi.c */ -int sb600_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev); +int amd_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev); /* wbsio_spi.c */ int wbsio_check_for_spi(struct board_cfg *); -- To view, visit https://review.coreboot.org/c/flashrom/+/74953 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1200ef25b6765f809c754ae0adcdcfe680c202fd Gerrit-Change-Number: 74953 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom: rename sb600spi.c to amd_spi.c
by Martin L Roth (Code Review) 04 May '23

04 May '23

Martin L Roth has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74952 ) Change subject: flashrom: rename sb600spi.c to amd_spi.c ...................................................................... flashrom: rename sb600spi.c to amd_spi.c The modern AMD platforms all use the code in sb600spi.c, but no longer have any relationship to that name. This makes the name more generic and easier to figure out what's in the file from the name. Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I13859de27e602cc6496684e6cb66b2dc2e21531a --- M MAINTAINERS M Makefile R amd_spi.c M include/programmer.h M meson.build 5 files changed, 19 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/74952/1 diff --git a/MAINTAINERS b/MAINTAINERS index 0adc258..8ae44b1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -128,10 +128,10 @@ # PROGRAMMERS ################################################################################ -AMD SB600SPI +AMD SPI M: Arthur Heymans <arthur(a)aheymans.xyz> S: Maintained -F: sb600spi.c +F: amd_spi.c CH347 M: Nicholas Chin <nic.c3.14(a)gmail.com> diff --git a/Makefile b/Makefile index 0e16942..2f8a8c1 100644 --- a/Makefile +++ b/Makefile @@ -590,7 +590,7 @@ ifeq ($(CONFIG_INTERNAL) $(CONFIG_INTERNAL_X86), yes yes) FEATURE_FLAGS += -D'CONFIG_INTERNAL=1' PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o \ - internal.o internal_par.o it87spi.o sb600spi.o superio.o amd_imc.o wbsio_spi.o mcp6x_spi.o \ + internal.o internal_par.o it87spi.o amd_spi.o superio.o amd_imc.o wbsio_spi.o mcp6x_spi.o \ ichspi.o dmi.o known_boards.o ACTIVE_PROGRAMMERS += internal endif diff --git a/sb600spi.c b/amd_spi.c similarity index 100% rename from sb600spi.c rename to amd_spi.c diff --git a/include/programmer.h b/include/programmer.h index 5b304f5..82bbd61 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -395,7 +395,7 @@ /* internal_par.c */ void internal_par_init(enum chipbustype buses); -/* sb600spi.c */ +/* amd_spi.c */ int sb600_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev); /* wbsio_spi.c */ diff --git a/meson.build b/meson.build index 2784c34..10cd74b 100644 --- a/meson.build +++ b/meson.build @@ -281,9 +281,9 @@ 'internal.c', 'internal_par.c', 'it87spi.c', - 'sb600spi.c', 'superio.c', 'amd_imc.c', + 'amd_spi.c', 'wbsio_spi.c', 'mcp6x_spi.c', 'ichspi.c', -- To view, visit https://review.coreboot.org/c/flashrom/+/74952 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13859de27e602cc6496684e6cb66b2dc2e21531a Gerrit-Change-Number: 74952 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: newchange

1 0

[XL] Change in flashrom[master]: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michael Niewöhner (Code Review) 04 May '23

04 May '23

Attention is currently required from: Nico Huber, Michał Żygowski, Paul Menzel. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: ite_ecfw: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... Patch Set 30: (1 comment) File ite_ecfw.c: https://review.coreboot.org/c/flashrom/+/55715/comment/07a44f68_aab65bf2 PS29, Line 696: } > Does it show something useful yet? AFAICS the firmware only returns one (hardcoded) byte 0xef [2], even though uEcFlash reads 3 bytes. When you read more you get unreliable results for the second and third byte ofc. I'm unsure what 0xef stands for tbh. One can read the eFlash id (RDID) via the DBGR interface like iteflash does (sending SPI cmd 0x9f). [1] My IT5570 on L140MU returns FF FF FE, which matches "EFLASH TYPE = 8315" in iteflash. I have no idea what that 8315 means. (KGD is "Known-Good-Die" by ESMT (vendor ID 0x8c) and/or Winbond (0xef).) The vendor id of my eFlash (0xff) matches "ITEID" in one of ITE's many flash tool implementations. Maybe they just forgot to update that hardcoded value to 0xff (=ITE)? Anyway, even uEcFlash is not doing anything with that value. [1] https://chromium.googlesource.com/chromiumos/platform/ec/+/master/util/itef… [2] ``` code:00005EA2 dat_04_get_flash_id: ; CODE XREF: FlashECCodeNew+3D↑j code:00005EA2 mov DPTR, #SPIIDBuf code:00005EA5 mov A, #0xEF code:00005EA7 movx @DPTR, A code:00005EA8 jnb Ext_Cb1.3, FlashECCodeNew_PM1 code:00005EAB movx A, @DPTR code:00005EAC mov DPTR, #PM2DO code:00005EAF movx @DPTR, A code:00005EB0 ljmp next code:00005EB3 ; --------------------------------------------------------------------------- code:00005EB3 code:00005EB3 FlashECCodeNew_PM1: ; CODE XREF: FlashECCodeNew+98↑j code:00005EB3 mov DPTR, #SPIIDBuf code:00005EB6 movx A, @DPTR code:00005EB7 mov DPTR, #PM1DO code:00005EBA movx @DPTR, A code:00005EBB ljmp next ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 30 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Wed, 03 May 2023 18:54:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c:Enable erase path optimisation
by Anastasia Klimchuk (Code Review) 03 May '23

03 May '23

Attention is currently required from: Edward O'Callaghan, Aarya. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 ) Change subject: flashrom.c:Enable erase path optimisation ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > There is a patch (thanks to Edward) which adds more logging to tests CB:74932 […] I noticed only now, there is a fix already CB:74882 , and the test pass already on the latest patchset. This is awesome! :) -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 3 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Wed, 03 May 2023 11:33:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c:Enable erase path optimisation
by Anastasia Klimchuk (Code Review) 03 May '23

03 May '23

Attention is currently required from: Edward O'Callaghan, Aarya. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 ) Change subject: flashrom.c:Enable erase path optimisation ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: There is a patch (thanks to Edward) which adds more logging to tests CB:74932 If you rebase this patch on the top of CB:74932, it will give more logging, and help to debug and fix. -- To view, visit https://review.coreboot.org/c/flashrom/+/74872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22 Gerrit-Change-Number: 74872 Gerrit-PatchSet: 3 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Wed, 03 May 2023 11:26:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests/chip_wp.c: Allow for logging during test
by Edward O'Callaghan (Code Review) 03 May '23

03 May '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74932 ) Change subject: tests/chip_wp.c: Allow for logging during test ...................................................................... tests/chip_wp.c: Allow for logging during test Hook logging callback so unit-tests print what they are doing. This make debug far easier for a failing test. BUG=none TEST=ninja test. Change-Id: I7ab0ff0915a76eea9857fc876493615c06193a37 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip_wp.c 1 file changed, 24 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/32/74932/1 diff --git a/tests/chip_wp.c b/tests/chip_wp.c index 2686f44..1fa6bb9 100644 --- a/tests/chip_wp.c +++ b/tests/chip_wp.c @@ -24,6 +24,12 @@ #include "programmer.h" #include "tests.h" +static int unittest_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap) +{ + if (level > FLASHROM_MSG_INFO) return 0; + return vfprintf(stderr, fmt, ap); +} + /* * Tests in this file do not use any mocking, because using write-protect * emulation in dummyflasher programmer is sufficient @@ -47,6 +53,8 @@ flashrom_layout_set(flash, *layout); } + flashrom_set_log_callback((flashrom_log_callback *)&unittest_print_cb); + assert_int_equal(0, programmer_init(&programmer_dummy, programmer_param)); /* Assignment below normally happens while probing, but this test is not probing. */ flash->mst = &registered_masters[0]; -- To view, visit https://review.coreboot.org/c/flashrom/+/74932 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ab0ff0915a76eea9857fc876493615c06193a37 Gerrit-Change-Number: 74932 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit