May 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Subrata Banik (Code Review) 23 May '22

23 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64540 ) Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 23 May 2022 12:43:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Edward O'Callaghan (Code Review) 23 May '22

23 May '22

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64540 ) Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... Patch Set 3: (2 comments) File ichspi.c: https://review.coreboot.org/c/flashrom/+/64540/comment/6d73b75d_b460fe51 PS2, Line 1349: static int ich_hwseq_read_status(const struct flashctx *flash, enum flash_reg reg_, uint8_t *value) > I think both functions should fail if reg_ is not SR1. Done https://review.coreboot.org/c/flashrom/+/64540/comment/5d99b916_e7b1bcf5 PS2, Line 1352: int len = 1; > I'd make len a constant in both functions. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 23 May 2022 12:39:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Edward O'Callaghan (Code Review) 23 May '22

23 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Nikolai Artemiev, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/64540 to look at the new patch set (#4). Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... ichspi.c: Implement read_write_status for wp The ichspi hwseq path has a opaque master specialisation that allows for reading and writing STATUS1 registers. Hook the callbacks with a implementation to allow for this so that writeprotect maybe supported though this path. Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M ichspi.c 1 file changed, 75 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/64540/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: spi25_statusreg.c: Allow opaque masters to hook spi_{rw}_register()
by Edward O'Callaghan (Code Review) 23 May '22

23 May '22

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64375 ) Change subject: spi25_statusreg.c: Allow opaque masters to hook spi_{rw}_register() ...................................................................... Patch Set 4: (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/64375/comment/7d79312b_17e4c9fc PS3, Line 35: } > nit: no need for curly braces here and below If I recall the correct way around, Angel Pons suggests to always use braces in flashrom style? -- To view, visit https://review.coreboot.org/c/flashrom/+/64375 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ab0d7f5f48338c8ecb118a69651c203fbc516ac Gerrit-Change-Number: 64375 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 23 May 2022 12:33:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Sergii Dmytruk (Code Review) 23 May '22

23 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64540 ) Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... Patch Set 3: (2 comments) File ichspi.c: https://review.coreboot.org/c/flashrom/+/64540/comment/36789db4_05e0f16c PS2, Line 1349: static int ich_hwseq_read_status(const struct flashctx *flash, enum flash_reg reg_, uint8_t *value) I think both functions should fail if reg_ is not SR1. https://review.coreboot.org/c/flashrom/+/64540/comment/b1d90ebf_a7d242f8 PS2, Line 1352: int len = 1; I'd make len a constant in both functions. It's used 15 lines below and one has to trace the code to understand that it's never updated. -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 23 May 2022 12:13:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg.c: Allow opaque masters to hook spi_{rw}_register()
by Sergii Dmytruk (Code Review) 23 May '22

23 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64375 ) Change subject: spi25_statusreg.c: Allow opaque masters to hook spi_{rw}_register() ...................................................................... Patch Set 4: (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/64375/comment/ba5e6e08_469938b1 PS3, Line 35: } nit: no need for curly braces here and below -- To view, visit https://review.coreboot.org/c/flashrom/+/64375 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ab0d7f5f48338c8ecb118a69651c203fbc516ac Gerrit-Change-Number: 64375 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 23 May 2022 12:13:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Subrata Banik (Code Review) 23 May '22

23 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64540 ) Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... Patch Set 3: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/64540/comment/c772e5df_bf03d8e8 PS1, Line 122: FIXME(quasisec): Port to new macros? - Control bits > Thanks Subrata, hopefully the updated patch addresses the issue? Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 23 May 2022 05:39:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Subrata Banik (Code Review) 23 May '22

23 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64540 ) Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 23 May 2022 05:38:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Edward O'Callaghan (Code Review) 23 May '22

23 May '22

Attention is currently required from: Subrata Banik, Nikolai Artemiev, Sergii Dmytruk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64540 ) Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... Patch Set 2: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/64540/comment/28e8c344_29c2ae91 PS1, Line 122: FIXME(quasisec): Port to new macros? - Control bits > > @Subrata, what did you suggest downstream to clean this up here? […] Thanks Subrata, hopefully the updated patch addresses the issue? -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 23 May 2022 02:53:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Edward O'Callaghan (Code Review) 23 May '22

23 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Hello build bot (Jenkins), Subrata Banik, Nikolai Artemiev, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/64540 to look at the new patch set (#3). Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... ichspi.c: Implement read_write_status for wp The ichspi hwseq path has a opaque master specialisation that allows for reading and writing STATUS1 registers. Hook the callbacks with a implementation to allow for this so that writeprotect maybe supported though this path. Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M ichspi.c 1 file changed, 65 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/64540/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2022