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

Change in flashrom[master]: meson: fix dependency on raw access
by Daniel Campello (Code Review) 17 Mar '21

17 Mar '21

Daniel Campello has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/51577 ) Change subject: meson: fix dependency on raw access ...................................................................... meson: fix dependency on raw access This is matching the following comment on Makefile: \# This is a dirty hack, but it saves us from checking all PCI drivers and all platforms manually. \# libpci may need raw memory, MSR or PCI port I/O on some platforms. \# Individual drivers might have the same needs as well. This changes fixes: ld.lld: error: undefined symbol: rget_io_perms >>> referenced by internal.c:243 (../flashrom-9999/internal.c:243) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_init) ld.lld: error: undefined symbol: mmio_writeb >>> referenced by internal.c:122 (../flashrom-9999/internal.c:122) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_chip_writeb) ld.lld: error: undefined symbol: mmio_writew >>> referenced by internal.c:128 (../flashrom-9999/internal.c:128) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_chip_writew) ld.lld: error: undefined symbol: mmio_writel >>> referenced by internal.c:134 (../flashrom-9999/internal.c:134) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_chip_writel) ld.lld: error: undefined symbol: mmio_readb >>> referenced by internal.c:140 (../flashrom-9999/internal.c:140) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_chip_readb) ld.lld: error: undefined symbol: mmio_readw >>> referenced by internal.c:146 (../flashrom-9999/internal.c:146) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_chip_readw) ld.lld: error: undefined symbol: mmio_readl >>> referenced by internal.c:152 (../flashrom-9999/internal.c:152) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_chip_readl) ld.lld: error: undefined symbol: mmio_readn >>> referenced by internal.c:158 (../flashrom-9999/internal.c:158) >>> libflashrom.so.1.0.0.p/internal.c.o:(internal_chip_readn) ld.lld: error: undefined symbol: physmap >>> referenced by flashrom.c:1454 (../flashrom-9999/flashrom.c:1454) >>> libflashrom.so.1.0.0.p/flashrom.c.o:(probe_flash) >>> referenced by flashrom.c:1454 (../flashrom-9999/flashrom.c:1454) >>> libflashrom.so.1.0.0.p/flashrom.c.o:(probe_flash) >>> referenced by flashrom.c >>> libflashrom.so.1.0.0.p/flashrom.c.o:(programmer_table) >>> referenced 2 more times ld.lld: error: undefined symbol: physunmap >>> referenced by flashrom.c >>> libflashrom.so.1.0.0.p/flashrom.c.o:(programmer_table) >>> referenced by flashrom.c >>> libflashrom.so.1.0.0.p/flashrom.c.o:(programmer_table) >>> referenced by flashrom.c >>> libflashrom.so.1.0.0.p/flashrom.c.o:(programmer_table) Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a Signed-off-by: Daniel Campello <campello(a)chromium.org> --- M meson.build 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/51577/1 diff --git a/meson.build b/meson.build index eac2134..d5895d8 100644 --- a/meson.build +++ b/meson.build @@ -105,6 +105,7 @@ if get_option('pciutils') srcs += 'pcidev.c' deps += dependency('libpci') + need_raw_access = true cargs += '-DNEED_PCI=1' else config_atahpt = false @@ -278,7 +279,6 @@ if config_rayer_spi srcs += 'rayer_spi.c' config_bitbang_spi = true - need_raw_access = true cargs += '-DCONFIG_RAYER_SPI=1' endif if config_satamv -- To view, visit https://review.coreboot.org/c/flashrom/+/51577 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3fa4ec7a735b81e989ba9fe2b53b18d0956627a Gerrit-Change-Number: 51577 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: meson: add missing meson options from ebuild
by Daniel Campello (Code Review) 17 Mar '21

17 Mar '21

