You are right, you didn't change the clocking. It just looks confusing
now. Btw. changing from CPOL=0 CPHA=0 to CPOL=1 CPHA=1 is no big deal,
the only difference is the idle state of the clock.

Patch set 2:Code-Review +2

View Change

3 comments:

• File bitbang_spi.c:

  • Patch Set #2, Line 47: static void bitbang_spi_set_sck_set_mosi(const struct bitbang_spi_master * const master, int sck, int mosi)

    Now that I looked again, the `sck` parameter is not needed, its
    always called with the same value. Could be renamed to `bitbang_
    spi_clear_sck_set_mosi()` if you want to get rid of it.

  • Patch Set #2, Line 114: bitbang_spi_set_sck_set_mosi(master, 0, 0);

    I don't really understand what you mean here. […]

    You are right. All I saw was that we start at 0 and the first
    thing we do in the loop is set to 0 again, that seemed wrong.
    And I see now why, the code was wrong all the time. Your change
    just makes it more obvious.

  • Patch Set #2, Line 157: bitbang_spi_set_sck(master, 0);

    You mean change from a CPOL=0, CPHA=0 controller into a CPOL=1, CPHA=1 controller (using terminology […]

    I didn't mean to change it to further optimize but to correct it.

    I hope you didn't stare at the diagram on Wikipedia as long as I
    did, it is wrong as well (or at least doesn't reflect the descrip-
    tion).

    Let's see what CPOL=0 CPHA=0 really means. Leading edge is 0 to 1,
    trailing edge is 1 to 0. Now it says "For CPHA=0, the "out" side
    changes the data on the trailing edge of the preceding clock cycle".
    But with this code, there is no preceding clock cycle. I'm not sure
    if that is valid. But it doesn't matter as all slave implementations
    just sample when they see the rising edge.

To view, visit change 26946. To unsubscribe, or for help writing mail filters, visit settings.