February 2023 - flashrom-gerrit

[S] Change in flashrom[master]: flashchips.c: Add reg_bits for W25Q256JW_DTR
by Subrata Banik (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Nikolai Artemiev, Kapil Porwal, Edward O'Callaghan, Anastasia Klimchuk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70549 ) Change subject: flashchips.c: Add reg_bits for W25Q256JW_DTR ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70549 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367 Gerrit-Change-Number: 70549 Gerrit-PatchSet: 6 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 08 Feb 2023 13:07:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips.c: Add reg_bits for W25Q256JW_DTR
by Kapil Porwal (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Nikolai Artemiev, Subrata Banik, Edward O'Callaghan, Anastasia Klimchuk. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70549 ) Change subject: flashchips.c: Add reg_bits for W25Q256JW_DTR ...................................................................... Patch Set 5: (1 comment) This change is ready for review. Patchset: PS5: I have verified this CL on google/rex. Please review. -- To view, visit https://review.coreboot.org/c/flashrom/+/70549 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8ac23f706d4293a7d7d11ad6b2f62526fb075367 Gerrit-Change-Number: 70549 Gerrit-PatchSet: 5 Gerrit-Owner: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 08 Feb 2023 12:52:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dummyflasher: use new API to register shutdown function
by Alexander Goncharov (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72408 ) Change subject: dummyflasher: use new API to register shutdown function ...................................................................... Patch Set 3: (2 comments) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/72408/comment/f356e0ba_4b39433e PS2, Line 79: uint8_t refs_cnt; > Can you add a comment on this: to explain that dummyflasher can register multiple masters and they a […] Done https://review.coreboot.org/c/flashrom/+/72408/comment/a596b066_5a29ee0f PS2, Line 1416: data->refs_cnt += ((dummy_buses_supported & BUS_PROG) != BUS_NONE); : data->refs_cnt += ((dummy_buses_supported & BUS_NONSPI) != BUS_NONE); : data->refs_cnt += ((dummy_buses_supported & BUS_SPI) != BUS_NONE); : : #if 0 : data->refs_cnt += (dummy_buses_supported & BUS_PROG) ? 1 : 0; : data->refs_cnt += (dummy_buses_supported & BUS_NONSPI) ? 1 : 0; : data->refs_cnt += (dummy_buses_supported & BUS_SPI) ? 1 : 0; : #endif > Another thing could be to add `&& !ret` to every condition. […] Your last idea is exactly what we need. Thank you! Here we go :) -- To view, visit https://review.coreboot.org/c/flashrom/+/72408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Gerrit-Change-Number: 72408 Gerrit-PatchSet: 3 Gerrit-Owner: 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 08 Feb 2023 08:37:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dummyflasher: use new API to register shutdown function
by Alexander Goncharov (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Edward O'Callaghan, Alexander Goncharov. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72408 to look at the new patch set (#3). Change subject: dummyflasher: use new API to register shutdown function ...................................................................... dummyflasher: use new API to register shutdown function This allows masters to register shutdown function in *_master struct, which means there is no need to call register_shutdown in init function, since this call is now a part of register_*_master. A dummy programmer can register masters for multiple buses that share a programmer's data (a pointer to struct emu_data) with each other. To avoid unexpected memory freeing by shutdown function, we need to keep track of how many buses are using the shared resource. Use the reference counting technique to achieve this. TEST=ninja test Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M dummyflasher.c 1 file changed, 43 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/72408/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/72408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Gerrit-Change-Number: 72408 Gerrit-PatchSet: 3 Gerrit-Owner: 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: internal: Move parallel logic into internal_par implementation
by Anastasia Klimchuk (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71834 ) Change subject: internal: Move parallel logic into internal_par implementation ...................................................................... Patch Set 2: Code-Review+2 (1 comment) File internal_par.c: https://review.coreboot.org/c/flashrom/+/71834/comment/d242db81_c9d35e53 PS1, Line 16: > Actually it is a independent programmer and probably _should_ be registered just by itself. Oh wow I didn't realise this :) -- To view, visit https://review.coreboot.org/c/flashrom/+/71834 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idabeceb59a36680f5fbb45d3ee4bd5dbf837373b Gerrit-Change-Number: 71834 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 08 Feb 2023 02:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ni845x_spi: pass global state through func params
by Anastasia Klimchuk (Code Review) 08 Feb '23

08 Feb '23

Attention is currently required from: Miklós Márton, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72154 ) Change subject: ni845x_spi: pass global state through func params ...................................................................... Patch Set 3: (1 comment) File ni845x_spi.c: https://review.coreboot.org/c/flashrom/+/72154/comment/f08a21d4_0216e860 PS2, Line 233: set_io_voltage_mV > This second argument (`set_io_voltage_mV`) contains a pointer to `io_voltage_in_mV` global var. […] I see, that's a good finding! Then: you need to add one more paragraph in commit message. I think this fits into a commit (as you said "Instead of relying on global variables, pass and use their value or pointer to functions where possible") but better be explicitly mentioned in commit message. As an example: The usage of `io_voltage_in_mV` global var in `usb8452_spi_set_io_voltage` function is replaced with existing function argument `set_io_voltage_mV` since `set_io_voltage_mV` already contains the pointer to global var. Miklós please tell us if we are missing something! -- To view, visit https://review.coreboot.org/c/flashrom/+/72154 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5daeb0839a4cc18b82d38cc06eeba88a619bec61 Gerrit-Change-Number: 72154 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 08 Feb 2023 00:13:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: dummyflasher: use new API to register shutdown function
by Anastasia Klimchuk (Code Review) 07 Feb '23

07 Feb '23

Attention is currently required from: Edward O'Callaghan, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72408 ) Change subject: dummyflasher: use new API to register shutdown function ...................................................................... Patch Set 2: (1 comment) File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/72408/comment/55b16e3f_7cb702b1 PS2, Line 1416: data->refs_cnt += ((dummy_buses_supported & BUS_PROG) != BUS_NONE); : data->refs_cnt += ((dummy_buses_supported & BUS_NONSPI) != BUS_NONE); : data->refs_cnt += ((dummy_buses_supported & BUS_SPI) != BUS_NONE); : : #if 0 : data->refs_cnt += (dummy_buses_supported & BUS_PROG) ? 1 : 0; : data->refs_cnt += (dummy_buses_supported & BUS_NONSPI) ? 1 : 0; : data->refs_cnt += (dummy_buses_supported & BUS_SPI) ? 1 : 0; : #endif > Okay, I understand, thanks for explanation! […] Another thing could be to add `&& !ret` to every condition. Because even if one register fails -> init fails -> no need to try register remaining masters? -- To view, visit https://review.coreboot.org/c/flashrom/+/72408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea Gerrit-Change-Number: 72408 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 07 Feb 2023 23:45:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal.c: Factor out laptop alerts into helper func
by Angel Pons (Code Review) 07 Feb '23

07 Feb '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71578 ) Change subject: internal.c: Factor out laptop alerts into helper func ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca Gerrit-Change-Number: 71578 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 07 Feb 2023 15:14:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal.c: Factor out laptop alerts into helper func
by Alexander Goncharov (Code Review) 07 Feb '23

07 Feb '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71578 ) Change subject: internal.c: Factor out laptop alerts into helper func ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca Gerrit-Change-Number: 71578 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 07 Feb 2023 15:11:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: wbsio_spi.c: Drop unmaintained driver path
by Thomas Heijligen (Code Review) 07 Feb '23

07 Feb '23

Attention is currently required from: Edward O'Callaghan, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72828 ) Change subject: wbsio_spi.c: Drop unmaintained driver path ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > As mentioned in the ML, could this be encapsulated so that the functionality doesn't need to be drop […] CB:72864 -- To view, visit https://review.coreboot.org/c/flashrom/+/72828 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I25c4d02126fcf183662dec73a800cac3988a1773 Gerrit-Change-Number: 72828 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(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-CC: 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: Tue, 07 Feb 2023 14:55:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2023