Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43123 )
Change subject: sb/intel/lynxpoint: Add PCH platform type function ......................................................................
sb/intel/lynxpoint: Add PCH platform type function
Current code only cares whether the PCH is LP or not. However, MRC wants to differentiate between desktop and non-LP mobile platforms as well. As the PCH is soldered onto the mainboard, add a facility to retrieve which platform coreboot is running on by checking the PCH's LPC device ID. The only user of the `pch_silicon_type` function is the `pch_is_lp` function so replace the former with the new `get_pch_platform_type` function. The function needs to be defined in both romstage and ramstage where PCI ops have different signatures, hence the two copies.
Change-Id: Ib6276e0069eaa069a365faf6ae02dd934307d36c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/early_pch.c M src/southbridge/intel/lynxpoint/pch.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 38 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/43123/1
diff --git a/src/southbridge/intel/lynxpoint/early_pch.c b/src/southbridge/intel/lynxpoint/early_pch.c index 92cbf01..80b8939 100644 --- a/src/southbridge/intel/lynxpoint/early_pch.c +++ b/src/southbridge/intel/lynxpoint/early_pch.c @@ -18,10 +18,25 @@ #include <southbridge/intel/common/gpio.h> #endif
+enum pch_platform_type get_pch_platform_type(void) +{ + const u16 did = pci_read_config16(PCH_LPC_DEV, PCI_DEVICE_ID); + + /* Check if this is a LPT-LP or WPT-LP device ID */ + if ((did & 0xff00) == 0x9c00) + return PCH_TYPE_ULT; + + /* Non-LP laptop SKUs have an odd device ID (least significant bit is one) */ + if (did & 1) + return PCH_TYPE_MOBILE; + + /* Desktop and Server SKUs have an even device ID */ + return PCH_TYPE_DESKTOP; +} + int pch_is_lp(void) { - u8 id = pci_read_config8(PCH_LPC_DEV, PCI_DEVICE_ID + 1); - return id == PCH_TYPE_LPT_LP; + return get_pch_platform_type() == PCH_TYPE_ULT; }
static void pch_enable_bars(void) diff --git a/src/southbridge/intel/lynxpoint/pch.c b/src/southbridge/intel/lynxpoint/pch.c index e3c5bb2..0a0e489 100644 --- a/src/southbridge/intel/lynxpoint/pch.c +++ b/src/southbridge/intel/lynxpoint/pch.c @@ -40,19 +40,25 @@ return pch_id; }
-int pch_silicon_type(void) +enum pch_platform_type get_pch_platform_type(void) { - static int pch_type = -1; + const u16 did = pci_read_config16(pch_get_lpc_device(), PCI_DEVICE_ID);
- if (pch_type < 0) - pch_type = pci_read_config8(pch_get_lpc_device(), - PCI_DEVICE_ID + 1); - return pch_type; + /* Check if this is a LPT-LP or WPT-LP device ID */ + if ((did & 0xff00) == 0x9c00) + return PCH_TYPE_ULT; + + /* Non-LP laptop SKUs have an odd device ID (least significant bit is one) */ + if (did & 1) + return PCH_TYPE_MOBILE; + + /* Desktop and Server SKUs have an even device ID */ + return PCH_TYPE_DESKTOP; }
int pch_is_lp(void) { - return pch_silicon_type() == PCH_TYPE_LPT_LP; + return get_pch_platform_type() == PCH_TYPE_ULT; }
u16 get_pmbase(void) diff --git a/src/southbridge/intel/lynxpoint/pch.h b/src/southbridge/intel/lynxpoint/pch.h index c59878e..896454f 100644 --- a/src/southbridge/intel/lynxpoint/pch.h +++ b/src/southbridge/intel/lynxpoint/pch.h @@ -34,10 +34,6 @@ * Bus 0:Device 20:Function 0 xHCI Controller */
-/* PCH types */ -#define PCH_TYPE_LPT 0x8c -#define PCH_TYPE_LPT_LP 0x9c - /* PCH stepping values for LPC device */ #define LPT_H_STEP_B0 0x02 #define LPT_H_STEP_C0 0x03 @@ -74,14 +70,21 @@
#ifndef __ACPI__
+/* PCH platform types, safe for MRC consumption */ +enum pch_platform_type { + PCH_TYPE_MOBILE = 0, + PCH_TYPE_DESKTOP = 1, /* or server */ + PCH_TYPE_ULT = 5, +}; + void usb_ehci_sleep_prepare(pci_devfn_t dev, u8 slp_typ); void usb_ehci_disable(pci_devfn_t dev); void usb_xhci_sleep_prepare(pci_devfn_t dev, u8 slp_typ); void usb_xhci_route_all(void);
+enum pch_platform_type get_pch_platform_type(void); int pch_silicon_revision(void); int pch_silicon_id(void); -int pch_silicon_type(void); int pch_is_lp(void); u16 get_pmbase(void); u16 get_gpiobase(void);
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43123
to look at the new patch set (#2).
Change subject: sb/intel/lynxpoint: Add PCH platform type function ......................................................................
sb/intel/lynxpoint: Add PCH platform type function
Current code only cares whether the PCH is LP or not. However, MRC wants to differentiate between desktop and non-LP mobile platforms as well. As the PCH is soldered onto the mainboard, add a facility to retrieve which platform coreboot is running on by checking the PCH's LPC device ID. The only user of the `pch_silicon_type` function is the `pch_is_lp` function so replace the former with the new `get_pch_platform_type` function. The function needs to be defined in both romstage and ramstage where PCI ops have different signatures, hence the two copies.
Change-Id: Ib6276e0069eaa069a365faf6ae02dd934307d36c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/southbridge/intel/lynxpoint/early_pch.c M src/southbridge/intel/lynxpoint/pch.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 38 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/43123/2
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43123 )
Change subject: sb/intel/lynxpoint: Add PCH platform type function ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43123 )
Change subject: sb/intel/lynxpoint: Add PCH platform type function ......................................................................
sb/intel/lynxpoint: Add PCH platform type function
Current code only cares whether the PCH is LP or not. However, MRC wants to differentiate between desktop and non-LP mobile platforms as well. As the PCH is soldered onto the mainboard, add a facility to retrieve which platform coreboot is running on by checking the PCH's LPC device ID. The only user of the `pch_silicon_type` function is the `pch_is_lp` function so replace the former with the new `get_pch_platform_type` function. The function needs to be defined in both romstage and ramstage where PCI ops have different signatures, hence the two copies.
Change-Id: Ib6276e0069eaa069a365faf6ae02dd934307d36c Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43123 Reviewed-by: Patrick Rudolph siro@das-labor.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/southbridge/intel/lynxpoint/early_pch.c M src/southbridge/intel/lynxpoint/pch.c M src/southbridge/intel/lynxpoint/pch.h 3 files changed, 38 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/southbridge/intel/lynxpoint/early_pch.c b/src/southbridge/intel/lynxpoint/early_pch.c index 92cbf01..80b8939 100644 --- a/src/southbridge/intel/lynxpoint/early_pch.c +++ b/src/southbridge/intel/lynxpoint/early_pch.c @@ -18,10 +18,25 @@ #include <southbridge/intel/common/gpio.h> #endif
+enum pch_platform_type get_pch_platform_type(void) +{ + const u16 did = pci_read_config16(PCH_LPC_DEV, PCI_DEVICE_ID); + + /* Check if this is a LPT-LP or WPT-LP device ID */ + if ((did & 0xff00) == 0x9c00) + return PCH_TYPE_ULT; + + /* Non-LP laptop SKUs have an odd device ID (least significant bit is one) */ + if (did & 1) + return PCH_TYPE_MOBILE; + + /* Desktop and Server SKUs have an even device ID */ + return PCH_TYPE_DESKTOP; +} + int pch_is_lp(void) { - u8 id = pci_read_config8(PCH_LPC_DEV, PCI_DEVICE_ID + 1); - return id == PCH_TYPE_LPT_LP; + return get_pch_platform_type() == PCH_TYPE_ULT; }
static void pch_enable_bars(void) diff --git a/src/southbridge/intel/lynxpoint/pch.c b/src/southbridge/intel/lynxpoint/pch.c index e3c5bb2..0a0e489 100644 --- a/src/southbridge/intel/lynxpoint/pch.c +++ b/src/southbridge/intel/lynxpoint/pch.c @@ -40,19 +40,25 @@ return pch_id; }
-int pch_silicon_type(void) +enum pch_platform_type get_pch_platform_type(void) { - static int pch_type = -1; + const u16 did = pci_read_config16(pch_get_lpc_device(), PCI_DEVICE_ID);
- if (pch_type < 0) - pch_type = pci_read_config8(pch_get_lpc_device(), - PCI_DEVICE_ID + 1); - return pch_type; + /* Check if this is a LPT-LP or WPT-LP device ID */ + if ((did & 0xff00) == 0x9c00) + return PCH_TYPE_ULT; + + /* Non-LP laptop SKUs have an odd device ID (least significant bit is one) */ + if (did & 1) + return PCH_TYPE_MOBILE; + + /* Desktop and Server SKUs have an even device ID */ + return PCH_TYPE_DESKTOP; }
int pch_is_lp(void) { - return pch_silicon_type() == PCH_TYPE_LPT_LP; + return get_pch_platform_type() == PCH_TYPE_ULT; }
u16 get_pmbase(void) diff --git a/src/southbridge/intel/lynxpoint/pch.h b/src/southbridge/intel/lynxpoint/pch.h index c59878e..896454f 100644 --- a/src/southbridge/intel/lynxpoint/pch.h +++ b/src/southbridge/intel/lynxpoint/pch.h @@ -34,10 +34,6 @@ * Bus 0:Device 20:Function 0 xHCI Controller */
-/* PCH types */ -#define PCH_TYPE_LPT 0x8c -#define PCH_TYPE_LPT_LP 0x9c - /* PCH stepping values for LPC device */ #define LPT_H_STEP_B0 0x02 #define LPT_H_STEP_C0 0x03 @@ -74,14 +70,21 @@
#ifndef __ACPI__
+/* PCH platform types, safe for MRC consumption */ +enum pch_platform_type { + PCH_TYPE_MOBILE = 0, + PCH_TYPE_DESKTOP = 1, /* or server */ + PCH_TYPE_ULT = 5, +}; + void usb_ehci_sleep_prepare(pci_devfn_t dev, u8 slp_typ); void usb_ehci_disable(pci_devfn_t dev); void usb_xhci_sleep_prepare(pci_devfn_t dev, u8 slp_typ); void usb_xhci_route_all(void);
+enum pch_platform_type get_pch_platform_type(void); int pch_silicon_revision(void); int pch_silicon_id(void); -int pch_silicon_type(void); int pch_is_lp(void); u16 get_pmbase(void); u16 get_gpiobase(void);
-
Angel Pons (Code Review) -
Patrick Rudolph (Code Review)