February 2021 - flashrom-gerrit

Change in flashrom[master]: flashchips: add definition of the XT25F02E SPI NOR flash
by Angel Pons (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Neil Armstrong. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50263 ) Change subject: flashchips: add definition of the XT25F02E SPI NOR flash ...................................................................... Patch Set 1: Code-Review+1 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/50263/comment/e262831f_4f21e27a PS1, Line 9: Technology Limited. nit: put this part on the next line (line length limit for commit messages is 72 characters) File flashchips.c: https://review.coreboot.org/c/flashrom/+/50263/comment/fb5e6510_34710e91 PS1, Line 19242: TEST_UNTESTED It would be great to test this flash chip, if you can. https://review.coreboot.org/c/flashrom/+/50263/comment/cf451758_7b364d63 PS1, Line 19253: , { : .eraseblocks = { {256 * 1024, 1} }, : .block_erase = spi_block_erase_c7, : } Missing one entry for opcode 0x60: }, { .eraseblocks = { {256 * 1024, 1} }, .block_erase = spi_block_erase_60, } Place it before the entry for opcode 0xc7 -- To view, visit https://review.coreboot.org/c/flashrom/+/50263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I295633c448c05520e4a6aa09c08bd7c9f2346d54 Gerrit-Change-Number: 50263 Gerrit-PatchSet: 1 Gerrit-Owner: Neil Armstrong <narmstrong(a)baylibre.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Neil Armstrong <narmstrong(a)baylibre.com> Gerrit-Comment-Date: Wed, 03 Feb 2021 16:10:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: add definition of the XT25F02E SPI NOR flash
by Neil Armstrong (Code Review) 03 Feb '21

03 Feb '21

Neil Armstrong has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/50263 ) Change subject: flashchips: add definition of the XT25F02E SPI NOR flash ...................................................................... flashchips: add definition of the XT25F02E SPI NOR flash This adds definition of the XT25F02E 2MBit SPI NOR Flash from XTX Technology Limited. This also adds the XTX Technology Limited vendor ID. Signed-off-by: Neil Armstrong <narmstrong(a)baylibre.com> Change-Id: I295633c448c05520e4a6aa09c08bd7c9f2346d54 --- M flashchips.c M flashchips.h 2 files changed, 35 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/50263/1 diff --git a/flashchips.c b/flashchips.c index 38b0bfd..cfac44d 100644 --- a/flashchips.c +++ b/flashchips.c @@ -19231,6 +19231,38 @@ }, { + .vendor = "XTX Technology Limited", + .name = "XT25F02E", + .bustype = BUS_SPI, + .manufacture_id = XTX_ID, + .model_id = XTX_XT25F02E, + .total_size = 256, + .page_size = 256, + .feature_bits = FEATURE_WRSR_WREN, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 64} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {64 * 1024, 4} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {256 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_srwd, + .unlock = spi_disable_blockprotect, + .write = spi_chip_write_256, + .read = spi_chip_read, + .voltage = {2700, 3600}, + }, + + { .vendor = "Zetta Device", .name = "ZD25D20", .bustype = BUS_SPI, diff --git a/flashchips.h b/flashchips.h index a85fa6a..b44a1d4 100644 --- a/flashchips.h +++ b/flashchips.h @@ -1021,6 +1021,9 @@ #define WINBOND_W49V002A 0xB0 #define WINBOND_W49V002FA 0x32 +#define XTX_ID 0x0B /* XTX Technology Limited */ +#define XTX_XT25F02E 0x4012 + #define ZETTADEVICE_ID 0xBA /* Zetta Device */ #define ZETTADEVICE_ZD25D20 0x2012 #define ZETTADEVICE_ZD25D40 0x2013 -- To view, visit https://review.coreboot.org/c/flashrom/+/50263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I295633c448c05520e4a6aa09c08bd7c9f2346d54 Gerrit-Change-Number: 50263 Gerrit-PatchSet: 1 Gerrit-Owner: Neil Armstrong <narmstrong(a)baylibre.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: programmer.h: Convert anon union to anon struct
by Angel Pons (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50246 ) Change subject: programmer.h: Convert anon union to anon struct ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/50246/comment/02fc5f79_4efbf4cf PS2, Line 29: printf("%d\n", *foo.b); In C, dereferencing a null pointer is undefined behavior. I'm not sure what the purpose of this code snippet is. If it's to show the problems of non-null but invalid pointers, I'd use pointers to functions with different signatures. Still, I believe this change is a good idea: it's better to have null pointers than non-null but invalid pointers. -- To view, visit https://review.coreboot.org/c/flashrom/+/50246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242 Gerrit-Change-Number: 50246 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 03 Feb 2021 11:20:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi.c: Move gpio 88 toggle outside write function
by Edward O'Callaghan (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Shiyu Sun, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49785 ) Change subject: realtek_mst_i2c_spi.c: Move gpio 88 toggle outside write function ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/49785/comment/064e8abb_9a5fe7a8 PS2, Line 9: Gpio 88 toggle is used as write protection disable/enable now : and we need that to happen at the initialization of programmer. Happy to +2 this however I still would like to see a better explanation as to why. -- To view, visit https://review.coreboot.org/c/flashrom/+/49785 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I237bf9f8aa0fcbb904e7f0c09c74fd179e8c70c1 Gerrit-Change-Number: 49785 Gerrit-PatchSet: 2 Gerrit-Owner: Shiyu Sun <sshiyu(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Shiyu Sun <sshiyu(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 03 Feb 2021 02:53:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Fix 4BA for non SPI masters
by Nick Vaccaro (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Edward O'Callaghan. Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50245 ) Change subject: flashrom.c: Fix 4BA for non SPI masters ...................................................................... Patch Set 1: (1 comment) This change is ready for review. Patchset: PS1: > thanks for the review however this was untested see TEST= line ;) hence no reviewers added. […] Understood, I was asked to review this CL in the bug. Please add me as a reviewer when ready. Thanks ! -- To view, visit https://review.coreboot.org/c/flashrom/+/50245 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieb59d9d69d5b4f5c1e5aac424898e94c9fe3790b Gerrit-Change-Number: 50245 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 03 Feb 2021 02:20:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Fix 4BA for non SPI masters
by Nick Vaccaro (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Edward O'Callaghan. Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50245 ) Change subject: flashrom.c: Fix 4BA for non SPI masters ...................................................................... Patch Set 1: Code-Review-1 (1 comment) Patchset: PS1: I still see failure with this CL. -- To view, visit https://review.coreboot.org/c/flashrom/+/50245 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieb59d9d69d5b4f5c1e5aac424898e94c9fe3790b Gerrit-Change-Number: 50245 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 03 Feb 2021 01:05:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: Convert anon union to anon struct
by Sam McNally (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, Angel Pons. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50246 ) Change subject: programmer.h: Convert anon union to anon struct ...................................................................... Patch Set 2: (1 comment) File programmer.h: https://review.coreboot.org/c/flashrom/+/50246/comment/82d9cab0_df91fec7 PS2, Line 752: struct { This will need corresponding updates to ensure unused masters are zeroed out. -- To view, visit https://review.coreboot.org/c/flashrom/+/50246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242 Gerrit-Change-Number: 50246 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 03 Feb 2021 01:03:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: Convert anon union to anon struct
by Alan Green (Code Review) 03 Feb '21

03 Feb '21

Attention is currently required from: Sam McNally, Stefan Reinauer, Edward O'Callaghan, Angel Pons. Alan Green has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50246 ) Change subject: programmer.h: Convert anon union to anon struct ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/50246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242 Gerrit-Change-Number: 50246 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 03 Feb 2021 00:36:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: Convert anon union to anon struct
by Edward O'Callaghan (Code Review) 03 Feb '21

03 Feb '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/50246 to look at the new patch set (#2). Change subject: programmer.h: Convert anon union to anon struct ...................................................................... programmer.h: Convert anon union to anon struct Convert the anon union of registered masters in the mst field of the flashctx to a anon struct. If we are going to dereference a pointer there in an undefined way we should crash and not plow ahead with invalid memory. i.e., As a minimal example, ``` struct idk { struct { int *a; int *b; }; }; int main() { struct idk foo = {0}; int i = 12; foo.a = &i; printf("%d\n", *foo.b); return 0; } ``` The user of the registered_masters type is therefore responsible for querying the buses_supported field before attempting to dereference a ptr field in the anon struct. BUG=? TEST=<needed> Change-Id: I576967a8599b923c902e39f177f39146291cc242 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M programmer.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/50246/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/50246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242 Gerrit-Change-Number: 50246 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: programmer.h: Convert anon union to anon struct
by Edward O'Callaghan (Code Review) 03 Feb '21

03 Feb '21

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/50246 ) Change subject: programmer.h: Convert anon union to anon struct ...................................................................... programmer.h: Convert anon union to anon struct Convert the anon union of registered masters in the mst field of the flashctx to a anon struct. If we are going to dereference a pointer there in an undefined way we should crash and not plow ahead with invalid memory. BUG=? TEST=<needed> Change-Id: I576967a8599b923c902e39f177f39146291cc242 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M programmer.h 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/50246/1 diff --git a/programmer.h b/programmer.h index b575a72..8cb8690 100644 --- a/programmer.h +++ b/programmer.h @@ -749,7 +749,7 @@ int register_par_master(const struct par_master *mst, const enum chipbustype buses); struct registered_master { enum chipbustype buses_supported; - union { + struct { struct par_master par; struct spi_master spi; struct opaque_master opaque; -- To view, visit https://review.coreboot.org/c/flashrom/+/50246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242 Gerrit-Change-Number: 50246 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2021