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

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L3273F
by DZ (Code Review) 30 Mar '24

30 Mar '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer. Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81562?usp=email to look at the new patch set (#5). Change subject: flashchips: Add support for MXIC MX25L3273F ...................................................................... flashchips: Add support for MXIC MX25L3273F The MX25L3273F has been tested by ch341a programmer : read, write, erase and wp. We have tested --wp-enable, --wp-disable, --wp-list and --wp-range commands for write-protect feature. MX25L12850F datasheet is available at the following URL: https://www.mxic.com.tw/Lists/Datasheet/Attachments/8661/MX25L3273F,%203V,%… Change-Id: I4adaaa796d1db34702e7b0ed8e6fb167a3a5f6d7 Signed-off-by: DanielZhang <danielzhang(a)mxic.com.cn> --- M flashchips.c 1 file changed, 45 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/81562/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/81562?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4adaaa796d1db34702e7b0ed8e6fb167a3a5f6d7 Gerrit-Change-Number: 81562 Gerrit-PatchSet: 5 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L3273F
by DZ (Code Review) 30 Mar '24

30 Mar '24

DZ has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81562?usp=email ) Change subject: flashchips: Add support for MXIC MX25L3273F ...................................................................... flashchips: Add support for MXIC MX25L3273F The MX25L12850F has been tested by ch341a programmer : read, write, erase and wp. We have tested --wp-enable, --wp-disable, --wp-list and --wp-range commands for write-protect feature. MX25L12850F datasheet is available at the following URL: https://www.mxic.com.tw/Lists/Datasheet/Attachments/8661/MX25L3273F,%203V,%… Change-Id: I4adaaa796d1db34702e7b0ed8e6fb167a3a5f6d7 Signed-off-by: DanielZhang <danielzhang(a)mxic.com.cn> --- M flashchips.c 1 file changed, 46 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/81562/1 diff --git a/flashchips.c b/flashchips.c index 204f34f..7833031 100644 --- a/flashchips.c +++ b/flashchips.c @@ -9465,6 +9465,52 @@ }, .decode_range = DECODE_RANGE_SPI25_BIT_CMP, }, + { + .vendor = "Macronix", + .name = "MX25L3273F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25L3205, + .total_size = 4096, + .page_size = 256, + /* OTP: 1024B total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_CFGR, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 128} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 64} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {4 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {4 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit 6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2650, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP} + }, + + .decode_range = DECODE_RANGE_SPI25, + }, { .vendor = "Macronix", -- To view, visit https://review.coreboot.org/c/flashrom/+/81562?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I4adaaa796d1db34702e7b0ed8e6fb167a3a5f6d7 Gerrit-Change-Number: 81562 Gerrit-PatchSet: 1 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: udelay: clock_getres() does not provide clock_gettime() resolution
by Brian Norris (Code Review) 30 Mar '24

30 Mar '24

Attention is currently required from: Anastasia Klimchuk, Chen, Nikolai Artemiev, Peter Marheine, Stefan Reinauer. Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80806?usp=email ) Change subject: udelay: clock_getres() does not provide clock_gettime() resolution ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/80806/comment/3f6393b2_e1e406ea : PS2, Line 10: Linux makes this clearer > Peter thank you SO MUCH! I think let's bring Brian to the thread on the mailing list. […] I've subscribed now, but there's no way to reply to messages that predate my subscription. I wish y'all had a _real_ archive, that provides intact metadata. [*] I don't have much to add anyway. I think I agree on all points 1-3; I'm not sure about #4 (realtime). I think I didn't want to go as aggressively on #1 and #3, because it's a real chore trying to convince people of aggressive changes... ...but if there's any continuation to that conversation, I may chime in. [*] If you're interested, I like lore.kernel.org: https://korg.docs.kernel.org/lore.html -- To view, visit https://review.coreboot.org/c/flashrom/+/80806?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icdc6e184eacb14d3bd27029e3fc75be4431d82b4 Gerrit-Change-Number: 80806 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Chen <c(a)jia.je> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Chen <c(a)jia.je> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sat, 30 Mar 2024 00:20:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: erasure_layout: do not copy region buffers if they are null
by Alexander Goncharov (Code Review) 28 Mar '24

28 Mar '24

Alexander Goncharov has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81548?usp=email ) Change subject: erasure_layout: do not copy region buffers if they are null ...................................................................... erasure_layout: do not copy region buffers if they are null memcpy() function expects 2nd parameter to be non-null. Make sure that the pointer is null before passing it to the function. Found-by: scan-build, clang v17.0.6 Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Change-Id: I99aad4119f9c11bea75e3419926cc833bc1f68c5 --- M erasure_layout.c 1 file changed, 8 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/81548/1 diff --git a/erasure_layout.c b/erasure_layout.c index 1dd6b60..762cd64 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -381,11 +381,15 @@ } _end: - memcpy(newcontents + region_start, old_start_buf, old_start - region_start); - memcpy(newcontents + old_end, old_end_buf, region_end - old_end); + if (old_start_buf) { + memcpy(newcontents + region_start, old_start_buf, old_start - region_start); + free(old_start_buf); + } - free(old_start_buf); - free(old_end_buf); + if (old_end_buf) { + memcpy(newcontents + old_end, old_end_buf, region_end - old_end); + free(old_end_buf); + } msg_cinfo("Erase/write done from %"PRIx32" to %"PRIx32"\n", region_start, region_end); return ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/81548?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I99aad4119f9c11bea75e3419926cc833bc1f68c5 Gerrit-Change-Number: 81548 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newchange

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Peter Marheine (Code Review) 28 Mar '24

28 Mar '24

Attention is currently required from: Peter Marheine, Thomas Heijligen. Hello Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81545?usp=email to look at the new patch set (#2). Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... udelay: only use OS time for delays, except on DOS As proposed on the mailing list ("RFC: remove the calibrated delay loop"), this removes the calibrated delay loop and uses OS-based timing functions for all delays because the calibrated delay loop can delay for shorter times than intended. When sleeping this now uses nanosleep() if available, and otherwise usleep(). When busy-looping, it uses clock_gettime() with CLOCK_MONOTONIC or CLOCK_REALTIME depending on availability, and gettimeofday() otherwise. The calibrated delay loop is retained for DOS only, because timer resolution on DJGPP is only about 50 milliseconds. Since typical delays in flashrom are around 10 microseconds, using OS timing there would regress performance by around 500x. The old implementation is reused with some branches removed based on the knowledge that timer resolution will not be better than about 50 milliseconds. Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f --- M Makefile M include/programmer.h M libflashrom.c M meson.build M udelay.c A udelay_dos.c 6 files changed, 203 insertions(+), 171 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/81545/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Peter Marheine (Code Review) 28 Mar '24

28 Mar '24

Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81545?usp=email ) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... udelay: only use OS time for delays, except on DOS As proposed on the mailing list ("RFC: remove the calibrated delay loop"), this removes the calibrated delay loop and uses OS-based timing functions for all delays because the calibrated delay loop can delay for shorter times than intended. When sleeping this now uses nanosleep() if available, and otherwise usleep(). When busy-looping, it uses clock_gettime() with CLOCK_MONOTONIC or CLOCK_REALTIME depending on availability, and gettimeofday() otherwise. The calibrated delay loop is retained for DOS only, because timer resolution on DJGPP is only about 50 milliseconds. Since typical delays in flashrom are around 10 microseconds, using OS timing there would regress performance by around 500x. The old implementation is reused with some branches removed based on the knowledge that timer resolution will not be better than about 50 milliseconds. Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f --- M Makefile M include/programmer.h M libflashrom.c M meson.build M udelay.c A udelay_dos.c 6 files changed, 203 insertions(+), 171 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/81545/1 diff --git a/Makefile b/Makefile index 9ce5906..b7b2837 100644 --- a/Makefile +++ b/Makefile @@ -391,6 +391,11 @@ LIB_OBJS = libflashrom.o layout.o erasure_layout.o flashrom.o udelay.o parallel.o programmer.o programmer_table.o \ helpers.o helpers_fileio.o ich_descriptors.o fmap.o platform/endian_$(ENDIAN).o platform/memaccess.o +ifeq ($(TARGET_OS), DOS) + LIB_OBJS += udelay_dos.o +else + LIB_OBJS += udelay.o +endif ############################################################################### # Frontend related stuff. diff --git a/include/programmer.h b/include/programmer.h index 15df0d4..939c8de 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -219,8 +219,6 @@ #endif /* udelay.c */ -void myusec_delay(unsigned int usecs); -void myusec_calibrate_delay(void); void internal_sleep(unsigned int usecs); void default_delay(unsigned int usecs); diff --git a/libflashrom.c b/libflashrom.c index 4a59e2a..7c885cf 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -36,7 +36,6 @@ { if (perform_selfcheck && selfcheck()) return 1; - myusec_calibrate_delay(); return 0; } diff --git a/meson.build b/meson.build index 7c46de4..1c8316e 100644 --- a/meson.build +++ b/meson.build @@ -100,17 +100,26 @@ 'sst49lfxxxc.c', 'sst_fwhub.c', 'stm50.c', - 'udelay.c', 'w29ee011.c', 'w39.c', 'writeprotect.c', 'writeprotect_ranges.c', ) +# Select an appropriate delay implementation for the target OS +delay_src = files('udelay.c') +if target_machine.system() == 'dos' + delay_src = files('udelay_dos.c') +endif +srcs += delay_src + # check for required symbols if cc.has_function('clock_gettime') add_project_arguments('-DHAVE_CLOCK_GETTIME=1', language : 'c') endif +if cc.has_function('nanosleep') + add_project_arguments('-DHAVE_NANOSLEEP=1', language : 'c') +endif if cc.has_function('strnlen') add_project_arguments('-DHAVE_STRNLEN=1', language : 'c') endif diff --git a/udelay.c b/udelay.c index cb7a9db..14900c7 100644 --- a/udelay.c +++ b/udelay.c @@ -27,8 +27,6 @@ #include "flash.h" #include "programmer.h" -static bool use_clock_gettime = false; - #if HAVE_CLOCK_GETTIME == 1 #ifdef _POSIX_MONOTONIC_CLOCK @@ -40,7 +38,13 @@ static void clock_usec_delay(int usecs) { struct timespec now; - clock_gettime(clock_id, &now); + if (clock_gettime(clock_id, &now)) { + /* Fall back to realtime clock if monotonic doesn't work */ + if (clock_id != CLOCK_REALTIME && errno == EINVAL) { + clock_id = CLOCK_REALTIME; + clock_gettime(clock_id, &now); + } + } const long end_nsec = now.tv_nsec + usecs * 1000L; const struct timespec end = { @@ -52,185 +56,40 @@ } while (now.tv_sec < end.tv_sec || (now.tv_sec == end.tv_sec && now.tv_nsec < end.tv_nsec)); } -static int clock_check_res(void) -{ - struct timespec res; - if (!clock_getres(clock_id, &res)) { - if (res.tv_sec == 0 && res.tv_nsec <= 100) { - msg_pinfo("Using clock_gettime for delay loops (clk_id: %d, resolution: %ldns).\n", - (int)clock_id, res.tv_nsec); - use_clock_gettime = true; - return 1; - } - } else if (clock_id != CLOCK_REALTIME && errno == EINVAL) { - /* Try again with CLOCK_REALTIME. */ - clock_id = CLOCK_REALTIME; - return clock_check_res(); - } - return 0; -} #else -static inline void clock_usec_delay(int usecs) {} -static inline int clock_check_res(void) { return 0; } - -#endif /* HAVE_CLOCK_GETTIME == 1 */ - -/* loops per microsecond */ -static unsigned long micro = 1; - -__attribute__ ((noinline)) void myusec_delay(unsigned int usecs) -{ - unsigned long i; - for (i = 0; i < usecs * micro; i++) { - /* Make sure the compiler doesn't optimize the loop away. */ - __asm__ volatile ("" : : "rm" (i) ); - } -} - -static unsigned long measure_os_delay_resolution(void) -{ - unsigned long timeusec; +static inline void clock_usec_delay(int usecs) { struct timeval start, end; - unsigned long counter = 0; + unsigned long elapsed = 0; gettimeofday(&start, NULL); - timeusec = 0; - while (!timeusec && (++counter < 1000000000)) { + while (elapsed < usecs) { gettimeofday(&end, NULL); - timeusec = 1000000 * (end.tv_sec - start.tv_sec) + + elapsed = 1000000 * (end.tv_sec - start.tv_sec) + (end.tv_usec - start.tv_usec); /* Protect against time going forward too much. */ if ((end.tv_sec > start.tv_sec) && ((end.tv_sec - start.tv_sec) >= LONG_MAX / 1000000 - 1)) - timeusec = 0; + elapsed = 0; /* Protect against time going backwards during leap seconds. */ if ((end.tv_sec < start.tv_sec) || (timeusec > LONG_MAX)) - timeusec = 0; + elapsed = 0; } - return timeusec; } -static unsigned long measure_delay(unsigned int usecs) -{ - unsigned long timeusec; - struct timeval start, end; - - gettimeofday(&start, NULL); - myusec_delay(usecs); - gettimeofday(&end, NULL); - timeusec = 1000000 * (end.tv_sec - start.tv_sec) + - (end.tv_usec - start.tv_usec); - /* Protect against time going forward too much. */ - if ((end.tv_sec > start.tv_sec) && - ((end.tv_sec - start.tv_sec) >= LONG_MAX / 1000000 - 1)) - timeusec = LONG_MAX; - /* Protect against time going backwards during leap seconds. */ - if ((end.tv_sec < start.tv_sec) || (timeusec > LONG_MAX)) - timeusec = 1; - - return timeusec; -} - -void myusec_calibrate_delay(void) -{ - if (clock_check_res()) - return; - - unsigned long count = 1000; - unsigned long timeusec, resolution; - int i, tries = 0; - - msg_pinfo("Calibrating delay loop... "); - resolution = measure_os_delay_resolution(); - if (resolution) { - msg_pdbg("OS timer resolution is %lu usecs, ", resolution); - } else { - msg_pinfo("OS timer resolution is unusable. "); - } - -recalibrate: - count = 1000; - while (1) { - timeusec = measure_delay(count); - if (timeusec > 1000000 / 4) - break; - if (count >= ULONG_MAX / 2) { - msg_pinfo("timer loop overflow, reduced precision. "); - break; - } - count *= 2; - } - tries ++; - - /* Avoid division by zero, but in that case the loop is shot anyway. */ - if (!timeusec) - timeusec = 1; - - /* Compute rounded up number of loops per microsecond. */ - micro = (count * micro) / timeusec + 1; - msg_pdbg("%luM loops per second, ", micro); - - /* Did we try to recalibrate less than 5 times? */ - if (tries < 5) { - /* Recheck our timing to make sure we weren't just hitting - * a scheduler delay or something similar. - */ - for (i = 0; i < 4; i++) { - if (resolution && (resolution < 10)) { - timeusec = measure_delay(100); - } else if (resolution && - (resolution < ULONG_MAX / 200)) { - timeusec = measure_delay(resolution * 10) * - 100 / (resolution * 10); - } else { - /* This workaround should be active for broken - * OS and maybe libpayload. The criterion - * here is horrible or non-measurable OS timer - * resolution which will result in - * measure_delay(100)=0 whereas a longer delay - * (1000 ms) may be sufficient - * to get a nonzero time measurement. - */ - timeusec = measure_delay(1000000) / 10000; - } - if (timeusec < 90) { - msg_pdbg("delay more than 10%% too short (got " - "%lu%% of expected delay), " - "recalculating... ", timeusec); - goto recalibrate; - } - } - } else { - msg_perr("delay loop is unreliable, trying to continue "); - } - - /* We're interested in the actual precision. */ - timeusec = measure_delay(10); - msg_pdbg("10 myus = %ld us, ", timeusec); - timeusec = measure_delay(100); - msg_pdbg("100 myus = %ld us, ", timeusec); - timeusec = measure_delay(1000); - msg_pdbg("1000 myus = %ld us, ", timeusec); - timeusec = measure_delay(10000); - msg_pdbg("10000 myus = %ld us, ", timeusec); - timeusec = measure_delay(resolution * 4); - msg_pdbg("%ld myus = %ld us, ", resolution * 4, timeusec); - - msg_pinfo("OK.\n"); -} +#endif /* HAVE_CLOCK_GETTIME == 1 */ /* Not very precise sleep. */ void internal_sleep(unsigned int usecs) { -#if IS_WINDOWS +#if HAVE_NANOSLEEP == 1 + nanosleep(&(struct timespec){usecs / 1000000, (usecs * 1000) % 1000000000UL}, NULL); +#elif IS_WINDOWS Sleep((usecs + 999) / 1000); -#elif defined(__DJGPP__) +#else sleep(usecs / 1000000); usleep(usecs % 1000000); -#else - nanosleep(&(struct timespec){usecs / 1000000, (usecs * 1000) % 1000000000UL}, NULL); #endif } @@ -240,21 +99,14 @@ /* If the delay is >0.1 s, use internal_sleep because timing does not need to be so precise. */ if (usecs > 100000) { internal_sleep(usecs); - } else if (use_clock_gettime) { - clock_usec_delay(usecs); } else { - myusec_delay(usecs); + clock_usec_delay(usecs); } } #else #include <libpayload.h> -void myusec_calibrate_delay(void) -{ - get_cpu_speed(); -} - void default_delay(unsigned int usecs) { udelay(usecs); diff --git a/udelay_dos.c b/udelay_dos.c new file mode 100644 index 0000000..aa33c0b --- /dev/null +++ b/udelay_dos.c @@ -0,0 +1,169 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2000 Silicon Integrated System Corporation + * Copyright (C) 2009,2010 Carl-Daniel Hailfinger + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <stdbool.h> +#include <unistd.h> +#include <errno.h> +#include <time.h> +#include <sys/time.h> +#include <stdlib.h> +#include <limits.h> +#include "flash.h" +#include "programmer.h" + +/* loops per microsecond */ +static unsigned long micro = 1; + +__attribute__ ((noinline)) void myusec_delay(unsigned int usecs) +{ + unsigned long i; + for (i = 0; i < usecs * micro; i++) { + /* Make sure the compiler doesn't optimize the loop away. */ + __asm__ volatile ("" : : "rm" (i) ); + } +} + +static unsigned long measure_os_delay_resolution(void) +{ + unsigned long timeusec; + struct timeval start, end; + unsigned long counter = 0; + + gettimeofday(&start, NULL); + timeusec = 0; + + while (!timeusec && (++counter < 1000000000)) { + gettimeofday(&end, NULL); + timeusec = 1000000 * (end.tv_sec - start.tv_sec) + + (end.tv_usec - start.tv_usec); + /* Protect against time going forward too much. */ + if ((end.tv_sec > start.tv_sec) && + ((end.tv_sec - start.tv_sec) >= LONG_MAX / 1000000 - 1)) + timeusec = 0; + /* Protect against time going backwards during leap seconds. */ + if ((end.tv_sec < start.tv_sec) || (timeusec > LONG_MAX)) + timeusec = 0; + } + return timeusec; +} + +static unsigned long measure_delay(unsigned int usecs) +{ + unsigned long timeusec; + struct timeval start, end; + + gettimeofday(&start, NULL); + myusec_delay(usecs); + gettimeofday(&end, NULL); + timeusec = 1000000 * (end.tv_sec - start.tv_sec) + + (end.tv_usec - start.tv_usec); + /* Protect against time going forward too much. */ + if ((end.tv_sec > start.tv_sec) && + ((end.tv_sec - start.tv_sec) >= LONG_MAX / 1000000 - 1)) + timeusec = LONG_MAX; + /* Protect against time going backwards during leap seconds. */ + if ((end.tv_sec < start.tv_sec) || (timeusec > LONG_MAX)) + timeusec = 1; + + return timeusec; +} + +void myusec_calibrate_delay(void) +{ + unsigned long count = 1000; + unsigned long timeusec, resolution; + int i, tries = 0; + + msg_pinfo("Calibrating delay loop... "); + /* Timing resolution on DJGPP is about 50ms, but measure it precisely. */ + resolution = measure_os_delay_resolution(); + if (resolution) { + msg_pdbg("OS timer resolution is %lu usecs, ", resolution); + } else { + msg_pinfo("OS timer resolution is unusable. "); + } + +recalibrate: + count = 1000; + while (1) { + timeusec = measure_delay(count); + if (timeusec > 1000000 / 4) + break; + if (count >= ULONG_MAX / 2) { + msg_pinfo("timer loop overflow, reduced precision. "); + break; + } + count *= 2; + } + tries ++; + + /* Avoid division by zero, but in that case the loop is shot anyway. */ + if (!timeusec) + timeusec = 1; + + /* Compute rounded up number of loops per microsecond. */ + micro = (count * micro) / timeusec + 1; + msg_pdbg("%luM loops per second, ", micro); + + /* Did we try to recalibrate less than 5 times? */ + if (tries < 5) { + /* Recheck our timing to make sure we weren't just hitting + * a scheduler delay or something similar. + */ + for (i = 0; i < 4; i++) { + timeusec = measure_delay(resolution * 10) * + 100 / (resolution * 10); + + if (timeusec < 90) { + msg_pdbg("delay more than 10%% too short (got " + "%lu%% of expected delay), " + "recalculating... ", timeusec); + goto recalibrate; + } + } + } else { + msg_perr("delay loop is unreliable, trying to continue "); + } + + /* We're interested in the actual precision. */ + timeusec = measure_delay(resolution * 4); + msg_pdbg("%ld myus = %ld us, ", resolution * 4, timeusec); + + msg_pinfo("OK.\n"); +} + +/* Not very precise sleep. */ +void internal_sleep(unsigned int usecs) +{ + sleep(usecs / 1000000); + usleep(usecs % 1000000); +} + +/* Precise delay. */ +void default_delay(unsigned int usecs) +{ + static bool calibrated = false; + + /* If the delay is >0.1 s, use internal_sleep because timing does not need to be so precise. */ + if (usecs > 100000) { + internal_sleep(usecs); + } else { + if (!calibrated) + myusec_calibrate_delay(); + myusec_delay(usecs); + } +} -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: doc/dev_guide: Add section about Jenkins build, and scan-build
by Anastasia Klimchuk (Code Review) 27 Mar '24

27 Mar '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81261?usp=email ) Change subject: doc/dev_guide: Add section about Jenkins build, and scan-build ...................................................................... doc/dev_guide: Add section about Jenkins build, and scan-build Change-Id: I416b632c55d1ceb925456ac8c8947dfbcef2e888 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81261 Reviewed-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M doc/dev_guide/development_guide.rst 1 file changed, 17 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved Stefan Reinauer: Looks good to me, approved diff --git a/doc/dev_guide/development_guide.rst b/doc/dev_guide/development_guide.rst index 0b64325..aac3a6c 100644 --- a/doc/dev_guide/development_guide.rst +++ b/doc/dev_guide/development_guide.rst @@ -191,6 +191,8 @@ Pushing a patch --------------- +Before pushing a patch, make sure it builds on your environment and all unit tests pass (see :doc:`building_from_source`). + To push patch to Gerrit, use the follow command: :code:`git push upstream HEAD:refs/for/main`. * If using HTTPS you will be prompted for the username and password you @@ -209,6 +211,21 @@ Alternatively, you can add a topic from a Gerrit UI after the patch in pushed (on the top-left section) of patch UI. +Checking the CI +--------------- + +Every patch needs to get a ``Verified +1`` label, typically from Jenkins. Once the patch is pushed +to Gerrit, Jenkins is added automatically and runs its build script. The script builds the patch with +various config options, and runs unit tests (for more details see source code of ``test_build.sh``). +Then, Jenkins gives the patch ``+1`` or ``-1`` vote, indicating success or fail. + +In case of failure, follow Jenkins link (which it adds as a comment to the patch), open Console output, +find the error and try to fix it. + +In addition to building and running unit tests, Jenkins also runs a scan-build over the patch. Ideally +you should check that your patch does not introduce new warnings. To see scan-build report, follow +Jenkins link -> Build artifacts -> scan build link for the given run. + Adding reviewers to the patch ----------------------------- -- To view, visit https://review.coreboot.org/c/flashrom/+/81261?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I416b632c55d1ceb925456ac8c8947dfbcef2e888 Gerrit-Change-Number: 81261 Gerrit-PatchSet: 3 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L12850F
by Alexander Goncharov (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: DZ. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81350?usp=email ) Change subject: flashchips: Add support for MXIC MX25L12850F ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/81350?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I71ac70d273904f94d015401f9d8df587084efad0 Gerrit-Change-Number: 81350 Gerrit-PatchSet: 4 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: DZ <danielzhang(a)mxic.com.cn> Gerrit-Comment-Date: Wed, 27 Mar 2024 15:23:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: classic_cli_manpage.rst: Update doc for custom_rst of raiden_debug_spi
by Anastasia Klimchuk (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81114?usp=email ) Change subject: classic_cli_manpage.rst: Update doc for custom_rst of raiden_debug_spi ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/81114?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ie2b084a3ed9bf40f91bfa81dbc95ec69d99d5ad0 Gerrit-Change-Number: 81114 Gerrit-PatchSet: 3 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 27 Mar 2024 11:25:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: udelay: clock_getres() does not provide clock_gettime() resolution
by Anastasia Klimchuk (Code Review) 27 Mar '24

27 Mar '24

Attention is currently required from: Brian Norris, Chen, Nikolai Artemiev, Peter Marheine, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/80806?usp=email ) Change subject: udelay: clock_getres() does not provide clock_gettime() resolution ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/80806/comment/a69f3f2f_32649c97 : PS2, Line 10: Linux makes this clearer > I looked at what FreeBSD does: […] Peter thank you SO MUCH! I think let's bring Brian to the thread on the mailing list. Brian, are you subscribed to the flashrom mailing list? If not, subscribe (https://flashrom.org/contact.html#mailing-list-1), and join the thread that Peter started, with title "RFC: remove the calibrated delay loop". Thank you! You started a good idea with your patch! :) -- To view, visit https://review.coreboot.org/c/flashrom/+/80806?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icdc6e184eacb14d3bd27029e3fc75be4431d82b4 Gerrit-Change-Number: 80806 Gerrit-PatchSet: 3 Gerrit-Owner: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Chen <c(a)jia.je> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Chen <c(a)jia.je> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 27 Mar 2024 11:23:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Brian Norris <briannorris(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2024