This patch adds mocks for libusb functions. To avoid dependency on
libusb.h, libusb symbols for context and device handle are redefined.
Real libusb functions are never called in tests anyway, cmocka wraps
work with this without complaints.
BUG=b:181803212
TEST=builds and ninja test
Change-Id: I38508dfb6d7c24d42522f22fcae0c5e410c5f7ea
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55934
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>
`mark_unsupported` disables all programmers passed as arguments and
adds them to the UNSUPPORTED_FEATURES list. It's conveniently used
with our dependency lists, e.g.
$(call mark_unsupported,$(NEEDS_LIBUSB1) $(NEEDS_LIBFTDI))
Change-Id: I6c9b66dfa89ec3d71675576d03fcebfdea30ce7b
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55873
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
This reverts commit cf1e8f7b90.
It's an indirect, spurious dependency. Why clutter the build system
with it? Instead, the functions these programmers actually depend
on should be factored into a compilation unit that doesn't need PCI.
No matter this change, compilation with `CONFIG_INTERNAL=no` is
broken because of these programmers right now.
Change-Id: I134eb7d74b44f92dfdea3c764f39bc3673739fa0
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55869
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Normalize the fallback paths:
* Always end with the newest, assumed compatible chipset.
* Perform tighter checks when it's about warnings only.
* If two chipsets seem compatible, always return the same
(this is currently the case for 8/9 series and 300/400
series which we can't distinguish).
Change-Id: Ic5a5fee870202173b3a9813b03ec261e8ee45155
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55651
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Only warn if the `freq_read` setting looks odd but don't override
our previous guess. The `freq_read` check was taken from `ifdtool`
but seems less reliable than our own detection scheme.
Change-Id: I658d76ec2567d1d660a18d0b0ae71c744e603e8f
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55650
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com>
Detection based on ICCRIBA and FMSBA became a little messy lately.
However, there's a new static difference: Since 300 series (Cannon
Point), there is an MDTBA field in FLUMAP1 that has always been 0
(reserved) before. Taking this into account, we can relax the checks
on ICCRIBA.
Change-Id: I587ad1abe390843d4a9e74431b6fc4b63f8ba512
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55647
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com>
Shutdown function was covering two different jobs here: 1) the actual
shutdown which is run at the end of the driver's lifecycle and
2) cleanup in cases when initialisation failed. Now, shutdown is only
doing its main job (#1), and the driver itself is doing cleanup
when init fails (#2).
The good thing is that now resources are released/closed immediately
in cases when init fails (vs shutdown function which was run at some
point later), and the driver leaves clean space after itself if init
fails.
And very importantly this unlocks API change which plans to move
register_shutdown inside register master API, see
https://review.coreboot.org/c/flashrom/+/51761
BUG=b:185191942
TEST=builds
Change-Id: I3273da907614a042d50090338c337dfd64695354
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55887
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Return values for buspirate_sendrecv come from serialport_write
and serialport_read, and those return 1s for any error. No need
to explicity assign ret = 1, because it is already 1 for error.
Follow up on commit 751afa88a7
where this idea was suggested.
BUG=b:185191942
TEST=builds
Change-Id: I31fd70f607dc965d5cac1cd0116faa447dbc177a
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55818
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
* Parameter names like `flashctx` for a `struct flashrom_flashctx`
don't add any value.
* `const` qualification of parameters is meaningless in forward
declarations. Arguments are always passed by copy and an API
user does not need to know what callees do with their copy.
Change-Id: Iadcc1670ff86578a400dec9e804d6dda93e0fcf0
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/54288
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
It initializes an empty layout. Currently the maximum number of entries
has to be specified, which will vanish once we use dynamic allocation
per entry.
We replace the two special cases `single_layout` and `ich_layout` with
dynamically allocated layouts. As a result, we have to take care to
release the `default_layout` in a flashctx once we are done with it.
Change-Id: I2ae7246493ff592e631cce924777925c7825e398
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33543
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Also, a `layout.c` internal version mutable_layout_next() that
allows to modify layout entries and a shorthand to look up an
entry by name, _layout_entry_by_name().
Use the new functions where applicable and the code is not
dropped later in this train, and also to compare the layouts
in flashrom_layout_read_from_ifd() in depth.
Change-Id: I284958471c61344d29d92c95d88475065a9ca9aa
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/33542
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
sys/io.h is platform specific, and also in tests environment we
don't need real functions anyway. Adding dummy implementation of
iopl is sufficient for tests. The rest of io is not needed
because hwaccess_x86_io_unittest.h re-defines macros OUTB/INB/etc
and those macros evaluate to test-only functions.
This is a follow up on commit 21e22ba8a7
which introduced hwaccess_x86_io_unittest.h
BUG=b:181803212
TEST=builds and ninja test on x86 (same as before)
Change-Id: I3f2f0408be7c00f954b899031b52b2b97ef19ca3
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55741
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
For all tests that exist as of today, drivers are built by default,
however config options can be disabled and in that case test should
not be run.
Technically, this is done by skipping the test.
BUG=b:181803212
TEST=1) Tested by adding into tests/meson.build
-DCONFIG_xxx=0
4 times (for every driver with test), and then running ninja test
Result: corresponding test is skipped, all other tests are passed
2) Running ninja test with default config settings (everything is
enabled, no overriding in test meson).
Result: all tests are passed.
3) Replacing one of config options in the patch with CONFIG_JLINK_SPI
which is disabled by default.
Result: corresponding test is skipped.
Change-Id: Ic1c48e41f658045a608f46636071f478ba646f77
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55295
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
We missed to `free(spi_data)` on one path. It also seems odd to leak
the return code of a locally used library into our common infrastruc-
ture, so normalize all error paths to return 1.
Change-Id: I5158d06127a9a8934b083e48b69d29c4d5a11831
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55696
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
The meaning of the variables is easy to misunderstand as some
states are merely implicit: All output pins that are not set
in the `cs_bits` mask will be constantly driven low. This may
be sheer coincidence as all programmers that need additional
pins driven use active-low signals to enable buffers.
While other pins stay low, *all* pins set in the `cs_bits`
mask are supposed to be toggled during SPI transactions.
Also drop some irritating dead code and try to explain things
in a comment.
Change-Id: I2b84ede01759c80f69d5ad17e43783d09ecd1107
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55695
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
This reverts commit ba6575de82.
Technically, the only thing that is wrong here is the lack of docu-
mentation (manpage update). However, as this change was succeeded by
a regressing fixup patch, it seems likely that the meaning of the
`csgpiol` parameter was just misunderstood and these changes were
not what the author intended.
Change-Id: I460237b9d275b1cd1d8a069f852d17dea393b14e
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/55694
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
