November 2021 - flashrom-gerrit

Change in flashrom[master]: chipset_enable.c: Enable Tiger Lake U support
by Edward O'Callaghan (Code Review) April 10, 2022

April 10, 2022

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37777 ) Change subject: chipset_enable.c: Enable Tiger Lake U support ...................................................................... chipset_enable.c: Enable Tiger Lake U support Did a basic local test with --flash-name and -r bios and ran strings to see if the bios contained reasonable data. BUG=b:146089922 Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipset_enable.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/37777/1 diff --git a/chipset_enable.c b/chipset_enable.c index b55852c..d5a2f03 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1996,6 +1996,7 @@ {0x8086, 0x9d56, B_S, NT, "Intel", "Kaby Lake Y Premium", enable_flash_pch100}, {0x8086, 0x9d58, B_S, NT, "Intel", "Kaby Lake U Premium", enable_flash_pch100}, {0x8086, 0x9d84, B_S, DEP, "Intel", "Cannon Lake U Premium", enable_flash_pch300}, + {0x8086, 0xa082, B_FS, NT, "Intel", "Tiger Lake U", enable_flash_pch300}, {0x8086, 0xa141, B_S, NT, "Intel", "Sunrise Point Desktop Sample", enable_flash_pch100}, {0x8086, 0xa142, B_S, NT, "Intel", "Sunrise Point Unknown Sample", enable_flash_pch100}, {0x8086, 0xa143, B_S, NT, "Intel", "H110", enable_flash_pch100}, -- To view, visit https://review.coreboot.org/c/flashrom/+/37777 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c13e0173c9b5e17d2ae197f4a4ab9aa2825c1b3 Gerrit-Change-Number: 37777 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

5 8

Change in flashrom[master]: usbdev: fix accepting shorter serial numbers than the real one.
by Miklós Márton (Code Review) March 16, 2022

March 16, 2022

Miklós Márton has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/34641 ) Change subject: usbdev: fix accepting shorter serial numbers than the real one. ...................................................................... usbdev: fix accepting shorter serial numbers than the real one. Change-Id: Ife8c4e0a957c4345e27ec9ae9e1480ca80fe505c --- M usbdev.c 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/34641/1 diff --git a/usbdev.c b/usbdev.c index 764ac04..7e41235 100644 --- a/usbdev.c +++ b/usbdev.c @@ -104,7 +104,8 @@ msg_pdbg("Serial number is %s\n", myserial); /* Filter out any serial number that does not commence with serialno */ - return 0 != strncmp(serialno, (char *)myserial, strlen(serialno)); + return strlen((char *)myserial) == strlen(serialno) + && 0 != strncmp(serialno, (char *)myserial, strlen(serialno)); } struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( -- To view, visit https://review.coreboot.org/c/flashrom/+/34641 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ife8c4e0a957c4345e27ec9ae9e1480ca80fe505c Gerrit-Change-Number: 34641 Gerrit-PatchSet: 1 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-MessageType: newchange

2 5

Change in flashrom[master]: Fix segfault when iomem=strict
by Evgeny Zinoviev (Code Review) March 9, 2022

March 9, 2022

Evgeny Zinoviev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/48361 ) Change subject: Fix segfault when iomem=strict ...................................................................... Fix segfault when iomem=strict As I can see, physmap_common() only returns ERROR_PTR or a real pointer but never NULL. Tested on Linux 5.9.12. Change-Id: I5b11957fc10fd5c7a4fbb7f54fb94d791590f373 Signed-off-by: Evgeny Zinoviev <me(a)ch1p.io> --- M dmi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/48361/1 diff --git a/dmi.c b/dmi.c index c44221c..7488d61 100644 --- a/dmi.c +++ b/dmi.c @@ -164,7 +164,7 @@ unsigned int i = 0, j = 0; uint8_t *dmi_table_mem = physmap_ro("DMI Table", base, len); - if (dmi_table_mem == NULL) { + if (dmi_table_mem == ERROR_PTR) { msg_perr("Unable to access DMI Table\n"); return; } -- To view, visit https://review.coreboot.org/c/flashrom/+/48361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5b11957fc10fd5c7a4fbb7f54fb94d791590f373 Gerrit-Change-Number: 48361 Gerrit-PatchSet: 1 Gerrit-Owner: Evgeny Zinoviev <me(a)ch1p.io> Gerrit-MessageType: newchange

3 3

Change in flashrom[master]: flashchips: Add W25Q64.V.M
by Keith Hui (Code Review) Dec. 26, 2021

Dec. 26, 2021

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/38578 ) Change subject: flashchips: Add W25Q64.V.M ...................................................................... flashchips: Add W25Q64.V.M Add Winbond W25Q64JV?M SPI flash chip. Some tests were done on a W25Q64JVSSIM mounted on a Sparkfun SOIC-DIP adapter, connected to an Arduino Mega 2560 r3 loaded with frser-duino. Probe and read works, but instability were observed in the connection between Arduino and the chip, so this is being submitted as untested. Datasheet: https://www.winbond.com/resource-files/w25q64jv%20revj%2003272018%20plus.pdf Change-Id: Ib7749835178ee6a1b0fe579e7a0c29e69a67933b Signed-off-by: Keith Hui <buurin(a)gmail.com> --- M flashchips.c M flashchips.h 2 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/38578/1 diff --git a/flashchips.c b/flashchips.c index 65dda0e..bfa2927 100644 --- a/flashchips.c +++ b/flashchips.c @@ -16936,6 +16936,46 @@ { .vendor = "Winbond", + .name = "W25Q64.V.M", + .bustype = BUS_SPI, + .manufacture_id = WINBOND_NEX_ID, + .model_id = WINBOND_NEX_W25Q64_V_M, + .total_size = 8192, + .page_size = 256, + /* supports SFDP */ + /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 2048} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 256} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 128} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {8 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {8 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_plain, + .unlock = spi_disable_blockprotect, + .write = spi_chip_write_256, + .read = spi_chip_read, + .voltage = {2700, 3600}, + }, + + { + .vendor = "Winbond", .name = "W25Q64.W", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, diff --git a/flashchips.h b/flashchips.h index 14ab6de..bdfadad 100644 --- a/flashchips.h +++ b/flashchips.h @@ -938,6 +938,7 @@ #define WINBOND_NEX_W25Q32_W 0x6016 /* W25Q32DW; W25Q32FV in QPI mode */ #define WINBOND_NEX_W25Q64_W 0x6017 /* W25Q64DW; W25Q64FV in QPI mode */ #define WINBOND_NEX_W25Q128_W 0x6018 /* W25Q128FW; W25Q128FV in QPI mode */ +#define WINBOND_NEX_W25Q64_V_M 0x7017 /* W25Q64JVSIM */ #define WINBOND_NEX_W25Q128_V_M 0x7018 /* W25Q128JVSM */ #define WINBOND_NEX_W25Q256JV_M 0x7019 /* W25Q256JV_M (QE=0) */ #define WINBOND_NEX_W25Q128_DTR 0x8018 /* W25Q128JW_DTR */ -- To view, visit https://review.coreboot.org/c/flashrom/+/38578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib7749835178ee6a1b0fe579e7a0c29e69a67933b Gerrit-Change-Number: 38578 Gerrit-PatchSet: 1 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-MessageType: newchange

2 9

Change in flashrom[master]: writeprotect.h,c: Add Winbond support (WIP)
by Edward O'Callaghan (Code Review) Dec. 5, 2021

Dec. 5, 2021

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45572 ) Change subject: writeprotect.h,c: Add Winbond support (WIP) ...................................................................... writeprotect.h,c: Add Winbond support (WIP) Adds Winbond support. WIP Change-Id: I54eedaa8e24c69d6d088e9ebcfb9a284e0fbe893 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M writeprotect.c M writeprotect.h 2 files changed, 1,068 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/45572/1 diff --git a/writeprotect.c b/writeprotect.c index 0cef19b..1cd3950 100644 --- a/writeprotect.c +++ b/writeprotect.c @@ -87,6 +87,43 @@ struct modifier_bits *m); }; +struct w25q_status { + /* this maps to register layout -- do not change ordering */ + unsigned char busy : 1; + unsigned char wel : 1; + unsigned char bp0 : 1; + unsigned char bp1 : 1; + unsigned char bp2 : 1; + unsigned char tb : 1; + unsigned char sec : 1; + unsigned char srp0 : 1; +} __attribute__ ((packed)); + +/* Status register for large flash layouts with 4 BP bits */ +struct w25q_status_large { + unsigned char busy : 1; + unsigned char wel : 1; + unsigned char bp0 : 1; + unsigned char bp1 : 1; + unsigned char bp2 : 1; + unsigned char bp3 : 1; + unsigned char tb : 1; + unsigned char srp0 : 1; +} __attribute__ ((packed)); + +struct w25q_status_2 { + unsigned char srp1 : 1; + unsigned char qe : 1; + unsigned char rsvd : 6; +} __attribute__ ((packed)); + +int w25_range_to_status(const struct flashctx *flash, + unsigned int start, unsigned int len, + struct w25q_status *status); +int w25_status_to_range(const struct flashctx *flash, + const struct w25q_status *status, + unsigned int *start, unsigned int *len); + /* * Mask to extract write-protect enable and range bits * Status register 1: @@ -100,6 +137,295 @@ #define MASK_WP_AREA_LARGE (0x9C) #define MASK_WP2_AREA (0x01) +static struct wp_range_descriptor w25q16_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x1f0000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x1e0000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x3, {0x1c0000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x4, {0x180000, 512 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x5, {0x100000, 1024 * 1024} }, + + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x000000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x4, {0x000000, 512 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x5, {0x000000, 1024 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x6, {0x000000, 2048 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x7, {0x000000, 2048 * 1024} }, + + { .m = { .sec = 1, .tb = 0 }, 0x1, {0x1ff000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x2, {0x1fe000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x3, {0x1fc000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x4, {0x1f8000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x5, {0x1f8000, 32 * 1024} }, + + { .m = { .sec = 1, .tb = 1 }, 0x1, {0x000000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x2, {0x000000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x3, {0x000000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x4, {0x000000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x5, {0x000000, 32 * 1024} }, +}; + +static struct wp_range_descriptor w25q32_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x3f0000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x3e0000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x3, {0x3c0000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x4, {0x380000, 512 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x5, {0x300000, 1024 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x6, {0x200000, 2048 * 1024} }, + + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x000000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x4, {0x000000, 512 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x5, {0x000000, 1024 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x6, {0x000000, 2048 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x7, {0x000000, 4096 * 1024} }, + + { .m = { .sec = 1, .tb = 0 }, 0x1, {0x3ff000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x2, {0x3fe000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x3, {0x3fc000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x4, {0x3f8000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x5, {0x3f8000, 32 * 1024} }, + + { .m = { .sec = 1, .tb = 1 }, 0x1, {0x000000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x2, {0x000000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x3, {0x000000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x4, {0x000000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x5, {0x000000, 32 * 1024} }, +}; + +static struct wp_range_descriptor w25q80_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x0f0000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x0e0000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x3, {0x0c0000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x4, {0x080000, 512 * 1024} }, + + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x000000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x4, {0x000000, 512 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x6, {0x000000, 1024 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x7, {0x000000, 1024 * 1024} }, + + { .m = { .sec = 1, .tb = 0 }, 0x1, {0x1ff000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x2, {0x1fe000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x3, {0x1fc000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x4, {0x1f8000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x5, {0x1f8000, 32 * 1024} }, + + { .m = { .sec = 1, .tb = 1 }, 0x1, {0x000000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x2, {0x000000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x3, {0x000000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x4, {0x000000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x5, {0x000000, 32 * 1024} }, +}; + +static struct wp_range_descriptor w25q64_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x7e0000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x7c0000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x3, {0x780000, 512 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x4, {0x700000, 1024 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x5, {0x600000, 2048 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x6, {0x400000, 4096 * 1024} }, + + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x000000, 512 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x4, {0x000000, 1024 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x5, {0x000000, 2048 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x6, {0x000000, 4096 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x7, {0x000000, 8192 * 1024} }, + + { .m = { .sec = 1, .tb = 0 }, 0x1, {0x7ff000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x2, {0x7fe000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x3, {0x7fc000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x4, {0x7f8000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 0 }, 0x5, {0x7f8000, 32 * 1024} }, + + { .m = { .sec = 1, .tb = 1 }, 0x1, {0x000000, 4 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x2, {0x000000, 8 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x3, {0x000000, 16 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x4, {0x000000, 32 * 1024} }, + { .m = { .sec = 1, .tb = 1 }, 0x5, {0x000000, 32 * 1024} }, +}; + +static struct wp_range_descriptor w25rq128_cmp0_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* NONE */ + + { .m = { .sec = 0, .tb = 0 }, 0x1, {0xfc0000, 256 * 1024} }, /* Upper 1/64 */ + { .m = { .sec = 0, .tb = 0 }, 0x2, {0xf80000, 512 * 1024} }, /* Upper 1/32 */ + { .m = { .sec = 0, .tb = 0 }, 0x3, {0xf00000, 1024 * 1024} }, /* Upper 1/16 */ + { .m = { .sec = 0, .tb = 0 }, 0x4, {0xe00000, 2048 * 1024} }, /* Upper 1/8 */ + { .m = { .sec = 0, .tb = 0 }, 0x5, {0xc00000, 4096 * 1024} }, /* Upper 1/4 */ + { .m = { .sec = 0, .tb = 0 }, 0x6, {0x800000, 8192 * 1024} }, /* Upper 1/2 */ + + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 256 * 1024} }, /* Lower 1/64 */ + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 512 * 1024} }, /* Lower 1/32 */ + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x000000, 1024 * 1024} }, /* Lower 1/16 */ + { .m = { .sec = 0, .tb = 1 }, 0x4, {0x000000, 2048 * 1024} }, /* Lower 1/8 */ + { .m = { .sec = 0, .tb = 1 }, 0x5, {0x000000, 4096 * 1024} }, /* Lower 1/4 */ + { .m = { .sec = 0, .tb = 1 }, 0x6, {0x000000, 8192 * 1024} }, /* Lower 1/2 */ + + { .m = { .sec = X, .tb = X }, 0x7, {0x000000, 16384 * 1024} }, /* ALL */ + + { .m = { .sec = 1, .tb = 0 }, 0x1, {0xfff000, 4 * 1024} }, /* Upper 1/4096 */ + { .m = { .sec = 1, .tb = 0 }, 0x2, {0xffe000, 8 * 1024} }, /* Upper 1/2048 */ + { .m = { .sec = 1, .tb = 0 }, 0x3, {0xffc000, 16 * 1024} }, /* Upper 1/1024 */ + { .m = { .sec = 1, .tb = 0 }, 0x4, {0xff8000, 32 * 1024} }, /* Upper 1/512 */ + { .m = { .sec = 1, .tb = 0 }, 0x5, {0xff8000, 32 * 1024} }, /* Upper 1/512 */ + + { .m = { .sec = 1, .tb = 1 }, 0x1, {0x000000, 4 * 1024} }, /* Lower 1/4096 */ + { .m = { .sec = 1, .tb = 1 }, 0x2, {0x000000, 8 * 1024} }, /* Lower 1/2048 */ + { .m = { .sec = 1, .tb = 1 }, 0x3, {0x000000, 16 * 1024} }, /* Lower 1/1024 */ + { .m = { .sec = 1, .tb = 1 }, 0x4, {0x000000, 32 * 1024} }, /* Lower 1/512 */ + { .m = { .sec = 1, .tb = 1 }, 0x5, {0x000000, 32 * 1024} }, /* Lower 1/512 */ +}; + +static struct wp_range_descriptor w25rq128_cmp1_ranges[] = { + { .m = { .sec = X, .tb = X }, 0x0, {0x000000, 16 * 1024 * 1024} }, /* ALL */ + + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x000000, 16128 * 1024} }, /* Lower 63/64 */ + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x000000, 15872 * 1024} }, /* Lower 31/32 */ + { .m = { .sec = 0, .tb = 0 }, 0x3, {0x000000, 15 * 1024 * 1024} }, /* Lower 15/16 */ + { .m = { .sec = 0, .tb = 0 }, 0x4, {0x000000, 14 * 1024 * 1024} }, /* Lower 7/8 */ + { .m = { .sec = 0, .tb = 0 }, 0x5, {0x000000, 12 * 1024 * 1024} }, /* Lower 3/4 */ + { .m = { .sec = 0, .tb = 0 }, 0x6, {0x000000, 8 * 1024 * 1024} }, /* Lower 1/2 */ + + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x040000, 16128 * 1024} }, /* Upper 63/64 */ + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x080000, 15872 * 1024} }, /* Upper 31/32 */ + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x100000, 15 * 1024 * 1024} }, /* Upper 15/16 */ + { .m = { .sec = 0, .tb = 1 }, 0x4, {0x200000, 14 * 1024 * 1024} }, /* Upper 7/8 */ + { .m = { .sec = 0, .tb = 1 }, 0x5, {0x400000, 12 * 1024 * 1024} }, /* Upper 3/4 */ + { .m = { .sec = 0, .tb = 1 }, 0x6, {0x800000, 8 * 1024 * 1024} }, /* Upper 1/2 */ + + { .m = { .sec = X, .tb = X }, 0x7, {0x000000, 0} }, /* NONE */ + + { .m = { .sec = 1, .tb = 0 }, 0x1, {0x000000, 16380 * 1024} }, /* Lower 4095/4096 */ + { .m = { .sec = 1, .tb = 0 }, 0x2, {0x000000, 16376 * 1024} }, /* Lower 2048/2048 */ + { .m = { .sec = 1, .tb = 0 }, 0x3, {0x000000, 16368 * 1024} }, /* Lower 1023/1024 */ + { .m = { .sec = 1, .tb = 0 }, 0x4, {0x000000, 16352 * 1024} }, /* Lower 511/512 */ + { .m = { .sec = 1, .tb = 0 }, 0x5, {0x000000, 16352 * 1024} }, /* Lower 511/512 */ + + { .m = { .sec = 1, .tb = 1 }, 0x1, {0x001000, 16380 * 1024} }, /* Upper 4095/4096 */ + { .m = { .sec = 1, .tb = 1 }, 0x2, {0x002000, 16376 * 1024} }, /* Upper 2047/2048 */ + { .m = { .sec = 1, .tb = 1 }, 0x3, {0x004000, 16368 * 1024} }, /* Upper 1023/1024 */ + { .m = { .sec = 1, .tb = 1 }, 0x4, {0x008000, 16352 * 1024} }, /* Upper 511/512 */ + { .m = { .sec = 1, .tb = 1 }, 0x5, {0x008000, 16352 * 1024} }, /* Upper 511/512 */ +}; + +static struct wp_range_descriptor w25rq256_cmp0_ranges[] = { + { .m = { .sec = X, .tb = X }, 0x0, {0x0000000, 0x0000000} }, /* NONE */ + + { .m = { .sec = X, .tb = 0 }, 0x1, {0x1ff0000, 64 * 1 * 1024} }, /* Upper 1/512 */ + { .m = { .sec = X, .tb = 0 }, 0x2, {0x1fe0000, 64 * 2 * 1024} }, /* Upper 1/256 */ + { .m = { .sec = X, .tb = 0 }, 0x3, {0x1fc0000, 64 * 4 * 1024} }, /* Upper 1/128 */ + { .m = { .sec = X, .tb = 0 }, 0x4, {0x1f80000, 64 * 8 * 1024} }, /* Upper 1/64 */ + { .m = { .sec = X, .tb = 0 }, 0x5, {0x1f00000, 64 * 16 * 1024} }, /* Upper 1/32 */ + { .m = { .sec = X, .tb = 0 }, 0x6, {0x1e00000, 64 * 32 * 1024} }, /* Upper 1/16 */ + { .m = { .sec = X, .tb = 0 }, 0x7, {0x1c00000, 64 * 64 * 1024} }, /* Upper 1/8 */ + { .m = { .sec = X, .tb = 0 }, 0x8, {0x1800000, 64 * 128 * 1024} }, /* Upper 1/4 */ + { .m = { .sec = X, .tb = 0 }, 0x9, {0x1000000, 64 * 256 * 1024} }, /* Upper 1/2 */ + + { .m = { .sec = X, .tb = 1 }, 0x1, {0x0000000, 64 * 1 * 1024} }, /* Lower 1/512 */ + { .m = { .sec = X, .tb = 1 }, 0x2, {0x0000000, 64 * 2 * 1024} }, /* Lower 1/256 */ + { .m = { .sec = X, .tb = 1 }, 0x3, {0x0000000, 64 * 4 * 1024} }, /* Lower 1/128 */ + { .m = { .sec = X, .tb = 1 }, 0x4, {0x0000000, 64 * 8 * 1024} }, /* Lower 1/64 */ + { .m = { .sec = X, .tb = 1 }, 0x5, {0x0000000, 64 * 16 * 1024} }, /* Lower 1/32 */ + { .m = { .sec = X, .tb = 1 }, 0x6, {0x0000000, 64 * 32 * 1024} }, /* Lower 1/16 */ + { .m = { .sec = X, .tb = 1 }, 0x7, {0x0000000, 64 * 64 * 1024} }, /* Lower 1/8 */ + { .m = { .sec = X, .tb = 1 }, 0x8, {0x0000000, 64 * 128 * 1024} }, /* Lower 1/4 */ + { .m = { .sec = X, .tb = 1 }, 0x9, {0x0000000, 64 * 256 * 1024} }, /* Lower 1/2 */ + + { .m = { .sec = X, .tb = X }, 0xa, {0x0000000, 64 * 512 * 1024} }, /* ALL */ + { .m = { .sec = X, .tb = X }, 0xb, {0x0000000, 64 * 512 * 1024} }, /* ALL */ + { .m = { .sec = X, .tb = X }, 0xc, {0x0000000, 64 * 512 * 1024} }, /* ALL */ + { .m = { .sec = X, .tb = X }, 0xd, {0x0000000, 64 * 512 * 1024} }, /* ALL */ + { .m = { .sec = X, .tb = X }, 0xe, {0x0000000, 64 * 512 * 1024} }, /* ALL */ + { .m = { .sec = X, .tb = X }, 0xf, {0x0000000, 64 * 512 * 1024} }, /* ALL */ +}; + +static struct wp_range_descriptor w25rq256_cmp1_ranges[] = { + { .m = { .sec = X, .tb = X }, 0x0, {0x0000000, 64 * 512 * 1024} }, /* ALL */ + + { .m = { .sec = X, .tb = 0 }, 0x1, {0x0000000, 64 * 511 * 1024} }, /* Lower 511/512 */ + { .m = { .sec = X, .tb = 0 }, 0x2, {0x0000000, 64 * 510 * 1024} }, /* Lower 255/256 */ + { .m = { .sec = X, .tb = 0 }, 0x3, {0x0000000, 64 * 508 * 1024} }, /* Lower 127/128 */ + { .m = { .sec = X, .tb = 0 }, 0x4, {0x0000000, 64 * 504 * 1024} }, /* Lower 63/64 */ + { .m = { .sec = X, .tb = 0 }, 0x5, {0x0000000, 64 * 496 * 1024} }, /* Lower 31/32 */ + { .m = { .sec = X, .tb = 0 }, 0x6, {0x0000000, 64 * 480 * 1024} }, /* Lower 15/16 */ + { .m = { .sec = X, .tb = 0 }, 0x7, {0x0000000, 64 * 448 * 1024} }, /* Lower 7/8 */ + { .m = { .sec = X, .tb = 0 }, 0x8, {0x0000000, 64 * 384 * 1024} }, /* Lower 3/4 */ + { .m = { .sec = X, .tb = 0 }, 0x9, {0x0000000, 64 * 256 * 1024} }, /* Lower 1/2 */ + + { .m = { .sec = X, .tb = 1 }, 0x1, {0x0010000, 64 * 511 * 1024} }, /* Upper 511/512 */ + { .m = { .sec = X, .tb = 1 }, 0x2, {0x0020000, 64 * 510 * 1024} }, /* Upper 255/256 */ + { .m = { .sec = X, .tb = 1 }, 0x3, {0x0040000, 64 * 508 * 1024} }, /* Upper 127/128 */ + { .m = { .sec = X, .tb = 1 }, 0x4, {0x0080000, 64 * 504 * 1024} }, /* Upper 63/64 */ + { .m = { .sec = X, .tb = 1 }, 0x5, {0x0100000, 64 * 496 * 1024} }, /* Upper 31/32 */ + { .m = { .sec = X, .tb = 1 }, 0x6, {0x0200000, 64 * 480 * 1024} }, /* Upper 15/16 */ + { .m = { .sec = X, .tb = 1 }, 0x7, {0x0400000, 64 * 448 * 1024} }, /* Upper 7/8 */ + { .m = { .sec = X, .tb = 1 }, 0x8, {0x0800000, 64 * 384 * 1024} }, /* Upper 3/4 */ + { .m = { .sec = X, .tb = 1 }, 0x9, {0x1000000, 64 * 256 * 1024} }, /* Upper 1/2 */ + + { .m = { .sec = X, .tb = X }, 0xa, {0x0000000, 0x0000000} }, /* NONE */ + { .m = { .sec = X, .tb = X }, 0xb, {0x0000000, 0x0000000} }, /* NONE */ + { .m = { .sec = X, .tb = X }, 0xc, {0x0000000, 0x0000000} }, /* NONE */ + { .m = { .sec = X, .tb = X }, 0xd, {0x0000000, 0x0000000} }, /* NONE */ + { .m = { .sec = X, .tb = X }, 0xe, {0x0000000, 0x0000000} }, /* NONE */ + { .m = { .sec = X, .tb = X }, 0xf, {0x0000000, 0x0000000} }, /* NONE */ +}; + +struct wp_range_descriptor w25x10_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x010000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 64 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x2, {0x000000, 128 * 1024} }, + { .m = { .sec = X, .tb = X }, 0x3, {0x000000, 128 * 1024} }, +}; + +struct wp_range_descriptor w25x20_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x030000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x020000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 128 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x3, {0x000000, 256 * 1024} }, +}; + +struct wp_range_descriptor w25x40_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x070000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x060000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x3, {0x040000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x000000, 256 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x4, {0x000000, 512 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x5, {0x000000, 512 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x6, {0x000000, 512 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x7, {0x000000, 512 * 1024} }, +}; + +struct wp_range_descriptor w25x80_ranges[] = { + { .m = { .sec = X, .tb = X }, 0, {0, 0} }, /* none */ + { .m = { .sec = 0, .tb = 0 }, 0x1, {0x0F0000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x2, {0x0E0000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x3, {0x0C0000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 0 }, 0x4, {0x080000, 512 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x1, {0x000000, 64 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x2, {0x000000, 128 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x3, {0x000000, 256 * 1024} }, + { .m = { .sec = 0, .tb = 1 }, 0x4, {0x000000, 512 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x5, {0x000000, 1024 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x6, {0x000000, 1024 * 1024} }, + { .m = { .sec = 0, .tb = X }, 0x7, {0x000000, 1024 * 1024} }, +}; + static uint8_t do_read_status(const struct flashctx *flash) { if (flash->chip->read_status) @@ -116,6 +442,591 @@ return spi_write_status_register(flash, status); } +/* FIXME: Move to spi25.c if it's a JEDEC standard opcode */ +static uint8_t w25q_read_status_register_2(const struct flashctx *flash) +{ + static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { 0x35 }; + unsigned char readarr[2]; + int ret; + + /* Read Status Register */ + ret = spi_send_command(flash, sizeof(cmd), sizeof(readarr), cmd, readarr); + if (ret) { + /* + * FIXME: make this a benign failure for now in case we are + * unable to execute the opcode + */ + msg_cdbg("RDSR2 failed!\n"); + readarr[0] = 0x00; + } + + return readarr[0]; +} + +/* Given a flash chip, this function returns its range table. */ +static int w25_range_table(const struct flashctx *flash, + struct wp_range_descriptor **descrs, + int *num_entries) +{ + *descrs = 0; + *num_entries = 0; + + switch (flash->chip->manufacture_id) { + case WINBOND_NEX_ID: + switch(flash->chip->model_id) { + case WINBOND_NEX_W25X10: + *descrs = w25x10_ranges; + *num_entries = ARRAY_SIZE(w25x10_ranges); + break; + case WINBOND_NEX_W25X20: + *descrs = w25x20_ranges; + *num_entries = ARRAY_SIZE(w25x20_ranges); + break; + case WINBOND_NEX_W25X40: + *descrs = w25x40_ranges; + *num_entries = ARRAY_SIZE(w25x40_ranges); + break; + case WINBOND_NEX_W25X80: + *descrs = w25x80_ranges; + *num_entries = ARRAY_SIZE(w25x80_ranges); + break; + case WINBOND_NEX_W25Q80_V: + *descrs = w25q80_ranges; + *num_entries = ARRAY_SIZE(w25q80_ranges); + break; + case WINBOND_NEX_W25Q16_V: + *descrs = w25q16_ranges; + *num_entries = ARRAY_SIZE(w25q16_ranges); + break; + case WINBOND_NEX_W25Q32_V: + case WINBOND_NEX_W25Q32_W: + //case WINBOND_NEX_W25Q32JW: + *descrs = w25q32_ranges; + *num_entries = ARRAY_SIZE(w25q32_ranges); + break; + case WINBOND_NEX_W25Q64_V: + case WINBOND_NEX_W25Q64_W: + *descrs = w25q64_ranges; + *num_entries = ARRAY_SIZE(w25q64_ranges); + break; + case WINBOND_NEX_W25Q128_DTR: + case WINBOND_NEX_W25Q128_V_M: + case WINBOND_NEX_W25Q128_V: + case WINBOND_NEX_W25Q128_W: + if (w25q_read_status_register_2(flash) & (1 << 6)) { + /* CMP == 1 */ + *descrs = w25rq128_cmp1_ranges; + *num_entries = ARRAY_SIZE(w25rq128_cmp1_ranges); + } else { + /* CMP == 0 */ + *descrs = w25rq128_cmp0_ranges; + *num_entries = ARRAY_SIZE(w25rq128_cmp0_ranges); + } + break; + case WINBOND_NEX_W25Q256JV_M: + if (w25q_read_status_register_2(flash) & (1 << 6)) { + /* CMP == 1 */ + *descrs = w25rq256_cmp1_ranges; + *num_entries = ARRAY_SIZE(w25rq256_cmp1_ranges); + } else { + /* CMP == 0 */ + *descrs = w25rq256_cmp0_ranges; + *num_entries = ARRAY_SIZE(w25rq256_cmp0_ranges); + } + break; + default: + msg_cerr("%s() %d: WINBOND flash chip mismatch (0x%04x)" + ", aborting\n", __func__, __LINE__, + flash->chip->model_id); + return -1; + } + break; + default: + msg_cerr("%s: flash vendor (0x%x) not found, aborting\n", + __func__, flash->chip->manufacture_id); + return -1; + } + + return 0; +} + +int w25_range_to_status(const struct flashctx *flash, + unsigned int start, unsigned int len, + struct w25q_status *status) +{ + struct wp_range_descriptor *descrs; + int i, range_found = 0; + int num_entries; + + if (w25_range_table(flash, &descrs, &num_entries)) + return -1; + + for (i = 0; i < num_entries; i++) { + struct wp_range *r = &descrs[i].range; + + msg_cspew("comparing range 0x%x 0x%x / 0x%x 0x%x\n", + start, len, r->start, r->len); + if ((start == r->start) && (len == r->len)) { + status->bp0 = descrs[i].bp & 1; + status->bp1 = descrs[i].bp >> 1; + status->bp2 = descrs[i].bp >> 2; + status->tb = descrs[i].m.tb; + status->sec = descrs[i].m.sec; + + range_found = 1; + break; + } + } + + if (!range_found) { + msg_cerr("%s: matching range not found\n", __func__); + return -1; + } + + return 0; +} + +int w25_status_to_range(const struct flashctx *flash, + const struct w25q_status *status, + unsigned int *start, unsigned int *len) +{ + struct wp_range_descriptor *descrs; + int i, status_found = 0; + int num_entries; + + if (w25_range_table(flash, &descrs, &num_entries)) + return -1; + + for (i = 0; i < num_entries; i++) { + int bp; + int table_bp, table_tb, table_sec; + + bp = status->bp0 | (status->bp1 << 1) | (status->bp2 << 2); + msg_cspew("comparing 0x%x 0x%x / 0x%x 0x%x / 0x%x 0x%x\n", + bp, descrs[i].bp, + status->tb, descrs[i].m.tb, + status->sec, descrs[i].m.sec); + table_bp = descrs[i].bp; + table_tb = descrs[i].m.tb; + table_sec = descrs[i].m.sec; + if ((bp == table_bp || table_bp == X) && + (status->tb == table_tb || table_tb == X) && + (status->sec == table_sec || table_sec == X)) { + *start = descrs[i].range.start; + *len = descrs[i].range.len; + + status_found = 1; + break; + } + } + + if (!status_found) { + msg_cerr("matching status not found\n"); + return -1; + } + + return 0; +} + +/* Given a [start, len], this function calls w25_range_to_status() to convert + * it to flash-chip-specific range bits, then sets into status register. + */ +static int w25_set_range(const struct flashctx *flash, + unsigned int start, unsigned int len) +{ + struct w25q_status status; + int tmp = 0; + int expected = 0; + + memset(&status, 0, sizeof(status)); + tmp = do_read_status(flash); + memcpy(&status, &tmp, 1); + msg_cdbg("%s: old status: 0x%02x\n", __func__, tmp); + + if (w25_range_to_status(flash, start, len, &status)) + return -1; + + msg_cdbg("status.busy: %x\n", status.busy); + msg_cdbg("status.wel: %x\n", status.wel); + msg_cdbg("status.bp0: %x\n", status.bp0); + msg_cdbg("status.bp1: %x\n", status.bp1); + msg_cdbg("status.bp2: %x\n", status.bp2); + msg_cdbg("status.tb: %x\n", status.tb); + msg_cdbg("status.sec: %x\n", status.sec); + msg_cdbg("status.srp0: %x\n", status.srp0); + + memcpy(&expected, &status, sizeof(status)); + do_write_status(flash, expected); + + tmp = do_read_status(flash); + msg_cdbg("%s: new status: 0x%02x\n", __func__, tmp); + if ((tmp & MASK_WP_AREA) != (expected & MASK_WP_AREA)) { + msg_cerr("expected=0x%02x, but actual=0x%02x.\n", + expected, tmp); + return 1; + } + + return 0; +} + +/* Print out the current status register value with human-readable text. */ +static int w25_wp_status(const struct flashctx *flash) +{ + struct w25q_status status; + int tmp; + unsigned int start, len; + int ret = 0; + + memset(&status, 0, sizeof(status)); + tmp = do_read_status(flash); + memcpy(&status, &tmp, 1); + msg_cinfo("WP: status: 0x%02x\n", tmp); + msg_cinfo("WP: status.srp0: %x\n", status.srp0); + msg_cinfo("WP: write protect is %s.\n", + status.srp0 ? "enabled" : "disabled"); + + msg_cinfo("WP: write protect range: "); + if (w25_status_to_range(flash, &status, &start, &len)) { + msg_cinfo("(cannot resolve the range)\n"); + ret = -1; + } else { + msg_cinfo("start=0x%08x, len=0x%08x\n", start, len); + } + + return ret; +} + +static int w25q_large_range_to_status(const struct flashctx *flash, + unsigned int start, unsigned int len, + struct w25q_status_large *status) +{ + struct wp_range_descriptor *descrs; + int i, range_found = 0; + int num_entries; + + if (w25_range_table(flash, &descrs, &num_entries)) + return -1; + + for (i = 0; i < num_entries; i++) { + struct wp_range *r = &descrs[i].range; + + msg_cspew("comparing range 0x%x 0x%x / 0x%x 0x%x\n", + start, len, r->start, r->len); + if ((start == r->start) && (len == r->len)) { + status->bp0 = descrs[i].bp & 1; + status->bp1 = descrs[i].bp >> 1; + status->bp2 = descrs[i].bp >> 2; + status->bp3 = descrs[i].bp >> 3; + /* + * For MX25U12835E chip, Top/Bottom (T/B) bit is not + * part of status register and in that bit position is + * Quad Enable (QE) + */ + if (flash->chip->manufacture_id != MACRONIX_ID || + flash->chip->model_id != MACRONIX_MX25U12835E) + status->tb = descrs[i].m.tb; + + range_found = 1; + break; + } + } + + if (!range_found) { + msg_cerr("%s: matching range not found\n", __func__); + return -1; + } + + return 0; +} + +static int w25_large_status_to_range(const struct flashctx *flash, + const struct w25q_status_large *status, + unsigned int *start, unsigned int *len) +{ + struct wp_range_descriptor *descrs; + int i, status_found = 0; + int num_entries; + + if (w25_range_table(flash, &descrs, &num_entries)) + return -1; + + for (i = 0; i < num_entries; i++) { + int bp; + int table_bp, table_tb; + + bp = status->bp0 | (status->bp1 << 1) | (status->bp2 << 2) | + (status->bp3 << 3); + msg_cspew("comparing 0x%x 0x%x / 0x%x 0x%x\n", + bp, descrs[i].bp, + status->tb, descrs[i].m.tb); + table_bp = descrs[i].bp; + table_tb = descrs[i].m.tb; + if ((bp == table_bp || table_bp == X) && + (status->tb == table_tb || table_tb == X)) { + *start = descrs[i].range.start; + *len = descrs[i].range.len; + + status_found = 1; + break; + } + } + + if (!status_found) { + msg_cerr("matching status not found\n"); + return -1; + } + + return 0; +} + +/* Given a [start, len], this function calls w25_range_to_status() to convert + * it to flash-chip-specific range bits, then sets into status register. + * Returns 0 if successful, -1 on error, and 1 if reading back was different. + */ +static int w25q_large_set_range(const struct flashctx *flash, + unsigned int start, unsigned int len) +{ + struct w25q_status_large status; + int tmp; + int expected = 0; + + memset(&status, 0, sizeof(status)); + tmp = do_read_status(flash); + memcpy(&status, &tmp, 1); + msg_cdbg("%s: old status: 0x%02x\n", __func__, tmp); + + if (w25q_large_range_to_status(flash, start, len, &status)) + return -1; + + msg_cdbg("status.busy: %x\n", status.busy); + msg_cdbg("status.wel: %x\n", status.wel); + msg_cdbg("status.bp0: %x\n", status.bp0); + msg_cdbg("status.bp1: %x\n", status.bp1); + msg_cdbg("status.bp2: %x\n", status.bp2); + msg_cdbg("status.bp3: %x\n", status.bp3); + msg_cdbg("status.tb: %x\n", status.tb); + msg_cdbg("status.srp0: %x\n", status.srp0); + + memcpy(&expected, &status, sizeof(status)); + do_write_status(flash, expected); + + tmp = do_read_status(flash); + msg_cdbg("%s: new status: 0x%02x\n", __func__, tmp); + if ((tmp & MASK_WP_AREA_LARGE) != (expected & MASK_WP_AREA_LARGE)) { + msg_cerr("expected=0x%02x, but actual=0x%02x.\n", + expected, tmp); + return 1; + } + + return 0; +} + +static int w25q_large_wp_status(const struct flashctx *flash) +{ + struct w25q_status_large sr1; + struct w25q_status_2 sr2; + uint8_t tmp[2]; + unsigned int start, len; + int ret = 0; + + memset(&sr1, 0, sizeof(sr1)); + tmp[0] = do_read_status(flash); + memcpy(&sr1, &tmp[0], 1); + + memset(&sr2, 0, sizeof(sr2)); + tmp[1] = w25q_read_status_register_2(flash); + memcpy(&sr2, &tmp[1], 1); + + msg_cinfo("WP: status: 0x%02x%02x\n", tmp[1], tmp[0]); + msg_cinfo("WP: status.srp0: %x\n", sr1.srp0); + msg_cinfo("WP: status.srp1: %x\n", sr2.srp1); + msg_cinfo("WP: write protect is %s.\n", + (sr1.srp0 || sr2.srp1) ? "enabled" : "disabled"); + + msg_cinfo("WP: write protect range: "); + if (w25_large_status_to_range(flash, &sr1, &start, &len)) { + msg_cinfo("(cannot resolve the range)\n"); + ret = -1; + } else { + msg_cinfo("start=0x%08x, len=0x%08x\n", start, len); + } + + return ret; +} + +/* Set/clear the SRP0 bit in the status register. */ +static int w25_set_srp0(const struct flashctx *flash, int enable) +{ + struct w25q_status status; + int tmp = 0; + int expected = 0; + + memset(&status, 0, sizeof(status)); + tmp = do_read_status(flash); + /* FIXME: this is NOT endian-free copy. */ + memcpy(&status, &tmp, 1); + msg_cdbg("%s: old status: 0x%02x\n", __func__, tmp); + + status.srp0 = enable ? 1 : 0; + memcpy(&expected, &status, sizeof(status)); + do_write_status(flash, expected); + + tmp = do_read_status(flash); + msg_cdbg("%s: new status: 0x%02x\n", __func__, tmp); + if ((tmp & MASK_WP_AREA) != (expected & MASK_WP_AREA)) + return 1; + + return 0; +} + +static int w25_enable_writeprotect(const struct flashctx *flash, + enum wp_mode wp_mode) +{ + int ret; + + if (wp_mode != WP_MODE_HARDWARE) { + msg_cerr("%s(): unsupported write-protect mode\n", __func__); + return 1; + } + + ret = w25_set_srp0(flash, 1); + if (ret) + msg_cerr("%s(): error=%d.\n", __func__, ret); + return ret; +} + +static int w25_disable_writeprotect(const struct flashctx *flash) +{ + int ret; + + ret = w25_set_srp0(flash, 0); + if (ret) + msg_cerr("%s(): error=%d.\n", __func__, ret); + + return ret; +} + +static int w25_list_ranges(const struct flashctx *flash) +{ + struct wp_range_descriptor *descrs; + int i, num_entries; + + if (w25_range_table(flash, &descrs, &num_entries)) + return -1; + + for (i = 0; i < num_entries; i++) { + msg_cinfo("start: 0x%06x, length: 0x%06x\n", + descrs[i].range.start, + descrs[i].range.len); + } + + return 0; +} + +static int w25q_wp_status(const struct flashctx *flash) +{ + struct w25q_status sr1; + struct w25q_status_2 sr2; + uint8_t tmp[2]; + unsigned int start, len; + int ret = 0; + + memset(&sr1, 0, sizeof(sr1)); + tmp[0] = do_read_status(flash); + memcpy(&sr1, &tmp[0], 1); + + memset(&sr2, 0, sizeof(sr2)); + tmp[1] = w25q_read_status_register_2(flash); + memcpy(&sr2, &tmp[1], 1); + + msg_cinfo("WP: status: 0x%02x%02x\n", tmp[1], tmp[0]); + msg_cinfo("WP: status.srp0: %x\n", sr1.srp0); + msg_cinfo("WP: status.srp1: %x\n", sr2.srp1); + msg_cinfo("WP: write protect is %s.\n", + (sr1.srp0 || sr2.srp1) ? "enabled" : "disabled"); + + msg_cinfo("WP: write protect range: "); + if (w25_status_to_range(flash, &sr1, &start, &len)) { + msg_cinfo("(cannot resolve the range)\n"); + ret = -1; + } else { + msg_cinfo("start=0x%08x, len=0x%08x\n", start, len); + } + + return ret; +} + +/* + * W25Q adds an optional byte to the standard WRSR opcode. If /CS is + * de-asserted after the first byte, then it acts like a JEDEC-standard + * WRSR command. if /CS is asserted, then the next data byte is written + * into status register 2. + */ +#define W25Q_WRSR_OUTSIZE 0x03 +static int w25q_write_status_register_WREN(const struct flashctx *flash, uint8_t s1, uint8_t s2) +{ + int result; + struct spi_command cmds[] = { + { + /* FIXME: WRSR requires either EWSR or WREN depending on chip type. */ + .writecnt = JEDEC_WREN_OUTSIZE, + .writearr = (const unsigned char[]){ JEDEC_WREN }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = W25Q_WRSR_OUTSIZE, + .writearr = (const unsigned char[]){ JEDEC_WRSR, s1, s2 }, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .writearr = NULL, + .readcnt = 0, + .readarr = NULL, + }}; + + result = spi_send_multicommand(flash, cmds); + if (result) { + msg_cerr("%s failed during command execution\n", + __func__); + } + + /* WRSR performs a self-timed erase before the changes take effect. */ + programmer_delay(100 * 1000); + + return result; +} + +/* + * Set/clear the SRP1 bit in status register 2. + * FIXME: make this more generic if other chips use the same SR2 layout + */ +static int w25q_set_srp1(const struct flashctx *flash, int enable) +{ + struct w25q_status sr1; + struct w25q_status_2 sr2; + uint8_t tmp, expected; + + tmp = do_read_status(flash); + memcpy(&sr1, &tmp, 1); + tmp = w25q_read_status_register_2(flash); + memcpy(&sr2, &tmp, 1); + + msg_cdbg("%s: old status 2: 0x%02x\n", __func__, tmp); + + sr2.srp1 = enable ? 1 : 0; + + memcpy(&expected, &sr2, 1); + w25q_write_status_register_WREN(flash, *((uint8_t *)&sr1), *((uint8_t *)&sr2)); + + tmp = w25q_read_status_register_2(flash); + msg_cdbg("%s: new status 2: 0x%02x\n", __func__, tmp); + if ((tmp & MASK_WP2_AREA) != (expected & MASK_WP2_AREA)) + return 1; + + return 0; +} + enum wp_mode get_wp_mode(const char *mode_str) { enum wp_mode wp_mode = WP_MODE_UNKNOWN; @@ -130,6 +1041,158 @@ return wp_mode; } +static int w25q_disable_writeprotect(const struct flashctx *flash, + enum wp_mode wp_mode) +{ + int ret = 1; + struct w25q_status_2 sr2; + uint8_t tmp; + + switch (wp_mode) { + case WP_MODE_HARDWARE: + ret = w25_set_srp0(flash, 0); + break; + case WP_MODE_POWER_CYCLE: + tmp = w25q_read_status_register_2(flash); + memcpy(&sr2, &tmp, 1); + if (sr2.srp1) { + msg_cerr("%s(): must disconnect power to disable " + "write-protection\n", __func__); + } else { + ret = 0; + } + break; + case WP_MODE_PERMANENT: + msg_cerr("%s(): cannot disable permanent write-protection\n", + __func__); + break; + default: + msg_cerr("%s(): invalid mode specified\n", __func__); + break; + } + + if (ret) + msg_cerr("%s(): error=%d.\n", __func__, ret); + + return ret; +} + +static int w25q_disable_writeprotect_default(const struct flashctx *flash) +{ + return w25q_disable_writeprotect(flash, WP_MODE_HARDWARE); +} + +static int w25q_enable_writeprotect(const struct flashctx *flash, + enum wp_mode wp_mode) +{ + int ret = 1; + struct w25q_status sr1; + struct w25q_status_2 sr2; + uint8_t tmp; + + switch (wp_mode) { + case WP_MODE_HARDWARE: + if (w25q_disable_writeprotect(flash, WP_MODE_POWER_CYCLE)) { + msg_cerr("%s(): cannot disable power cycle WP mode\n", + __func__); + break; + } + + tmp = do_read_status(flash); + memcpy(&sr1, &tmp, 1); + if (sr1.srp0) + ret = 0; + else + ret = w25_set_srp0(flash, 1); + + break; + case WP_MODE_POWER_CYCLE: + if (w25q_disable_writeprotect(flash, WP_MODE_HARDWARE)) { + msg_cerr("%s(): cannot disable hardware WP mode\n", + __func__); + break; + } + + tmp = w25q_read_status_register_2(flash); + memcpy(&sr2, &tmp, 1); + if (sr2.srp1) + ret = 0; + else + ret = w25q_set_srp1(flash, 1); + + break; + case WP_MODE_PERMANENT: + tmp = do_read_status(flash); + memcpy(&sr1, &tmp, 1); + if (sr1.srp0 == 0) { + ret = w25_set_srp0(flash, 1); + if (ret) { + msg_perr("%s(): cannot enable SRP0 for " + "permanent WP\n", __func__); + break; + } + } + + tmp = w25q_read_status_register_2(flash); + memcpy(&sr2, &tmp, 1); + if (sr2.srp1 == 0) { + ret = w25q_set_srp1(flash, 1); + if (ret) { + msg_perr("%s(): cannot enable SRP1 for " + "permanent WP\n", __func__); + break; + } + } + + break; + default: + msg_perr("%s(): invalid mode %d\n", __func__, wp_mode); + break; + } + + if (ret) + msg_cerr("%s(): error=%d.\n", __func__, ret); + return ret; +} + +/* W25P, W25X, and many flash chips from various vendors */ +struct wp wp_w25 = { + .list_ranges = w25_list_ranges, + .set_range = w25_set_range, + .enable = w25_enable_writeprotect, + .disable = w25_disable_writeprotect, + .wp_status = w25_wp_status, + +}; + +/* W25Q series has features such as a second status register and SFDP */ +struct wp wp_w25q = { + .list_ranges = w25_list_ranges, + .set_range = w25_set_range, + .enable = w25q_enable_writeprotect, + /* + * By default, disable hardware write-protection. We may change + * this later if we want to add fine-grained write-protect disable + * as a command-line option. + */ + .disable = w25q_disable_writeprotect_default, + .wp_status = w25q_wp_status, +}; + +/* W25Q large series has 4 block-protect bits */ +struct wp wp_w25q_large = { + .list_ranges = w25_list_ranges, + .set_range = w25q_large_set_range, + .enable = w25q_enable_writeprotect, + /* + * By default, disable hardware write-protection. We may change + * this later if we want to add fine-grained write-protect disable + * as a command-line option. + */ + .disable = w25q_disable_writeprotect_default, + .wp_status = w25q_large_wp_status, +}; + /* Given a flash chip, this function returns its writeprotect info. */ static int generic_range_table(const struct flashctx *flash, struct wp_context **wp, diff --git a/writeprotect.h b/writeprotect.h index bded4c8..632f169 100644 --- a/writeprotect.h +++ b/writeprotect.h @@ -35,6 +35,11 @@ int (*wp_status)(const struct flashctx *flash); }; +/* winbond w25-series */ +extern struct wp wp_w25; /* older winbond chips (w25p, w25x, etc) */ +extern struct wp wp_w25q; +extern struct wp wp_w25q_large; /* large winbond chips (>= 32MB) */ + extern struct wp wp_generic; enum wp_mode get_wp_mode(const char *mode_str); -- To view, visit https://review.coreboot.org/c/flashrom/+/45572 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I54eedaa8e24c69d6d088e9ebcfb9a284e0fbe893 Gerrit-Change-Number: 45572 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 5

Change in flashrom[master]: spi25_statusreg: implement w25q_write_status_register()
by Edward O'Callaghan (Code Review) Dec. 5, 2021

Dec. 5, 2021

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45749 ) Change subject: spi25_statusreg: implement w25q_write_status_register() ...................................................................... spi25_statusreg: implement w25q_write_status_register() Change-Id: Idd9f818cb3464b3221330e7aa66f44b2b6d2adf7 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipdrivers.h M spi25_statusreg.c 2 files changed, 24 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/45749/1 diff --git a/chipdrivers.h b/chipdrivers.h index 4e76588..c2764b9 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -65,6 +65,7 @@ uint8_t spi_read_status_register(const struct flashctx *flash); uint8_t spi_read_status_register_2(const struct flashctx *flash); int spi_write_status_register(const struct flashctx *flash, int status); +int w25q_write_status_register(const struct flashctx *flash, uint8_t s0, uint8_t s1); void spi_prettyprint_status_register_bit(uint8_t status, int bit); int spi_prettyprint_status_register_plain(struct flashctx *flash); int spi_prettyprint_status_register_default_welwip(struct flashctx *flash); diff --git a/spi25_statusreg.c b/spi25_statusreg.c index cad92e6..fb49c96 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -27,6 +27,15 @@ int result; int i = 0; /* + * W25Q adds an optional byte to the standard WRSR opcode. If /CS is + * de-asserted after the first byte, then it acts like a JEDEC-standard + * WRSR command. if /CS is asserted, then the next data byte is written + * into status register 2. + */ + uint8_t s0 = status & 0xff; + uint8_t s1 = (status >> 8) & 0xff; + + /* * WRSR requires either EWSR or WREN depending on chip type. * The code below relies on the fact hat EWSR and WREN have the same * INSIZE and OUTSIZE. @@ -38,8 +47,8 @@ .readcnt = 0, .readarr = NULL, }, { - .writecnt = JEDEC_WRSR_OUTSIZE, - .writearr = (const unsigned char[]){ JEDEC_WRSR, (unsigned char) status }, + .writecnt = s1 > 0 ? (JEDEC_WRSR_OUTSIZE + 1) /* W25Q_WRSR_OUTSIZE */ : JEDEC_WREN_OUTSIZE, + .writearr = (const unsigned char[]){ JEDEC_WRSR, s0, s1 }, .readcnt = 0, .readarr = NULL, }, { @@ -90,6 +99,18 @@ return ret; } +int w25q_write_status_register(const struct flashctx *flash, uint8_t s0, uint8_t s1) +{ + int feature_bits = flash->chip->feature_bits; + int ret = 1; + + if (!(feature_bits & FEATURE_WRSR_WREN)) + msg_cdbg("Missing status register write definition\n"); + if (feature_bits & FEATURE_WRSR_WREN) + ret = spi_write_status_register_flag(flash, ((s1 << 8) | s0), JEDEC_WREN); + return ret; +} + static uint8_t spi_read_status_register_generic(const struct flashctx *flash, uint8_t opcode) { const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { opcode }; -- To view, visit https://review.coreboot.org/c/flashrom/+/45749 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idd9f818cb3464b3221330e7aa66f44b2b6d2adf7 Gerrit-Change-Number: 45749 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 1

Change in flashrom[master]: flashchips.c: Add writeprotect to selected chips
by Edward O'Callaghan (Code Review) Dec. 5, 2021

Dec. 5, 2021

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45752 ) Change subject: flashchips.c: Add writeprotect to selected chips ...................................................................... flashchips.c: Add writeprotect to selected chips These chips are the ones tested in the ChromiumOS Flashrom fork so we are just upstreaming those at the moment. BUG=b:153800563 BRANCH=none TEST=builds Change-Id: I90c34ff9c34ff40fa186244024614c40954f4498 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashchips.c 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/45752/1 diff --git a/flashchips.c b/flashchips.c index 7d10abf..7ece354 100644 --- a/flashchips.c +++ b/flashchips.c @@ -21,6 +21,7 @@ #include "flash.h" #include "flashchips.h" #include "chipdrivers.h" +#include "writeprotect.h" /** * List of supported flash chips. @@ -5959,6 +5960,7 @@ .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {1695, 1950}, + .wp = &wp_generic, }, { @@ -6396,6 +6398,7 @@ .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {2700, 3600}, + .wp = &wp_generic, }, { @@ -8277,6 +8280,7 @@ .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) supported */ .voltage = {2700, 3600}, + .wp = &wp_generic, }, { @@ -8613,6 +8617,7 @@ .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) supported */ .voltage = {2700, 3600}, + .wp = &wp_generic, }, { @@ -8767,6 +8772,7 @@ .write = spi_chip_write_256, .read = spi_chip_read, /* Fast read (0x0B) and multi I/O supported */ .voltage = {2700, 3600}, + .wp = &wp_generic, }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/45752 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I90c34ff9c34ff40fa186244024614c40954f4498 Gerrit-Change-Number: 45752 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

2 4

Change in flashrom[master]: spi25: Add 'spi_read_status_register_2()' helper
by Edward O'Callaghan (Code Review) Dec. 5, 2021

Dec. 5, 2021

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45748 ) Change subject: spi25: Add 'spi_read_status_register_2()' helper ...................................................................... spi25: Add 'spi_read_status_register_2()' helper Winbond chips have a WRSR opcode of 0x35 interestingly. Add helper to allow for reading it. This is later used by writeprotection support. BUG=b:153800563 BRANCH=none TEST=builds Change-Id: I2d25f22ced17ab94d122e86b270357723b45458e Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M chipdrivers.h M spi25_statusreg.c 2 files changed, 13 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/45748/1 diff --git a/chipdrivers.h b/chipdrivers.h index cf03811..4e76588 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -63,6 +63,7 @@ /* spi25_statusreg.c */ uint8_t spi_read_status_register(const struct flashctx *flash); +uint8_t spi_read_status_register_2(const struct flashctx *flash); int spi_write_status_register(const struct flashctx *flash, int status); void spi_prettyprint_status_register_bit(uint8_t status, int bit); int spi_prettyprint_status_register_plain(struct flashctx *flash); diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 34f9ad4..cad92e6 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -90,9 +90,9 @@ return ret; } -uint8_t spi_read_status_register(const struct flashctx *flash) +static uint8_t spi_read_status_register_generic(const struct flashctx *flash, uint8_t opcode) { - static const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { JEDEC_RDSR }; + const unsigned char cmd[JEDEC_RDSR_OUTSIZE] = { opcode }; /* FIXME: No workarounds for driver/hardware bugs in generic code. */ unsigned char readarr[2]; /* JEDEC_RDSR_INSIZE=1 but wbsio needs 2 */ int ret; @@ -108,6 +108,16 @@ return readarr[0]; } +uint8_t spi_read_status_register(const struct flashctx *flash) +{ + return spi_read_status_register_generic(flash, JEDEC_RDSR); +} + +uint8_t spi_read_status_register_2(const struct flashctx *flash) +{ + return spi_read_status_register_generic(flash, 0x35); /* Unclear if this is a std JEDEC opcode. */ +} + /* A generic block protection disable. * Tests if a protection is enabled with the block protection mask (bp_mask) and returns success otherwise. * Tests if the register bits are locked with the lock_mask (lock_mask). -- To view, visit https://review.coreboot.org/c/flashrom/+/45748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d25f22ced17ab94d122e86b270357723b45458e Gerrit-Change-Number: 45748 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

3 5

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Anastasia Klimchuk (Code Review) Nov. 30, 2021

Nov. 30, 2021

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 18: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/598c3d42_7f331586 PS17, Line 220: ret = flashrom_wp_set_range(flash, &cfg, &range); : if (ret) { : msg_gerr("The chip does not support the requested range.\n"); : return ret; : } : : /* Write range before other operations (i.e. enabling HW protection) */ : ret = write_wp_config(flash, &cfg); : if (ret) : return ret; > > It's good to know there is no harm, but is there any effect of doing 1,2 without 3? What would be […] Ok, thank you for explaining, so error messages is the only inconvenience, is that correct? I am wondering about printing messages. Currently it is done by `msg_gerr`, but I don't think it is specific to cli_classic? msg_gerr together with msg_pdbg, msg_ginfo etc are used literally everywhere in codebase, including libflashrom.c. Why they can't be used inside flashrom_wp_set_range or mega_flashrom_wp_set_range? -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 18 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 30 Nov 2021 23:53:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Sergii Dmytruk (Code Review) Nov. 30, 2021

Nov. 30, 2021

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 18: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/c9108ecb_0a6f01ef PS17, Line 220: ret = flashrom_wp_set_range(flash, &cfg, &range); : if (ret) { : msg_gerr("The chip does not support the requested range.\n"); : return ret; : } : : /* Write range before other operations (i.e. enabling HW protection) */ : ret = write_wp_config(flash, &cfg); : if (ret) : return ret; > It's good to know there is no harm, but is there any effect of doing 1,2 without 3? What would be the most common usage, 1-2-3 or 1-2? 1-2 case is harmless and quite useless since WP config is essentially opaque. > The usual intention is that a user would try to set the range (read/set_range/write), check that it succeeded, and only then set the mode (read/set_mode/write). So we could possibly change the set_mode and set_range functions to also write to the chip, but there are still the problems. In this case making calls do more work will make more sense if config gets completely removed from the API as a result, but that needs some kind of error object that can be converted to a string or just `char **error` that gets set to a descriptive error message. -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 18 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 30 Nov 2021 23:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0