Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email )
Change subject: buspirate_spi: add support for hiz output with pullups=off ......................................................................
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(-)
Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Dreg: Looks good to me, but someone else must approve
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