[coreboot] How is our SPI API supposed to work?

Julius Werner jwerner at chromium.org
Fri Apr 20 00:51:48 CEST 2018


> How does it mean two different things? The pairing of a controller and
> device where a full duplex operation is needed but the controller
> doesn't support things should indeed fail. I do not undertand how
> xfer() means two different things. Whenever pairing hardware together
> one needs to validate that things work when married.

spi_xfer_two_vectors() takes a call like this:

   struct spi_op vectors[2] = { {.dout = out, .din = NULL, ...}, {.dout =
NULL, .din = in, ...};
   spi_xfer_vector(..., vectors, 2);

and turns it into

   spi_xfer(..., dout, ..., din, ...);

Those are two different things. The former means "first transfer dout on
MOSI and ignore MISO, then read in din on MISO and send zeroes or whatever
out on MOSI". The latter means "do a full-duplex transaction where you read
from MISO into din and write dout out on MOSI at the same time". If you did
the latter call on a controller supporting full-duplex it wouldn't actually
be a correct SPI flash transaction. But all the controllers that use
spi_xfer_two_vectors() have implemetations of xfer() that don't actually do
the thing I just described... instead they do the "first out, then in"
behavior even though that's normally not what spi_xfer(..., dout, ..., din,
...) means. I think those implementations of xfer() are wrong and violating
the API. If they were corrected, spi_xfer_two_vectors() wouldn't make sense
the way it works right now.

> OK. It's just moving code around.

Yeah, I just want to shuffle code around. Nothing is broken right now, it's
just working in a confusing and inconsistent way. We should have an API
where the callbacks share the same semantics across all implementations
(and also document those semantics more clearly).



More information about the coreboot mailing list