Attention is currently required from: Richard Hughes, David Hendricks, Mario Limonciello, Edward O'Callaghan, Daniel Campello. Hello build bot (Jenkins), Richard Hughes, David Hendricks, Mario Limonciello, Edward O'Callaghan, Richard Hughes, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51552 to look at the new patch set (#2). Change subject: meson: add missing meson options from ebuild ...................................................................... meson: add missing meson options from ebuild This change adds missing meson options from chromiumos-overlay flashrom ebuild in order to migrate to meson. Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: I97d2fd687aa21533b86f9af446038bfe3da1f7d3 --- M meson.build M meson_options.txt 2 files changed, 8 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/51552/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I97d2fd687aa21533b86f9af446038bfe3da1f7d3 Gerrit-Change-Number: 51552 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com> Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com> Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Mario Limonciello <superm1(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: meson: add missing meson options from ebuild
by Richard Hughes (Code Review) 17 Mar '21

17 Mar '21

Attention is currently required from: Richard Hughes, David Hendricks, Mario Limonciello, Edward O'Callaghan, Daniel Campello. Richard Hughes has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51552 ) Change subject: meson: add missing meson options from ebuild ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: The wiki option is fine, but I don't think the static library option is correct. I think if you change the `shared_library()` to just `library()` then you can do `-Ddefault_library=static` and everything should magically work. I didn't know a static library was ever desired, so this is why I didn't use `library()` on initial submission. Apologies. -- To view, visit https://review.coreboot.org/c/flashrom/+/51552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I97d2fd687aa21533b86f9af446038bfe3da1f7d3 Gerrit-Change-Number: 51552 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Mario Limonciello <superm1(a)gmail.com> Gerrit-Reviewer: Richard Hughes <hughsient(a)gmail.com> Gerrit-Reviewer: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Mario Limonciello <superm1(a)gmail.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Wed, 17 Mar 2021 10:44:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: rayer_spi.c: Remove forward-declarations
by Angel Pons (Code Review) 17 Mar '21

17 Mar '21

Attention is currently required from: Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51555 ) Change subject: rayer_spi.c: Remove forward-declarations ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51555 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e3c82967a169d6a2512ffa17d1e0c78eafb2797 Gerrit-Change-Number: 51555 Gerrit-PatchSet: 2 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 17 Mar 2021 09:42:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: rayer_spi.c: Remove forward-declarations
by Anastasia Klimchuk (Code Review) 16 Mar '21

16 Mar '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51555 to look at the new patch set (#2). Change subject: rayer_spi.c: Remove forward-declarations ...................................................................... rayer_spi.c: Remove forward-declarations Reorder functions to avoid forward-declarations. This was aimed to be done for all spi masters in the earlier patch however this file was missed. BUG=b:140394053 TEST=builds Change-Id: I0e3c82967a169d6a2512ffa17d1e0c78eafb2797 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M rayer_spi.c 1 file changed, 45 insertions(+), 54 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/51555/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51555 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e3c82967a169d6a2512ffa17d1e0c78eafb2797 Gerrit-Change-Number: 51555 Gerrit-PatchSet: 2 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: rayer_spi.c: Remove forward-declarations
by Edward O'Callaghan (Code Review) 16 Mar '21

16 Mar '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51555 ) Change subject: rayer_spi.c: Remove forward-declarations ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51555 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e3c82967a169d6a2512ffa17d1e0c78eafb2797 Gerrit-Change-Number: 51555 Gerrit-PatchSet: 1 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 17 Mar 2021 03:53:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: rayer_spi.c: Remove forward-declarations
by Anastasia Klimchuk (Code Review) 16 Mar '21

16 Mar '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/51555 ) Change subject: rayer_spi.c: Remove forward-declarations ...................................................................... rayer_spi.c: Remove forward-declarations Reorder functions to avoid forward-declarations. I aimed to do this for all spi masters in earlier patch, but missed this one. BUG=b:140394053 TEST=builds Change-Id: I0e3c82967a169d6a2512ffa17d1e0c78eafb2797 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M rayer_spi.c 1 file changed, 45 insertions(+), 54 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/55/51555/1 diff --git a/rayer_spi.c b/rayer_spi.c index 8e869e6..cde008f 100644 --- a/rayer_spi.c +++ b/rayer_spi.c @@ -31,6 +31,11 @@ #include "programmer.h" #include "hwaccess.h" +static uint16_t lpt_iobase; + +/* Cached value of last byte sent. */ +static uint8_t lpt_outbyte; + /* We have two sets of pins, out and in. The numbers for both sets are * independent and are bitshift values, not real pin numbers. * Default settings are for the RayeR hardware. @@ -59,8 +64,20 @@ .miso_bit = 6, }; -static void dlc5_preinit(const void *); -static int dlc5_shutdown(void *); +static void dlc5_preinit(const void *data) { + msg_pdbg("dlc5_preinit\n"); + /* Assert pin 6 to receive MISO. */ + lpt_outbyte |= (1<<4); + OUTB(lpt_outbyte, lpt_iobase); +} + +static int dlc5_shutdown(void *data) { + msg_pdbg("dlc5_shutdown\n"); + /* De-assert pin 6 to force MISO low. */ + lpt_outbyte &= ~(1<<4); + OUTB(lpt_outbyte, lpt_iobase); + return 0; +} static const struct rayer_pinout xilinx_dlc5 = { .cs_bit = 2, @@ -71,8 +88,18 @@ .shutdown = dlc5_shutdown, }; -static void byteblaster_preinit(const void *); -static int byteblaster_shutdown(void *); +static void byteblaster_preinit(const void *data){ + msg_pdbg("byteblaster_preinit\n"); + /* Assert #EN signal. */ + OUTB(2, lpt_iobase + 2 ); +} + +static int byteblaster_shutdown(void *data){ + msg_pdbg("byteblaster_shutdown\n"); + /* De-Assert #EN signal. */ + OUTB(0, lpt_iobase + 2 ); + return 0; +} static const struct rayer_pinout altera_byteblastermv = { .cs_bit = 1, @@ -83,8 +110,20 @@ .shutdown = byteblaster_shutdown, }; -static void stk200_preinit(const void *); -static int stk200_shutdown(void *); +static void stk200_preinit(const void *data) { + msg_pdbg("stk200_init\n"); + /* Assert #EN signals, set LED signal. */ + lpt_outbyte = (1 << 6) ; + OUTB(lpt_outbyte, lpt_iobase); +} + +static int stk200_shutdown(void *data) { + msg_pdbg("stk200_shutdown\n"); + /* Assert #EN signals, clear LED signal. */ + lpt_outbyte = (1 << 2) | (1 << 3); + OUTB(lpt_outbyte, lpt_iobase); + return 0; +} static const struct rayer_pinout atmel_stk200 = { .cs_bit = 7, @@ -121,11 +160,6 @@ static const struct rayer_pinout *pinout = NULL; -static uint16_t lpt_iobase; - -/* Cached value of last byte sent. */ -static uint8_t lpt_outbyte; - static void rayer_bitbang_set_cs(int val) { lpt_outbyte &= ~(1 << pinout->cs_bit); @@ -237,49 +271,6 @@ return 0; } -static void byteblaster_preinit(const void *data){ - msg_pdbg("byteblaster_preinit\n"); - /* Assert #EN signal. */ - OUTB(2, lpt_iobase + 2 ); -} - -static int byteblaster_shutdown(void *data){ - msg_pdbg("byteblaster_shutdown\n"); - /* De-Assert #EN signal. */ - OUTB(0, lpt_iobase + 2 ); - return 0; -} - -static void stk200_preinit(const void *data) { - msg_pdbg("stk200_init\n"); - /* Assert #EN signals, set LED signal. */ - lpt_outbyte = (1 << 6) ; - OUTB(lpt_outbyte, lpt_iobase); -} - -static int stk200_shutdown(void *data) { - msg_pdbg("stk200_shutdown\n"); - /* Assert #EN signals, clear LED signal. */ - lpt_outbyte = (1 << 2) | (1 << 3); - OUTB(lpt_outbyte, lpt_iobase); - return 0; -} - -static void dlc5_preinit(const void *data) { - msg_pdbg("dlc5_preinit\n"); - /* Assert pin 6 to receive MISO. */ - lpt_outbyte |= (1<<4); - OUTB(lpt_outbyte, lpt_iobase); -} - -static int dlc5_shutdown(void *data) { - msg_pdbg("dlc5_shutdown\n"); - /* De-assert pin 6 to force MISO low. */ - lpt_outbyte &= ~(1<<4); - OUTB(lpt_outbyte, lpt_iobase); - return 0; -} - #else #error PCI port I/O access is not supported on this architecture yet. #endif -- To view, visit https://review.coreboot.org/c/flashrom/+/51555 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e3c82967a169d6a2512ffa17d1e0c78eafb2797 Gerrit-Change-Number: 51555 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: meson: add missing meson options from ebuild
by Daniel Campello (Code Review) 16 Mar '21

16 Mar '21

Daniel Campello has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/51552 ) Change subject: meson: add missing meson options from ebuild ...................................................................... meson: add missing meson options from ebuild This change adds missing meson options from chromiumos-overlay flashrom ebuild in order to migrate to meson. Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: I97d2fd687aa21533b86f9af446038bfe3da1f7d3 --- M meson.build M meson_options.txt 2 files changed, 48 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/51552/1 diff --git a/meson.build b/meson.build index 73df722..71dfa06 100644 --- a/meson.build +++ b/meson.build @@ -68,6 +68,8 @@ config_stlinkv3_spi = get_option('config_stlinkv3_spi') config_lspcon_i2c_spi = get_option('config_lspcon_i2c_spi') config_realtek_mst_i2c_spi = get_option('config_realtek_mst_i2c_spi') +config_print_wiki= get_option('print_wiki') +config_static= get_option('static') cargs = [] deps = [] @@ -91,7 +93,7 @@ if get_option('usb') srcs += 'usbdev.c' srcs += 'usb_device.c' - deps += dependency('libusb-1.0') + deps += dependency('libusb-1.0', static : config_static) else config_ch341a_spi = false config_dediprog = false @@ -103,7 +105,7 @@ # some programmers require libpci if get_option('pciutils') srcs += 'pcidev.c' - deps += dependency('libpci') + deps += dependency('libpci', static : config_static) cargs += '-DNEED_PCI=1' else config_atahpt = false @@ -163,7 +165,7 @@ if config_jlink_spi srcs += 'jlink_spi.c' cargs += '-DCONFIG_JLINK_SPI=1' - deps += dependency('libjaylink') + deps += dependency('libjaylink', static : config_static) endif if config_drkaiser srcs += 'drkaiser.c' @@ -176,7 +178,7 @@ if config_ft2232_spi srcs += 'ft2232_spi.c' cargs += '-DCONFIG_FT2232_SPI=1' - deps += dependency('libftdi1') + deps += dependency('libftdi1', static : config_static) cargs += '-DHAVE_FT232H=1' endif if config_gfxnvidia @@ -377,23 +379,41 @@ mapfile = 'libflashrom.map' vflag = '-Wl,--version-script,@0@/@1@'.format(meson.current_source_dir(), mapfile) -flashrom = shared_library( - 'flashrom', - sources : [ - srcs, - ], - soversion : lt_current, - version : lt_version, - dependencies : [ - deps, - ], - c_args : [ - cargs, - ], - install : true, - link_args : vflag, - link_depends : mapfile, -) +if config_static + flashrom = static_library( + 'flashrom', + sources : [ + srcs, + ], + dependencies : [ + deps, + ], + c_args : [ + cargs, + ], + install : true, + link_args : vflag, + link_depends : mapfile, + ) +else + flashrom = shared_library( + 'flashrom', + sources : [ + srcs, + ], + soversion : lt_current, + version : lt_version, + dependencies : [ + deps, + ], + c_args : [ + cargs, + ], + install : true, + link_args : vflag, + link_depends : mapfile, + ) +endif version = meson.project_version() #strip leading characters @@ -429,6 +449,11 @@ dependencies : deps ) +if config_print_wiki + srcs += 'print_wiki.c' + cargs += '-DCONFIG_PRINT_WIKI=1' +endif + # we can't just link_with libflashrom as we require all the internal symbols... executable( 'flashrom', diff --git a/meson_options.txt b/meson_options.txt index b599d66..5f6c7d3 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -1,5 +1,7 @@ option('pciutils', type : 'boolean', value : true, description : 'use pciutils') option('usb', type : 'boolean', value : true, description : 'use libusb1') +option('print_wiki', type : 'boolean', value : true, description : 'Print Wiki') +option('static', type : 'boolean', value : false, description : 'static') option('config_atahpt', type : 'boolean', value : false, description : 'Highpoint (HPT) ATA/RAID controllers') option('config_atapromise', type : 'boolean', value : false, description : 'Promise ATA controller') -- To view, visit https://review.coreboot.org/c/flashrom/+/51552 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I97d2fd687aa21533b86f9af446038bfe3da1f7d3 Gerrit-Change-Number: 51552 Gerrit-PatchSet: 1 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Add support for GD25LQ256D
by Nico Huber (Code Review) 16 Mar '21

16 Mar '21

Attention is currently required from: Drew Fustini, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51113 ) Change subject: Add support for GD25LQ256D ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I have to admit this is the first time I've looked at the flashrom source code so I may not be the b […] Please test the changes that Angel suggested. One thing about write tests: flashrom compares the current contents of the flash chip to the provided data. If they are equal, flashrom does nothing. In your verbose write log you can actually see that. In the list of blocks in the end: 0x000000-0x000fff:S The S tells us that the block was *S*kipped. All of them, it seems. To really test writes, one could use random data, e.g. dd if=/dev/urandom bs=1M count=32 of=random.rom flashrom -p ... -w random.rom If that says VERIFIED, we're all good. Also, if you have the time, it would be nice to test these additional feature flags: * FEATURE_4BA_READ, * FEATURE_4BA_WRITE, and * FEATURE_4BA_EXT_ADDR (this last one without any other FEATURE_4BA*) each with new random data. -- To view, visit https://review.coreboot.org/c/flashrom/+/51113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If1b745cac070d8deb96ce65137fd9060b148b965 Gerrit-Change-Number: 51113 Gerrit-PatchSet: 1 Gerrit-Owner: Drew Fustini <drew(a)beagleboard.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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Attention: Drew Fustini <drew(a)beagleboard.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 16 Mar 2021 18:17:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Drew Fustini <drew(a)beagleboard.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_mtd: Switch fopen() to open() for MTD devices
by Douglas Anderson (Code Review) 16 Mar '21

16 Mar '21

Douglas Anderson has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/50206 ) Change subject: linux_mtd: Switch fopen() to open() for MTD devices ...................................................................... Patch Set 2: (1 comment) File linux_mtd.c: https://review.coreboot.org/c/flashrom/+/50206/comment/55079206_83e35ecd PS2, Line 206: if (read(dev_fileno, buf + i, step) != step) { > It's probably not hard to get around. (Sorry for not replying earlier, > I didn't expect this to get abandoned this fast.) I'm happy to restore it if you want. ;-) I moved to "Abandoned" state because it seemed like there was general consensus that we didn't want to do it. Just yell if you want it restored. > This should directly map to a read of the flash right? Then we could > just re-try the current block (a few times?) if `>= 0 && != step`. Sure, we could. My bias, which you seemed to share in your other comment, is to leave well enough alone. Thus it doesn't really take much to convince me to abandon this change. My original motivation for posting this was that I don't like ignoring review feedback and Hung-Te requested it. If you and he both agree that there's some benefit for the change then I'm happy to keep spinning but I'm not going to fight for it. In my opinion if we find a case where the current stdio-based implementation is broken then we can switch to the direct ops, but until then it's just churn and you've shown that at least one tricky corner case might not be right. Why mess with it? > I couldn't find any documentation specific to the MTD device nodes, > do you know any? I'm also not much experienced with Linux device > nodes in general. I guess there could be exceptions to the read() > behavior, and it's fair to assume Linux behaviour in this driver > anyway. Yeah, I'm not an expert either. I mostly ended up down this rat hole because flashrom was failing on me and I wanted to fix it. ;-) I took a quick look and don't see any obvious documentation that would cover this type of thing, so probably it's tribal knowledge / read the source code. -- To view, visit https://review.coreboot.org/c/flashrom/+/50206 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifeb00b049ba5aa0bc8fdc591ac1f9861ad5d428d Gerrit-Change-Number: 50206 Gerrit-PatchSet: 2 Gerrit-Owner: Douglas Anderson <dianders(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Tue, 16 Mar 2021 15:09:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Douglas Anderson <dianders(a)chromium.org> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2021