Hung-Te Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34161 )
Change subject: board/kukui: Remove ADC tolerance from boardid ......................................................................
board/kukui: Remove ADC tolerance from boardid
The tolerance of ADC is +-10mV, but the resistors may also introduce 1% variation, and causing the final measured voltage to vary around 5%.
By the advisory from hardware team, checking the tolerance seems not really solving or helping anything so we should just ignore that and try to find best matched ID (this also aligns to what Gru did).
BUG=b:136990271 TEST=Booted on Krane and no longer seeing ADC out of range BRANCH=None
Change-Id: Ie02ca5aaafbcfa8f411d973ad0266eee385d6878 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/kukui/boardid.c 1 file changed, 1 insertion(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34161/1
diff --git a/src/mainboard/google/kukui/boardid.c b/src/mainboard/google/kukui/boardid.c index 71f051f..ad06666 100644 --- a/src/mainboard/google/kukui/boardid.c +++ b/src/mainboard/google/kukui/boardid.c @@ -82,15 +82,7 @@ for (id = 0; id < ADC_LEVELS - 1; id++) if (value < (voltages[id] + voltages[id + 1]) / 2) break; - - /* The last level is NC and may be larger than standard tolerance. */ - const int tolerance = 10000; /* 10,000 uV */ - if (id < ADC_LEVELS - 1 && ABS(value - voltages[id]) > tolerance) { - printk(BIOS_ERR, "ADC channel %u value out of range: %d\n", - channel, value); - assert(0); - } - + printk(BIOS_DEBUG, "ADC[%d]: Raw value=%d ID=%d\n", channel, value, id); return id; }
You-Cheng Syu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34161 )
Change subject: board/kukui: Remove ADC tolerance from boardid ......................................................................
Patch Set 1: Code-Review+1
Hello Yu-Ping Wu, Julius Werner, You-Cheng Syu, Ben Ho, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34161
to look at the new patch set (#2).
Change subject: board/kukui: Remove ADC tolerance from boardid ......................................................................
board/kukui: Remove ADC tolerance from boardid
The tolerance of ADC is +-10mV, but the resistors may also introduce 1% variation, and causing the final measured voltage to vary around 5%.
By the advisory from hardware team, checking the tolerance seems not really solving or helping anything so we should just ignore that and try to find best matched ID (this also aligns to what Gru did).
BUG=b:136990271 TEST=Booted on Krane and no longer seeing ADC out of range BRANCH=None
Change-Id: Ie02ca5aaafbcfa8f411d973ad0266eee385d6878 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/mainboard/google/kukui/boardid.c 1 file changed, 1 insertion(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/34161/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34161 )
Change subject: board/kukui: Remove ADC tolerance from boardid ......................................................................
Patch Set 2: Code-Review+1
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34161 )
Change subject: board/kukui: Remove ADC tolerance from boardid ......................................................................
Patch Set 2: Code-Review+2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34161 )
Change subject: board/kukui: Remove ADC tolerance from boardid ......................................................................
Patch Set 2: Code-Review+1
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34161 )
Change subject: board/kukui: Remove ADC tolerance from boardid ......................................................................
board/kukui: Remove ADC tolerance from boardid
The tolerance of ADC is +-10mV, but the resistors may also introduce 1% variation, and causing the final measured voltage to vary around 5%.
By the advisory from hardware team, checking the tolerance seems not really solving or helping anything so we should just ignore that and try to find best matched ID (this also aligns to what Gru did).
BUG=b:136990271 TEST=Booted on Krane and no longer seeing ADC out of range BRANCH=None
Change-Id: Ie02ca5aaafbcfa8f411d973ad0266eee385d6878 Signed-off-by: Hung-Te Lin hungte@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/34161 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Yu-Ping Wu yupingso@google.com Reviewed-by: You-Cheng Syu youcheng@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/kukui/boardid.c 1 file changed, 1 insertion(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Julius Werner: Looks good to me, approved You-Cheng Syu: Looks good to me, but someone else must approve Yu-Ping Wu: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/kukui/boardid.c b/src/mainboard/google/kukui/boardid.c index 71f051f..ad06666 100644 --- a/src/mainboard/google/kukui/boardid.c +++ b/src/mainboard/google/kukui/boardid.c @@ -82,15 +82,7 @@ for (id = 0; id < ADC_LEVELS - 1; id++) if (value < (voltages[id] + voltages[id + 1]) / 2) break; - - /* The last level is NC and may be larger than standard tolerance. */ - const int tolerance = 10000; /* 10,000 uV */ - if (id < ADC_LEVELS - 1 && ABS(value - voltages[id]) > tolerance) { - printk(BIOS_ERR, "ADC channel %u value out of range: %d\n", - channel, value); - assert(0); - } - + printk(BIOS_DEBUG, "ADC[%d]: Raw value=%d ID=%d\n", channel, value, id); return id; }
-
Hung-Te Lin (Code Review) -
Julius Werner (Code Review)
-
Martin Roth (Code Review)
-
Paul Menzel (Code Review)
-
You-Cheng Syu (Code Review)
-
Yu-Ping Wu (Code Review)