April 2017 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[staging]: Move register decodes into enable_flash_ich_handle_gcs()
by Youness Alaoui (Code Review) 24 Apr '17

24 Apr '17

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18882 ) Change subject: Move register decodes into enable_flash_ich_handle_gcs() ...................................................................... Patch Set 2: Code-Review+1 Ignore previous comment, I figured out (5 seconds after posting) where to look for the info, and the top_swap is indeed bit 0 only of 0x3414 register. The previous code was wrong as you mentioned in the commit log. -- To view, visit https://review.coreboot.org/18882 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40addec98cb6840763adad30f9d0e27dadce6d1e Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: Move register decodes into enable_flash_ich_handle_gcs()
by Youness Alaoui (Code Review) 24 Apr '17

24 Apr '17

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18882 ) Change subject: Move register decodes into enable_flash_ich_handle_gcs() ...................................................................... Patch Set 2: The only comment I'd have is whether checking only bit 0 of top_swap is correct. You changed top_swap from being `mmio_readb(rcrb + 0x3414)` into `mmio_readb(rcrb + 0x3414) & 1`. While that makes it valid bool, it's not necessarily a valid cast of uint8_t into a bool. I don't have the spec (wouldn't know where/what to look for) to check if the bit 0 of rcrb+0x3414 is the one that needs to be used for top_swap, but a better cast would have been to do : > top_swap = mmio_readb(rcrb + 0x3414) != 0; The rest looks good, but please verify the above before merging. -- To view, visit https://review.coreboot.org/18882 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40addec98cb6840763adad30f9d0e27dadce6d1e Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: hwaccess: Constify `addr` argument of MMIO read functions
by Youness Alaoui (Code Review) 24 Apr '17

24 Apr '17

Youness Alaoui has posted comments on this change. ( https://review.coreboot.org/18881 ) Change subject: hwaccess: Constify `addr` argument of MMIO read functions ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/18881 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1419241a8332b74103b3921df2c615bdf91346a7 Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philippe Mathieu-Daudé <philippe.mathieu.daude(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: flashchips: Add untested Winbond W25Q128.W
by Nico Huber (Code Review) 24 Apr '17

24 Apr '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/19436 ) Change subject: flashchips: Add untested Winbond W25Q128.W ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/#/c/19436/1/flashchips.c File flashchips.c: PS1, Line 14828: W25Q128.W > FWIW, the W25Q128FV has the same model ID when operating in QPI mode (and a Same for the smaller chips (according to comments in flashchips.h). And we don't seem to worry about them. -- To view, visit https://review.coreboot.org/19436 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: flashchips: Add untested Winbond W25Q128.W
by build bot (Jenkins) (Code Review) 24 Apr '17

24 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19436 ) Change subject: flashchips: Add untested Winbond W25Q128.W ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/245/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/257/ : SUCCESS -- To view, visit https://review.coreboot.org/19436 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: flashchips: Add untested Winbond W25Q128.W
by Nico Huber (Code Review) 24 Apr '17

24 Apr '17

Hello David Hendricks, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/19436 to look at the new patch set (#2). Change subject: flashchips: Add untested Winbond W25Q128.W ...................................................................... flashchips: Add untested Winbond W25Q128.W Only difference to its sibling W25Q128.V seems to be the supply voltage. Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flashchips.c M flashchips.h 2 files changed, 41 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/19436/2 -- To view, visit https://review.coreboot.org/19436 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Gerrit-PatchSet: 2 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins)

1 0

Change in flashrom[staging]: flashchips: Add untested Winbond W25Q128.W
by David Hendricks (Code Review) 24 Apr '17

24 Apr '17

David Hendricks has posted comments on this change. ( https://review.coreboot.org/19436 ) Change subject: flashchips: Add untested Winbond W25Q128.W ...................................................................... Patch Set 1: Code-Review+1 (1 comment) https://review.coreboot.org/#/c/19436/1/flashchips.c File flashchips.c: PS1, Line 14828: W25Q128.W FWIW, the W25Q128FV has the same model ID when operating in QPI mode (and a different model when in normal mode). I'm not sure if it's worth worrying about, though. -- To view, visit https://review.coreboot.org/19436 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

Change in flashrom[staging]: flashchips: Add untested Winbond W25Q128.W
by build bot (Jenkins) (Code Review) 24 Apr '17

24 Apr '17

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/19436 ) Change subject: flashchips: Add untested Winbond W25Q128.W ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/244/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/256/ : SUCCESS -- To view, visit https://review.coreboot.org/19436 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: No

1 0

Change in flashrom[staging]: flashchips: Add untested Winbond W25Q128.W
by Nico Huber (Code Review) 24 Apr '17

24 Apr '17

Nico Huber has uploaded a new change for review. ( https://review.coreboot.org/19436 ) Change subject: flashchips: Add untested Winbond W25Q128.W ...................................................................... flashchips: Add untested Winbond W25Q128.W Only difference to its sibling W25Q128.V seems to be the supply voltage. Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Signed-off-by: Nico Huber <nico.h(a)gmx.de> --- M flashchips.c 1 file changed, 40 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/19436/1 diff --git a/flashchips.c b/flashchips.c index 40b6b8e..7ef361b 100644 --- a/flashchips.c +++ b/flashchips.c @@ -14825,6 +14825,46 @@ { .vendor = "Winbond", + .name = "W25Q128.W", + .bustype = BUS_SPI, + .manufacture_id = WINBOND_NEX_ID, + .model_id = WINBOND_NEX_W25Q128_W, + .total_size = 16384, + .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, 4096} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 512} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_plain, /* TODO: improve */ + .unlock = spi_disable_blockprotect, + .write = spi_chip_write_256, + .read = spi_chip_read, + .voltage = {1650, 1950}, + }, + + { + .vendor = "Winbond", .name = "W25X10", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, -- To view, visit https://review.coreboot.org/19436 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I34ce7f1bdd0d2fb1b065031e5a689bb16ffc70db Gerrit-PatchSet: 1 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>

1 0

Change in flashrom[staging]: cli_classic: Add option (-A, --noverify-all)
by Nico Huber (Code Review) 24 Apr '17

24 Apr '17

Nico Huber has posted comments on this change. ( https://review.coreboot.org/17950 ) Change subject: cli_classic: Add option (-A, --noverify-all) ...................................................................... Patch Set 7: (1 comment) https://review.coreboot.org/#/c/17950/7/cli_classic.c File cli_classic.c: PS7, Line 58: A > Maybe we should decide that not every little configuration bit needs to be I'd be fine with `-N`. IMHO, inverted logic isn't a bad thing here. It emphasizes that we do as much verification as possible by default. -- To view, visit https://review.coreboot.org/17950 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I40b5983f56d62821d17b827b88b73d1d41a30bd7 Gerrit-PatchSet: 7 Gerrit-Project: flashrom Gerrit-Branch: staging Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Stefan Tauner <stefan.tauner(a)gmx.at> Gerrit-Reviewer: build bot (Jenkins) Gerrit-HasComments: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2017