Change in coreboot[master]: mb/google/dedede: Enable ESPI configure IO decode range for chrome EC
Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: Enable ESPI configure IO decode range for chrome EC ......................................................................
mb/google/dedede: Enable ESPI configure IO decode range for chrome EC
Configure below ESPI IO decode ranges:
1. 0x200-020F: EC host command range. 2. 0x800-0x8FF: EC host command args and params. 3. 0x900-0x9ff: EC memory map range.
Change-Id: I1e450d6e45242180de715746b9852634de2669c6 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/39121/1
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index c17620b..03fea79 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -19,6 +19,12 @@ register "pmc_gpe0_dw1" = "GPP_D" register "pmc_gpe0_dw2" = "GPP_H"
+ # EC host command ranges are in 0x800-0x8ff & 0x200-0x20f + register "gen1_dec" = "0x00fc0801" + register "gen2_dec" = "0x000c0201" + # EC memory map range is 0x900-0x9ff + register "gen3_dec" = "0x00fc0901" + # USB Port Configuration register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C0 register "usb2_ports[1]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C1
Aamir Bohra has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
mb/google/dedede: configure ESPI IO decode range for chrome EC
Configure below ESPI IO decode ranges:
1. 0x200-020F: EC host command range. 2. 0x800-0x8FF: EC host command args and params. 3. 0x900-0x9ff: EC memory map range.
Change-Id: I1e450d6e45242180de715746b9852634de2669c6 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb 1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/39121/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
LGTM, but just a thought, do we want to move the LPC generic IO decode ranges to our EC driver? Then we don't have to keep adding it to every mainboard's devicetree.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... PS2, Line 23: f Can you please confirm what 0xfc mask indicates? Is it byte accessible or block accessible(256 bytes)?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... PS2, Line 23: f
Can you please confirm what 0xfc mask indicates? Is it byte accessible or block accessible(256 bytes […]
It's a mask on the address applied (and is 0x3F in this case), meaning decode all 6 available address bits.
From the EDS: "Note that PCH does not provide decode down to the word or byte level."
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
(1 comment)
Patch Set 2:
LGTM, but just a thought, do we want to move the LPC generic IO decode ranges to our EC driver? Then we don't have to keep adding it to every mainboard's devicetree.
This I/O decode range configuration seems to be Intel SoC specific - not the range itself, but the register value. So moving to the EC driver may be tricky and might require SoC knowledge in there.
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... PS2, Line 23: f
It's a mask on the address applied (and is 0x3F in this case), meaning decode all 6 available addres […]
Discussed offline with Tim and we found that 0xfc translates to 0x3f number of words as accessible in that region.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
(1 comment)
Patch Set 2:
(1 comment)
Patch Set 2:
LGTM, but just a thought, do we want to move the LPC generic IO decode ranges to our EC driver? Then we don't have to keep adding it to every mainboard's devicetree.
This I/O decode range configuration seems to be Intel SoC specific - not the range itself, but the register value. So moving to the EC driver may be tricky and might require SoC knowledge in there.
Actually, these are the I/O windows that Chrome EC uses -- 0x800, 0x900, 0x200. The way they get mapped to LPC I/O regs is specific to Intel, but I think we can do some work to avoid having to duplicate this across mainboards. I believe this was done differently on APL/GLK, but it still required mainboard to provide a function to do the work.
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... File src/mainboard/google/dedede/variants/baseboard/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/39121/2/src/mainboard/google/dedede... PS2, Line 23: f
Discussed offline with Tim and we found that 0xfc translates to 0x3f number of words as accessible i […]
dwords*
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
Patch Set 2:
(1 comment)
Patch Set 2:
(1 comment)
Patch Set 2:
LGTM, but just a thought, do we want to move the LPC generic IO decode ranges to our EC driver? Then we don't have to keep adding it to every mainboard's devicetree.
This I/O decode range configuration seems to be Intel SoC specific - not the range itself, but the register value. So moving to the EC driver may be tricky and might require SoC knowledge in there.
Actually, these are the I/O windows that Chrome EC uses -- 0x800, 0x900, 0x200. The way they get mapped to LPC I/O regs is specific to Intel, but I think we can do some work to avoid having to duplicate this across mainboards. I believe this was done differently on APL/GLK, but it still required mainboard to provide a function to do the work.
A possible approach is to let the EC driver return the I/O range along with the size info (in dwords or bytes). SoCs use the range information to map to the LPC I/O range registers. That way we can avoid duplicating the configuration in the mainboard.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
(1 comment)
Patch Set 2:
(1 comment)
Patch Set 2:
LGTM, but just a thought, do we want to move the LPC generic IO decode ranges to our EC driver? Then we don't have to keep adding it to every mainboard's devicetree.
This I/O decode range configuration seems to be Intel SoC specific - not the range itself, but the register value. So moving to the EC driver may be tricky and might require SoC knowledge in there.
Actually, these are the I/O windows that Chrome EC uses -- 0x800, 0x900, 0x200. The way they get mapped to LPC I/O regs is specific to Intel, but I think we can do some work to avoid having to duplicate this across mainboards. I believe this was done differently on APL/GLK, but it still required mainboard to provide a function to do the work.
A possible approach is to let the EC driver return the I/O range along with the size info (in dwords or bytes). SoCs use the range information to map to the LPC I/O range registers. That way we can avoid duplicating the configuration in the mainboard.
For now this can go in, while we look into all the mainboards to do clean-up the redundancy. I will take that as an action item on me.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
(1 comment)
Patch Set 2:
(1 comment)
Patch Set 2:
LGTM, but just a thought, do we want to move the LPC generic IO decode ranges to our EC driver? Then we don't have to keep adding it to every mainboard's devicetree.
This I/O decode range configuration seems to be Intel SoC specific - not the range itself, but the register value. So moving to the EC driver may be tricky and might require SoC knowledge in there.
Actually, these are the I/O windows that Chrome EC uses -- 0x800, 0x900, 0x200. The way they get mapped to LPC I/O regs is specific to Intel, but I think we can do some work to avoid having to duplicate this across mainboards. I believe this was done differently on APL/GLK, but it still required mainboard to provide a function to do the work.
A possible approach is to let the EC driver return the I/O range along with the size info (in dwords or bytes). SoCs use the range information to map to the LPC I/O range registers. That way we can avoid duplicating the configuration in the mainboard.
For now this can go in, while we look into all the mainboards to do clean-up the redundancy. I will take that as an action item on me.
Sure, we can go ahead with this for now. When you start exploring do look at google_chromeec_ioport_range(). It provides the range as you mentioned.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2: Code-Review+2
V Sowmya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 2: Code-Review+2
Patch Set 2:
(1 comment)
Patch Set 2:
(1 comment)
Patch Set 2:
LGTM, but just a thought, do we want to move the LPC generic IO decode ranges to our EC driver? Then we don't have to keep adding it to every mainboard's devicetree.
This I/O decode range configuration seems to be Intel SoC specific - not the range itself, but the register value. So moving to the EC driver may be tricky and might require SoC knowledge in there.
Actually, these are the I/O windows that Chrome EC uses -- 0x800, 0x900, 0x200. The way they get mapped to LPC I/O regs is specific to Intel, but I think we can do some work to avoid having to duplicate this across mainboards. I believe this was done differently on APL/GLK, but it still required mainboard to provide a function to do the work.
Sure, but to my knowledge, LPC/eSPI is x86-specific anyway, ARM uses i2c or spi. So, the ec_lpc enable() function should be able to program the ranges that it uses. I would think Wilco would be the only outlier here, besides maybe some ancient boards (I see butterly, parrot stout have different ranges?)
Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
mb/google/dedede: configure ESPI IO decode range for chrome EC
Configure below ESPI IO decode ranges:
1. 0x200-020F: EC host command range. 2. 0x800-0x8FF: EC host command args and params. 3. 0x900-0x9ff: EC memory map range.
Change-Id: I1e450d6e45242180de715746b9852634de2669c6 Signed-off-by: Aamir Bohra aamir.bohra@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39121 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Karthik Ramasubramanian kramasub@google.com Reviewed-by: V Sowmya v.sowmya@intel.com Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/dedede/variants/baseboard/devicetree.cb 1 file changed, 6 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved V Sowmya: Looks good to me, approved Karthik Ramasubramanian: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb index 2529e53..d5f58ba 100644 --- a/src/mainboard/google/dedede/variants/baseboard/devicetree.cb +++ b/src/mainboard/google/dedede/variants/baseboard/devicetree.cb @@ -20,6 +20,12 @@ register "pmc_gpe0_dw1" = "GPP_C" register "pmc_gpe0_dw2" = "GPP_D"
+ # EC host command ranges are in 0x800-0x8ff & 0x200-0x20f + register "gen1_dec" = "0x00fc0801" + register "gen2_dec" = "0x000c0201" + # EC memory map range is 0x900-0x9ff + register "gen3_dec" = "0x00fc0901" + # USB Port Configuration register "usb2_ports[0]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C0 register "usb2_ports[1]" = "USB2_PORT_MID(OC_SKIP)" # Type-C Port C1
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39121 )
Change subject: mb/google/dedede: configure ESPI IO decode range for chrome EC ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3 Emulation targets: EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/923 EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/922 EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/921
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aamir Bohra (Code Review)
-
Furquan Shaikh (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)
-
V Sowmya (Code Review)