[PATCH v2 0/6] pinctrl: Fixes and updates to Intel drivers

[PATCH v2 0/6] pinctrl: Fixes and updates to Intel drivers

[Mika Westerberg]

This series fixes one issue found on Broxton, updates the core driver to
use devm_gpiochip_add_data() and finally add support for the next
generation Intel GPIO hardware available starting from Intel Gemini Lake SoC.

The Gemini Lake SoC brings hardware debouncer per-pad and 1k additional
pull-down. In order to configure the hardware debouncer, this series adds a
new function to pinctrl core; pinctrl_gpio_set_config(). This function can
be used by gpiolib-based GPIO drivers to ask the backing pinctrl driver to
apply certain configuration, like setting debounce time.

Changes from v1:
 * Added Reviewed-by from Andy
 * Use goto exit_unlock in intel_config_set_debounce()
 * Features are now prefixed with PINCTRL_FEATURE_*

Mika Westerberg (6):
  pinctrl: broxton: Use correct PADCFGLOCK offset
  pinctrl: intel: Convert to use devm_gpiochip_add_data()
  pinctrl: Add a possibility to configure pins from a gpiolib based drivers
  pinctrl: intel: Add support for hardware debouncer
  pinctrl: intel: Add support for 1k additional pull-down
  pinctrl: intel: Add Intel Gemini Lake pin controller support

 drivers/pinctrl/core.c                       |  28 ++
 drivers/pinctrl/intel/Kconfig                |   8 +
 drivers/pinctrl/intel/Makefile               |   1 +
 drivers/pinctrl/intel/pinctrl-broxton.c      |   3 +-
 drivers/pinctrl/intel/pinctrl-geminilake.c   | 512 +++++++++++++++++++++++++++
 drivers/pinctrl/intel/pinctrl-intel.c        | 178 ++++++++--
 drivers/pinctrl/intel/pinctrl-intel.h        |   8 +-
 drivers/pinctrl/intel/pinctrl-sunrisepoint.c |   1 -
 drivers/pinctrl/pinconf.c                    |  12 +
 drivers/pinctrl/pinconf.h                    |  10 +
 include/linux/pinctrl/consumer.h             |   8 +
 11 files changed, 744 insertions(+), 25 deletions(-)
 create mode 100644 drivers/pinctrl/intel/pinctrl-geminilake.c

-- 
2.11.0

[PATCH v2 1/6] pinctrl: broxton: Use correct PADCFGLOCK offset

[Mika Westerberg]

PADCFGLOCK (and PADCFGLOCK_TX) offset in Broxton actually starts at 0x060
and not 0x090 as used in the driver. Fix it to use the correct offset.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/pinctrl/intel/pinctrl-broxton.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-broxton.c b/drivers/pinctrl/intel/pinctrl-broxton.c
index 59cb7a6fc5be..901b356b09d7 100644
--- a/drivers/pinctrl/intel/pinctrl-broxton.c
+++ b/drivers/pinctrl/intel/pinctrl-broxton.c
@@ -19,7 +19,7 @@
 
 #define BXT_PAD_OWN	0x020
 #define BXT_HOSTSW_OWN	0x080
-#define BXT_PADCFGLOCK	0x090
+#define BXT_PADCFGLOCK	0x060
 #define BXT_GPI_IE	0x110
 
 #define BXT_COMMUNITY(s, e)				\
-- 
2.11.0

[PATCH v2 2/6] pinctrl: intel: Convert to use devm_gpiochip_add_data()

[Mika Westerberg]

This simplifies error handling and allows us to drop intel_pinctrl_remove()
completely.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/pinctrl/intel/pinctrl-broxton.c      |  1 -
 drivers/pinctrl/intel/pinctrl-intel.c        | 23 ++++-------------------
 drivers/pinctrl/intel/pinctrl-intel.h        |  2 --
 drivers/pinctrl/intel/pinctrl-sunrisepoint.c |  1 -
 4 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-broxton.c b/drivers/pinctrl/intel/pinctrl-broxton.c
