EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
soc/intel/alderlake: Align chipset.cb with pci_devs.h
Refer pci_devs.h naming to align chipset.cb. Correct thc0, thc1 and add cnvi_bt.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iac33983dc12ed4e5b9257c50d82adc8e4e728ad6 --- M src/soc/intel/alderlake/chipset.cb 1 file changed, 9 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/48153/1
diff --git a/src/soc/intel/alderlake/chipset.cb b/src/soc/intel/alderlake/chipset.cb index ff81560..de880e3 100644 --- a/src/soc/intel/alderlake/chipset.cb +++ b/src/soc/intel/alderlake/chipset.cb @@ -13,18 +13,19 @@ device pci 08.0 alias gna off end device pci 09.0 alias north_tracehub off end device pci 0a.0 alias crashlog off end - device pci 0d.0 alias north_xhci off end - device pci 0d.1 alias north_xdci off end - device pci 0d.2 alias tbt_dma0 off end - device pci 0d.3 alias tbt_dma1 off end + device pci 0d.0 alias tcss_xhci off end + device pci 0d.1 alias tcss_xdci off end + device pci 0d.2 alias tcss_dma0 off end + device pci 0d.3 alias tcss_dma1 off end device pci 0e.0 alias vmd off end - device pci 10.6 alias thc0 off end - device pci 10.7 alias thc1 off end + device pci 10.0 alias thc0 off end + device pci 10.1 alias thc1 off end + device pci 10.2 alias cnvi_bt off end device pci 12.0 alias ish off end device pci 12.6 alias gspi2 off end device pci 13.0 alias gspi3 off end - device pci 14.0 alias south_xhci off end - device pci 14.1 alias south_xdci off end + device pci 14.0 alias xhci off end + device pci 14.1 alias usb_otg off end device pci 14.2 alias shared_sram off end device pci 14.3 alias cnvi_wifi off end device pci 15.0 alias i2c0 off end
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... PS2, Line 16: tcss_xhci off end : device pci 0d.1 alias tcss_xdci off end : device pci 0d.2 alias tcss_dma0 off end : device pci 0d.3 alias tcss_dma1 off end I was using the convention from soc/intel/tigerlake/chipset.cb, WDYT?
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... PS2, Line 22: device pci 10.1 alias thc1 off end : device pci 10.2 alias cnvi_bt off end I thought it seemed weird, but the EDS version I have (#630094, august 2020, rev 0.7) says functions 6 and 7 ... which would be another PCI bus violation, w/o a function 0. What version of the EDS do you have?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... PS2, Line 16: tcss_xhci off end : device pci 0d.1 alias tcss_xdci off end : device pci 0d.2 alias tcss_dma0 off end : device pci 0d.3 alias tcss_dma1 off end
I was using the convention from soc/intel/tigerlake/chipset. […]
coreboot/src/soc/intel/alderlake/include/soc/pci_devs.h, I follow this header file. I think this naming can easier understand..north and south are pretty confusing.
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... PS2, Line 22: device pci 10.1 alias thc1 off end : device pci 10.2 alias cnvi_bt off end
I thought it seemed weird, but the EDS version I have (#630094, august 2020, rev 0. […]
coreboot/src/soc/intel/alderlake/include/soc/pci_devs.h, I follow this header file.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 3:
@Subrata, can you give some comment for the EDS and the pci_devs.h are not identical?
/* PCH Devices */ #define PCH_DEV_SLOT_SIO0 0x10 #define PCH_DEVFN_THC0 _PCH_DEVFN(SIO0, 0) #define PCH_DEVFN_THC1 _PCH_DEVFN(SIO0, 1) #define PCH_DEVFN_CNVI_BT _PCH_DEVFN(SIO0, 2)
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 3:
Patch Set 3:
@Subrata, can you give some comment for the EDS and the pci_devs.h are not identical?
/* PCH Devices */ #define PCH_DEV_SLOT_SIO0 0x10 #define PCH_DEVFN_THC0 _PCH_DEVFN(SIO0, 0) #define PCH_DEVFN_THC1 _PCH_DEVFN(SIO0, 1) #define PCH_DEVFN_CNVI_BT _PCH_DEVFN(SIO0, 2)
This is documentation issue, on ADL-P, we have THC0 as 0:0x10:0 where else on ADL-S its on 0:0x10:6 because I2C6 is on 0:0x10:0 there
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
@Subrata, can you give some comment for the EDS and the pci_devs.h are not identical?
/* PCH Devices */ #define PCH_DEV_SLOT_SIO0 0x10 #define PCH_DEVFN_THC0 _PCH_DEVFN(SIO0, 0) #define PCH_DEVFN_THC1 _PCH_DEVFN(SIO0, 1) #define PCH_DEVFN_CNVI_BT _PCH_DEVFN(SIO0, 2)
This is documentation issue, on ADL-P, we have THC0 as 0:0x10:0 where else on ADL-S its on 0:0x10:6 because I2C6 is on 0:0x10:0 there
So do we need to separate ADL-P and ADL-S in soc code? I think brya is use the ADL-P, will ADL-S be used for chrome or other coreboot project?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 3:
Patch Set 3:
@Subrata, can you give some comment for the EDS and the pci_devs.h are not identical?
/* PCH Devices */ #define PCH_DEV_SLOT_SIO0 0x10 #define PCH_DEVFN_THC0 _PCH_DEVFN(SIO0, 0) #define PCH_DEVFN_THC1 _PCH_DEVFN(SIO0, 1) #define PCH_DEVFN_CNVI_BT _PCH_DEVFN(SIO0, 2)
This is documentation issue, on ADL-P, we have THC0 as 0:0x10:0 where else on ADL-S its on 0:0x10:6 because I2C6 is on 0:0x10:0 there
So do we need to separate ADL-P and ADL-S in soc code? I think brya is use the ADL-P, will ADL-S be used for chrome or other coreboot project?
you can ignore ADL-S for now and stay with ADL-P as is
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 4:
Patch Set 3:
Patch Set 3:
Patch Set 3:
Patch Set 3:
@Subrata, can you give some comment for the EDS and the pci_devs.h are not identical?
/* PCH Devices */ #define PCH_DEV_SLOT_SIO0 0x10 #define PCH_DEVFN_THC0 _PCH_DEVFN(SIO0, 0) #define PCH_DEVFN_THC1 _PCH_DEVFN(SIO0, 1) #define PCH_DEVFN_CNVI_BT _PCH_DEVFN(SIO0, 2)
This is documentation issue, on ADL-P, we have THC0 as 0:0x10:0 where else on ADL-S its on 0:0x10:6 because I2C6 is on 0:0x10:0 there
So do we need to separate ADL-P and ADL-S in soc code? I think brya is use the ADL-P, will ADL-S be used for chrome or other coreboot project?
you can ignore ADL-S for now and stay with ADL-P as is
Thanks for clarifying Subrata!
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... PS2, Line 16: tcss_xhci off end : device pci 0d.1 alias tcss_xdci off end : device pci 0d.2 alias tcss_dma0 off end : device pci 0d.3 alias tcss_dma1 off end
coreboot/src/soc/intel/alderlake/include/soc/pci_devs.h, I follow this header file. […]
That's fine. FYI, it's from the old southbridge (slow I/O) / northbridge (fast I/O) distinction. Sometimes the core+uncore is still called the north complex, and the PCH is the south complex.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... PS2, Line 16: tcss_xhci off end : device pci 0d.1 alias tcss_xdci off end : device pci 0d.2 alias tcss_dma0 off end : device pci 0d.3 alias tcss_dma1 off end
That's fine. FYI, it's from the old southbridge (slow I/O) / northbridge (fast I/O) distinction. […]
I knew this. The old school things lol. You can see fsp registers use the tscc_xhci..etc. This can help people to match fsp registers setting and the pci device in a clear way.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... File src/soc/intel/alderlake/chipset.cb:
https://review.coreboot.org/c/coreboot/+/48153/2/src/soc/intel/alderlake/chi... PS2, Line 16: tcss_xhci off end : device pci 0d.1 alias tcss_xdci off end : device pci 0d.2 alias tcss_dma0 off end : device pci 0d.3 alias tcss_dma1 off end
I knew this. The old school things lol. You can see fsp registers use the tscc_xhci..etc. […]
sorry I'm sure you do 😊
Tim Wawrzynczak has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48153 )
Change subject: soc/intel/alderlake: Align chipset.cb with pci_devs.h ......................................................................
soc/intel/alderlake: Align chipset.cb with pci_devs.h
Refer pci_devs.h naming to align chipset.cb. Correct thc0, thc1 and add cnvi_bt.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Iac33983dc12ed4e5b9257c50d82adc8e4e728ad6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/48153 Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/alderlake/chipset.cb 1 file changed, 9 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/intel/alderlake/chipset.cb b/src/soc/intel/alderlake/chipset.cb index ff81560..de880e3 100644 --- a/src/soc/intel/alderlake/chipset.cb +++ b/src/soc/intel/alderlake/chipset.cb @@ -13,18 +13,19 @@ device pci 08.0 alias gna off end device pci 09.0 alias north_tracehub off end device pci 0a.0 alias crashlog off end - device pci 0d.0 alias north_xhci off end - device pci 0d.1 alias north_xdci off end - device pci 0d.2 alias tbt_dma0 off end - device pci 0d.3 alias tbt_dma1 off end + device pci 0d.0 alias tcss_xhci off end + device pci 0d.1 alias tcss_xdci off end + device pci 0d.2 alias tcss_dma0 off end + device pci 0d.3 alias tcss_dma1 off end device pci 0e.0 alias vmd off end - device pci 10.6 alias thc0 off end - device pci 10.7 alias thc1 off end + device pci 10.0 alias thc0 off end + device pci 10.1 alias thc1 off end + device pci 10.2 alias cnvi_bt off end device pci 12.0 alias ish off end device pci 12.6 alias gspi2 off end device pci 13.0 alias gspi3 off end - device pci 14.0 alias south_xhci off end - device pci 14.1 alias south_xdci off end + device pci 14.0 alias xhci off end + device pci 14.1 alias usb_otg off end device pci 14.2 alias shared_sram off end device pci 14.3 alias cnvi_wifi off end device pci 15.0 alias i2c0 off end
-
EricR Lai (Code Review) -
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)