Attention is currently required from: Xiang W, Edward O'Callaghan, Angel Pons.
8 comments:
Commit Message:
Sorry I misunderstood.
The change does two things: 1. implements a better mode 0, 2. adds the
option of other modes. The former is most welcome. I also like to get the
whole patch merged, under one condition: The commit message has to give a
reasonable incentive to add the optional modes.
For instance, what chip needs it to be supported? and what other changes
are in the queue to make flashrom support it? Currently all the common code
assumes that the programmer either implements mode 0 or 3 and that all flash
chips support both. If we need to support other chips, the common code would
have to change too, right?
Commit Message:
Patch Set #26, Line 11: may not be
This "may not be" is rather vague. Do we know a real-world use case?
Patchset:
Sorry, I'm very late to this review. There is still the early question
about the reason for the change. Basically, do we have a real-world use
case?
File bitbang_spi.c:
Patch Set #26, Line 171: _set_mosi
Removing the spurious MOSI settings seems fine, but it should be
mentioned in the commit message (or happen in a preceding commit).
Patch Set #26, Line 34: data transmission
Technically, it's the data sampling (or capturing, as Wikipedia puts it)
that happens, not the transmission.
Patch Set #26, Line 96: bitbang_spi_set_sck(master, !cpol);
The first clock change always needs to happen with a delay after
asserting CS#. Both cpha cases should start with a delay.
Patch Set #26, Line 184: if (strlen(mode)) {
So an empty string is not reported as error?
Patch Set #26, Line 187: mode == endptr
This does not account for trailing garbage. It's better to check for
the null termination, e.g.
if (*mode == '\0' || *endptr != '\0' || v > 3 || v < 0) {(this includes checking for an empty string)
To view, visit change 49255. To unsubscribe, or for help writing mail filters, visit settings.