- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: Rename cli_classic.h to a more adequate cli_getop.h
by Antonio Vázquez Blanco (Code Review) 03 Nov '24

03 Nov '24

Antonio Vázquez Blanco has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84983?usp=email ) Change subject: Rename cli_classic.h to a more adequate cli_getop.h ...................................................................... Rename cli_classic.h to a more adequate cli_getop.h The header only defines getop related stuff so it seems more intuitive this way. Also, use SPDX to reduce header comment sizes. Change-Id: I5986828f3bdec583af6f7b02cbe1a9c45ed2000f Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> --- M cli_classic.c M cli_getopt.c D include/cli_classic.h A include/cli_getopt.h 4 files changed, 50 insertions(+), 68 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/84983/1 diff --git a/cli_classic.c b/cli_classic.c index 3343438..3b6b6c8 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -24,7 +24,7 @@ #include <string.h> #include <stdbool.h> #include <stdlib.h> -#include <cli_classic.h> +#include "cli_getopt.h" #include "flash.h" #include "flashchips.h" #include "fmap.h" diff --git a/cli_getopt.c b/cli_getopt.c index fc51fcf..8e5a330 100644 --- a/cli_getopt.c +++ b/cli_getopt.c @@ -1,37 +1,21 @@ /* * This file is part of the flashrom project. - * It comes originally from the musl libc project and is licensed under the - * terms of the MIT license. + * It comes originally from the musl libc project. * - * Copyringht (C) 2023 Rich Felker and the musl authors - * Adjusted for flashrom by Thomas Heijligen<thomas.heijligen(a)secunet.com> - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. + * SPDX-FileCopyrightText: 2023 Rich Felker and the musl authors + * SPDX-FileCopyrightText: Thomas Heijligen <thomas.heijligen(a)secunet.com> + * SPDX-FileCopyrightText: 2024 Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> + * SPDX-License-Identifier: MIT */ +#include "cli_getopt.h" + #include <unistd.h> #include <wchar.h> #include <string.h> #include <limits.h> #include <stdlib.h> -#include "cli_classic.h" -#include "flash.h" +#include "flash.h" // msg_gerr char *optarg; int optind=1, opterr=1, optopt, optpos; diff --git a/include/cli_classic.h b/include/cli_classic.h deleted file mode 100644 index eb1f0fe..0000000 --- a/include/cli_classic.h +++ /dev/null @@ -1,43 +0,0 @@ -/* - * This file is part of the flashrom project. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef CLI_CLASSIC_H -#define CLI_CLASSIC_H - -#ifdef HAVE_GETOPT_H -#include <getopt.h> -#else - -#define no_argument 0 -#define required_argument 1 -#define optional_argument 2 - -extern char *optarg; -extern int optind, opterr, optopt; - -struct option { - const char *name; - int has_arg; - int *flag; - int val; -}; - -int getopt (int argc, char *const *argv, const char *shortopts); -int getopt_long (int argc, char *const *argv, const char *shortopts, - const struct option *longopts, int *longind); -int getopt_long_only (int argc, char *const *argv, const char *shortopts, - const struct option *longopts, int *longind); - -#endif /* HAVE_GETOPT_H */ -#endif /* CLI_CLASSIC_H */ diff --git a/include/cli_getopt.h b/include/cli_getopt.h new file mode 100644 index 0000000..5c84828 --- /dev/null +++ b/include/cli_getopt.h @@ -0,0 +1,41 @@ +/* + * This file is part of the flashrom project. + * + * SPDX-FileCopyrightText: 2024 Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef __CLI_GETOPT_H__ +#define __CLI_GETOPT_H__ + +/** + * This header is responsible for either including a standard getop + * implementation header or to provide a compatible one. + */ + +#ifdef HAVE_GETOPT_H +#include <getopt.h> +#else + +#define no_argument 0 +#define required_argument 1 +#define optional_argument 2 + +extern char *optarg; +extern int optind, opterr, optopt; + +struct option { + const char *name; + int has_arg; + int *flag; + int val; +}; + +int getopt (int argc, char *const *argv, const char *shortopts); +int getopt_long (int argc, char *const *argv, const char *shortopts, + const struct option *longopts, int *longind); +int getopt_long_only (int argc, char *const *argv, const char *shortopts, + const struct option *longopts, int *longind); + +#endif /* HAVE_GETOPT_H */ +#endif /* __CLI_GETOPT_H__ */ -- To view, visit https://review.coreboot.org/c/flashrom/+/84983?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5986828f3bdec583af6f7b02cbe1a9c45ed2000f Gerrit-Change-Number: 84983 Gerrit-PatchSet: 1 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>

1 0

[S] Change in flashrom[main]: Split usbdev declarations to a separate header.
by Miklós Márton (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Antonio Vázquez Blanco. Miklós Márton has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84982?usp=email ) Change subject: Split usbdev declarations to a separate header. ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 2 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Nov 2024 18:09:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: Split usbdev declarations to a separate header.
by Antonio Vázquez Blanco (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Miklós Márton. Antonio Vázquez Blanco has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84982?usp=email ) Change subject: Split usbdev declarations to a separate header. ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > Apart from the typo LGTM. Updated. Thank you very much! Commit Message: https://review.coreboot.org/c/flashrom/+/84982/comment/e4a04b3c_3673681f?us… : PS1, Line 7: usbvev > usbdev typo? Acknowledged -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 2 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Nov 2024 18:08:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Miklós Márton <martonmiklosqdev(a)gmail.com>

1 0

[S] Change in flashrom[main]: Split usbdev declarations to a separate header.
by Antonio Vázquez Blanco (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Antonio Vázquez Blanco. Hello Miklós Márton, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84982?usp=email to look at the new patch set (#2). Change subject: Split usbdev declarations to a separate header. ...................................................................... Split usbdev declarations to a separate header. This is a simple refactor that aims to simplify maintenance and to clarify file dependency inside the project. Currently, most of the declarations reside in programmer.h making it difficult to really understand file dependency. Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> --- M dediprog.c M developerbox_spi.c M include/programmer.h A include/usbdev.h M stlinkv3_spi.c M usbdev.c 6 files changed, 34 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/84982/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 2 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>

1 0

[S] Change in flashrom[main]: Split usbvev declarations to a separate header.
by Miklós Márton (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Antonio Vázquez Blanco. Miklós Márton has posted comments on this change by Antonio Vázquez Blanco. ( https://review.coreboot.org/c/flashrom/+/84982?usp=email ) Change subject: Split usbvev declarations to a separate header. ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: Apart from the typo LGTM. Commit Message: https://review.coreboot.org/c/flashrom/+/84982/comment/cbc316aa_ce9c0c22?us… : PS1, Line 7: usbvev usbdev typo? -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 1 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Nov 2024 17:31:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: Split usbvev declarations to a separate header.
by Antonio Vázquez Blanco (Code Review) 03 Nov '24

03 Nov '24

Antonio Vázquez Blanco has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84982?usp=email ) Change subject: Split usbvev declarations to a separate header. ...................................................................... Split usbvev declarations to a separate header. This is a simple refactor that aims to simplify maintenance and to clarify file dependency inside the project. Currently, most of the declarations reside in programmer.h making it difficult to really understand file dependency. Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Signed-off-by: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> --- M dediprog.c M developerbox_spi.c M include/programmer.h A include/usbdev.h M stlinkv3_spi.c M usbdev.c 6 files changed, 34 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/84982/1 diff --git a/dediprog.c b/dediprog.c index aa3a1cf..11e7d6c 100644 --- a/dediprog.c +++ b/dediprog.c @@ -26,6 +26,7 @@ #include "chipdrivers.h" #include "programmer.h" #include "spi.h" +#include "usbdev.h" /* LIBUSB_CALL ensures the right calling conventions on libusb callbacks. * However, the macro is not defined everywhere. m( diff --git a/developerbox_spi.c b/developerbox_spi.c index 64b7e8a..67d3d68 100644 --- a/developerbox_spi.c +++ b/developerbox_spi.c @@ -35,6 +35,7 @@ #include <libusb.h> #include "programmer.h" #include "spi.h" +#include "usbdev.h" /* Bit positions for each pin. */ #define DEVELOPERBOX_SPI_SCK 0 diff --git a/include/programmer.h b/include/programmer.h index 5ed9c8a..babb0bc 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -526,13 +526,4 @@ (flash->chip->feature_bits & (FEATURE_4BA_ENTER | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_ENTER_EAR7)); } -/* usbdev.c */ -struct libusb_device_handle; -struct libusb_context; -struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( - struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno); -struct libusb_device_handle *usb_dev_get_by_vid_pid_number( - struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); - - #endif /* !__PROGRAMMER_H__ */ diff --git a/include/usbdev.h b/include/usbdev.h new file mode 100644 index 0000000..3bd64b3 --- /dev/null +++ b/include/usbdev.h @@ -0,0 +1,27 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2024 Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __USBDEV_H__ +#define __USBDEV_H__ 1 + +#include <libusb.h> + +struct libusb_device_handle *usb_dev_get_by_vid_pid_serial( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, const char *serialno); +struct libusb_device_handle *usb_dev_get_by_vid_pid_number( + struct libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num); + +#endif /* __USBDEV_H__ */ diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 29aa8d3..a5c2289 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -26,6 +26,7 @@ #include "flash.h" #include "programmer.h" #include "spi.h" +#include "usbdev.h" #include <libusb.h> #include <limits.h> diff --git a/usbdev.c b/usbdev.c index 846ed58..02627d8 100644 --- a/usbdev.c +++ b/usbdev.c @@ -15,11 +15,13 @@ * GNU General Public License for more details. */ + +#include "usbdev.h" + +#include "flash.h" // msg_perr, msg_pdbg... #include <inttypes.h> #include <stdbool.h> #include <string.h> -#include <libusb.h> -#include "programmer.h" /* * Check whether we should filter the current device. -- To view, visit https://review.coreboot.org/c/flashrom/+/84982?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9d819ea1c5bd51289d02189c1dff367ce6d25617 Gerrit-Change-Number: 84982 Gerrit-PatchSet: 1 Gerrit-Owner: Antonio Vázquez Blanco <antoniovazquezblanco(a)gmail.com>

1 0

[XS] Change in flashrom[main]: doc: Change link from gitiles to github
by Anastasia Klimchuk (Code Review) 03 Nov '24

03 Nov '24

Attention is currently required from: Peter Marheine. Hello Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84960?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: doc: Change link from gitiles to github ...................................................................... doc: Change link from gitiles to github gitiles are not always available, so the link was not always working, which could make readers confused. While we are here, add missing link to Dev Guide. Change-Id: I9103e5199bdf1b65fa3136aa01259a85e788a251 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/about_flashrom/team.rst 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/84960/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84960?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9103e5199bdf1b65fa3136aa01259a85e788a251 Gerrit-Change-Number: 84960 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Peter Marheine <pmarheine(a)chromium.org>

1 0

[L] Change in flashrom[main]: cli_client: Add rpmc command support
by Matti Finder (Code Review) 02 Nov '24

02 Nov '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84840?usp=email ) Change subject: cli_client: Add rpmc command support ...................................................................... Patch Set 8: (2 comments) File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/84840/comment/d67204e8_05fca319?us… : PS7, Line 329: : RPMC commands : ^^^^^^^^^^^^^ > Perhaps you can add a little intro which mentions that commands are available only if chip model sup […] Done File include/flash.h: https://review.coreboot.org/c/flashrom/+/84840/comment/fc5dc377_535149a4?us… : PS4, Line 581: rpmc_ctx > Ah, I think I understand now (also after reading CB:84934 with implementation). […] Yeah as of now the only way to use the rpmc comands is to set `-c "SFDP-capable chip"` manually. That is also what I did for testing. I think for vendors that still have enough ids to give special ones to rpmc capable devices, we can just or the FEATURE_FLASH_HARDENING bit and set the struct rpmc_ctx values accordingly. In the beginning I also thought about extending the probe function to also probe for rpmc capabilites or adding a new capability check to set feature bits. That seemed like a lot of hard to test changes, but it might be worth it for the future. But I'm not sure how the future of flashrom should look like. If maintaining a complicated flashchips.c is worth it, when most of the information can also be parsed from sfdp pages (for newer flashchips). -- To view, visit https://review.coreboot.org/c/flashrom/+/84840?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I36c823bbee65f256eb6edabe6f058321c9a0cfa1 Gerrit-Change-Number: 84840 Gerrit-PatchSet: 8 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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: Sat, 02 Nov 2024 16:35:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 02 Nov '24

02 Nov '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 4: (7 comments) Patchset: PS3: > Given that you agreed in the other comment, you can add yourself in MAINTAINERS file and include in […] Done File rpmc.c: https://review.coreboot.org/c/flashrom/+/84934/comment/1d16add1_cfaf8bb3?us… : PS3, Line 131: Unsupported > Maybe `Unknown` instead of `Unsupported`, because it's not even in our enum. Done https://review.coreboot.org/c/flashrom/+/84934/comment/1d4bee90_291806c2?us… : PS3, Line 223: { > You don't need { } when the body of if conditional is only one line. […] Done https://review.coreboot.org/c/flashrom/+/84934/comment/d9dba094_29df7b76?us… : PS3, Line 224: return -1; > I am adding comment here, as this is an example, but it applies to other code in this file. […] My original of how to handle the return values wasn't that good, and just lead to a lot of questions. I have now switched it over to using a rpmc_result enum. Let me know if that makes the return values better to understand File sfdp.c: https://review.coreboot.org/c/flashrom/+/84934/comment/b066ede5_2b8a2ec1?us… : PS3, Line 446: hdrs[i].id > Can these header IDs ever repeat from one header to the other? (probably not?) […] No these ids are meant to be unique. From how I understand the original code. We query the device which maps these tables somewhere in the flash memory. Since page 0 is mandatory it is always the first one that is read. The ones after that are device specific. So there is only a relation for the first one. https://review.coreboot.org/c/flashrom/+/84934/comment/064aff5a_cd306c1a?us… : PS3, Line 449: msg_cdbg("The chip contains an unknown " : "version of the JEDEC flash " : "parameters table, skipping it.\n"); > I know it was like this before, but while you are here... maybe you can add `hdrs[i]. […] Done https://review.coreboot.org/c/flashrom/+/84934/comment/4aded69f_151b7de2?us… : PS3, Line 461: 1 > It's in hex few lines above (0x01 on line 448) and here is decimal. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 4 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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: Sat, 02 Nov 2024 16:32:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: cli_client: Add rpmc command support
by Matti Finder (Code Review) 02 Nov '24

02 Nov '24

Attention is currently required from: Matti Finder, Peter Marheine. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84840?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: cli_client: Add rpmc command support ...................................................................... cli_client: Add rpmc command support This commit adds uses the new rpmc command implementation to add them as a new feature to the cli_client. Also adds the necessary documentation for this new feature. Tested on the Winbond W25R128JV as a 'SFDP-capable chip'. This patch was done to add rpmc command support to flashrom. This enables users to write root keys to their flash chips while they flash data on the chip. This might become useful in the future as rpmc support is extended in coreboot. Also adds debug tools to flashrom, which might be useful in implementing coreboots rpmc support. Change-Id: I36c823bbee65f256eb6edabe6f058321c9a0cfa1 Signed-off-by: Matti Finder <matti.finder(a)gmail.com> --- M cli_classic.c M doc/classic_cli_manpage.rst M doc/release_notes/devel.rst 3 files changed, 257 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/84840/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/84840?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I36c823bbee65f256eb6edabe6f058321c9a0cfa1 Gerrit-Change-Number: 84840 Gerrit-PatchSet: 8 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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: Matti Finder <matti.finder(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit