Change in coreboot[master]: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Now that soc_get_pmc_mux_device() is gone, the PMC MUX connector devices can be hooked up together via devicetree aliases.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ib51764da5b3c029f9ac7ac60199a0aedfc7f29b1 --- M src/mainboard/google/volteer/variants/baseboard/devicetree.cb M src/mainboard/google/volteer/variants/delbin/overridetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/todor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/overridetree.cb M src/mainboard/google/volteer/variants/volteer2/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 9 files changed, 67 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/45878/1
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb index e32b2f9..102c2cc 100644 --- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb @@ -47,7 +47,7 @@ register "HeciEnabled" = "1"
# FSP configuration - register "SaGv" = "SaGv_Enabled" + register "SaGv" = "SaGv_Disabled" register "SmbusEnable" = "0"
register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)" # Type-A Port A0 diff --git a/src/mainboard/google/volteer/variants/delbin/overridetree.cb b/src/mainboard/google/volteer/variants/delbin/overridetree.cb index fc549c7..1b603f0 100644 --- a/src/mainboard/google/volteer/variants/delbin/overridetree.cb +++ b/src/mainboard/google/volteer/variants/delbin/overridetree.cb @@ -56,6 +56,13 @@ device i2c 15 on end end end # I2C5 0xA0C6 + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device pci 1f.2 hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -66,14 +73,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/terrador/overridetree.cb b/src/mainboard/google/volteer/variants/terrador/overridetree.cb index 42d3f2f..5fd136a 100644 --- a/src/mainboard/google/volteer/variants/terrador/overridetree.cb +++ b/src/mainboard/google/volteer/variants/terrador/overridetree.cb @@ -127,6 +127,13 @@ device i2c 15 on end end end # I2C5 0xA0C6 + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device pci 1f.2 hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -137,14 +144,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "3" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/todor/overridetree.cb b/src/mainboard/google/volteer/variants/todor/overridetree.cb index 42d3f2f..5fd136a 100644 --- a/src/mainboard/google/volteer/variants/todor/overridetree.cb +++ b/src/mainboard/google/volteer/variants/todor/overridetree.cb @@ -127,6 +127,13 @@ device i2c 15 on end end end # I2C5 0xA0C6 + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device pci 1f.2 hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -137,14 +144,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "3" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/volteer/overridetree.cb b/src/mainboard/google/volteer/variants/volteer/overridetree.cb index 5079493..9615985 100644 --- a/src/mainboard/google/volteer/variants/volteer/overridetree.cb +++ b/src/mainboard/google/volteer/variants/volteer/overridetree.cb @@ -208,6 +208,13 @@ end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -217,14 +224,14 @@ register "usb2_port_number" = "9" register "usb3_port_number" = "1" # SBU & HSL follow CC - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on + device generic 1 alias conn1 on probe DB_USB USB4_GEN2 probe DB_USB USB3_ACTIVE probe DB_USB USB4_GEN3 @@ -235,7 +242,7 @@ register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU & HSL follow CC - device generic 1 on + device generic 1 alias conn1 on probe DB_USB USB3_PASSIVE end end diff --git a/src/mainboard/google/volteer/variants/volteer2/overridetree.cb b/src/mainboard/google/volteer/variants/volteer2/overridetree.cb index 0bb82f1..bf382b7 100644 --- a/src/mainboard/google/volteer/variants/volteer2/overridetree.cb +++ b/src/mainboard/google/volteer/variants/volteer2/overridetree.cb @@ -7,6 +7,7 @@
device domain 0 on device pci 05.0 on end # IPU 0x9A19 + device pci 0d.2 on end # tbt_dma0 device pci 15.0 on chip drivers/i2c/generic register "hid" = ""10EC5682"" @@ -166,6 +167,13 @@ end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device pci 1f.2 hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -175,14 +183,14 @@ register "usb2_port_number" = "9" register "usb3_port_number" = "1" # SBU & HSL follow CC - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on + device generic 1 alias conn1 on probe DB_USB USB4_GEN2 probe DB_USB USB3_ACTIVE probe DB_USB USB4_GEN3 @@ -193,12 +201,12 @@ register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU & HSL follow CC - device generic 1 on + device generic 1 alias conn1 on probe DB_USB USB3_PASSIVE end end end end - end # PMC + end end end diff --git a/src/mainboard/google/volteer/variants/voxel/overridetree.cb b/src/mainboard/google/volteer/variants/voxel/overridetree.cb index 43296d5..4ec4849 100644 --- a/src/mainboard/google/volteer/variants/voxel/overridetree.cb +++ b/src/mainboard/google/volteer/variants/voxel/overridetree.cb @@ -120,6 +120,13 @@ device i2c 15 on end end end # I2C5 0xA0C6 + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device pci 1f.2 hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -130,14 +137,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb index 5965372..a7ed0bb 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb @@ -262,6 +262,8 @@ device pci 1e.3 off end # GSPI1 0xA0AB device pci 1f.0 on chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] device pnp 0c09.0 on end end end # eSPI 0xA080 - A09F @@ -276,14 +278,14 @@ register "usb3_port_number" = "3" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "7" register "usb3_port_number" = "4" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb index 90e7928..4b3859e 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb @@ -258,6 +258,8 @@ device pci 1e.3 off end # GSPI1 0xA0AB device pci 1f.0 on chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] device pnp 0c09.0 on end end end # eSPI 0xA080 - A09F @@ -272,14 +274,14 @@ register "usb3_port_number" = "3" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "5" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end
Tim Wawrzynczak has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Now that soc_get_pmc_mux_device() is gone, the PMC MUX connector devices can be hooked up together via devicetree aliases.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ib51764da5b3c029f9ac7ac60199a0aedfc7f29b1 --- M src/mainboard/google/volteer/variants/delbin/overridetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/todor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/overridetree.cb M src/mainboard/google/volteer/variants/volteer2/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 8 files changed, 66 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/45878/2
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... PS2, Line 245: conn1 How does it know which conn1 reference one to use? I am surprised this doesn't error out with the alias 'conn1' already existing, maybe because the find_alias() check in sconfig is only looking at base_root_dev?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... PS2, Line 245: conn1
How does it know which conn1 reference one to use? I am surprised this doesn't error out with the a […]
Haha yeah, I did notice that but forgot about it. I think it'll just use the "first" one. find_alias is recursive though, so I would have thought it could find it... definitely buggy, but I mean semantically whichever one is successfully probed...
I wonder if maybe this isn't a bad thing... b/c in the end, only one or zero of them will be probed. What do you think about adding a check in fw_config after the init() to handle this case, to assign the alias back in chromeec to whichever one was probed successfully? We need more code to handle this case either way.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... PS2, Line 245: conn1
Haha yeah, I did notice that but forgot about it. I think it'll just use the "first" one. […]
Agreed it does make the alias more useful if we can have it assigned to multiple devices as long as only 1 gets enabled after probing in the end.
maybe sconfig could have the alias as a list of devices, and then the runtime probing could print an error if more than one ends up enabled because it might not be assigned to the expected device.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... PS2, Line 245: conn1
Haha yeah, I did notice that but forgot about it. I think it'll just use the "first" one. […]
Confirmed, it just picked the first one. But what we really wanted here was the ability to set a register's value differently based on fw_config probing, which would require keeping around a list of the possible options and then some machinery to assign them...
Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45878
to look at the new patch set (#3).
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Now that soc_get_pmc_mux_device() is gone, the PMC MUX connector devices can be hooked up together via devicetree aliases.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ib51764da5b3c029f9ac7ac60199a0aedfc7f29b1 --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/baseboard/include/baseboard/variants.h M src/mainboard/google/volteer/variants/delbin/overridetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/todor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/Makefile.inc M src/mainboard/google/volteer/variants/volteer/overridetree.cb A src/mainboard/google/volteer/variants/volteer/variant.c M src/mainboard/google/volteer/variants/volteer2/Makefile.inc M src/mainboard/google/volteer/variants/volteer2/overridetree.cb A src/mainboard/google/volteer/variants/volteer2/variant.c M src/mainboard/google/volteer/variants/voxel/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 14 files changed, 123 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/45878/3
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... PS3, Line 20: variant_init do you need this indirection? wouldn't the search for port1 weed out the variants that don' t need it?
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45878
to look at the new patch set (#4).
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Now that soc_get_pmc_mux_device() is gone, the PMC MUX connector devices can be hooked up together via devicetree aliases.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ib51764da5b3c029f9ac7ac60199a0aedfc7f29b1 --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/delbin/overridetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/todor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/overridetree.cb M src/mainboard/google/volteer/variants/volteer2/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 9 files changed, 95 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/45878/4
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... PS3, Line 20: variant_init
do you need this indirection? wouldn't the search for port1 weed out the variants that don' t need i […]
Ah, I hadn't looked through all of the variants to see if this would break any of the others. Looks like not, I'll rearrange.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... PS3, Line 20: variant_init
Ah, I hadn't looked through all of the variants to see if this would break any of the others. […]
port1 is present on all variants, right? Are we relying on the fw_config_probe on all variants to filter out the devices that don't need the change in configuration?
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... PS4, Line 29: This is an ugly hack Yep :(
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... PS4, Line 32: dev_find_matching_device_on_bus(__pci_0_1f_2->link_list, Isn't this looking at the wrong child? pmc (__pci_0_1f_2) |_ pmc_mux (generic 0) |_ pmc_mux conn (generic 0) |_ pmc_mux conn (generic 1)
Wouldn't this look for a generic 1 device under pmc and never find any matching device?
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45878
to look at the new patch set (#5).
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Now that soc_get_pmc_mux_device() is gone, the PMC MUX connector devices can be hooked up together via devicetree aliases.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ib51764da5b3c029f9ac7ac60199a0aedfc7f29b1 --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/delbin/overridetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/todor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/overridetree.cb M src/mainboard/google/volteer/variants/volteer2/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 9 files changed, 112 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/45878/5
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... PS4, Line 29: This is an ugly hack
Yep :(
I do intend to come back and clean it up 🐨 How about `pcidev_path_on_root(PCH_DEVFN_PMC)` ?
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... PS4, Line 32: dev_find_matching_device_on_bus(__pci_0_1f_2->link_list,
Isn't this looking at the wrong child? […]
Yes, it certainly is, good catch, needs to traverse its grandchildren.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/3/src/mainboard/google/voltee... PS3, Line 20: variant_init
port1 is present on all variants, right? Are we relying on the fw_config_probe on all variants to fi […]
I think they all have a port 1, but as you said, fw_config will effectively filter out the devices that don't need the config change.
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... File src/mainboard/google/volteer/variants/volteer/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/45878/2/src/mainboard/google/voltee... PS2, Line 245: conn1
Confirmed, it just picked the first one. […]
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 6: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... PS6, Line 23: dev->path.generic.id == 1; You can also add an additional check to ensure that dev->chip_ops == drivers_intel_pmc_mux_conn_ops just in case we end up adding another generic non-PMC conn device.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 6:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... PS4, Line 29: This is an ugly hack
I do intend to come back and clean it up 🐨 How about `pcidev_path_on_root(PCH_DEVFN_PMC)` ?
Done
https://review.coreboot.org/c/coreboot/+/45878/4/src/mainboard/google/voltee... PS4, Line 32: dev_find_matching_device_on_bus(__pci_0_1f_2->link_list,
Yes, it certainly is, good catch, needs to traverse its grandchildren.
Done
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... PS6, Line 23: dev->path.generic.id == 1;
You can also add an additional check to ensure that dev->chip_ops == drivers_intel_pmc_mux_conn_ops […]
Sure could, but those checks are so, how shall we say, inelegant 😄? There's got to be a better way to establish device "identity" than a `void *`... or maybe a new bus type "virtual" or similar
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... PS6, Line 23: dev->path.generic.id == 1;
Sure could, but those checks are so, how shall we say, inelegant 😄?
Agreed. Ideally we should be solving this at the sconfig/devicetree level rather than having to check at runtime.
There's got to be a better way to establish device "identity" than a `void *`...
Currently, the only way to ensure that you are inspecting the right structure is looking at the device->chip_ops. That gives us some confidence that future changes to the hierarchy won't silently change bits in an unrelated structure.
or maybe a new bus type "virtual" or similar
That would run into the same problem. In this case, you can restrict to using virtual only for pmc_mux, but we might run into similar problem with other device/buses as well.
Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Duncan Laurie,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45878
to look at the new patch set (#7).
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Now that soc_get_pmc_mux_device() is gone, the PMC MUX connector devices can be hooked up together via devicetree aliases.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ib51764da5b3c029f9ac7ac60199a0aedfc7f29b1 --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/delbin/overridetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/todor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/overridetree.cb M src/mainboard/google/volteer/variants/volteer2/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 9 files changed, 114 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/45878/7
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/45878/6/src/mainboard/google/voltee... PS6, Line 23: dev->path.generic.id == 1;
Sure could, but those checks are so, how shall we say, inelegant 😄?
Agreed. Ideally we should be solving this at the sconfig/devicetree level rather than having to check at runtime.
100% agree
There's got to be a better way to establish device "identity" than a `void *`...
Currently, the only way to ensure that you are inspecting the right structure is looking at the device->chip_ops. That gives us some confidence that future changes to the hierarchy won't silently change bits in an unrelated structure.
That's fair, it could be tricky to catch later, I'll add the ugly code 😋
or maybe a new bus type "virtual" or similar
That would run into the same problem. In this case, you can restrict to using virtual only for pmc_mux, but we might run into similar problem with other device/buses as well.
Yeah, that was a strawman, I guess I mean some way in devtree to add an "identity" and/or "class of device"
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
Patch Set 7: Code-Review+2
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45878 )
Change subject: tigerlake mainboards: switch to devtree aliases for PMC MUX connectors ......................................................................
tigerlake mainboards: switch to devtree aliases for PMC MUX connectors
Now that soc_get_pmc_mux_device() is gone, the PMC MUX connector devices can be hooked up together via devicetree aliases.
Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Change-Id: Ib51764da5b3c029f9ac7ac60199a0aedfc7f29b1 Reviewed-on: https://review.coreboot.org/c/coreboot/+/45878 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/volteer/mainboard.c M src/mainboard/google/volteer/variants/delbin/overridetree.cb M src/mainboard/google/volteer/variants/terrador/overridetree.cb M src/mainboard/google/volteer/variants/todor/overridetree.cb M src/mainboard/google/volteer/variants/volteer/overridetree.cb M src/mainboard/google/volteer/variants/volteer2/overridetree.cb M src/mainboard/google/volteer/variants/voxel/overridetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb M src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb 9 files changed, 114 insertions(+), 47 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c index 03a78fd..016572a 100644 --- a/src/mainboard/google/volteer/mainboard.c +++ b/src/mainboard/google/volteer/mainboard.c @@ -9,14 +9,65 @@ #include <fw_config.h> #include <security/tpm/tss.h> #include <soc/gpio.h> +#include <soc/pci_devs.h> #include <soc/ramstage.h> #include <vendorcode/google/chromeos/chromeos.h> #include <variant/gpio.h> #include <vb2_api.h>
+#include "drivers/intel/pmc_mux/conn/chip.h" + +extern struct chip_operations drivers_intel_pmc_mux_conn_ops; + +static bool is_port1(struct device *dev) +{ + return dev->path.type == DEVICE_PATH_GENERIC && dev->path.generic.id == 1 && + dev->chip_ops == &drivers_intel_pmc_mux_conn_ops; +} + +static void typec_orientation_fixup(void) +{ + /* + * TODO: This is an ugly hack, see if there's a better way to accomplish this same thing + * via fw_config + devicetree, i.e., change a register's value depending on fw_config + * probing. + */ + const struct device *pmc; + const struct device *mux; + const struct device *conn; + + pmc = pcidev_path_on_root(PCH_DEVFN_PMC); + if (!pmc || !pmc->link_list->children) { + printk(BIOS_ERR, "%s: unable to find PMC device or its mux\n", __func__); + return; + } + + /* + * Find port 1 underneath PMC.MUX; some variants may not have this defined, so it's okay + * to just silently return here. + */ + mux = pmc->link_list->children; + conn = dev_find_matching_device_on_bus(mux->link_list, is_port1); + if (!conn) + return; + + if (fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN2)) || + fw_config_probe(FW_CONFIG(DB_USB, USB3_ACTIVE)) || + fw_config_probe(FW_CONFIG(DB_USB, USB4_GEN3)) || + fw_config_probe(FW_CONFIG(DB_USB, USB3_NO_A))) { + struct drivers_intel_pmc_mux_conn_config *config = conn->chip_info; + + if (config) { + printk(BIOS_INFO, "Configure Right Type-C port orientation for retimer\n"); + config->sbu_orientation = TYPEC_ORIENTATION_NORMAL; + } + } +} + static void mainboard_init(struct device *dev) { mainboard_ec_init(); + typec_orientation_fixup(); }
static void add_fw_config_oem_string(const struct fw_config *config, void *arg) diff --git a/src/mainboard/google/volteer/variants/delbin/overridetree.cb b/src/mainboard/google/volteer/variants/delbin/overridetree.cb index 5ecfccf..ba02d8c 100644 --- a/src/mainboard/google/volteer/variants/delbin/overridetree.cb +++ b/src/mainboard/google/volteer/variants/delbin/overridetree.cb @@ -70,6 +70,13 @@ device i2c 15 on end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -80,14 +87,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/terrador/overridetree.cb b/src/mainboard/google/volteer/variants/terrador/overridetree.cb index ae26e79..d62a303 100644 --- a/src/mainboard/google/volteer/variants/terrador/overridetree.cb +++ b/src/mainboard/google/volteer/variants/terrador/overridetree.cb @@ -172,6 +172,13 @@ device i2c 15 on end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -182,14 +189,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "3" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/todor/overridetree.cb b/src/mainboard/google/volteer/variants/todor/overridetree.cb index ae26e79..d62a303 100644 --- a/src/mainboard/google/volteer/variants/todor/overridetree.cb +++ b/src/mainboard/google/volteer/variants/todor/overridetree.cb @@ -172,6 +172,13 @@ device i2c 15 on end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -182,14 +189,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "3" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/volteer/overridetree.cb b/src/mainboard/google/volteer/variants/volteer/overridetree.cb index c5b4c72..a047e87 100644 --- a/src/mainboard/google/volteer/variants/volteer/overridetree.cb +++ b/src/mainboard/google/volteer/variants/volteer/overridetree.cb @@ -221,6 +221,13 @@ end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -230,27 +237,12 @@ register "usb2_port_number" = "9" register "usb3_port_number" = "1" # SBU & HSL follow CC - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" - # SBU is fixed, HSL follows CC - register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on - probe DB_USB USB4_GEN2 - probe DB_USB USB3_ACTIVE - probe DB_USB USB4_GEN3 - probe DB_USB USB3_NO_A - end - end - chip drivers/intel/pmc_mux/conn - register "usb2_port_number" = "4" - register "usb3_port_number" = "2" - # SBU & HSL follow CC - device generic 1 on - probe DB_USB USB3_PASSIVE - end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/volteer2/overridetree.cb b/src/mainboard/google/volteer/variants/volteer2/overridetree.cb index 3f666ac..a36a844 100644 --- a/src/mainboard/google/volteer/variants/volteer2/overridetree.cb +++ b/src/mainboard/google/volteer/variants/volteer2/overridetree.cb @@ -93,7 +93,7 @@ TEMP_PCT(42, 36),}}}" device generic 0 on end end - end # DPTF 0x9A03 + end device ref ipu on end # IPU 0x9A19 device ref i2c0 on chip drivers/i2c/generic @@ -254,6 +254,13 @@ end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -263,27 +270,12 @@ register "usb2_port_number" = "9" register "usb3_port_number" = "1" # SBU & HSL follow CC - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" - # SBU is fixed, HSL follows CC - register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on - probe DB_USB USB4_GEN2 - probe DB_USB USB3_ACTIVE - probe DB_USB USB4_GEN3 - probe DB_USB USB3_NO_A - end - end - chip drivers/intel/pmc_mux/conn - register "usb2_port_number" = "4" - register "usb3_port_number" = "2" - # SBU & HSL follow CC - device generic 1 on - probe DB_USB USB3_PASSIVE - end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/google/volteer/variants/voxel/overridetree.cb b/src/mainboard/google/volteer/variants/voxel/overridetree.cb index e8be8e3..184007e 100644 --- a/src/mainboard/google/volteer/variants/voxel/overridetree.cb +++ b/src/mainboard/google/volteer/variants/voxel/overridetree.cb @@ -187,6 +187,13 @@ device i2c 15 on end end end + device ref pch_espi on + chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] + device pnp 0c09.0 on end + end + end device ref pmc hidden # The pmc_mux chip driver is a placeholder for the # PMC.MUX device in the ACPI hierarchy. @@ -197,14 +204,14 @@ register "usb3_port_number" = "1" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "4" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb index de93c99..e16bd1f 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up3/devicetree.cb @@ -306,6 +306,8 @@ end # GSPI1 0xA0AB device pci 1f.0 on chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] device pnp 0c09.0 on end end end # eSPI 0xA080 - A09F @@ -320,14 +322,14 @@ register "usb3_port_number" = "3" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "7" register "usb3_port_number" = "4" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end diff --git a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb index 4078894..e6c0585 100644 --- a/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb +++ b/src/mainboard/intel/tglrvp/variants/tglrvp_up4/devicetree.cb @@ -310,6 +310,8 @@ end # GSPI1 0xA0AB device pci 1f.0 on chip ec/google/chromeec + use conn0 as mux_conn[0] + use conn1 as mux_conn[1] device pnp 0c09.0 on end end end # eSPI 0xA080 - A09F @@ -324,14 +326,14 @@ register "usb3_port_number" = "3" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 0 on end + device generic 0 alias conn0 on end end chip drivers/intel/pmc_mux/conn register "usb2_port_number" = "5" register "usb3_port_number" = "2" # SBU is fixed, HSL follows CC register "sbu_orientation" = "TYPEC_ORIENTATION_NORMAL" - device generic 1 on end + device generic 1 alias conn1 on end end end end
-
Duncan Laurie (Code Review) -
Furquan Shaikh (Code Review)
-
Tim Wawrzynczak (Code Review)