Skip to contentSkip to conversation

pinctrl: intel: Introduce common flags for GPIO mapping scheme

PATCH Superseded Andy Shevchenko 2 participants 6 messages Mar 31, 2020 – Apr 1, 2020

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/4 pinctrl: intel: Introduce common flags for GPIO mapping scheme

Few drivers are using the same flag to tell Intel pin control core how to interpret GPIO base.

Provide a generic flags so all drivers can use.

---
 drivers/pinctrl/intel/pinctrl-intel.c | 19 +++++++++++++------
 drivers/pinctrl/intel/pinctrl-intel.h |  5 +++--
 2 files changed, 16 insertions(+), 8 deletions(-)
drivers/pinctrl/intel/pinctrl-intel.c +13 -6
index 74fdfd2b9ff5..a1b286dc7008 100644
@@ -798,7 +798,7 @@ static int intel_gpio_to_pin(struct intel_pinctrl *pctrl, unsigned int offset,
 		for (j = 0; j < comm->ngpps; j++) {
 			const struct intel_padgroup *pgrp = &comm->gpps[j];
 
-			if (pgrp->gpio_base < 0)
+			if (pgrp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 				continue;
 
 			if (offset >= pgrp->gpio_base &&
@@ -1138,7 +1138,7 @@ static int intel_gpio_add_community_ranges(struct intel_pinctrl *pctrl,
 	for (i = 0; i < community->ngpps; i++) {
 		const struct intel_padgroup *gpp = &community->gpps[i];
 
-		if (gpp->gpio_base < 0)
+		if (gpp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 			continue;
 
 		ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
@@ -1180,7 +1180,7 @@ static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)
 		for (j = 0; j < community->ngpps; j++) {
 			const struct intel_padgroup *gpp = &community->gpps[j];
 
-			if (gpp->gpio_base < 0)
+			if (gpp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 				continue;
 
 			if (gpp->gpio_base + gpp->size > ngpio)
@@ -1276,8 +1276,15 @@ static int intel_pinctrl_add_padgroups(struct intel_pinctrl *pctrl,
 		if (gpps[i].size > 32)
 			return -EINVAL;
 
-		if (!gpps[i].gpio_base)
-			gpps[i].gpio_base = gpps[i].base;
+		/* Special treatment for GPIO base */
+		switch (gpps[i].gpio_base) {
+			case INTEL_GPIO_BASE_MATCH:
+				gpps[i].gpio_base = gpps[i].base;
+				break;
+			case INTEL_GPIO_BASE_NOMAP:
+			default:
+				break;
+		}
 
 		gpps[i].padown_num = padown_num;
 
@@ -1596,7 +1603,7 @@ static void intel_restore_hostown(struct intel_pinctrl *pctrl, unsigned int c,
 	struct device *dev = pctrl->dev;
 	u32 requested;
 
-	if (padgrp->gpio_base < 0)
+	if (padgrp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 		return;
 
 	requested = intel_gpio_is_requested(&pctrl->chip, padgrp->gpio_base, padgrp->size);
drivers/pinctrl/intel/pinctrl-intel.h +3 -2
index c6f066f6d3fb..df11bd6e4a80 100644
@@ -53,8 +53,7 @@ struct intel_function {
  * @reg_num: GPI_IS register number
  * @base: Starting pin of this group
  * @size: Size of this group (maximum is 32).
- * @gpio_base: Starting GPIO base of this group (%0 if matches with @base,
- *	       and %-1 if no GPIO mapping should be created)
+ * @gpio_base: Starting GPIO base of this group
  * @padown_num: PAD_OWN register number (assigned by the core driver)
  *
  * If pad groups of a community are not the same size, use this structure
@@ -64,6 +63,8 @@ struct intel_padgroup {
 	unsigned int reg_num;
 	unsigned int base;
 	unsigned int size;
+#define INTEL_GPIO_BASE_MATCH	0	/* matches with @base */
+#define INTEL_GPIO_BASE_NOMAP	(-1)	/* no GPIO mapping should be created */
 	int gpio_base;
 	unsigned int padown_num;
 };
Signature · 1 line
2.25.1
#1 lore raw

PATCH 2/4 pinctrl: cannonlake: Use generic flag for special GPIO base treatment

Since we have a generic flag for special GPIO base treatment, use it in the driver.

---
 drivers/pinctrl/intel/pinctrl-cannonlake.c | 58 +++++++++++-----------
 1 file changed, 28 insertions(+), 30 deletions(-)
drivers/pinctrl/intel/pinctrl-cannonlake.c +28 -30
index f51b27bbf9f1..515f57a0d180 100644
@@ -30,8 +30,6 @@
 		.gpio_base = (g),			\
 	}
 
-#define CNL_NO_GPIO	-1
-
 #define CNL_COMMUNITY(b, s, e, o, g)			\
 	{						\
 		.barno = (b),				\
@@ -377,27 +375,27 @@ static const struct intel_padgroup cnlh_community0_gpps[] = {
 };
 
 static const struct intel_padgroup cnlh_community1_gpps[] = {
-	CNL_GPP(0, 51, 74, 64),			/* GPP_C */
-	CNL_GPP(1, 75, 98, 96),			/* GPP_D */
-	CNL_GPP(2, 99, 106, 128),		/* GPP_G */
-	CNL_GPP(3, 107, 114, CNL_NO_GPIO),	/* AZA */
-	CNL_GPP(4, 115, 146, 160),		/* vGPIO_0 */
-	CNL_GPP(5, 147, 154, CNL_NO_GPIO),	/* vGPIO_1 */
+	CNL_GPP(0, 51, 74, 64),				/* GPP_C */
+	CNL_GPP(1, 75, 98, 96),				/* GPP_D */
+	CNL_GPP(2, 99, 106, 128),			/* GPP_G */
+	CNL_GPP(3, 107, 114, INTEL_GPIO_BASE_NOMAP),	/* AZA */
+	CNL_GPP(4, 115, 146, 160),			/* vGPIO_0 */
+	CNL_GPP(5, 147, 154, INTEL_GPIO_BASE_NOMAP),	/* vGPIO_1 */
 };
 
 static const struct intel_padgroup cnlh_community3_gpps[] = {
-	CNL_GPP(0, 155, 178, 192),		/* GPP_K */
-	CNL_GPP(1, 179, 202, 224),		/* GPP_H */
-	CNL_GPP(2, 203, 215, 256),		/* GPP_E */
-	CNL_GPP(3, 216, 239, 288),		/* GPP_F */
-	CNL_GPP(4, 240, 248, CNL_NO_GPIO),	/* SPI */
+	CNL_GPP(0, 155, 178, 192),			/* GPP_K */
+	CNL_GPP(1, 179, 202, 224),			/* GPP_H */
+	CNL_GPP(2, 203, 215, 256),			/* GPP_E */
+	CNL_GPP(3, 216, 239, 288),			/* GPP_F */
+	CNL_GPP(4, 240, 248, INTEL_GPIO_BASE_NOMAP),	/* SPI */
 };
 
 static const struct intel_padgroup cnlh_community4_gpps[] = {
-	CNL_GPP(0, 249, 259, CNL_NO_GPIO),	/* CPU */
-	CNL_GPP(1, 260, 268, CNL_NO_GPIO),	/* JTAG */
-	CNL_GPP(2, 269, 286, 320),		/* GPP_I */
-	CNL_GPP(3, 287, 298, 352),		/* GPP_J */
+	CNL_GPP(0, 249, 259, INTEL_GPIO_BASE_NOMAP),	/* CPU */
+	CNL_GPP(1, 260, 268, INTEL_GPIO_BASE_NOMAP),	/* JTAG */
+	CNL_GPP(2, 269, 286, 320),			/* GPP_I */
+	CNL_GPP(3, 287, 298, 352),			/* GPP_J */
 };
 
 static const unsigned int cnlh_spi0_pins[] = { 40, 41, 42, 43 };
@@ -790,25 +788,25 @@ static const struct intel_function cnllp_functions[] = {
 };
 
 static const struct intel_padgroup cnllp_community0_gpps[] = {
-	CNL_GPP(0, 0, 24, 0),			/* GPP_A */
-	CNL_GPP(1, 25, 50, 32),			/* GPP_B */
-	CNL_GPP(2, 51, 58, 64),			/* GPP_G */
-	CNL_GPP(3, 59, 67, CNL_NO_GPIO),	/* SPI */
+	CNL_GPP(0, 0, 24, 0),				/* GPP_A */
+	CNL_GPP(1, 25, 50, 32),				/* GPP_B */
+	CNL_GPP(2, 51, 58, 64),				/* GPP_G */
+	CNL_GPP(3, 59, 67, INTEL_GPIO_BASE_NOMAP),	/* SPI */
 };
 
 static const struct intel_padgroup cnllp_community1_gpps[] = {
-	CNL_GPP(0, 68, 92, 96),			/* GPP_D */
-	CNL_GPP(1, 93, 116, 128),		/* GPP_F */
-	CNL_GPP(2, 117, 140, 160),		/* GPP_H */
-	CNL_GPP(3, 141, 172, 192),		/* vGPIO */
-	CNL_GPP(4, 173, 180, 224),		/* vGPIO */
+	CNL_GPP(0, 68, 92, 96),				/* GPP_D */
+	CNL_GPP(1, 93, 116, 128),			/* GPP_F */
+	CNL_GPP(2, 117, 140, 160),			/* GPP_H */
+	CNL_GPP(3, 141, 172, 192),			/* vGPIO */
+	CNL_GPP(4, 173, 180, 224),			/* vGPIO */
 };
 
 static const struct intel_padgroup cnllp_community4_gpps[] = {
-	CNL_GPP(0, 181, 204, 256),		/* GPP_C */
-	CNL_GPP(1, 205, 228, 288),		/* GPP_E */
-	CNL_GPP(2, 229, 237, CNL_NO_GPIO),	/* JTAG */
-	CNL_GPP(3, 238, 243, CNL_NO_GPIO),	/* HVCMOS */
+	CNL_GPP(0, 181, 204, 256),			/* GPP_C */
+	CNL_GPP(1, 205, 228, 288),			/* GPP_E */
+	CNL_GPP(2, 229, 237, INTEL_GPIO_BASE_NOMAP),	/* JTAG */
+	CNL_GPP(3, 238, 243, INTEL_GPIO_BASE_NOMAP),	/* HVCMOS */
 };
 
 static const struct intel_community cnllp_communities[] = {
Signature · 1 line
2.25.1
#2 lore raw

PATCH 3/4 pinctrl: icelake: Use generic flag for special GPIO base treatment

In reply to Andy Shevchenko

Since we have a generic flag for special GPIO base treatment, use it in the driver.

---
 drivers/pinctrl/intel/pinctrl-icelake.c | 30 ++++++++++++-------------
 1 file changed, 14 insertions(+), 16 deletions(-)
drivers/pinctrl/intel/pinctrl-icelake.c +14 -16
index 6489e9bbb61f..429b5a83acf0 100644
@@ -29,8 +29,6 @@
 		.gpio_base = (g),			\
 	}
 
-#define ICL_NO_GPIO	-1
-
 #define ICL_COMMUNITY(b, s, e, g)			\
 	{						\
 		.barno = (b),				\
@@ -305,29 +303,29 @@ static const struct pinctrl_pin_desc icllp_pins[] = {
 };
 
 static const struct intel_padgroup icllp_community0_gpps[] = {
-	ICL_GPP(0, 0, 7, 0),			/* GPP_G */
-	ICL_GPP(1, 8, 33, 32),			/* GPP_B */
-	ICL_GPP(2, 34, 58, 64),			/* GPP_A */
+	ICL_GPP(0, 0, 7, 0),				/* GPP_G */
+	ICL_GPP(1, 8, 33, 32),				/* GPP_B */
+	ICL_GPP(2, 34, 58, 64),				/* GPP_A */
 };
 
 static const struct intel_padgroup icllp_community1_gpps[] = {
-	ICL_GPP(0, 59, 82, 96),			/* GPP_H */
-	ICL_GPP(1, 83, 103, 128),		/* GPP_D */
-	ICL_GPP(2, 104, 123, 160),		/* GPP_F */
-	ICL_GPP(3, 124, 152, 192),		/* vGPIO */
+	ICL_GPP(0, 59, 82, 96),				/* GPP_H */
+	ICL_GPP(1, 83, 103, 128),			/* GPP_D */
+	ICL_GPP(2, 104, 123, 160),			/* GPP_F */
+	ICL_GPP(3, 124, 152, 192),			/* vGPIO */
 };
 
 static const struct intel_padgroup icllp_community4_gpps[] = {
-	ICL_GPP(0, 153, 176, 224),		/* GPP_C */
-	ICL_GPP(1, 177, 182, ICL_NO_GPIO),	/* HVCMOS */
-	ICL_GPP(2, 183, 206, 256),		/* GPP_E */
-	ICL_GPP(3, 207, 215, ICL_NO_GPIO),	/* JTAG */
+	ICL_GPP(0, 153, 176, 224),			/* GPP_C */
+	ICL_GPP(1, 177, 182, INTEL_GPIO_BASE_NOMAP),	/* HVCMOS */
+	ICL_GPP(2, 183, 206, 256),			/* GPP_E */
+	ICL_GPP(3, 207, 215, INTEL_GPIO_BASE_NOMAP),	/* JTAG */
 };
 
 static const struct intel_padgroup icllp_community5_gpps[] = {
-	ICL_GPP(0, 216, 223, 288),		/* GPP_R */
-	ICL_GPP(1, 224, 231, 320),		/* GPP_S */
-	ICL_GPP(2, 232, 240, ICL_NO_GPIO),	/* SPI */
+	ICL_GPP(0, 216, 223, 288),			/* GPP_R */
+	ICL_GPP(1, 224, 231, 320),			/* GPP_S */
+	ICL_GPP(2, 232, 240, INTEL_GPIO_BASE_NOMAP),	/* SPI */
 };
 
 static const struct intel_community icllp_communities[] = {
Signature · 1 line
2.25.1
#3 lore raw

PATCH 4/4 pinctrl: tigerlake: Use generic flag for special GPIO base treatment

In reply to Andy Shevchenko

Since we have a generic flag for special GPIO base treatment, use it in the driver.

---
 drivers/pinctrl/intel/pinctrl-tigerlake.c | 32 +++++++++++------------
 1 file changed, 15 insertions(+), 17 deletions(-)
drivers/pinctrl/intel/pinctrl-tigerlake.c +15 -17
index 08a86f6fdea6..bcfd7548e282 100644
@@ -21,8 +21,6 @@
 #define TGL_GPI_IS	0x100
 #define TGL_GPI_IE	0x120
 
-#define TGL_NO_GPIO	-1
-
 #define TGL_GPP(r, s, e, g)				\
 	{						\
 		.reg_num = (r),				\
@@ -342,30 +340,30 @@ static const struct pinctrl_pin_desc tgllp_pins[] = {
 };
 
 static const struct intel_padgroup tgllp_community0_gpps[] = {
-	TGL_GPP(0, 0, 25, 0),			/* GPP_B */
-	TGL_GPP(1, 26, 41, 32),			/* GPP_T */
-	TGL_GPP(2, 42, 66, 64),			/* GPP_A */
+	TGL_GPP(0, 0, 25, 0),				/* GPP_B */
+	TGL_GPP(1, 26, 41, 32),				/* GPP_T */
+	TGL_GPP(2, 42, 66, 64),				/* GPP_A */
 };
 
 static const struct intel_padgroup tgllp_community1_gpps[] = {
-	TGL_GPP(0, 67, 74, 96),			/* GPP_S */
-	TGL_GPP(1, 75, 98, 128),		/* GPP_H */
-	TGL_GPP(2, 99, 119, 160),		/* GPP_D */
-	TGL_GPP(3, 120, 143, 192),		/* GPP_U */
-	TGL_GPP(4, 144, 170, 224),		/* vGPIO */
+	TGL_GPP(0, 67, 74, 96),				/* GPP_S */
+	TGL_GPP(1, 75, 98, 128),			/* GPP_H */
+	TGL_GPP(2, 99, 119, 160),			/* GPP_D */
+	TGL_GPP(3, 120, 143, 192),			/* GPP_U */
+	TGL_GPP(4, 144, 170, 224),			/* vGPIO */
 };
 
 static const struct intel_padgroup tgllp_community4_gpps[] = {
-	TGL_GPP(0, 171, 194, 256),		/* GPP_C */
-	TGL_GPP(1, 195, 219, 288),		/* GPP_F */
-	TGL_GPP(2, 220, 225, TGL_NO_GPIO),	/* HVCMOS */
-	TGL_GPP(3, 226, 250, 320),		/* GPP_E */
-	TGL_GPP(4, 251, 259, TGL_NO_GPIO),	/* JTAG */
+	TGL_GPP(0, 171, 194, 256),			/* GPP_C */
+	TGL_GPP(1, 195, 219, 288),			/* GPP_F */
+	TGL_GPP(2, 220, 225, INTEL_GPIO_BASE_NOMAP),	/* HVCMOS */
+	TGL_GPP(3, 226, 250, 320),			/* GPP_E */
+	TGL_GPP(4, 251, 259, INTEL_GPIO_BASE_NOMAP),	/* JTAG */
 };
 
 static const struct intel_padgroup tgllp_community5_gpps[] = {
-	TGL_GPP(0, 260, 267, 352),		/* GPP_R */
-	TGL_GPP(1, 268, 276, TGL_NO_GPIO),	/* SPI */
+	TGL_GPP(0, 260, 267, 352),			/* GPP_R */
+	TGL_GPP(1, 268, 276, INTEL_GPIO_BASE_NOMAP),	/* SPI */
 };
 
 static const struct intel_community tgllp_communities[] = {
Signature · 1 line
2.25.1
maintainer
#4 lore raw
In reply to Andy Shevchenko
Quoted 84 lines · Andy Shevchenko

Few drivers are using the same flag to tell Intel pin control core how to interpret GPIO base.

Provide a generic flags so all drivers can use.

---
 drivers/pinctrl/intel/pinctrl-intel.c | 19 +++++++++++++------
 drivers/pinctrl/intel/pinctrl-intel.h |  5 +++--
 2 files changed, 16 insertions(+), 8 deletions(-)
drivers/pinctrl/intel/pinctrl-intel.c +13 -6
index 74fdfd2b9ff5..a1b286dc7008 100644
@@ -798,7 +798,7 @@ static int intel_gpio_to_pin(struct intel_pinctrl *pctrl, unsigned int offset,
 		for (j = 0; j < comm->ngpps; j++) {
 			const struct intel_padgroup *pgrp = &comm->gpps[j];
 
-			if (pgrp->gpio_base < 0)
+			if (pgrp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 				continue;
 
 			if (offset >= pgrp->gpio_base &&
@@ -1138,7 +1138,7 @@ static int intel_gpio_add_community_ranges(struct intel_pinctrl *pctrl,
 	for (i = 0; i < community->ngpps; i++) {
 		const struct intel_padgroup *gpp = &community->gpps[i];
 
-		if (gpp->gpio_base < 0)
+		if (gpp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 			continue;
 
 		ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev),
@@ -1180,7 +1180,7 @@ static unsigned int intel_gpio_ngpio(const struct intel_pinctrl *pctrl)
 		for (j = 0; j < community->ngpps; j++) {
 			const struct intel_padgroup *gpp = &community->gpps[j];
 
-			if (gpp->gpio_base < 0)
+			if (gpp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 				continue;
 
 			if (gpp->gpio_base + gpp->size > ngpio)
@@ -1276,8 +1276,15 @@ static int intel_pinctrl_add_padgroups(struct intel_pinctrl *pctrl,
 		if (gpps[i].size > 32)
 			return -EINVAL;
 
-		if (!gpps[i].gpio_base)
-			gpps[i].gpio_base = gpps[i].base;
+		/* Special treatment for GPIO base */
+		switch (gpps[i].gpio_base) {
+			case INTEL_GPIO_BASE_MATCH:
+				gpps[i].gpio_base = gpps[i].base;
+				break;
+			case INTEL_GPIO_BASE_NOMAP:
+			default:
+				break;
+		}
 
 		gpps[i].padown_num = padown_num;
 
@@ -1596,7 +1603,7 @@ static void intel_restore_hostown(struct intel_pinctrl *pctrl, unsigned int c,
 	struct device *dev = pctrl->dev;
 	u32 requested;
 
-	if (padgrp->gpio_base < 0)
+	if (padgrp->gpio_base == INTEL_GPIO_BASE_NOMAP)
 		return;
 
 	requested = intel_gpio_is_requested(&pctrl->chip, padgrp->gpio_base, padgrp->size);
drivers/pinctrl/intel/pinctrl-intel.h +1 -2
index c6f066f6d3fb..df11bd6e4a80 100644
@@ -53,8 +53,7 @@ struct intel_function {
  * @reg_num: GPI_IS register number
  * @base: Starting pin of this group
  * @size: Size of this group (maximum is 32).
- * @gpio_base: Starting GPIO base of this group (%0 if matches with @base,
- *	       and %-1 if no GPIO mapping should be created)
+ * @gpio_base: Starting GPIO base of this group
  * @padown_num: PAD_OWN register number (assigned by the core driver)
  *
  * If pad groups of a community are not the same size, use this structure

drivers/pinctrl/intel/pinctrl-intel.h struct intel_padgroup { · line 67

Quoted 5 lines
drivers/pinctrl/intel/pinctrl-intel.h +2 -0
@@ -64,6 +63,8 @@ struct intel_padgroup {
 	unsigned int reg_num;
 	unsigned int base;
 	unsigned int size;
+#define INTEL_GPIO_BASE_MATCH	0	/* matches with @base */
+#define INTEL_GPIO_BASE_NOMAP	(-1)	/* no GPIO mapping should be created */

Maybe use enum and add kernel-doc there?

Quoted 5 lines
drivers/pinctrl/intel/pinctrl-intel.h +0 -0
 	int gpio_base;
 	unsigned int padown_num;
 };
Signature · 1 line
2.25.1
#5 lore raw
Quoted 5 lines · Mika Westerberg
Quoted 4 lines · Andy Shevchenko

Few drivers are using the same flag to tell Intel pin control core how to interpret GPIO base.

Provide a generic flags so all drivers can use.

...

Quoted 4 lines
Quoted 3 lines

- * @gpio_base: Starting GPIO base of this group (%0 if matches with @base,
- * and %-1 if no GPIO mapping should be created)
+ * @gpio_base: Starting GPIO base of this group

Quoted 10 lines

drivers/pinctrl/intel/pinctrl-intel.h struct intel_padgroup { · line 67

Quoted 6 lines
drivers/pinctrl/intel/pinctrl-intel.h +2 -0
@@ -64,6 +63,8 @@ struct intel_padgroup {
 	unsigned int reg_num;
 	unsigned int base;
 	unsigned int size;
+#define INTEL_GPIO_BASE_MATCH	0	/* matches with @base */
+#define INTEL_GPIO_BASE_NOMAP	(-1)	/* no GPIO mapping should be created */

Maybe use enum and add kernel-doc there?

Quoted 1 line
drivers/pinctrl/intel/pinctrl-intel.h +0 -0
 	int gpio_base;

Will do it in v2, thanks!

Signature · 2 lines
With Best Regards,
Andy Shevchenko