- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: hwaccess_x86_io: refactor rget_io_perms()
by Nico Huber (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61275 ) Change subject: hwaccess_x86_io: refactor rget_io_perms() ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42 Gerrit-Change-Number: 61275 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Feb 2022 16:42:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: add CONFIG_MEDIATEK_I2C_SPI to DEPENDS_ON_LINUX_I2C
by Thomas Heijligen (Code Review) 22 Feb '22

22 Feb '22

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62217 ) Change subject: Makefile: add CONFIG_MEDIATEK_I2C_SPI to DEPENDS_ON_LINUX_I2C ...................................................................... Makefile: add CONFIG_MEDIATEK_I2C_SPI to DEPENDS_ON_LINUX_I2C The Mediatek i2c spi driver requires the linux i2c stack. Change-Id: Ic90048c549fecb8c051750a92c5b35403f07fbf4 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/62217/1 diff --git a/Makefile b/Makefile index 416fa19..26befc7 100644 --- a/Makefile +++ b/Makefile @@ -172,6 +172,7 @@ CONFIG_MSTARDDC_SPI \ CONFIG_LSPCON_I2C_SPI \ CONFIG_REALTEK_MST_I2C_SPI \ + CONFIG_MEDIATEK_I2C_SPI \ ifeq ($(CONFIG_ENABLE_LIBUSB1_PROGRAMMERS), no) $(call disable_all,$(DEPENDS_ON_LIBUSB1)) -- To view, visit https://review.coreboot.org/c/flashrom/+/62217 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic90048c549fecb8c051750a92c5b35403f07fbf4 Gerrit-Change-Number: 62217 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Makefile: print version info as part of the config target
by Thomas Heijligen (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Felix Singer, Nico Huber. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62199 ) Change subject: Makefile: print version info as part of the config target ...................................................................... Patch Set 2: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/62199/comment/ebe43f8e_d7c6cb51 PS2, Line 899: PROGRAM > What if somebody runs `make libflashrom.a`? I think we can just […] Should we just hard-code $(PROGRAM) as flashrom in the whole file? -- To view, visit https://review.coreboot.org/c/flashrom/+/62199 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1a846acfd8d2e0a9fc8b02c078b6ac0342438490 Gerrit-Change-Number: 62199 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 21 Feb 2022 16:14:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess_x86_io: clean header concept
by Thomas Heijligen (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61276 ) Change subject: hwaccess_x86_io: clean header concept ...................................................................... Patch Set 11: (1 comment) Patchset: PS11: I've updated the way how to assign the preproccessor values. This should make the statements easy to read. -- To view, visit https://review.coreboot.org/c/flashrom/+/61276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Gerrit-Change-Number: 61276 Gerrit-PatchSet: 11 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Feb 2022 16:09:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess_x86_io: refactor rget_io_perms()
by Thomas Heijligen (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61275 ) Change subject: hwaccess_x86_io: refactor rget_io_perms() ...................................................................... Patch Set 7: (3 comments) File hwaccess_x86_io.c: https://review.coreboot.org/c/flashrom/+/61275/comment/461e6567_7c136594 PS4, Line 124: msg_perr("Make sure you are root.\n"); > If we change this already... There were some reports lately that the message […] Done https://review.coreboot.org/c/flashrom/+/61275/comment/62169873_92675f15 PS4, Line 126: "reboot, or reboot into single user mode.\n"); > would be nice to keep the alignment, i.e. […] Done https://review.coreboot.org/c/flashrom/+/61275/comment/4dcb6f46_40f5f5a5 PS4, Line 128: "that your kernel configuration has the option INSECURE enabled.\n"); > would be nice to keep the alignment, i.e. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/61275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42 Gerrit-Change-Number: 61275 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Feb 2022 16:04:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess_x86_io: refactor rget_io_perms()
by Thomas Heijligen (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Thomas Heijligen, Angel Pons. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61275 to look at the new patch set (#7). Change subject: hwaccess_x86_io: refactor rget_io_perms() ...................................................................... hwaccess_x86_io: refactor rget_io_perms() Abstract the different I/O Port permission methods in own functions. Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M hwaccess_x86_io.c 1 file changed, 98 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/61275/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/61275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42 Gerrit-Change-Number: 61275 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: hwaccess_x86_io: clean header concept
by Thomas Heijligen (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Nico Huber, Angel Pons. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61276 to look at the new patch set (#11). Change subject: hwaccess_x86_io: clean header concept ...................................................................... hwaccess_x86_io: clean header concept Move all function implementations into the .c file Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile M hwaccess_x86_io.c M hwaccess_x86_io.h M meson.build 4 files changed, 298 insertions(+), 188 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/61276/11 -- To view, visit https://review.coreboot.org/c/flashrom/+/61276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Gerrit-Change-Number: 61276 Gerrit-PatchSet: 11 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Nico Huber (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Hung-Te Lin, Subrata Banik, Edward O'Callaghan. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 2: (1 comment) File big_lock.h: https://review.coreboot.org/c/flashrom/+/62213/comment/aea89654_e6e0be8c PS2, Line 1: 2012 > > Copyright lines are supposed to tell the date of the original publication. […] As the commit message suggests, it's not new, just new to this repository. -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 15:51:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Subrata Banik (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Hung-Te Lin, Nico Huber, Edward O'Callaghan. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 2: (1 comment) File big_lock.h: https://review.coreboot.org/c/flashrom/+/62213/comment/e509d8e8_f12599f6 PS2, Line 1: 2012 > Copyright lines are supposed to tell the date of the original publication. Exactly that is my point as well. This is new file right, at least git history shows the same. Is it reasonable that a new file has copyright year from past is my point. > There are two lines... Was there anything significant added in the meantime? oO -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 15:07:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Nico Huber (Code Review) 22 Feb '22

22 Feb '22

Attention is currently required from: Hung-Te Lin, Subrata Banik, Edward O'Callaghan. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 2: (3 comments) Patchset: PS2: Do we need a "big lock"? libusb based drivers, for instance, they have to call libusb_claim_interface() (I'm not sure though, if that's enough on all platforms). So they might be covered already. Generally, I'd prefer to lock individual resources, not the whole program. Wasn't flashrom used in Chromebook manufacturing? Seems possible that they run only one instance per host, but I wouldn't have expected that. FWIW, a sane candidate for locking is our physmap infrastructure. We could actually lock individual resources by encoding the base address into the file name, e.g. `flashrom.physmap-0x12345678.lock`. Port-based I/O also comes to mind. File big_lock.h: https://review.coreboot.org/c/flashrom/+/62213/comment/2f76fcbf_426a5bff PS2, Line 1: 2012 > need update may be ? Copyright lines are supposed to tell the date of the original publication. There are two lines... Was there anything significant added in the meantime? oO File flashrom.c: https://review.coreboot.org/c/flashrom/+/62213/comment/a40bde94_ae1e2d6d PS1, Line 142: programmer_entry > Not just futility - this is more like "can you run flashrom without getting root permission (required by lock)". I think people will want to simulate a flashrom call using dummy programmer (to create an image file) without adding 'sudo'. Why would a lock need root permission? Can't we just place a file in `/tmp/`? > > But on the other hand, it's also probably not a bad idea if we always require 'sudo' - even for unit tests. I'm fine if you want to keep the biglock always there. FWIW, it's generally considered a bad idea to require `sudo`. -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 15:03:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> 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

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit