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

Change in flashrom[master]: lspcon_i2c_spi: Extract I2C bus parameter handling
by Edward O'Callaghan (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52830 ) Change subject: lspcon_i2c_spi: Extract I2C bus parameter handling ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52830 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I006b311c88feea37fe4b217f769b21ca1505def9 Gerrit-Change-Number: 52830 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 May 2021 12:19:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: lspcon_i2c_spi: Extract I2C bus parameter handling
by Angel Pons (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52830 ) Change subject: lspcon_i2c_spi: Extract I2C bus parameter handling ...................................................................... Patch Set 2: (1 comment) File i2c_helper_linux.c: https://review.coreboot.org/c/flashrom/+/52830/comment/2f4ff52d_5a0c704b PS1, Line 80: * TODO: Move this function to a common place, it is not specific to I2C */ > Just drop it all together I suppose. Gone -- To view, visit https://review.coreboot.org/c/flashrom/+/52830 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I006b311c88feea37fe4b217f769b21ca1505def9 Gerrit-Change-Number: 52830 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 May 2021 12:09:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: lspcon_i2c_spi: Extract I2C bus parameter handling
by Edward O'Callaghan (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52830 ) Change subject: lspcon_i2c_spi: Extract I2C bus parameter handling ...................................................................... Patch Set 1: (1 comment) File i2c_helper_linux.c: https://review.coreboot.org/c/flashrom/+/52830/comment/a113fbe7_f0fbf72d PS1, Line 80: * TODO: Move this function to a common place, it is not specific to I2C */ > If one removes all the references to "bus" from this function, what remains is a function that parse […] Just drop it all together I suppose. -- To view, visit https://review.coreboot.org/c/flashrom/+/52830 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I006b311c88feea37fe4b217f769b21ca1505def9 Gerrit-Change-Number: 52830 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 May 2021 11:59:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: Add missing braces
by Edward O'Callaghan (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52827 ) Change subject: realtek_mst_i2c_spi: Add missing braces ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52827 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I69b762391165177857e9331f79f54b01149cf339 Gerrit-Change-Number: 52827 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 May 2021 11:56:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: Convert anon union to anon struct
by Edward O'Callaghan (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Stefan Reinauer, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50246 ) Change subject: programmer.h: Convert anon union to anon struct ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/50246/comment/9f5e1e45_c1f72a38 PS4, Line 14: The user of the registered_masters type is therefore : responsible for querying the buses_supported field before : attempting to dereference a ptr field in the anon struct. > Can the user check which one out of three is not null? It's a good question. I guess I mean "user" here to mean things that call "spi_send_command()" such as in spi25.c. The actual case where this exploded was when w25q_read_status_register_2() used spi_send_command(), which requires that the master be a spi_master where as Hardware-sequenced ICH that is an opaque master was being used. Essentially the union is a bit of a trap. -- To view, visit https://review.coreboot.org/c/flashrom/+/50246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242 Gerrit-Change-Number: 50246 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 May 2021 09:22:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: Add missing braces
by Angel Pons (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52827 ) Change subject: realtek_mst_i2c_spi: Add missing braces ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Is this Linux kernel coding style? Asking for myself, I will gradually read it. Yes, but with an exception about line length: https://www.flashrom.org/Development_Guidelines#Coding_style -- To view, visit https://review.coreboot.org/c/flashrom/+/52827 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I69b762391165177857e9331f79f54b01149cf339 Gerrit-Change-Number: 52827 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 May 2021 09:07:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: lspcon_i2c_spi: Extract I2C bus parameter handling
by Angel Pons (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52830 ) Change subject: lspcon_i2c_spi: Extract I2C bus parameter handling ...................................................................... Patch Set 1: (1 comment) File i2c_helper_linux.c: https://review.coreboot.org/c/flashrom/+/52830/comment/530d31ed_c4e9217c PS1, Line 80: * TODO: Move this function to a common place, it is not specific to I2C */ > I guess this todo is fixed now in this commit? If one removes all the references to "bus" from this function, what remains is a function that parses a string to obtain an integer. This new function is not specific to I2C. Reading my comment again, I agree that it doesn't make much sense. Any suggestions on how to reword it, or should I drop it altogether? -- To view, visit https://review.coreboot.org/c/flashrom/+/52830 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I006b311c88feea37fe4b217f769b21ca1505def9 Gerrit-Change-Number: 52830 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Peter Marheine <pmarheine(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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 03 May 2021 09:06:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h: Convert anon union to anon struct
by Anastasia Klimchuk (Code Review) 03 May '21

03 May '21

Attention is currently required from: Stefan Reinauer, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50246 ) Change subject: programmer.h: Convert anon union to anon struct ...................................................................... Patch Set 4: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/50246/comment/0181bd39_5d2811fe PS4, Line 14: The user of the registered_masters type is therefore : responsible for querying the buses_supported field before : attempting to dereference a ptr field in the anon struct. Can the user check which one out of three is not null? -- To view, visit https://review.coreboot.org/c/flashrom/+/50246 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I576967a8599b923c902e39f177f39146291cc242 Gerrit-Change-Number: 50246 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Alan Green <avg(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 03 May 2021 08:28:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 03 May '21

03 May '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Samir Ibradžić. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Implement spi_send_multicommand() ...................................................................... Patch Set 32: (1 comment) Patchset: PS32: > I wondered what device was used for testing so far and […] Thanks for your effort! hm, I´m not sure what´s wrong. Saving a few seconds is not what I was looking for... Some information: * the 1,36MHz on the screenshot is wrong (sorry for missleading!!) It´s the distance between the cursors which may be placed arbitrarily * I´m using default divisor and on my osci screenshots taken last week that results in something between 25-30 MHz (did not measure, but estimate from screenshots) * lsusb says bcdDevice 7.00 as well * I run: `make CONFIG_ENABLE_LIBUSB1_PROGRAMMERS=no`. So I´m not using libusb 1.0 maybe this leads to some difference? * I´m doing tests on my side with an N25Q128..3E chip (has quite the same parameters as W25Q128FV") * I´m running flashrom on a virutal machine (don´t think this makes any difference) * at SPI-Bus level the multicmd+chunk4k looks much better and faster (i.e. less bursts) so I have no idea why this should not lead to an overall speed increase... (regardless of the setup) NB: yepp, I noticed that with the erase block size, as well and have it on my todo list. I was thinking about something like comandline parameter to select block size or an extra parameter at flashchips.c to mark the preferred block size. May be we should open a new patch for that to discuss ideas... (it was just because of lack of support for this patch for month why I did not step further) -- To view, visit https://review.coreboot.org/c/flashrom/+/40477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a Gerrit-Change-Number: 40477 Gerrit-PatchSet: 32 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Alan Green <avg(a)google.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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> 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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Mon, 03 May 2021 07:27:04 +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]: pickit2_spi.c: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 03 May '21

03 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/+/52774 ) Change subject: pickit2_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 2: (1 comment) File pickit2_spi.c: https://review.coreboot.org/c/flashrom/+/52774/comment/9e0db6a6_26e095d2 PS1, Line 472: pickit2_data = calloc(1, sizeof(*pickit2_data)); > No NULL check? (Could be avoided by not using a struct. […] This my mistake sorry :( NULL check added, I do need a struct. -- To view, visit https://review.coreboot.org/c/flashrom/+/52774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibacc4738bee02c371c41583d321e0337128ad18a Gerrit-Change-Number: 52774 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: 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 03 May 2021 07:24:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2021