Skip to content

Commit

Permalink
linux_mtd: Import driver from ChromiumOS
Browse files Browse the repository at this point in the history
This imports a series of patches from chromiumos for MTD support.
The patches are squashed to ease review and original Change-Ids have
been removed to avoid confusing Gerrit.

There are a few changes to integrate the code:
- Conflict resolution
- Makefile changes
- Remove file library usage from linux_mtd. We may revisit this and use
  it for other Linux interfaces later on.
- Switch to using file stream functions for reads and writes.

This consolidated patch is
Signed-off-by: David Hendricks <dhendricks@fb.com>

The first commit's message is:
Initial MTD support

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@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/272983
Reviewed-by: Shawn N <shawnn@chromium.org>

This is the 2nd commit message:

linux_mtd: Fix compilation errors

This fixes compilation errors from the initial import patch.

Signed-off-by: David Hendricks <dhendricks@fb.com>

This is the 3rd commit message:

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@chromium.org>

Reviewed-on: https://chromium-review.googlesource.com/302145
Commit-Ready: David Hendricks <dhendrix@chromium.org>
Tested-by: David Hendricks <dhendrix@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>

This is the 4th commit message:

linux_mtd: Support for NO_ERASE type devices

Some mtd devices have the MTD_NO_ERASE flag set. This means
these devices don't require an erase to write and might not have
implemented an erase function. We should be conservative and skip
erasing altogether, falling back to performing writes over the whole
flash.

BUG=b:35104688
TESTED=Zaius flash is now written correctly for the 0xff regions.

Signed-off-by: William A. Kennington III <wak@google.com>
Reviewed-on: https://chromium-review.googlesource.com/472128
Commit-Ready: William Kennington <wak@google.com>
Tested-by: William Kennington <wak@google.com>
Reviewed-by: Brian Norris <briannorris@chromium.org>

This is the 5th commit message:

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

Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/344006
Reviewed-by: David Hendricks <dhendrix@chromium.org>

This is the 6th commit message:

linux_mtd: make read/write loop chunks consistent, and documented

Theoretically, there should be no maximum size for the read() and
write() syscalls on an MTD (well, except for the size of the entire
device). But practical concerns (i.e., bugs) have meant we don't quite
do this.

For reads:
Bug https://b/35573113 shows that some SPI-based MTD drivers don't yet
handle very large transactions. So we artificially limit this to
block-sized chunks.

For writes:
It's not clear there is a hard limit. Some drivers will already split
large writes into smaller chunks automatically. Others don't do any
splitting. At any rate, using *small* chunks can actually be a problem
for some devices (b:35104688), as they get worse performance (doing an
internal read/modify/write). This could be fixed in other ways by
advertizing their true "write chunk size" to user space somehow, but
this isn't so easy.

As a simpler fix, we can just increase the loop increment to match the
read loop. Per David, the original implementation (looping over page
chunks) was just being paranoid.

So this patch:
 * clarifies comments in linux_mtd_read(), to note that the chunking is
   somewhat of a hack that ideally can be fixed (with bug reference)
 * simplifies the linux_mtd_write() looping to match the structure in
   linux_mtd_read(), including dropping several unnecessary seeks, and
   correcting the error messages (they referred to "reads" and had the
   wrong parameters)
 * change linux_mtd_write() to align its chunks to eraseblocks, not page
   sizes

Note that the "->page_size" parameter is still somewhat ill-defined, and
only set by the upper layers for "opaque" flash. And it's not actually
used in this driver now. If we could figure out what we really want to
use it for, then we could try to set it appropriately.

BRANCH=none
BUG=b:35104688
TEST=various flashrom tests on Kevin
TEST=Reading and writing to flash works on our zaius machines over mtd

Change-Id: I3d6bb282863a5cf69909e28a1fc752b35f1b9599
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/505409
Reviewed-by: David Hendricks <dhendrix@chromium.org>
Reviewed-by: Martin Roth <martinroth@chromium.org>
Reviewed-by: William Kennington <wak@google.com>
Reviewed-on: https://review.coreboot.org/25706
Tested-by: David Hendricks <david.hendricks@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
  • Loading branch information
dhendrix committed May 17, 2018
1 parent 291764a commit f9a3055
Show file tree
Hide file tree
Showing 7 changed files with 475 additions and 2 deletions.
35 changes: 34 additions & 1 deletion Makefile
Expand Up @@ -341,6 +341,11 @@ endif
ifneq ($(TARGET_OS), Linux)
# Android is handled internally as separate OS, but it supports CONFIG_LINUX_SPI and CONFIG_MSTARDDC_SPI
ifneq ($(TARGET_OS), Android)
ifeq ($(CONFIG_LINUX_MTD), yes)
UNSUPPORTED_FEATURES += CONFIG_LINUX_MTD=yes
else
override CONFIG_LINUX_MTD = no
endif
ifeq ($(CONFIG_LINUX_SPI), yes)
UNSUPPORTED_FEATURES += CONFIG_LINUX_SPI=yes
else
Expand Down Expand Up @@ -624,7 +629,8 @@ CONFIG_DEDIPROG ?= yes
# 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.
Expand Down Expand Up @@ -902,6 +908,12 @@ PROGRAMMER_OBJS += satamv.o
NEED_LIBPCI += CONFIG_SATAMV
endif

ifeq ($(CONFIG_LINUX_MTD), yes)
# This is a totally ugly hack.
FEATURE_CFLAGS += $(call debug_shell,grep -q "LINUX_MTD_SUPPORT := yes" .features && printf "%s" "-D'CONFIG_LINUX_MTD=1'")
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'")
Expand Down Expand Up @@ -1277,6 +1289,18 @@ int main(int argc, char **argv)
endef
export UTSNAME_TEST

define LINUX_MTD_TEST
#include <mtd/mtd-user.h>

int main(int argc, char **argv)
{
(void) argc;
(void) argv;
return 0;
}
endef
export LINUX_MTD_TEST

define LINUX_SPI_TEST
#include <linux/types.h>
#include <linux/spi/spidev.h>
Expand Down Expand Up @@ -1333,6 +1357,15 @@ ifneq ($(NEED_LIBFTDI), )
( echo "not found."; echo "FTDISUPPORT := no" >> .features.tmp ) } \
2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE)
endif
ifeq ($(CONFIG_LINUX_MTD), yes)
@printf "Checking if Linux MTD headers are present... " | tee -a $(BUILD_DETAILS_FILE)
@echo "$$LINUX_MTD_TEST" > .featuretest.c
@printf "\nexec: %s\n" "$(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX)" >>$(BUILD_DETAILS_FILE)
@ { $(CC) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest$(EXEC_SUFFIX) >&2 && \
( echo "yes."; echo "LINUX_MTD_SUPPORT := yes" >> .features.tmp ) || \
( echo "no."; echo "LINUX_MTD_SUPPORT := no" >> .features.tmp ) } \
2>>$(BUILD_DETAILS_FILE) | tee -a $(BUILD_DETAILS_FILE)
endif
ifeq ($(CONFIG_LINUX_SPI), yes)
@printf "Checking if Linux SPI headers are present... " | tee -a $(BUILD_DETAILS_FILE)
@echo "$$LINUX_SPI_TEST" > .featuretest.c
Expand Down
1 change: 1 addition & 0 deletions flash.h
Expand Up @@ -131,6 +131,7 @@ enum write_granularity {
* other flash chips, such as the ENE KB9012 internal flash, work the opposite way.
*/
#define FEATURE_ERASED_ZERO (1 << 16)
#define FEATURE_NO_ERASE (1 << 17)

#define ERASED_VALUE(flash) (((flash)->chip->feature_bits & FEATURE_ERASED_ZERO) ? 0x00 : 0xff)

Expand Down
15 changes: 15 additions & 0 deletions flashrom.8.tmpl
Expand Up @@ -289,6 +289,8 @@ bitbanging adapter)
.sp
.BR "* ogp_spi" " (for SPI flash ROMs on Open Graphics Project graphics card)"
.sp
.BR "* linux_mtd" " (for SPI flash ROMs accessible via /dev/mtdX on Linux)"
.sp
.BR "* linux_spi" " (for SPI flash ROMs accessible via /dev/spidevX.Y on Linux)"
.sp
.BR "* usbblaster_spi" " (for SPI flash ROMs attached to an Altera USB-Blaster compatible cable)"
Expand Down Expand Up @@ -1007,6 +1009,19 @@ parameter as explained in the
.B nic3com et al.\&
section above.
.SS
.BR "linux_mtd " programmer
.IP
You may specify the MTD device to use with the
.sp
.B " flashrom \-p linux_mtd:dev=/dev/mtdX"
.sp
syntax where
.B /dev/mtdX
is the Linux device node for your MTD device. If left unspecified the first MTD
device found (e.g. /dev/mtd0) will be used by default.
.sp
Please note that the linux_mtd driver only works on Linux.
.SS
.BR "linux_spi " programmer
.IP
You have to specify the SPI controller to use with the
Expand Down
15 changes: 14 additions & 1 deletion flashrom.c
Expand Up @@ -341,6 +341,18 @@ const struct programmer_entry programmer_table[] = {
},
#endif

#if CONFIG_LINUX_MTD == 1
{
.name = "linux_mtd",
.type = OTHER,
.devs.note = "Device files /dev/mtd*\n",
.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",
Expand Down Expand Up @@ -1772,7 +1784,8 @@ static int read_erase_write_block(struct flashctx *const flashctx,
bool skipped = true;
uint8_t *const curcontents = info->curcontents + info->erase_start;
const uint8_t erased_value = ERASED_VALUE(flashctx);
if (need_erase(curcontents, newcontents, erase_len, flashctx->chip->gran, erased_value)) {
if (!(flashctx->chip->feature_bits & FEATURE_NO_ERASE) &&
need_erase(curcontents, newcontents, erase_len, flashctx->chip->gran, erased_value)) {
if (erase_block(flashctx, info, erasefn))
goto _free_ret;
/* Erase was successful. Adjust curcontents. */
Expand Down
3 changes: 3 additions & 0 deletions internal.c
Expand Up @@ -230,6 +230,9 @@ int internal_init(void)
*/
internal_buses_supported = BUS_NONSPI;

if (try_mtd() == 0)
return 0;

/* Initialize PCI access for flash enables */
if (pci_init_common() != 0)
return 1;
Expand Down

0 comments on commit f9a3055

Please sign in to comment.