March 2023 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flashchips: Mark XMC XM25QH256C as TEST_OK_PR
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73362 ) Change subject: flashchips: Mark XMC XM25QH256C as TEST_OK_PR ...................................................................... flashchips: Mark XMC XM25QH256C as TEST_OK_PR As reported on the mailing list[0] this flashchip was successfully probed and read with an ch341a programmer. [0] https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/message/BFB… Change-Id: I68d0315f7b29f27ac84374ea7cc69dca207bbacb Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M flashchips.c 1 file changed, 16 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/73362/1 diff --git a/flashchips.c b/flashchips.c index c24e264..70e9fa9 100644 --- a/flashchips.c +++ b/flashchips.c @@ -20359,7 +20359,7 @@ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ | FEATURE_4BA_WRITE | FEATURE_WRSR2, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PR, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = -- To view, visit https://review.coreboot.org/c/flashrom/+/73362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68d0315f7b29f27ac84374ea7cc69dca207bbacb Gerrit-Change-Number: 73362 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: meson: revert to meson version 0.53.0
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73361 ) Change subject: meson: revert to meson version 0.53.0 ...................................................................... meson: revert to meson version 0.53.0 Debian Bullseye, current stable release, ships only with meson 0.63.2. Pass arguments to `sphinx-doc` via `'-Dkey=value` instead of environment variabled to stay with older meson version. Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M doc/conf.py M doc/meson.build M meson.build 3 files changed, 17 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/73361/1 diff --git a/doc/conf.py b/doc/conf.py index d06f538..79d8aae 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -11,7 +11,6 @@ project = 'flashrom' # copyright = '2023, The flashrom authors' author = 'The flashrom authors' -release = os.getenv('FLASHROM_VERSION') # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-confi… diff --git a/doc/meson.build b/doc/meson.build index bdf4493..8826e33 100644 --- a/doc/meson.build +++ b/doc/meson.build @@ -14,8 +14,7 @@ custom_target( 'man-pages', - command : [sphinx, '-b', 'man', '-q', '-d', '@PRIVATE_DIR@', '@CURRENT_SOURCE_DIR@', '@OUTDIR@'], - env : {'FLASHROM_VERSION' : meson.project_version() }, + command : [sphinx, '-b', 'man', '-q', '-d', '@PRIVATE_DIR@', '-Drelease=' + meson.project_version(),'@CURRENT_SOURCE_DIR@', '@OUTDIR@'], build_always_stale : true, # sphinx handles rebuilds output : man_outputs, install : true, @@ -26,8 +25,7 @@ if get_option('documentation').auto() or get_option('documtation').enabled() custom_target( 'documentation', - command : [sphinx, '-b', 'html', '-q', '-d', '@PRIVATE_DIR@', '@CURRENT_SOURCE_DIR@', '@OUTDIR@/html'], - env : {'FLASHROM_VERSION' : meson.project_version() }, + command : [sphinx, '-b', 'html', '-q', '-d', '@PRIVATE_DIR@', '-Drelease=' + meson.project_version(),'@CURRENT_SOURCE_DIR@', '@OUTDIR@/html'], build_always_stale : true, # sphinx handles rebuilds output : 'html', install : true, diff --git a/meson.build b/meson.build index caeea6e..2ecb5fa 100644 --- a/meson.build +++ b/meson.build @@ -1,7 +1,7 @@ project('flashromutils', 'c', version : run_command('util/getversion.sh', '--version', check : true).stdout().strip(), license : 'GPL-2.0', - meson_version : '>=0.57.0', + meson_version : '>=0.53.0', default_options : [ 'warning_level=2', 'c_std=c99', -- To view, visit https://review.coreboot.org/c/flashrom/+/73361 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff9b8307c741a247a652cf666935c9485fa493fa Gerrit-Change-Number: 73361 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

[L] Change in flashrom[master]: [WIP] doc: Add build instructions
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73359 ) Change subject: [WIP] doc: Add build instructions ...................................................................... [WIP] doc: Add build instructions Those build instructions are mostly based on `Documentation/building.md` and the wiki. Some parts, e.g. the DOS and cross-compiling sections, are not fully working with the master branch yet. There are `.. todo::` sections which can be solved at a later stage. Change-Id: I96771e98b313a6d26dd2be940ff37998d4124324 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- D Documentation/building.md A doc/building_from_source.rst M doc/conf.py M doc/index.rst 4 files changed, 248 insertions(+), 148 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/73359/1 diff --git a/Documentation/building.md b/Documentation/building.md deleted file mode 100644 index e5d9412..0000000 --- a/Documentation/building.md +++ /dev/null @@ -1,147 +0,0 @@ -# Compiling from Source with meson (recommended) - -## Dependencies: - - * C compiler (GCC | Clang) * - * meson >=0.53.0 * - * ninja * - * pkg-config * - * cmocka ** - * system-headers *** - * libpci *** - * libusb1 >=1.0.9 *** - * libftdi1 *** - * libjaylink *** - -\* Compile time dependency -\** For unit-testing only -\*** Runtime / Programmer specific - -## Build Options: - * classic_cli=auto/enabled/disabled - * classic_cli_default_programmer=<programmer_name>:<programmer_params> - * classic_cli_print_wiki=auto/enabled/disabled - * tests=auto/enabled/disabled - * ich_descriptors_tool=auto/enabled/disabled - * use_internal_dmi=true/false - * programmer=... - -## Configure -``` -meson builddir -D<your_options> -``` -- __builddir__ is the directory in which flashrom will be build -- for all available options see `meson_options.txt` - -## Compile -``` -ninja -C builddir -``` - -## Install -``` -ninja -C builddir install -``` - -## Run unit tests -``` -ninja -C builddir test -``` - -### Run unit tests with code coverage -#### gcov -Due to a bug in lcov, the html file will only be correct if lcov is not -installed and gcovr is installed. See -https://github.com/linux-test-project/lcov/issues/168 -https://github.com/mesonbuild/meson/issues/6747 -``` -meson setup buildcov -Db_coverage=true -ninja -C buildcov test -ninja -C buildcov coverage -``` - -#### llvm -https://clang.llvm.org/docs/SourceBasedCodeCoverage.html -``` -env CC=clang meson setup buildclangcov -Dllvm_cov=enabled -ninja -C buildclangcov test -ninja -C buildclangcov llvm-cov-tests -``` - -## System specific information -### Ubuntu / Debian (Linux) - * __linux-headers__ are version specific -``` -apt-get install -y gcc meson ninja-build pkg-config libcmocka-dev \ - linux-headers-generic libpci-dev libusb-1.0-0-dev libftdi1-dev \ - libjaylink-dev -``` - -### ArchLinux / Manjaro - * __libjaylink__ is not available through the package manager -``` -pacman -S --noconfirm gcc meson ninja pkg-config cmocka \ - pciutils libusb libftdi -``` - -### NixOS / Nixpkgs -``` -nix-shell <flashrom_source>/util/shell.nix -``` -or -``` -nix-shell -p meson ninja pkg-config cmocka pciutils libusb1 libftdi1 libjaylink -``` - -### OpenSUSE -``` -zypper install -y gcc meson ninja pkg-config libcmocka-devel \ - pciutils-devel libusb-1_0-devel libftdi1-devel \ - libjaylink-devel -``` - -### Alpine -``` -apk add build-base meson ninja pkgconf cmocka-dev pciutils-dev libusb-dev libftdi1-dev libjaylink-dev linux-headers -``` - - -### Freebsd / DragonFly BSD - * Tests are not working yet and must be disabled with `-Dtests=disabled` - * __libjaylink__ is not available through the package manager - * __libusb1__ is part of the base system -``` -pkg install pkgconf meson ninja cmocka libpci libftdi1 -``` - -### OpenBSD - * Tests are not working yet and must be disabled with `-Dtests=disabled` - * __libjaylink__ is not available through the package manager -``` -pkg_add install meson ninja pkg-config cmocka pciutils libusb1 libftdi1 -``` - -### NetBSD - * Tests are not working yet and must be disabled with `-Dtests=disabled` - * __libjaylink__ is not available through the package manager - * note: https://www.cambus.net/installing-ca-certificates-on-netbsd/ -``` -pkgin install meson ninja pkg-config cmocka pciutils libusb1 libftdi1 -``` - -### OpenIndiana (Illumos, Solaris, SunOS) - * Tests are not working yet and must be disabled with `-Dtests=disabled` - * __libpci__ missing, pciutils is build without it - * __libftdi1__, __libjaylink__ is not available through the package manager - * TODO: replace __build-essential__ with the default compiler -``` -pkg install build-essential meson ninja cmocka libusb-1 -``` - -### MacOS (Homebrew) - * Tests are not working yet and must be disabled with `-Dtests=disabled` - * Internal, PCI programmer not supported - * __libjaylink__ is not available through the package manager -``` -brew install meson ninja pkg-config cmocka libusb libftdi -``` diff --git a/doc/building_from_source.rst b/doc/building_from_source.rst new file mode 100644 index 0000000..f850c3d --- /dev/null +++ b/doc/building_from_source.rst @@ -0,0 +1,226 @@ +Building from Source +===================== + +You're going to need the following tools to get started: + +* gcc or clang +* meson +* ninja +* pkg-config +* sphinx-build* + +| \* optional, to build man-pages and html documentation + +And the following dependencies: + +* cmocka* +* linux-headers** +* libpci** +* libusb1** +* libftdi1** +* libjaylink** + +| \* optional, for building unit testing +| \** optional, depending on the selected programmer + +If you are cross compiling, install the dependencies for your target. + +TL;DR +---- +:: + + meson setup build + meson compile -C build + meson install -C build + + +Installing dependencies +----------------------- + +.. todo:: Move the bullet points to `tabs <https://www.w3schools.com/howto/howto_js_tabs.asp>`_ + + * No external dependencies (documentation should be build without fetching all of pypi) + * No Javascript? + +* Linux + * Debian / Ubuntu + :: + + apt-get install -y \ + gcc meson ninja-build pkg-config python3-sphinx \ + libcmocka-dev libpci-dev libusb-1.0-0-dev libftdi1-dev libjaylink-dev + + * ArchLinux / Manjaro + :: + + pacman -S --noconfirm \ + gcc meson ninja pkg-config python-sphinx cmocka \ + pciutils libusb libftdi libjaylink + + * openSUSE / SUSE + :: + + zypper install -y \ + gcc meson ninja pkg-config python3-Sphinx \ + libcmocka-devel pciutils-devel libusb-1_0-devel libftdi1-devel libjaylink-devel + + * NixOS / nixpkgs + * There is a ``shell.nix`` under ``scripts/`` + :: + + nix-shell -p \ + gcc meson ninja pkg-config sphinx \ + cmocka pciutils libusb1 libftdi1 libjaylink + + + * Alpine Linux + :: + + apk add \ + build-base meson ninja pkgconf py3-sphinx \ + cmocka-dev pciutils-dev libusb-dev libjaylink-dev + +* Windows + .. todo:: Add build instructions to build under MSYS2, CYGWIN or cross compiling from Linux + +* MacOS + * Homebrew + * ``libpci`` is not available through the package manager + * ``libjaylink`` is not available through the package manager + :: + + brew install \ + meson ninja pkg-config sphinx-doc \ + libusb libftdi + +* BSD + * FreeBSD / DragonFlyBSD + * ``libusb1`` is part of the system + * ``libjaylink`` is not available through the package manager + :: + + pkg install \ + meson ninja pkgconf py-sphinx \ + cmocka libpci libftdi + + * OpenBSD + * ``libjaylink`` is not available through the package manager + :: + + pkg_add install \ + meson ninja pkg-config py-sphinx\ + cmocka pciutils libusb1 libftdi1 + + * NetBSD + * ``libjaylink`` is not available through the package manager + * note: https://www.cambus.net/installing-ca-certificates-on-netbsd/ + :: + + pkgin install \ + meson ninja pkg-config py-sphinx \ + cmocka pciutils libusb1 libftdi1 + +* OpenIndiana (Illumos, Solaris, SunOS) + * ``libpci`` missing, pciutils is build without it + * ``libftdi1`` & ``libjaylink`` are not available through the package manager + * TODO: replace __build-essential__ with the default compiler + :: + + pkg install build-essential meson ninja cmocka libusb-1 + + +* DJGPP-DOS + * Get `DJGPP <https://www.delorie.com/djgpp/>`_ + * A great build script can be found `here <https://github.com/andrewwutw/build-djgpp>`_ + * Download the `pciutils <https://mj.ucw.cz/sw/pciutils/>`_ sources + | Run the following commands in the the pciutils directory to build libpci for DOS. + | Replace ``<DOS_INSTALL_ROOT>`` with your cross-compile install root. + :: + + make install-lib \ + ZLIB=no \ + DNS=no \ + HOST=i386-djgpp-djgpp \ + CROSS_COMPILE=i586-pc-msdosdjgpp- \ + STRIP="--strip-program=i586-pc-msdosdjgpp-strip -s" \ + PREFIX=<DOS_INSTALL_ROOT> + + Point pkg-config to the ``<DOS_INSTALL_ROOT>`` :: + + export PKG_CONFIG_SYSROOT=<DOS_INSTALL_ROOT> + + * To compile flashrom use the ``meson_cross/i586_djgpp_dos.txt`` cross-file + * You will need `CWSDPMI.EXE <https://sandmann.dotster.com/cwsdpmi/>`_ to run flashrom + +* libpayload + .. todo:: Add building instructions for libpayload + + +Configuration +------------- +In the flashrom repository run:: + + meson setup [builtin options] [flashrom options] <builddir> + +Mesons ``[builtin options]`` can be displayed with ``meson setup --help``. +The flashrom specific options can be found in ``meson_options.txt`` in the top-level +directory of flashrom and are used like in cmake with ``-Doption=value`` + +.. todo:: Write a sphinx extension to render ``meson_options.txt`` here + + +Configuration for Crossbuilds +----------------------------- +Flashrom specific ross-files can be found in the ``meson_cross`` folder. +To use them run:: + + meson setup --cross-file <path/to/crossfile> [builtin options] [flashrom options] <builddir> + +The options are the same as the normal configuration options. For more information see +https://mesonbuild.com/Cross-compilation.html + + +Compiling +--------- +Run:: + + meson compile -C <builddir> + + +Reconfiguration +--------------- +If you are not happy with the initial configuration, e.g. a programmer is missing, +or you want to review it, run:: + + meson configure [updated builtin options] [updated flashrom options] + + +Unit Tests +---------- +To execute the unit tests run:: + + meson test -C <builddir> + +You will get a summary of the unit test results at the end. + +To get a code coverage report install ``gcovr`` for GCC or ``lcov`` and ``gen-html`` for LLVM +and reconfigure the build with ``-Db_coverage=true``. Run:: + + ninja -C <builddir> coverage + +For more information see `the meson documentation <https://mesonbuild.com/Unit-tests.html#coverage>`_ + +Installing +---------- +Run:: + + meson install -C <builddir> + +This will install flashrom under the PREFIX selected in the configuration phase. Default is ``/usr/local``. + + +Create distribution package +--------------------------- +To create a distribution tarball from your <builddir>, run:: + + meson dist -C <builddir> diff --git a/doc/conf.py b/doc/conf.py index d06f538..42bda1f 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -18,11 +18,15 @@ master_doc = 'index' # this is needed for old versions -extensions = [] +extensions = [ + 'sphinx.ext.todo' +] #templates_path = ['_templates'] exclude_patterns = [] +todo_include_todos = True + # -- Options for HTML output ------------------------------------------------- diff --git a/doc/index.rst b/doc/index.rst index d309a03..c12a321 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -6,6 +6,7 @@ .. toctree:: :hidden: + building_from_source classic_cli_manpage .. include:: ../README -- To view, visit https://review.coreboot.org/c/flashrom/+/73359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96771e98b313a6d26dd2be940ff37998d4124324 Gerrit-Change-Number: 73359 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: libflashrom: Provide flashrom_flash_getname() API
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73286 ) Change subject: libflashrom: Provide flashrom_flash_getname() API ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: In the API we have `struct flashrom_flashchip_info` and `flashrom_supported_flash_chips()`. Why not making a function `struct flashrom_flashchip_info *flashrom_get_active_flashchip(struct flashrom_flashctx*)`, which reuses parts of the existing API. -- To view, visit https://review.coreboot.org/c/flashrom/+/73286 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3dc32e261e6d54230d93f06b91f723811792112a Gerrit-Change-Number: 73286 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 01 Mar 2023 10:03:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dummyflasher: Add basic WP support for opaque VARIABLE_SIZE chip
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Attention is currently required from: Nikolai Artemiev. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73289 ) Change subject: dummyflasher: Add basic WP support for opaque VARIABLE_SIZE chip ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/73289/comment/9f2d7b55_8d46705b PS2, Line 9: Since VARIABLE_SIZE emulated chips do not correspond to actual flash chip models, no active protection modes are supported: Please use a line break here -- To view, visit https://review.coreboot.org/c/flashrom/+/73289 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4348e0175b8c743365904f5e61fdb69e3f4f4db5 Gerrit-Change-Number: 73289 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 01 Mar 2023 09:56:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Reduce the prog global state machine
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70603 ) Change subject: flashrom.c: Reduce the prog global state machine ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: > Unfortunately not as the call sites do not have access to the prog but rather the inner flashctx. […] Good idea with the video chat to get all onto the same page! I'm also not happy with the name of the variable, but it's not a deal breaker. -- To view, visit https://review.coreboot.org/c/flashrom/+/70603 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I93dfe09fe877acd32c4f22665921544ccd9323f6 Gerrit-Change-Number: 70603 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 01 Mar 2023 09:51:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Drop default_spi_probe_opcode for NULL case
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Attention is currently required from: Edward O'Callaghan, Arthur Heymans, Aarya, Anastasia Klimchuk, Nicholas Chin. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70264 ) Change subject: tree/: Drop default_spi_probe_opcode for NULL case ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70264 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12 Gerrit-Change-Number: 70264 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> 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-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 01 Mar 2023 09:45:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: spi: Make default cmd helpers static internal
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73337 ) Change subject: spi: Make default cmd helpers static internal ...................................................................... Patch Set 1: Code-Review+2 -- 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-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-Comment-Date: Wed, 01 Mar 2023 09:44:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: spi: Make 'default_spi_send_multicommand' the default unless defined
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67481 ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67481 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> --- 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, 29 insertions(+), 30 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved diff --git a/asm106x.c b/asm106x.c index fe35f94..78c0b74 100644 --- a/asm106x.c +++ b/asm106x.c @@ -123,7 +123,6 @@ .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = asm106x_command, .shutdown = asm106x_shutdown, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .probe_opcode = default_spi_probe_opcode, diff --git a/bitbang_spi.c b/bitbang_spi.c index 7919e45..6488650 100644 --- a/bitbang_spi.c +++ b/bitbang_spi.c @@ -143,7 +143,6 @@ .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = bitbang_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = bitbang_spi_shutdown, diff --git a/buspirate_spi.c b/buspirate_spi.c index a40fb70..192d274 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -178,7 +178,6 @@ .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, .command = NULL, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = buspirate_spi_shutdown, diff --git a/ch341a_spi.c b/ch341a_spi.c index 75eaf86..b9be18e 100644 --- a/ch341a_spi.c +++ b/ch341a_spi.c @@ -413,7 +413,6 @@ .max_data_read = 4 * 1024, .max_data_write = 4 * 1024, .command = ch341a_spi_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = ch341a_spi_shutdown, diff --git a/ch347_spi.c b/ch347_spi.c index c3326b0..e828f00 100644 --- a/ch347_spi.c +++ b/ch347_spi.c @@ -253,7 +253,6 @@ .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = ch347_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, diff --git a/dediprog.c b/dediprog.c index 723f4a8..60ce6e6 100644 --- a/dediprog.c +++ b/dediprog.c @@ -1034,7 +1034,6 @@ .max_data_read = 16, /* 18 seems to work fine as well, but 19 times out sometimes with FW 5.15. */ .max_data_write = 16, .command = dediprog_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = dediprog_spi_read, .write_256 = dediprog_spi_write_256, .write_aai = dediprog_spi_write_aai, diff --git a/digilent_spi.c b/digilent_spi.c index 55cb6ec..5fa60b9 100644 --- a/digilent_spi.c +++ b/digilent_spi.c @@ -333,7 +333,6 @@ .max_data_read = 252, .max_data_write = 252, .command = digilent_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = digilent_spi_shutdown, diff --git a/dirtyjtag_spi.c b/dirtyjtag_spi.c index 42169ed..9f0e43b 100644 --- a/dirtyjtag_spi.c +++ b/dirtyjtag_spi.c @@ -193,7 +193,6 @@ .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = dirtyjtag_spi_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .write_aai = default_spi_write_aai, diff --git a/dummyflasher.c b/dummyflasher.c index 0897332..a68889b 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -939,7 +939,6 @@ .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_UNSPECIFIED, .command = dummy_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = dummy_spi_write_256, .shutdown = dummy_shutdown, diff --git a/it87spi.c b/it87spi.c index 34dbea0..b2da875 100644 --- a/it87spi.c +++ b/it87spi.c @@ -314,7 +314,6 @@ .max_data_read = 3, .max_data_write = MAX_DATA_UNSPECIFIED, .command = it8716f_spi_send_command, - .multicommand = default_spi_send_multicommand, .map_flash_region = physmap, .unmap_flash_region = physunmap, .read = it8716f_spi_chip_read, diff --git a/jlink_spi.c b/jlink_spi.c index ad2ca78..7743c40 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -184,7 +184,6 @@ /* Maximum data write size in one go (excluding opcode+address). */ .max_data_write = JTAG_MAX_TRANSFER_SIZE - 5, .command = jlink_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .features = SPI_MASTER_4BA, diff --git a/linux_spi.c b/linux_spi.c index d65b3e8..d849fe4 100644 --- a/linux_spi.c +++ b/linux_spi.c @@ -116,7 +116,6 @@ .max_data_read = MAX_DATA_UNSPECIFIED, /* TODO? */ .max_data_write = MAX_DATA_UNSPECIFIED, /* TODO? */ .command = linux_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = linux_spi_read, .write_256 = linux_spi_write_256, .shutdown = linux_spi_shutdown, diff --git a/mediatek_i2c_spi.c b/mediatek_i2c_spi.c index c15df05..e9c3c8c 100644 --- a/mediatek_i2c_spi.c +++ b/mediatek_i2c_spi.c @@ -456,7 +456,6 @@ // Leave room for 1-byte command and up to a 4-byte address. .max_data_write = I2C_SMBUS_BLOCK_MAX - 5, .command = mediatek_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = mediatek_shutdown, diff --git a/mstarddc_spi.c b/mstarddc_spi.c index bd65281..1172cb2 100644 --- a/mstarddc_spi.c +++ b/mstarddc_spi.c @@ -142,7 +142,6 @@ .max_data_read = 256, .max_data_write = 256, .command = mstarddc_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = mstarddc_spi_shutdown, diff --git a/ni845x_spi.c b/ni845x_spi.c index fd5057d..dec6f6f 100644 --- a/ni845x_spi.c +++ b/ni845x_spi.c @@ -532,7 +532,6 @@ .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = ni845x_spi_transmit, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = ni845x_spi_shutdown, diff --git a/parade_lspcon.c b/parade_lspcon.c index a50346e..8b92789 100644 --- a/parade_lspcon.c +++ b/parade_lspcon.c @@ -432,7 +432,6 @@ .max_data_read = 16, .max_data_write = 12, .command = parade_lspcon_send_command, - .multicommand = default_spi_send_multicommand, .read = parade_lspcon_read, .write_256 = parade_lspcon_write_256, .write_aai = parade_lspcon_write_aai, diff --git a/pickit2_spi.c b/pickit2_spi.c index 86c5403..56894d9 100644 --- a/pickit2_spi.c +++ b/pickit2_spi.c @@ -380,7 +380,6 @@ .max_data_read = 40, .max_data_write = 40, .command = pickit2_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = pickit2_shutdown, diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index d1a09d7..5111ae9 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -1320,7 +1320,6 @@ .max_data_read = 0, .max_data_write = 0, .command = NULL, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = raiden_debug_spi_shutdown, diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index 2a5d5ed..ee6390c 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -436,7 +436,6 @@ .max_data_read = 16, .max_data_write = 8, .command = realtek_mst_i2c_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = realtek_mst_i2c_spi_read, .write_256 = realtek_mst_i2c_spi_write_256, .write_aai = realtek_mst_i2c_spi_write_aai, diff --git a/sb600spi.c b/sb600spi.c index 5b9ac45..2e6c2ec 100644 --- a/sb600spi.c +++ b/sb600spi.c @@ -599,7 +599,6 @@ .max_data_read = FIFO_SIZE_OLD, .max_data_write = FIFO_SIZE_OLD - 3, .command = sb600_spi_send_command, - .multicommand = default_spi_send_multicommand, .map_flash_region = physmap, .unmap_flash_region = physunmap, .read = default_spi_read, @@ -612,7 +611,6 @@ .max_data_read = FIFO_SIZE_YANGTZE - 3, /* Apparently the big SPI 100 buffer is not a ring buffer. */ .max_data_write = FIFO_SIZE_YANGTZE - 3, .command = spi100_spi_send_command, - .multicommand = default_spi_send_multicommand, .map_flash_region = physmap, .unmap_flash_region = physunmap, .read = default_spi_read, @@ -625,7 +623,6 @@ .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = FIFO_SIZE_YANGTZE - 3, .command = spi100_spi_send_command, - .multicommand = default_spi_send_multicommand, .map_flash_region = physmap, .unmap_flash_region = physunmap, .read = promontory_read_memmapped, diff --git a/serprog.c b/serprog.c index 5ddbe3d..4d93e07 100644 --- a/serprog.c +++ b/serprog.c @@ -461,7 +461,6 @@ .max_data_read = MAX_DATA_READ_UNLIMITED, .max_data_write = MAX_DATA_WRITE_UNLIMITED, .command = serprog_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .probe_opcode = default_spi_probe_opcode, diff --git a/spi.c b/spi.c index ca27cbb..4fe0fa5 100644 --- a/spi.c +++ b/spi.c @@ -37,7 +37,9 @@ int spi_send_multicommand(const struct flashctx *flash, struct spi_command *cmds) { - return flash->mst->spi.multicommand(flash, 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, @@ -153,10 +155,9 @@ } } - if (!mst->write_256 || !mst->read || - !mst->multicommand || !mst->probe_opcode || + 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 == default_spi_send_multicommand || !mst->multicommand))) { msg_perr("%s called with incomplete master definition. " "Please report a bug at flashrom(a)flashrom.org\n", __func__); diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index 16a66b4..726cdcf 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -466,7 +466,6 @@ .max_data_read = UINT16_MAX, .max_data_write = UINT16_MAX, .command = stlinkv3_spi_transmit, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = stlinkv3_spi_shutdown, diff --git a/usbblaster_spi.c b/usbblaster_spi.c index 75d5be7..958d4ea 100644 --- a/usbblaster_spi.c +++ b/usbblaster_spi.c @@ -168,7 +168,6 @@ .max_data_read = 256, .max_data_write = 256, .command = usbblaster_spi_send_command, - .multicommand = default_spi_send_multicommand, .read = default_spi_read, .write_256 = default_spi_write_256, .shutdown = usbblaster_shutdown, diff --git a/wbsio_spi.c b/wbsio_spi.c index d635a09..ee79ef2 100644 --- a/wbsio_spi.c +++ b/wbsio_spi.c @@ -186,7 +186,6 @@ .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, .command = wbsio_spi_send_command, - .multicommand = default_spi_send_multicommand, .map_flash_region = physmap, .unmap_flash_region = physunmap, .read = wbsio_spi_read, -- 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: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> 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-MessageType: merged

1 0

[M] Change in flashrom[master]: spi: Make 'default_spi_send_multicommand' the default unless defined
by Thomas Heijligen (Code Review) 01 Mar '23

01 Mar '23

Attention is currently required from: Edward O'Callaghan, Arthur Heymans, Nicholas Chin. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67481 ) Change subject: spi: Make 'default_spi_send_multicommand' the default unless defined ...................................................................... Patch Set 3: Code-Review+2 -- 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> 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-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Wed, 01 Mar 2023 09:43:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2023