Hello CK HU,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/46391
to review the following change.
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
mb/google/asurada: Get ram id from AUX_IN3
Mapping the voltage of AUX_IN3 to ram id.
Signed-off-by: CK Hu ck.hu@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 --- M src/mainboard/google/asurada/boardid.c 1 file changed, 55 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46391/1
diff --git a/src/mainboard/google/asurada/boardid.c b/src/mainboard/google/asurada/boardid.c index 2c8efcd..6dc1f7a 100644 --- a/src/mainboard/google/asurada/boardid.c +++ b/src/mainboard/google/asurada/boardid.c @@ -1,9 +1,58 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <assert.h> #include <boardid.h> +#include <console/console.h> +#include <soc/auxadc.h>
/* board_id is provided by ec/google/chromeec/ec_boardid.c */
+#define ADC_LEVELS 15 + +enum { + RAM_ID_CHANNEL = 3, +}; + +static const int ram_voltages[ADC_LEVELS] = { + /* ID : Voltage (unit: uV) */ + /* 0 : */ 74300, + /* 1 : */ 211700, + /* 2 : */ 318800, + /* 3 : */ 428600, + /* 4 : */ 541700, + /* 5 : */ 665800, + /* 6 : */ 781400, + /* 7 : */ 900000, + /* 8 : */ 1023100, + /* 9 : */ 1137000, + /* 10 : */ 1240000, + /* 11 : */ 1342600, + /* 12 : */ 1457500, + /* 13 : */ 1575600, + /* 14 : */ 1683600, +}; + +static const int *adc_voltages[] = { + [RAM_ID_CHANNEL] = ram_voltages, +}; + +static uint32_t get_adc_index(unsigned int channel) +{ + int value = auxadc_get_voltage(channel); + + assert(channel < ARRAY_SIZE(adc_voltages)); + const int *voltages = adc_voltages[channel]; + assert(voltages); + + /* Find the closest voltage */ + uint32_t id; + for (id = 0; id < ADC_LEVELS - 1; id++) + if (value < (voltages[id] + voltages[id + 1]) / 2) + break; + printk(BIOS_DEBUG, "ADC[%d]: Raw value=%d ID=%d\n", channel, value, id); + return id; +} + uint32_t sku_id(void) { return 0; @@ -11,5 +60,10 @@
uint32_t ram_code(void) { - return 0; + static uint32_t cached_ram_code = BOARD_ID_INIT; + + if (cached_ram_code == BOARD_ID_INIT) + cached_ram_code = get_adc_index(RAM_ID_CHANNEL); + + return cached_ram_code; }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/1/src/mainboard/google/asurad... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/1/src/mainboard/google/asurad... PS1, Line 51: break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/46391/1/src/mainboard/google/asurad... PS1, Line 51: break; please, no spaces at the start of a line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/2/src/mainboard/google/asurad... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/2/src/mainboard/google/asurad... PS2, Line 51: break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/46391/2/src/mainboard/google/asurad... PS2, Line 51: break; please, no spaces at the start of a line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/3/src/mainboard/google/asurad... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/3/src/mainboard/google/asurad... PS3, Line 51: break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/46391/3/src/mainboard/google/asurad... PS3, Line 51: break; please, no spaces at the start of a line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/4/src/mainboard/google/asurad... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/4/src/mainboard/google/asurad... PS4, Line 51: break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/46391/4/src/mainboard/google/asurad... PS4, Line 51: break; please, no spaces at the start of a line
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46391/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46391/4//COMMIT_MSG@9 PS4, Line 9: Mapping the voltage of AUX_IN3 to ram id. Where is the map documented?
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/5/src/mainboard/google/asurad... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/5/src/mainboard/google/asurad... PS5, Line 51: break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/46391/5/src/mainboard/google/asurad... PS5, Line 51: break; please, no spaces at the start of a line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 6:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/6/src/mainboard/google/asurad... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/6/src/mainboard/google/asurad... PS6, Line 51: break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/46391/6/src/mainboard/google/asurad... PS6, Line 51: break; please, no spaces at the start of a line
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/7/src/mainboard/google/asurad... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/7/src/mainboard/google/asurad... PS7, Line 51: break; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/46391/7/src/mainboard/google/asurad... PS7, Line 51: break; please, no spaces at the start of a line
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get ram id from AUX_IN3 ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46391/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46391/4//COMMIT_MSG@7 PS4, Line 7: b/google/asurada: Get ram id from AUX_IN3 : : Mapping the voltage of AUX_IN3 to ram id. Maybe change to:
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Hello build bot (Jenkins), CK HU,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46391
to look at the new patch set (#8).
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Signed-off-by: CK Hu ck.hu@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 --- M src/mainboard/google/asurada/boardid.c 1 file changed, 56 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46391/8
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 8:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46391/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46391/4//COMMIT_MSG@9 PS4, Line 9: Mapping the voltage of AUX_IN3 to ram id.
Where is the map documented?
Ack
https://review.coreboot.org/c/coreboot/+/46391/4//COMMIT_MSG@7 PS4, Line 7: b/google/asurada: Get ram id from AUX_IN3 : : Mapping the voltage of AUX_IN3 to ram id.
Maybe change to: […]
Ack
Hello build bot (Jenkins), CK HU,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46391
to look at the new patch set (#15).
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Signed-off-by: CK Hu ck.hu@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 --- M src/mainboard/google/asurada/boardid.c 1 file changed, 56 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46391/15
Hello build bot (Jenkins), CK HU,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46391
to look at the new patch set (#22).
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Signed-off-by: CK Hu ck.hu@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 --- M src/mainboard/google/asurada/boardid.c 1 file changed, 56 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46391/22
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 22:
(5 comments)
https://review.coreboot.org/c/coreboot/+/46391/22//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46391/22//COMMIT_MSG@11 PS22, Line 11: For Asurada the RAM code can be read from ADC channel 3. Can be merged to previous line. Be aware of the 72 line length limit.
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 18: /* 0 : */ 74300, Why not use
[0] = 74300,
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 41: int unsigned int
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 53: d u
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 53: d u
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 22:
(3 comments)
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 16: int unsigned int
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 35: int unsigned int
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 53: d u
Hello build bot (Jenkins), CK HU,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46391
to look at the new patch set (#23).
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Signed-off-by: CK Hu ck.hu@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 --- M 3rdparty/amd_blobs M 3rdparty/blobs M 3rdparty/intel-microcode M 3rdparty/qc_blobs M src/mainboard/google/asurada/boardid.c 5 files changed, 60 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46391/23
Hello build bot (Jenkins), CK HU,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46391
to look at the new patch set (#24).
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Signed-off-by: CK Hu ck.hu@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 --- M src/mainboard/google/asurada/boardid.c 1 file changed, 56 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46391/24
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 24:
(7 comments)
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 16: int
unsigned int
Done
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 18: /* 0 : */ 74300,
Why not use […]
Done
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 35: int
unsigned int
Done
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 41: int
unsigned int
Done
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 53: d
u
Done
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 53: d
u
Done
https://review.coreboot.org/c/coreboot/+/46391/22/src/mainboard/google/asura... PS22, Line 53: d
u
Done
Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46391/22//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46391/22//COMMIT_MSG@11 PS22, Line 11: For Asurada the RAM code can be read from ADC channel 3.
Can be merged to previous line. Be aware of the 72 line length limit.
Done
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 24:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46391/24/src/mainboard/google/asura... File src/mainboard/google/asurada/boardid.c:
https://review.coreboot.org/c/coreboot/+/46391/24/src/mainboard/google/asura... PS24, Line 44: const int unsigned *voltages = adc_voltages[channel]; type 'const int unsigned *' should be specified in [[un]signed] [short|int|long|long long] order
Hello build bot (Jenkins), CK HU,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46391
to look at the new patch set (#25).
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Signed-off-by: CK Hu ck.hu@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 --- M src/mainboard/google/asurada/boardid.c 1 file changed, 56 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/46391/25
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 25: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
Patch Set 25: Code-Review+2
Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46391 )
Change subject: mb/google/asurada: Get RAM code from ADC 3 ......................................................................
mb/google/asurada: Get RAM code from ADC 3
On Chromebooks the RAM code is implemented by the resistor straps that we can read and decode from ADC. For Asurada the RAM code can be read from ADC channel 3.
Signed-off-by: CK Hu ck.hu@mediatek.com Signed-off-by: Yidi Lin yidi.lin@mediatek.com Change-Id: Iaadabea1b6aa91c48b137f7c6784ab7ee0adc473 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46391 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Hung-Te Lin hungte@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/google/asurada/boardid.c 1 file changed, 56 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Hung-Te Lin: Looks good to me, approved
diff --git a/src/mainboard/google/asurada/boardid.c b/src/mainboard/google/asurada/boardid.c index 2c8efcd..cb91812 100644 --- a/src/mainboard/google/asurada/boardid.c +++ b/src/mainboard/google/asurada/boardid.c @@ -1,9 +1,59 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <assert.h> #include <boardid.h> +#include <console/console.h> +#include <soc/auxadc.h>
/* board_id is provided by ec/google/chromeec/ec_boardid.c */
+#define ADC_LEVELS 15 + +enum { + RAM_ID_CHANNEL = 3, +}; + +static const unsigned int ram_voltages[ADC_LEVELS] = { + /* ID : Voltage (unit: uV) */ + [0] = 74300, + [1] = 211700, + [2] = 318800, + [3] = 428600, + [4] = 541700, + [5] = 665800, + [6] = 781400, + [7] = 900000, + [8] = 1023100, + [9] = 1137000, + [10] = 1240000, + [11] = 1342600, + [12] = 1457500, + [13] = 1575600, + [14] = 1683600, +}; + +static const unsigned int *adc_voltages[] = { + [RAM_ID_CHANNEL] = ram_voltages, +}; + +static uint32_t get_adc_index(unsigned int channel) +{ + unsigned int value = auxadc_get_voltage_uv(channel); + + assert(channel < ARRAY_SIZE(adc_voltages)); + const unsigned int *voltages = adc_voltages[channel]; + assert(voltages); + + /* Find the closest voltage */ + uint32_t id; + for (id = 0; id < ADC_LEVELS - 1; id++) + if (value < (voltages[id] + voltages[id + 1]) / 2) + break; + + printk(BIOS_DEBUG, "ADC[%u]: Raw value=%u ID=%u\n", channel, value, id); + return id; +} + uint32_t sku_id(void) { return 0; @@ -11,5 +61,10 @@
uint32_t ram_code(void) { - return 0; + static uint32_t cached_ram_code = BOARD_ID_INIT; + + if (cached_ram_code == BOARD_ID_INIT) + cached_ram_code = get_adc_index(RAM_ID_CHANNEL); + + return cached_ram_code; }
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Hung-Te Lin (Code Review)
-
Paul Menzel (Code Review)
-
Yidi Lin (Code Review)
-
Yu-Ping Wu (Code Review)