- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: Improve IO permission error messages
by Anastasia Klimchuk (Code Review) 24 May '23

24 May '23

Attention is currently required from: Miklós Márton, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62878 ) Change subject: Improve IO permission error messages ...................................................................... Patch Set 2: Code-Review+1 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/62878/comment/6dfb9196_c7a076a6 PS2, Line 7: Improve IO permission error messages We typically add prefix to commit title, so here it can be `hwaccess_x86_io:<the rest of title>` https://review.coreboot.org/c/flashrom/+/62878/comment/b7eb4333_04e2cb67 PS2, Line 9: - Display the BSD hints only when compiled for a specific BSD : - On Linux check the user's uid to see if flashrom run with root privileges : - Add a note about the dmesg check if the flashrom run as root and have no : IO privilege Seems like lines are longer than 72 chars and wrap. We have max line length 72 chars for commit message. Patchset: PS2: Thanks for the patch! I am wondering if you can add testing info in commit message? (if you did testing) -- To view, visit https://review.coreboot.org/c/flashrom/+/62878 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6a6f60a5f0ac8f2b51c74661f7dad30571819680 Gerrit-Change-Number: 62878 Gerrit-PatchSet: 2 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 24 May 2023 12:31:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: README: Extract instructions for meson and make into separate docs
by Anastasia Klimchuk (Code Review) 23 May '23

23 May '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/75125 ) Change subject: README: Extract instructions for meson and make into separate docs ...................................................................... README: Extract instructions for meson and make into separate docs This patch extracts building/installing/packaging documentation for meson and make into two separate doc files, and then links these files from README. Re-structure README so that it gives only a brief overview of build instructions and links to full instructions for meson and make. Ticket: https://ticket.coreboot.org/issues/489 Change-Id: I2d5900538d54c43efcc8c5b7010df5d867f3b190 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/75125 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M README.rst M doc/dev_guide/building_from_source.rst A doc/dev_guide/building_with_make.rst M doc/dev_guide/index.rst 4 files changed, 240 insertions(+), 180 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/README.rst b/README.rst index ff73341..428ae57 100644 --- a/README.rst +++ b/README.rst @@ -20,193 +20,33 @@ Please see the flashrom(8) manpage :doc:`classic_cli_manpage`. -Build Instructions ------------------- +Building / installing / packaging +--------------------------------- flashrom supports building with **make** and **meson**. -Meson build system supports almost all the environments, although not exactly -all of them. Full meson support is on the roadmap in the nearest future. -To build flashrom with meson, follow the instruction and information in +TLDR, building with meson +""""""""""""""""""""""""" + +:: + + meson setup builddir + meson compile -C builddir + meson install -C builddir + +For full detailed instructions, follow the information in :doc:`dev_guide/building_from_source` -If you are unsure which build system to use, and/or don't know what's the -difference, use make for now. +TLDR, building with make +"""""""""""""""""""""""" -The rest of Build Instructions below refers to building flashrom with make. - -**To build flashrom you need to install the following software:** - - * C compiler (GCC / clang) - * pkg-config - - * pciutils+libpci (if you want support for mainboard or PCI device flashing) - * libusb (if you want FT2232, Dediprog or USB-Blaster support) - * libftdi (if you want FT2232 or USB-Blaster support) - * libjaylink (if you want support for SEGGER J-Link and compatible devices) - -**Linux et al:** - - * pciutils / libpci - * pciutils-devel / pciutils-dev / libpci-dev - * zlib-devel / zlib1g-dev (needed if libpci was compiled with libz support) - -**On FreeBSD, you need the following ports:** - - * devel/gmake - * devel/libpci - -**On OpenBSD, you need the following ports:** - - * devel/gmake - * sysutils/pciutils - -**To compile on Linux, use**:: +:: make - -**To compile on FreeBSD, OpenBSD or DragonFly BSD, use**:: - - gmake - -**To compile on Nexenta, use**:: - - make - -**To compile on Solaris, use**:: - - gmake LDFLAGS="-L$pathtolibpci" CC="gcc -I$pathtopciheaders" CFLAGS=-O2 - -**To compile on NetBSD (with pciutils, libftdi, libusb installed in /usr/pkg/), use**:: - - gmake - -**To compile and run on Darwin/Mac OS X:** - -Install DirectHW from coresystems GmbH. -DirectHW is available at https://www.coreboot.org/DirectHW . - -**To cross-compile on Linux for DOS:** - -Get packages of the DJGPP cross compiler and install them: - - * djgpp-filesystem djgpp-gcc djgpp-cpp djgpp-runtime djgpp-binutils - -As an alternative, the DJGPP web site offers packages for download as well: - - * djcross-binutils-2.29.1-1ap.x86_64.rpm - * djcross-gcc-7.2.0-1ap.x86_64.rpm - * djcrx-2.05-5.x86_64.rpm - -The cross toolchain packages for your distribution may have slightly different -names (look for packages named *djgpp*). - -Alternatively, you could use a script to build it from scratch: -https://github.com/andrewwutw/build-djgpp - -You will need the libpci and libgetopt library source trees and -their compiled static libraries and header files installed in some -directory say libpci-libgetopt/, which will be later specified with -LIBS_BASE parameter during flashrom compilation. Easiest way to -handle it is to put pciutils, libgetopt and flashrom directories -in one subdirectory. There will be an extra subdirectory libpci-libgetopt -created, which will contain compiled libpci and libgetopt. - -Download pciutils 3.5.6 and apply https://flashrom.org/File:Pciutils-3.5.6.patch.gz -Compile pciutils, using following command line:: - - make ZLIB=no DNS=no HOST=i386-djgpp-djgpp CROSS_COMPILE=i586-pc-msdosdjgpp- \ - PREFIX=/ DESTDIR=$PWD/../libpci-libgetopt \ - STRIP="--strip-program=i586-pc-msdosdjgpp-strip -s" install install-lib - -Download and compile with 'make' https://flashrom.org/File:Libgetopt.tar.gz - -Copy the libgetopt.a to ../libpci-libgetopt/lib and -getopt.h to ../libpci-libgetopt/include - -Enter the flashrom directory:: - - make CC=i586-pc-msdosdjgpp-gcc STRIP=i586-pc-msdosdjgpp-strip LIBS_BASE=../libpci-libgetopt/ strip - -If you like, you can compress the resulting executable with UPX:: - - upx -9 flashrom.exe - -To run flashrom.exe, download https://flashrom.org/File:Csdpmi7b.zip and -unpack CWSDPMI.EXE into the current directory or one in PATH. - -**To cross-compile on Linux for Windows:** - -Get packages of the MinGW cross compiler and install them:: - - mingw32-filesystem mingw32-cross-cpp mingw32-cross-binutils mingw32-cross-gcc - mingw32-runtime mingw32-headers - -The cross toolchain packages for your distribution may have slightly different -names (look for packages named *mingw*). -PCI-based programmers (internal etc.) are not supported on Windows. -Run (change CC= and STRIP= settings where appropriate):: - - make CC=i686-w64-mingw32-gcc STRIP=i686-w64-mingw32-strip - -**Processor architecture dependent features:** - -On non-x86 architectures a few programmers don't work (yet) because they -use port-based I/O which is not directly available on non-x86. Those -programmers will be disabled automatically if you run "make". - -**Compiler quirks:** - -If you are using clang and if you want to enable only one driver, you may hit an -overzealous compiler warning from clang. Compile with "make WARNERROR=no" to -force it to continue and enjoy. - -**Bindings:** - -Foreign function interface bindings for the rust language are included in the -bindings folder. These are not compiled as part of the normal build process. -See the readme under bindings/rust for more information. - - -Installation ------------- - -In order to install flashrom and the manpage into /usr/local, type:: - make install -For installation in a different directory use DESTDIR, e.g. like this:: - - make DESTDIR=/usr install - -If you have insufficient permissions for the destination directory, use sudo -by adding sudo in front of the commands above. - - -Packaging ---------- - -To package flashrom and remove dependencies on Git, either use:: - - make export - -or:: - - make tarball - -``make export`` will export all flashrom files from the Git repository at -revision HEAD into a directory named ``$EXPORTDIR/flashrom-$RELEASENAME`` -and will additionally add a ``versioninfo.inc`` file in that directory to -contain the Git revision of the exported tree and a date for the manual -page. - -``make tarball`` will simply tar up the result of make export and compress -it with bzip2. - -The snapshot tarballs are the result of ``make tarball`` and require no -further processing. Some git files (for example the rust bindings) are omitted -from the tarball, as controlled by the .gitattributes files. - +For full detailed instructions, follow the information in +:doc:`dev_guide/building_with_make` Contact ------- diff --git a/doc/dev_guide/building_from_source.rst b/doc/dev_guide/building_from_source.rst index 42a16f0..9c2959e 100644 --- a/doc/dev_guide/building_from_source.rst +++ b/doc/dev_guide/building_from_source.rst @@ -240,15 +240,27 @@ Installing ---------- -Run:: +To install flashrom and documentation, run:: meson install -C <builddir> This will install flashrom under the PREFIX selected in the configuration phase. Default is ``/usr/local``. +To install into a different directory use DESTDIR, like this:: + + DESTDIR=/your/destination/directory meson install -C <your_build_dir> + +You can also set the prefix during configuration with:: + + meson setup --prefix <DESTDIR> <your_build_dir> Create distribution package --------------------------- -To create a distribution tarball from your <builddir>, run:: +To create a distribution tarball from your ``builddir``, run:: meson dist -C <builddir> + +This will collect all git tracked files and pack them into an archive. + +Current flashrom version is in the VERSION file. To release a new flashrom +version you need to change VERSION file and tag the changing commit. diff --git a/doc/dev_guide/building_with_make.rst b/doc/dev_guide/building_with_make.rst new file mode 100644 index 0000000..90a3e46 --- /dev/null +++ b/doc/dev_guide/building_with_make.rst @@ -0,0 +1,185 @@ +Building with make +================== + +TLDR +---- + +:: + + make + make install + +Build instructions +------------------ + +**To build flashrom you need to install the following software:** + + * C compiler (GCC / clang) + * pkg-config + + * pciutils+libpci (if you want support for mainboard or PCI device flashing) + * libusb (if you want FT2232, Dediprog or USB-Blaster support) + * libftdi (if you want FT2232 or USB-Blaster support) + * libjaylink (if you want support for SEGGER J-Link and compatible devices) + +**Linux et al:** + + * pciutils / libpci + * pciutils-devel / pciutils-dev / libpci-dev + * zlib-devel / zlib1g-dev (needed if libpci was compiled with libz support) + +**On FreeBSD, you need the following ports:** + + * devel/gmake + * devel/libpci + +**On OpenBSD, you need the following ports:** + + * devel/gmake + * sysutils/pciutils + +**To compile on Linux, use**:: + + make + +**To compile on FreeBSD, OpenBSD or DragonFly BSD, use**:: + + gmake + +**To compile on Nexenta, use**:: + + make + +**To compile on Solaris, use**:: + + gmake LDFLAGS="-L$pathtolibpci" CC="gcc -I$pathtopciheaders" CFLAGS=-O2 + +**To compile on NetBSD (with pciutils, libftdi, libusb installed in /usr/pkg/), use**:: + + gmake + +**To compile and run on Darwin/Mac OS X:** + +Install DirectHW from coresystems GmbH. +DirectHW is available at https://www.coreboot.org/DirectHW . + +**To cross-compile on Linux for DOS:** + +Get packages of the DJGPP cross compiler and install them: + + * djgpp-filesystem djgpp-gcc djgpp-cpp djgpp-runtime djgpp-binutils + +As an alternative, the DJGPP web site offers packages for download as well: + + * djcross-binutils-2.29.1-1ap.x86_64.rpm + * djcross-gcc-7.2.0-1ap.x86_64.rpm + * djcrx-2.05-5.x86_64.rpm + +The cross toolchain packages for your distribution may have slightly different +names (look for packages named *djgpp*). + +Alternatively, you could use a script to build it from scratch: +https://github.com/andrewwutw/build-djgpp + +You will need the libpci and libgetopt library source trees and +their compiled static libraries and header files installed in some +directory say libpci-libgetopt/, which will be later specified with +LIBS_BASE parameter during flashrom compilation. Easiest way to +handle it is to put pciutils, libgetopt and flashrom directories +in one subdirectory. There will be an extra subdirectory libpci-libgetopt +created, which will contain compiled libpci and libgetopt. + +Download pciutils 3.5.6 and apply https://flashrom.org/File:Pciutils-3.5.6.patch.gz +Compile pciutils, using following command line:: + + make ZLIB=no DNS=no HOST=i386-djgpp-djgpp CROSS_COMPILE=i586-pc-msdosdjgpp- \ + PREFIX=/ DESTDIR=$PWD/../libpci-libgetopt \ + STRIP="--strip-program=i586-pc-msdosdjgpp-strip -s" install install-lib + +Download and compile with 'make' https://flashrom.org/File:Libgetopt.tar.gz + +Copy the libgetopt.a to ../libpci-libgetopt/lib and +getopt.h to ../libpci-libgetopt/include + +Enter the flashrom directory:: + + make CC=i586-pc-msdosdjgpp-gcc STRIP=i586-pc-msdosdjgpp-strip LIBS_BASE=../libpci-libgetopt/ strip + +If you like, you can compress the resulting executable with UPX:: + + upx -9 flashrom.exe + +To run flashrom.exe, download https://flashrom.org/File:Csdpmi7b.zip and +unpack CWSDPMI.EXE into the current directory or one in PATH. + +**To cross-compile on Linux for Windows:** + +Get packages of the MinGW cross compiler and install them:: + + mingw32-filesystem mingw32-cross-cpp mingw32-cross-binutils mingw32-cross-gcc + mingw32-runtime mingw32-headers + +The cross toolchain packages for your distribution may have slightly different +names (look for packages named *mingw*). +PCI-based programmers (internal etc.) are not supported on Windows. +Run (change CC= and STRIP= settings where appropriate):: + + make CC=i686-w64-mingw32-gcc STRIP=i686-w64-mingw32-strip + +**Processor architecture dependent features:** + +On non-x86 architectures a few programmers don't work (yet) because they +use port-based I/O which is not directly available on non-x86. Those +programmers will be disabled automatically if you run "make". + +**Compiler quirks:** + +If you are using clang and if you want to enable only one driver, you may hit an +overzealous compiler warning from clang. Compile with "make WARNERROR=no" to +force it to continue and enjoy. + +**Bindings:** + +Foreign function interface bindings for the rust language are included in the +bindings folder. These are not compiled as part of the normal build process. +See the readme under bindings/rust for more information. + + +Installation +------------ + +In order to install flashrom and the manpage into /usr/local, type:: + + make install + +For installation in a different directory use DESTDIR, e.g. like this:: + + make DESTDIR=/usr install + +If you have insufficient permissions for the destination directory, use sudo +by adding sudo in front of the commands above. + + +Packaging +--------- + +To package flashrom and remove dependencies on Git, either use:: + + make export + +or:: + + make tarball + +``make export`` will export all flashrom files from the Git repository at +revision HEAD into a directory named ``$EXPORTDIR/flashrom-$RELEASENAME`` +and will additionally add a ``versioninfo.inc`` file in that directory to +contain the Git revision of the exported tree and a date for the manual +page. + +``make tarball`` will simply tar up the result of make export and compress +it with bzip2. + +The snapshot tarballs are the result of ``make tarball`` and require no +further processing. Some git files (for example the rust bindings) are omitted +from the tarball, as controlled by the .gitattributes files. diff --git a/doc/dev_guide/index.rst b/doc/dev_guide/index.rst index 9c6c0b1..b540127 100644 --- a/doc/dev_guide/index.rst +++ b/doc/dev_guide/index.rst @@ -4,4 +4,5 @@ .. toctree:: :maxdepth: 1 - building_from_source \ No newline at end of file + building_from_source + building_with_make -- To view, visit https://review.coreboot.org/c/flashrom/+/75125 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d5900538d54c43efcc8c5b7010df5d867f3b190 Gerrit-Change-Number: 75125 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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-MessageType: merged

1 0

[M] Change in flashrom[master]: util: Add docker to automate creation of documentation
by Anastasia Klimchuk (Code Review) 23 May '23

23 May '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/75344 ) Change subject: util: Add docker to automate creation of documentation ...................................................................... util: Add docker to automate creation of documentation Build with: docker build --force-rm -t "doc.flashrom.org" \ "$PWD/util/docker/flashrom.org/" Generate HTML files: mkdir -p "$PWD/doc/_build/" docker run -it --rm \ --user "$(id -u):$(id -g)" \ -v "$PWD/:/data-in/:ro" \ -v "$PWD/doc/_build/:/data-out/" \ doc.flashrom.org Live web server: docker run -it --rm \ --net=host -v "$PWD/:/data-in/:ro" \ doc.flashrom.org livehtml Change-Id: I889fa5f98e2fd6ef896ba4bb30aa5d985ff82622 Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/75344 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A util/docker/flashrom.org/Dockerfile A util/docker/flashrom.org/README.md A util/docker/flashrom.org/ditaa.sh A util/docker/flashrom.org/makeSphinx.sh 4 files changed, 106 insertions(+), 0 deletions(-) Approvals: Anastasia Klimchuk: Verified; Looks good to me, approved diff --git a/util/docker/flashrom.org/Dockerfile b/util/docker/flashrom.org/Dockerfile new file mode 100644 index 0000000..23f5f50 --- /dev/null +++ b/util/docker/flashrom.org/Dockerfile @@ -0,0 +1,29 @@ +FROM alpine:3.8 + +COPY makeSphinx.sh /makeSphinx.sh + +ADD https://sourceforge.net/projects/ditaa/files/ditaa/0.9/ditaa0_9.zip/download /tmp/ditaa.zip + +RUN apk add --no-cache python3 make bash git openjdk8-jre ttf-dejavu fontconfig \ + && pip3 install --upgrade --no-cache-dir pip \ + && pip3 install --no-cache-dir \ + sphinx===1.8.3 \ + sphinx_rtd_theme===0.4.2 \ + recommonmark===0.5.0 \ + sphinx_autobuild===0.7.1 \ + sphinxcontrib-ditaa===0.6 \ + && chmod 755 /makeSphinx.sh +RUN cd /tmp \ + && unzip ditaa.zip \ + && mv ditaa0_9.jar /usr/lib +ADD ditaa.sh /usr/bin/ditaa + +VOLUME /data-in /data-out + +# For Sphinx-autobuild +# Port 8000 - HTTP server +# Port 35729 - websockets connection to allow automatic browser reloads after each build +EXPOSE 8000 35729 + +ENTRYPOINT ["/bin/bash", "/makeSphinx.sh"] +CMD [] diff --git a/util/docker/flashrom.org/README.md b/util/docker/flashrom.org/README.md new file mode 100644 index 0000000..313d0d7 --- /dev/null +++ b/util/docker/flashrom.org/README.md @@ -0,0 +1,32 @@ +# doc.coreboot.org + Docker container for generating and developing documentation for doc.coreboot.org + +**NOTE**: All paths are from the base of the coreboot git repo. + +### Build + +```sh + docker build --force-rm -t "doc.flashrom.org" "$PWD/util/docker/flashrom.org/" +``` + +### Generating production HTML + +```sh +# To ensure the output directory is given the correct permissions, make sure to +# created it before running docker the first time. +mkdir -p "$PWD/doc/_build/" + +docker run -it --rm \ + --user "$(id -u):$(id -g)" \ + -v "$PWD/:/data-in/:ro" \ + -v "$PWD/doc/_build/:/data-out/" \ + doc.flashrom.org +``` + +### live reloaded with web server +On the host machine, open a browser to the address http://0.0.0.0:8000 +```sh +docker run -it --rm \ + --net=host -v "$PWD/:/data-in/:ro" \ + doc.flashrom.org livehtml +``` diff --git a/util/docker/flashrom.org/ditaa.sh b/util/docker/flashrom.org/ditaa.sh new file mode 100755 index 0000000..637379f --- /dev/null +++ b/util/docker/flashrom.org/ditaa.sh @@ -0,0 +1,2 @@ +#!/usr/bin/env sh +exec java -jar /usr/lib/ditaa0_9.jar $* diff --git a/util/docker/flashrom.org/makeSphinx.sh b/util/docker/flashrom.org/makeSphinx.sh new file mode 100755 index 0000000..5b6ea03 --- /dev/null +++ b/util/docker/flashrom.org/makeSphinx.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +if [ "$1" == "livehtml" ]; then + echo "Starting live documentation build" + cd /data-in/ && sphinx-autobuild -b html doc /tmp/build/html +else + echo "Starting production documentation build" + cd /data-in/ \ + && sphinx-build -b html doc /tmp/build/html \ + && rm -rf /data-out/* \ + && mv /tmp/build/html/* /data-out/ +fi -- To view, visit https://review.coreboot.org/c/flashrom/+/75344 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I889fa5f98e2fd6ef896ba4bb30aa5d985ff82622 Gerrit-Change-Number: 75344 Gerrit-PatchSet: 3 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: programmer: Use correct type for flashbase
by Anastasia Klimchuk (Code Review) 23 May '23

