- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: spi: Make default cmd helpers static internal
by Edward O'Callaghan (Code Review) 28 Feb '23

28 Feb '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73337 ) Change subject: spi: Make default cmd helpers static internal ...................................................................... spi: Make default cmd helpers static internal Avoid these leaking into driver implementations as a NULL field now implies their implementation. This removes one source of a driver bug where both `mst->command` AND `mst->multicommand` are set to default implementations which is actually a cyclical control flow condition. The driver however must still have either `mst->command` OR `mst->multicommand` defined and so both cannot be NULL. This simplifies the code and driver development. Change-Id: I4ef95846c2f005cf4aa727f31548c6877d2d4801 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M include/programmer.h M spi.c 2 files changed, 40 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/73337/1 diff --git a/include/programmer.h b/include/programmer.h index f72f1d7..f6bc8ce 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -318,9 +318,6 @@ void *data; }; -int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, - const unsigned char *writearr, unsigned char *readarr); -int default_spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds); int default_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int default_spi_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int default_spi_write_aai(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); diff --git a/spi.c b/spi.c index 4fe0fa5..3b3f622 100644 --- a/spi.c +++ b/spi.c @@ -26,23 +26,7 @@ #include "programmer.h" #include "spi.h" -int spi_send_command(const struct flashctx *flash, unsigned int writecnt, - unsigned int readcnt, const unsigned char *writearr, - unsigned char *readarr) -{ - if (flash->mst->spi.command) - return flash->mst->spi.command(flash, writecnt, readcnt, writearr, readarr); - return default_spi_send_command(flash, writecnt, readcnt, writearr, readarr); -} - -int spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds) -{ - if (flash->mst->spi.multicommand) - return flash->mst->spi.multicommand(flash, cmds); - return default_spi_send_multicommand(flash, cmds); -} - -int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, +static int default_spi_send_command(const struct flashctx *flash, unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) @@ -63,7 +47,7 @@ return spi_send_multicommand(flash, cmd); } -int default_spi_send_multicommand(const struct flashctx *flash, +static int default_spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds) { int result = 0; @@ -74,6 +58,22 @@ return result; } +int spi_send_command(const struct flashctx *flash, unsigned int writecnt, + unsigned int readcnt, const unsigned char *writearr, + unsigned char *readarr) +{ + if (flash->mst->spi.command) + return flash->mst->spi.command(flash, writecnt, readcnt, writearr, readarr); + return default_spi_send_command(flash, writecnt, readcnt, writearr, readarr); +} + +int spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds) +{ + if (flash->mst->spi.multicommand) + return flash->mst->spi.multicommand(flash, cmds); + return default_spi_send_multicommand(flash, cmds); +} + int default_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { @@ -156,8 +156,7 @@ } if (!mst->write_256 || !mst->read || !mst->probe_opcode || - ((mst->command == default_spi_send_command || !mst->command) && - (mst->multicommand == default_spi_send_multicommand || !mst->multicommand))) { + (!mst->command && !mst->multicommand)) { msg_perr("%s called with incomplete master definition. " "Please report a bug at flashrom(a)flashrom.org\n", __func__); -- To view, visit https://review.coreboot.org/c/flashrom/+/73337 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4ef95846c2f005cf4aa727f31548c6877d2d4801 Gerrit-Change-Number: 73337 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: spi: Make 'default_spi_send_multicommand' the default unless defined
by Edward O'Callaghan (Code Review) 28 Feb '23

28 Feb '23

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Thomas Heijligen, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67481 to look at the new patch set (#3). Change subject: spi: Make 'default_spi_send_multicommand' the default unless defined ...................................................................... spi: Make 'default_spi_send_multicommand' the default unless defined A NULL func pointer is necessary and sufficient for the condition `NULL func pointer => default_spi_send_multicommand' as to not need this explicit specification of 'default'. Therefore drop the explicit need to specify the 'default_spi_send_multicommand' callback function pointer in the spi_master struct. This is a reasonable default for every other driver in the tree with only a few exceptions. This simplifies the code and driver development. Change-Id: I6cc24bf982da3d5251d391eb397db43dd10280e8 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M asm106x.c M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M ch347_spi.c M dediprog.c M digilent_spi.c M dirtyjtag_spi.c M dummyflasher.c M it87spi.c M jlink_spi.c M linux_spi.c M mediatek_i2c_spi.c M mstarddc_spi.c M ni845x_spi.c M parade_lspcon.c M pickit2_spi.c M raiden_debug_spi.c M realtek_mst_i2c_spi.c M sb600spi.c M serprog.c M spi.c M stlinkv3_spi.c M usbblaster_spi.c M wbsio_spi.c 25 files changed, 26 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/67481/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/67481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6cc24bf982da3d5251d391eb397db43dd10280e8 Gerrit-Change-Number: 67481 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: ch347t_spi.c: Move ch347_device struct to flashctx
by Nicholas Chin (Code Review) 28 Feb '23

28 Feb '23

Nicholas Chin has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/71066 ) Change subject: ch347t_spi.c: Move ch347_device struct to flashctx ...................................................................... Abandoned Not needed anymore, CB:70573 was merged which already contained these changes -- To view, visit https://review.coreboot.org/c/flashrom/+/71066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If3ff96dedd2b591200dd69f54df753668853beca Gerrit-Change-Number: 71066 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[XL] Change in flashrom[master]: move manpage to sphinx
by Thomas Heijligen (Code Review) 28 Feb '23

28 Feb '23

