March 2023 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: dmi.c: Pass is_laptop by ref into dmi
by Edward O'Callaghan (Code Review) 28 Mar '23

28 Mar '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71622 ) Change subject: dmi.c: Pass is_laptop by ref into dmi ...................................................................... dmi.c: Pass is_laptop by ref into dmi Prefix the remaining global cases with `g_` to avoid shadowing issues and for easy greping. Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71622 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> --- M board_enable.c M dmi.c M include/programmer.h M internal.c 4 files changed, 47 insertions(+), 31 deletions(-) Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved diff --git a/board_enable.c b/board_enable.c index f0df211..0531fab 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2282,7 +2282,7 @@ static int p2_not_a_laptop(void) { /* label this board as not a laptop */ - is_laptop = 0; + g_is_laptop = 0; msg_pdbg("Laptop detection overridden by P2 board enable.\n"); return 0; } @@ -2292,7 +2292,7 @@ */ static int p2_whitelist_laptop(void) { - is_laptop = 1; + g_is_laptop = 1; g_laptop_ok = true; msg_pdbg("Whitelisted laptop detected.\n"); return 0; diff --git a/dmi.c b/dmi.c index beeb10f..94e47fb 100644 --- a/dmi.c +++ b/dmi.c @@ -151,11 +151,11 @@ return newbuf; } -static void dmi_chassis_type(uint8_t code) +static int dmi_chassis_type(uint8_t code) { unsigned int i; code &= 0x7f; /* bits 6:0 are chassis type, 7th bit is the lock bit */ - is_laptop = 2; + int is_laptop = 2; for (i = 0; i < ARRAY_SIZE(dmi_chassis_types); i++) { if (code == dmi_chassis_types[i].type) { msg_pdbg("DMI string chassis-type: \"%s\"\n", dmi_chassis_types[i].name); @@ -163,9 +163,10 @@ break; } } + return is_laptop; } -static void dmi_table(uint32_t base, uint16_t len, uint16_t num) +static void dmi_table(uint32_t base, uint16_t len, uint16_t num, int *is_laptop) { unsigned int i = 0, j = 0; @@ -198,7 +199,7 @@ if(data[0] == 3) { if (data + 5 < limit) - dmi_chassis_type(data[5]); + *is_laptop = dmi_chassis_type(data[5]); else /* the table is broken, but laptop detection is optional, hence continue. */ msg_pwarn("DMI table is broken (chassis_type out of bounds)!\n"); } else @@ -232,7 +233,7 @@ } #if SM_SUPPORT -static int smbios_decode(uint8_t *buf) +static int smbios_decode(uint8_t *buf, int *is_laptop) { /* TODO: other checks mentioned in the conformance guidelines? */ if (!dmi_checksum(buf, buf[0x05]) || @@ -240,23 +241,23 @@ !dmi_checksum(buf + 0x10, 0x0F)) return 0; - dmi_table(mmio_readl(buf + 0x18), mmio_readw(buf + 0x16), mmio_readw(buf + 0x1C)); + dmi_table(mmio_readl(buf + 0x18), mmio_readw(buf + 0x16), mmio_readw(buf + 0x1C), is_laptop); return 1; } #endif -static int legacy_decode(uint8_t *buf) +static int legacy_decode(uint8_t *buf, int *is_laptop) { if (!dmi_checksum(buf, 0x0F)) return 1; - dmi_table(mmio_readl(buf + 0x08), mmio_readw(buf + 0x06), mmio_readw(buf + 0x0C)); + dmi_table(mmio_readl(buf + 0x08), mmio_readw(buf + 0x06), mmio_readw(buf + 0x0C), is_laptop); return 0; } -static int dmi_fill(void) +static int dmi_fill(int *is_laptop) { size_t fp; uint8_t *dmi_mem; @@ -274,12 +275,12 @@ for (fp = 0; fp <= 0xFFF0; fp += 16) { #if SM_SUPPORT if (memcmp(dmi_mem + fp, "_SM_", 4) == 0 && fp <= 0xFFE0) { - if (smbios_decode(dmi_mem + fp)) // FIXME: length check + if (smbios_decode(dmi_mem + fp), is_laptop) // FIXME: length check goto out; } else #endif if (memcmp(dmi_mem + fp, "_DMI_", 5) == 0) - if (legacy_decode(dmi_mem + fp) == 0) { + if (legacy_decode(dmi_mem + fp, is_laptop) == 0) { ret = 0; goto out; } @@ -350,7 +351,7 @@ return result; } -static int dmi_fill(void) +static int dmi_fill(int *is_laptop) { unsigned int i; char *chassis_type; @@ -367,10 +368,10 @@ return 0; /* chassis-type handling is optional anyway */ msg_pdbg("DMI string chassis-type: \"%s\"\n", chassis_type); - is_laptop = 2; + *is_laptop = 2; for (i = 0; i < ARRAY_SIZE(dmi_chassis_types); i++) { if (strcasecmp(chassis_type, dmi_chassis_types[i].name) == 0) { - is_laptop = dmi_chassis_types[i].is_laptop; + *is_laptop = dmi_chassis_types[i].is_laptop; break; } } @@ -391,7 +392,7 @@ return 0; } -void dmi_init(void) +void dmi_init(int *is_laptop) { /* Register shutdown function before we allocate anything. */ if (register_shutdown(dmi_shutdown, NULL)) { @@ -399,11 +400,11 @@ return; } - /* dmi_fill fills the dmi_strings array, and if possible sets the global is_laptop variable. */ - if (dmi_fill() != 0) + /* dmi_fill fills the dmi_strings array, and if possible set the is_laptop parameter. */ + if (dmi_fill(is_laptop) != 0) return; - switch (is_laptop) { + switch (*is_laptop) { case 1: msg_pdbg("Laptop detected via DMI.\n"); break; diff --git a/include/programmer.h b/include/programmer.h index 304a216..ab482e6 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -244,7 +244,7 @@ /* dmi.c */ #if defined(__i386__) || defined(__x86_64__) -void dmi_init(void); +void dmi_init(int *is_laptop); bool dmi_is_supported(void); int dmi_match(const char *pattern); #endif // defined(__i386__) || defined(__x86_64__) @@ -263,7 +263,7 @@ #endif #if CONFIG_INTERNAL == 1 -extern int is_laptop; +extern int g_is_laptop; extern bool g_laptop_ok; extern bool force_boardmismatch; void probe_superio(void); diff --git a/internal.c b/internal.c index 0c23326..5d836e5 100644 --- a/internal.c +++ b/internal.c @@ -27,7 +27,7 @@ #include "hwaccess_x86_io.h" #endif -int is_laptop = 0; +int g_is_laptop = 0; bool g_laptop_ok = false; bool force_boardmismatch = false; @@ -108,12 +108,11 @@ return 0; } -// FIXME: remove '_' suffix from parameters once global shadowing is fixed. -static void report_nonwl_laptop_detected(int is_laptop_, bool laptop_ok) +static void report_nonwl_laptop_detected(int is_laptop, bool laptop_ok) { - if (is_laptop_ && !laptop_ok) { + if (is_laptop && !laptop_ok) { msg_pinfo("========================================================================\n"); - if (is_laptop_ == 1) { + if (is_laptop == 1) { msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n" "internal buses have been disabled for safety reasons.\n\n"); } else { @@ -204,9 +203,9 @@ } } - is_laptop = 2; /* Assume that we don't know by default. */ + g_is_laptop = 2; /* Assume that we don't know by default. */ - dmi_init(); + dmi_init(&g_is_laptop); /* In case Super I/O probing would cause pretty explosions. */ board_handle_before_superio(force_boardenable); @@ -229,7 +228,7 @@ * this isn't a laptop. Board-enables may override this, * non-legacy buses (SPI and opaque atm) are probed anyway. */ - if (is_laptop && !(g_laptop_ok || force_laptop || (not_a_laptop && is_laptop == 2))) + if (g_is_laptop && !(g_laptop_ok || force_laptop || (not_a_laptop && g_is_laptop == 2))) internal_buses_supported = BUS_NONE; /* try to enable it. Failure IS an option, since not all motherboards @@ -258,7 +257,7 @@ internal_par_init(internal_buses_supported); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ - report_nonwl_laptop_detected(is_laptop, g_laptop_ok); + report_nonwl_laptop_detected(g_is_laptop, g_laptop_ok); ret = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/71622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Gerrit-Change-Number: 71622 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: tree/: Rename 'laptop_ok-> g_laptop_ok' to avoid shadowing
by Edward O'Callaghan (Code Review) 28 Mar '23

28 Mar '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73646 ) Change subject: tree/: Rename 'laptop_ok-> g_laptop_ok' to avoid shadowing ...................................................................... tree/: Rename 'laptop_ok-> g_laptop_ok' to avoid shadowing Avoid global symbol shadowing in local functions. Change-Id: Idfca0b7e46d8051bf680227250cf40483e19dc53 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73646 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Sam McNally <sammc(a)google.com> --- M board_enable.c M chipset_enable.c M include/programmer.h M internal.c 4 files changed, 27 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved diff --git a/board_enable.c b/board_enable.c index d35b151..f0df211 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2293,7 +2293,7 @@ static int p2_whitelist_laptop(void) { is_laptop = 1; - laptop_ok = true; + g_laptop_ok = true; msg_pdbg("Whitelisted laptop detected.\n"); return 0; } diff --git a/chipset_enable.c b/chipset_enable.c index e1dd6a3..37a48fc 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -826,7 +826,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (boot_buses & BUS_SPI) - laptop_ok = true; + g_laptop_ok = true; return 0; } @@ -971,7 +971,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (!ret && (boot_buses & BUS_SPI)) - laptop_ok = true; + g_laptop_ok = true; _freepci_ret: pci_free_dev(spi_dev); @@ -1087,7 +1087,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (boot_buses & BUS_SPI) - laptop_ok = true; + g_laptop_ok = true; return 0; } @@ -1676,7 +1676,7 @@ /* Suppress unknown laptop warning if we booted from SPI. */ if (!ret && want_spi) - laptop_ok = true; + g_laptop_ok = true; return ret; } diff --git a/include/programmer.h b/include/programmer.h index d8d8b4a..304a216 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -264,7 +264,7 @@ #if CONFIG_INTERNAL == 1 extern int is_laptop; -extern bool laptop_ok; +extern bool g_laptop_ok; extern bool force_boardmismatch; void probe_superio(void); int register_superio(struct superio s); diff --git a/internal.c b/internal.c index c75b472..0c23326 100644 --- a/internal.c +++ b/internal.c @@ -28,7 +28,7 @@ #endif int is_laptop = 0; -bool laptop_ok = false; +bool g_laptop_ok = false; bool force_boardmismatch = false; @@ -109,9 +109,9 @@ } // FIXME: remove '_' suffix from parameters once global shadowing is fixed. -static void report_nonwl_laptop_detected(int is_laptop_, bool laptop_ok_) +static void report_nonwl_laptop_detected(int is_laptop_, bool laptop_ok) { - if (is_laptop_ && !laptop_ok_) { + if (is_laptop_ && !laptop_ok) { msg_pinfo("========================================================================\n"); if (is_laptop_ == 1) { msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n" @@ -159,7 +159,7 @@ return ret; /* Unconditionally reset global state from previous operation. */ - laptop_ok = false; + g_laptop_ok = false; /* Default to Parallel/LPC/FWH flash devices. If a known host controller * is found, the host controller init routine sets the @@ -229,7 +229,7 @@ * this isn't a laptop. Board-enables may override this, * non-legacy buses (SPI and opaque atm) are probed anyway. */ - if (is_laptop && !(laptop_ok || force_laptop || (not_a_laptop && is_laptop == 2))) + if (is_laptop && !(g_laptop_ok || force_laptop || (not_a_laptop && is_laptop == 2))) internal_buses_supported = BUS_NONE; /* try to enable it. Failure IS an option, since not all motherboards @@ -258,7 +258,7 @@ internal_par_init(internal_buses_supported); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ - report_nonwl_laptop_detected(is_laptop, laptop_ok); + report_nonwl_laptop_detected(is_laptop, g_laptop_ok); ret = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/73646 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idfca0b7e46d8051bf680227250cf40483e19dc53 Gerrit-Change-Number: 73646 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashchips: Add Macronix MX25V1635F support
by Anastasia Klimchuk (Code Review) 26 Mar '23

26 Mar '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73824 ) Change subject: flashchips: Add Macronix MX25V1635F support ...................................................................... flashchips: Add Macronix MX25V1635F support See https://www.mxic.com.tw/Lists/Datasheet/Attachments/8662/MX25V1635F,%202.5V… . I've tested this patch with the MX25V1635F I have here, using serprog and ftdi by (re)writing a few images to the flash and seeing if changes were stored correctly. This also included erasing and rewriting the memory with completely different data, so erase is tested, too. Change-Id: I58ddaaa96ef410d50dde3aaa20376c5bbf0f370b Signed-off-by: PoroCYon <p(a)pcy.be> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73824 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c M include/flashchips.h 2 files changed, 56 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index ad0dbad..dc6e882 100644 --- a/flashchips.c +++ b/flashchips.c @@ -9644,6 +9644,41 @@ .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ .voltage = {2300, 3600}, }, + { + .vendor = "Macronix", + .name = "MX25V1635F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25V1635F, + .total_size = 2048, + .page_size = 256, + /* OTP: 8KiB total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_SCUR, + .tested = TEST_OK_PREW, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { { 4 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 64} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 32} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {2 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, /* Multi I/O supported */ + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2300, 3600}, + }, { .vendor = "Macronix", diff --git a/include/flashchips.h b/include/flashchips.h index 70ed544..aa59a6d 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -514,6 +514,7 @@ #define MACRONIX_MX66L1G45G 0x201b /* MX66L1G45G */ #define MACRONIX_MX25V4035F 0x2313 #define MACRONIX_MX25V8035F 0x2314 +#define MACRONIX_MX25V1635F 0x2315 /* MX25V1636E */ #define MACRONIX_MX25U8032E 0x2534 #define MACRONIX_MX25U1635E 0x2535 #define MACRONIX_MX25U3235E 0x2536 /* Same as MX25U6435F */ -- To view, visit https://review.coreboot.org/c/flashrom/+/73824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I58ddaaa96ef410d50dde3aaa20376c5bbf0f370b Gerrit-Change-Number: 73824 Gerrit-PatchSet: 3 Gerrit-Owner: PoroCYon <p(a)pcy.be> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashchips: Add Macronix MX25V8035F support
by Anastasia Klimchuk (Code Review) 26 Mar '23

26 Mar '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73823 ) Change subject: flashchips: Add Macronix MX25V8035F support ...................................................................... flashchips: Add Macronix MX25V8035F support See https://www.macronix.com/Lists/Datasheet/Attachments/8405/MX25V8035F,%202.5… . I've only tested this patch with the MX25V1635F I have here, though other chips in the series exist as well. Tested using serprog and ftdi by writing a few images to the flash and seeing if changes were stored correctly. Change-Id: Ic5be2da4cfa2a2ff044a519bb6f367f21c15e4b8 Signed-off-by: PoroCYon <p(a)pcy.be> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73823 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c M include/flashchips.h 2 files changed, 56 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 745cee5..ad0dbad 100644 --- a/flashchips.c +++ b/flashchips.c @@ -9609,6 +9609,41 @@ .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ .voltage = {2300, 3600}, }, + { + .vendor = "Macronix", + .name = "MX25V8035F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25V8035F, + .total_size = 1024, + .page_size = 256, + /* OTP: 8KiB total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_SCUR, + .tested = TEST_UNTESTED, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { { 4 * 1024, 256} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 32} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 16} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {1 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, /* Multi I/O supported */ + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2300, 3600}, + }, { .vendor = "Macronix", diff --git a/include/flashchips.h b/include/flashchips.h index 58a6d0e..70ed544 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -513,6 +513,7 @@ #define MACRONIX_MX66L51235F 0x201a /* MX66L51235F, MX25L51245G */ #define MACRONIX_MX66L1G45G 0x201b /* MX66L1G45G */ #define MACRONIX_MX25V4035F 0x2313 +#define MACRONIX_MX25V8035F 0x2314 #define MACRONIX_MX25U8032E 0x2534 #define MACRONIX_MX25U1635E 0x2535 #define MACRONIX_MX25U3235E 0x2536 /* Same as MX25U6435F */ -- To view, visit https://review.coreboot.org/c/flashrom/+/73823 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5be2da4cfa2a2ff044a519bb6f367f21c15e4b8 Gerrit-Change-Number: 73823 Gerrit-PatchSet: 2 Gerrit-Owner: PoroCYon <p(a)pcy.be> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashchips: Add Macronix MX25V4035F support
by Anastasia Klimchuk (Code Review) 26 Mar '23

26 Mar '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73582 ) Change subject: flashchips: Add Macronix MX25V4035F support ...................................................................... flashchips: Add Macronix MX25V4035F support See https://www.macronix.com/Lists/Datasheet/Attachments/8670/MX25V4035F,%202.5… . I've only tested this patch with the MX25V1635F I have here, though other chips in the series exist as well. Tested using serprog and ftdi by writing a few images to the flash and seeing if changes were stored correctly. Change-Id: I8b26926c354b840ca7b14b4c5cb000e3c02f5137 Signed-off-by: PoroCYon <p(a)pcy.be> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73582 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c M include/flashchips.h 2 files changed, 57 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 6764c33..745cee5 100644 --- a/flashchips.c +++ b/flashchips.c @@ -9576,6 +9576,42 @@ { .vendor = "Macronix", + .name = "MX25V4035F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25V4035F, + .total_size = 512, + .page_size = 256, + /* OTP: 8KiB total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_SCUR, + .tested = TEST_UNTESTED, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { { 4 * 1024, 128} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 16} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 8} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {512 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, /* Multi I/O supported */ + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2300, 3600}, + }, + + { + .vendor = "Macronix", .name = "MX25U12835F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, diff --git a/include/flashchips.h b/include/flashchips.h index 5df42dc..58a6d0e 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -512,6 +512,7 @@ #define MACRONIX_MX25L1635E 0x2515 /* MX25L1635{E} */ #define MACRONIX_MX66L51235F 0x201a /* MX66L51235F, MX25L51245G */ #define MACRONIX_MX66L1G45G 0x201b /* MX66L1G45G */ +#define MACRONIX_MX25V4035F 0x2313 #define MACRONIX_MX25U8032E 0x2534 #define MACRONIX_MX25U1635E 0x2535 #define MACRONIX_MX25U3235E 0x2536 /* Same as MX25U6435F */ -- To view, visit https://review.coreboot.org/c/flashrom/+/73582 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8b26926c354b840ca7b14b4c5cb000e3c02f5137 Gerrit-Change-Number: 73582 Gerrit-PatchSet: 3 Gerrit-Owner: PoroCYon <p(a)pcy.be> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: doc: Add contact page
by Alexander Goncharov (Code Review) 24 Mar '23

24 Mar '23

Attention is currently required from: Thomas Heijligen. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73822 ) Change subject: doc: Add contact page ...................................................................... Patch Set 4: Code-Review+1 (1 comment) File doc/contact.rst: https://review.coreboot.org/c/flashrom/+/73822/comment/63f233d2_7f92ab5c PS4, Line 85: * Looks like it shouldn't be part of the list -- To view, visit https://review.coreboot.org/c/flashrom/+/73822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibfba6a59c5a945b4238d16e07a07584f94159568 Gerrit-Change-Number: 73822 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Fri, 24 Mar 2023 10:02:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: MAINTAINERS: Re-order programmer entries alphabetically
by Anastasia Klimchuk (Code Review) 24 Mar '23

24 Mar '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73965 ) Change subject: MAINTAINERS: Re-order programmer entries alphabetically ...................................................................... MAINTAINERS: Re-order programmer entries alphabetically Change-Id: I7e9013fd4cd3b9ea6ecdb45fabbc3c57c4076a17 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/73965 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M MAINTAINERS 1 file changed, 19 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Miklós Márton: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved diff --git a/MAINTAINERS b/MAINTAINERS index 78ec209..294597c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -150,6 +150,11 @@ S: Supported F: linux_mtd.c +National Instruments USB-845x +M: Miklos Marton <martonmiklosqdev(a)gmail.com> +S: Maintained +F: ni845x_spi.c + RAIDEN DEBUG SPI M: Nikolai Artemiev <nartemiev(a)google.com> S: Maintained @@ -160,11 +165,6 @@ S: Maintained F: stlinkv3_spi.c -National Instruments USB-845x -M: Miklos Marton <martonmiklosqdev(a)gmail.com> -S: Maintained -F: ni845x_spi.c - ################################################################################ # TESTS ################################################################################ -- To view, visit https://review.coreboot.org/c/flashrom/+/73965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7e9013fd4cd3b9ea6ecdb45fabbc3c57c4076a17 Gerrit-Change-Number: 73965 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: atavia: Use a atavia_offset locally to determin offset
by Anastasia Klimchuk (Code Review) 24 Mar '23

24 Mar '23

Attention is currently required from: Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73825 ) Change subject: atavia: Use a atavia_offset locally to determin offset ...................................................................... Patch Set 1: Code-Review+1 (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/73825/comment/dc3596a1_6eed7b46 PS1, Line 7: Use a atavia_offset locally to determin offset I think this should be more specific: `Always initialise atavia_offset from programmer param` And the below, in commit description you can add more clarification like `Remove custom atavia_map function and detect the value of atavia_offset from programmer param. This means to use atavia_offset directly <and the the rest of your description>` File atavia.c: https://review.coreboot.org/c/flashrom/+/73825/comment/190bbf64_32432ff5 PS1, Line 58: try and error trial and error https://review.coreboot.org/c/flashrom/+/73825/comment/4d91b4b7_fcc1d42b PS1, Line 57: /* The atavia_offset is some kind of magic to get the driver working. : * You can find the correct value in the documentation or by try and error. : */ I think you need to add new line: /* * comment */ https://review.coreboot.org/c/flashrom/+/73825/comment/9fbb0a24_ed9e1e9c PS1, Line 140: char *arg = extract_programmer_param_str(cfg, "offset"); It seems like offset should be a required parameter now? I don't think the default offset 0 would ever work? -- To view, visit https://review.coreboot.org/c/flashrom/+/73825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie0ec85c276f03942d6b9435a746ec329b2b95926 Gerrit-Change-Number: 73825 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 24 Mar 2023 07:49:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ni845x_spi: refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 24 Mar '23

24 Mar '23

Attention is currently required from: Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72160 ) Change subject: ni845x_spi: refactor singleton states into reentrant pattern ...................................................................... Patch Set 2: (5 comments) File ni845x_spi.c: https://review.coreboot.org/c/flashrom/+/72160/comment/b93cd2df_5955a65b PS2, Line 228: uInt32 device_handle I thought the purpose of the first patch in the chain CB:72154 is to do this type of work: adding arguments to functions and passing values as arguments instead of using globals? Is there a reason why this specific diff, adding `uInt32 device_handle` as an argument to `usb8452_spi_set_io_voltage` cannot be done in CB:72154 ? https://review.coreboot.org/c/flashrom/+/72160/comment/d5aa83ad_978fcd32 PS2, Line 395: data These are actually arguments, not global data https://review.coreboot.org/c/flashrom/+/72160/comment/cdc05f96_16ca4d87 PS2, Line 575: uInt32 device_handle `device_handle` declared as `Int32` in the `ni845x_spi_data` struct https://review.coreboot.org/c/flashrom/+/72160/comment/52da9761_537b4bbd PS2, Line 640: You won't need previous patch CB:72159 if you do: 1) allocate data here, and use data->member_name from here and below 2) add `free(data)` in shutdown function 3) call shutdown function with `data` in `err:` label branch Also it would be consistent with other programmers, all of them which have data struct they free data in shutdown function. what do you think? https://review.coreboot.org/c/flashrom/+/72160/comment/893aa9e6_c124bbb0 PS2, Line 663: NULL and now you need to pass `data` here, now you have data! -- To view, visit https://review.coreboot.org/c/flashrom/+/72160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I45fcb8e20582cb0c532c4a9f0c78543a25f8d484 Gerrit-Change-Number: 72160 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 24 Mar 2023 07:31:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ni845x_spi: handle errors using goto during initialization
by Anastasia Klimchuk (Code Review) 24 Mar '23

24 Mar '23

Attention is currently required from: Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72156 ) Change subject: ni845x_spi: handle errors using goto during initialization ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72156 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie9620d59db229729fd8523f99b0917d938bcc4ed Gerrit-Change-Number: 72156 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 24 Mar 2023 07:06:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2023