23 May '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/75328 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: programmer: Use correct type for flashbase ...................................................................... programmer: Use correct type for flashbase The flashbase is a machine-sized integer representation of address space and so use the appropriate type that is correctly sized to encode such data. The flashbase is assigned to 'base' in 'map_flash()' and the type correctly changed to uintptr_t in commit 4e32ec19b124a7 therefore makes for a consistent type usage whenever stored. While `sizeof(unsigned long)` and `sizeof(uintptr_t)` are both `8` under most circumstances on a 64bit platform and thus have enough bits to represent all addresses on the platform, the C standard does not guarantee this. Only `uintptr_t` and `void *` has a guaranteed isomorphism as `uintptr_t` is defined by the platforms toolchain support whereas the conversion from `void *` to an integer is implementation defined and that the memory address value may contain additional bits describing the validation data or provenance of the address. Therefore a integer is insufficient to contain all the necessary information for that specific platform so this may not always work out for all platforms and toolchain combinations. Spotted-by: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Change-Id: Ib9057e438731b9cccde0e24d5c8f758c3af1d47f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/75328 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Martin Roth <martin.roth(a)amd.corp-partner.google.com> --- M flashrom.c M include/programmer.h 2 files changed, 38 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Martin Roth: Looks good to me, but someone else must approve diff --git a/flashrom.c b/flashrom.c index 19afb54..b6e5cf8 100644 --- a/flashrom.c +++ b/flashrom.c @@ -21,6 +21,7 @@ #include <stdbool.h> #include <stdio.h> +#include <stdint.h> #include <sys/types.h> #include <string.h> #include <unistd.h> @@ -48,7 +49,7 @@ struct decode_sizes max_rom_decode; /* If nonzero, used as the start address of bottom-aligned flash. */ -unsigned long flashbase; +uintptr_t flashbase; /* Is writing allowed with this programmer? */ bool programmer_may_write; diff --git a/include/programmer.h b/include/programmer.h index 5b304f5..562f58a 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -292,7 +292,7 @@ // FIXME: These need to be local, not global extern struct decode_sizes max_rom_decode; extern bool programmer_may_write; -extern unsigned long flashbase; +extern uintptr_t flashbase; /* used in programmer_enable.c */ char *extract_programmer_param_str(const struct programmer_cfg *cfg, const char *param_name); /* spi.c */ -- To view, visit https://review.coreboot.org/c/flashrom/+/75328 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib9057e438731b9cccde0e24d5c8f758c3af1d47f Gerrit-Change-Number: 75328 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: meson: Add support for ni845x_spi on Windows
by Miklós Márton (Code Review) 23 May '23