index 901b356b09d7..8ec2b6a77745 100644
--- a/drivers/pinctrl/intel/pinctrl-broxton.c
+++ b/drivers/pinctrl/intel/pinctrl-broxton.c
@@ -1058,7 +1058,6 @@ static const struct dev_pm_ops bxt_pinctrl_pm_ops = {
 
 static struct platform_driver bxt_pinctrl_driver = {
 	.probe = bxt_pinctrl_probe,
-	.remove = intel_pinctrl_remove,
 	.driver = {
 		.name = "broxton-pinctrl",
 		.acpi_match_table = bxt_pinctrl_acpi_match,
diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 1e139672f1af..447405809340 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -884,7 +884,7 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 	pctrl->chip.base = -1;
 	pctrl->irq = irq;
 
-	ret = gpiochip_add_data(&pctrl->chip, pctrl);
+	ret = devm_gpiochip_add_data(pctrl->dev, &pctrl->chip, pctrl);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to register gpiochip\n");
 		return ret;
@@ -894,7 +894,7 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 				     0, 0, pctrl->soc->npins);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to add GPIO pin range\n");
-		goto fail;
+		return ret;
 	}
 
 	/*
@@ -907,24 +907,19 @@ static int intel_gpio_probe(struct intel_pinctrl *pctrl, int irq)
 			       dev_name(pctrl->dev), pctrl);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to request interrupt\n");
-		goto fail;
+		return ret;
 	}
 
 	ret = gpiochip_irqchip_add(&pctrl->chip, &intel_gpio_irqchip, 0,
 				   handle_bad_irq, IRQ_TYPE_NONE);
 	if (ret) {
 		dev_err(pctrl->dev, "failed to add irqchip\n");
-		goto fail;
+		return ret;
 	}
 
 	gpiochip_set_chained_irqchip(&pctrl->chip, &intel_gpio_irqchip, irq,
 				     NULL);
 	return 0;
-
-fail:
-	gpiochip_remove(&pctrl->chip);
-
-	return ret;
 }
 
 static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
@@ -1046,16 +1041,6 @@ int intel_pinctrl_probe(struct platform_device *pdev,
 }
 EXPORT_SYMBOL_GPL(intel_pinctrl_probe);
 
-int intel_pinctrl_remove(struct platform_device *pdev)
-{
-	struct intel_pinctrl *pctrl = platform_get_drvdata(pdev);
-
-	gpiochip_remove(&pctrl->chip);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(intel_pinctrl_remove);
-
 #ifdef CONFIG_PM_SLEEP
 static bool intel_pinctrl_should_save(struct intel_pinctrl *pctrl, unsigned pin)
 {
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index b60215793017..c22c44485c5d 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -121,8 +121,6 @@ struct intel_pinctrl_soc_data {
 
 int intel_pinctrl_probe(struct platform_device *pdev,
 			const struct intel_pinctrl_soc_data *soc_data);
-int intel_pinctrl_remove(struct platform_device *pdev);
-
 #ifdef CONFIG_PM_SLEEP
 int intel_pinctrl_suspend(struct device *dev);
 int intel_pinctrl_resume(struct device *dev);
diff --git a/drivers/pinctrl/intel/pinctrl-sunrisepoint.c b/drivers/pinctrl/intel/pinctrl-sunrisepoint.c
index c725a5313b4e..9877526c0807 100644
--- a/drivers/pinctrl/intel/pinctrl-sunrisepoint.c
+++ b/drivers/pinctrl/intel/pinctrl-sunrisepoint.c
@@ -574,7 +574,6 @@ static const struct dev_pm_ops spt_pinctrl_pm_ops = {
 
 static struct platform_driver spt_pinctrl_driver = {
 	.probe = spt_pinctrl_probe,
-	.remove = intel_pinctrl_remove,
 	.driver = {
 		.name = "sunrisepoint-pinctrl",
 		.acpi_match_table = spt_pinctrl_acpi_match,
-- 
2.11.0

[PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

[Mika Westerberg]

When a GPIO driver is backed by a pinctrl driver the GPIO driver often
needs to call the pinctrl driver to configure certain things, like
whether the pin is used as input or output. In addition to this there
are other pin configurations applicable to GPIOs such as debounce
timeout.

To support this we introduce a new function pinctrl_gpio_set_config()
that can be used by gpiolib based driver to pass configuration requests
to the backing pinctrl driver.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/pinctrl/core.c           | 28 ++++++++++++++++++++++++++++
 drivers/pinctrl/pinconf.c        | 12 ++++++++++++
 drivers/pinctrl/pinconf.h        | 10 ++++++++++
 include/linux/pinctrl/consumer.h |  8 ++++++++
 4 files changed, 58 insertions(+)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index fb38e208f32d..114b7a7bbaea 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -688,6 +688,34 @@ int pinctrl_gpio_direction_output(unsigned gpio)
 }
 EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output);
 
+/**
+ * pinctrl_gpio_set_config() - Apply configs to given GPIO pin
+ * @gpio: the GPIO pin number from the GPIO subsystem number space
+ *
+ * This function should *ONLY* be used from gpiolib-based GPIO drivers, if
+ * they need to call the underlying pin controller to change GPIO config
+ * (for example set debounce time).
+ */
+int pinctrl_gpio_set_config(unsigned gpio, unsigned long *configs,
+			    size_t nconfigs)
+{
+	struct pinctrl_gpio_range *range;
+	struct pinctrl_dev *pctldev;
+	int ret, pin;
+
+	ret = pinctrl_get_device_gpio_range(gpio, &pctldev, &range);
+	if (ret)
+		return ret;
+
+	mutex_lock(&pctldev->mutex);
+	pin = gpio_to_pin(range, gpio);
+	ret = pinconf_gpio_set_config(pctldev, pin, configs, nconfigs);
+	mutex_unlock(&pctldev->mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_set_config);
+
 static struct pinctrl_state *find_state(struct pinctrl *p,
 					const char *name)
 {
diff --git a/drivers/pinctrl/pinconf.c b/drivers/pinctrl/pinconf.c
index 799048f3c8d4..537c219a4f60 100644
--- a/drivers/pinctrl/pinconf.c
+++ b/drivers/pinctrl/pinconf.c
@@ -200,6 +200,18 @@ int pinconf_apply_setting(struct pinctrl_setting const *setting)
 	return 0;
 }
 
+int pinconf_gpio_set_config(struct pinctrl_dev *pctldev, unsigned pin,
+			    unsigned long *configs, size_t nconfigs)
+{
+	const struct pinconf_ops *ops;
+
+	ops = pctldev->desc->confops;
+	if (!ops)
+		return -EINVAL;
+
+	return ops->pin_config_set(pctldev, pin, configs, nconfigs);
+}
+
 #ifdef CONFIG_DEBUG_FS
 
 static void pinconf_show_config(struct seq_file *s, struct pinctrl_dev *pctldev,
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index 55c75780b3b2..6e94e101357f 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -20,6 +20,9 @@ int pinconf_map_to_setting(struct pinctrl_map const *map,
 void pinconf_free_setting(struct pinctrl_setting const *setting);
 int pinconf_apply_setting(struct pinctrl_setting const *setting);
 
+int pinconf_gpio_set_config(struct pinctrl_dev *pctldev, unsigned pin,
+			    unsigned long *configs, size_t nconfigs);
+
 /*
  * You will only be interested in these if you're using PINCONF
  * so don't supply any stubs for these.
@@ -56,6 +59,13 @@ static inline int pinconf_apply_setting(struct pinctrl_setting const *setting)
 	return 0;
 }
 
+static inline int pinconf_gpio_set_config(struct pinctrl_dev *pctldev,
+					  unsigned pin, unsigned long *configs,
+					  size_t nconfigs)
+{
+	return -ENXIO;
+}
+
 #endif
 
 #if defined(CONFIG_PINCONF) && defined(CONFIG_DEBUG_FS)
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index d7e5d608faa7..bf4dce0c04ea 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -29,6 +29,8 @@ extern int pinctrl_request_gpio(unsigned gpio);
 extern void pinctrl_free_gpio(unsigned gpio);
 extern int pinctrl_gpio_direction_input(unsigned gpio);
 extern int pinctrl_gpio_direction_output(unsigned gpio);
+extern int pinctrl_gpio_set_config(unsigned gpio, unsigned long *configs,
+				  size_t nconfigs);
 
 extern struct pinctrl * __must_check pinctrl_get(struct device *dev);
 extern void pinctrl_put(struct pinctrl *p);
@@ -80,6 +82,12 @@ static inline int pinctrl_gpio_direction_output(unsigned gpio)
 	return 0;
 }
 
+static inline int pinctrl_gpio_set_config(unsigned gpio, unsigned long *configs,
+					  size_t nconfigs)
+{
+	return 0;
+}
+
 static inline struct pinctrl * __must_check pinctrl_get(struct device *dev)
 {
 	return NULL;
-- 
2.11.0

[PATCH v2 4/6] pinctrl: intel: Add support for hardware debouncer

[Mika Westerberg]

The next generation Intel GPIO hardware has two additional registers
PADCFG2 and PADCFG3. The latter is marked as reserved but the former
includes configuration for per-pad hardware debouncer.

This patch adds support for that in the Intel pinctrl core driver. Since
these are additional features on top of the current generation hardware,
we use revision number and feature flags to enable this if detected.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 139 +++++++++++++++++++++++++++++++++-
 drivers/pinctrl/intel/pinctrl-intel.h |   5 ++
 2 files changed, 143 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 447405809340..2d8722fa701d 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -12,6 +12,7 @@
 
 #include <linux/module.h>
 #include <linux/interrupt.h>
+#include <linux/log2.h>
 #include <linux/gpio/driver.h>
 #include <linux/platform_device.h>
 #include <linux/pinctrl/pinctrl.h>
@@ -23,6 +24,10 @@
 #include "pinctrl-intel.h"
 
 /* Offset from regs */
+#define REVID				0x000
+#define REVID_SHIFT			16
+#define REVID_MASK			GENMASK(31, 16)
+
 #define PADBAR				0x00c
 #define GPI_IS				0x100
 #define GPI_GPE_STS			0x140
@@ -41,6 +46,7 @@
 #define PADCFG0_RXEVCFG_EDGE		1
 #define PADCFG0_RXEVCFG_DISABLED	2
 #define PADCFG0_RXEVCFG_EDGE_BOTH	3
+#define PADCFG0_PREGFRXSEL		BIT(24)
 #define PADCFG0_RXINV			BIT(23)
 #define PADCFG0_GPIROUTIOXAPIC		BIT(20)
 #define PADCFG0_GPIROUTSCI		BIT(19)
@@ -62,9 +68,17 @@
 #define PADCFG1_TERM_5K			2
 #define PADCFG1_TERM_1K			1
 
+#define PADCFG2				0x008
+#define PADCFG2_DEBEN			BIT(0)
+#define PADCFG2_DEBOUNCE_SHIFT		1
+#define PADCFG2_DEBOUNCE_MASK		GENMASK(4, 1)
+
+#define DEBOUNCE_PERIOD			31250 /* ns */
+
 struct intel_pad_context {
 	u32 padcfg0;
 	u32 padcfg1;
+	u32 padcfg2;
 };
 
 struct intel_community_context {
@@ -126,13 +140,19 @@ static void __iomem *intel_get_padcfg(struct intel_pinctrl *pctrl, unsigned pin,
 {
 	const struct intel_community *community;
 	unsigned padno;
+	size_t nregs;
 
 	community = intel_get_community(pctrl, pin);
 	if (!community)
 		return NULL;
 
 	padno = pin_to_padno(community, pin);
-	return community->pad_regs + reg + padno * 8;
+	nregs = (community->features & PINCTRL_FEATURE_DEBOUNCE) ? 4 : 2;
+
+	if (reg == PADCFG2 && !(community->features & PINCTRL_FEATURE_DEBOUNCE))
+		return NULL;
+
+	return community->pad_regs + reg + padno * nregs * 4;
 }
 
 static bool intel_pad_owned_by_host(struct intel_pinctrl *pctrl, unsigned pin)
@@ -244,6 +264,7 @@ static void intel_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 			       unsigned pin)
 {
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	void __iomem *padcfg;
 	u32 cfg0, cfg1, mode;
 	bool locked, acpi;
 
@@ -263,6 +284,11 @@ static void intel_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
 
 	seq_printf(s, "0x%08x 0x%08x", cfg0, cfg1);
 
+	/* Dump the additional PADCFG registers if available */
+	padcfg = intel_get_padcfg(pctrl, pin, PADCFG2);
+	if (padcfg)
+		seq_printf(s, " 0x%08x", readl(padcfg));
+
 	locked = intel_pad_locked(pctrl, pin);
 	acpi = intel_pad_acpi_mode(pctrl, pin);
 
@@ -475,6 +501,24 @@ static int intel_config_get(struct pinctrl_dev *pctldev, unsigned pin,
 
 		break;
 
+	case PIN_CONFIG_INPUT_DEBOUNCE: {
+		void __iomem *padcfg2;
+		u32 v;
+
+		padcfg2 = intel_get_padcfg(pctrl, pin, PADCFG2);
+		if (!padcfg2)
+			return -ENOTSUPP;
+
+		v = readl(padcfg2);
+		if (!(v & PADCFG2_DEBEN))
+			return -EINVAL;
+
+		v = (v & PADCFG2_DEBOUNCE_MASK) >> PADCFG2_DEBOUNCE_SHIFT;
+		arg = BIT(v) * DEBOUNCE_PERIOD / 1000;
+
+		break;
+	}
+
 	default:
 		return -ENOTSUPP;
 	}
@@ -552,6 +596,53 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned pin,
 	return ret;
 }
 
+static int intel_config_set_debounce(struct intel_pinctrl *pctrl, unsigned pin,
+				     unsigned debounce)
+{
+	void __iomem *padcfg0, *padcfg2;
+	unsigned long flags;
+	u32 value0, value2;
+	int ret = 0;
+
+	padcfg2 = intel_get_padcfg(pctrl, pin, PADCFG2);
+	if (!padcfg2)
+		return -ENOTSUPP;
+
+	padcfg0 = intel_get_padcfg(pctrl, pin, PADCFG0);
+
+	raw_spin_lock_irqsave(&pctrl->lock, flags);
+
+	value0 = readl(padcfg0);
+	value2 = readl(padcfg2);
+
+	/* Disable glitch filter and debouncer */
+	value0 &= ~PADCFG0_PREGFRXSEL;
+	value2 &= ~(PADCFG2_DEBEN | PADCFG2_DEBOUNCE_MASK);
+
+	if (debounce) {
+		unsigned long v;
+
+		v = order_base_2(debounce * 1000 / DEBOUNCE_PERIOD);
+		if (v < 3 || v > 15) {
+			ret = -EINVAL;
+			goto exit_unlock;
+		} else {
+			/* Enable glitch filter and debouncer */
+			value0 |= PADCFG0_PREGFRXSEL;
+			value2 |= v << PADCFG2_DEBOUNCE_SHIFT;
+			value2 |= PADCFG2_DEBEN;
+		}
+	}
+
+	writel(value0, padcfg0);
+	writel(value2, padcfg2);
+
+exit_unlock:
+	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+
+	return ret;
+}
+
 static int intel_config_set(struct pinctrl_dev *pctldev, unsigned pin,
 			  unsigned long *configs, unsigned nconfigs)
 {
@@ -571,6 +662,13 @@ static int intel_config_set(struct pinctrl_dev *pctldev, unsigned pin,
 				return ret;
 			break;
 
+		case PIN_CONFIG_INPUT_DEBOUNCE:
+			ret = intel_config_set_debounce(pctrl, pin,
+				pinconf_to_config_argument(configs[i]));
+			if (ret)
+				return ret;
+			break;
+
 		default:
 			return -ENOTSUPP;
 		}
@@ -637,6 +735,17 @@ static int intel_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
 	return pinctrl_gpio_direction_output(chip->base + offset);
 }
 
+static int intel_gpio_set_debounce(struct gpio_chip *chip, unsigned offset,
+				   unsigned debounce)
+{
+	unsigned long configs[] = {
+		PIN_CONF_PACKED(PIN_CONFIG_INPUT_DEBOUNCE, debounce),
+	};
+
+	return pinctrl_gpio_set_config(chip->base + offset, configs,
+				       ARRAY_SIZE(configs));
+}
+
 static const struct gpio_chip intel_gpio_chip = {
 	.owner = THIS_MODULE,
 	.request = gpiochip_generic_request,
@@ -645,6 +754,7 @@ static const struct gpio_chip intel_gpio_chip = {
 	.direction_output = intel_gpio_direction_output,
 	.get = intel_gpio_get,
 	.set = intel_gpio_set,
+	.set_debounce = intel_gpio_set_debounce,
 };
 
 static void intel_gpio_irq_ack(struct irq_data *d)
@@ -1000,6 +1110,18 @@ int intel_pinctrl_probe(struct platform_device *pdev,
 		if (IS_ERR(regs))
 			return PTR_ERR(regs);
 
+		/*
+		 * Determine community features based on the revision if
+		 * not specified already.
+		 */
+		if (!community->features) {
+			u32 rev;
+
+			rev = (readl(regs + REVID) & REVID_MASK) >> REVID_SHIFT;
+			if (rev >= 0x94)
+				community->features |= PINCTRL_FEATURE_DEBOUNCE;
+		}
+
 		/* Read offset of the pad configuration registers */
 		padbar = readl(regs + PADBAR);
 
@@ -1073,6 +1195,7 @@ int intel_pinctrl_suspend(struct device *dev)
 	pads = pctrl->context.pads;
 	for (i = 0; i < pctrl->soc->npins; i++) {
 		const struct pinctrl_pin_desc *desc = &pctrl->soc->pins[i];
+		void __iomem *padcfg;
 		u32 val;
 
 		if (!intel_pinctrl_should_save(pctrl, desc->number))
@@ -1082,6 +1205,10 @@ int intel_pinctrl_suspend(struct device *dev)
 		pads[i].padcfg0 = val & ~PADCFG0_GPIORXSTATE;
 		val = readl(intel_get_padcfg(pctrl, desc->number, PADCFG1));
 		pads[i].padcfg1 = val;
+
+		padcfg = intel_get_padcfg(pctrl, desc->number, PADCFG2);
+		if (padcfg)
+			pads[i].padcfg2 = readl(padcfg);
 	}
 
 	communities = pctrl->context.communities;
@@ -1154,6 +1281,16 @@ int intel_pinctrl_resume(struct device *dev)
 			dev_dbg(dev, "restored pin %u padcfg1 %#08x\n",
 				desc->number, readl(padcfg));
 		}
+
+		padcfg = intel_get_padcfg(pctrl, desc->number, PADCFG2);
+		if (padcfg) {
+			val = readl(padcfg);
+			if (val != pads[i].padcfg2) {
+				writel(pads[i].padcfg2, padcfg);
+				dev_dbg(dev, "restored pin %u padcfg2 %#08x\n",
+					desc->number, readl(padcfg));
+			}
+		}
 	}
 
 	communities = pctrl->context.communities;
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index c22c44485c5d..1ff5abf309e3 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -58,6 +58,7 @@ struct intel_function {
  * @gpp_size: Maximum number of pads in each group, such as PADCFGLOCK,
  *            HOSTSW_OWN,  GPI_IS, GPI_IE, etc.
  * @npins: Number of pins in this community
+ * @features: Additional features supported by the hardware
  * @regs: Community specific common registers (reserved for core driver)
  * @pad_regs: Community specific pad registers (reserved for core driver)
  * @ngpps: Number of groups (hw groups) in this community (reserved for
@@ -72,11 +73,15 @@ struct intel_community {
 	unsigned pin_base;
 	unsigned gpp_size;
 	size_t npins;
+	unsigned features;
 	void __iomem *regs;
 	void __iomem *pad_regs;
 	size_t ngpps;
 };
 
+/* Additional features supported by the hardware */
+#define PINCTRL_FEATURE_DEBOUNCE	BIT(0)
+
 #define PIN_GROUP(n, p, m)			\
 	{					\
 		.name = (n),			\
-- 
2.11.0

[PATCH v2 5/6] pinctrl: intel: Add support for 1k additional pull-down

[Mika Westerberg]

The next generation Intel GPIO hardware supports additional 1k pull-down
per-pad. Add support for this to the Intel core pinctrl driver.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/pinctrl/intel/pinctrl-intel.c | 18 +++++++++++++++++-
 drivers/pinctrl/intel/pinctrl-intel.h |  1 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index 2d8722fa701d..f6fcfd061b74 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -450,12 +450,14 @@ static int intel_config_get(struct pinctrl_dev *pctldev, unsigned pin,
 {
 	struct intel_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
 	enum pin_config_param param = pinconf_to_config_param(*config);
+	const struct intel_community *community;
 	u32 value, term;
 	u16 arg = 0;
 
 	if (!intel_pad_owned_by_host(pctrl, pin))
 		return -ENOTSUPP;
 
+	community = intel_get_community(pctrl, pin);
 	value = readl(intel_get_padcfg(pctrl, pin, PADCFG1));
 	term = (value & PADCFG1_TERM_MASK) >> PADCFG1_TERM_SHIFT;
 
@@ -491,6 +493,11 @@ static int intel_config_get(struct pinctrl_dev *pctldev, unsigned pin,
 			return -EINVAL;
 
 		switch (term) {
+		case PADCFG1_TERM_1K:
+			if (!(community->features & PINCTRL_FEATURE_1K_PD))
+				return -EINVAL;
+			arg = 1000;
+			break;
 		case PADCFG1_TERM_5K:
 			arg = 5000;
 			break;
@@ -532,6 +539,7 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned pin,
 {
 	unsigned param = pinconf_to_config_param(config);
 	unsigned arg = pinconf_to_config_argument(config);
+	const struct intel_community *community;
 	void __iomem *padcfg1;
 	unsigned long flags;
 	int ret = 0;
@@ -539,6 +547,7 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned pin,
 
 	raw_spin_lock_irqsave(&pctrl->lock, flags);
 
+	community = intel_get_community(pctrl, pin);
 	padcfg1 = intel_get_padcfg(pctrl, pin, PADCFG1);
 	value = readl(padcfg1);
 
@@ -581,6 +590,11 @@ static int intel_config_set_pull(struct intel_pinctrl *pctrl, unsigned pin,
 		case 5000:
 			value |= PADCFG1_TERM_5K << PADCFG1_TERM_SHIFT;
 			break;
+		case 1000:
+			if (!(community->features & PINCTRL_FEATURE_1K_PD))
+				return -EINVAL;
+			value |= PADCFG1_TERM_1K << PADCFG1_TERM_SHIFT;
+			break;
 		default:
 			ret = -EINVAL;
 		}
@@ -1118,8 +1132,10 @@ int intel_pinctrl_probe(struct platform_device *pdev,
 			u32 rev;
 
 			rev = (readl(regs + REVID) & REVID_MASK) >> REVID_SHIFT;
-			if (rev >= 0x94)
+			if (rev >= 0x94) {
 				community->features |= PINCTRL_FEATURE_DEBOUNCE;
+				community->features |= PINCTRL_FEATURE_1K_PD;
+			}
 		}
 
 		/* Read offset of the pad configuration registers */
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index 1ff5abf309e3..fe9521f345b5 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -81,6 +81,7 @@ struct intel_community {
 
 /* Additional features supported by the hardware */
 #define PINCTRL_FEATURE_DEBOUNCE	BIT(0)
+#define PINCTRL_FEATURE_1K_PD		BIT(1)
 
 #define PIN_GROUP(n, p, m)			\
 	{					\
-- 
2.11.0

[PATCH v2 6/6] pinctrl: intel: Add Intel Gemini Lake pin controller support

[Mika Westerberg]

This driver adds pinctrl/GPIO support for Intel Gemini Lake SoC. The
GPIO controller is based on the next generation GPIO hardware but still
compatible with the one supported by the Intel core pinctrl/GPIO driver.

This commit includes material from David E. Box.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/pinctrl/intel/Kconfig              |   8 +
 drivers/pinctrl/intel/Makefile             |   1 +
 drivers/pinctrl/intel/pinctrl-geminilake.c | 512 +++++++++++++++++++++++++++++
 3 files changed, 521 insertions(+)
 create mode 100644 drivers/pinctrl/intel/pinctrl-geminilake.c

diff --git a/drivers/pinctrl/intel/Kconfig b/drivers/pinctrl/intel/Kconfig
index 00fb055a4897..396830a41127 100644
--- a/drivers/pinctrl/intel/Kconfig
+++ b/drivers/pinctrl/intel/Kconfig
@@ -56,6 +56,14 @@ config PINCTRL_BROXTON
 	  Broxton pinctrl driver provides an interface that allows
 	  configuring of SoC pins and using them as GPIOs.
 
+config PINCTRL_GEMINILAKE
+	tristate "Intel Gemini Lake SoC pinctrl and GPIO driver"
+	depends on ACPI
+	select PINCTRL_INTEL
+	help
+	  This pinctrl driver provides an interface that allows configuring
+	  of Intel Gemini Lake SoC pins and using them as GPIOs.
+
 config PINCTRL_SUNRISEPOINT
 	tristate "Intel Sunrisepoint pinctrl and GPIO driver"
 	depends on ACPI
diff --git a/drivers/pinctrl/intel/Makefile b/drivers/pinctrl/intel/Makefile
index 30803078f09e..12f3af5b2ca5 100644
--- a/drivers/pinctrl/intel/Makefile
+++ b/drivers/pinctrl/intel/Makefile
@@ -5,4 +5,5 @@ obj-$(CONFIG_PINCTRL_CHERRYVIEW)	+= pinctrl-cherryview.o
 obj-$(CONFIG_PINCTRL_MERRIFIELD)	+= pinctrl-merrifield.o
 obj-$(CONFIG_PINCTRL_INTEL)		+= pinctrl-intel.o
 obj-$(CONFIG_PINCTRL_BROXTON)		+= pinctrl-broxton.o
+obj-$(CONFIG_PINCTRL_GEMINILAKE)	+= pinctrl-geminilake.o
 obj-$(CONFIG_PINCTRL_SUNRISEPOINT)	+= pinctrl-sunrisepoint.o
diff --git a/drivers/pinctrl/intel/pinctrl-geminilake.c b/drivers/pinctrl/intel/pinctrl-geminilake.c
new file mode 100644
index 000000000000..a6b94c930007
--- /dev/null
+++ b/drivers/pinctrl/intel/pinctrl-geminilake.c
@@ -0,0 +1,512 @@
+/*
+ * Intel Gemini Lake SoC pinctrl/GPIO driver
+ *
+ * Copyright (C) 2017 Intel Corporation
+ * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-intel.h"
+
+#define GLK_PAD_OWN	0x020
+#define GLK_HOSTSW_OWN	0x0b0
+#define GLK_PADCFGLOCK	0x080
+#define GLK_GPI_IE	0x110
+
+#define GLK_COMMUNITY(s, e)				\
+	{						\
+		.padown_offset = GLK_PAD_OWN,		\
+		.padcfglock_offset = GLK_PADCFGLOCK,	\
+		.hostown_offset = GLK_HOSTSW_OWN,	\
+		.ie_offset = GLK_GPI_IE,		\
+		.gpp_size = 32,                         \
+		.pin_base = (s),			\
+		.npins = ((e) - (s) + 1),		\
+	}
+
+/* GLK */
+static const struct pinctrl_pin_desc glk_northwest_pins[] = {
+	PINCTRL_PIN(0, "TCK"),
+	PINCTRL_PIN(1, "TRST_B"),
+	PINCTRL_PIN(2, "TMS"),
+	PINCTRL_PIN(3, "TDI"),
+	PINCTRL_PIN(4, "TDO"),
+	PINCTRL_PIN(5, "JTAGX"),
+	PINCTRL_PIN(6, "CX_PREQ_B"),
+	PINCTRL_PIN(7, "CX_PRDY_B"),
+	PINCTRL_PIN(8, "GPIO_8"),
+	PINCTRL_PIN(9, "GPIO_9"),
+	PINCTRL_PIN(10, "GPIO_10"),
+	PINCTRL_PIN(11, "GPIO_11"),
+	PINCTRL_PIN(12, "GPIO_12"),
+	PINCTRL_PIN(13, "GPIO_13"),
+	PINCTRL_PIN(14, "GPIO_14"),
+	PINCTRL_PIN(15, "GPIO_15"),
+	PINCTRL_PIN(16, "GPIO_16"),
+	PINCTRL_PIN(17, "GPIO_17"),
+	PINCTRL_PIN(18, "GPIO_18"),
+	PINCTRL_PIN(19, "GPIO_19"),
+	PINCTRL_PIN(20, "GPIO_20"),
+	PINCTRL_PIN(21, "GPIO_21"),
+	PINCTRL_PIN(22, "GPIO_22"),
+	PINCTRL_PIN(23, "GPIO_23"),
+	PINCTRL_PIN(24, "GPIO_24"),
+	PINCTRL_PIN(25, "GPIO_25"),
+	PINCTRL_PIN(26, "GPIO_26"),
+	PINCTRL_PIN(27, "GPIO_27"),
+	PINCTRL_PIN(28, "GPIO_28"),
+	PINCTRL_PIN(29, "GPIO_29"),
+	PINCTRL_PIN(30, "GPIO_30"),
+	PINCTRL_PIN(31, "GPIO_31"),
+	PINCTRL_PIN(32, "GPIO_32"),
+	PINCTRL_PIN(33, "GPIO_33"),
+	PINCTRL_PIN(34, "GPIO_34"),
+	PINCTRL_PIN(35, "GPIO_35"),
+	PINCTRL_PIN(36, "GPIO_36"),
+	PINCTRL_PIN(37, "GPIO_37"),
+	PINCTRL_PIN(38, "GPIO_38"),
+	PINCTRL_PIN(39, "GPIO_39"),
+	PINCTRL_PIN(40, "GPIO_40"),
+	PINCTRL_PIN(41, "GPIO_41"),
+	PINCTRL_PIN(42, "GP_INTD_DSI_TE1"),
+	PINCTRL_PIN(43, "GP_INTD_DSI_TE2"),
+	PINCTRL_PIN(44, "USB_OC0_B"),
+	PINCTRL_PIN(45, "USB_OC1_B"),
+	PINCTRL_PIN(46, "DSI_I2C_SDA"),
+	PINCTRL_PIN(47, "DSI_I2C_SCL"),
+	PINCTRL_PIN(48, "PMC_I2C_SDA"),
+	PINCTRL_PIN(49, "PMC_I2C_SCL"),
+	PINCTRL_PIN(50, "LPSS_I2C0_SDA"),
+	PINCTRL_PIN(51, "LPSS_I2C0_SCL"),
+	PINCTRL_PIN(52, "LPSS_I2C1_SDA"),
+	PINCTRL_PIN(53, "LPSS_I2C1_SCL"),
+	PINCTRL_PIN(54, "LPSS_I2C2_SDA"),
+	PINCTRL_PIN(55, "LPSS_I2C2_SCL"),
+	PINCTRL_PIN(56, "LPSS_I2C3_SDA"),
+	PINCTRL_PIN(57, "LPSS_I2C3_SCL"),
+	PINCTRL_PIN(58, "LPSS_I2C4_SDA"),
+	PINCTRL_PIN(59, "LPSS_I2C4_SCL"),
+	PINCTRL_PIN(60, "LPSS_UART0_RXD"),
+	PINCTRL_PIN(61, "LPSS_UART0_TXD"),
+	PINCTRL_PIN(62, "LPSS_UART0_RTS_B"),
+	PINCTRL_PIN(63, "LPSS_UART0_CTS_B"),
+	PINCTRL_PIN(64, "LPSS_UART2_RXD"),
+	PINCTRL_PIN(65, "LPSS_UART2_TXD"),
+	PINCTRL_PIN(66, "LPSS_UART2_RTS_B"),
+	PINCTRL_PIN(67, "LPSS_UART2_CTS_B"),
+	PINCTRL_PIN(68, "PMC_SPI_FS0"),
+	PINCTRL_PIN(69, "PMC_SPI_FS1"),
+	PINCTRL_PIN(70, "PMC_SPI_FS2"),
+	PINCTRL_PIN(71, "PMC_SPI_RXD"),
+	PINCTRL_PIN(72, "PMC_SPI_TXD"),
+	PINCTRL_PIN(73, "PMC_SPI_CLK"),
+	PINCTRL_PIN(74, "THERMTRIP_B"),
+	PINCTRL_PIN(75, "PROCHOT_B"),
+	PINCTRL_PIN(76, "EMMC_RST_B"),
+	PINCTRL_PIN(77, "GPIO_212"),
+	PINCTRL_PIN(78, "GPIO_213"),
+	PINCTRL_PIN(79, "GPIO_214"),
+};
+
+static const unsigned int glk_northwest_uart1_pins[] = { 26, 27, 28, 29 };
+static const unsigned int glk_northwest_pwm0_pins[] = { 42 };
+static const unsigned int glk_northwest_pwm1_pins[] = { 43 };
+static const unsigned int glk_northwest_pwm2_pins[] = { 44 };
+static const unsigned int glk_northwest_pwm3_pins[] = { 45 };
+static const unsigned int glk_northwest_i2c0_pins[] = { 50, 51 };
+static const unsigned int glk_northwest_i2c1_pins[] = { 52, 53 };
+static const unsigned int glk_northwest_i2c2_pins[] = { 54, 55 };
+static const unsigned int glk_northwest_i2c3_pins[] = { 56, 57 };
+static const unsigned int glk_northwest_i2c4_pins[] = { 58, 59 };
+static const unsigned int glk_northwest_uart0_pins[] = { 60, 61, 62, 63 };
+static const unsigned int glk_northwest_uart2_pins[] = { 64, 65, 66, 67 };
+
+static const struct intel_pingroup glk_northwest_groups[] = {
+	PIN_GROUP("uart1_grp", glk_northwest_uart1_pins, 2),
+	PIN_GROUP("pwm0_grp", glk_northwest_pwm0_pins, 2),
+	PIN_GROUP("pwm1_grp", glk_northwest_pwm1_pins, 2),
+	PIN_GROUP("pwm2_grp", glk_northwest_pwm2_pins, 2),
+	PIN_GROUP("pwm3_grp", glk_northwest_pwm3_pins, 2),
+	PIN_GROUP("i2c0_grp", glk_northwest_i2c0_pins, 1),
+	PIN_GROUP("i2c1_grp", glk_northwest_i2c1_pins, 1),
+	PIN_GROUP("i2c2_grp", glk_northwest_i2c2_pins, 1),
+	PIN_GROUP("i2c3_grp", glk_northwest_i2c3_pins, 1),
+	PIN_GROUP("i2c4_grp", glk_northwest_i2c4_pins, 1),
+	PIN_GROUP("uart0_grp", glk_northwest_uart0_pins, 1),
+	PIN_GROUP("uart2_grp", glk_northwest_uart2_pins, 1),
+};
+
+static const char * const glk_northwest_uart1_groups[] = { "uart1_grp" };
+static const char * const glk_northwest_pwm0_groups[] = { "pwm0_grp" };
+static const char * const glk_northwest_pwm1_groups[] = { "pwm1_grp" };
+static const char * const glk_northwest_pwm2_groups[] = { "pwm2_grp" };
+static const char * const glk_northwest_pwm3_groups[] = { "pwm3_grp" };
+static const char * const glk_northwest_i2c0_groups[] = { "i2c0_grp" };
+static const char * const glk_northwest_i2c1_groups[] = { "i2c1_grp" };
+static const char * const glk_northwest_i2c2_groups[] = { "i2c2_grp" };
+static const char * const glk_northwest_i2c3_groups[] = { "i2c3_grp" };
+static const char * const glk_northwest_i2c4_groups[] = { "i2c4_grp" };
+static const char * const glk_northwest_uart0_groups[] = { "uart0_grp" };
+static const char * const glk_northwest_uart2_groups[] = { "uart2_grp" };
+
+static const struct intel_function glk_northwest_functions[] = {
+	FUNCTION("uart1", glk_northwest_uart1_groups),
+	FUNCTION("pmw0", glk_northwest_pwm0_groups),
+	FUNCTION("pmw1", glk_northwest_pwm1_groups),
+	FUNCTION("pmw2", glk_northwest_pwm2_groups),
+	FUNCTION("pmw3", glk_northwest_pwm3_groups),
+	FUNCTION("i2c0", glk_northwest_i2c0_groups),
+	FUNCTION("i2c1", glk_northwest_i2c1_groups),
+	FUNCTION("i2c2", glk_northwest_i2c2_groups),
+	FUNCTION("i2c3", glk_northwest_i2c3_groups),
+	FUNCTION("i2c4", glk_northwest_i2c4_groups),
+	FUNCTION("uart0", glk_northwest_uart0_groups),
+	FUNCTION("uart2", glk_northwest_uart2_groups),
+};
+
+static const struct intel_community glk_northwest_communities[] = {
+	GLK_COMMUNITY(0, 79),
+};
+
+static const struct intel_pinctrl_soc_data glk_northwest_soc_data = {
+	.uid = "1",
+	.pins = glk_northwest_pins,
+	.npins = ARRAY_SIZE(glk_northwest_pins),
+	.groups = glk_northwest_groups,
+	.ngroups = ARRAY_SIZE(glk_northwest_groups),
+	.functions = glk_northwest_functions,
+	.nfunctions = ARRAY_SIZE(glk_northwest_functions),
+	.communities = glk_northwest_communities,
+	.ncommunities = ARRAY_SIZE(glk_northwest_communities),
+};
+
+static const struct pinctrl_pin_desc glk_north_pins[] = {
+	PINCTRL_PIN(0, "SVID0_ALERT_B"),
+	PINCTRL_PIN(1, "SVID0_DATA"),
+	PINCTRL_PIN(2, "SVID0_CLK"),
+	PINCTRL_PIN(3, "LPSS_SPI_0_CLK"),
+	PINCTRL_PIN(4, "LPSS_SPI_0_FS0"),
+	PINCTRL_PIN(5, "LPSS_SPI_0_FS1"),
+	PINCTRL_PIN(6, "LPSS_SPI_0_RXD"),
+	PINCTRL_PIN(7, "LPSS_SPI_0_TXD"),
+	PINCTRL_PIN(8, "LPSS_SPI_1_CLK"),
+	PINCTRL_PIN(9, "LPSS_SPI_1_FS0"),
+	PINCTRL_PIN(10, "LPSS_SPI_1_FS1"),
+	PINCTRL_PIN(11, "LPSS_SPI_1_FS2"),
+	PINCTRL_PIN(12, "LPSS_SPI_1_RXD"),
+	PINCTRL_PIN(13, "LPSS_SPI_1_TXD"),
+	PINCTRL_PIN(14, "FST_SPI_CS0_B"),
+	PINCTRL_PIN(15, "FST_SPI_CS1_B"),
+	PINCTRL_PIN(16, "FST_SPI_MOSI_IO0"),
+	PINCTRL_PIN(17, "FST_SPI_MISO_IO1"),
+	PINCTRL_PIN(18, "FST_SPI_IO2"),
+	PINCTRL_PIN(19, "FST_SPI_IO3"),
+	PINCTRL_PIN(20, "FST_SPI_CLK"),
+	PINCTRL_PIN(21, "FST_SPI_CLK_FB"),
+	PINCTRL_PIN(22, "PMU_PLTRST_B"),
+	PINCTRL_PIN(23, "PMU_PWRBTN_B"),
+	PINCTRL_PIN(24, "PMU_SLP_S0_B"),
+	PINCTRL_PIN(25, "PMU_SLP_S3_B"),
+	PINCTRL_PIN(26, "PMU_SLP_S4_B"),
+	PINCTRL_PIN(27, "SUSPWRDNACK"),
+	PINCTRL_PIN(28, "EMMC_PWR_EN_B"),
+	PINCTRL_PIN(29, "PMU_AC_PRESENT"),
+	PINCTRL_PIN(30, "PMU_BATLOW_B"),
+	PINCTRL_PIN(31, "PMU_RESETBUTTON_B"),
+	PINCTRL_PIN(32, "PMU_SUSCLK"),
+	PINCTRL_PIN(33, "SUS_STAT_B"),
+	PINCTRL_PIN(34, "LPSS_I2C5_SDA"),
+	PINCTRL_PIN(35, "LPSS_I2C5_SCL"),
+	PINCTRL_PIN(36, "LPSS_I2C6_SDA"),
+	PINCTRL_PIN(37, "LPSS_I2C6_SCL"),
+	PINCTRL_PIN(38, "LPSS_I2C7_SDA"),
+	PINCTRL_PIN(39, "LPSS_I2C7_SCL"),
+	PINCTRL_PIN(40, "PCIE_WAKE0_B"),
+	PINCTRL_PIN(41, "PCIE_WAKE1_B"),
+	PINCTRL_PIN(42, "PCIE_WAKE2_B"),
+	PINCTRL_PIN(43, "PCIE_WAKE3_B"),
+	PINCTRL_PIN(44, "PCIE_CLKREQ0_B"),
+	PINCTRL_PIN(45, "PCIE_CLKREQ1_B"),
+	PINCTRL_PIN(46, "PCIE_CLKREQ2_B"),
+	PINCTRL_PIN(47, "PCIE_CLKREQ3_B"),
+	PINCTRL_PIN(48, "HV_DDI0_DDC_SDA"),
+	PINCTRL_PIN(49, "HV_DDI0_DDC_SCL"),
+	PINCTRL_PIN(50, "HV_DDI1_DDC_SDA"),
+	PINCTRL_PIN(51, "HV_DDI1_DDC_SCL"),
+	PINCTRL_PIN(52, "PANEL0_VDDEN"),
+	PINCTRL_PIN(53, "PANEL0_BKLTEN"),
+	PINCTRL_PIN(54, "PANEL0_BKLTCTL"),
+	PINCTRL_PIN(55, "HV_DDI0_HPD"),
+	PINCTRL_PIN(56, "HV_DDI1_HPD"),
+	PINCTRL_PIN(57, "HV_EDP_HPD"),
+	PINCTRL_PIN(58, "GPIO_134"),
+	PINCTRL_PIN(59, "GPIO_135"),
+	PINCTRL_PIN(60, "GPIO_136"),
+	PINCTRL_PIN(61, "GPIO_137"),
+	PINCTRL_PIN(62, "GPIO_138"),
+	PINCTRL_PIN(63, "GPIO_139"),
+	PINCTRL_PIN(64, "GPIO_140"),
+	PINCTRL_PIN(65, "GPIO_141"),
+	PINCTRL_PIN(66, "GPIO_142"),
+	PINCTRL_PIN(67, "GPIO_143"),
+	PINCTRL_PIN(68, "GPIO_144"),
+	PINCTRL_PIN(69, "GPIO_145"),
+	PINCTRL_PIN(70, "GPIO_146"),
+	PINCTRL_PIN(71, "LPC_ILB_SERIRQ"),
+	PINCTRL_PIN(72, "LPC_CLKOUT0"),
+	PINCTRL_PIN(73, "LPC_CLKOUT1"),
+	PINCTRL_PIN(74, "LPC_AD0"),
+	PINCTRL_PIN(75, "LPC_AD1"),
+	PINCTRL_PIN(76, "LPC_AD2"),
+	PINCTRL_PIN(77, "LPC_AD3"),
+	PINCTRL_PIN(78, "LPC_CLKRUNB"),
+	PINCTRL_PIN(79, "LPC_FRAMEB"),
+};
+
+static const unsigned int glk_north_spi0_pins[] = { 3, 4, 5, 6, 7 };
+static const unsigned int glk_north_spi1_pins[] = { 8, 9, 10, 11, 12, 13 };
+static const unsigned int glk_north_i2c5_pins[] = { 34, 35 };
+static const unsigned int glk_north_i2c6_pins[] = { 36, 37 };
+static const unsigned int glk_north_i2c7_pins[] = { 38, 39 };
+static const unsigned int glk_north_uart0_pins[] = { 62, 63, 64, 65 };
+static const unsigned int glk_north_spi0b_pins[] = { 66, 67, 68, 69, 70 };
+
+static const struct intel_pingroup glk_north_groups[] = {
+	PIN_GROUP("spi0_grp", glk_north_spi0_pins, 1),
+	PIN_GROUP("spi1_grp", glk_north_spi1_pins, 1),
+	PIN_GROUP("i2c5_grp", glk_north_i2c5_pins, 1),
+	PIN_GROUP("i2c6_grp", glk_north_i2c6_pins, 1),
+	PIN_GROUP("i2c7_grp", glk_north_i2c7_pins, 1),
+	PIN_GROUP("uart0_grp", glk_north_uart0_pins, 2),
+	PIN_GROUP("spi0b_grp", glk_north_spi0b_pins, 2),
+};
+
+static const char * const glk_north_spi0_groups[] = { "spi0_grp", "spi0b_grp" };
+static const char * const glk_north_spi1_groups[] = { "spi1_grp" };
+static const char * const glk_north_i2c5_groups[] = { "i2c5_grp" };
+static const char * const glk_north_i2c6_groups[] = { "i2c6_grp" };
+static const char * const glk_north_i2c7_groups[] = { "i2c7_grp" };
+static const char * const glk_north_uart0_groups[] = { "uart0_grp" };
+
+static const struct intel_function glk_north_functions[] = {
+	FUNCTION("spi0", glk_north_spi0_groups),
+	FUNCTION("spi1", glk_north_spi1_groups),
+	FUNCTION("i2c5", glk_north_i2c5_groups),
+	FUNCTION("i2c6", glk_north_i2c6_groups),
+	FUNCTION("i2c7", glk_north_i2c7_groups),
+	FUNCTION("uart0", glk_north_uart0_groups),
+};
+
+static const struct intel_community glk_north_communities[] = {
+	GLK_COMMUNITY(0, 79),
+};
+
+static const struct intel_pinctrl_soc_data glk_north_soc_data = {
+	.uid = "2",
+	.pins = glk_north_pins,
+	.npins = ARRAY_SIZE(glk_north_pins),
+	.groups = glk_north_groups,
+	.ngroups = ARRAY_SIZE(glk_north_groups),
+	.functions = glk_north_functions,
+	.nfunctions = ARRAY_SIZE(glk_north_functions),
+	.communities = glk_north_communities,
+	.ncommunities = ARRAY_SIZE(glk_north_communities),
+};
+
+static const struct pinctrl_pin_desc glk_audio_pins[] = {
+	PINCTRL_PIN(0, "AVS_I2S0_MCLK"),
+	PINCTRL_PIN(1, "AVS_I2S0_BCLK"),
+	PINCTRL_PIN(2, "AVS_I2S0_WS_SYNC"),
+	PINCTRL_PIN(3, "AVS_I2S0_SDI"),
+	PINCTRL_PIN(4, "AVS_I2S0_SDO"),
+	PINCTRL_PIN(5, "AVS_I2S1_MCLK"),
+	PINCTRL_PIN(6, "AVS_I2S1_BCLK"),
+	PINCTRL_PIN(7, "AVS_I2S1_WS_SYNC"),
+	PINCTRL_PIN(8, "AVS_I2S1_SDI"),
+	PINCTRL_PIN(9, "AVS_I2S1_SDO"),
+	PINCTRL_PIN(10, "AVS_HDA_BCLK"),
+	PINCTRL_PIN(11, "AVS_HDA_WS_SYNC"),
+	PINCTRL_PIN(12, "AVS_HDA_SDI"),
+	PINCTRL_PIN(13, "AVS_HDA_SDO"),
+	PINCTRL_PIN(14, "AVS_HDA_RSTB"),
+	PINCTRL_PIN(15, "AVS_M_CLK_A1"),
+	PINCTRL_PIN(16, "AVS_M_CLK_B1"),
+	PINCTRL_PIN(17, "AVS_M_DATA_1"),
+	PINCTRL_PIN(18, "AVS_M_CLK_AB2"),
+	PINCTRL_PIN(19, "AVS_M_DATA_2"),
+};
+
+static const struct intel_community glk_audio_communities[] = {
+	GLK_COMMUNITY(0, 19),
+};
+
+static const struct intel_pinctrl_soc_data glk_audio_soc_data = {
+	.uid = "3",
+	.pins = glk_audio_pins,
+	.npins = ARRAY_SIZE(glk_audio_pins),
+	.communities = glk_audio_communities,
+	.ncommunities = ARRAY_SIZE(glk_audio_communities),
+};
+
+static const struct pinctrl_pin_desc glk_scc_pins[] = {
+	PINCTRL_PIN(0, "SMB_ALERTB"),
+	PINCTRL_PIN(1, "SMB_CLK"),
+	PINCTRL_PIN(2, "SMB_DATA"),
+	PINCTRL_PIN(3, "SDCARD_LVL_WP"),
+	PINCTRL_PIN(4, "SDCARD_CLK"),
+	PINCTRL_PIN(5, "SDCARD_CLK_FB"),
+	PINCTRL_PIN(6, "SDCARD_D0"),
+	PINCTRL_PIN(7, "SDCARD_D1"),
+	PINCTRL_PIN(8, "SDCARD_D2"),
+	PINCTRL_PIN(9, "SDCARD_D3"),
+	PINCTRL_PIN(10, "SDCARD_CMD"),
+	PINCTRL_PIN(11, "SDCARD_CD_B"),
+	PINCTRL_PIN(12, "SDCARD_PWR_DOWN_B"),
+	PINCTRL_PIN(13, "GPIO_210"),
+	PINCTRL_PIN(14, "OSC_CLK_OUT_0"),
+	PINCTRL_PIN(15, "OSC_CLK_OUT_1"),
+	PINCTRL_PIN(16, "CNV_BRI_DT"),
+	PINCTRL_PIN(17, "CNV_BRI_RSP"),
+	PINCTRL_PIN(18, "CNV_RGI_DT"),
+	PINCTRL_PIN(19, "CNV_RGI_RSP"),
+	PINCTRL_PIN(20, "CNV_RF_RESET_B"),
+	PINCTRL_PIN(21, "XTAL_CLKREQ"),
+	PINCTRL_PIN(22, "SDIO_CLK_FB"),
+	PINCTRL_PIN(23, "EMMC0_CLK"),
+	PINCTRL_PIN(24, "EMMC0_CLK_FB"),
+	PINCTRL_PIN(25, "EMMC0_D0"),
+	PINCTRL_PIN(26, "EMMC0_D1"),
+	PINCTRL_PIN(27, "EMMC0_D2"),
+	PINCTRL_PIN(28, "EMMC0_D3"),
+	PINCTRL_PIN(29, "EMMC0_D4"),
+	PINCTRL_PIN(30, "EMMC0_D5"),
+	PINCTRL_PIN(31, "EMMC0_D6"),
+	PINCTRL_PIN(32, "EMMC0_D7"),
+	PINCTRL_PIN(33, "EMMC0_CMD"),
+	PINCTRL_PIN(34, "EMMC0_STROBE"),
+};
+
+static const unsigned int glk_scc_i2c7_pins[] = { 1, 2 };
+static const unsigned int glk_scc_sdcard_pins[] = {
+	3, 4, 5, 6, 7, 8, 9, 10, 11, 12,
+};
+static const unsigned int glk_scc_sdio_pins[] = { 16, 17, 18, 19, 20, 21, 22 };
+static const unsigned int glk_scc_uart1_pins[] = { 16, 17, 18, 19 };
+static const unsigned int glk_scc_emmc_pins[] = {
+	23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34,
+};
+
+static const struct intel_pingroup glk_scc_groups[] = {
+	PIN_GROUP("i2c7_grp", glk_scc_i2c7_pins, 2),
+	PIN_GROUP("sdcard_grp", glk_scc_sdcard_pins, 1),
+	PIN_GROUP("sdio_grp", glk_scc_sdio_pins, 2),
+	PIN_GROUP("uart1_grp", glk_scc_uart1_pins, 3),
+	PIN_GROUP("emmc_grp", glk_scc_emmc_pins, 1),
+};
+
+static const char * const glk_scc_i2c7_groups[] = { "i2c7_grp" };
+static const char * const glk_scc_sdcard_groups[] = { "sdcard_grp" };
+static const char * const glk_scc_sdio_groups[] = { "sdio_grp" };
+static const char * const glk_scc_uart1_groups[] = { "uart1_grp" };
+static const char * const glk_scc_emmc_groups[] = { "emmc_grp" };
+
+static const struct intel_function glk_scc_functions[] = {
+	FUNCTION("i2c7", glk_scc_i2c7_groups),
+	FUNCTION("sdcard", glk_scc_sdcard_groups),
+	FUNCTION("sdio", glk_scc_sdio_groups),
+	FUNCTION("uart1", glk_scc_uart1_groups),
+	FUNCTION("emmc", glk_scc_emmc_groups),
+};
+
+static const struct intel_community glk_scc_communities[] = {
+	GLK_COMMUNITY(0, 34),
+};
+
+static const struct intel_pinctrl_soc_data glk_scc_soc_data = {
+	.uid = "4",
+	.pins = glk_scc_pins,
+	.npins = ARRAY_SIZE(glk_scc_pins),
+	.groups = glk_scc_groups,
+	.ngroups = ARRAY_SIZE(glk_scc_groups),
+	.functions = glk_scc_functions,
+	.nfunctions = ARRAY_SIZE(glk_scc_functions),
+	.communities = glk_scc_communities,
+	.ncommunities = ARRAY_SIZE(glk_scc_communities),
+};
+
+static const struct intel_pinctrl_soc_data *glk_pinctrl_soc_data[] = {
+	&glk_northwest_soc_data,
+	&glk_north_soc_data,
+	&glk_audio_soc_data,
+	&glk_scc_soc_data,
+	NULL,
+};
+
+static const struct acpi_device_id glk_pinctrl_acpi_match[] = {
+	{ "INT3453" },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, glk_pinctrl_acpi_match);
+
+static int glk_pinctrl_probe(struct platform_device *pdev)
+{
+	const struct intel_pinctrl_soc_data *soc_data = NULL;
+	struct acpi_device *adev;
+	int i;
+
+	adev = ACPI_COMPANION(&pdev->dev);
+	if (!adev)
+		return -ENODEV;
+
+	for (i = 0; glk_pinctrl_soc_data[i]; i++) {
+		if (!strcmp(adev->pnp.unique_id,
+			    glk_pinctrl_soc_data[i]->uid)) {
+			soc_data = glk_pinctrl_soc_data[i];
+			break;
+		}
+	}
+
+	if (!soc_data)
+		return -ENODEV;
+
+	return intel_pinctrl_probe(pdev, soc_data);
+}
+
+static const struct dev_pm_ops glk_pinctrl_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_pinctrl_suspend,
+				     intel_pinctrl_resume)
+};
+
+static struct platform_driver glk_pinctrl_driver = {
+	.probe = glk_pinctrl_probe,
+	.driver = {
+		.name = "geminilake-pinctrl",
+		.acpi_match_table = glk_pinctrl_acpi_match,
+		.pm = &glk_pinctrl_pm_ops,
+	},
+};
+
+static int __init glk_pinctrl_init(void)
+{
+	return platform_driver_register(&glk_pinctrl_driver);
+}
+subsys_initcall(glk_pinctrl_init);
+
+static void __exit glk_pinctrl_exit(void)
+{
+	platform_driver_unregister(&glk_pinctrl_driver);
+}
+module_exit(glk_pinctrl_exit);
+
+MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
+MODULE_DESCRIPTION("Intel Gemini Lake SoC pinctrl/GPIO driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

Re: [PATCH v2 1/6] pinctrl: broxton: Use correct PADCFGLOCK offset

[Linus Walleij]

On Tue, Jan 10, 2017 at 3:31 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> PADCFGLOCK (and PADCFGLOCK_TX) offset in Broxton actually starts at 0x060
> and not 0x090 as used in the driver. Fix it to use the correct offset.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

This patch applied for fixes.

Yours,
Linus Walleij

Re: [PATCH v2 2/6] pinctrl: intel: Convert to use devm_gpiochip_add_data()

[Linus Walleij]

On Tue, Jan 10, 2017 at 3:31 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> This simplifies error handling and allows us to drop intel_pinctrl_remove()
> completely.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Patch applied for next.

Yours,
Linus Walleij

Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

[Linus Walleij]

On Tue, Jan 10, 2017 at 3:31 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> When a GPIO driver is backed by a pinctrl driver the GPIO driver often
> needs to call the pinctrl driver to configure certain things, like
> whether the pin is used as input or output. In addition to this there
> are other pin configurations applicable to GPIOs such as debounce
> timeout.
>
> To support this we introduce a new function pinctrl_gpio_set_config()
> that can be used by gpiolib based driver to pass configuration requests
> to the backing pinctrl driver.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

OK so this is needed.

But let's first pause and discuss this, because I have some stuff on my
mind here.

First this kernel-internal ABI from <linux/gpio/driver.h>:

struct gpio_chip {
(...)
        int                     (*set_debounce)(struct gpio_chip *chip,
                                                unsigned offset,
                                                unsigned debounce);
        int                     (*set_single_ended)(struct gpio_chip *chip,
                                                unsigned offset,
                                                enum single_ended_mode mode);
(...)

It's not going to scale. We need to replace this with something like

int (*set_config)(struct gpio_chip *chip, unsigned offset, unsigned
long config);

Where "config" takes the packed format described in
<linux/pinctrl/pinconf-generic.h>
and nothing else, anything else is just inviting disaster.

We can also later add:

int (*get_config)(struct gpio_chip *chip, unsigned offset, unsigned
long *config);

We can then  set and get arbitrary configs on GPIO lines, and the
drivers can simply implement a switch() for the configs they handle
else return -ENOTSUPP.

But right now only set_config() would be enough.

Maybe stuff needs to be split out of that header to be shared between
GPIO and pinctrl but hopefully you could just include it.

Then we change all in-kernel users of these two APIs over to set_config().

THEN we can think about cross-calling to pin control using the API
from this patch. It should be a simple matter of just passing along the
same config argument since we're using generic pin config.

It's not like it's impossible to merge this patch first, but I want to get some
order here.

Are you convenient with doing the above patch as part of this series, or
shall I do it first so you can rebase on it? (Will take some time if I
do it...)

We need this because GPIO is going to need more and more config
to be done by pinctrl on its behalf, and it will have to go all the
way to userspace in many cases, so we need this infrastructure in
place.

Yours,
Linus Walleij

Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

[Mika Westerberg]

On Wed, Jan 11, 2017 at 02:06:56PM +0100, Linus Walleij wrote:
> On Tue, Jan 10, 2017 at 3:31 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > When a GPIO driver is backed by a pinctrl driver the GPIO driver often
> > needs to call the pinctrl driver to configure certain things, like
> > whether the pin is used as input or output. In addition to this there
> > are other pin configurations applicable to GPIOs such as debounce
> > timeout.
> >
> > To support this we introduce a new function pinctrl_gpio_set_config()
> > that can be used by gpiolib based driver to pass configuration requests
> > to the backing pinctrl driver.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> OK so this is needed.

The alternative would be to just handle this internally in
pinctrl-intel.c but I thought it is better to make the functionality
available for other drivers as well.

> But let's first pause and discuss this, because I have some stuff on my
> mind here.
> 
> First this kernel-internal ABI from <linux/gpio/driver.h>:
> 
> struct gpio_chip {
> (...)
>         int                     (*set_debounce)(struct gpio_chip *chip,
>                                                 unsigned offset,
>                                                 unsigned debounce);
>         int                     (*set_single_ended)(struct gpio_chip *chip,
>                                                 unsigned offset,
>                                                 enum single_ended_mode mode);
> (...)
> 
> It's not going to scale. We need to replace this with something like
> 
> int (*set_config)(struct gpio_chip *chip, unsigned offset, unsigned
> long config);
> 
> Where "config" takes the packed format described in
> <linux/pinctrl/pinconf-generic.h>
> and nothing else, anything else is just inviting disaster.
> 
> We can also later add:
> 
> int (*get_config)(struct gpio_chip *chip, unsigned offset, unsigned
> long *config);
> 
> We can then  set and get arbitrary configs on GPIO lines, and the
> drivers can simply implement a switch() for the configs they handle
> else return -ENOTSUPP.
> 
> But right now only set_config() would be enough.
> 
> Maybe stuff needs to be split out of that header to be shared between
> GPIO and pinctrl but hopefully you could just include it.
> 
> Then we change all in-kernel users of these two APIs over to set_config().
> 
> THEN we can think about cross-calling to pin control using the API
> from this patch. It should be a simple matter of just passing along the
> same config argument since we're using generic pin config.
> 
> It's not like it's impossible to merge this patch first, but I want to get some
> order here.
> 
> Are you convenient with doing the above patch as part of this series, or
> shall I do it first so you can rebase on it? (Will take some time if I
> do it...)

Sure, I can take a look at it.

> We need this because GPIO is going to need more and more config
> to be done by pinctrl on its behalf, and it will have to go all the
> way to userspace in many cases, so we need this infrastructure in
> place.

OK.

Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

[Mika Westerberg]

On Wed, Jan 11, 2017 at 03:33:04PM +0200, Mika Westerberg wrote:
> > But let's first pause and discuss this, because I have some stuff on my
> > mind here.
> > 
> > First this kernel-internal ABI from <linux/gpio/driver.h>:
> > 
> > struct gpio_chip {
> > (...)
> >         int                     (*set_debounce)(struct gpio_chip *chip,
> >                                                 unsigned offset,
> >                                                 unsigned debounce);
> >         int                     (*set_single_ended)(struct gpio_chip *chip,
> >                                                 unsigned offset,
> >                                                 enum single_ended_mode mode);
> > (...)
> > 
> > It's not going to scale. We need to replace this with something like
> > 
> > int (*set_config)(struct gpio_chip *chip, unsigned offset, unsigned
> > long config);
> > 
> > Where "config" takes the packed format described in
> > <linux/pinctrl/pinconf-generic.h>
> > and nothing else, anything else is just inviting disaster.
> > 
> > We can also later add:
> > 
> > int (*get_config)(struct gpio_chip *chip, unsigned offset, unsigned
> > long *config);
> > 
> > We can then  set and get arbitrary configs on GPIO lines, and the
> > drivers can simply implement a switch() for the configs they handle
> > else return -ENOTSUPP.
> > 
> > But right now only set_config() would be enough.
> > 
> > Maybe stuff needs to be split out of that header to be shared between
> > GPIO and pinctrl but hopefully you could just include it.
> > 
> > Then we change all in-kernel users of these two APIs over to set_config().
> > 
> > THEN we can think about cross-calling to pin control using the API
> > from this patch. It should be a simple matter of just passing along the
> > same config argument since we're using generic pin config.
> > 
> > It's not like it's impossible to merge this patch first, but I want to get some
> > order here.
> > 
> > Are you convenient with doing the above patch as part of this series, or
> > shall I do it first so you can rebase on it? (Will take some time if I
> > do it...)
> 
> Sure, I can take a look at it.

Hmm, looking at users of .set_debounce() I can see that the debounce
time can be quite large. For example some signals which are connected to
physical push-buttons may need > 64ms debounce time.

However, the current pinconfig value is defined to be unsigned long
which on 32-bit architecture is 32-bits. From that the higher 16-bits
are used as config leaving the value to be 16-bits. This gives maximum
debounce time of 65535us. I don't think it can cover all the uses of
.set_debounce(). This could also be problematic when specifying values
for pull resistors.

One solution is to convert the packed value to be u64 instead, leaving
up to 48-bits for the value. Alternatively we could provide a scale
field with the packed format.

What do you think?

Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

[Linus Walleij]

On Thu, Jan 12, 2017 at 10:22 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> Hmm, looking at users of .set_debounce() I can see that the debounce
> time can be quite large. For example some signals which are connected to
> physical push-buttons may need > 64ms debounce time.
>
> However, the current pinconfig value is defined to be unsigned long
> which on 32-bit architecture is 32-bits. From that the higher 16-bits
> are used as config leaving the value to be 16-bits. This gives maximum
> debounce time of 65535us. I don't think it can cover all the uses of
> .set_debounce(). This could also be problematic when specifying values
> for pull resistors.
>
> One solution is to convert the packed value to be u64 instead, leaving
> up to 48-bits for the value. Alternatively we could provide a scale
> field with the packed format.

Hm yeah as long as all in-kernel users survive I don't see why we
couldn't just make it 64bit. Is it a big deal?

A scale field (multiplier) can also work, I don't know which is most
elegant.

Yours,
Linus Walleij

Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

[Mika Westerberg]

On Fri, Jan 13, 2017 at 04:36:42PM +0100, Linus Walleij wrote:
> On Thu, Jan 12, 2017 at 10:22 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > Hmm, looking at users of .set_debounce() I can see that the debounce
> > time can be quite large. For example some signals which are connected to
> > physical push-buttons may need > 64ms debounce time.
> >
> > However, the current pinconfig value is defined to be unsigned long
> > which on 32-bit architecture is 32-bits. From that the higher 16-bits
> > are used as config leaving the value to be 16-bits. This gives maximum
> > debounce time of 65535us. I don't think it can cover all the uses of
> > .set_debounce(). This could also be problematic when specifying values
> > for pull resistors.
> >
> > One solution is to convert the packed value to be u64 instead, leaving
> > up to 48-bits for the value. Alternatively we could provide a scale
> > field with the packed format.
> 
> Hm yeah as long as all in-kernel users survive I don't see why we
> couldn't just make it 64bit. Is it a big deal?

As long as everyone is using those macros and inline functions from
pinconf-generic.h, I think the conversion should be pretty
straightforward.

Re: [PATCH v2 3/6] pinctrl: Add a possibility to configure pins from a gpiolib based drivers

[Linus Walleij]

On Fri, Jan 13, 2017 at 5:33 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Fri, Jan 13, 2017 at 04:36:42PM +0100, Linus Walleij wrote:
>> On Thu, Jan 12, 2017 at 10:22 AM, Mika Westerberg
>> <mika.westerberg@linux.intel.com> wrote:
>>
>> > Hmm, looking at users of .set_debounce() I can see that the debounce
>> > time can be quite large. For example some signals which are connected to
>> > physical push-buttons may need > 64ms debounce time.
>> >
>> > However, the current pinconfig value is defined to be unsigned long
>> > which on 32-bit architecture is 32-bits. From that the higher 16-bits
>> > are used as config leaving the value to be 16-bits. This gives maximum
>> > debounce time of 65535us. I don't think it can cover all the uses of
>> > .set_debounce(). This could also be problematic when specifying values
>> > for pull resistors.
>> >
>> > One solution is to convert the packed value to be u64 instead, leaving
>> > up to 48-bits for the value. Alternatively we could provide a scale
>> > field with the packed format.
>>
>> Hm yeah as long as all in-kernel users survive I don't see why we
>> couldn't just make it 64bit. Is it a big deal?
>
> As long as everyone is using those macros and inline functions from
> pinconf-generic.h, I think the conversion should be pretty
> straightforward.

I think I just make it a strict requirement that if people want to use
the pinctrl back-end for GPIO they simply have to support generic
pin control. It's not like they have something else already, and
converting a driver is not any unreasonable amount of work.

Yours,
Linus Walleij