Nick Vaccaro has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
mb/google/volteer: update volteer cbi definition
Update volteer's cbi definition in devicetree.cb to match the current definition for volteer.
BUG=none TEST=emerge-volteer coreboot chromeos-bootimage, flash and boot volteer to kernel.
Change-Id: I761893818231880d86fd13cfa61319157d06a7d5 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 1 file changed, 5 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/42331/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index a55ef7d..ac2a22a 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -1,8 +1,10 @@ fw_config field USB_DB 0 3 - option NONE 0 - option USB4 1 - option USB3 2 + option DB_NONE 0 + option DB_USB4_GEN2 1 + option DB_USB3 2 + option DB_USB4_GEN3 3 + option DB_USB3_PASSIVE 4 end field THERMAL 4 7 end field AUDIO 8 10
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE malefor uses a different encoding for this (it uses DB #1). is it too late to have malefor use the same assignment? otherwise, we'll have to update the override trees as well.
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE
malefor uses a different encoding for this (it uses DB #1). […]
Where do you see malefor using that?
William Wei has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE
Where do you see malefor using that?
Malefor define DB#1 during /project/volteer/malefor/config.star, but it seems not match baseboard EC code, I'll submit another CL to change it to 2. Please notice that this CL add DB_USB3_PASSIVE which EC code not defined, see https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master...
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE
Malefor define DB#1 during /project/volteer/malefor/config. […]
@willam, thanks - keeping these assignments consistent between baseboard and variants to the extent possible makes life easier.
DB #4 is already in config.star, but it's labelled USB3-LOW_COST there. i'm pretty sure it's supposed to be the same thing - we'll have to get the naming straightened out. full support is still a work-in-progress.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE
@willam, thanks - keeping these assignments consistent between baseboard and variants […]
These field options should really be defined in program/volteer/program.star in order to go in baseboard/devicetree.cb and if it is only defined in the project/volteer/config.star then we should be defining the options in overridetree.cb. (but I don't think that is what we want and that isn't going to work for the EC anyway)
William Wei has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE
These field options should really be defined in program/volteer/program. […]
DB fw_config id's definition moved from project/volteer/config.star to program/volteer/program.star and add DB#5 just now, maybe we should sync and add DB id 5 here.
Keith Short has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE
DB fw_config id's definition moved from project/volteer/config.star to program/volteer/program. […]
CL:*3119423 has the update to volteer/program.star and has merged.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update volteer cbi definition ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/2/src/mainboard/google/voltee... PS2, Line 3: DB_NONE do you want to rename these to match program.star?
Hello William Wei, build bot (Jenkins), Furquan Shaikh, Caveh Jalali, Duncan Laurie, Jes Klinke, Dossym Nurmukhanov,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42331
to look at the new patch set (#3).
Change subject: mb/google/volteer: update fw_config definition ......................................................................
mb/google/volteer: update fw_config definition
Update fw_config definition in devicetree.cb to match current definition for volteer.
BUG=b:159157584 TEST=none
Change-Id: I761893818231880d86fd13cfa61319157d06a7d5 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 1 file changed, 17 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/42331/3
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update fw_config definition ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/1/src/mainboard/google/voltee... PS1, Line 7: DB_USB3_PASSIVE
CL:*3119423 has the update to volteer/program.star and has merged.
Updated this CL to match changes to volteer/program.star.
https://review.coreboot.org/c/coreboot/+/42331/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/2/src/mainboard/google/voltee... PS2, Line 3: DB_NONE
do you want to rename these to match program. […]
Done
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update fw_config definition ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/3/src/mainboard/google/voltee... PS3, Line 3: DB_USB i think we don't need the prefixes in options. looking at sconfig, we always compose the symbol names with the field name and option name concatenated, so this is kinda redundant.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update fw_config definition ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/3/src/mainboard/google/voltee... PS3, Line 3: DB_USB
i think we don't need the prefixes in options. […]
Ya in the starlark config the variable names with prefix are useful, but it is true that in coreboot it is always paired with the field name so we could leave it out. (like audio is now)
Hello build bot (Jenkins), William Wei, Furquan Shaikh, Caveh Jalali, Duncan Laurie, Jes Klinke, Dossym Nurmukhanov,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42331
to look at the new patch set (#4).
Change subject: mb/google/volteer: update fw_config definition ......................................................................
mb/google/volteer: update fw_config definition
Update fw_config definition in devicetree.cb to match current definition for volteer.
BUG=b:159157584 TEST=none
Change-Id: I761893818231880d86fd13cfa61319157d06a7d5 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 1 file changed, 17 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/42331/4
Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update fw_config definition ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42331/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/42331/3/src/mainboard/google/voltee... PS3, Line 3: DB_USB
Ya in the starlark config the variable names with prefix are useful, but it is true that in coreboot […]
Done
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update fw_config definition ......................................................................
Patch Set 5: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42331 )
Change subject: mb/google/volteer: update fw_config definition ......................................................................
mb/google/volteer: update fw_config definition
Update fw_config definition in devicetree.cb to match current definition for volteer.
BUG=b:159157584 TEST=none
Change-Id: I761893818231880d86fd13cfa61319157d06a7d5 Signed-off-by: Nick Vaccaro nvaccaro@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42331 Reviewed-by: Caveh Jalali caveh@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb 1 file changed, 17 insertions(+), 9 deletions(-)
Approvals: build bot (Jenkins): Verified Caveh Jalali: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index 9732aa8..9d044b9 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -1,8 +1,11 @@ fw_config - field USB_DB 0 3 - option NONE 0 - option USB4 1 - option USB3 2 + field DB_USB 0 3 + option USB_ABSENT 0 + option USB4_GEN2 1 + option USB3_ACTIVE 2 + option USB4_GEN3 3 + option USB3_PASSIVE 4 + option USB3_NO_A 5 end field THERMAL 4 7 end field AUDIO 8 10 @@ -12,12 +15,17 @@ option MAX98373_ALC5682_SNDW 3 end field TABLETMODE 11 - option DISABLED 0 - option ENABLED 1 + option TABLETMODE_DISABLED 0 + option TABLETMODE_ENABLED 1 end - field LTE_DB 12 - option ABSENT 0 - option PRESENT 1 + field DB_LTE 12 13 + option LTE_ABSENT 0 + option LTE_PRESENT 1 + end + field DB_SD 16 19 + option SD_ABSENT 0 + option SD_GL9755S 1 + option SD_RTS5261 2 end end
-
Caveh Jalali (Code Review) -
Duncan Laurie (Code Review)
-
Keith Short (Code Review)
-
Nick Vaccaro (Code Review)
-
Patrick Georgi (Code Review)
-
William Wei (Code Review)