Attention is currently required from: David Reguera Garcia, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email )
Change subject: buspirate_spi: add support for hiz output with pullups=off
......................................................................
Patch Set 11: Code-Review+2
(1 comment)
Patchset:
PS11:
Thank you David for your patience, all looks good.
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 11
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Comment-Date: Thu, 07 Dec 2023 03:55:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen.
David Reguera Garcia has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email )
Change subject: buspirate_spi: add support for hiz output with pullups=off
......................................................................
Patch Set 11:
(1 comment)
File buspirate_spi.c:
https://review.coreboot.org/c/flashrom/+/79299/comment/52afa53f_4d062162 :
PS8, Line 390: else if (strcasecmp("off", tmp) == 0)
: {
: if (pullup)
: {
> Done
Take a look to my new PR
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 11
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 07 Dec 2023 03:34:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: David Reguera Garcia <dreg(a)rootkit.es>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen.
David Reguera Garcia has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email )
Change subject: buspirate_spi: add support for hiz output with pullups=off
......................................................................
Patch Set 11: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 11
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 07 Dec 2023 03:32:52 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, David Reguera Garcia, Thomas Heijligen.
Hello Anastasia Klimchuk, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/79299?usp=email
to look at the new patch set (#11).
The following approvals got outdated and were removed:
Code-Review+1 by David Reguera Garcia, Verified+1 by build bot (Jenkins)
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(a)rootkit.es>
---
M buspirate_spi.c
M doc/classic_cli_manpage.rst
2 files changed, 45 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/79299/11
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 11
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Edward O'Callaghan, Hsuan Ting Chen.
Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79303?usp=email )
Change subject: flashrom_tester: Drop technical debt
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
Hi Edward:
Could you help review these two CLs which break the current flashrom_tester?
--
To view, visit https://review.coreboot.org/c/flashrom/+/79303?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I1ea174c1760fee3e41a2ef0e9779badf0c51298d
Gerrit-Change-Number: 79303
Gerrit-PatchSet: 1
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com>
Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Thu, 07 Dec 2023 03:30:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen.
David Reguera Garcia has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email )
Change subject: buspirate_spi: add support for hiz output with pullups=off
......................................................................
Patch Set 10: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 10
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 07 Dec 2023 03:27:36 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen.
David Reguera Garcia has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email )
Change subject: buspirate_spi: add support for hiz output with pullups=off
......................................................................
Patch Set 10:
(1 comment)
Patchset:
PS10:
Fixed coding style
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 10
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 07 Dec 2023 03:27:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen.
Hello Anastasia Klimchuk, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/79299?usp=email
to look at the new patch set (#10).
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(a)rootkit.es>
---
M buspirate_spi.c
M doc/classic_cli_manpage.rst
2 files changed, 41 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/79299/10
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 10
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen.
David Reguera Garcia has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email )
Change subject: buspirate_spi: add support for hiz output with pullups=off
......................................................................
Patch Set 9:
(5 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/79299/comment/f2be117f_b4b877f9 :
PS8, Line 12:
> Since you merged two patches together, you can add a paragraph from the other patch here, you can ju […]
Done
Patchset:
PS9:
all suggestions done (I hope)
File buspirate_spi.c:
https://review.coreboot.org/c/flashrom/+/79299/comment/27de89ef_46233fd7 :
PS8, Line 374: if (strcasecmp("on", tmp) == 0)
: pullup = true;
: else if (strcasecmp("off", tmp) == 0)
: ; // ignore
: else {
: msg_perr("Invalid pullups state. Use on/off.\n");
: free(tmp);
: return 1;
: }
> For this if-else-elseif and the ones below: […]
Done
https://review.coreboot.org/c/flashrom/+/79299/comment/7246ec32_1c8f4047 :
PS8, Line 390: else if (strcasecmp("off", tmp) == 0)
: {
: if (pullup)
: {
> I think this can be more compact: […]
Done
https://review.coreboot.org/c/flashrom/+/79299/comment/cfc7525a_2683bc8d :
PS8, Line 394: pullups on & hiz off at same time is not possible
> Just slightly add on this message: […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 9
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 07 Dec 2023 03:23:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, David Reguera Garcia, Thomas Heijligen.
Hello Anastasia Klimchuk, Thomas Heijligen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/79299?usp=email
to look at the new patch set (#9).
The following approvals got outdated and were removed:
Code-Review+1 by Anastasia Klimchuk, Code-Review+1 by David Reguera Garcia, Verified+1 by build bot (Jenkins)
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(a)rootkit.es>
---
M buspirate_spi.c
M doc/classic_cli_manpage.rst
2 files changed, 47 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/79299/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855
Gerrit-Change-Number: 79299
Gerrit-PatchSet: 9
Gerrit-Owner: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: David Reguera Garcia <dreg(a)rootkit.es>
Gerrit-MessageType: newpatchset