November 2021 - flashrom-gerrit

Change in flashrom[master]: Makefile: Revise build options for Linux specific headers
by Nico Huber (Code Review) 07 Nov '21

07 Nov '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58627 ) Change subject: Makefile: Revise build options for Linux specific headers ...................................................................... Makefile: Revise build options for Linux specific headers Clean up the feature target by outsourcing the test to an own variable. Change the print output and don't write to the build-details file. This is in preparation for further changes. Change-Id: I18fc27252afb49fa7d1f2787faee2b5b669275aa Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58627 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile A Makefile.d/linux_i2c_test.c A Makefile.d/linux_mtd_test.c A Makefile.d/linux_spi_test.c M Makefile.include 5 files changed, 48 insertions(+), 82 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/Makefile b/Makefile index bf363de..9af625e 100644 --- a/Makefile +++ b/Makefile @@ -170,6 +170,9 @@ HAS_UTSNAME := $(call c_compile_test, Makefile.d/utsname_test.c) HAS_CLOCK_GETTIME := $(call c_compile_test, Makefile.d/clock_gettime_test.c) +HAS_LINUX_MTD := $(call c_compile_test, Makefile.d/linux_mtd_test.c) +HAS_LINUX_SPI := $(call c_compile_test, Makefile.d/linux_spi_test.c) +HAS_LINUX_I2C := $(call c_compile_test, Makefile.d/linux_i2c_test.c) ifeq ($(TARGET_OS), $(filter $(TARGET_OS), FreeBSD OpenBSD DragonFlyBSD)) override CPPFLAGS += -I/usr/local/include @@ -239,9 +242,15 @@ $(call mark_unsupported,$(DEPENDS_ON_LIBUSB1) $(DEPENDS_ON_LIBFTDI) $(DEPENDS_ON_LIBJAYLINK)) endif -# Android is handled internally as separate OS, but it supports about the same drivers as Linux. -ifneq ($(TARGET_OS), $(filter $(TARGET_OS), Linux Android)) -$(call mark_unsupported,CONFIG_LINUX_MTD CONFIG_LINUX_SPI) +ifeq ($(HAS_LINUX_MTD), no) +$(call mark_unsupported,CONFIG_LINUX_MTD) +endif + +ifeq ($(HAS_LINUX_SPI), no) +$(call mark_unsupported,CONFIG_LINUX_SPI) +endif + +ifeq ($(HAS_LINUX_I2C), no) $(call mark_unsupported,CONFIG_MSTARDDC_SPI CONFIG_LSPCON_I2C_SPI CONFIG_REALTEK_MST_I2C_SPI) endif @@ -250,10 +259,6 @@ $(call mark_unsupported,CONFIG_RAYER_SPI) endif -ifeq ($(TARGET_OS), Linux) -CONFIG_LINUX_I2C_HELPER = yes -endif - # Disable the internal programmer on unsupported architectures (everything but x86 and mipsel) ifneq ($(ARCH)-little, $(filter $(ARCH), x86 mips)-$(ENDIAN)) $(call mark_unsupported,CONFIG_INTERNAL) @@ -679,21 +684,17 @@ endif ifeq ($(CONFIG_LINUX_MTD), yes) -# This is a totally ugly hack. -FEATURE_CFLAGS += $(call debug_shell,grep -q "LINUX_MTD_SUPPORT := yes" .features && printf "%s" "-D'CONFIG_LINUX_MTD=1'") +FEATURE_CFLAGS += -D'CONFIG_LINUX_MTD=1' PROGRAMMER_OBJS += linux_mtd.o endif ifeq ($(CONFIG_LINUX_SPI), yes) -# This is a totally ugly hack. -FEATURE_CFLAGS += $(call debug_shell,grep -q "LINUX_SPI_SUPPORT := yes" .features && printf "%s" "-D'CONFIG_LINUX_SPI=1'") +FEATURE_CFLAGS += -D'CONFIG_LINUX_SPI=1' PROGRAMMER_OBJS += linux_spi.o endif ifeq ($(CONFIG_MSTARDDC_SPI), yes) -# This is a totally ugly hack. -FEATURE_CFLAGS += $(call debug_shell,grep -q "LINUX_I2C_SUPPORT := yes" .features && printf "%s" "-D'CONFIG_MSTARDDC_SPI=1'") -NEED_LINUX_I2C += CONFIG_MSTARDDC_SPI +FEATURE_CFLAGS += -D'CONFIG_MSTARDDC_SPI=1' PROGRAMMER_OBJS += mstarddc_spi.o endif @@ -741,7 +742,7 @@ PROGRAMMER_OBJS += ni845x_spi.o endif -ifeq ($(CONFIG_LINUX_I2C_HELPER), yes) +ifeq ($(HAS_LINUX_I2C), yes) LIB_OBJS += i2c_helper_linux.o endif @@ -1023,33 +1024,6 @@ ( echo "not found."; echo "FTDISUPPORT := no" >> .features.tmp ) } \ 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) endif -ifeq ($(CONFIG_LINUX_MTD), yes) - @printf "Checking if Linux MTD headers are present... " | tee -a $(BUILD_DETAILS_FILE) - @echo "$$LINUX_MTD_TEST" > .featuretest.c - @printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE) - @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \ - ( echo "yes."; echo "LINUX_MTD_SUPPORT := yes" >> .features.tmp ) || \ - ( echo "no."; echo "LINUX_MTD_SUPPORT := no" >> .features.tmp ) } \ - 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) -endif -ifeq ($(CONFIG_LINUX_SPI), yes) - @printf "Checking if Linux SPI headers are present... " | tee -a $(BUILD_DETAILS_FILE) - @echo "$$LINUX_SPI_TEST" > .featuretest.c - @printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE) - @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \ - ( echo "yes."; echo "LINUX_SPI_SUPPORT := yes" >> .features.tmp ) || \ - ( echo "no."; echo "LINUX_SPI_SUPPORT := no" >> .features.tmp ) } \ - 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) -endif -ifneq ($(NEED_LINUX_I2C), ) - @printf "Checking if Linux I2C headers are present... " | tee -a $(BUILD_DETAILS_FILE) - @echo "$$LINUX_I2C_TEST" > .featuretest.c - @printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE) - @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \ - ( echo "yes."; echo "LINUX_I2C_SUPPORT := yes" >> .features.tmp ) || \ - ( echo "no."; echo "LINUX_I2C_SUPPORT := no" >> .features.tmp ) } \ - 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) -endif ifeq ($(CONFIG_NI845X_SPI), yes) @printf "Checking for NI USB-845x installation... " | tee -a $(BUILD_DETAILS_FILE) @echo "$$NI845X_TEST" > .featuretest.c @@ -1066,10 +1040,14 @@ exit 1; }; } \ 2>>$(BUILD_DETAILS_FILE); echo $? >&3 ; } | tee -a $(BUILD_DETAILS_FILE) >&4; } 3>&1;} | { read rc ; exit ${rc}; } } 4>&1 endif - @echo "Checking for header \"sys/utsname.h\": $(HAS_UTSNAME)" - @echo "Checking for function \"clock_gettime\": $(HAS_CLOCK_GETTIME)" @$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features @rm -f .featuretest.c .featuretest$(EXEC_SUFFIX) + @echo "Checking for header \"mtd/mtd-user.h\": $(HAS_LINUX_MTD)" + @echo "Checking for header \"linux/spi/spidev.h\": $(HAS_LINUX_SPI)" + @echo "Checking for header \"linux/i2c-dev.h\": $(HAS_LINUX_I2C)" + @echo "Checking for header \"linux/i2c.h\": $(HAS_LINUX_I2C)" + @echo "Checking for header \"sys/utsname.h\": $(HAS_UTSNAME)" + @echo "Checking for function \"clock_gettime\": $(HAS_CLOCK_GETTIME)" $(PROGRAM).8.html: $(PROGRAM).8 @groff -mandoc -Thtml $< >$@ diff --git a/Makefile.d/linux_i2c_test.c b/Makefile.d/linux_i2c_test.c new file mode 100644 index 0000000..768226b --- /dev/null +++ b/Makefile.d/linux_i2c_test.c @@ -0,0 +1,9 @@ +#include <linux/i2c-dev.h> +#include <linux/i2c.h> + +int main(int argc, char **argv) +{ + (void)argc; + (void)argv; + return 0; +} diff --git a/Makefile.d/linux_mtd_test.c b/Makefile.d/linux_mtd_test.c new file mode 100644 index 0000000..d254e24 --- /dev/null +++ b/Makefile.d/linux_mtd_test.c @@ -0,0 +1,8 @@ +#include <mtd/mtd-user.h> + +int main(int argc, char **argv) +{ + (void)argc; + (void)argv; + return 0; +} diff --git a/Makefile.d/linux_spi_test.c b/Makefile.d/linux_spi_test.c new file mode 100644 index 0000000..a4d2657 --- /dev/null +++ b/Makefile.d/linux_spi_test.c @@ -0,0 +1,9 @@ +#include <linux/types.h> +#include <linux/spi/spidev.h> + +int main(int argc, char **argv) +{ + (void)argc; + (void)argv; + return 0; +} diff --git a/Makefile.include b/Makefile.include index 0ea1234..dccc69d 100644 --- a/Makefile.include +++ b/Makefile.include @@ -130,44 +130,6 @@ endef export FTDI_232H_TEST -define LINUX_MTD_TEST -#include <mtd/mtd-user.h> - -int main(int argc, char **argv) -{ - (void) argc; - (void) argv; - return 0; -} -endef -export LINUX_MTD_TEST - -define LINUX_SPI_TEST -#include <linux/types.h> -#include <linux/spi/spidev.h> - -int main(int argc, char **argv) -{ - (void) argc; - (void) argv; - return 0; -} -endef -export LINUX_SPI_TEST - -define LINUX_I2C_TEST -#include <linux/i2c-dev.h> -#include <linux/i2c.h> - -int main(int argc, char **argv) -{ - (void) argc; - (void) argv; - return 0; -} -endef -export LINUX_I2C_TEST - define NI845X_TEST #include <ni845x.h> 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/58627 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I18fc27252afb49fa7d1f2787faee2b5b669275aa Gerrit-Change-Number: 58627 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: Makefile: Revise utsname and clock_gettime test
by Nico Huber (Code Review) 07 Nov '21

07 Nov '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58618 ) Change subject: Makefile: Revise utsname and clock_gettime test ...................................................................... Makefile: Revise utsname and clock_gettime test Clean up the feature target by outsourcing the test to an own variable. Change the print output and don't write to the build-details file. HAS_CLOCK_GETTIME=no replaces DISABLE_CLOCK_GETTIME=yes This is in preparation for further changes. Change-Id: Ie1f43b3d5a8ad79bff3f9bbc21f359ec35abc42a Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58618 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile A Makefile.d/clock_gettime_test.c A Makefile.d/utsname_test.c M Makefile.include 4 files changed, 31 insertions(+), 47 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/Makefile b/Makefile index e84248a..bf363de 100644 --- a/Makefile +++ b/Makefile @@ -168,6 +168,9 @@ override ARCH := $(call c_macro_test, Makefile.d/arch_test.h) override ENDIAN := $(call c_macro_test, Makefile.d/endian_test.h) +HAS_UTSNAME := $(call c_compile_test, Makefile.d/utsname_test.c) +HAS_CLOCK_GETTIME := $(call c_compile_test, Makefile.d/clock_gettime_test.c) + ifeq ($(TARGET_OS), $(filter $(TARGET_OS), FreeBSD OpenBSD DragonFlyBSD)) override CPPFLAGS += -I/usr/local/include override LDFLAGS += -L/usr/local/lib @@ -841,13 +844,17 @@ CLI_OBJS += print_wiki.o endif -FEATURE_CFLAGS += $(call debug_shell,grep -q "UTSNAME := yes" .features && printf "%s" "-D'HAVE_UTSNAME=1'") - # We could use PULLED_IN_LIBS, but that would be ugly. FEATURE_LIBS += $(call debug_shell,grep -q "NEEDLIBZ := yes" .libdeps && printf "%s" "-lz") -FEATURE_CFLAGS += $(call debug_shell,grep -q "CLOCK_GETTIME := yes" .features && printf "%s" "-D'HAVE_CLOCK_GETTIME=1'") -FEATURE_LIBS += $(call debug_shell,grep -q "CLOCK_GETTIME := yes" .features && printf "%s" "-lrt") +ifeq ($(HAS_UTSNAME), yes) +FEATURE_CFLAGS += -D'HAVE_UTSNAME=1' +endif + +ifeq ($(HAS_CLOCK_GETTIME), yes) +FEATURE_CFLAGS += -D'HAVE_CLOCK_GETTIME=1' +FEATURE_LIBS += -lrt +endif LIBFLASHROM_OBJS = $(CHIP_OBJS) $(PROGRAMMER_OBJS) $(LIB_OBJS) OBJS = $(CLI_OBJS) $(LIBFLASHROM_OBJS) @@ -1059,24 +1066,8 @@ exit 1; }; } \ 2>>$(BUILD_DETAILS_FILE); echo $? >&3 ; } | tee -a $(BUILD_DETAILS_FILE) >&4; } 3>&1;} | { read rc ; exit ${rc}; } } 4>&1 endif - @printf "Checking for utsname support... " | tee -a $(BUILD_DETAILS_FILE) - @echo "$$UTSNAME_TEST" > .featuretest.c - @printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE) - @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \ - ( echo "found."; echo "UTSNAME := yes" >> .features.tmp ) || \ - ( echo "not found."; echo "UTSNAME := no" >> .features.tmp ) } 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) - @printf "Checking for clock_gettime support... " | tee -a $(BUILD_DETAILS_FILE) -ifeq ($(DISABLE_CLOCK_GETTIME), yes) - @ { ( echo "disabled."; echo "CLOCK_GETTIME := no" >>.features.tmp ) } \ - 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) -else - @echo "$$CLOCK_GETTIME_TEST" >.featuretest.c - @printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -lrt .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE) - @ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) -lrt .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \ - ( echo "found."; echo "CLOCK_GETTIME := yes" >>.features.tmp ) || \ - ( echo "not found."; echo "CLOCK_GETTIME := no" >>.features.tmp ) } \ - 2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE) -endif + @echo "Checking for header \"sys/utsname.h\": $(HAS_UTSNAME)" + @echo "Checking for function \"clock_gettime\": $(HAS_CLOCK_GETTIME)" @$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features @rm -f .featuretest.c .featuretest$(EXEC_SUFFIX) diff --git a/Makefile.d/clock_gettime_test.c b/Makefile.d/clock_gettime_test.c new file mode 100644 index 0000000..000aa42 --- /dev/null +++ b/Makefile.d/clock_gettime_test.c @@ -0,0 +1,9 @@ +#include <time.h> +int main(int argc, char **argv) +{ + (void)argc; + (void)argv; + struct timespec res; + clock_gettime(CLOCK_REALTIME, &res); + return 0; +} diff --git a/Makefile.d/utsname_test.c b/Makefile.d/utsname_test.c new file mode 100644 index 0000000..7bc6665 --- /dev/null +++ b/Makefile.d/utsname_test.c @@ -0,0 +1,9 @@ +#include <sys/utsname.h> +struct utsname osinfo; +int main(int argc, char **argv) +{ + (void)argc; + (void)argv; + uname(&osinfo); + return 0; +} diff --git a/Makefile.include b/Makefile.include index 2b5a344..0ea1234 100644 --- a/Makefile.include +++ b/Makefile.include @@ -130,19 +130,6 @@ endef export FTDI_232H_TEST -define UTSNAME_TEST -#include <sys/utsname.h> -struct utsname osinfo; -int main(int argc, char **argv) -{ - (void) argc; - (void) argv; - uname (&osinfo); - return 0; -} -endef -export UTSNAME_TEST - define LINUX_MTD_TEST #include <mtd/mtd-user.h> @@ -181,18 +168,6 @@ endef export LINUX_I2C_TEST -define CLOCK_GETTIME_TEST -#include <time.h> - -int main(int argc, char **argv) -{ - struct timespec res; - clock_gettime(CLOCK_REALTIME, &res); - return 0; -} -endef -export CLOCK_GETTIME_TEST - define NI845X_TEST #include <ni845x.h> 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/58618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie1f43b3d5a8ad79bff3f9bbc21f359ec35abc42a Gerrit-Change-Number: 58618 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin Lucina <martin(a)lucina.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: Makefile: compiler target: separate fixed text and value by a colon
by Nico Huber (Code Review) 07 Nov '21

07 Nov '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58617 ) Change subject: Makefile: compiler target: separate fixed text and value by a colon ...................................................................... Makefile: compiler target: separate fixed text and value by a colon Continue to use the "key: value" format like for the C compiler. Use only shell code for TARGET_OS comparison. Change-Id: I69959c20aa2e43ed67b3057c37e964a34cdab136 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58617 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M Makefile 1 file changed, 6 insertions(+), 8 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/Makefile b/Makefile index 1c3181a..e84248a 100644 --- a/Makefile +++ b/Makefile @@ -893,17 +893,15 @@ @if [ $(CC_WORKING) = yes ]; \ then $(CC) --version 2>/dev/null | head -1; \ else echo no; echo Aborting.; exit 1; fi - @echo Target arch is $(ARCH) + @echo "Target arch: $(ARCH)" @if [ $(ARCH) = unknown ]; then echo Aborting.; exit 1; fi - @echo Target OS is $(TARGET_OS) + @echo "Target OS: $(TARGET_OS)" @if [ $(TARGET_OS) = unknown ]; then echo Aborting.; exit 1; fi - @echo Target endian is $(ENDIAN) + @if [ $(TARGET_OS) = libpayload ] && ! $(CC) --version 2>&1 | grep -q coreboot; then \ + echo " Warning: It seems you are not using coreboot's reference compiler."; \ + echo " This might work but usually does not, please beware."; fi + @echo "Target endian: $(ENDIAN)" @if [ $(ENDIAN) = unknown ]; then echo Aborting.; exit 1; fi -ifeq ($(TARGET_OS), libpayload) - @$(CC) --version 2>&1 | grep -q coreboot || \ - ( echo "Warning: It seems you are not using coreboot's reference compiler."; \ - echo "This might work but usually does not, please beware." ) -endif hwlibs: compiler @printf "" > .libdeps 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/58617 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I69959c20aa2e43ed67b3057c37e964a34cdab136 Gerrit-Change-Number: 58617 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: [RFC] flashchips, writeprotect_ranges: add range decoding function
by Sergii Dmytruk (Code Review) 07 Nov '21

07 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58480 ) Change subject: [RFC] flashchips, writeprotect_ranges: add range decoding function ...................................................................... Patch Set 8: (2 comments) File flashchips.c: https://review.coreboot.org/c/flashrom/+/58480/comment/915a64fb_78e1dcf9 PS8, Line 6762: }, Looks like you forgot to initialize `.decode_range` field here. https://review.coreboot.org/c/flashrom/+/58480/comment/d737c342_c916b81d PS8, Line 8660: nit: extra empty line -- To view, visit https://review.coreboot.org/c/flashrom/+/58480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id163ed80938a946a502ed116e48e8236e36eb203 Gerrit-Change-Number: 58480 Gerrit-PatchSet: 8 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 07 Nov 2021 16:06:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add wraps for __xstat variants of stat
by Nico Huber (Code Review) 07 Nov '21

07 Nov '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58103 ) Change subject: tests: Add wraps for __xstat variants of stat ...................................................................... Patch Set 4: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/58103/comment/5cc1f7b6_adc45ed4 PS2, Line 9: all > Environment this applies to: __xstat64 wrap is needed to run unit tests with emerge under chromium c […] That's enough information to retest this if ever necessary. I hope this will not turn into a cat-and-mouse game where we'd have to add more wraps in the future. But if it does: we could try to mock the header file as well. -- To view, visit https://review.coreboot.org/c/flashrom/+/58103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Gerrit-Change-Number: 58103 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 07 Nov 2021 15:41:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips.c: big erase blocksize first
by Nico Huber (Code Review) 07 Nov '21

07 Nov '21

Attention is currently required from: Simon Buhrow, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58467 ) Change subject: flashchips.c: big erase blocksize first ...................................................................... Patch Set 5: (3 comments) Patchset: PS5: > The clou is the condition […] I'm generally not a fan of special cases. However, if we can find a contained solution for the full-chip erase case, I wouldn't block it. Technically, checks like `per_blockfn == &erase_block` are a layering violation, i.e. something is done in a lower layer that should have been handled at a higher level. I left an inline comment about how I would avoid it. Eventually, I would prefer a generic solution for the small block-size problem, I think it's not hard to achieve. We should replace the current fallback-to-bigger-blocks strategy with one that checks if consecutive calls of a small erase function would completely fill a bigger one. There's one obstacle, however: The fallback strategy was necessary because some programmers can restrict what commands we can send. I would handle that by filtering the list of erase functions right after the chip was probed. File flashrom.c: https://review.coreboot.org/c/flashrom/+/58467/comment/242d4715_0b2ac6b4 PS5, Line 1261: if ((block_size == flashctx->chip->total_size * 1024) && per_blockfn == &erase_block){ I guess this would be much easier to handle in flashrom_flash_erase(). If the size matches the full chip and the last eraseblocks[] entry too, then call that eraser directly, and if it fails fall back to erase_by_layout(). No loops and definitely no nested loops necessary. https://review.coreboot.org/c/flashrom/+/58467/comment/e3d2c7a5_3870fea2 PS5, Line 1304: } Why change this? Doesn't the code handle this case gracefully? -- To view, visit https://review.coreboot.org/c/flashrom/+/58467 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I862ce0b5f8912565e43c340578d8126aa2e6aa3b Gerrit-Change-Number: 58467 Gerrit-PatchSet: 5 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sun, 07 Nov 2021 15:09:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Buhrow Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Delete obsolete comment
by Nico Huber (Code Review) 07 Nov '21

07 Nov '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58742 ) Change subject: flashrom.c: Delete obsolete comment ...................................................................... flashrom.c: Delete obsolete comment Change-Id: Ibd53fe34c05f87d7ecc0d6eee6463f9da3a174d4 Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/58742 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M flashrom.c 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 3987bb9..48d953b 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1377,7 +1377,6 @@ * be erased. * * @param flashctx Flash context to be used. - * @param buffer Buffer of full chip size to read into. * @return 0 on success, * 1 if all available erase functions failed. */ -- To view, visit https://review.coreboot.org/c/flashrom/+/58742 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibd53fe34c05f87d7ecc0d6eee6463f9da3a174d4 Gerrit-Change-Number: 58742 Gerrit-PatchSet: 2 Gerrit-Owner: Simon Buhrow Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: chipset_enable.c: Mark Intel Z390 as DEP
by Melvyn (Code Review) 05 Nov '21

05 Nov '21

Attention is currently required from: Melvyn. Melvyn has restored this change. ( https://review.coreboot.org/c/flashrom/+/58774 ) Change subject: chipset_enable.c: Mark Intel Z390 as DEP ...................................................................... Restored -- To view, visit https://review.coreboot.org/c/flashrom/+/58774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If14d45c144bb32a1d1046185d4476ea29e4d0912 Gerrit-Change-Number: 58774 Gerrit-PatchSet: 3 Gerrit-Owner: Melvyn <melvyn2(a)brcok.tk> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Melvyn <melvyn2(a)brcok.tk> Gerrit-MessageType: restore

1 0

Change in flashrom[master]: chipset_enable.c: Mark Intel Z390 as DEP
by Angel Pons (Code Review) 05 Nov '21

05 Nov '21

Attention is currently required from: Melvyn. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58774 ) Change subject: chipset_enable.c: Mark Intel Z390 as DEP ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/58774/comment/c16e194e_48d8a69a PS3, Line 12: melvyn2 > I'm not sure how I missed the bright red text there, sorry about that. […] I might be able to sign off this patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/58774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If14d45c144bb32a1d1046185d4476ea29e4d0912 Gerrit-Change-Number: 58774 Gerrit-PatchSet: 3 Gerrit-Owner: Melvyn <melvyn2(a)brcok.tk> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Melvyn <melvyn2(a)brcok.tk> Gerrit-Comment-Date: Fri, 05 Nov 2021 17:57:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Melvyn <melvyn2(a)brcok.tk> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: Add W25Q64JV
by Simon Buhrow (Code Review) 05 Nov '21

05 Nov '21

Simon Buhrow has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/58971 ) Change subject: flashchips: Add W25Q64JV ...................................................................... flashchips: Add W25Q64JV I have successfully tested it with FT2232H-programmer. Change-Id: Ia9a32146b225eca66e9a6bfef45be5f2b24aef46 Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de> --- M flashchips.c M flashchips.h 2 files changed, 41 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/71/58971/1 diff --git a/flashchips.c b/flashchips.c index 434c46d..b89c6be 100644 --- a/flashchips.c +++ b/flashchips.c @@ -17869,6 +17869,46 @@ { .vendor = "Winbond", + .name = "W25Q64JV", + .bustype = BUS_SPI, + .manufacture_id = WINBOND_NEX_ID, + .model_id = WINBOND_NEX_W25Q64JV, + .total_size = 8192, + .page_size = 256, + /* supports SFDP */ + /* QPI enable 0x38*/ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI, + .tested = TEST_OK_PREW, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 2048} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 256} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 128} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {8 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {8 * 1024 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_plain, /* TODO: improve */ + .unlock = spi_disable_blockprotect, + .write = spi_chip_write_256, + .read = spi_chip_read, + .voltage = {2700, 3600}, + }, + + { + .vendor = "Winbond", .name = "W25Q64.W", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, diff --git a/flashchips.h b/flashchips.h index e190dca..ba97719 100644 --- a/flashchips.h +++ b/flashchips.h @@ -976,6 +976,7 @@ #define WINBOND_NEX_W25Q64_W 0x6017 /* W25Q64DW; W25Q64FV in QPI mode */ #define WINBOND_NEX_W25Q128_W 0x6018 /* W25Q128FW; W25Q128FV in QPI mode */ #define WINBOND_NEX_W25Q256_W 0x6019 /* W25Q256JW */ +#define WINBOND_NEX_W25Q64JV 0x7017 /* W25Q64JV */ #define WINBOND_NEX_W25Q128_V_M 0x7018 /* W25Q128JVSM */ #define WINBOND_NEX_W25Q256JV_M 0x7019 /* W25Q256JV_M (QE=0) */ #define WINBOND_NEX_W25Q32JW_M 0x8016 /* W25Q32JW...M */ -- To view, visit https://review.coreboot.org/c/flashrom/+/58971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia9a32146b225eca66e9a6bfef45be5f2b24aef46 Gerrit-Change-Number: 58971 Gerrit-PatchSet: 1 Gerrit-Owner: Simon Buhrow Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2021