Attention is currently required from: Nico Huber, Miklós Márton, Edward O'Callaghan.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57003 )
Change subject: ni845x_spi: add missing const specifier to the spi_transmit function
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> >> I actually started to work on this yesterday. […]
I realised my comment may seem odd on the top of Angel's comment.
What I wanted to say, I remember there was a conversation few weeks ago in irc, about writing a test for ni845x_spi, and it can use all the tricks that existing tests are using. So I thought maybe this is what you started?
I think, test would be ideal, thank you so much for your work! (I am happy to help)
--
To view, visit https://review.coreboot.org/c/flashrom/+/57003
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c14361283b2da3725d9ba2cae0e1007e6be3d66
Gerrit-Change-Number: 57003
Gerrit-PatchSet: 1
Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.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: 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: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 22:22:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Miklós Márton <martonmiklosqdev(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Miklós Márton, Edward O'Callaghan.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57003 )
Change subject: ni845x_spi: add missing const specifier to the spi_transmit function
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
> > Many thanks for the quick review! […]
>> I actually started to work on this yesterday.
Wow, this is very cool! Thanks!
Do you write a test for ni845x_spi?
--
To view, visit https://review.coreboot.org/c/flashrom/+/57003
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c14361283b2da3725d9ba2cae0e1007e6be3d66
Gerrit-Change-Number: 57003
Gerrit-PatchSet: 1
Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.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: 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: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 22:01:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Miklós Márton <martonmiklosqdev(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56103 )
Change subject: spi_master: Use new API to register shutdown function
......................................................................
Patch Set 7:
(1 comment)
Patchset:
PS7:
> You can avoid losing votes by doing the rebase and edit as individual steps. […]
This is useful, thanks! I had no idea.
--
To view, visit https://review.coreboot.org/c/flashrom/+/56103
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140
Gerrit-Change-Number: 56103
Gerrit-PatchSet: 7
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: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Wed, 18 Aug 2021 21:07:59 +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
Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56103 )
Change subject: spi_master: Use new API to register shutdown function
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/56103
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140
Gerrit-Change-Number: 56103
Gerrit-PatchSet: 7
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: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 09:02:01 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56103 )
Change subject: spi_master: Use new API to register shutdown function
......................................................................
Patch Set 7: Code-Review+2
(1 comment)
Patchset:
PS7:
> I rebased and updated commit message, and lost all the votes.
You can avoid losing votes by doing the rebase and edit as individual steps.
If more than one thing happens at once, Gerrit can't tell anymore what happened.
--
To view, visit https://review.coreboot.org/c/flashrom/+/56103
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140
Gerrit-Change-Number: 56103
Gerrit-PatchSet: 7
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: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 08:40:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Miklós Márton, Edward O'Callaghan, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57003 )
Change subject: ni845x_spi: add missing const specifier to the spi_transmit function
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Many thanks for the quick review!
>
> Well the internal API seems to be changed since the ni845x_spi introduction and I have not built flashrom on Windows since then. As we discussed before it might be useful to create a mock C file for the ni845x.h library header to being able to perfrom least compilation tests (without Windows, mingw, and installed NI drivers). I actually started to work on this yesterday.
Speaking of mocks, this gives me an idea: how about making something that not only serves to build-test the code, but also allows running cmocka tests on ni845x?
--
To view, visit https://review.coreboot.org/c/flashrom/+/57003
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c14361283b2da3725d9ba2cae0e1007e6be3d66
Gerrit-Change-Number: 57003
Gerrit-PatchSet: 1
Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.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: 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: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 06:59:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Miklós Márton <martonmiklosqdev(a)gmail.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk.
Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57003 )
Change subject: ni845x_spi: add missing const specifier to the spi_transmit function
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I see, the function pointer declaration in `struct spi_master` has `const struct flashctx *flash`.
Many thanks for the quick review!
Well the internal API seems to be changed since the ni845x_spi introduction and I have not built flashrom on Windows since then. As we discussed before it might be useful to create a mock C file for the ni845x.h library header to being able to perfrom least compilation tests (without Windows, mingw, and installed NI drivers). I actually started to work on this yesterday.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57003
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c14361283b2da3725d9ba2cae0e1007e6be3d66
Gerrit-Change-Number: 57003
Gerrit-PatchSet: 1
Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.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: 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: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 06:24:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Miklós Márton, Edward O'Callaghan, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57003 )
Change subject: ni845x_spi: add missing const specifier to the spi_transmit function
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
Patchset:
PS1:
I see, the function pointer declaration in `struct spi_master` has `const struct flashctx *flash`.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57003
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c14361283b2da3725d9ba2cae0e1007e6be3d66
Gerrit-Change-Number: 57003
Gerrit-PatchSet: 1
Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.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: 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: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Wed, 18 Aug 2021 06:07:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment