Change in flashrom[master]: [NOTFORMERGE] Add thoughts about bitbang CPOL/CPHA patches

[Nico Huber (Code Review)]

Attention is currently required from: Xiang W, Angel Pons.
Hello Xiang W, Angel Pons,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/flashrom/+/55862

to review the following change.

Change subject: [NOTFORMERGE] Add thoughts about bitbang CPOL/CPHA patches
......................................................................

[NOTFORMERGE] Add thoughts about bitbang CPOL/CPHA patches

Change-Id: I6a4b64c686497a7df99ea50273ed90105d669858
Signed-off-by: Nico Huber 
---
M Documentation/bitbang_spi_analysis.md
1 file changed, 95 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/55862/1

diff --git a/Documentation/bitbang_spi_analysis.md b/Documentation/bitbang_spi_analysis.md
index 293abe5..02196af 100644
--- a/Documentation/bitbang_spi_analysis.md
+++ b/Documentation/bitbang_spi_analysis.md
@@ -148,3 +148,98 @@
 This would have to be tested. It even seems possible that raising
 `.half_period` above 0 together with this optimization might
 increase the speed and reliability.
+
+What happens with CB:49255 (bitbang-spi.c: support clock polarity and phase)
+----------------------------------------------------------------------------
+
+Let's assume the default `cpol = 0`, `cpha = 0`.
+
+         __  h    h   h    h   h    h   h    h
+     CS#   |_____________________________________
+                ____     ____     ____     ____
+     SCK ______|    |___|    |___|    |___|    |_
+
+    MOSI XXXXXXX111111111222222222222222222222222
+
+    MISO XXXXXXXXXXXXXXXXXXXXXX333333333444444444
+                                  ^        ^
+
+MOSI is off by `.half_period`. It should be sampled right after the
+rising edge of `SCK` where it's unstable. Still, how does it look
+like with `.half_period = 0`?
+
+         __
+     CS#   |_____________
+             _  _  _  _
+     SCK ___| || || || |_
+
+    MOSI XXXX111222222222
+
+    MISO XXXXXXXXXX333444
+                   ^  ^
+
+Roughly the same, although MISO looks a little tighter. Note that
+the `SCK` pattern is the same now for writing and reading. This is
+because programming `MOSI` happens at the wrong time.
+
+Fixup to CB:49255 (bitbang_spi.c: fix spi sequence in time and rename)
+----------------------------------------------------------------------
+
+         __  h    h   h    h   h    h   h    h
+     CS#   |_____________________________________
+                 ___      ___     ____     ____
+     SCK _______|   |____|   |___|    |___|    |_
+
+    MOSI XXXXXX1111111112222222222222222222222222
+
+    MISO XXXXXXXXXXXXXXXXXXXXXX333333333444444444
+                                  ^        ^
+
+This gives a little more margin to the botched `MOSI` setting. But,
+if looked at with a high `.half_period`, it would still look the
+same. However, with `.half_period = 0`
+
+         __
+     CS#   |_____________
+                   _  _
+     SCK ____||_||| || |_
+
+    MOSI XXX1112222222222
+
+    MISO XXXXXXXXXX333444
+                   ^  ^
+
+things look better. The rising edge of `SCK` aligns again with
+`MOSI`. So the fixup fixes the regression for programmers with
+`.half_period = 0`. Not necessarily for other values.
+
+Let's have a final look at this version for programmers that
+pack the `SCK` setting with programming `MOSI` and sampling
+`MISO`.
+
+         __  h   h   h   h   h   h   h   h
+     CS#   |_________________________________
+                ___     ___     ___     ___
+     SCK ______|   |___|   |___|   |___|   |_
+
+    MOSI XXXXXX111111112222222222222222222222
+
+    MISO XXXXXXXXXXXXXXXXXXXX3333333344444444
+                               ^       ^
+
+As things are packed, it's the same as without the fixup. And
+without `.half_period`:
+
+         __
+     CS#   |_________
+
+     SCK ___||||||||_
+
+    MOSI XXX112222222
+
+    MISO XXXXXXXX3344
+                ^ ^
+
+`MOSI` is still less likely to be stable at the rising edge of
+`SCK`. For sampling `MISO` it's the same as for current `master`
+(after commit 455a6fc8).