- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: rayer_spi.c: Implement `set_sck_set_mosi` operation
by Angel Pons (Code Review) 10 Jun '21

10 Jun '21

Angel Pons has restored this change. ( https://review.coreboot.org/c/flashrom/+/49265 ) Change subject: rayer_spi.c: Implement `set_sck_set_mosi` operation ...................................................................... Restored -- To view, visit https://review.coreboot.org/c/flashrom/+/49265 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieb05a35967685c2444d064cb61cec98e2a00f590 Gerrit-Change-Number: 49265 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-MessageType: restore

1 0

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Nico Huber (Code Review) 10 Jun '21

10 Jun '21

Attention is currently required from: Xiang W, Edward O'Callaghan, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 26: (1 comment) File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/93f32fe3_53aa1fb6 PS26, Line 121: programmer_delay(master->half_period); : bitbang_spi_set_sck_set_mosi(master, !cpol, (val >> i) & 1); : programmer_delay(master->half_period); : bitbang_spi_set_sck(master, cpol); This does not look right and is probably what led to the follow up commit. With cpha=0, the receiver is supposed to sample the data at the first edge. So the sequence should be: * set mosi * delay * first edge * delay * second edge * (followed without delay by the next mosi setting) -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 26 Gerrit-Owner: Xiang W <wxjstz(a)126.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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 10 Jun 2021 20:21:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: bitbang-spi.c: support clock polarity and phase
by Nico Huber (Code Review) 10 Jun '21

10 Jun '21

Attention is currently required from: Xiang W, Edward O'Callaghan, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49255 ) Change subject: bitbang-spi.c: support clock polarity and phase ...................................................................... Patch Set 26: (8 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/49255/comment/fba2547c_d605eaa9 PS1, Line 8: > Sorry I misunderstood. The change does two things: 1. implements a better mode 0, 2. adds the option of other modes. The former is most welcome. I also like to get the whole patch merged, under one condition: The commit message has to give a reasonable incentive to add the optional modes. For instance, what chip needs it to be supported? and what other changes are in the queue to make flashrom support it? Currently all the common code assumes that the programmer either implements mode 0 or 3 and that all flash chips support both. If we need to support other chips, the common code would have to change too, right? Commit Message: https://review.coreboot.org/c/flashrom/+/49255/comment/af59a92c_a15486b2 PS26, Line 11: may not be This "may not be" is rather vague. Do we know a real-world use case? Patchset: PS26: Sorry, I'm very late to this review. There is still the early question about the reason for the change. Basically, do we have a real-world use case? File bitbang_spi.c: https://review.coreboot.org/c/flashrom/+/49255/comment/147d1383_28ee3080 PS26, Line 171: _set_mosi Removing the spurious MOSI settings seems fine, but it should be mentioned in the commit message (or happen in a preceding commit). https://review.coreboot.org/c/flashrom/+/49255/comment/cad0abf5_d7823283 PS26, Line 34: data transmission Technically, it's the data sampling (or capturing, as Wikipedia puts it) that happens, not the transmission. https://review.coreboot.org/c/flashrom/+/49255/comment/b4c52ed0_ba3e8580 PS26, Line 96: bitbang_spi_set_sck(master, !cpol); The first clock change always needs to happen with a delay after asserting CS#. Both cpha cases should start with a delay. https://review.coreboot.org/c/flashrom/+/49255/comment/c2ff835a_2f5e6f13 PS26, Line 184: if (strlen(mode)) { So an empty string is not reported as error? https://review.coreboot.org/c/flashrom/+/49255/comment/e5719916_50b36152 PS26, Line 187: mode == endptr This does not account for trailing garbage. It's better to check for the null termination, e.g. if (*mode == '\0' || *endptr != '\0' || v > 3 || v < 0) { (this includes checking for an empty string) -- To view, visit https://review.coreboot.org/c/flashrom/+/49255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04c1dfe132d756119229b27c3cd611d1be1abc8d Gerrit-Change-Number: 49255 Gerrit-PatchSet: 26 Gerrit-Owner: Xiang W <wxjstz(a)126.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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-Attention: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 10 Jun 2021 20:13:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Xiang W <wxjstz(a)126.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer_table: move each entry to the associated programmer source
by Angel Pons (Code Review) 10 Jun '21

10 Jun '21

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52946 ) Change subject: programmer_table: move each entry to the associated programmer source ...................................................................... Patch Set 12: (1 comment) File programmer.h: https://review.coreboot.org/c/flashrom/+/52946/comment/73431de3_961b68e3 PS4, Line 402: #if CONFIG_DUMMY == 1 > Please just drop all these `#if`. The only difference it makes is > whether we get a compile-time or link-time error if something goes > wrong. I don't think it matters or even justifies the clutter. > > Mentioning the file name is also a bit redundant. Maybe just a single > comment /* programmer drivers */ ? > > Doesn't have to be in this patch, though CB:55387 > Thank you so much! Your long message is very helpful, I think now I actually understand: extern declaration by itself is fine, as long as all the usages are guarded. I made CB:55386 on coreboot earlier today. It introduces a helper function to transform a compile-time error into a link-time error, which allows dropping some preprocessor usage in code. I just forced build errors by not enabling `DEBUG_BOOT_STATE` and manually inverting the condition in src/lib/hardwaremain.c in order to show you the difference between a compile-time error and a link-time error. Compile-time error excerpt: src/lib/hardwaremain.c: In function 'bs_call_callbacks': src/lib/hardwaremain.c:279:15: error: 'struct boot_state_callback' has no member named 'location' bscb, bscb->location); The `location` member is inside a text block guarded by a conditional preprocessor directive. Since the `CONFIG(DEBUG_BOOT_STATE)` condition is false, the preprocessor does not pass the contents of this text block to the compiler, and the compiler does not "see" the `location` member in the struct definition. When it reaches the `bscb->location` part, the compiler complains about the non-existent `location` member. Link-time error excerpt: coreboot-builds/HP_280_G2/generated/ramstage.o: in function `bscb_location': src/include/bootstate.h:114: undefined reference to `_dead_code_assertion_failed' The `dead_code_t` macro inside the `bscb_location` function evaluates to some expression that calls the `_dead_code_assertion_failed` function, which is intentionally never defined (only declared). Compilation succeeds: the source code can be translated into a `hardwaremain.o` object file, which contains external definitions and references for linking. However, when the linker tries to resolve the external references, it cannot find a definition for the `_dead_code_assertion_failed` function, and bails out. The advantage of link-time errors over compile-time errors (i.e. what CB:55386 leverages) is that the compiler can automatically optimise out the code that would cause a link-time error. CB:55386 works because `bscb_location` is never reached when the `CONFIG(DEBUG_BOOT_STATE)` condition is false, and the compiler can thus optimise `bscb_location` away along with the `_dead_code_assertion_failed` function call. -- To view, visit https://review.coreboot.org/c/flashrom/+/52946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d02bd789f0299e936eb86819b3b15b5ea2bb921 Gerrit-Change-Number: 52946 Gerrit-PatchSet: 12 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Comment-Date: Thu, 10 Jun 2021 14:14:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: remove compile guard from programmer drivers
by Angel Pons (Code Review) 10 Jun '21

10 Jun '21

Attention is currently required from: Nico Huber, Thomas Heijligen, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55387 ) Change subject: programmer.h: remove compile guard from programmer drivers ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55387 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I01e6a785269c3e0bd648eeaee217a7a855ab0853 Gerrit-Change-Number: 55387 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 10 Jun 2021 13:27:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips.c: Mark MT25QL256 as tested
by Nico Huber (Code Review) 10 Jun '21

10 Jun '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/50720 ) Change subject: flashchips.c: Mark MT25QL256 as tested ...................................................................... flashchips.c: Mark MT25QL256 as tested As mentioned in mail from Bernd.Stoeferle(a)elbitsystems-de.com on 22.12.2020. Change-Id: Ie49332333f49a40f7bd8f3b5e42a8e2ad6995618 Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/50720 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M flashchips.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index ef5ea5e..950f09b 100644 --- a/flashchips.c +++ b/flashchips.c @@ -11644,7 +11644,7 @@ /* supports SFDP */ /* OTP: 64B total; read 0x4B, write 0x42 */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/50720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie49332333f49a40f7bd8f3b5e42a8e2ad6995618 Gerrit-Change-Number: 50720 Gerrit-PatchSet: 4 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: buspirate_spi.c: Separate shutdown from failed init cleanup
by Angel Pons (Code Review) 10 Jun '21

10 Jun '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52877 ) Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52877 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0 Gerrit-Change-Number: 52877 Gerrit-PatchSet: 4 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 10 Jun 2021 13:25:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: buspirate_spi.c: Separate shutdown from failed init cleanup
by Angel Pons (Code Review) 10 Jun '21

10 Jun '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52877 ) Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 4: Code-Review+1 (1 comment) File buspirate_spi.c: https://review.coreboot.org/c/flashrom/+/52877/comment/3038fc96_58ceddfa PS4, Line 620: ret = 1; For another patch: We could get rid of these `ret = 1;` statements and just use the existing non-zero value. -- To view, visit https://review.coreboot.org/c/flashrom/+/52877 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0 Gerrit-Change-Number: 52877 Gerrit-PatchSet: 4 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 10 Jun 2021 13:25:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: remove compile guard from programmer drivers
by Thomas Heijligen (Code Review) 10 Jun '21

10 Jun '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/55387 ) Change subject: programmer.h: remove compile guard from programmer drivers ...................................................................... programmer.h: remove compile guard from programmer drivers The definition of external structs musst not be guarded. See discussion under review.coreboot.org/52946. Change-Id: I01e6a785269c3e0bd648eeaee217a7a855ab0853 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M programmer.h 1 file changed, 43 insertions(+), 187 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/55387/1 diff --git a/programmer.h b/programmer.h index be6010d..9d1a9ad 100644 --- a/programmer.h +++ b/programmer.h @@ -57,6 +57,47 @@ extern const struct programmer_entry *const programmer_table[]; extern const size_t programmer_table_size; +/* programmer drivers */ +extern const struct programmer_entry programmer_internal; +extern const struct programmer_entry programmer_dummy; +extern const struct programmer_entry programmer_nic3com; +extern const struct programmer_entry programmer_gfxnvidia; +extern const struct programmer_entry programmer_raiden_debug_spi; +extern const struct programmer_entry programmer_drkaiser; +extern const struct programmer_entry programmer_nicrealtek; +extern const struct programmer_entry programmer_nicnatsemi; +extern const struct programmer_entry programmer_nicintel; +extern const struct programmer_entry programmer_nicintel_spi; +extern const struct programmer_entry programmer_nicintel_eeprom; +extern const struct programmer_entry programmer_ogp_spi; +extern const struct programmer_entry programmer_satamv; +extern const struct programmer_entry programmer_satasii; +extern const struct programmer_entry programmer_atahpt; +extern const struct programmer_entry programmer_atavia; +extern const struct programmer_entry programmer_atapromise; +extern const struct programmer_entry programmer_it8212; +extern const struct programmer_entry programmer_ft2232_spi; +extern const struct programmer_entry programmer_usbblaster_spi; +extern const struct programmer_entry programmer_mstarddc_spi; +extern const struct programmer_entry programmer_pickit2_spi; +extern const struct programmer_entry programmer_stlinkv3_spi; +extern const struct programmer_entry programmer_rayer_spi; +extern const struct programmer_entry programmer_pony_spi; +extern const struct programmer_entry programmer_buspirate_spi; +extern const struct programmer_entry programmer_linux_mtd; +extern const struct programmer_entry programmer_linux_spi; +extern const struct programmer_entry programmer_dediprog; +extern const struct programmer_entry programmer_developerbox; +extern const struct programmer_entry programmer_ch341a_spi; +extern const struct programmer_entry programmer_digilent_spi; +extern const struct programmer_entry programmer_ene_lpc; +extern const struct programmer_entry programmer_jlink_spi; +extern const struct programmer_entry programmer_ni845x_spi; +extern const struct programmer_entry programmer_mec1308; +extern const struct programmer_entry programmer_serprog; +extern const struct programmer_entry programmer_lspcon_i2c_spi; +extern const struct programmer_entry programmer_realtek_mst_i2c_spi; + int programmer_init(const struct programmer_entry *prog, const char *param); int programmer_shutdown(void); @@ -241,7 +282,6 @@ void probe_superio(void); int register_superio(struct superio s); extern enum chipbustype internal_buses_supported; -extern const struct programmer_entry programmer_internal; #endif /* hwaccess.c */ @@ -277,178 +317,9 @@ void rmmio_valw(void *addr); void rmmio_vall(void *addr); -/* dummyflasher.c */ -#if CONFIG_DUMMY == 1 -extern const struct programmer_entry programmer_dummy; -#endif - -/* nic3com.c */ -#if CONFIG_NIC3COM == 1 -extern const struct programmer_entry programmer_nic3com; -#endif - -/* gfxnvidia.c */ -#if CONFIG_GFXNVIDIA == 1 -extern const struct programmer_entry programmer_gfxnvidia; -#endif - -/* raiden_debug_spi.c */ -#if CONFIG_RAIDEN_DEBUG_SPI == 1 -extern const struct programmer_entry programmer_raiden_debug_spi; -#endif - -/* drkaiser.c */ -#if CONFIG_DRKAISER == 1 -extern const struct programmer_entry programmer_drkaiser; -#endif - -/* nicrealtek.c */ -#if CONFIG_NICREALTEK == 1 -extern const struct programmer_entry programmer_nicrealtek; -#endif - -/* nicnatsemi.c */ -#if CONFIG_NICNATSEMI == 1 -extern const struct programmer_entry programmer_nicnatsemi; -#endif - -/* nicintel.c */ -#if CONFIG_NICINTEL == 1 -extern const struct programmer_entry programmer_nicintel; -#endif - -/* nicintel_spi.c */ -#if CONFIG_NICINTEL_SPI == 1 -extern const struct programmer_entry programmer_nicintel_spi; -#endif - -/* nicintel_eeprom.c */ -#if CONFIG_NICINTEL_EEPROM == 1 -extern const struct programmer_entry programmer_nicintel_eeprom; -#endif - -/* ogp_spi.c */ -#if CONFIG_OGP_SPI == 1 -extern const struct programmer_entry programmer_ogp_spi; -#endif - -/* satamv.c */ -#if CONFIG_SATAMV == 1 -extern const struct programmer_entry programmer_satamv; -#endif - -/* satasii.c */ -#if CONFIG_SATASII == 1 -extern const struct programmer_entry programmer_satasii; -#endif - -/* atahpt.c */ -#if CONFIG_ATAHPT == 1 -extern const struct programmer_entry programmer_atahpt; -#endif - -/* atavia.c */ -#if CONFIG_ATAVIA == 1 -extern const struct programmer_entry programmer_atavia; -#endif - -/* atapromise.c */ -#if CONFIG_ATAPROMISE == 1 -extern const struct programmer_entry programmer_atapromise; -#endif - -/* it8212.c */ -#if CONFIG_IT8212 == 1 -extern const struct programmer_entry programmer_it8212; -#endif - -/* ft2232_spi.c */ -#if CONFIG_FT2232_SPI == 1 -extern const struct programmer_entry programmer_ft2232_spi; -#endif - -/* usbblaster_spi.c */ -#if CONFIG_USBBLASTER_SPI == 1 -extern const struct programmer_entry programmer_usbblaster_spi; -#endif - -/* mstarddc_spi.c */ -#if CONFIG_MSTARDDC_SPI == 1 -extern const struct programmer_entry programmer_mstarddc_spi; -#endif - -/* pickit2_spi.c */ -#if CONFIG_PICKIT2_SPI == 1 -extern const struct programmer_entry programmer_pickit2_spi; -#endif - -/* stlinkv3_spi.c */ -#if CONFIG_STLINKV3_SPI == 1 -extern const struct programmer_entry programmer_stlinkv3_spi; -#endif - -/* rayer_spi.c */ -#if CONFIG_RAYER_SPI == 1 -extern const struct programmer_entry programmer_rayer_spi; -#endif - -/* pony_spi.c */ -#if CONFIG_PONY_SPI == 1 -extern const struct programmer_entry programmer_pony_spi; -#endif - /* bitbang_spi.c */ int register_spi_bitbang_master(const struct bitbang_spi_master *master, void *spi_data); -/* buspirate_spi.c */ -#if CONFIG_BUSPIRATE_SPI == 1 -extern const struct programmer_entry programmer_buspirate_spi; -#endif - -/* linux_mtd.c */ -#if CONFIG_LINUX_MTD == 1 -extern const struct programmer_entry programmer_linux_mtd; -#endif - -/* linux_spi.c */ -#if CONFIG_LINUX_SPI == 1 -extern const struct programmer_entry programmer_linux_spi; -#endif - -/* dediprog.c */ -#if CONFIG_DEDIPROG == 1 -extern const struct programmer_entry programmer_dediprog; -#endif - -/* developerbox_spi.c */ -#if CONFIG_DEVELOPERBOX_SPI == 1 -extern const struct programmer_entry programmer_developerbox; -#endif - -/* ch341a_spi.c */ -#if CONFIG_CH341A_SPI == 1 -extern const struct programmer_entry programmer_ch341a_spi; -#endif - -/* digilent_spi.c */ -#if CONFIG_DIGILENT_SPI == 1 -extern const struct programmer_entry programmer_digilent_spi; -#endif - -/* ene_lpc.c */ -#if CONFIG_ENE_LPC == 1 -extern const struct programmer_entry programmer_ene_lpc; -#endif - -/* jlink_spi.c */ -#if CONFIG_JLINK_SPI == 1 -extern const struct programmer_entry programmer_jlink_spi; -#endif - -/* ni845x_spi.c */ -#if CONFIG_NI845X_SPI == 1 -extern const struct programmer_entry programmer_ni845x_spi; -#endif /* flashrom.c */ struct decode_sizes { @@ -553,10 +424,7 @@ /* mcp6x_spi.c */ int mcp6x_spi_init(int want_spi); -/* mec1308.c */ -#if CONFIG_MEC1308 == 1 -extern const struct programmer_entry programmer_mec1308; -#endif + /* sb600spi.c */ int sb600_probe_spi(struct pci_dev *dev); @@ -611,10 +479,7 @@ extern int registered_master_count; int register_master(const struct registered_master *mst); -/* serprog.c */ -#if CONFIG_SERPROG == 1 -extern const struct programmer_entry programmer_serprog; -#endif + /* serial.c */ #if IS_WINDOWS @@ -682,14 +547,5 @@ struct libusb_device_handle *usb_dev_get_by_vid_pid_number( struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); -/* lspcon_i2c_spi.c */ -#if CONFIG_LSPCON_I2C_SPI == 1 -extern const struct programmer_entry programmer_lspcon_i2c_spi; -#endif - -/* realtek_mst_i2c_spi.c */ -#if CONFIG_REALTEK_MST_I2C_SPI == 1 -extern const struct programmer_entry programmer_realtek_mst_i2c_spi; -#endif #endif /* !__PROGRAMMER_H__ */ -- To view, visit https://review.coreboot.org/c/flashrom/+/55387 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I01e6a785269c3e0bd648eeaee217a7a855ab0853 Gerrit-Change-Number: 55387 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: nicrealtek.c: Refactor singleton states into reentrant pattern
by Nico Huber (Code Review) 10 Jun '21

10 Jun '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/55108 ) Change subject: nicrealtek.c: Refactor singleton states into reentrant pattern ...................................................................... nicrealtek.c: Refactor singleton states into reentrant pattern Move global singleton states into a struct and store within the par_master data field for the life-time of the driver. This is one of the steps on the way to move par_master data memory management behind the initialisation API, for more context see other patches under the same topic "register_master_api". BUG=b:185191942 TEST=builds Change-Id: If0cb0fefb53b2c6bb65a85f4c8dc6f323954dd0c Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/55108 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M nicrealtek.c 1 file changed, 31 insertions(+), 10 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/nicrealtek.c b/nicrealtek.c index 53d3c3d..81d305b 100644 --- a/nicrealtek.c +++ b/nicrealtek.c @@ -24,8 +24,11 @@ #define PCI_VENDOR_ID_REALTEK 0x10ec #define PCI_VENDOR_ID_SMC1211 0x1113 -static uint32_t io_base_addr = 0; -static int bios_rom_addr, bios_rom_data; +struct nicrealtek_data { + uint32_t io_base_addr; + int bios_rom_addr; + int bios_rom_data; +}; static const struct dev_entry nics_realtek[] = { {0x10ec, 0x8139, OK, "Realtek", "RTL8139/8139C/8139C+"}, @@ -37,38 +40,41 @@ static void nicrealtek_chip_writeb(const struct flashctx *flash, uint8_t val, chipaddr addr) { + struct nicrealtek_data *data = flash->mst->par.data; + /* Output addr and data, set WE to 0, set OE to 1, set CS to 0, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x0A0000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); /* Output addr and data, set WE to 1, set OE to 1, set CS to 1, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x1E0000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); } static uint8_t nicrealtek_chip_readb(const struct flashctx *flash, const chipaddr addr) { + struct nicrealtek_data *data = flash->mst->par.data; uint8_t val; /* FIXME: Can we skip reading the old data and simply use 0? */ /* Read old data. */ - val = INB(io_base_addr + bios_rom_data); + val = INB(data->io_base_addr + data->bios_rom_data); /* Output new addr and old data, set WE to 1, set OE to 0, set CS to 0, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x060000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); /* Read new data. */ - val = INB(io_base_addr + bios_rom_data); + val = INB(data->io_base_addr + data->bios_rom_data); /* Output addr and new data, set WE to 1, set OE to 1, set CS to 1, * enable software access. */ OUTL(((uint32_t)addr & 0x01FFFF) | 0x1E0000 | (val << 24), - io_base_addr + bios_rom_addr); + data->io_base_addr + data->bios_rom_addr); return val; } @@ -87,12 +93,16 @@ static int nicrealtek_shutdown(void *data) { /* FIXME: We forgot to disable software access again. */ + free(data); return 0; } static int nicrealtek_init(void) { struct pci_dev *dev = NULL; + uint32_t io_base_addr = 0; + int bios_rom_addr; + int bios_rom_data; if (rget_io_perms()) return 1; @@ -119,10 +129,21 @@ break; } - if (register_shutdown(nicrealtek_shutdown, NULL)) + struct nicrealtek_data *data = calloc(1, sizeof(*data)); + if (!data) { + msg_perr("Unable to allocate space for PAR master data\n"); return 1; + } + data->io_base_addr = io_base_addr; + data->bios_rom_addr = bios_rom_addr; + data->bios_rom_data = bios_rom_data; - register_par_master(&par_master_nicrealtek, BUS_PARALLEL, NULL); + if (register_shutdown(nicrealtek_shutdown, data)) { + free(data); + return 1; + } + + register_par_master(&par_master_nicrealtek, BUS_PARALLEL, data); return 0; } -- To view, visit https://review.coreboot.org/c/flashrom/+/55108 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If0cb0fefb53b2c6bb65a85f4c8dc6f323954dd0c Gerrit-Change-Number: 55108 Gerrit-PatchSet: 3 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit