1
0
mirror of https://review.coreboot.org/flashrom.git synced 2025-04-26 22:52:34 +02:00

29 Commits

Author SHA1 Message Date
Arnaud Ferraris
34b1a6aa57 linux_mtd: fix build with clang >= 19
Starting with version 19, clang issues a warning when using `strlen()`
for initializing a static array's size. This causes the build to fail as
the project also sets `-Werror`.

This is fixed by using `sizeof()` instead, which is guaranteed to be
evaluated at compilation time and therefore not triggering the
problematic warning.

Change-Id: If470a65702e9ae08e4303123a0014e53a1fee56e
Signed-off-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84856
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Maximilian Brune <maximilian.brune@9elements.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
2024-11-01 23:38:55 +00:00
Anastasia Klimchuk
75dc0655b9 Complete and fix progress feature implementation for all operations
Original progress reporting implemented in CB:49643 and it has some
issues, for example:

    size_t start_address = start;
    size_t end_address = len - start;

End address is anything but length minus start address.

    update_progress(flash,
                    FLASHROM_PROGRESS_READ,
                    /*current*/ start - start_address + to_read,
                    /*total*/ end_address);

Total should just be length if that's how current value is computed.

---

libflashrom needs to know total size ahead of time.
That's init_progress() and changed update_progress().

It also needs to store the last current value to be able to update it.
That's stage_progress in flashrom_flashctx.

Measuring accurately amount of data which will be read/erased/written
isn't easy because things can be skipped as optimizations. The next
patch in the chain aims to address this, there are TODO/FIXME
comments there.

---

CLI shares terminal with the rest of the code and has to maintain more
state to handle that reasonably well.

Similar to CB:64668, an effort is made to keep the progress on a
single line. Non-progress output is kept track of to know when
moving to a new line cannot be avoided.

---

A script to test the CLI:

\#!/bin/bash
t=${1:-rewW}
shift

if [[ $t =~ r ]]; then
    echo ">>> READ"
    ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -r dump.rom --progress "$@"
    echo
fi

if [[ $t =~ e ]]; then
    echo ">>> ERASE"
    ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -E --progress "$@"
    echo
fi

if [[ $t =~ w ]]; then
    echo ">>> WRITE (without erase)"
    dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null
    ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -w zero.rom --progress "$@"
    echo
fi

if [[ $t =~ W ]]; then
    echo ">>> WRITE (with erase)"
    dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null
    dd if=/dev/random of=random.rom bs=1M count=16 2> /dev/null
    ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz,image=random.rom -w zero.rom --progress "$@"
    echo
fi

Co-developed-by: Anastasia Klimchuk <aklm@flashrom.org>
Co-developed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84102
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
2024-10-27 06:13:11 +00:00
Brian Norris
0802655531 linux_mtd: Provide no-op delay implementation
Flashrom has several magic programmer_delay() calls scattered throughout
its codebase (see cli_classic.s/main() and
flashrom.c/flashrom_image_write(), at least). These delays are
superfluous for the linux_mtd programmer, because it's an opaque
programmer in which all protocol details are handled by the kernel
driver.

Stub out the delay function, so we don't waste up to 1.1 seconds of
needless delay time, depending on the operation and CLI vs libflashrom
usage.

BUG=b:325539928
TEST=`elogtool add 0xa7` on a linux_mtd system (such as a Mediatek
       Chromebook):
     Time improvement - reduces from ~1.5s wall clock to about 0.4s
     CPU usage: -90%, as most of the CPU cycles (before this change) are
       spent in needless magic-delay busy loops

Change-Id: I3ac69d3fd7cfc689c5b32c130d044516ed846c29
Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/80771
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
2024-03-14 10:14:51 +00:00
Anastasia Klimchuk
347dba5414 linux_mtd: Mark Opaque chip as tested for WP
Since linux_mtd supports write-protect, its probe function needs
to mark Opaque chip as tested for WP. Programmers which are
opaque masters are responsible for populating flashchip#tested
struct in probe function.

Without the patch, any operation running via linux_mtd displays
a message "This flash part has status UNTESTED for operations: WP".
With the patch, the message is not displayed anymore.

BUG=b:258755442
BRANCH=none
TEST=flashrom -p host on ARM dut
Found Programmer flash chip "Opaque flash chip"
 (8192 kB, Programmer-specific) on host.
No operations were specified.

Change-Id: Icc0521c28555a93f26ce66bdbeaa68590f10c358
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69518
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
2022-11-21 01:27:03 +00:00
Edward O'Callaghan
67d5015617 drivers/: Make 'fallback_{un}map' the default unless defined
Drop the explicit need to specify the default 'fallback_{un}map'
callback function pointer from the 'programmer_entry' struct.
This is a reasonable default for every other driver in the tree
with only a select few exceptions [atavia, serprog, dummyflasher
and internal].

Thus this simplifies driver development and paves way
to remove the 'programmer' global handle.

Change-Id: I5ea7bd68f7ae2cd4af9902ef07255ab6ce0bfdb3
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67404
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2022-10-08 18:36:21 +00:00
Edward O'Callaghan
4b503bee39 drivers/: Make 'internal_delay' the default unless defined
Drop the explicit need to specify the default 'internal_delay'
callback function pointer in the programmer_entry struct.
This is a reasonable default for every other driver in the
tree with only the two exceptions of ch341a_spi.c and serprog.c.

Thus this simplifies driver development.

Change-Id: I17460bc2c0aebcbb48c8dfa052b260991525cc49
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67391
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Reviewed-by: Nikolai Artemiev <nartemiev@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-09-27 04:30:43 +00:00
Felix Singer
a715893e02 linux_mtd.c: Retype appropriate attributes with bool
Use the bool type instead of an integer for appropriate attributes,
since this represents their purpose much better.

Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: I8be4e08e714047d155dcc032cc3d8dabb422b27f
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66881
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2022-09-08 02:03:30 +00:00
Edward O'Callaghan
5c710ea54a tree: Port programmers to pass programmer_cfg to extractors
Ran;
```
 $ find -name '*.c' -exec sed -i 's/extract_programmer_param_str(NULL/extract_programmer_param_str(cfg/g' '{}' \;
```

Manually fix i2c_helper_linux.c and other cases after.

Treat cases of;
 - pcidev.c , and
 - usb_device.c
as exceptional to be dealt with in later patches.

Change-Id: If7b7987e803d35582dda219652a6fc3ed5729b47
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66656
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
2022-09-07 01:56:58 +00:00
Edward O'Callaghan
1233e63833 tree: Allow passing programmer_cfg directly to programmer
Modify the type signature of each programmer entry-point
xxx_init() functions to allow for the consumption of the
programmer parameterisation string data.

```
 $ find -name '*.c' -exec sed -i 's/_init(void)/_init(const char *prog_param)/g' '{}' \;
 $ find -name '*.c' -exec sed -i 's/get_params(/get_params(const char *prog_param, /g' '{}' \;
 $ find -name '*.c' -exec sed -i 's/const char \*prog_param)/const struct programmer_cfg *cfg)/g' '{}' \;
 $ find -name '*.c' -exec sed -i 's/const char \*prog_param,/const struct programmer_cfg *cfg,/g' '{}' \;
```
and manually fix up any remaining parts.

Change-Id: I8bab51a635b9d3a43e1619a7a32b334f4ce2cdd2
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66655
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
2022-09-07 01:50:58 +00:00
Edward O'Callaghan
e316f1970d tree: Change signature of extract_programmer_param_str()
Results can be reproduced with the following invocation;
```
 $ find -name '*.c' -exec sed -i 's/extract_programmer_param_str(/extract_programmer_param_str(NULL, /g' '{}' \;
```

This allows for a pointer to the actual programmer parameters
to be passed instead of a global.

Change-Id: I781a328fa280e0a9601050dd99a75af72c39c899
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66654
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
2022-09-07 01:49:49 +00:00
Felix Singer
5c644d86ef linux_mtd.c: Rename variable param to param_str
Rename the variable param to param_str to indicate that this is the raw
string representation.

Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: I78a207f11b964424a85e693f0004a33ee7de2e7e
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66567
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-by: Thomas Heijligen <src@posteo.de>
2022-08-14 23:42:17 +00:00
Chinmay Lonkar
1bb5ddde60 Add str extension to extract_programmer_param function name
This patch changes the function name of extract_programmer_param() to
extract_programmer_param_str() as this function name will clearly
specify that it returns the value of the given parameter as a string.

Signed-off-by: Chinmay Lonkar <chinmay20220@gmail.com>
Change-Id: Id7b9fff4d3e1de22abd31b8123a1d237cd0f5c97
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65521
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Reviewed-by: Thomas Heijligen <src@posteo.de>
2022-07-02 14:34:19 +00:00
Thomas Heijligen
b554cdc91e tree: indent struct *_master consistently with tabs
Use `<tab>.key<tab>*= <value>,`

TEST: `make VERSION=0 MAN_DATE=0` returns the same flashrom binary
before and after the patch

Change-Id: I1c45ea9804ca09e040d7ac98255042f58b01f8ef
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65363
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
2022-06-27 08:56:00 +00:00
Richard Hughes
40892b0c08 libflashrom: Return progress state to the library user
Projects using libflashrom like fwupd expect the user to wait for the
operation to complete. To avoid the user thinking the process has
"hung" or "got stuck" report back the progress complete of the erase,
write and read operations.

Add a new --progress flag to the CLI to report progress of operations.

Include a test for the dummy spi25 device.

TEST=./test_build.sh; ./flashrom -p lspcon_i2c_spi:bus=7 -r /dev/null --progress

Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Signed-off-by: Richard Hughes <richard@hughsie.com>
Signed-off-by: Daniel Campello <campello@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/49643
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
2022-05-25 08:08:13 +00:00
Nikolai Artemiev
9b20174fda libflashrom,linux_mtd: add linux_mtd writeprotect support
This commit adds a generic framework to allow opaque programmers to
implement writeprotect operations and uses the framework to support
writeprotect operations on linux MTD device files.

The generic framework comprises three new functions in
`struct opaque_master` that are called from libflashrom:
- wp_write_cfg()
- wp_read_cfg()
- wp_get_ranges()

For linux_mtd, only the read/write functions are implemented. Linux's
MTD interface doesn't provide a way to get available ranges, so calling
get_wp_ranges() on the linux_mtd master will return
FLASHROM_WP_ERR_RANGE_LIST_UNAVAILABLE.

BUG=b:182223106
BRANCH=none
TEST=WP ops on hana DUT (MT8173) with W25Q32DW flash
TEST=flashrom --wp-enable --wp-range <non-empty> succeeds
TEST=flashrom --wp-enable --wp-range <empty> fails as expected
TEST=flashrom --wp-disable --wp-range <empty> succeeds
TEST=flashrom --wp-disable --wp-range <non-empty> fails as expected
TEST=flashrom --wp-status succeeds

Change-Id: I5c86e28cdec44bec49ba1d36f8ab62241b9b01da
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/61897
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
2022-04-05 23:47:46 +00:00
Nikolai Artemiev
89c73b5a74 linux_mtd: check ioctl() return value properly
Make the linux_mtd driver treat any negative return value from the
MEMERASE ioctl as an error. Previously it only treated -1 as an error.

BUG=b:213561594,b:210973586,b:182223106
BRANCH=none
TEST=builds

Change-Id: I40cfbdee2ab608fbe6c17d9cac6ec53ff224d9a4
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/60996
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
2022-01-14 07:31:50 +00:00
Anastasia Klimchuk
9db8e12c16 opaque_master: Use new API to register shutdown function
This allows opaque masters to register shutdown function in
opaque_master struct, which means there is no need to call
register_shutdown in init function, since this call is now a part
of register_opaque_master.

As a consequence of using new API, two things are happening here:
1) No resource leakage anymore in case register_shutdown() would fail,
2) Fixed propagation of register_opaque_master() return values.

BUG=b:185191942
TEST=1) builds and ninja test including CB:56413
2) on ARMv7 device
flashrom -p linux_mtd -V
-> using linux_mtd, chip found

Change-Id: Id8471a117556edcbf9694752fabe05cf4501ce70
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/56825
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
2021-08-25 02:23:34 +00:00
Anastasia Klimchuk
b261bec1ee opaque_master: Move shutdown function above opaque_master struct
This patch prepares opaque masters to use new API which allows to
register shutdown function in opaque_master struct. See also later
patch in this chain, where opaque masters are converted to new API.

BUG=b:185191942
TEST=builds and ninja test
Comparing flashrom binary before and after the patch,
make clean && make CONFIG_EVERYTHING=yes VERSION=none
binary is the same

Change-Id: I5000cfceeba859a76177a17c1cb7d1c1e9fc03fe
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/56824
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
2021-08-17 16:31:10 +00:00
Anastasia Klimchuk
c87a770f5d linux_mtd: Free param right after its last usage
Param is only used in the first half of init function, and it is
local, so there is no need to keep it until the end. This makes
handling error paths in the second half of init function shorter,
because those paths can just return 1 instead of going to a label.

BUG=b:185191942
TEST=builds and ninja test from CB:56413

Change-Id: I126a8d06297ef4d42bc93a73f7067ccc1352d1e9
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/56822
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2021-08-17 16:29:35 +00:00
Thomas Heijligen
4f5169df5f programmer_table: move each entry to the associated programmer source
Change-Id: I3d02bd789f0299e936eb86819b3b15b5ea2bb921
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52946
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-06-10 12:49:16 +00:00
Angel Pons
f41d24823c treewide: Drop unnecessary uses of memset/memcpy
Simply provide an initialiser or use a direct assignment instead.

Change-Id: I07385375cd8eec8a95874001b402b2c17ec09e09
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55267
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-06-09 16:07:45 +00:00
Anastasia Klimchuk
9a2f91d7eb programmer: Make use of new register_opaque_master() API
Pass pointers to dynamically allocated data to
register_opaque_master(). This way we can avoid a mutable global.

BUG=b:185191942
TEST=builds

Change-Id: I160810cd67f782131962e96fc6d20e2987fb0390
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/54171
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-05-23 04:19:22 +00:00
Anastasia Klimchuk
9309bed290 programmer: Smoothen register_opaque_master() API
It was impossible to register a const struct opaque_master that would
point to dynamically allocated `data`. Fix that so that we won't
have to create more mutable globals.

BUG=b:185191942
TEST=builds

Change-Id: Id3adb4cf04ae04dbe87ddb96f30871cb5f7c8ff0
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/54170
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-05-23 04:18:54 +00:00
Nikolai Artemiev
3dc5af7e09 linux_mtd: drop 'mtd_' prefix from variable/field names
BUG=b:161951062
BRANCH=none
TEST=builds

Change-Id: I2503c98e9111d1fecd911473f65eeea7031cfdc3
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/53953
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
2021-05-13 01:02:07 +00:00
Nikolai Artemiev
af26c9675d linux_mtd: move global state into programmer data field
BUG=b:161951062
BRANCH=none
TEST=builds, reading /dev/mtd0 on Oak succeeds

Change-Id: I5ce6900e4892ed5687cfddb245dfe5461a3e2e84
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/53947
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
2021-05-13 00:59:21 +00:00
Douglas Anderson
a2f2f3f5ee linux_mtd: Disable buffering on the mtd device
We open the device node for the MTD device with this:
  dev_fp = fopen(dev_path, "r+")

In C fopen() is allowed to provide _buffered_ access to the file.
That means that the standard library is allowed to read ahead and/or
return cached data.  That's really not what we want for something like
this.  Let's turn it off.

This fixes a problem where flashrom would sometimes fail to "verify"
that it erased the flash.  The error message would look something like
this:

Erasing and writing flash chip... FAILED at 0x0000e220! Expected=0xff, Found=0xe9, failed byte count from 0x0000e200-0x0000e2ff: 0xdc
 failed byte count from 0x0000e000-0x0000efff: 0xffffffff
 ERASE_FAILED
FAILED!
Uh oh. Erase/write failed. Checking if anything changed.

After the failure I could read the flash device with a new invocation
of flashrom and I would see that, indeed, the erase had worked.

Tracing in the kernel showed that when the failure happened we saw a
pattern that looked like this:
* Read 0x0b00 bytes starting at 0x0000d000
* Read 0x1000 bytes starting at 0x0000db00
* Erase 0x1000 bytes starting at 0x0000e000

...and then there was _not_ a read after the erase.  It can be assumed
that, since userspace had already read 0xdb00 - 0xeaff that it was
looking at old buffered data after the erase.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Change-Id: I989afd83a33013b2756a0090d6b08245613215c6
Reviewed-on: https://review.coreboot.org/c/flashrom/+/50155
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-02-01 15:21:29 +00:00
Jacob Garber
ba7199958c linux_mtd: Fix param memory leak
extract_programmer_param() stores allocated memory in param, so make
sure it is freed at the end of the function.

Change-Id: I363e66b49c1ed4034ac058b94a938c8bb197e048
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Found-by: Coverity CID 1403823
Reviewed-on: https://review.coreboot.org/c/flashrom/+/34847
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: David Hendricks <david.hendricks@gmail.com>
2019-09-24 19:07:02 +00:00
David Hendricks
b0247b3acb linux_mtd: Bail out early if sysfs node doesn't exist
This checks that the MTD sysfs node we will use actually exists prior
to calling setup code. Although the setup code will eventually catch
such an error, we need to think about the use case before printing a
possibly irrelevant/confusing error message to the terminal.

This patch makes it so that we only print an error message if the
user specifies a non-existent MTD device. Otherwise, the failure is
considered benign and we only print a debug message prior to bailing
out.

Change-Id: I8dc965eecc68cd305a989016869c688fe1a3921f
Signed-off-by: David Hendricks <dhendricks@fb.com>
Reviewed-on: https://review.coreboot.org/26500
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
2018-06-24 11:47:50 +00:00
David Hendricks
f9a3055480 linux_mtd: Import driver from ChromiumOS
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>
2018-05-17 16:49:16 +00:00