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

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Edward O'Callaghan (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: -Code-Review (1 comment) Patchset: PS1: I thought this was Anastasia work? -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 12 May 2021 01:10:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Edward O'Callaghan (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 12 May 2021 01:08:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: Make use of new register_spi_master() API
by Edward O'Callaghan (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54067 ) Change subject: programmer: Make use of new register_spi_master() API ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id7821f1db3284b7b5b3d0abfd878b979c53870a1 Gerrit-Change-Number: 54067 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Wed, 12 May 2021 00:59:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: Make use of new register_spi_master() API
by Anastasia Klimchuk (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54067 ) Change subject: programmer: Make use of new register_spi_master() API ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/54067 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id7821f1db3284b7b5b3d0abfd878b979c53870a1 Gerrit-Change-Number: 54067 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 00:49:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer: Smoothen register_spi_master() API
by Anastasia Klimchuk (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54066 ) Change subject: programmer: Smoothen register_spi_master() API ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File spi.c: https://review.coreboot.org/c/flashrom/+/54066/comment/f157fcea_1f0e2c50 PS1, Line 151: if (data) : rmst.spi.data = data; This would need to move to register_master at some point, because the same situation exists now for par_master and opaque_master. Means, we need similar patch for register_par_master/opaque which changes the callsites in the tree. And then later internal change to move these two lines into register_master? For par masters, I haven't started refactoring them, maybe it's a good time to intro a patch like this for par masters? Do you mind if I do it or would you like to do it? Anyway, thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/54066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f Gerrit-Change-Number: 54066 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 12 May 2021 00:48:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: usbblaster_spi.c: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54044 ) Change subject: usbblaster_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 1: (2 comments) File usbblaster_spi.c: https://review.coreboot.org/c/flashrom/+/54044/comment/f384c34a_eba322bf PS1, Line 55: struct ftdi_context ftdic; > Having a struct with only one member looks odd, but it makes sense for consistency with other progra […] Oh I had the same question in CB:52774 and I explained why I am doing this (in short, mostly so that I can calloc and free data). But I wrote a much longer comment in CB:52774 , you are very welcome to read all that :) https://review.coreboot.org/c/flashrom/+/54044/comment/0bf389c6_806eb73d PS1, Line 185: struct ftdi_context ftdic; > We could allocate `usbblaster_data` early and use `usbblaster_data->ftdic` instead of having a secon […] I usually try to allocate data as late as possible to simplify error paths (as soon as you allocate data, you need to take care to free it for all error paths after that). So it was intentional, can I leave it? what do you think? -- To view, visit https://review.coreboot.org/c/flashrom/+/54044 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia81f9f40c7eab430a8b304d0b197ce7c75bf5ace Gerrit-Change-Number: 54044 Gerrit-PatchSet: 1 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Tue, 11 May 2021 23:54:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 ) Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... Patch Set 16: (2 comments) Patchset: PS16: I have a big question: is this patch ready? what do reviewers think? Is there anything left for me to do, so that it can be submitted? Thank you! File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/51487/comment/1df063ee_261d2a80 PS13, Line 37: unsigned char > Hmmm, the functions there are only implemented for Net and OpenBSD. I don't […] I put your types back: I see you did way more thinking about this. I wasn't thinking much - just took what I saw in hwaccess. Tests are working one way or the other. I did rename priv into state, if you don't mind? Priv doesn't tell much apart from that it is something private. -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 16 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 11 May 2021 23:30:35 +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]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 11 May '21

11 May '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk, Simon Glass. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Simon Glass, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51487 to look at the new patch set (#16). Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c This patch includes mocks for io operations in hwaccess_x86_io.h because those are needed to test lifecycle of mec1308.c and ene_lpc.c BUG=b:181803212 TEST=builds and ninja test Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A hwaccess_x86_io_unittest.h M meson.build M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 7 files changed, 247 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/16 -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 16 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Simon Glass <sjg(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 11 May '21

11 May '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk, Simon Glass. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Simon Glass, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51487 to look at the new patch set (#15). Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c This patch includes mocks for io operations in hwaccess_x86_io.h because those are needed to test lifecycle of mec1308.c and ene_lpc.c BUG=b:181803212 TEST=builds and ninja test Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A hwaccess_x86_io_unittest.h M meson.build M tests/init_shutdown.c M tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 7 files changed, 247 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/15 -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 15 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: Simon Glass <sjg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Simon Glass <sjg(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashchips: Fix 4BA advertisement for dummy chip
by Angel Pons (Code Review) 11 May '21

11 May '21

Attention is currently required from: Nico Huber, Namyoon Woo, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54068 ) Change subject: flashchips: Fix 4BA advertisement for dummy chip ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54068 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia7340835ce1680d197f250bdb5990ab2ffe3671f Gerrit-Change-Number: 54068 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Namyoon Woo <namyoon(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 11 May 2021 17:19:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021