[coreboot-gerrit] New patch to review for coreboot: skylake: fix SMI GPI status handling

Aaron Durbin (adurbin@chromium.org) gerrit at coreboot.org
Wed Aug 12 17:52:32 CEST 2015


Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/11208

-gerrit

commit c6de95c4c23cb1ae328145e17f8e9c48b31b74ad
Author: Aaron Durbin <adurbin at chromium.org>
Date:   Sat Aug 8 01:11:32 2015 -0500

    skylake: fix SMI GPI status handling
    
    The current construction for processing SMI GPI events
    didn't allow for the mainboard to query the state of a
    particular GPI for the snapshotted SMI event. The
    skylake part can route GPIs from any (there are design
    limitations) GPIO group. Those status and enable registers
    are within the GPIO community so one needs to gather
    all the possibilities in order to query the state.
    
    The call chain did this:
    southbridge_smi_gpi(
    	clear_alt_smi_status() -> reset_alt_smi_status() ->
    	print_all_smi_status() -> return 0)
    
    As a replacement the following functions and types are
    introduced:
    
    struct gpi_status - represent gpi status.
    gpi_status_get() - per gpi query on struct gpi_status
    gpi_clear_get_smi_status() - clear and retrieve SMI GPI status
    mainboard_smi_gpi_handler() - mainboard handler using gpi_status
    
    Also remove gpio_enable_all_smi() as that construct was never
    used, but it also is quite heavy handed in that it would
    enable SMI generation for all GPIs.
    
    BUG=chrome-os-partner:43778
    BRANCH=None
    TEST=Built.
    
    Original-Change-Id: Ief977e60de65d9964b8ee58f2433cae5c93872ca
    Original-Signed-off-by: Aaron Durbin <adurbin at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/291933
    Original-Reviewed-by: Duncan Laurie <dlaurie at chromium.org>
    
    Change-Id: Ida009393c6af88ffe910195dc79a4c0d2a4c029e
    Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
 src/soc/intel/skylake/gpio.c             | 245 +++++++++++++++----------------
 src/soc/intel/skylake/include/soc/gpio.h |  21 +--
 src/soc/intel/skylake/include/soc/pm.h   |   4 -
 src/soc/intel/skylake/include/soc/smm.h  |   5 +
 src/soc/intel/skylake/pmutil.c           |  48 ------
 src/soc/intel/skylake/smihandler.c       |  10 +-
 6 files changed, 140 insertions(+), 193 deletions(-)

diff --git a/src/soc/intel/skylake/gpio.c b/src/soc/intel/skylake/gpio.c
index 6a904bd..5493cbd 100644
--- a/src/soc/intel/skylake/gpio.c
+++ b/src/soc/intel/skylake/gpio.c
@@ -17,6 +17,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <arch/io.h>
+#include <console/console.h>
 #include <device/device.h>
 #include <device/pci.h>
 #include <gpio.h>
@@ -57,6 +58,36 @@ static const struct gpio_community communities[] = {
 	},
 };
 
+static const char *gpio_group_names[GPIO_NUM_GROUPS] = {
+	"GPP_A",
+	"GPP_B",
+	"GPP_C",
+	"GPP_D",
+	"GPP_E",
+	"GPP_F",
+	"GPP_G",
+	"GPD",
+};
+
+static inline size_t gpios_in_community(const struct gpio_community *comm)
+{
+	/* max is inclusive */
+	return comm->max - comm->min + 1;
+}
+
+static inline size_t groups_in_community(const struct gpio_community *comm)
+{
+	size_t n = gpios_in_community(comm) + GPIO_MAX_NUM_PER_GROUP - 1;
+	return n / GPIO_MAX_NUM_PER_GROUP;
+}
+
+static inline int gpio_index_gpd(gpio_t gpio)
+{
+	if (gpio >= GPD0 && gpio <= GPD11)
+		return 1;
+	return 0;
+}
+
 static const struct gpio_community *gpio_get_community(gpio_t pad)
 {
 	size_t i;
@@ -71,6 +102,90 @@ static const struct gpio_community *gpio_get_community(gpio_t pad)
 	return NULL;
 }
 