Attention is currently required from: Alexander Goncharov. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72619 ) Change subject: move manpage to sphinx ...................................................................... Patch Set 12: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/72619/comment/efeb9e08_d49b1807 PS11, Line 4: meson_version : '>=0.57.0', > How about mentioning it in the commit message? Something like: […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/72619 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee9f1164c5913e47385e6f7d51dc7775a58b5a67 Gerrit-Change-Number: 72619 Gerrit-PatchSet: 12 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:21:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[master]: move manpage to sphinx
by Thomas Heijligen (Code Review) 28 Feb '23

28 Feb '23

Attention is currently required from: Thomas Heijligen. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72619 to look at the new patch set (#12). Change subject: move manpage to sphinx ...................................................................... move manpage to sphinx Use sphinx (sphinx-doc.org) to generate the UNIX man page from an reStructuredText file instead of dealing with plain groff. Use `meson setup -Dman-pages=enabled` to build the man page, and `meson setup -Ddocumentation=enabled` to build the web documentation explicitly. Both are enabled automatically if sphinx-build is found. The man page will be installed as `<meson_mandir>/man8/flashrom.8` and The html documentation in <meson_datadir>/doc/flashrom/html`. The Makefile builds only the man-page format. Increase the minimum version of meson from 0.53.0 to 0.57.0 to be able to pass environment variables to the custom_target() command. That is needed to pass the FLASHROM_VERSION to the documentation. Change-Id: Iee9f1164c5913e47385e6f7d51dc7775a58b5a67 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M .gitignore M Makefile A doc/classic_cli_manpage.rst A doc/conf.py A doc/index.rst A doc/meson.build D flashrom.8.tmpl M meson.build M meson_options.txt 9 files changed, 1,494 insertions(+), 1,870 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/72619/12 -- To view, visit https://review.coreboot.org/c/flashrom/+/72619 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee9f1164c5913e47385e6f7d51dc7775a58b5a67 Gerrit-Change-Number: 72619 Gerrit-PatchSet: 12 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: meson: use mesons configuration_data for to evaluate cflags
by Thomas Heijligen (Code Review) 28 Feb '23

28 Feb '23

Attention is currently required from: Thomas Heijligen. Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/72656 to look at the new patch set (#6). Change subject: meson: use mesons configuration_data for to evaluate cflags ...................................................................... meson: use mesons configuration_data for to evaluate cflags Use mesons build in system for configurations instead of managing them by our own. Change-Id: Ie91b4e1377b9d6d347511d4727c0a26a314b1e57 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M doc/meson.build M meson.build 2 files changed, 39 insertions(+), 79 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/72656/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/72656 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie91b4e1377b9d6d347511d4727c0a26a314b1e57 Gerrit-Change-Number: 72656 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: doc/classic_cli_manpage: add missing chip for ft2232_spi programmer
by Thomas Heijligen (Code Review) 28 Feb '23

28 Feb '23

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk, Alexander Goncharov. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73260 ) Change subject: doc/classic_cli_manpage: add missing chip for ft2232_spi programmer ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73260 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id25f6444bfe871f7b01ae2caff23ce973b44567a Gerrit-Change-Number: 73260 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Charles Parent <cha.parent(a)gmail.com> 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-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Tue, 28 Feb 2023 13:21:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ch341a_spi: Refactor singleton states into reentrant pattern
by Alexander Goncharov (Code Review) 28 Feb '23

28 Feb '23

Attention is currently required from: Anastasia Klimchuk. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72807 ) Change subject: ch341a_spi: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: (2 comments) File ch341a_spi.c: https://review.coreboot.org/c/flashrom/+/72807/comment/facf031d_250d076f PS3, Line 80: struct libusb_transfer *transfer_out; : struct libusb_transfer *transfer_ins[USB_IN_TRANSFERS]; : : /* Accumulate delays to be plucked between CS deassertion and CS assertions. */ : unsigned int stored_delay_us; : : struct libusb_device_handle *handle; > Will default values for these be the same as before? […] Yes, they will. Memory in ch341a_spi_data is allocated at the beginning of initialization using `calloc()`, so it will be 0s/NULLs. https://review.coreboot.org/c/flashrom/+/72807/comment/f419795f_090d8665 PS3, Line 461: msg_pwarn("Cannot detach the existing USB driver. Claiming the interface may fail. %s\n", : libusb_error_name(ret)); > Is this an error branch? It should `goto free_data` then. […] This is warn branch. So, no need to do something -- To view, visit https://review.coreboot.org/c/flashrom/+/72807 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9fe72bff88b7f8778c1199bdab8ba43bf32e1c8c Gerrit-Change-Number: 72807 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 28 Feb 2023 12:52:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: doc/classic_cli_manpage: add missing chip for ft2232_spi programmer
by Alexander Goncharov (Code Review) 28 Feb '23

28 Feb '23

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/+/73260 to look at the new patch set (#2). Change subject: doc/classic_cli_manpage: add missing chip for ft2232_spi programmer ...................................................................... doc/classic_cli_manpage: add missing chip for ft2232_spi programmer FTDI FT4233H chip was added in commit bc2e3b6b79dd1383cdf3d965df2fc5c0d6596b02 Change-Id: Id25f6444bfe871f7b01ae2caff23ce973b44567a Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> --- M doc/classic_cli_manpage.rst 1 file changed, 14 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/73260/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/73260 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id25f6444bfe871f7b01ae2caff23ce973b44567a Gerrit-Change-Number: 73260 Gerrit-PatchSet: 2 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: Charles Parent <cha.parent(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[XL] Change in flashrom[master]: move manpage to sphinx
by Alexander Goncharov (Code Review) 28 Feb '23

28 Feb '23

Attention is currently required from: Thomas Heijligen. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72619 ) Change subject: move manpage to sphinx ...................................................................... Patch Set 11: Code-Review+1 (2 comments) Patchset: PS11: Great job :) File meson.build: https://review.coreboot.org/c/flashrom/+/72619/comment/deca7a37_729900e8 PS11, Line 4: meson_version : '>=0.57.0', How about mentioning it in the commit message? Something like: "Increase the minimum version of meson to at least 0.57.0 to be able to pass environment variables to the custom_target() command." -- To view, visit https://review.coreboot.org/c/flashrom/+/72619 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee9f1164c5913e47385e6f7d51dc7775a58b5a67 Gerrit-Change-Number: 72619 Gerrit-PatchSet: 11 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 28 Feb 2023 12:40:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit