- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: linux_mtd: print correct wp-status info when whole flash is locked
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Julius Werner, Brian Norris, I'd like you to do a code review. Please visit https://review.coreboot.org/22509 to review the following change. Change subject: linux_mtd: print correct wp-status info when whole flash is locked ...................................................................... linux_mtd: print correct wp-status info when whole flash is locked We would never reach the 'end_found' condition if the flash is entirely write-protected. BUG=none TEST=`flashrom --wp-status` when whole flash is write-protected BRANCH=none Change-Id: I0cb1cba321a831f02037fc228ee35aab36de4d4e Signed-off-by: Brian Norris <briannorris(a)chromium.org> Reviewed-on: https://chromium-review.googlesource.com/349245 Reviewed-by: David Hendricks <dhendrix(a)chromium.org> Reviewed-by: Julius Werner <jwerner(a)chromium.org> --- M linux_mtd.c 1 file changed, 7 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/22509/1 diff --git a/linux_mtd.c b/linux_mtd.c index 7d1e286..cd12d29 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -484,8 +484,8 @@ static int mtd_wp_status(const struct flashchip *flash) { - uint32_t start = 0, end = 0; - int start_found = 0, end_found = 0; + uint32_t start = 0, len = 0; + int start_found = 0; unsigned int u; /* For now, assume only one contiguous region can be locked (NOR) */ @@ -506,23 +506,20 @@ start = erase_info.start; start_found = 1; } + len += mtd_erasesize; } else if (rc == 0) { if (start_found) { - end = erase_info.start; - end_found = 1; + /* TODO: changes required for supporting non-contiguous locked regions */ + break; } } - if (start_found && end_found) { - /* TODO: changes required for supporting non-contiguous locked regions */ - break; - } } msg_cinfo("WP: write protect is %s.\n", - (start_found && end_found)? "enabled": "disabled"); + start_found ? "enabled": "disabled"); msg_pinfo("WP: write protect range: start=0x%08x, " - "len=0x%08x\n", start, end - start); + "len=0x%08x\n", start, len); return 0; } -- To view, visit https://review.coreboot.org/22509 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0cb1cba321a831f02037fc228ee35aab36de4d4e Gerrit-Change-Number: 22509 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>

1 0

Change in flashrom[master]: mtd: print a hint for users when lock/unlock fails
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Brian Norris, I'd like you to do a code review. Please visit https://review.coreboot.org/22510 to review the following change. Change subject: mtd: print a hint for users when lock/unlock fails ...................................................................... mtd: print a hint for users when lock/unlock fails Some people have trouble with error messages. Let's make this one more obvious. BRANCH=none BUG=chrome-os-partner:54736 TEST=`flashrom --wp-disable`, when WP# is asserted (and therefore you can't change the flash Status Register) Change-Id: I4fe40c88da2faad9a9df40da9f9df216ed5119af Signed-off-by: Brian Norris <briannorris(a)chromium.org> Reviewed-on: https://chromium-review.googlesource.com/363925 Reviewed-by: David Hendricks <dhendrix(a)chromium.org> --- M linux_mtd.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/22510/1 diff --git a/linux_mtd.c b/linux_mtd.c index cd12d29..99a7ec0 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -450,6 +450,7 @@ if (ioctl(dev_fd, MEMUNLOCK, &entire_chip) == -1) { msg_perr("%s: Failed to disable write-protection, ioctl: %s\n", __func__, strerror(errno)); + msg_perr("Did you disable WP#?\n"); return 1; } @@ -476,6 +477,7 @@ if (ioctl(dev_fd, MEMUNLOCK, &erase_info) == -1) { msg_perr("%s: ioctl: %s\n", __func__, strerror(errno)); + msg_perr("Did you disable WP#?\n"); return 1; } -- To view, visit https://review.coreboot.org/22510 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4fe40c88da2faad9a9df40da9f9df216ed5119af Gerrit-Change-Number: 22510 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>

1 0

Change in flashrom[master]: linux_mtd: do reads in eraseblock-sized chunks
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Brian Norris, I'd like you to do a code review. Please visit https://review.coreboot.org/22508 to review the following change. Change subject: linux_mtd: do reads in eraseblock-sized chunks ...................................................................... linux_mtd: do reads in eraseblock-sized chunks It's probably not the best idea to try to do an 8MB read in one syscall. Theoretically, this should work; but MTD just relies on the SPI driver to deliver the whole read in one transfer, and many SPI drivers haven't been tested well with large transfer sizes. I'd consider this a workaround, but it's still good to have IMO. BUG=chrome-os-partner:53215 TEST=boot kevin; `flashrom --read ...` TEST=check for performance regression on oak BRANCH=none Change-Id: I3a8e160795cd24ec1851c00394892536a2a9d0c7 Signed-off-by: Brian Norris <briannorris(a)chromium.org> Reviewed-on: https://chromium-review.googlesource.com/344006 Reviewed-by: David Hendricks <dhendrix(a)chromium.org> --- M linux_mtd.c 1 file changed, 15 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/22508/1 diff --git a/linux_mtd.c b/linux_mtd.c index f09c572..7d1e286 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -187,15 +187,26 @@ static int linux_mtd_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { + unsigned int eb_size = flash->chip->block_erasers[0].eraseblocks[0].size; + unsigned int i; + if (lseek(dev_fd, start, SEEK_SET) != start) { msg_perr("Cannot seek to 0x%06x: %s\n", start, strerror(errno)); return 1; } - if (read(dev_fd, buf, len) != len) { - msg_perr("Cannot read 0x%06x bytes at 0x%06x: %s\n", - start, len, strerror(errno)); - return 1; + for (i = 0; i < len; ) { + /* Try to align reads to eraseblock size */ + unsigned int step = eb_size - ((start + i) % eb_size); + step = min(step, len - i); + + if (read(dev_fd, buf + i, step) != step) { + msg_perr("Cannot read 0x%06x bytes at 0x%06x: %s\n", + step, start + i, strerror(errno)); + return 1; + } + + i += step; } return 0; @@ -265,8 +276,6 @@ } static struct opaque_master opaque_master_linux_mtd = { - /* FIXME: Do we need to change these (especially write) as per - * page size requirements? */ .max_data_read = MAX_DATA_UNSPECIFIED, .max_data_write = MAX_DATA_UNSPECIFIED, .probe = linux_mtd_probe, -- To view, visit https://review.coreboot.org/22508 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3a8e160795cd24ec1851c00394892536a2a9d0c7 Gerrit-Change-Number: 22508 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>

1 0

Change in flashrom[master]: linux_mtd: display correct write-protect info
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/22507 Change subject: linux_mtd: display correct write-protect info ...................................................................... linux_mtd: display correct write-protect info The wp_status call to linux_mtd currently display nothing if write-protect is disabled. Display correct message and match the output with other driver. BUG=chrome-os-partner:40208 TEST=`flashrom --wp-disable; flashrom --wp-status` should display: WP: write protect is disabled. WP: write protect range: start=0x00000000, len=0x00000000 Change-Id: Icc3727702d2813b875a7e3a7151b44a7853967bd Reviewed-on: https://chromium-review.googlesource.com/340882 Commit-Ready: Wei-Ning Huang <wnhuang(a)chromium.org> Tested-by: Wei-Ning Huang <wnhuang(a)chromium.org> Reviewed-by: Ricky Liang <jcliang(a)chromium.org> Reviewed-by: David Hendricks <dhendrix(a)chromium.org> --- M linux_mtd.c 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/22507/1 diff --git a/linux_mtd.c b/linux_mtd.c index 6055aef..f09c572 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -505,14 +505,16 @@ } if (start_found && end_found) { - msg_pinfo("WP: write protect range: start=0x%08x, " - "len=0x%08x\n", start, end - start); - /* TODO: Replace this break with "start = end = 0" if - * we want to support non-contiguous locked regions */ + /* TODO: changes required for supporting non-contiguous locked regions */ break; } } + msg_cinfo("WP: write protect is %s.\n", + (start_found && end_found)? "enabled": "disabled"); + msg_pinfo("WP: write protect range: start=0x%08x, " + "len=0x%08x\n", start, end - start); + return 0; } -- To view, visit https://review.coreboot.org/22507 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Icc3727702d2813b875a7e3a7151b44a7853967bd Gerrit-Change-Number: 22507 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>

1 0

Change in flashrom[master]: linux_mtd: fix wp range detection in mtd_wp_status
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Nicolas Boichat, I'd like you to do a code review. Please visit https://review.coreboot.org/22506 to review the following change. Change subject: linux_mtd: fix wp range detection in mtd_wp_status ...................................................................... linux_mtd: fix wp range detection in mtd_wp_status The logic in wp range detection in mtd_wp_status is wrong. The first block (start == 0) may possible be write-protected, thus we can't use 'if (!start)' for detecting start of write-protection. Since start and end are both uint32_t, we add two new int variables to flag the status. Signed-off-by: Wei-Ning Huang <wnhuang(a)google.com> BUG=chrome-os-partner:40208 TEST=`flashrom --wp-enable --wp-range 0x0 0x200000` then `flashrom --wp-status` should display: 'WP: write protect range: start=0x00000000, len=0x00200000' Change-Id: Id8934c2ba997ff8558a9be3a5aa05b56272c9e85 Reviewed-on: https://chromium-review.googlesource.com/330225 Commit-Ready: Wei-Ning Huang <wnhuang(a)chromium.org> Tested-by: Wei-Ning Huang <wnhuang(a)chromium.org> Reviewed-by: David Hendricks <dhendrix(a)chromium.org> Reviewed-by: Nicolas Boichat <drinkcat(a)chromium.org> --- M linux_mtd.c 1 file changed, 9 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/22506/1 diff --git a/linux_mtd.c b/linux_mtd.c index c707167..6055aef 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -476,6 +476,7 @@ static int mtd_wp_status(const struct flashchip *flash) { uint32_t start = 0, end = 0; + int start_found = 0, end_found = 0; unsigned int u; /* For now, assume only one contiguous region can be locked (NOR) */ @@ -492,14 +493,18 @@ msg_perr("%s: ioctl: %s\n", __func__, strerror(errno)); return 1; } else if (rc == 1) { - if (!start) + if (!start_found) { start = erase_info.start; + start_found = 1; + } } else if (rc == 0) { - if (start) - end = erase_info.start - 1; + if (start_found) { + end = erase_info.start; + end_found = 1; + } } - if (start && end) { + if (start_found && end_found) { msg_pinfo("WP: write protect range: start=0x%08x, " "len=0x%08x\n", start, end - start); /* TODO: Replace this break with "start = end = 0" if -- To view, visit https://review.coreboot.org/22506 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id8934c2ba997ff8558a9be3a5aa05b56272c9e85 Gerrit-Change-Number: 22506 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nicolas Boichat <drinkcat(a)chromium.org> Gerrit-Reviewer: Wei-Ning Huang <wnhuang(a)google.com>

1 0

Change in flashrom[master]: linux_mtd: fix mtd_wp_status
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Daniel Kurtz, I'd like you to do a code review. Please visit https://review.coreboot.org/22505 to review the following change. Change subject: linux_mtd: fix mtd_wp_status ...................................................................... linux_mtd: fix mtd_wp_status Fix invalid loop range in mtd_wp_status, which ends the for loop prematurely. BUG=chrome-os-partner:40208 TEST=`flashrom --wp-status` should output something like: 'WP: write protect range: start=0x00001000, len=0x0000efff' Change-Id: Ic582bb034c0d2f811813fb6a25696ccddef43e8f Reviewed-on: https://chromium-review.googlesource.com/329927 Commit-Ready: Wei-Ning Huang <wnhuang(a)chromium.org> Tested-by: Wei-Ning Huang <wnhuang(a)chromium.org> Reviewed-by: Daniel Kurtz <djkurtz(a)chromium.org> --- M linux_mtd.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/22505/1 diff --git a/linux_mtd.c b/linux_mtd.c index d8d835d..c707167 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -480,7 +480,7 @@ /* For now, assume only one contiguous region can be locked (NOR) */ /* FIXME: use flash struct members instead of raw MTD values here */ - for (u = 0; u < mtd_total_size / mtd_erasesize; u += mtd_erasesize) { + for (u = 0; u < mtd_total_size; u += mtd_erasesize) { int rc; struct erase_info_user erase_info = { .start = u, -- To view, visit https://review.coreboot.org/22505 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ic582bb034c0d2f811813fb6a25696ccddef43e8f Gerrit-Change-Number: 22505 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)chromium.org> Gerrit-Reviewer: Wei-Ning Huang <wnhuang(a)google.com>

1 0

Change in flashrom[master]: MTD write-protect support
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Brian Norris, I'd like you to do a code review. Please visit https://review.coreboot.org/22504 to review the following change. Change subject: MTD write-protect support ...................................................................... MTD write-protect support ** WIP: Do not merge (yet) ** This adds MTD write protect functionality. Caveats: - --wp-range and --wp-enable must be used simultaneously - --wp-disable will clear the chip's block protect range by default. If the user specifies a range using --wp-range in the same invocation to flashrom, then that range will be cleared instead. BUG=chrome-os-partner:40208,chromium:523575 BRANCH=none TEST=needs testing. Change-Id: Ifed2ad510a70fe1944f93f48e411769977afb5ca Signed-off-by: David Hendricks <dhendrix(a)chromium.org> Reviewed-on: https://chromium-review.googlesource.com/275381 Reviewed-by: Brian Norris <briannorris(a)chromium.org> --- M linux_mtd.c 1 file changed, 150 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/22504/1 diff --git a/linux_mtd.c b/linux_mtd.c index 064065f..d8d835d 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -34,6 +34,9 @@ #include "file.h" #include "flash.h" #include "programmer.h" +#if 0 +#include "writeprotect.h" +#endif #define LINUX_DEV_ROOT "/dev" #define LINUX_MTD_SYSFS_ROOT "/sys/class/mtd" @@ -49,6 +52,10 @@ static unsigned long int mtd_total_size; static unsigned long int mtd_numeraseregions; static unsigned long int mtd_erasesize; /* only valid if numeraseregions is 0 */ + +#if 0 +static struct wp wp_mtd; /* forward declaration */ +#endif /* read a string from a sysfs file and sanitize it */ static int read_sysfs_string(const char *filename, char *buf, int len) @@ -171,6 +178,9 @@ flash->chip->block_erasers[0].eraseblocks[0].size = mtd_erasesize; flash->chip->block_erasers[0].eraseblocks[0].count = mtd_total_size / mtd_erasesize; +#if 0 + flash->chip->wp = &wp_mtd; +#endif return 1; } @@ -369,3 +379,143 @@ msg_pdbg("%s: %s\n", __func__, ret == 0 ? "success." : "failed."); return ret; } + +#if 0 +/* + * Write-protect functions. + */ +static int mtd_wp_list_ranges(const struct flashchip *flash) +{ + /* TODO: implement this */ + msg_perr("--wp-list is not currently implemented for MTD.\n"); + return 1; +} + +/* + * We only have MEMLOCK to enable write-protection for a particular block, + * so we need to do force the user to use --wp-range and --wp-enable + * command-line arguments simultaneously. (Fortunately, CrOS factory + * installer does this already). + * + * The --wp-range argument is processed first and will set these variables + * which --wp-enable will use afterward. + */ +static unsigned int wp_range_start; +static unsigned int wp_range_len; +static int wp_set_range_called = 0; + +static int mtd_wp_set_range(const struct flashchip *flash, + unsigned int start, unsigned int len) +{ + wp_range_start = start; + wp_range_len = len; + + wp_set_range_called = 1; + return 0; +} + +static int mtd_wp_enable_writeprotect(const struct flashchip *flash, enum wp_mode mode) +{ + struct erase_info_user entire_chip = { + .start = 0, + .length = mtd_total_size, + }; + struct erase_info_user desired_range = { + .start = wp_range_start, + .length = wp_range_len, + }; + + if (!wp_set_range_called) { + msg_perr("For MTD, --wp-range and --wp-enable must be " + "used simultaneously.\n"); + return 1; + } + + /* + * MTD handles write-protection additively, so whatever new range is + * specified is added to the range which is currently protected. To be + * consistent with flashrom behavior with other programmer interfaces, + * we need to disable the current write protection and then enable + * it for the desired range. + */ + if (ioctl(dev_fd, MEMUNLOCK, &entire_chip) == -1) { + msg_perr("%s: Failed to disable write-protection, ioctl: %s\n", + __func__, strerror(errno)); + return 1; + } + + if (ioctl(dev_fd, MEMLOCK, &desired_range) == -1) { + msg_perr("%s: Failed to enable write-protection, ioctl: %s\n", + __func__, strerror(errno)); + return 1; + } + + return 0; +} + +static int mtd_wp_disable_writeprotect(const struct flashchip *flash) +{ + struct erase_info_user erase_info; + + if (wp_set_range_called) { + erase_info.start = wp_range_start; + erase_info.length = wp_range_len; + } else { + erase_info.start = 0; + erase_info.length = mtd_total_size; + } + + if (ioctl(dev_fd, MEMUNLOCK, &erase_info) == -1) { + msg_perr("%s: ioctl: %s\n", __func__, strerror(errno)); + return 1; + } + + return 0; +} + +static int mtd_wp_status(const struct flashchip *flash) +{ + uint32_t start = 0, end = 0; + unsigned int u; + + /* For now, assume only one contiguous region can be locked (NOR) */ + /* FIXME: use flash struct members instead of raw MTD values here */ + for (u = 0; u < mtd_total_size / mtd_erasesize; u += mtd_erasesize) { + int rc; + struct erase_info_user erase_info = { + .start = u, + .length = mtd_erasesize, + }; + + rc = ioctl(dev_fd, MEMISLOCKED, &erase_info); + if (rc < 0) { + msg_perr("%s: ioctl: %s\n", __func__, strerror(errno)); + return 1; + } else if (rc == 1) { + if (!start) + start = erase_info.start; + } else if (rc == 0) { + if (start) + end = erase_info.start - 1; + } + + if (start && end) { + msg_pinfo("WP: write protect range: start=0x%08x, " + "len=0x%08x\n", start, end - start); + /* TODO: Replace this break with "start = end = 0" if + * we want to support non-contiguous locked regions */ + break; + } + } + + return 0; +} + +static struct wp wp_mtd = { + .list_ranges = mtd_wp_list_ranges, + .set_range = mtd_wp_set_range, + .enable = mtd_wp_enable_writeprotect, + .disable = mtd_wp_disable_writeprotect, + .wp_status = mtd_wp_status, +}; +#endif -- To view, visit https://review.coreboot.org/22504 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifed2ad510a70fe1944f93f48e411769977afb5ca Gerrit-Change-Number: 22504 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>

1 0

Change in flashrom[master]: linux_mtd: Fix compilation errors
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/22502 Change subject: linux_mtd: Fix compilation errors ...................................................................... linux_mtd: Fix compilation errors This fixes compilation errors from the initial import patch. Change-Id: I216fab281f7a20a5819de3adf610b770a0b3d37c Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M linux_mtd.c 1 file changed, 26 insertions(+), 37 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/22502/1 diff --git a/linux_mtd.c b/linux_mtd.c index e1b11cd..07db2e9 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -22,6 +22,7 @@ #include <fcntl.h> #include <inttypes.h> #include <libgen.h> +#include <limits.h> #include <stdio.h> #include <stdlib.h> #include <mtd/mtd-user.h> @@ -42,32 +43,12 @@ static int dev_fd = -1; -static char *mtd_device_name; static int mtd_device_is_writeable; /* Size info is presented in bytes in sysfs. */ static unsigned long int mtd_total_size; static unsigned long int mtd_numeraseregions; static unsigned long int mtd_erasesize; /* only valid if numeraseregions is 0 */ - -static int stat_mtd_files(char *dev_path, char *sysfs_path) -{ - struct stat s; - - errno = 0; - if (stat(dev_path, &s) < 0) { - msg_pdbg("Cannot stat \"%s\": %s\n", dev_path, strerror(errno)); - return 1; - } - - if (lstat(sysfs_path, &s) < 0) { - msg_pdbg("Cannot stat \"%s\" : %s\n", - sysfs_path, strerror(errno)); - return 1; - } - - return 0; -} /* read a string from a sysfs file and sanitize it */ static int read_sysfs_string(const char *filename, char *buf, int len) @@ -109,7 +90,6 @@ static int read_sysfs_int(const char *filename, unsigned long int *val) { - uint32_t tmp; char buf[32]; char *endptr; @@ -181,17 +161,20 @@ return 0; } -static int linux_mtd_probe(struct flashchip *flash) +static int linux_mtd_probe(struct flashctx *flash) { - flash->tested = TEST_OK_PREW; - flash->total_size = mtd_total_size / 1024; /* bytes -> kB */ - flash->block_erasers[0].eraseblocks[0].size = mtd_erasesize; - flash->block_erasers[0].eraseblocks[0].count = + flash->chip->tested.probe = OK; + flash->chip->tested.read = OK; + flash->chip->tested.erase = OK; + flash->chip->tested.write = OK; + flash->chip->total_size = mtd_total_size / 1024; /* bytes -> kB */ + flash->chip->block_erasers[0].eraseblocks[0].size = mtd_erasesize; + flash->chip->block_erasers[0].eraseblocks[0].count = mtd_total_size / mtd_erasesize; return 1; } -static int linux_mtd_read(struct flashchip *flash, uint8_t *buf, +static int linux_mtd_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { if (lseek(dev_fd, start, SEEK_SET) != start) { @@ -209,13 +192,13 @@ } /* this version assumes we must divide the write request into pages ourselves */ -static int linux_mtd_write(struct flashchip *flash, uint8_t *buf, +static int linux_mtd_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) { unsigned int page; unsigned int chunksize, page_size; - chunksize = page_size = flash->page_size; + chunksize = page_size = flash->chip->page_size; if (!mtd_device_is_writeable) return 1; @@ -246,10 +229,9 @@ return 0; } -static int linux_mtd_erase(struct flashchip *flash, +static int linux_mtd_erase(struct flashctx *flash, unsigned int start, unsigned int len) { - struct region_info_user region_info; uint32_t u; if (mtd_numeraseregions != 0) { @@ -272,7 +254,7 @@ return 0; } -static struct opaque_programmer programmer_linux_mtd = { +static struct opaque_master opaque_master_linux_mtd = { /* FIXME: Do we need to change these (especially write) as per * page size requirements? */ .max_data_read = MAX_DATA_UNSPECIFIED, @@ -318,8 +300,18 @@ msg_pdbg("%s: sysfs_path: \"%s\", dev_path: \"%s\"\n", __func__, sysfs_path, dev_path); - if (stat_mtd_files(dev_path, sysfs_path)) + struct stat s; + errno = 0; + if (stat(dev_path, &s) < 0) { + msg_pdbg("Cannot stat \"%s\": %s\n", dev_path, strerror(errno)); goto linux_mtd_setup_exit; + } + + if (lstat(sysfs_path, &s) < 0) { + msg_pdbg("Cannot stat \"%s\" : %s\n", + sysfs_path, strerror(errno)); + goto linux_mtd_setup_exit; + } if (get_mtd_info()) goto linux_mtd_setup_exit; @@ -351,9 +343,6 @@ int dev_num = -1; /* linux_mtd_setup will search if dev_num < 0 */ int ret = 1; - if (alias && alias->type != ALIAS_HOST) - return 1; - param = extract_programmer_param("dev"); if (param) { char *endptr; @@ -373,7 +362,7 @@ if (register_shutdown(linux_mtd_shutdown, NULL)) goto linux_mtd_init_exit; - register_opaque_programmer(&programmer_linux_mtd); + register_opaque_master(&opaque_master_linux_mtd); ret = 0; linux_mtd_init_exit: -- To view, visit https://review.coreboot.org/22502 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I216fab281f7a20a5819de3adf610b770a0b3d37c Gerrit-Change-Number: 22502 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>

1 0

Change in flashrom[master]: linux_mtd: Suppress message if NOR device not found
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Shawn Nematbakhsh, I'd like you to do a code review. Please visit https://review.coreboot.org/22503 to review the following change. Change subject: linux_mtd: Suppress message if NOR device not found ...................................................................... linux_mtd: Suppress message if NOR device not found This just suppresses a message that might cause confusion for unsuspecting users. BUG=none BRANCH=none TEST=ran on veyron_mickey, "NOR type device not found" message no longer appears under normal circumstances. Signed-off-by: David Hendricks <dhendrix(a)chromium.org> Change-Id: I76d50847e6c4a431d30ad39bcd3994059679172d Reviewed-on: https://chromium-review.googlesource.com/302145 Commit-Ready: David Hendricks <dhendrix(a)chromium.org> Tested-by: David Hendricks <dhendrix(a)chromium.org> Reviewed-by: Shawn N <shawnn(a)chromium.org> --- M linux_mtd.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/22503/1 diff --git a/linux_mtd.c b/linux_mtd.c index 07db2e9..064065f 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -276,7 +276,7 @@ tmp = (char *)scanft(LINUX_MTD_SYSFS_ROOT, "type", "nor", 1); if (!tmp) { - msg_perr("%s: NOR type device not found.\n", __func__); + msg_pdbg("%s: NOR type device not found.\n", __func__); goto linux_mtd_setup_exit; } -- To view, visit https://review.coreboot.org/22503 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I76d50847e6c4a431d30ad39bcd3994059679172d Gerrit-Change-Number: 22503 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Shawn Nematbakhsh <shawnn(a)chromium.org>

1 0

Change in flashrom[master]: Initial MTD support
by David Hendricks (Code Review) 17 Nov '17

17 Nov '17

Hello Shawn Nematbakhsh, I'd like you to do a code review. Please visit https://review.coreboot.org/22501 to review the following change. Change subject: Initial MTD support ...................................................................... Initial MTD support (This patch was ported from chromiumos @cebee89a) This adds MTD support to flashrom so that we can read, erase, and write content on a NOR flash chip via MTD. BUG=chrome-os-partner:40208 BRANCH=none TEST=read, write, and erase works on Oak Signed-off-by: David Hendricks <dhendrix(a)chromium.org> Change-Id: Ib3b8963874722ea80299e9298101409408d6c253 Reviewed-on: https://chromium-review.googlesource.com/272983 Reviewed-by: Shawn N <shawnn(a)chromium.org> --- M Makefile M flashrom.c M internal.c A linux_mtd.c M programmer.h 5 files changed, 416 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/01/22501/1 diff --git a/Makefile b/Makefile index 26bf343..8b2f7c5 100644 --- a/Makefile +++ b/Makefile @@ -623,7 +623,8 @@ # Always enable Marvell SATA controllers for now. CONFIG_SATAMV ?= yes -# Enable Linux spidev interface by default. We disable it on non-Linux targets. +# Enable Linux spidev and MTD interfaces by default. We disable them on non-Linux targets. +CONFIG_LINUX_MTD ?= yes CONFIG_LINUX_SPI ?= yes # Always enable ITE IT8212F PATA controllers for now. @@ -901,6 +902,11 @@ NEED_LIBPCI += CONFIG_SATAMV endif +ifeq ($(CONFIG_LINUX_MTD), yes) +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'") diff --git a/flashrom.c b/flashrom.c index 4fe1843..6329041 100644 --- a/flashrom.c +++ b/flashrom.c @@ -344,6 +344,17 @@ }, #endif +#if CONFIG_LINUX_MTD == 1 + { + .name = "linux_mtd", + .type = OTHER, + .init = linux_mtd_init, + .map_flash_region = fallback_map, + .unmap_flash_region = fallback_unmap, + .delay = internal_delay, + }, +#endif + #if CONFIG_LINUX_SPI == 1 { .name = "linux_spi", diff --git a/internal.c b/internal.c index 4062b64..5395178 100644 --- a/internal.c +++ b/internal.c @@ -243,6 +243,11 @@ if (pci_init_common() != 0) return 1; +#if CONFIG_LINUX_MTD == 1 + if (!linux_mtd_init()) + return 0; +#endif + if (processor_flash_enable()) { msg_perr("Processor detection/init failed.\n" "Aborting.\n"); diff --git a/linux_mtd.c b/linux_mtd.c new file mode 100644 index 0000000..e1b11cd --- /dev/null +++ b/linux_mtd.c @@ -0,0 +1,382 @@ +/* + * This file is part of the flashrom project. + * + * Copyright 2015 Google Inc. + * + * 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; version 2 of the License. + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <ctype.h> +#include <errno.h> +#include <fcntl.h> +#include <inttypes.h> +#include <libgen.h> +#include <stdio.h> +#include <stdlib.h> +#include <mtd/mtd-user.h> +#include <string.h> +#include <sys/ioctl.h> +#include <sys/stat.h> +#include <unistd.h> + +#include "file.h" +#include "flash.h" +#include "programmer.h" + +#define LINUX_DEV_ROOT "/dev" +#define LINUX_MTD_SYSFS_ROOT "/sys/class/mtd" + +/* enough space for LINUX_MTD_SYSFS_ROOT + directory name + filename */ +static char sysfs_path[PATH_MAX]; + +static int dev_fd = -1; + +static char *mtd_device_name; +static int mtd_device_is_writeable; + +/* Size info is presented in bytes in sysfs. */ +static unsigned long int mtd_total_size; +static unsigned long int mtd_numeraseregions; +static unsigned long int mtd_erasesize; /* only valid if numeraseregions is 0 */ + +static int stat_mtd_files(char *dev_path, char *sysfs_path) +{ + struct stat s; + + errno = 0; + if (stat(dev_path, &s) < 0) { + msg_pdbg("Cannot stat \"%s\": %s\n", dev_path, strerror(errno)); + return 1; + } + + if (lstat(sysfs_path, &s) < 0) { + msg_pdbg("Cannot stat \"%s\" : %s\n", + sysfs_path, strerror(errno)); + return 1; + } + + return 0; +} + +/* read a string from a sysfs file and sanitize it */ +static int read_sysfs_string(const char *filename, char *buf, int len) +{ + int fd, bytes_read, i; + char path[strlen(LINUX_MTD_SYSFS_ROOT) + 32]; + + snprintf(path, sizeof(path), "%s/%s", sysfs_path, filename); + + if ((fd = open(path, O_RDONLY)) < 0) { + msg_perr("Cannot open %s\n", path); + return 1; + } + + if ((bytes_read = read(fd, buf, len - 1)) < 0) { + msg_perr("Cannot read %s\n", path); + close(fd); + return 1; + } + + buf[bytes_read] = '\0'; + + /* + * Files from sysfs sometimes contain a newline or other garbage that + * can confuse functions like strtoul() and ruin formatting in print + * statements. Replace the first non-printable character (space is + * considered printable) with a proper string terminator. + */ + for (i = 0; i < len; i++) { + if (!isprint(buf[i])) { + buf[i] = '\0'; + break; + } + } + + close(fd); + return 0; +} + +static int read_sysfs_int(const char *filename, unsigned long int *val) +{ + uint32_t tmp; + char buf[32]; + char *endptr; + + if (read_sysfs_string(filename, buf, sizeof(buf))) + return 1; + + errno = 0; + *val = strtoul(buf, &endptr, 0); + if (endptr != &buf[strlen(buf)]) { + msg_perr("Error reading %s\n", filename); + return 1; + } + + if (errno) { + msg_perr("Error reading %s: %s\n", filename, strerror(errno)); + return 1; + } + + return 0; +} + +/* returns 0 to indicate success, non-zero to indicate error */ +static int get_mtd_info(void) +{ + unsigned long int tmp; + char mtd_device_name[32]; + + /* Flags */ + if (read_sysfs_int("flags", &tmp)) + return 1; + if (tmp & MTD_WRITEABLE) { + /* cache for later use by write function */ + mtd_device_is_writeable = 1; + } + + /* Device name */ + if (read_sysfs_string("name", mtd_device_name, sizeof(mtd_device_name))) + return 1; + + /* Total size */ + if (read_sysfs_int("size", &mtd_total_size)) + return 1; + if (__builtin_popcount(mtd_total_size) != 1) { + msg_perr("MTD size is not a power of 2\n"); + return 1; + } + + /* Erase size */ + if (read_sysfs_int("erasesize", &mtd_erasesize)) + return 1; + if (__builtin_popcount(mtd_erasesize) != 1) { + msg_perr("MTD erase size is not a power of 2\n"); + return 1; + } + + /* Erase regions */ + if (read_sysfs_int("numeraseregions", &mtd_numeraseregions)) + return 1; + if (mtd_numeraseregions != 0) { + msg_perr("Non-uniform eraseblock size is unsupported.\n"); + return 1; + } + + msg_pspew("%s: device_name: \"%s\", is_writeable: %d, " + "numeraseregions: %lu, total_size: %lu, erasesize: %lu\n", + __func__, mtd_device_name, mtd_device_is_writeable, + mtd_numeraseregions, mtd_total_size, mtd_erasesize); + + return 0; +} + +static int linux_mtd_probe(struct flashchip *flash) +{ + flash->tested = TEST_OK_PREW; + flash->total_size = mtd_total_size / 1024; /* bytes -> kB */ + flash->block_erasers[0].eraseblocks[0].size = mtd_erasesize; + flash->block_erasers[0].eraseblocks[0].count = + mtd_total_size / mtd_erasesize; + return 1; +} + +static int linux_mtd_read(struct flashchip *flash, uint8_t *buf, + unsigned int start, unsigned int len) +{ + if (lseek(dev_fd, start, SEEK_SET) != start) { + msg_perr("Cannot seek to 0x%06x: %s\n", start, strerror(errno)); + return 1; + } + + if (read(dev_fd, buf, len) != len) { + msg_perr("Cannot read 0x%06x bytes at 0x%06x: %s\n", + start, len, strerror(errno)); + return 1; + } + + return 0; +} + +/* this version assumes we must divide the write request into pages ourselves */ +static int linux_mtd_write(struct flashchip *flash, uint8_t *buf, + unsigned int start, unsigned int len) +{ + unsigned int page; + unsigned int chunksize, page_size; + + chunksize = page_size = flash->page_size; + + if (!mtd_device_is_writeable) + return 1; + + for (page = start / page_size; + page <= (start + len - 1) / page_size; page++) { + unsigned int i, starthere, lenhere; + + starthere = max(start, page * page_size); + lenhere = min(start + len, (page + 1) * page_size) - starthere; + for (i = 0; i < lenhere; i += chunksize) { + unsigned int towrite = min(chunksize, lenhere - i); + + if (lseek(dev_fd, starthere, SEEK_SET) != starthere) { + msg_perr("Cannot seek to 0x%06x: %s\n", + start, strerror(errno)); + return 1; + } + + if (write(dev_fd, &buf[starthere - start], towrite) != towrite) { + msg_perr("Cannot read 0x%06x bytes at 0x%06x: " + "%s\n", start, len, strerror(errno)); + return 1; + } + } + } + + return 0; +} + +static int linux_mtd_erase(struct flashchip *flash, + unsigned int start, unsigned int len) +{ + struct region_info_user region_info; + uint32_t u; + + if (mtd_numeraseregions != 0) { + /* TODO: Support non-uniform eraseblock size using + use MEMGETREGIONCOUNT/MEMGETREGIONINFO ioctls */ + } + + for (u = 0; u < len; u += mtd_erasesize) { + struct erase_info_user erase_info = { + .start = start + u, + .length = mtd_erasesize, + }; + + if (ioctl(dev_fd, MEMERASE, &erase_info) == -1) { + msg_perr("%s: ioctl: %s\n", __func__, strerror(errno)); + return 1; + } + } + + return 0; +} + +static struct opaque_programmer programmer_linux_mtd = { + /* FIXME: Do we need to change these (especially write) as per + * page size requirements? */ + .max_data_read = MAX_DATA_UNSPECIFIED, + .max_data_write = MAX_DATA_UNSPECIFIED, + .probe = linux_mtd_probe, + .read = linux_mtd_read, + .write = linux_mtd_write, + .erase = linux_mtd_erase, +}; + +/* Returns 0 if setup is successful, non-zero to indicate error */ +static int linux_mtd_setup(int dev_num) +{ + char dev_path[16]; /* "/dev/mtdN" */ + int ret = 1; + + if (dev_num < 0) { + char *tmp, *p; + + tmp = (char *)scanft(LINUX_MTD_SYSFS_ROOT, "type", "nor", 1); + if (!tmp) { + msg_perr("%s: NOR type device not found.\n", __func__); + goto linux_mtd_setup_exit; + } + + /* "tmp" should be something like "/sys/blah/mtdN/type" */ + p = tmp + strlen(LINUX_MTD_SYSFS_ROOT); + while (p[0] == '/') + p++; + + if (sscanf(p, "mtd%d", &dev_num) != 1) { + msg_perr("Can't obtain device number from \"%s\"\n", p); + free(tmp); + goto linux_mtd_setup_exit; + } + free(tmp); + } + + snprintf(sysfs_path, sizeof(sysfs_path), "%s/mtd%d", + LINUX_MTD_SYSFS_ROOT, dev_num); + snprintf(dev_path, sizeof(dev_path), "%s/mtd%d", + LINUX_DEV_ROOT, dev_num); + msg_pdbg("%s: sysfs_path: \"%s\", dev_path: \"%s\"\n", + __func__, sysfs_path, dev_path); + + if (stat_mtd_files(dev_path, sysfs_path)) + goto linux_mtd_setup_exit; + + if (get_mtd_info()) + goto linux_mtd_setup_exit; + + if ((dev_fd = open(dev_path, O_RDWR)) == -1) { + msg_pdbg("%s: failed to open %s: %s\n", __func__, + dev_path, strerror(errno)); + goto linux_mtd_setup_exit; + } + + ret = 0; +linux_mtd_setup_exit: + return ret; +} + +static int linux_mtd_shutdown(void *data) +{ + if (dev_fd != -1) { + close(dev_fd); + dev_fd = -1; + } + + return 0; +} + +int linux_mtd_init(void) +{ + char *param; + int dev_num = -1; /* linux_mtd_setup will search if dev_num < 0 */ + int ret = 1; + + if (alias && alias->type != ALIAS_HOST) + return 1; + + param = extract_programmer_param("dev"); + if (param) { + char *endptr; + + dev_num = strtol(param, &endptr, 0); + if ((param == endptr) || (dev_num < 0)) { + msg_perr("Invalid device number %s. Use flashrom -p " + "linux_mtd:dev=N where N is a valid MTD " + "device number\n", param); + goto linux_mtd_init_exit; + } + } + + if (linux_mtd_setup(dev_num)) + goto linux_mtd_init_exit; + + if (register_shutdown(linux_mtd_shutdown, NULL)) + goto linux_mtd_init_exit; + + register_opaque_programmer(&programmer_linux_mtd); + + ret = 0; +linux_mtd_init_exit: + msg_pdbg("%s: %s\n", __func__, ret == 0 ? "success." : "failed."); + return ret; +} diff --git a/programmer.h b/programmer.h index a98b713..d3a77bc 100644 --- a/programmer.h +++ b/programmer.h @@ -99,6 +99,9 @@ #if CONFIG_LINUX_SPI == 1 PROGRAMMER_LINUX_SPI, #endif +#if CONFIG_LINUX_MTD == 1 + PROGRAMMER_LINUX_MTD, +#endif #if CONFIG_USBBLASTER_SPI == 1 PROGRAMMER_USBBLASTER_SPI, #endif @@ -525,6 +528,11 @@ int buspirate_spi_init(void); #endif +/* linux_mtd.c */ +#if CONFIG_LINUX_MTD == 1 +int linux_mtd_init(void); +#endif + /* linux_spi.c */ #if CONFIG_LINUX_SPI == 1 int linux_spi_init(void); @@ -587,6 +595,9 @@ #if CONFIG_OGP_SPI == 1 || CONFIG_NICINTEL_SPI == 1 || CONFIG_RAYER_SPI == 1 || CONFIG_PONY_SPI == 1 || (CONFIG_INTERNAL == 1 && (defined(__i386__) || defined(__x86_64__))) SPI_CONTROLLER_BITBANG, #endif +#if CONFIG_LINUX_MTD == 1 + SPI_CONTROLLER_LINUX_MTD, +#endif #if CONFIG_LINUX_SPI == 1 SPI_CONTROLLER_LINUX, #endif -- To view, visit https://review.coreboot.org/22501 To unsubscribe, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib3b8963874722ea80299e9298101409408d6c253 Gerrit-Change-Number: 22501 Gerrit-PatchSet: 1 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Shawn Nematbakhsh <shawnn(a)chromium.org>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit