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>
A NULL func pointer is necessary and sufficient for the
condition `NULL func pointer => true' as to not need this
boilerplate as it implies default behaviour of a supported
opcode within the `check_block_eraser()` match supported loop.
Ran;
```
$ find . -name '*.[c,h]' -exec sed -i '/.probe_opcode = default_spi_probe_opcode,/d' '{}' \;
```
Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70264
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
A NULL func pointer is necessary and sufficient for the
condition `NULL func pointer => default_spi_send_multicommand' as to not
need this explicit specification of 'default'.
Therefore drop the explicit need to specify the
'default_spi_send_multicommand' callback function pointer in the
spi_master struct. This is a reasonable default for every other driver
in the tree with only a few exceptions.
This simplifies the code and driver development.
Change-Id: I6cc24bf982da3d5251d391eb397db43dd10280e8
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67481
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
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>
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>
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>
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>
get_param()'s co-domain should either be well defined or
error and return. By prematurely initialising we can
obscure compiler warnings for unchecked error branches.
Change-Id: I50bb6db24bb74c6b492d6a0fad9d9225dbb650ac
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66911
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Currently, each programmer parameter has their own temp variable to
store their raw value into it. That's not needed since these variables
are only used for a short time to do some configuration and stay unused
then. Thus, use only one variable for all of them.
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: I48502161add9b59d97f5eeb46f5984c075ad924a
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65909
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Use the bool type instead of integer for options, since this represents
the purpose of their variables much better.
Also, use the bool type for the attribute `reset` from the struct
`realtek_mst_i2c_spi_data`, which is used to store the value of
the parameter `reset_mcu`.
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: Idffd860e0de0bb82eec6102087e2e19783c32521
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65943
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Set the default value for programmer options just before the
user-provided parameters are evaluated. The values get overridden if the
user specified their own values, else the pre-set values are used. This
way, we get rid of these else-blocks.
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: Ibb43aaa4d70ee0827587288c658f01bcef583ddd
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65936
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
realtek_mst_i2c_spi is the only programmer which uses hyphens instead of
underscores in its parameter names. Thus, for consistency, rename the
parameters so that they use underscores.
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: I5ff6d8d432d875670fcaa2088e9cf9d9f1b83dc2
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65935
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Currently i2c programmers do not have a safe allow listing
mechanism via board_enable to facilitate fully qualified
chip detection.
Since i2c addresses alone can overlap a user may make the mistake
of using the wrong programmer. Although unlikely, it is within the
realm of possibility that a user could accidently somehow program
another chip on their board.
Change-Id: Ifb303989fdb67f7267002bd0425f3d050450ec93
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65545
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Add a field, probe_opcode, to struct spi_master which points to a
function returning a bool by checking if a given command is supported by
the programmer in use. This is used for getting a whitelist of commands
supported by the programmer, as some programmers like ichspi don't
support all opcodes.
Most programmers use the default function, which just returns true.
ICHSPI and dummyflasher use their specialized function.
Change-Id: I6852ef92788221f471a859c879f8aff42558d36d
Signed-off-by: Aarya Chaumal <aarya.chaumal@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65183
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Avoid confusion from the comment. While technically a GPIO
can do anything, like drive a LED. The GPIO pin 88 *is*
meant to drive the WP line of the SPI flash, that is its
purpose.
Change-Id: If718d41a27931380e5f7ebdb75b9863da0c61559
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65546
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
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>
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>
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>
This allows spi masters to register shutdown function in spi_master
struct, which means there is no need to call register_shutdown in init
function, since this call is now a part of register_spi_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_spi_master() return values.
Basic testing: when I comment out free(data) in linux_spi_shutdown, test
fails with error
../linux_spi.c:235: note: block 0x55a4db276510 allocated here
ERROR: linux_spi_init_and_shutdown_test_success leaked 1 block(s)
Means, shutdown function is invoked.
BUG=b:185191942
TEST= 1) builds and ninja test including CB:56911
2) On ARMv7 device
flashrom -p linux_spi -V
-> using linux_spi, chip found
3) On x86_64 AMD device
flashrom -p internal -V
-> this is actually using sb600spi, chip found
Change-Id: Ib60300f9ddb295a255d5ef3f8da0e07064207140
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/56103
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
This patch prepares spi masters to use new API which allows to
register shutdown function in spi_master struct. See also later
patch in this chain, where spi 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: I50716686552b4ddcc6089d5afadb19ef59d9f9b4
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/56101
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
In some earlier refactoring the init function for this programmer was
broken such that it never succeeds, since the return value was never set
to a value other than a generic failure. Fix it to return success unless
there is a problem.
TEST=flashrom -p realtek_mst_i2c_spi:bus=n --flash-size now works
when n is a bus that has a connected MST.
Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/56799
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Spelling out the struct type name hurts readability and introduces
opportunities for bugs to happen when the pointer variable type is
changed but the corresponding sizeof is (are) not.
TEST=`make CONFIG_EVERYTHING=yes CONFIG_JLINK_SPI=no VERSION=none -j`
with and without this patch; the flashrom executable does not change.
Change-Id: Icc0b60ca6ef9f5ece6ed2a0e03600bb6ccd7dcc6
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55266
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Some more spi masters are now ready to get the advantage of new API
and pass pointers to dynamically allocated data to
register_spi_master(). This way we can avoid some mutable globals.
BUG=b:185191942
TEST=./flashrom --programmer raiden_debug_spi -r $(mktemp)
./flashrom --programmer raiden_debug_spi -v /tmp/tmp.Fch5QLVb4R
Change-Id: If72f54c28a95b402b3565fd14ea481f734e1c970
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/54889
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
It was impossible to register a const struct spi_master that would
point to dynamically allocated `data`. Fix that so that we won't
have to create more mutable globals.
Change-Id: I0c753b3db050fb87d4bbe2301a7ead854f28456f
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/54066
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
This fixes building with musl libc on alpine:amd64-v3.9.
Change-Id: I043e3d8c2d2498e94b5e7577a7378c8c3e0e6c81
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/53998
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
This allows using `buspath` to specify which I2C device to use.
Change-Id: Ibdf07a9fde0ddfcda1c0bfa35a3e7cde5c22cedb
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52831
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
As per the coding style, if one branch of a conditional statement needs
braces, all other branches need to have braces as well.
Change-Id: I69b762391165177857e9331f79f54b01149cf339
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52827
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Gpio 88 toggle is used as write protection disable/enable now
and we need that to happen at the initialization of programmer.
Background:
The RTD devices has an interesting implementation where the
flag we need to flash is `aa aa aa ff ff`. However, after reset,
the boot firmware of RTD device will overwrite this flag value to
`aa aa aa ff aa`. Given this evidence, the root cause would be
that the boot firmware is doing something with protection enable
by itself. This explains why the message
'Block protection cannot be disabled' is shown since the block
protection is called before write operation.
BUG=b:147402710,b:152558985,b:178766553
BRANCH=none
TEST=flashrom -p realtek_mst_i2c_spi:bus=x,reset-mcu=1,enter-isp=1 -w
image.bin
Signed-off-by: Shiyu Sun <sshiyu@chromium.org>
Change-Id: I237bf9f8aa0fcbb904e7f0c09c74fd179e8c70c1
Reviewed-on: https://review.coreboot.org/c/flashrom/+/49785
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The return value for reset function can not be guaranteed when
reset success. There is no way to check if reset success or not.
BUG=b:147402710,b:152558985
BRANCH=none
TEST=builds
Signed-off-by: Shiyu Sun <sshiyu@chromium.org>
Change-Id: Ia6200f7150db4368c26d8dfe779a9e85184b1b06
Reviewed-on: https://review.coreboot.org/c/flashrom/+/49786
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Be explicit to mask the first byte after the shifts as
highlighted by Angel Pons.
BUG=none
BRANCH=none
TEST=builds
Change-Id: I7d1215678094d709e79b8f8c96aa3810586cd72e
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Spotted-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/48974
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Shiyu Sun <sshiyu@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Update the PAGE_SIZE to 128 as fix r/w on different devices,
also fix the write page mapping for it.
BUG=b:147402710
TEST=build and run flashrom to read&write on multiple devices
Signed-off-by: Shiyu Sun <sshiyu@chromium.org>
Change-Id: Ifcdd3548519eb37440e67fcf6206279cff05b159
Reviewed-on: https://review.coreboot.org/c/flashrom/+/48840
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Check ISP mode before doing reset and add waiting
after the enter ISP mode command.
BUG=None
TEST=build and run mst commands
Signed-off-by: Shiyu Sun <sshiyu@chromium.org>
Change-Id: Ib1ab8370eb6335a77bb293fc98a8ab7be465db4f
Reviewed-on: https://review.coreboot.org/c/flashrom/+/48662
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
This is needed to avoid attempt entering ISP mode multiple
times. The ISP mode can only exit after a reset, so once the
reset MCU parameter is set to 0, the device will not able to
exit from ISP mode and hence shouldn't enter ISP again on
the next operation.
Without exit ISP mode, the device data, like firmware version,
will not show the correct value, this param will also help
to identify this situation.
BUG=b:152558985,b:148745673
BRANCH=none
TEST=build and run:
$ flashrom -p realtek_mst_i2c_spi:bus=x,reset-mcu=0,enter-isp=1 \
-l layout -i PARTITION1:fw -w
$ flashrom -p realtek_mst_i2c_spi:bus=x,reset-mcu=0,enter-isp=0 \
-l layout -i FLAG1:flag -w
then either reset computer to allow update to take effect, or:
$ flashrom -p realtek_mst_i2c_spi:bus=x,reset-mcu=1,enter-isp=0 \
--flash-size
to trigger the update.
Signed-off-by: Shiyu Sun <sshiyu@chromium.org>
Change-Id: I58931ac8b42ab55829f102d243aea6fcfd632e3e
Reviewed-on: https://review.coreboot.org/c/flashrom/+/46623
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Here we provide a helper function to allow indexing MCU configuration
registers. The 0x9F port allows access to these MCU configuration
registers followed by the high and then low bytes of the register
address we wish to index written into 0xF5 or 0xF4 respectively, a
read or write can then be made via 0xF5.
For the configuration of GPIO pins on the chip, there are two relevant
register address, 0x104F for pin direction (sink input or push-pull
in-out) configuration and 0xFE3F for pin data values (1 to push-pull
and 0 to sink). The reference design uses GPIO 88 to strap the
write protection pin and so we provide a function that allows the call
site to toggle this state and therefore de-assert hardware write
protection of the external spi flash.
BUG=b:152558985,b:148745673
BRANCH=none
TEST=builds && verified the write protection get disabled.
Change-Id: I1aed0086f917e31bebb51857ad5cce138158fe82
Signed-off-by: Shiyu Sun <sshiyu@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/46089
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Ensure that when bus number and reset params are specified
at the same time are both correctly parsed by get_params().
Also renames the goto err cleanup path to make it clear.
Change-Id: Icb45b1ab39181b0f1a2dec1cce549d30db984936
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Spotted-by: Shiyu Sun <sshiyu@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/45944
Reviewed-by: Sam McNally <sammc@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Modify the spi master as to not automatically reset the
MCU on tear-down unless explicitly stated by a param.
Change-Id: Ib70bf7399e7541f30b6905cdb950a6fb7b74ae18
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/45674
Reviewed-by: Sam McNally <sammc@google.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Remove MCU reset on init as this was only introduced
when MCU fw requirements for correct flashing were unknown
however it turns out no MCU fw is required to flash and so
no MCU reset should occur upon initialization.
Change-Id: Ia03f94effc4b720964638c032bbde5acfb13960d
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/45896
Reviewed-by: Sam McNally <sammc@google.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Turns out the MST likely doesn't need these so-called defaults
to be written for the purposes of spi flashing.
BUG=b:152558985,b:148745673
BRANCH=none
TEST=builds
Change-Id: Ieb938cf0805b22692d61db23795208c9be962b60
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41124
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
Chip erasures take much longer than sector and bank
erasures. Allow the wait loop helper to multiply the
max timeout in this very specific case while quickly
timeout for other ops that are expected to be shorter.
V.2: Fix nonsense fall though warn-err
BUG=b:152558985,b:148745673
BRANCH=none
TEST=flashrom -p realtek_mst_i2c_spi:bus=8 -E &&
flashrom -p realtek_mst_i2c_spi:bus=8 -w foo
(cycle)..
Change-Id: I4a36aa3101827e69eb244775d25bbb476d4bb780
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41123
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
Turns out broken erasures highlighted some of the issues
in the write256 implementation. After a fair amount of
time deciphering scarce documentation details a correct
implementation was finally derived.
V.2: Rename 'start_program() -> execute_write()' to
clarify the intention and not to overload the term
'program' since the MST actually runs a 'program'
itself.
BUG=b:152558985,b:148745673
BRANCH=none
TEST=flashrom -p realtek_mst_i2c_spi:bus=8 -E &&
flashrom -p realtek_mst_i2c_spi:bus=8 -w foo &&
flashrom -p realtek_mst_i2c_spi:bus=8 -r foo &&
hexdump -C foo
Change-Id: If61ff95697f886d3301a907b76283322c39ef5c7
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41080
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
Before issuing SPI opcodes into 0x61 the top three BITS of
0x60 need to be carefully crafted. Correctly craft these
in the case of SPI erasures and document this registers
expectations. Clean up remaining debug comments while we
are here.
BUG=b:152558985,b:148745673
BRANCH=none
TEST=flashrom -p realtek_mst_i2c_spi:bus=8 -E &&
flashrom -p realtek_mst_i2c_spi:bus=8 -r foo &&
hexdump -C foo
Change-Id: Ib11ba8f63b11a1c5ebaa68deb7971648de8c2ecd
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41079
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
Try to document some of the register magics with plausible
names for readability.
BUG=b:152558985,b:148745673
BRANCH=none
TEST=builds
Change-Id: I97313f6f14438e4cbfc62faa7242cf6fc271f387
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/41022
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
