Kevin Chiu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
mb/google/kukui: fix SKUID config for burnet/esche
ADCIN2 (LCM_ID) now was PD 200K but we might get incorrect voltage in different condition: developer mode ADC[2]: Raw value=232459 ID=1
recovery mode: ADC[2]: Raw value=36144 ID=0
in order to match sku definition of burnet/esche: burnet: 0x11 esche: 0x10
we'll need to bypass ADCIN2 to update SKUID accordingly.
BUG=b:170916885 BRANCH=master TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 5 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/1
diff --git a/src/mainboard/google/kukui/boardid.c b/src/mainboard/google/kukui/boardid.c index 548f36b..71df979 100644 --- a/src/mainboard/google/kukui/boardid.c +++ b/src/mainboard/google/kukui/boardid.c @@ -96,6 +96,11 @@ } }
+ if (CONFIG(BOARD_GOOGLE_BURNET)) { + cached_sku_id = (0x10 | get_adc_index(SKU_ID_CHANNEL)); + return cached_sku_id; + } + /* * The SKU (later used for device tree matching) is combined from: * ADC2[4bit/H] = straps on LCD module (type of panel).
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... PS1, Line 99: if (CONFIG(BOARD_GOOGLE_BURNET)) { : cached_sku_id = (0x10 | get_adc_index(SKU_ID_CHANNEL)); : return cached_sku_id; : } : given Jacuzzi followers should not use need LCD module, I'd recommend changing this for all Jacuzzi:
uint32_t lcm_id = 0;
if (!CONFIG(BOARD_GOOGLE_JACUZZI_COMMON)) lcm_id = get_adc_index(LCM_ID_CHANNEL);
.... cached_sku_id = lcm_id << 4 | get_adc_index(SKU_ID_CHANNEL);
And please also change the burnet/esche model file to only do 0x0, 0x1 (not 0x10, 0x11).
Chen-Tsung Hsieh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... PS1, Line 99: if (CONFIG(BOARD_GOOGLE_BURNET)) { : cached_sku_id = (0x10 | get_adc_index(SKU_ID_CHANNEL)); : return cached_sku_id; : } :
given Jacuzzi followers should not use need LCD module, I'd recommend changing this for all Jacuzzi: […]
The LCM_ID HW strap of kappa & juniper are actually 1. Their sku-ids are 0x10 (not 0x0) now. I'm worried about changing the rule of sku-id for MP devices.
I'd recommend that we still focus on debuging the floating HW strap value of LCM_ID. Then, we don't have to patch anything.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... PS1, Line 99: if (CONFIG(BOARD_GOOGLE_BURNET)) { : cached_sku_id = (0x10 | get_adc_index(SKU_ID_CHANNEL)); : return cached_sku_id; : } :
The LCM_ID HW strap of kappa & juniper are actually 1. Their sku-ids are 0x10 (not 0x0) now. I'm worried about changing the rule of sku-id for MP devices.
Yes but they don't have other SKUs (single SKU) so it's probably not a big deal - especially there is only one DTS for them.
In the meantime we can also hard-code LCMID to 1 for JACUZZI devices.
I'd recommend that we still focus on debuging the floating HW strap value of LCM_ID. Then, we don't have to patch anything.
That's also true, but I was concerned maybe juniper and kappa have exactly the same issue (floating LCM_ID) and we didn't notice it (due to single SKU, no matching issue). In that sense hard-coding the LCM may also be a good idea for future.
Conclusion: I think we should keep debugging, and maybe also check shipped devices as well. And if that is a hardware issue that cannot be easily fixed, we should consider putting a hard-coded value to JACUZZI LCM_ID.
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... PS1, Line 99: if (CONFIG(BOARD_GOOGLE_BURNET)) { : cached_sku_id = (0x10 | get_adc_index(SKU_ID_CHANNEL)); : return cached_sku_id; : } :
The LCM_ID HW strap of kappa & juniper are actually 1. […]
so we should hard-code LCM_ID to 1 for all JACUZZI device for now? and sure, we'll keep checking hardware in parallel. thanks.
Chen-Tsung Hsieh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... PS1, Line 99: if (CONFIG(BOARD_GOOGLE_BURNET)) { : cached_sku_id = (0x10 | get_adc_index(SKU_ID_CHANNEL)); : return cached_sku_id; : } :
so we should hard-code LCM_ID to 1 for all JACUZZI device for now? and sure, we'll keep checking har […]
I agree to hard-code LCM_ID to 1 (even if we resolve the floating LCM_ID issue from HW). LCM_ID is marked as 'NOT USED IN JACUUZI' in schematic. Hard-coding it with config BOARD_GOOGLE_JACUZZI_COMMON is more consistent to that statement.
Hello Hung-Te Lin, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#2).
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
mb/google/kukui: fix SKUID config for burnet/esche
ADCIN2 (LCM_ID) now was PD 200K but we might get incorrect voltage in different condition: developer mode ADC[2]: Raw value=232459 ID=1
recovery mode: ADC[2]: Raw value=36144 ID=0
in order to match sku definition of burnet/esche: burnet: 0x11 esche: 0x10
we'll need to bypass ADCIN2 to update SKUID accordingly.
BUG=b:170916885 BRANCH=master TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/2
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/1/src/mainboard/google/kukui/... PS1, Line 99: if (CONFIG(BOARD_GOOGLE_BURNET)) { : cached_sku_id = (0x10 | get_adc_index(SKU_ID_CHANNEL)); : return cached_sku_id; : } :
I agree to hard-code LCM_ID to 1 (even if we resolve the floating LCM_ID issue from HW). […]
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 2: Code-Review+1
@yupingso, what do you think about this?
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix SKUID config for burnet/esche ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46442/2//COMMIT_MSG@7 PS2, Line 7: mb/google/kukui: fix SKUID config for burnet/esche : : ADCIN2 (LCM_ID) now was PD 200K but we might get incorrect voltage in : different condition: : developer mode : ADC[2]: Raw value=232459 ID=1 : : recovery mode: : ADC[2]: Raw value=36144 ID=0 : : in order to match sku definition of burnet/esche: : burnet: 0x11 : esche: 0x10 : : we'll need to bypass ADCIN2 to update SKUID accordingly. mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers
The LCM ID is not really used on Jacuzzi followers and the ADC may return unexpected value on some boards, so we want to change that to a fixed value for all Jacuzzi followers.
Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#3).
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers
The LCM ID is not really used on Jacuzzi followers and the ADC may return unexpected value on some boards, so we want to change that to a fixed value for all Jacuzzi followers.
BUG=b:170916885 BRANCH=master TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/3
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46442/2//COMMIT_MSG@7 PS2, Line 7: mb/google/kukui: fix SKUID config for burnet/esche : : ADCIN2 (LCM_ID) now was PD 200K but we might get incorrect voltage in : different condition: : developer mode : ADC[2]: Raw value=232459 ID=1 : : recovery mode: : ADC[2]: Raw value=36144 ID=0 : : in order to match sku definition of burnet/esche: : burnet: 0x11 : esche: 0x10 : : we'll need to bypass ADCIN2 to update SKUID accordingly.
mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers […]
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/3/src/mainboard/google/kukui/... PS3, Line 196: uint32_t lcm_id = 1; give we're not reusing this later, move this to inside the config block, e.g.:
if (CONFIG...) { uint32_t lcm_id = 1; /* LCM is unused on Jacuzzi followers */ cached_sku_id = (lcm_id << 4 | get_adc_index(SKU_ID_CHANNEL)); ...
Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#4).
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers
The LCM ID is not really used on Jacuzzi followers and the ADC may return unexpected value on some boards, so we want to change that to a fixed value for all Jacuzzi followers.
BUG=b:170916885 BRANCH=master TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/4
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/3/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/3/src/mainboard/google/kukui/... PS3, Line 196: uint32_t lcm_id = 1;
give we're not reusing this later, move this to inside the config block, e.g.: […]
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 4:
(2 comments)
Really sorry but can we change the implementation again to minimize the duplication?
https://review.coreboot.org/c/coreboot/+/46442/4/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/4/src/mainboard/google/kukui/... PS4, Line 171: /* Returns the ID for LCD module (type of panel). */ static uint8_t lcm_id(void) { /* LCM is unused on Jacuzzi followers. */ if (CONFIG(BOARD_GOOGLE_JACUZZI_COMMON)) return 1;
get_adc_index(SKU_ID_CHANNEL)); }
https://review.coreboot.org/c/coreboot/+/46442/4/src/mainboard/google/kukui/... PS4, Line 209: get_adc_index(LCM_ID_CHANNEL) lcm_id()
Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#5).
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers
The LCM ID is not really used on Jacuzzi followers and the ADC may return unexpected value on some boards, so we want to change that to a fixed value for all Jacuzzi followers.
BUG=b:170916885 BRANCH=master TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/5
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46442/4/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/4/src/mainboard/google/kukui/... PS4, Line 171:
/* Returns the ID for LCD module (type of panel). */ […]
Done
https://review.coreboot.org/c/coreboot/+/46442/4/src/mainboard/google/kukui/... PS4, Line 209: get_adc_index(LCM_ID_CHANNEL)
lcm_id()
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 5: Code-Review+2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/5/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/5/src/mainboard/google/kukui/... PS5, Line 173: Could you remove this blank line? Also it's nice to take the chance to fix the one before sku_id().
Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#6).
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers
The LCM ID is not really used on Jacuzzi followers and the ADC may return unexpected value on some boards, so we want to change that to a fixed value for all Jacuzzi followers.
BUG=b:170916885 BRANCH=master TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 10 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/6
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/5/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/5/src/mainboard/google/kukui/... PS5, Line 173:
Could you remove this blank line? Also it's nice to take the chance to fix the one before sku_id().
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 6: Code-Review+2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: fix LCMID to be 0x01 for Jacuzzi followers ......................................................................
Patch Set 6: Code-Review+2
Hung-Te Lin has uploaded a new patch set (#7) to the change originally created by Kevin Chiu. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 18 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/7
Hung-Te Lin has uploaded a new patch set (#8) to the change originally created by Kevin Chiu. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/boardid.c 1 file changed, 23 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/8
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/8/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/8/src/mainboard/google/kukui/... PS8, Line 179: CONFIG(BOARD_GOOGLE_JUNIPER) || CONFIG(BOARD_GOOGLE_KAPPA) || : CONFIG(BOARD_GOOGLE_BURNET) || CONFIG(BOARD_GOOGLE_ESCHE) What do you think if we add a Kconfig option for this?
Hung-Te Lin has uploaded a new patch set (#9) to the change originally created by Kevin Chiu. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/boardid.c 2 files changed, 21 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/9
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/8/src/mainboard/google/kukui/... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/8/src/mainboard/google/kukui/... PS8, Line 179: CONFIG(BOARD_GOOGLE_JUNIPER) || CONFIG(BOARD_GOOGLE_KAPPA) || : CONFIG(BOARD_GOOGLE_BURNET) || CONFIG(BOARD_GOOGLE_ESCHE)
What do you think if we add a Kconfig option for this?
Done
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 9: Code-Review+2
Hung-Te Lin has uploaded a new patch set (#10) to the change originally created by Kevin Chiu. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/boardid.c 2 files changed, 21 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/10
Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#11).
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/boardid.c 2 files changed, 20 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/11
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/10/src/mainboard/google/kukui... File src/mainboard/google/kukui/Kconfig:
https://review.coreboot.org/c/coreboot/+/46442/10/src/mainboard/google/kukui... PS10, Line 82: default 0x1 if BOARD_GOOGLE_JUNIPER || BOARD_GOOGLE_KAPPA || BOARD_GOOGLE_DAMU || BOARD_GOOGLE_BURNET || BOARD_GOOGLE_ESCHE Sorry, but now this line seems to be too long. Can we wrap it?
Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#12).
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/boardid.c 2 files changed, 21 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/12
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/10/src/mainboard/google/kukui... File src/mainboard/google/kukui/Kconfig:
https://review.coreboot.org/c/coreboot/+/46442/10/src/mainboard/google/kukui... PS10, Line 82: default 0x1 if BOARD_GOOGLE_JUNIPER || BOARD_GOOGLE_KAPPA || BOARD_GOOGLE_DAMU || BOARD_GOOGLE_BURNET || BOARD_GOOGLE_ESCHE
Sorry, but now this line seems to be too long. […]
Done
Hung-Te Lin has uploaded a new patch set (#13) to the change originally created by Kevin Chiu. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/boardid.c 2 files changed, 22 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/13
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 13:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46442/10/src/mainboard/google/kukui... File src/mainboard/google/kukui/Kconfig:
https://review.coreboot.org/c/coreboot/+/46442/10/src/mainboard/google/kukui... PS10, Line 82: default 0x1 if BOARD_GOOGLE_JUNIPER || BOARD_GOOGLE_KAPPA || BOARD_GOOGLE_DAMU || BOARD_GOOGLE_BURNET || BOARD_GOOGLE_ESCHE
Sorry, but now this line seems to be too long. […]
Done
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... PS11, Line 186: / : ui I prefer to keep one blank line between this commend and sku_id, because this is NOT the function-comment for sku_id; it's explaining for functions below.
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... PS11, Line 186: / : ui
I prefer to keep one blank line between this commend and sku_id, because this is NOT the function-co […]
got it, let me add it back, thanks.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... PS11, Line 186: / : ui
got it, let me add it back, thanks.
no no no, let's move it to top of this file to prevent confusion. please check my latest version.
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/11/src/mainboard/google/kukui... PS11, Line 186: / : ui
no no no, let's move it to top of this file to prevent confusion. please check my latest version.
yes, just got your update there, thanks.
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 13:
Hi reviewers, could we merge this one if no other concern? thanks.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/13/src/mainboard/google/kukui... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/13/src/mainboard/google/kukui... PS13, Line 15: /* : * The boardid.c should provide board_id, sku_id, and ram_code. : * board_id is provided by ec/google/chromeec/ec_boardid.c. : * sku_id and ram_code are defined in this file. : */ Put this right after license?
/* SPDX-License-Identifier: GPL-2.0-only */ <<<BLANK LINE>>> /* * The boardid.c... */ <<<BLANK LINE>>> #include <...>
Hello Hung-Te Lin, build bot (Jenkins), Yu-Ping Wu,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46442
to look at the new patch set (#14).
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/boardid.c 2 files changed, 22 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/42/46442/14
Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 14:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46442/13/src/mainboard/google/kukui... File src/mainboard/google/kukui/boardid.c:
https://review.coreboot.org/c/coreboot/+/46442/13/src/mainboard/google/kukui... PS13, Line 15: /* : * The boardid.c should provide board_id, sku_id, and ram_code. : * board_id is provided by ec/google/chromeec/ec_boardid.c. : * sku_id and ram_code are defined in this file. : */
Put this right after license? […]
Done
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 14: Code-Review+2
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
Patch Set 14: Code-Review+2
Hung-Te Lin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46442 )
Change subject: mb/google/kukui: change Jacuzzi followers LCMID to fixed value ......................................................................
mb/google/kukui: change Jacuzzi followers LCMID to fixed value
The LCM ID is not really used on Jacuzzi followers and the reference design expects ADC to return 0. However, there were hardware design issues so the returned value became unexpected numbers.
- Juniper and Kappa returns 1. - Burnet and Esche returns 1 on normal boot, and 0 on recovery boot. - Cerise and Stern usually returns 0, and sometimes 1.
To fix that, we are changing LCM ID to fixed value for Jacuzzi followers.
BUG=b:170916885,b:171365301 BRANCH=kukui TEST=1. emerge-jacuzzi coreboot 2. check burnet/esche skuid correctly
Change-Id: I3b43b9153315ec65e9168c4e84ea844dff14d446 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46442 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Hung-Te Lin hungte@chromium.org Reviewed-by: Yu-Ping Wu yupingso@google.com --- M src/mainboard/google/kukui/Kconfig M src/mainboard/google/kukui/boardid.c 2 files changed, 22 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved Yu-Ping Wu: Looks good to me, approved
diff --git a/src/mainboard/google/kukui/Kconfig b/src/mainboard/google/kukui/Kconfig index 2237efa..23a4169 100644 --- a/src/mainboard/google/kukui/Kconfig +++ b/src/mainboard/google/kukui/Kconfig @@ -77,4 +77,10 @@ default 0x10 if BOARD_GOOGLE_BURNET || BOARD_GOOGLE_ESCHE || BOARD_GOOGLE_FENNEL || BOARD_GOOGLE_CERISE || BOARD_GOOGLE_STERN default 0x0
+config BOARD_OVERRIDE_LCM_ID + hex + default 0x1 if BOARD_GOOGLE_JUNIPER || BOARD_GOOGLE_KAPPA || BOARD_GOOGLE_DAMU + default 0x1 if BOARD_GOOGLE_BURNET || BOARD_GOOGLE_ESCHE + default 0x0 + endif diff --git a/src/mainboard/google/kukui/boardid.c b/src/mainboard/google/kukui/boardid.c index 47b0d9b..6c7547c 100644 --- a/src/mainboard/google/kukui/boardid.c +++ b/src/mainboard/google/kukui/boardid.c @@ -1,5 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+/* + * The boardid.c should provide board_id, sku_id, and ram_code. + * board_id is provided by ec/google/chromeec/ec_boardid.c. + * sku_id and ram_code are defined in this file. + */ + #include <assert.h> #include <boardid.h> #include <console/console.h> @@ -169,7 +175,15 @@ return 0; }
-/* board_id is provided by ec/google/chromeec/ec_boardid.c */ +/* Returns the ID for LCD module (type of panel). */ +static uint8_t lcm_id(void) +{ + /* LCM is unused on Jacuzzi followers. */ + if (CONFIG(BOARD_GOOGLE_JACUZZI_COMMON)) + return CONFIG_BOARD_OVERRIDE_LCM_ID; + + return get_adc_index(LCM_ID_CHANNEL); +}
uint32_t sku_id(void) { @@ -200,7 +214,7 @@ * ADC4[4bit/L] = SKU ID from board straps. */ cached_sku_id = (wfc_id() << 8 | - get_adc_index(LCM_ID_CHANNEL) << 4 | + lcm_id() << 4 | get_adc_index(SKU_ID_CHANNEL));
return cached_sku_id;
-
Chen-Tsung Hsieh (Code Review) -
Hung-Te Lin (Code Review)
-
Kevin Chiu (Code Review)
-
Yu-Ping Wu (Code Review)