On Thu, Apr 19, 2018 at 4:51 PM, Julius Werner jwerner@chromium.org wrote:
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.
I said it was helper to fuse the ops. Sure we can invert the code paths.
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).
I can send some CLs out.