Attention is currently required from: Angel Pons.
Hello Angel Pons,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/55860
to review the following change.
Change subject: [DRAFT] Document bitbang SPI thoughts
......................................................................
[DRAFT] Document bitbang SPI thoughts
Change-Id: I2437ad736b6677ad6eefa17a7b2d65d6f489f890
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
A Documentation/bitbang_spi_analysis.md
1 file changed, 150 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/55860/1
diff --git a/Documentation/bitbang_spi_analysis.md b/Documentation/bitbang_spi_analysis.md
new file mode 100644
index 0000000..765aeb7
--- /dev/null
+++ b/Documentation/bitbang_spi_analysis.md
@@ -0,0 +1,150 @@
+Analysis of changes to flashrom's `bitbang_spi` driver
+======================================================
+
+* `.half_period` is considered 3 chars long, marked by an `h`.
+
+* Delays incurred by programming or reading a hardware value
+ are 1 char long. So are transmission delays including the
+ time the other endpoint needs to react.
+
+* Assumed is `mode 0`, i.e. `CPOL = 0` and `CPHA = 0`. Changes
+ should happen after a falling edge of `SCK` or `CS#` and
+ sampling after a rising edge of `SCK`.
+
+* `^` denotes a sampling at our end.
+
+Before commit b623f403 (bitbang_spi: Add functions to optimize xfers)
+---------------------------------------------------------------------
+
+ __ h h h h h h h h
+ CS# |_________________________________________
+ ____ ____ ____ ____
+ SCK _______| |____| |____| |____| |_
+
+ MOSI ___11111111112222222222333333333344444444445
+
+ MISO XXXX1111111111222222222233333333334444444444
+ ^ ^ ^ ^
+
+This looks quite balanced actually. Now, what happens if we set
+`.half_period = 0`?
+
+ __
+ CS# |_________________
+ _ _ _ _
+ SCK ____| |_| |_| |_| |_
+
+ MOSI ___11112222333344445
+
+ MISO XXXX1111222233334444
+ ^ ^ ^ ^
+
+Looks ok'ish. Only notice that we sample a little early. It might
+be better to sample after the falling edge of `SCK`, however that
+would make the code incompatible to `.half_period != 0` :-/
+
+With commit b623f403 (bitbang_spi: Add functions to optimize xfers)
+-------------------------------------------------------------------
+
+We explicitly set `SCK = 0` before setting `MOSI` now, which adds
+a tiny delay at the start:
+
+ __ h h h h h h h h
+ CS# |__________________________________________
+ ____ ____ ____ ____
+ SCK ________| |____| |____| |____| |_
+
+ MOSI ____11111111112222222222333333333344444444445
+
+ MISO XXXX11111111111222222222233333333334444444444
+ ^ ^ ^ ^
+
+Nothing to spectacular with `.half_period = 0` either:
+
+ __
+ CS# |__________________
+ _ _ _ _
+ SCK _____| |_| |_| |_| |_
+
+ MOSI ____11112222333344445
+
+ MISO XXXX11111222233334444
+ ^ ^ ^ ^
+
+With commit 455a6fc8 (bitbang_spi: Add half-duplex optimizations)
+-----------------------------------------------------------------
+
+We assume half-duplex from now on. This removes a sampling delay
+per bit on our end during the first, writing phase, and a program-
+ming delay per bit during the second, read phase.
+
+We still program `MOSI = 0` at the start of every byte *read*,
+though. Don't know why. It incurs a programming delay per byte.
+We'll mark this with `o` for odd. We'll also assume bytes are
+only 2 bits long, to not make the diagrams explode.
+
+ __ h h h h h h h h
+ CS# |_______________________________________
+ ___ ___ ____ ____
+ SCK ________| |____| |____| |___| |_
+
+ MOSI ____111111111222222222____________________
+
+ MISO XXXXXXXXXXXXXXXXXXXXXXX3333333333444444444
+ ^ ^
+
+Note that during the write phase, the clock-high periods are
+shorter now, and during the read phase, the clock-lock periods
+are shorter. Now, once again, let's remove the `.half_period`
+delays (vim makes this awesomely easy btw. ^^).
+
+ __
+ CS# |_______________
+ _ _
+ SCK _____||_||_| || ||
+
+ MOSI ____111222________
+
+ MISO XXXXXXXXXXX3333444
+ ^ ^
+
+This looks pretty tense. But remember: everyone is supposed
+to sample after a rising edge of `SCK`. Which is right in the
+center of our `MOSI` output. With `MISO`, however, things
+look bad. This is assuming programming, sampling and a trans-
+mission round-trip all take the same time. *If* the round-trip
+is not longer, we should still be safe.
+
+Making use of 455a6fc8's optimizations
+--------------------------------------
+
+Now what happens if we pack setting `SCK = 0` with setting
+`MOSI` and setting `SCK = 1` with sampling `MISO`?
+
+ __ h h h h h h h h
+ CS# |___________________________________
+ ___ ___ ___ ___
+ SCK ________| |___| |___| |___| |_
+
+ MOSI ____1111111122222222__________________
+
+ MISO XXXXXXXXXXXXXXXXXXXXXX3333333344444444
+ ^ ^
+
+A nice picture once more, but what happens if we ditch
+`.half_period`?
+
+ __
+ CS# |___________
+
+ SCK _____||||||||_
+
+ MOSI ____1122______
+
+ MISO XXXXXXXXXX3344
+ ^ ^
+
+We'd definitely sample very early. Could it work? I don't know.
+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.
--
To view, visit https://review.coreboot.org/c/flashrom/+/55860
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2437ad736b6677ad6eefa17a7b2d65d6f489f890
Gerrit-Change-Number: 55860
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Nico Huber.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49265 )
Change subject: rayer_spi.c: Implement `set_sck_set_mosi`
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> Need to think some more about this as I just realized that this driver […]
Hmmm, good point.
CB:49267 and CB:55720 are for nicintel_spi, which sets `.half_delay = 1`. And both were tested by Idwer and me.
--
To view, visit https://review.coreboot.org/c/flashrom/+/49265
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ieb05a35967685c2444d064cb61cec98e2a00f590
Gerrit-Change-Number: 49265
Gerrit-PatchSet: 3
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org>
Gerrit-CC: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Fri, 25 Jun 2021 15:44:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49265 )
Change subject: rayer_spi.c: Implement `set_sck_set_mosi`
......................................................................
Patch Set 3: -Code-Review
(1 comment)
Patchset:
PS3:
Need to think some more about this as I just realized that this driver
(among others) sets `.half_delay = 0`. This will make things more
susceptible wrt. timing changes.
--
To view, visit https://review.coreboot.org/c/flashrom/+/49265
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ieb05a35967685c2444d064cb61cec98e2a00f590
Gerrit-Change-Number: 49265
Gerrit-PatchSet: 3
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org>
Gerrit-CC: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 25 Jun 2021 15:41:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner.
Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 )
Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS4:
> Oh wow, I wasn't aware of board_enable :O Thanks for that hint. […]
Yeah, I started some work on the Super I/O is preceding patch: 55849
However it is still much to do. The code for Super I/Os is too scattered.
--
To view, visit https://review.coreboot.org/c/flashrom/+/55715
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
Gerrit-Change-Number: 55715
Gerrit-PatchSet: 10
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Comment-Date: Fri, 25 Jun 2021 15:20:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons.
Hello Felix Singer, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/55715
to look at the new patch set (#10).
Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
......................................................................
ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
Add a new programmer supporting ITE Embedded Controllers found on
TUXEDO laptops. Tested on TUXEDO InfinityBook S 14 Gen6 and 15 Gen6
with EC firmware updates from IBV, EC versions 1.07.02TR1 and
1.07.04TR4.
Example standard command to update EC firmware on a TUXEDO laptop:
"flashrom -p ite_ec:romsize=128K,autoload=disable -w eL14MU02.TR1"
Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
---
M Makefile
M flashrom.8.tmpl
M it87spi.c
A ite_ec.c
A ite_ec.h
M programmer.h
M programmer_table.c
7 files changed, 1,111 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/55715/10
--
To view, visit https://review.coreboot.org/c/flashrom/+/55715
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031
Gerrit-Change-Number: 55715
Gerrit-PatchSet: 10
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset