Anastasia Klimchuk submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Dreg: Looks good to me, but someone else must approve
buspirate_spi: add support for hiz output with pullups=off

When working with low-voltage chips, the internal 10k pull-ups
of the Bus Pirate might be too high. In such cases, it's necessary
to create an external pull-up using lower-value resistors. For this,
you can use the 'hiz' parameter. This way, the Bus Pirate will
operate as an open drain with 'pullups=off'

So, why not just use the 'pullups=on' option for this scenario? I'm
trying to prevent a very typical human error in the training sessions
I conduct.

For example, in the previous session, a user might have left the VPU
(vextern) connected to 5V, and now they need to access a 1.8V chip.
If 'pullups=on' is used, the following will happen:
5V (VPU) --> CD4060 --> 2K (internal Bus Pirate) --> MOSI target chip.
By having the option to set a 'hiz=on', this can be avoided. Since the
pull-ups will remain deactivated while the Bus Pirate will function in
an open-drain mode

Return init error for invalid values of pullups, hiz, psus. Previously,
invalid values of the params pullups, hiz, psus were handled as "off"
(i.e. disabled). This created a potential human error when users were
running flashrom with e.g. pullups=1 and expected pullups to be on,
while the value 1 was handled as invalid and off.

Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Signed-off-by: David Reguera Garcia <dreg@rootkit.es>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/79299
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
---
M buspirate_spi.c
M doc/classic_cli_manpage.rst
2 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/buspirate_spi.c b/buspirate_spi.c
index 72c28b0..3a1e414 100644
--- a/buspirate_spi.c
+++ b/buspirate_spi.c
@@ -325,6 +325,7 @@
int spispeed = 0x7;
int serialspeed_index = -1;
int ret = 0;
+ bool hiz = false;
bool pullup = false;
bool psu = false;
bool aux = true;
@@ -370,23 +371,49 @@

tmp = extract_programmer_param_str(cfg, "pullups");
if (tmp) {
- if (strcasecmp("on", tmp) == 0)
+ if (strcasecmp("on", tmp) == 0) {
pullup = true;
- else if (strcasecmp("off", tmp) == 0)
+ } else if (strcasecmp("off", tmp) == 0) {
; // ignore
- else
- msg_perr("Invalid pullups state, not using them.\n");
+ } else {
+ msg_perr("Invalid pullups state. Use on/off.\n");
+ free(tmp);
+ return 1;
+ }
+ }
+ free(tmp);
+
+ tmp = extract_programmer_param_str(cfg, "hiz");
+ if (tmp) {
+ if (strcasecmp("on", tmp) == 0) {
+ hiz = true;
+ } else if (strcasecmp("off", tmp) == 0) {
+ if (pullup) {
+ msg_perr("Invalid combination: pullups=on & hiz=off at same time is not possible.\n");
+ free(tmp);
+ return 1;
+ } else {
+ ; // ignore
+ }
+ } else {
+ msg_perr("Invalid hiz state. Use on/off.\n");
+ free(tmp);
+ return 1;
+ }
}
free(tmp);

tmp = extract_programmer_param_str(cfg, "psus");
if (tmp) {
- if (strcasecmp("on", tmp) == 0)
+ if (strcasecmp("on", tmp) == 0) {
psu = true;
- else if (strcasecmp("off", tmp) == 0)
+ } else if (strcasecmp("off", tmp) == 0) {
; // ignore
- else
- msg_perr("Invalid psus state, not enabling.\n");
+ } else {
+ msg_perr("Invalid psus state. Use on/off.\n");
+ free(tmp);
+ return 1;
+ }
}
free(tmp);

@@ -692,9 +719,9 @@

/* Set SPI config: output type, idle, clock edge, sample */
bp_commbuf[0] = 0x80 | 0xa;
- if (pullup) {
+ if (pullup || hiz) {
bp_commbuf[0] &= ~(1 << 3);
- msg_pdbg("Pull-ups enabled, so using HiZ pin output! (Open-Drain mode)\n");
+ msg_pdbg("Pull-ups or HiZ enabled, so using HiZ pin output! (Open-Drain mode)\n");
}
ret = buspirate_sendrecv(bp_commbuf, 1, 1);
if (ret)
diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst
index 5d6b7ef..0cd45ca 100644
--- a/doc/classic_cli_manpage.rst
+++ b/doc/classic_cli_manpage.rst
@@ -772,6 +772,14 @@
More information about the Bus Pirate pull-up resistors and their purpose is available
`in a guide by dangerousprototypes <http://dangerousprototypes.com/docs/Practical_guide_to_Bus_Pirate_pull-up_resistors>`_.

+When working with low-voltage chips, the internal 10k pull-ups of the Bus Pirate might be too high. In such cases, it's necessary to create an external pull-up using lower-value resistors.
+
+For this, you can use the ``hiz`` parameter. This way, the Bus Pirate will operate as an open drain. Syntax is::
+
+ flashrom -p buspirate_spi:hiz=state
+
+where ``state`` can be ``on`` or ``off``.
+
The state of the Bus Pirate power supply pins is controllable through an optional ``psus`` parameter. Syntax is::

flashrom -p buspirate_spi:psus=state

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

Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 12
Gerrit-Owner: Dreg <dreg@rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm@chromium.org>
Gerrit-Reviewer: Dreg <dreg@rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src@posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged