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

Change in flashrom[master]: dummyflasher: move struct declaration & probe_variable_size to spi.(h|c)
by Anastasia Klimchuk (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Namyoon Woo, Thomas Heijligen, Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63831 ) Change subject: dummyflasher: move struct declaration & probe_variable_size to spi.(h|c) ...................................................................... Patch Set 1: (2 comments) File spi.c: https://review.coreboot.org/c/flashrom/+/63831/comment/f33c6738_dd2637e2 PS1, Line 133: int probe_variable_size(struct flashctx *flash) : { : unsigned int i; : const struct emu_data *emu_data = flash->mst->spi.data; This is not probing anything, right? Just emulating. The name does not indicate the emulation however... I wasn't thinking about it while the function was in dummy, because dummy is all about emulation. But now it is in spi.c. Also it relies on emu_data, what happens if there is some other data in the context? another struct type? I understand these questions are not about build system, it's just some thoughts triggered by the change. https://review.coreboot.org/c/flashrom/+/63831/comment/877bb195_ad9e5c4d PS1, Line 138: /* Skip the probing if we don't emulate this chip. */ : if (!emu_data || emu_data->emu_chip != EMULATE_VARIABLE_SIZE) : return 0; Why this branch returns 0? There was no probing done? I know it was like this before :) Thinking aloud. -- To view, visit https://review.coreboot.org/c/flashrom/+/63831 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic93c8b9ba7b9f7ce5fe49326c8de34070ca83a2e Gerrit-Change-Number: 63831 Gerrit-PatchSet: 1 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: Namyoon Woo <namyoon(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Namyoon Woo <namyoon(a)google.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 26 Apr 2022 23:14:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Close GitHub PRs and issues automatically
by Martin Roth (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63671 ) Change subject: Close GitHub PRs and issues automatically ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > I agree with Felix's analysis. Sure, I think that's reasonable. I just wanted to present it as an option. -- To view, visit https://review.coreboot.org/c/flashrom/+/63671 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8076f0fb964970ffd05f355b9d1e33a65aa7a3c8 Gerrit-Change-Number: 63671 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Apr 2022 16:34:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Reduce read size for FT232H
by Peter Farley (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Nico Huber, Angel Pons. Peter Farley has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63798 ) Change subject: ft2232_spi: Reduce read size for FT232H ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/63798/comment/e2745922_9b29997a PS1, Line 9: FT232H-based devices are unable to read over 256 bytes of flash in one > So, with the 1536B chunk size, do you always get full URBs succeeding? i.e. […] When the URB requests succeed, yeah, they are always 1536 bytes. And the last URB_BULK in reports -ENOENT. The only thing I am powering via the FT232H board is the flash chip, yeah. The chip is standalone in a socket that is only attached to the FT232H board. Using forced mode without the flash, I don't get any RX packets once it starts attempting to read the flash. It attempts a 1536B URB_BULK in request, but it just gives a -ENOENT a few seconds later. Using -f _with_ the chip installed it gives similar results to not using -f. This is the command I am using for reference: ./flashrom -p ft2232_spi:type=232H -c "W25Q80BW" -VVV -r dummy.rom -f -- To view, visit https://review.coreboot.org/c/flashrom/+/63798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a45fb8ac8ff37bbf01b0cb6b88851cf74af495d Gerrit-Change-Number: 63798 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Farley <far.peter1(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Apr 2022 11:41:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Peter Farley <far.peter1(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi: Reduce read size for FT232H
by Nico Huber (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Angel Pons, Peter Farley. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63798 ) Change subject: ft2232_spi: Reduce read size for FT232H ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/63798/comment/649a4785_64586ed2 PS1, Line 9: FT232H-based devices are unable to read over 256 bytes of flash in one > > One more general thought: Is there any other software running that might […] So, with the 1536B chunk size, do you always get full URBs succeeding? i.e. it's always 1536B or nothing? It all seems still very odd to me. I wonder if this is not some very subtle hardware issue. Do you power anything via the FT232H board? the flash chip I assume? is there anything else (e.g. the chip on some (main)board)? We could try some sort of dry-run without the flash chip, e.g. disconnect the chip, and then do a forced read by adding `-c <flashchip> -f` to a read command. That should lower the things that could go wrong hardware-wise. -- To view, visit https://review.coreboot.org/c/flashrom/+/63798 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a45fb8ac8ff37bbf01b0cb6b88851cf74af495d Gerrit-Change-Number: 63798 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Farley <far.peter1(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Farley <far.peter1(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Apr 2022 10:59:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Peter Farley <far.peter1(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: Treat warnings as errors
by Thomas Heijligen (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63852 ) Change subject: meson: Treat warnings as errors ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/63852 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I06c72abf1c861f9b35736e02ef7f4a4fb6909b97 Gerrit-Change-Number: 63852 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 26 Apr 2022 07:18:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: Add optimisation level s and disable debug in meson build
by Thomas Heijligen (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63606 ) Change subject: meson: Add optimisation level s and disable debug in meson build ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: > I read more into meson docs and I discovered that optimisation and debug options are linked together […] I think this approach is fine. There is no need to overengineer it just to match the exact situation of the Makefile. -- To view, visit https://review.coreboot.org/c/flashrom/+/63606 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4ca96a866529cac320e66516ef280d5100ceefab Gerrit-Change-Number: 63606 Gerrit-PatchSet: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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: Peter Marheine <pmarheine(a)chromium.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, 26 Apr 2022 07:17:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add -W options from Makefile into meson warning_flags
by Thomas Heijligen (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Anastasia Klimchuk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58561 ) Change subject: Add -W options from Makefile into meson warning_flags ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58561 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id401bfd642dc3c13d85bd9a2dba56ada38714c25 Gerrit-Change-Number: 58561 Gerrit-PatchSet: 5 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 26 Apr 2022 07:06:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: outsource platform specific code to `platform/meson.build`
by Thomas Heijligen (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63826 ) Change subject: meson: outsource platform specific code to `platform/meson.build` ...................................................................... Patch Set 2: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/63826/comment/410f4089_8cd83caa PS2, Line 133: if host_machine.system() in ['cygwin', 'windows'] > Would it be reasonable to put this Windows detection in platform/ too? Or is OS detection a differen […] Yes, we can move it here first. This is one of the symbols I want to get rid off and split the code into a windows and non windows file inside the platform dir. -- To view, visit https://review.coreboot.org/c/flashrom/+/63826 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I88044a3f903f316138483dd872a6d95f8686ae69 Gerrit-Change-Number: 63826 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 26 Apr 2022 06:56:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: meson: Add optimisation level s and disable debug in meson build
by Anastasia Klimchuk (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63606 ) Change subject: meson: Add optimisation level s and disable debug in meson build ...................................................................... Patch Set 2: (3 comments) Patchset: PS1: > Meson uses debug build as default. So the binary get a lot bigger than with make. […] Yes, thank you so much! This explains. PS1: > and also which meson version? The end result is that I also disabled debug, as it is in Makefile, and now yes, binaries are very similar size. Why not exactly the same, I am very interested to understand why, I will look into that next. However now, since I disabled debug, binary size before and after changed a lot, almost twice smaller after :) Patchset: PS2: I read more into meson docs and I discovered that optimisation and debug options are linked together. There are two possible approaches for meson build: 1) set optimisation and debug, that's what the patch is doing 2) set buildtype and don't set any of optimisation and debug, in this case optimisation and debug will be calculated from buildtype But none of available buildtypes gives a combination of optimisation and debug that makefile is using. If we want to sync with makefile, we need to set optimisation and debug. Otherwise that will be a diff. -- To view, visit https://review.coreboot.org/c/flashrom/+/63606 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4ca96a866529cac320e66516ef280d5100ceefab Gerrit-Change-Number: 63606 Gerrit-PatchSet: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 26 Apr 2022 05:57:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add -W options from Makefile into meson warning_flags
by Anastasia Klimchuk (Code Review) 26 Apr '22

26 Apr '22

Attention is currently required from: Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58561 ) Change subject: Add -W options from Makefile into meson warning_flags ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/58561/comment/de415b5d_2f3d1408 PS4, Line 13: warning_level=2 means -Wall and -Wextra as it is set in > Needs update. This is now in the next patch. This is actually not needed in any patch, I discovered that warning_level=2 already sets -Wall and -Wextra. I still wanted to mention in commit message, so that it is clear why -Wall and -Wextra and not needed in the patch. So I added info in commit message. I tried to re-write it a bit, so that it is more clear. -- To view, visit https://review.coreboot.org/c/flashrom/+/58561 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id401bfd642dc3c13d85bd9a2dba56ada38714c25 Gerrit-Change-Number: 58561 Gerrit-PatchSet: 5 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 26 Apr 2022 05:47:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2022