Nico Huber has posted comments on this change. ( https://review.coreboot.org/26946
Change subject: bitbang_spi: Add functions to optimize xfers
Patch Set 2: Code-Review+2
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.
PS2, 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.
PS2, 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.
PS2, 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-
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 https://review.coreboot.org/26946
To unsubscribe, or for help writing mail filters, visit
Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org>
Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org>
Gerrit-Reviewer: Idwer Vollering <vidwer(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 26 Jun 2018 18:06:43 +0000