+static size_t community_clr_get_smi_sts(const struct gpio_community *comm,
+					uint32_t *sts)
+{
+	uint8_t *regs;
+	size_t i;
+	uint32_t *gpi_status_reg;
+	uint32_t *gpi_en_reg;
+	const size_t num_grps = groups_in_community(comm);
+
+	/* Not all groups can be routed to SMI. However, the registers
+	 * read as 0. In order to simplify the logic read everything from
+	 * each community. */
+	regs = pcr_port_regs(comm->port_id);
+	gpi_status_reg = (void *)&regs[GPI_SMI_STS_OFFSET];
+	gpi_en_reg = (void *)&regs[GPI_SMI_EN_OFFSET];
+	for (i = 0; i < num_grps; i++) {
+		sts[i] = read32(gpi_status_reg + i) & read32(gpi_en_reg + i);
+		/* Clear the enabled and set status bits. */
+		write32(gpi_status_reg + i, sts[i]);
+	}
+
+	return num_grps;
+}
+
+static void print_gpi_status(uint32_t status, const char *grp_name)
+{
+	int i;
+
+	if (!status)
+		return;
+
+	for (i = 31; i >= 0; i--) {
+		if (status & (1 << i))
+			printk(BIOS_DEBUG, "%s%d ", grp_name, i);
+	}
+}
+
+void gpi_clear_get_smi_status(struct gpi_status *sts)
+{
+	int i;
+	int do_print;
+	size_t sts_index = 0;
+
+	for (i = 0; i < ARRAY_SIZE(communities); i++) {
+		const struct gpio_community *comm = &communities[i];
+		sts_index += community_clr_get_smi_sts(comm,
+						&sts->grp[sts_index]);
+	}
+
+	do_print = 0;
+	for (i = 0; i < ARRAY_SIZE(sts->grp); i++) {
+		if (sts->grp[i] == 0)
+			continue;
+		do_print = 1;
+		break;
+	}
+
+	if (!do_print)
+		return;
+
+	printk(BIOS_DEBUG, "GPI_SMI_STS: ");
+	for (i = 0; i < ARRAY_SIZE(sts->grp); i++)
+		print_gpi_status(sts->grp[i], gpio_group_names[i]);
+	printk(BIOS_DEBUG, "\n");
+}
+
+int gpi_status_get(const struct gpi_status *sts, gpio_t gpi)
+{
+	const uint32_t *gpi_sts;
+
+	/* Check if valid gpi */
+	if (gpio_get_community(gpi) == NULL)
+		return 0;
+
+	/* If not in GPD group the index is a linear function based on
+	 * GPI number and GPIO_MAX_NUM_PER_GROUP. */
+	if (gpio_index_gpd(gpi))
+		gpi_sts = &sts->grp[GPD];
+	else
+		gpi_sts = &sts->grp[gpi / GPIO_MAX_NUM_PER_GROUP];
+
+	return !!(*gpi_sts & (1 << (gpi % GPIO_MAX_NUM_PER_GROUP)));
+}
+
 void gpio_route_gpe(uint16_t gpe0_route)
 {
 	int i;
@@ -272,133 +387,3 @@ void gpio_set(gpio_t gpio_num, int value)
 	write32(&dw_regs[0], reg);
 	/* GPIO port ids support posted write semantics. */
 }
-
-/* Keep the ordering intact GPP_A ~ G, GPD.
- * As the gpio/smi functions gpio_get_smi_status() and
- * gpio_enable_groupsmi() depends on this ordering.
- */
-static const GPIO_GROUP_INFO gpio_group_info[] = {
-	/* GPP_A */
-	{
-		.community = PID_GPIOCOM0,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPP_A_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPP_A_PAD_MAX,
-		.smistsoffset = R_PCH_PCR_GPIO_GPP_A_SMI_STS,
-		.smienoffset = R_PCH_PCR_GPIO_GPP_A_SMI_EN,
-	},
-	/* GPP_B */
-	{
-		.community = PID_GPIOCOM0,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPP_B_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPP_B_PAD_MAX,
-		.smistsoffset = R_PCH_PCR_GPIO_GPP_B_SMI_STS,
-		.smienoffset = R_PCH_PCR_GPIO_GPP_B_SMI_EN,
-	},
-	/* GPP_C */
-	{
-		.community = PID_GPIOCOM1,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPP_C_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPP_C_PAD_MAX,
-		.smistsoffset = R_PCH_PCR_GPIO_GPP_C_SMI_STS,
-		.smienoffset = R_PCH_PCR_GPIO_GPP_C_SMI_EN,
-	},
-	/* GPP_D */
-	{
-		.community = PID_GPIOCOM1,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPP_D_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPP_D_PAD_MAX,
-		.smistsoffset = R_PCH_PCR_GPIO_GPP_D_SMI_STS,
-		.smienoffset = R_PCH_PCR_GPIO_GPP_D_SMI_EN,
-	},
-	/* GPP_E */
-	{
-		.community = PID_GPIOCOM1,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPP_E_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPP_E_PAD_MAX,
-		.smistsoffset = R_PCH_PCR_GPIO_GPP_E_SMI_STS,
-		.smienoffset = R_PCH_PCR_GPIO_GPP_E_SMI_EN,
-	},
-	/* GPP_F */
-	{
-		.community = PID_GPIOCOM3,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPP_F_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPP_F_PAD_MAX,
-		.smistsoffset = NO_REGISTER_PROPERTY,
-		.smienoffset = NO_REGISTER_PROPERTY,
-	},
-	/* GPP_G */
-	{
-		.community = PID_GPIOCOM3,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPP_G_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPP_G_PAD_MAX,
-		.smistsoffset = NO_REGISTER_PROPERTY,
-		.smienoffset = NO_REGISTER_PROPERTY,
-	},
-	/* GPD */
-	{
-		.community = PID_GPIOCOM2,
-		.padcfgoffset = R_PCH_PCR_GPIO_GPD_PADCFG_OFFSET,
-		.padpergroup = V_PCH_GPIO_GPD_PAD_MAX,
-		.smistsoffset = NO_REGISTER_PROPERTY,
-		.smienoffset = NO_REGISTER_PROPERTY,
-	},
-};
-
-void gpio_clear_all_smi(void)
-{
-	u32 gpiogroupinfolength;
-	u32 gpioindex = 0;
-
-	gpiogroupinfolength = sizeof(gpio_group_info) / sizeof(GPIO_GROUP_INFO);
-
-	for (gpioindex = 0; gpioindex < gpiogroupinfolength; gpioindex++) {
-		/*Check if group has GPI SMI register */
-		if (gpio_group_info[gpioindex].smistsoffset ==
-		    NO_REGISTER_PROPERTY)
-			continue;
-		/* Clear all GPI SMI Status bits by writing '1' */
-		pcr_write32(gpio_group_info[gpioindex].community,
-			    gpio_group_info[gpioindex].smistsoffset,
-			    0xFFFFFFFF);
-	}
-}
-
-void gpio_get_smi_status(u32 status[GPIO_COMMUNITY_MAX])
-{
-	u32 num_of_communities;
-	u32 gpioindex;
-	u32 outputvalue = 0;
-
-	num_of_communities = ARRAY_SIZE(gpio_group_info);
-
-	for (gpioindex = 0; gpioindex < num_of_communities; gpioindex++) {
-		/*Check if group has GPI SMI register */
-		if (gpio_group_info[gpioindex].smistsoffset ==
-		    NO_REGISTER_PROPERTY)
-			continue;
-		/* Read SMI status register */
-		pcr_read32(gpio_group_info[gpioindex].community,
-			   gpio_group_info[gpioindex].smistsoffset,
-			   &outputvalue);
-		status[gpioindex] = outputvalue;
-	}
-}
-
-void gpio_enable_all_smi(void)
-{
-	u32 gpiogroupinfolength;
-	u32 gpioindex = 0;
-
-	gpiogroupinfolength = sizeof(gpio_group_info) / sizeof(GPIO_GROUP_INFO);
-
-	for (gpioindex = 0; gpioindex < gpiogroupinfolength; gpioindex++) {
-		/*Check if group has GPI SMI register */
-		if (gpio_group_info[gpioindex].smienoffset ==
-		    NO_REGISTER_PROPERTY)
-			continue;
-		/* Set all GPI SMI Enable bits by writing '1' */
-		pcr_write32(gpio_group_info[gpioindex].community,
-			    gpio_group_info[gpioindex].smienoffset,
-			    0xFFFFFFFF);
-	}
-}
diff --git a/src/soc/intel/skylake/include/soc/gpio.h b/src/soc/intel/skylake/include/soc/gpio.h
index 2116c7e..488a2b5 100644
--- a/src/soc/intel/skylake/include/soc/gpio.h
+++ b/src/soc/intel/skylake/include/soc/gpio.h
@@ -28,19 +28,22 @@
 #include <stddef.h>
 #include <soc/gpio_fsp.h>
 
-/* SOC has 8 GPIO communities GPP A~G, GPD */
-#define GPIO_COMMUNITY_MAX		8
-
 typedef uint32_t gpio_t;
 
-/* Clear GPIO SMI Status */
-void gpio_clear_all_smi(void);
+/* Structure to represent GPI status for GPE and SMI. Use helper
+ * functions for interrogating particular GPIs. */
+struct gpi_status {
+	uint32_t grp[GPIO_NUM_GROUPS];
+};
 
-/* Get GPIO SMI Status */
-void gpio_get_smi_status(u32 status[GPIO_COMMUNITY_MAX]);
+/*
+ * Clear GPI SMI status and fill in the structure representing enabled
+ * and set status.
+ */
+void gpi_clear_get_smi_status(struct gpi_status *sts);
 
-/* Enable GPIO SMI  */
-void gpio_enable_all_smi(void);
+/* Return 1 if gpio is set in the gpi_status struct. Otherwise 0. */
+int gpi_status_get(const struct gpi_status *sts, gpio_t gpi);
 
 /*
  * Set the GPIO groups for the GPE blocks. The gpe0_route is interpreted
diff --git a/src/soc/intel/skylake/include/soc/pm.h b/src/soc/intel/skylake/include/soc/pm.h
index c0a165e..3daa320 100644
--- a/src/soc/intel/skylake/include/soc/pm.h
+++ b/src/soc/intel/skylake/include/soc/pm.h
@@ -174,10 +174,6 @@ uint32_t clear_smi_status(void);
 void enable_smi(uint32_t mask);
 void disable_smi(uint32_t mask);
 
-/* ALT_GP_SMI */
-uint32_t clear_alt_smi_status(void);
-void reset_alt_smi_status(void);
-
 /* TCO */
 uint32_t clear_tco_status(void);
 void enable_tco_sci(void);
diff --git a/src/soc/intel/skylake/include/soc/smm.h b/src/soc/intel/skylake/include/soc/smm.h
index fbae6ef..212a444 100644
--- a/src/soc/intel/skylake/include/soc/smm.h
+++ b/src/soc/intel/skylake/include/soc/smm.h
@@ -25,6 +25,7 @@
 #include <cpu/x86/msr.h>
 #include <soc/intel/common/romstage.h>
 #include <soc/intel/common/memmap.h>
+#include <soc/gpio.h>
 
 struct ied_header {
 	char signature[10];
@@ -51,6 +52,10 @@ struct smm_relocation_params {
 	int smm_save_state_in_msrs;
 };
 
+/* Mainboard handler for GPI SMIs*/
+void mainboard_smi_gpi_handler(const struct gpi_status *sts);
+
+
 #if IS_ENABLED(CONFIG_HAVE_SMI_HANDLER)
 int smm_initialize(void);
 void smm_relocate(void);
diff --git a/src/soc/intel/skylake/pmutil.c b/src/soc/intel/skylake/pmutil.c
index 8e54db4..b048c99 100644
--- a/src/soc/intel/skylake/pmutil.c
+++ b/src/soc/intel/skylake/pmutil.c
@@ -210,54 +210,6 @@ void disable_smi(u32 mask)
 	outl(smi_en, ACPI_BASE_ADDRESS + SMI_EN);
 }
 
-
-/*
- * ALT_GP_SMI
- */
-
-/* Clear GPIO SMI status and return events that are enabled and active */
-void reset_alt_smi_status(void)
-{
-	/*Clear GPIO SMI Status*/
-	gpio_clear_all_smi();
-}
-
-/* Print GPIO SMI status bits */
-static u32 print_alt_smi_status(void)
-{
-	u32 alt_sts[GPIO_COMMUNITY_MAX];
-	int gpio_index;
-	/* GPIO Communities GPP_A ~ E support SMI */
-	const char gpiowell[] = {
-		[0] = 'A',
-		[1] = 'B',
-		[2] = 'C',
-		[3] = 'D',
-		[4] = 'E'
-	};
-
-	printk(BIOS_DEBUG, "ALT_STS: ");
-	gpio_get_smi_status(alt_sts);
-	/* GPP_A to GPP_E GPIO has Status and Enable functionality*/
-	for (gpio_index = 0; gpio_index < ARRAY_SIZE(gpiowell);
-		gpio_index++) {
-		printk(BIOS_DEBUG, "GPIO Group_%c\n",
-				gpiowell[gpio_index]);
-		print_gpio_status(alt_sts[gpio_index], 0);
-	}
-
-	printk(BIOS_DEBUG, "\n");
-
-	return 0;
-}
-
-/* Print, clear, and return GPIO SMI status */
-u32 clear_alt_smi_status(void)
-{
-	reset_alt_smi_status();
-	return print_alt_smi_status();
-}
-
 /*
  * TCO
  */
diff --git a/src/soc/intel/skylake/smihandler.c b/src/soc/intel/skylake/smihandler.c
index 6c95955..fa5ef04 100644
--- a/src/soc/intel/skylake/smihandler.c
+++ b/src/soc/intel/skylake/smihandler.c
@@ -351,12 +351,18 @@ static void southbridge_smi_gpe0(void)
 	clear_gpe_status();
 }
 
+void __attribute__((weak))
+mainboard_smi_gpi_handler(const struct gpi_status *sts) { }
+
 static void southbridge_smi_gpi(void)
 {
-	mainboard_smi_gpi(clear_alt_smi_status());
+	struct gpi_status smi_sts;
+
+	gpi_clear_get_smi_status(&smi_sts);
+	mainboard_smi_gpi_handler(&smi_sts);
 
 	/* Clear again after mainboard handler */
-	clear_alt_smi_status();
+	gpi_clear_get_smi_status(&smi_sts);
 }
 
 static void southbridge_smi_mc(void)



More information about the coreboot-gerrit mailing list