23 May '23

Attention is currently required from: Thomas Heijligen. Miklós Márton has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75236 ) Change subject: meson: Add support for ni845x_spi on Windows ...................................................................... Patch Set 4: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/75236/comment/017db468_717535fe PS4, Line 10: meson setup -Dprogrammer=ni845x build meson setup -Dprogrammer=ni845x_spi build Patchset: PS4: I have cherry picked this commit: https://github.com/flashrom/flashrom/commit/57f75bbbf481cba53cd719acb25fa2d… to get the ni845x_spi to get to compilable state, but the build with mingw failed with: https://gist.github.com/martonmiklos/95aaca98fd29620e7a1c6a2f160858fe -- To view, visit https://review.coreboot.org/c/flashrom/+/75236 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2d32f11852ac1a5184af8e8683ca1914a6e72973 Gerrit-Change-Number: 75236 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 22 May 2023 21:29:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: amd_imc.c: Cleanup C89ism and redundant NULL check
by Edward O'Callaghan (Code Review) 22 May '23

22 May '23

Attention is currently required from: Angel Pons, Martin Roth. Hello Angel Pons, Martin Roth, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/75359 to look at the new patch set (#2). Change subject: amd_imc.c: Cleanup C89ism and redundant NULL check ...................................................................... amd_imc.c: Cleanup C89ism and redundant NULL check A bit of spring cleaning here to freshen up some C89ism's, drop a redundant NULLality check in a static local function and canonicalise branch predictions. Also const correct some static locals and add missing include while here. This makes the code overall shorter, more consistent and succinct. Change-Id: I1dbae500010fa50a67609efb3cbb5778b684ad04 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M amd_imc.c 1 file changed, 29 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/75359/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/75359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbae500010fa50a67609efb3cbb5778b684ad04 Gerrit-Change-Number: 75359 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: amd_imc.c: Cleanup C89ism and redundant NULL check
by Edward O'Callaghan (Code Review) 22 May '23

22 May '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/75359 ) Change subject: amd_imc.c: Cleanup C89ism and redundant NULL check ...................................................................... amd_imc.c: Cleanup C89ism and redundant NULL check A bit of spring cleaning here to freshen up some C89ism's, drop a redundant NULLality check in a static local function and canonicalise branch predictions. Also const correct some static locals while here. This makes the code overall shorter and more consistent and succinct. Change-Id: I1dbae500010fa50a67609efb3cbb5778b684ad04 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M amd_imc.c 1 file changed, 27 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/75359/1 diff --git a/amd_imc.c b/amd_imc.c index d57728e..e50aa48 100644 --- a/amd_imc.c +++ b/amd_imc.c @@ -34,21 +34,12 @@ static uint16_t get_sio_port(struct pci_dev *dev) { - uint16_t ec_port; - - if (!dev) { - return 0; - } - - ec_port = pci_read_word(dev, 0xa4); - /* EcPortActive? */ + const uint16_t ec_port = pci_read_word(dev, 0xa4); if (!(ec_port & 0x1)) return 0; - ec_port &= ~0x1; - - return ec_port; + return ec_port & ~0x1; } /* Wait for up to 10 ms for a response. */ @@ -67,8 +58,6 @@ static uint16_t mbox_get_port(uint16_t sio_port) { - uint16_t mbox_port; - enter_conf_mode_ec(sio_port); /* Go to LDN 9, mailbox */ @@ -80,7 +69,7 @@ return 0; } - mbox_port = sio_read(sio_port, 0x60) << 8; + uint16_t mbox_port = sio_read(sio_port, 0x60) << 8; mbox_port |= sio_read(sio_port, 0x61); exit_conf_mode_ec(sio_port); @@ -90,19 +79,16 @@ /* Returns negative values when IMC is inactive, positive values on errors */ static int imc_send_cmd(struct pci_dev *dev, uint8_t cmd) { - uint16_t sio_port; - uint16_t mbox_port; - /* IntegratedEcPresent? */ if (!(pci_read_byte(dev, 0x40) & (1 << 7))) return -1; - sio_port = get_sio_port(dev); + const uint16_t sio_port = get_sio_port(dev); if (!sio_port) return -1; msg_pdbg2("IMC SIO is at 0x%x.\n", sio_port); - mbox_port = mbox_get_port(sio_port); + const uint16_t mbox_port = mbox_get_port(sio_port); if (!mbox_port) return -1; msg_pdbg2("IMC MBOX is at 0x%x.\n", mbox_port); @@ -119,9 +105,8 @@ static int imc_resume(void *data) { struct pci_dev *dev = data; - int ret = imc_send_cmd(dev, 0xb5); - - if (ret != 0) + const int ret = imc_send_cmd(dev, 0xb5); + if (ret) msg_pinfo("Resuming IMC failed.\n"); else msg_pdbg2("IMC resumed.\n"); @@ -131,7 +116,7 @@ int amd_imc_shutdown(struct pci_dev *dev) { /* Try to put IMC to sleep */ - int ret = imc_send_cmd(dev, 0xb4); + const int ret = imc_send_cmd(dev, 0xb4); /* No IMC activity detectable, assume we are fine */ if (ret < 0) { @@ -139,7 +124,7 @@ return 0; } - if (ret != 0) { + if (ret) { msg_perr("Shutting down IMC failed.\n"); return ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/75359 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1dbae500010fa50a67609efb3cbb5778b684ad04 Gerrit-Change-Number: 75359 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom: rename sb600spi.c to amd_spi.c
by Edward O'Callaghan (Code Review) 22 May '23

22 May '23

Attention is currently required from: Nico Huber, Martin L Roth, Thomas Heijligen, Angel Pons, Arthur Heymans, Anastasia Klimchuk, Martin Roth. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74952 ) Change subject: flashrom: rename sb600spi.c to amd_spi.c ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: > Martin, I apologise, you can ignore previous comment. […] btw, this was originally purposed in CB:36434 but never went anyway unfortunately. I originally chose FCH as a name to be consistent with ICH and as that is where the SPI controller is situated physically within the fabric of the FCH. Technically, 'ichspi' is confusing as it contains both ich and pch implementations. I did ask AMD at the time what could be a better name and how the driver could be best split up. I just couldn't get a clear answer at the time on what the names should be? I suspect what we want is to rename this to amd_spi.c initially but then split this driver and have amd_fch to refer to the common functions? I am going to lean on Martins judgement here since he has best visibility and understanding of this peripheral controller across many generations. In the case of ichspi it should probably have the same treatment/theme. -- To view, visit https://review.coreboot.org/c/flashrom/+/74952 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13859de27e602cc6496684e6cb66b2dc2e21531a Gerrit-Change-Number: 74952 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> 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-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Mon, 22 May 2023 03:03:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom MAINTAINERS: Add Martin Roth as reviewer for AMD SPI
by Edward O'Callaghan (Code Review) 22 May '23

22 May '23

Attention is currently required from: Martin L Roth, Martin Roth. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74954 ) Change subject: flashrom MAINTAINERS: Add Martin Roth as reviewer for AMD SPI ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/74954 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I27a07be1549ef070ad72b8e657d72170c7e85620 Gerrit-Change-Number: 74954 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> 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: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Comment-Date: Mon, 22 May 2023 02:49:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom: Update 'sb600' references to 'amd'
by Edward O'Callaghan (Code Review) 22 May '23

22 May '23

Attention is currently required from: Martin L Roth, Thomas Heijligen, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74953 ) Change subject: flashrom: Update 'sb600' references to 'amd' ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File amd_spi.c: https://review.coreboot.org/c/flashrom/+/74953/comment/89920bd9_23e434aa PS1, Line 156: amd_spibar Minor: Since these function parameters are stack local to the working function, how about just dropping the prefix? -- To view, visit https://review.coreboot.org/c/flashrom/+/74953 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1200ef25b6765f809c754ae0adcdcfe680c202fd Gerrit-Change-Number: 74953 Gerrit-PatchSet: 1 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 22 May 2023 02:49:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit