[PATCH 0/6] Various improvements for samsung-pinctrl driver

[PATCH 0/6] Various improvements for samsung-pinctrl driver

[Tomasz Figa]

This series fixes various shortcommings of the Samsung pin control driver
that have been spotted while using it on various platforms with multiple
use cases and requirements.

Most of the patches are independent of each other, with the exception of
patch 4/6 which depends on patch 3/6, due to consolidation of IRQ chips.

Please see individual patches for more details.

Tested on Exynos4412-based Trats2 board with display, camera and GPIO keys
enabled.

Tomasz Figa (6):
  pinctrl: samsung: Decouple direction setting from pinctrl
  pinctrl: samsung: Handle GPIO request and free using pinctrl helpers
  pinctrl: exynos: Consolidate irq_chips of GPIO and WKUP EINTs
  pinctrl: exynos: Lock GPIOs as interrupts when used as EINTs
  pinctrl: samsung: Allow grouping multiple pinmux/pinconf nodes
  pinctrl: samsung: Allow pin value to be initialized using pinfunc

 .../bindings/pinctrl/samsung-pinctrl.txt           |  24 +-
 drivers/pinctrl/pinctrl-exynos.c                   | 376 ++++-------
 drivers/pinctrl/pinctrl-samsung.c                  | 728 ++++++++++++---------
 drivers/pinctrl/pinctrl-samsung.h                  |  19 +-
 4 files changed, 568 insertions(+), 579 deletions(-)

-- 
1.9.3

[PATCH 1/6] pinctrl: samsung: Decouple direction setting from pinctrl

[Tomasz Figa]

This patch makes the pinctrl-samsung driver configure GPIO direction on
its own, without using the pinctrl_gpio_direction_*() "helpers". The
rationale behind this change is as follows:
 - pinctrl-samsung does not need translation from GPIO namespace to
   pinctrl namespace to handle GPIO operations - GPIO chip and offset
   therein are enough to calculate necessary offsets and bit masks in
   constant time,
 - the pinctrl_gpio_direction_*() functions do not do anything useful
   other than translating the pin into pinctrl namespace and calling the
   .gpio_set_direction() from pinmux_ops of the controller,
 - the undesirable side effect of using those helpers is losing the
   ability to change GPIO direction in atomic context, because they
   explicitly use a mutex for synchronization,

Results of this patch are:
 - fixed warnings about scheduling while atomic in code that needs to
   set GPIO direction in atomic context (e.g. interrupt handler),
 - reduced overhead of bitbanging drivers that use gpio_direction_*(),
   e.g. i2c-gpio.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 drivers/pinctrl/pinctrl-samsung.c | 99 +++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 55 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
index 3e61d0f..779c8bc 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -340,50 +340,6 @@ static void samsung_pinmux_disable(struct pinctrl_dev *pctldev,
 	samsung_pinmux_setup(pctldev, selector, group, false);
 }
 
-/*
- * The calls to gpio_direction_output() and gpio_direction_input()
- * leads to this function call (via the pinctrl_gpio_direction_{input|output}()
- * function called from the gpiolib interface).
- */
-static int samsung_pinmux_gpio_set_direction(struct pinctrl_dev *pctldev,
-		struct pinctrl_gpio_range *range, unsigned offset, bool input)
-{
-	struct samsung_pin_bank_type *type;
-	struct samsung_pin_bank *bank;
-	struct samsung_pinctrl_drv_data *drvdata;
-	void __iomem *reg;
-	u32 data, pin_offset, mask, shift;
-	unsigned long flags;
-
-	bank = gc_to_pin_bank(range->gc);
-	type = bank->type;
-	drvdata = pinctrl_dev_get_drvdata(pctldev);
-
-	pin_offset = offset - bank->pin_base;
-	reg = drvdata->virt_base + bank->pctl_offset +
-					type->reg_offset[PINCFG_TYPE_FUNC];
-
-	mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1;
-	shift = pin_offset * type->fld_width[PINCFG_TYPE_FUNC];
-	if (shift >= 32) {
-		/* Some banks have two config registers */
-		shift -= 32;
-		reg += 4;
-	}
-
-	spin_lock_irqsave(&bank->slock, flags);
-
-	data = readl(reg);
-	data &= ~(mask << shift);
-	if (!input)
-		data |= FUNC_OUTPUT << shift;
-	writel(data, reg);
-
-	spin_unlock_irqrestore(&bank->slock, flags);
-
-	return 0;
-}
-
 /* list of pinmux callbacks for the pinmux vertical in pinctrl core */
 static const struct pinmux_ops samsung_pinmux_ops = {
 	.get_functions_count	= samsung_get_functions_count,
@@ -391,7 +347,6 @@ static const struct pinmux_ops samsung_pinmux_ops = {
 	.get_function_groups	= samsung_pinmux_get_groups,
 	.enable			= samsung_pinmux_enable,
 	.disable		= samsung_pinmux_disable,
-	.gpio_set_direction	= samsung_pinmux_gpio_set_direction,
 };
 
 /* set or get the pin config settings for a specified pin */
@@ -540,25 +495,59 @@ static int samsung_gpio_get(struct gpio_chip *gc, unsigned offset)
 }
 
 /*
- * gpiolib gpio_direction_input callback function. The setting of the pin
- * mux function as 'gpio input' will be handled by the pinctrl susbsystem
- * interface.
+ * The calls to gpio_direction_output() and gpio_direction_input()
+ * leads to this function call.
  */
+static int samsung_gpio_set_direction(struct gpio_chip *gc,
+					     unsigned offset, bool input)
+{
+	struct samsung_pin_bank_type *type;
+	struct samsung_pin_bank *bank;
+	struct samsung_pinctrl_drv_data *drvdata;
+	void __iomem *reg;
+	u32 data, mask, shift;
+	unsigned long flags;
+
+	bank = gc_to_pin_bank(gc);
+	type = bank->type;
+	drvdata = bank->drvdata;
+
+	reg = drvdata->virt_base + bank->pctl_offset +
+					type->reg_offset[PINCFG_TYPE_FUNC];
+
+	mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1;
+	shift = offset * type->fld_width[PINCFG_TYPE_FUNC];
+	if (shift >= 32) {
+		/* Some banks have two config registers */
+		shift -= 32;
+		reg += 4;
+	}
+
+	spin_lock_irqsave(&bank->slock, flags);
+
+	data = readl(reg);
+	data &= ~(mask << shift);
+	if (!input)
+		data |= FUNC_OUTPUT << shift;
+	writel(data, reg);
+
+	spin_unlock_irqrestore(&bank->slock, flags);
+
+	return 0;
+}
+
+/* gpiolib gpio_direction_input callback function. */
 static int samsung_gpio_direction_input(struct gpio_chip *gc, unsigned offset)
 {
-	return pinctrl_gpio_direction_input(gc->base + offset);
+	return samsung_gpio_set_direction(gc, offset, true);
 }
 
-/*
- * gpiolib gpio_direction_output callback function. The setting of the pin
- * mux function as 'gpio output' will be handled by the pinctrl susbsystem
- * interface.
- */
+/* gpiolib gpio_direction_output callback function. */
 static int samsung_gpio_direction_output(struct gpio_chip *gc, unsigned offset,
 							int value)
 {
 	samsung_gpio_set(gc, offset, value);
-	return pinctrl_gpio_direction_output(gc->base + offset);
+	return samsung_gpio_set_direction(gc, offset, false);
 }
 
 /*
-- 
1.9.3

[PATCH 2/6] pinctrl: samsung: Handle GPIO request and free using pinctrl helpers

[Tomasz Figa]

This patch adds .request() and .free() operations to gpio_chip of
pinctrl-samsung driver, which call pinctrl request and free helpers to
request and free pinctrl pin along with GPIO pin.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 drivers/pinctrl/pinctrl-samsung.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
index 779c8bc..6e099d6 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -779,7 +779,8 @@ static int samsung_pinctrl_register(struct platform_device *pdev,
 		pin_bank = &drvdata->ctrl->pin_banks[bank];
 		pin_bank->grange.name = pin_bank->name;
 		pin_bank->grange.id = bank;
-		pin_bank->grange.pin_base = pin_bank->pin_base;
+		pin_bank->grange.pin_base = drvdata->ctrl->base
+						+ pin_bank->pin_base;
 		pin_bank->grange.base = pin_bank->gpio_chip.base;
 		pin_bank->grange.npins = pin_bank->gpio_chip.ngpio;
 		pin_bank->grange.gc = &pin_bank->gpio_chip;
@@ -789,7 +790,19 @@ static int samsung_pinctrl_register(struct platform_device *pdev,
 	return 0;
 }
 
+static int samsung_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+	return pinctrl_request_gpio(chip->base + offset);
+}
+
+static void samsung_gpio_free(struct gpio_chip *chip, unsigned offset)
+{
+	pinctrl_free_gpio(chip->base + offset);
+}
+
 static const struct gpio_chip samsung_gpiolib_chip = {
+	.request = samsung_gpio_request,
+	.free = samsung_gpio_free,
 	.set = samsung_gpio_set,
 	.get = samsung_gpio_get,
 	.direction_input = samsung_gpio_direction_input,
-- 
1.9.3

[PATCH 3/6] pinctrl: exynos: Consolidate irq_chips of GPIO and WKUP EINTs

[Tomasz Figa]

Handling of irq_chip operations for GPIO and WKUP external interrupts
is mostly the same, with the difference being offset of registers.
However currently the driver has all the code duplicated for both EINT
types, which is undesirable, because changes in irq_chip operations have
to be done to both instances of the same code.

This patch fixes this by creating exynos_irq_chip struct that has normal
irq_chip struct embedded and contain differences between particular EINT
types, which are three register offsets. One instance of code is removed
and the new structure is used instead to fetch necessary data instead of
samsung_pin_ctrl struct used previously.

While at it, the patch removes Exynos-specific fields from
aforementioned structure to improve layering of the driver.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 drivers/pinctrl/pinctrl-exynos.c  | 307 ++++++++------------------------------
 drivers/pinctrl/pinctrl-samsung.h |  17 ---
 2 files changed, 60 insertions(+), 264 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index 9609c23..003bfd8 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -33,6 +33,18 @@
 #include "pinctrl-samsung.h"
 #include "pinctrl-exynos.h"
 
+struct exynos_irq_chip {
+	struct irq_chip chip;
+
+	u32 eint_con;
+	u32 eint_mask;
+	u32 eint_pend;
+};
+
+static inline struct exynos_irq_chip *to_exynos_irq_chip(struct irq_chip *chip)
+{
+	return container_of(chip, struct exynos_irq_chip, chip);
+}
 
 static struct samsung_pin_bank_type bank_type_off = {
 	.fld_width = { 4, 1, 2, 2, 2, 2, },
@@ -50,11 +62,13 @@ static const struct of_device_id exynos_wkup_irq_ids[] = {
 	{ }
 };
 
-static void exynos_gpio_irq_mask(struct irq_data *irqd)
+static void exynos_irq_mask(struct irq_data *irqd)
 {
+	struct irq_chip *chip = irq_data_get_irq_chip(irqd);
+	struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip);
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
 	struct samsung_pinctrl_drv_data *d = bank->drvdata;
-	unsigned long reg_mask = d->ctrl->geint_mask + bank->eint_offset;
+	unsigned long reg_mask = our_chip->eint_mask + bank->eint_offset;
 	unsigned long mask;
 	unsigned long flags;
 
@@ -67,20 +81,24 @@ static void exynos_gpio_irq_mask(struct irq_data *irqd)
 	spin_unlock_irqrestore(&bank->slock, flags);
 }
 
-static void exynos_gpio_irq_ack(struct irq_data *irqd)
+static void exynos_irq_ack(struct irq_data *irqd)
 {
+	struct irq_chip *chip = irq_data_get_irq_chip(irqd);
+	struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip);
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
 	struct samsung_pinctrl_drv_data *d = bank->drvdata;
-	unsigned long reg_pend = d->ctrl->geint_pend + bank->eint_offset;
+	unsigned long reg_pend = our_chip->eint_pend + bank->eint_offset;
 
 	writel(1 << irqd->hwirq, d->virt_base + reg_pend);
 }
 
-static void exynos_gpio_irq_unmask(struct irq_data *irqd)
+static void exynos_irq_unmask(struct irq_data *irqd)
 {
+	struct irq_chip *chip = irq_data_get_irq_chip(irqd);
+	struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip);
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
 	struct samsung_pinctrl_drv_data *d = bank->drvdata;
-	unsigned long reg_mask = d->ctrl->geint_mask + bank->eint_offset;
+	unsigned long reg_mask = our_chip->eint_mask + bank->eint_offset;
 	unsigned long mask;
 	unsigned long flags;
 
@@ -93,7 +111,7 @@ static void exynos_gpio_irq_unmask(struct irq_data *irqd)
 	 * masked.
 	 */
 	if (irqd_get_trigger_type(irqd) & IRQ_TYPE_LEVEL_MASK)
-		exynos_gpio_irq_ack(irqd);
+		exynos_irq_ack(irqd);
 
 	spin_lock_irqsave(&bank->slock, flags);
 
@@ -104,16 +122,17 @@ static void exynos_gpio_irq_unmask(struct irq_data *irqd)
 	spin_unlock_irqrestore(&bank->slock, flags);
 }
 
-static int exynos_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
+static int exynos_irq_set_type(struct irq_data *irqd, unsigned int type)
 {
+	struct irq_chip *chip = irq_data_get_irq_chip(irqd);
+	struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip);
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
 	struct samsung_pin_bank_type *bank_type = bank->type;
 	struct samsung_pinctrl_drv_data *d = bank->drvdata;
-	struct samsung_pin_ctrl *ctrl = d->ctrl;
 	unsigned int pin = irqd->hwirq;
 	unsigned int shift = EXYNOS_EINT_CON_LEN * pin;
 	unsigned int con, trig_type;
-	unsigned long reg_con = ctrl->geint_con + bank->eint_offset;
+	unsigned long reg_con = our_chip->eint_con + bank->eint_offset;
 	unsigned long flags;
 	unsigned int mask;
 
@@ -167,12 +186,17 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
 /*
  * irq_chip for gpio interrupts.
  */
-static struct irq_chip exynos_gpio_irq_chip = {
-	.name		= "exynos_gpio_irq_chip",
-	.irq_unmask	= exynos_gpio_irq_unmask,
-	.irq_mask	= exynos_gpio_irq_mask,
-	.irq_ack		= exynos_gpio_irq_ack,
-	.irq_set_type	= exynos_gpio_irq_set_type,
+static struct exynos_irq_chip exynos_gpio_irq_chip = {
+	.chip = {
+		.name = "exynos_gpio_irq_chip",
+		.irq_unmask = exynos_irq_unmask,
+		.irq_mask = exynos_irq_mask,
+		.irq_ack = exynos_irq_ack,
+		.irq_set_type = exynos_irq_set_type,
+	},
+	.eint_con = EXYNOS_GPIO_ECON_OFFSET,
+	.eint_mask = EXYNOS_GPIO_EMASK_OFFSET,
+	.eint_pend = EXYNOS_GPIO_EPEND_OFFSET,
 };
 
 static int exynos_gpio_irq_map(struct irq_domain *h, unsigned int virq,
@@ -181,7 +205,7 @@ static int exynos_gpio_irq_map(struct irq_domain *h, unsigned int virq,
 	struct samsung_pin_bank *b = h->host_data;
 
 	irq_set_chip_data(virq, b);
-	irq_set_chip_and_handler(virq, &exynos_gpio_irq_chip,
+	irq_set_chip_and_handler(virq, &exynos_gpio_irq_chip.chip,
 					handle_level_irq);
 	set_irq_flags(virq, IRQF_VALID);
 	return 0;
@@ -202,7 +226,7 @@ static irqreturn_t exynos_eint_gpio_irq(int irq, void *data)
 	struct samsung_pin_bank *bank = ctrl->pin_banks;
 	unsigned int svc, group, pin, virq;
 
-	svc = readl(d->virt_base + ctrl->svc);
+	svc = readl(d->virt_base + EXYNOS_SVC_OFFSET);
 	group = EXYNOS_SVC_GROUP(svc);
 	pin = svc & EXYNOS_SVC_NUM_MASK;
 
@@ -279,119 +303,6 @@ err_domains:
 	return ret;
 }
 
-static void exynos_wkup_irq_mask(struct irq_data *irqd)
-{
-	struct samsung_pin_bank *b = irq_data_get_irq_chip_data(irqd);
-	struct samsung_pinctrl_drv_data *d = b->drvdata;
-	unsigned long reg_mask = d->ctrl->weint_mask + b->eint_offset;
-	unsigned long mask;
-	unsigned long flags;
-
-	spin_lock_irqsave(&b->slock, flags);
-
-	mask = readl(d->virt_base + reg_mask);
-	mask |= 1 << irqd->hwirq;
-	writel(mask, d->virt_base + reg_mask);
-
-	spin_unlock_irqrestore(&b->slock, flags);
-}
-
-static void exynos_wkup_irq_ack(struct irq_data *irqd)
-{
-	struct samsung_pin_bank *b = irq_data_get_irq_chip_data(irqd);
-	struct samsung_pinctrl_drv_data *d = b->drvdata;
-	unsigned long pend = d->ctrl->weint_pend + b->eint_offset;
-
-	writel(1 << irqd->hwirq, d->virt_base + pend);
-}
-
-static void exynos_wkup_irq_unmask(struct irq_data *irqd)
-{
-	struct samsung_pin_bank *b = irq_data_get_irq_chip_data(irqd);
-	struct samsung_pinctrl_drv_data *d = b->drvdata;
-	unsigned long reg_mask = d->ctrl->weint_mask + b->eint_offset;
-	unsigned long mask;
-	unsigned long flags;
-
-	/*
-	 * Ack level interrupts right before unmask
-	 *
-	 * If we don't do this we'll get a double-interrupt.  Level triggered
-	 * interrupts must not fire an interrupt if the level is not
-	 * _currently_ active, even if it was active while the interrupt was
-	 * masked.
-	 */
-	if (irqd_get_trigger_type(irqd) & IRQ_TYPE_LEVEL_MASK)
-		exynos_wkup_irq_ack(irqd);
-
-	spin_lock_irqsave(&b->slock, flags);
-
-	mask = readl(d->virt_base + reg_mask);
-	mask &= ~(1 << irqd->hwirq);
-	writel(mask, d->virt_base + reg_mask);
-
-	spin_unlock_irqrestore(&b->slock, flags);
-}
-
-static int exynos_wkup_irq_set_type(struct irq_data *irqd, unsigned int type)
-{
-	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
-	struct samsung_pin_bank_type *bank_type = bank->type;
-	struct samsung_pinctrl_drv_data *d = bank->drvdata;
-	unsigned int pin = irqd->hwirq;
-	unsigned long reg_con = d->ctrl->weint_con + bank->eint_offset;
-	unsigned long shift = EXYNOS_EINT_CON_LEN * pin;
-	unsigned long con, trig_type;
-	unsigned long flags;
-	unsigned int mask;
-
-	switch (type) {
-	case IRQ_TYPE_EDGE_RISING:
-		trig_type = EXYNOS_EINT_EDGE_RISING;
-		break;
-	case IRQ_TYPE_EDGE_FALLING:
-		trig_type = EXYNOS_EINT_EDGE_FALLING;
-		break;
-	case IRQ_TYPE_EDGE_BOTH:
-		trig_type = EXYNOS_EINT_EDGE_BOTH;
-		break;
-	case IRQ_TYPE_LEVEL_HIGH:
-		trig_type = EXYNOS_EINT_LEVEL_HIGH;
-		break;
-	case IRQ_TYPE_LEVEL_LOW:
-		trig_type = EXYNOS_EINT_LEVEL_LOW;
-		break;
-	default:
-		pr_err("unsupported external interrupt type\n");
-		return -EINVAL;
-	}
-
-	if (type & IRQ_TYPE_EDGE_BOTH)
-		__irq_set_handler_locked(irqd->irq, handle_edge_irq);
-	else
-		__irq_set_handler_locked(irqd->irq, handle_level_irq);
-
-	con = readl(d->virt_base + reg_con);
-	con &= ~(EXYNOS_EINT_CON_MASK << shift);
-	con |= trig_type << shift;
-	writel(con, d->virt_base + reg_con);
-
-	reg_con = bank->pctl_offset + bank_type->reg_offset[PINCFG_TYPE_FUNC];
-	shift = pin * bank_type->fld_width[PINCFG_TYPE_FUNC];
-	mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
-
-	spin_lock_irqsave(&bank->slock, flags);
-
-	con = readl(d->virt_base + reg_con);
-	con &= ~(mask << shift);
-	con |= EXYNOS_EINT_FUNC << shift;
-	writel(con, d->virt_base + reg_con);
-
-	spin_unlock_irqrestore(&bank->slock, flags);
-
-	return 0;
-}
-
 static u32 exynos_eint_wake_mask = 0xffffffff;
 
 u32 exynos_get_eint_wake_mask(void)
@@ -417,13 +328,18 @@ static int exynos_wkup_irq_set_wake(struct irq_data *irqd, unsigned int on)
 /*
  * irq_chip for wakeup interrupts
  */
-static struct irq_chip exynos_wkup_irq_chip = {
-	.name	= "exynos_wkup_irq_chip",
-	.irq_unmask	= exynos_wkup_irq_unmask,
-	.irq_mask	= exynos_wkup_irq_mask,
-	.irq_ack	= exynos_wkup_irq_ack,
-	.irq_set_type	= exynos_wkup_irq_set_type,
-	.irq_set_wake	= exynos_wkup_irq_set_wake,
+static struct exynos_irq_chip exynos_wkup_irq_chip = {
+	.chip = {
+		.name = "exynos_wkup_irq_chip",
+		.irq_unmask = exynos_irq_unmask,
+		.irq_mask = exynos_irq_mask,
+		.irq_ack = exynos_irq_ack,
+		.irq_set_type = exynos_irq_set_type,
+		.irq_set_wake = exynos_wkup_irq_set_wake,
+	},
+	.eint_con = EXYNOS_WKUP_ECON_OFFSET,
+	.eint_mask = EXYNOS_WKUP_EMASK_OFFSET,
+	.eint_pend = EXYNOS_WKUP_EPEND_OFFSET,
 };
 
 /* interrupt handler for wakeup interrupts 0..15 */
@@ -464,7 +380,6 @@ static void exynos_irq_demux_eint16_31(unsigned int irq, struct irq_desc *desc)
 	struct irq_chip *chip = irq_get_chip(irq);
 	struct exynos_muxed_weint_data *eintd = irq_get_handler_data(irq);
 	struct samsung_pinctrl_drv_data *d = eintd->banks[0]->drvdata;
-	struct samsung_pin_ctrl *ctrl = d->ctrl;
 	unsigned long pend;
 	unsigned long mask;
 	int i;
@@ -473,8 +388,10 @@ static void exynos_irq_demux_eint16_31(unsigned int irq, struct irq_desc *desc)
 
 	for (i = 0; i < eintd->nr_banks; ++i) {
 		struct samsung_pin_bank *b = eintd->banks[i];
-		pend = readl(d->virt_base + ctrl->weint_pend + b->eint_offset);
-		mask = readl(d->virt_base + ctrl->weint_mask + b->eint_offset);
+		pend = readl(d->virt_base + EXYNOS_WKUP_EPEND_OFFSET
+				+ b->eint_offset);
+		mask = readl(d->virt_base + EXYNOS_WKUP_EMASK_OFFSET
+				+ b->eint_offset);
 		exynos_irq_demux_eint(pend & ~mask, b->irq_domain);
 	}
 
@@ -484,7 +401,8 @@ static void exynos_irq_demux_eint16_31(unsigned int irq, struct irq_desc *desc)
 static int exynos_wkup_irq_map(struct irq_domain *h, unsigned int virq,
 					irq_hw_number_t hw)
 {
-	irq_set_chip_and_handler(virq, &exynos_wkup_irq_chip, handle_level_irq);
+	irq_set_chip_and_handler(virq, &exynos_wkup_irq_chip.chip,
+					handle_level_irq);
 	irq_set_chip_data(virq, h->host_data);
 	set_irq_flags(virq, IRQF_VALID);
 	return 0;
@@ -703,13 +621,6 @@ struct samsung_pin_ctrl s5pv210_pin_ctrl[] = {
 		/* pin-controller instance 0 data */
 		.pin_banks	= s5pv210_pin_bank,
 		.nr_banks	= ARRAY_SIZE(s5pv210_pin_bank),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.weint_con	= EXYNOS_WKUP_ECON_OFFSET,
-		.weint_mask	= EXYNOS_WKUP_EMASK_OFFSET,
-		.weint_pend	= EXYNOS_WKUP_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.eint_wkup_init = exynos_eint_wkup_init,
 		.suspend	= exynos_pinctrl_suspend,
@@ -758,10 +669,6 @@ struct samsung_pin_ctrl exynos3250_pin_ctrl[] = {
 		/* pin-controller instance 0 data */
 		.pin_banks	= exynos3250_pin_banks0,
 		.nr_banks	= ARRAY_SIZE(exynos3250_pin_banks0),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -770,13 +677,6 @@ struct samsung_pin_ctrl exynos3250_pin_ctrl[] = {
 		/* pin-controller instance 1 data */
 		.pin_banks	= exynos3250_pin_banks1,
 		.nr_banks	= ARRAY_SIZE(exynos3250_pin_banks1),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.weint_con	= EXYNOS_WKUP_ECON_OFFSET,
-		.weint_mask	= EXYNOS_WKUP_EMASK_OFFSET,
-		.weint_pend	= EXYNOS_WKUP_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.eint_wkup_init = exynos_eint_wkup_init,
 		.suspend	= exynos_pinctrl_suspend,
@@ -843,10 +743,6 @@ struct samsung_pin_ctrl exynos4210_pin_ctrl[] = {
 		/* pin-controller instance 0 data */
 		.pin_banks	= exynos4210_pin_banks0,
 		.nr_banks	= ARRAY_SIZE(exynos4210_pin_banks0),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -855,13 +751,6 @@ struct samsung_pin_ctrl exynos4210_pin_ctrl[] = {
 		/* pin-controller instance 1 data */
 		.pin_banks	= exynos4210_pin_banks1,
 		.nr_banks	= ARRAY_SIZE(exynos4210_pin_banks1),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.weint_con	= EXYNOS_WKUP_ECON_OFFSET,
-		.weint_mask	= EXYNOS_WKUP_EMASK_OFFSET,
-		.weint_pend	= EXYNOS_WKUP_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.eint_wkup_init = exynos_eint_wkup_init,
 		.suspend	= exynos_pinctrl_suspend,
@@ -942,10 +831,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = {
 		/* pin-controller instance 0 data */
 		.pin_banks	= exynos4x12_pin_banks0,
 		.nr_banks	= ARRAY_SIZE(exynos4x12_pin_banks0),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -954,13 +839,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = {
 		/* pin-controller instance 1 data */
 		.pin_banks	= exynos4x12_pin_banks1,
 		.nr_banks	= ARRAY_SIZE(exynos4x12_pin_banks1),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.weint_con	= EXYNOS_WKUP_ECON_OFFSET,
-		.weint_mask	= EXYNOS_WKUP_EMASK_OFFSET,
-		.weint_pend	= EXYNOS_WKUP_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.eint_wkup_init = exynos_eint_wkup_init,
 		.suspend	= exynos_pinctrl_suspend,
@@ -970,10 +848,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = {
 		/* pin-controller instance 2 data */
 		.pin_banks	= exynos4x12_pin_banks2,
 		.nr_banks	= ARRAY_SIZE(exynos4x12_pin_banks2),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -982,10 +856,6 @@ struct samsung_pin_ctrl exynos4x12_pin_ctrl[] = {
 		/* pin-controller instance 3 data */
 		.pin_banks	= exynos4x12_pin_banks3,
 		.nr_banks	= ARRAY_SIZE(exynos4x12_pin_banks3),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -1058,13 +928,6 @@ struct samsung_pin_ctrl exynos5250_pin_ctrl[] = {
 		/* pin-controller instance 0 data */
 		.pin_banks	= exynos5250_pin_banks0,
 		.nr_banks	= ARRAY_SIZE(exynos5250_pin_banks0),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.weint_con	= EXYNOS_WKUP_ECON_OFFSET,
-		.weint_mask	= EXYNOS_WKUP_EMASK_OFFSET,
-		.weint_pend	= EXYNOS_WKUP_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.eint_wkup_init = exynos_eint_wkup_init,
 		.suspend	= exynos_pinctrl_suspend,
@@ -1074,10 +937,6 @@ struct samsung_pin_ctrl exynos5250_pin_ctrl[] = {
 		/* pin-controller instance 1 data */
 		.pin_banks	= exynos5250_pin_banks1,
 		.nr_banks	= ARRAY_SIZE(exynos5250_pin_banks1),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -1086,10 +945,6 @@ struct samsung_pin_ctrl exynos5250_pin_ctrl[] = {
 		/* pin-controller instance 2 data */
 		.pin_banks	= exynos5250_pin_banks2,
 		.nr_banks	= ARRAY_SIZE(exynos5250_pin_banks2),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -1098,10 +953,6 @@ struct samsung_pin_ctrl exynos5250_pin_ctrl[] = {
 		/* pin-controller instance 3 data */
 		.pin_banks	= exynos5250_pin_banks3,
 		.nr_banks	= ARRAY_SIZE(exynos5250_pin_banks3),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.suspend	= exynos_pinctrl_suspend,
 		.resume		= exynos_pinctrl_resume,
@@ -1158,13 +1009,6 @@ struct samsung_pin_ctrl exynos5260_pin_ctrl[] = {
 		/* pin-controller instance 0 data */
 		.pin_banks	= exynos5260_pin_banks0,
 		.nr_banks	= ARRAY_SIZE(exynos5260_pin_banks0),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.weint_con	= EXYNOS_WKUP_ECON_OFFSET,
-		.weint_mask	= EXYNOS_WKUP_EMASK_OFFSET,
-		.weint_pend	= EXYNOS_WKUP_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.eint_wkup_init = exynos_eint_wkup_init,
 		.label		= "exynos5260-gpio-ctrl0",
@@ -1172,20 +1016,12 @@ struct samsung_pin_ctrl exynos5260_pin_ctrl[] = {
 		/* pin-controller instance 1 data */
 		.pin_banks	= exynos5260_pin_banks1,
 		.nr_banks	= ARRAY_SIZE(exynos5260_pin_banks1),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.label		= "exynos5260-gpio-ctrl1",
 	}, {
 		/* pin-controller instance 2 data */
 		.pin_banks	= exynos5260_pin_banks2,
 		.nr_banks	= ARRAY_SIZE(exynos5260_pin_banks2),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.label		= "exynos5260-gpio-ctrl2",
 	},
@@ -1256,13 +1092,6 @@ struct samsung_pin_ctrl exynos5420_pin_ctrl[] = {
 		/* pin-controller instance 0 data */
 		.pin_banks	= exynos5420_pin_banks0,
 		.nr_banks	= ARRAY_SIZE(exynos5420_pin_banks0),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.weint_con	= EXYNOS_WKUP_ECON_OFFSET,
-		.weint_mask	= EXYNOS_WKUP_EMASK_OFFSET,
-		.weint_pend	= EXYNOS_WKUP_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.eint_wkup_init = exynos_eint_wkup_init,
 		.label		= "exynos5420-gpio-ctrl0",
@@ -1270,40 +1099,24 @@ struct samsung_pin_ctrl exynos5420_pin_ctrl[] = {
 		/* pin-controller instance 1 data */
 		.pin_banks	= exynos5420_pin_banks1,
 		.nr_banks	= ARRAY_SIZE(exynos5420_pin_banks1),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.label		= "exynos5420-gpio-ctrl1",
 	}, {
 		/* pin-controller instance 2 data */
 		.pin_banks	= exynos5420_pin_banks2,
 		.nr_banks	= ARRAY_SIZE(exynos5420_pin_banks2),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.label		= "exynos5420-gpio-ctrl2",
 	}, {
 		/* pin-controller instance 3 data */
 		.pin_banks	= exynos5420_pin_banks3,
 		.nr_banks	= ARRAY_SIZE(exynos5420_pin_banks3),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.label		= "exynos5420-gpio-ctrl3",
 	}, {
 		/* pin-controller instance 4 data */
 		.pin_banks	= exynos5420_pin_banks4,
 		.nr_banks	= ARRAY_SIZE(exynos5420_pin_banks4),
-		.geint_con	= EXYNOS_GPIO_ECON_OFFSET,
-		.geint_mask	= EXYNOS_GPIO_EMASK_OFFSET,
-		.geint_pend	= EXYNOS_GPIO_EPEND_OFFSET,
-		.svc		= EXYNOS_SVC_OFFSET,
 		.eint_gpio_init = exynos_eint_gpio_init,
 		.label		= "exynos5420-gpio-ctrl4",
 	},
diff --git a/drivers/pinctrl/pinctrl-samsung.h b/drivers/pinctrl/pinctrl-samsung.h
index b3e41fa..e2dce47 100644
--- a/drivers/pinctrl/pinctrl-samsung.h
+++ b/drivers/pinctrl/pinctrl-samsung.h
@@ -156,13 +156,6 @@ struct samsung_pin_bank {
  * @nr_banks: number of pin banks.
  * @base: starting system wide pin number.
  * @nr_pins: number of pins supported by the controller.
- * @geint_con: offset of the ext-gpio controller registers.
- * @geint_mask: offset of the ext-gpio interrupt mask registers.
- * @geint_pend: offset of the ext-gpio interrupt pending registers.
- * @weint_con: offset of the ext-wakeup controller registers.
- * @weint_mask: offset of the ext-wakeup interrupt mask registers.
- * @weint_pend: offset of the ext-wakeup interrupt pending registers.
- * @svc: offset of the interrupt service register.
  * @eint_gpio_init: platform specific callback to setup the external gpio
  *	interrupts for the controller.
  * @eint_wkup_init: platform specific callback to setup the external wakeup
@@ -176,16 +169,6 @@ struct samsung_pin_ctrl {
 	u32		base;
 	u32		nr_pins;
 
-	u32		geint_con;
-	u32		geint_mask;
-	u32		geint_pend;
-
-	u32		weint_con;
-	u32		weint_mask;
-	u32		weint_pend;
-
-	u32		svc;
-
 	int		(*eint_gpio_init)(struct samsung_pinctrl_drv_data *);
 	int		(*eint_wkup_init)(struct samsung_pinctrl_drv_data *);
 	void		(*suspend)(struct samsung_pinctrl_drv_data *);
-- 
1.9.3

[PATCH 4/6] pinctrl: exynos: Lock GPIOs as interrupts when used as EINTs

[Tomasz Figa]

Currently after configuring a GPIO pin as an interrupt related pinmux
registers are changed, but there is no protection from calling
gpio_direction_*() in a badly written driver, which would cause the same
pinmux register to be reconfigured for regular input/output and this
disabling interrupt capability of the pin.

This patch addresses this issue by moving pinmux reconfiguration to
.irq_startup() callback of irq_chip and calling gpio_lock_as_irq()
helper to prevent reconfiguration of pin direction.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
---
 drivers/pinctrl/pinctrl-exynos.c  | 69 +++++++++++++++++++++++++++++++++++----
 drivers/pinctrl/pinctrl-samsung.h |  1 +
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-exynos.c b/drivers/pinctrl/pinctrl-exynos.c
index 003bfd8..a209cb4 100644
--- a/drivers/pinctrl/pinctrl-exynos.c
+++ b/drivers/pinctrl/pinctrl-exynos.c
@@ -127,14 +127,10 @@ static int exynos_irq_set_type(struct irq_data *irqd, unsigned int type)
 	struct irq_chip *chip = irq_data_get_irq_chip(irqd);
 	struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip);
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
-	struct samsung_pin_bank_type *bank_type = bank->type;
 	struct samsung_pinctrl_drv_data *d = bank->drvdata;
-	unsigned int pin = irqd->hwirq;
-	unsigned int shift = EXYNOS_EINT_CON_LEN * pin;
+	unsigned int shift = EXYNOS_EINT_CON_LEN * irqd->hwirq;
 	unsigned int con, trig_type;
 	unsigned long reg_con = our_chip->eint_con + bank->eint_offset;
-	unsigned long flags;
-	unsigned int mask;
 
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:
@@ -167,8 +163,32 @@ static int exynos_irq_set_type(struct irq_data *irqd, unsigned int type)
 	con |= trig_type << shift;
 	writel(con, d->virt_base + reg_con);
 
+	return 0;
+}
+
+static unsigned int exynos_irq_startup(struct irq_data *irqd)
+{
+	struct irq_chip *chip = irq_data_get_irq_chip(irqd);
+	struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip);
+	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+	struct samsung_pin_bank_type *bank_type = bank->type;
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
+	unsigned int shift = EXYNOS_EINT_CON_LEN * irqd->hwirq;
+	unsigned long reg_con = our_chip->eint_con + bank->eint_offset;
+	unsigned long flags;
+	unsigned int mask;
+	unsigned int con;
+	int ret;
+
+	ret = gpio_lock_as_irq(&bank->gpio_chip, irqd->hwirq);
+	if (ret) {
+		dev_err(bank->gpio_chip.dev, "unable to lock pin %s-%lu IRQ\n",
+			bank->name, irqd->hwirq);
+		return ret;
+	}
+
 	reg_con = bank->pctl_offset + bank_type->reg_offset[PINCFG_TYPE_FUNC];
-	shift = pin * bank_type->fld_width[PINCFG_TYPE_FUNC];
+	shift = irqd->hwirq * bank_type->fld_width[PINCFG_TYPE_FUNC];
 	mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
 
 	spin_lock_irqsave(&bank->slock, flags);
@@ -180,9 +200,42 @@ static int exynos_irq_set_type(struct irq_data *irqd, unsigned int type)
 
 	spin_unlock_irqrestore(&bank->slock, flags);
 
+	exynos_irq_unmask(irqd);
+
 	return 0;
 }
 
+static void exynos_irq_shutdown(struct irq_data *irqd)
+{
+	struct irq_chip *chip = irq_data_get_irq_chip(irqd);
+	struct exynos_irq_chip *our_chip = to_exynos_irq_chip(chip);
+	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+	struct samsung_pin_bank_type *bank_type = bank->type;
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
+	unsigned int shift = EXYNOS_EINT_CON_LEN * irqd->hwirq;
+	unsigned long reg_con = our_chip->eint_con + bank->eint_offset;
+	unsigned long flags;
+	unsigned int mask;
+	unsigned int con;
+
+	reg_con = bank->pctl_offset + bank_type->reg_offset[PINCFG_TYPE_FUNC];
+	shift = irqd->hwirq * bank_type->fld_width[PINCFG_TYPE_FUNC];
+	mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
+
+	exynos_irq_mask(irqd);
+
+	spin_lock_irqsave(&bank->slock, flags);
+
+	con = readl(d->virt_base + reg_con);
+	con &= ~(mask << shift);
+	con |= FUNC_INPUT << shift;
+	writel(con, d->virt_base + reg_con);
+
+	spin_unlock_irqrestore(&bank->slock, flags);
+
+	gpio_unlock_as_irq(&bank->gpio_chip, irqd->hwirq);
+}
+
 /*
  * irq_chip for gpio interrupts.
  */
@@ -193,6 +246,8 @@ static struct exynos_irq_chip exynos_gpio_irq_chip = {
 		.irq_mask = exynos_irq_mask,
 		.irq_ack = exynos_irq_ack,
 		.irq_set_type = exynos_irq_set_type,
+		.irq_startup = exynos_irq_startup,
+		.irq_shutdown = exynos_irq_shutdown,
 	},
 	.eint_con = EXYNOS_GPIO_ECON_OFFSET,
 	.eint_mask = EXYNOS_GPIO_EMASK_OFFSET,
@@ -336,6 +391,8 @@ static struct exynos_irq_chip exynos_wkup_irq_chip = {
 		.irq_ack = exynos_irq_ack,
 		.irq_set_type = exynos_irq_set_type,
 		.irq_set_wake = exynos_wkup_irq_set_wake,
+		.irq_startup = exynos_irq_startup,
+		.irq_shutdown = exynos_irq_shutdown,
 	},
 	.eint_con = EXYNOS_WKUP_ECON_OFFSET,
 	.eint_mask = EXYNOS_WKUP_EMASK_OFFSET,
diff --git a/drivers/pinctrl/pinctrl-samsung.h b/drivers/pinctrl/pinctrl-samsung.h
index e2dce47..4d7566a 100644
--- a/drivers/pinctrl/pinctrl-samsung.h
+++ b/drivers/pinctrl/pinctrl-samsung.h
@@ -26,6 +26,7 @@
 #include <linux/gpio.h>
 
 /* pinmux function number for pin as gpio output line */
+#define FUNC_INPUT	0x0
 #define FUNC_OUTPUT	0x1
 
 /**
-- 
1.9.3

[PATCH 5/6] pinctrl: samsung: Allow grouping multiple pinmux/pinconf nodes

[Tomasz Figa]

One of remaining limitations of current pinctrl-samsung driver was
the inability to parse multiple pinmux/pinconf group nodes grouped
inside a single device tree node. It made defining groups of pins for
single purpose, but with different parameters very inconvenient.

This patch implements Tegra-like support for grouping multiple pinctrl
groups inside one device tree node, by completely changing the way
pin groups and functions are parsed from device tree. The code creating
pinctrl maps from DT nodes has been borrowed from pinctrl-tegra, while
the initial creation of groups and functions has been completely
rewritten with following assumptions:
 - each group consists of just one pin and does not depend on data
   from device tree,
 - each function is represented by a device tree child node of the
   pin controller, which in turn can contain multiple child nodes
   for pins that need to have different configuration values.

Device Tree bindings are fully backwards compatible. New functionality
can be used by defining a new pinctrl group consisting of several child
nodes, as on following example:

	sd4_bus8: sd4-bus-width8 {
		part-1 {
			samsung,pins = "gpk0-3", "gpk0-4",
					"gpk0-5", "gpk0-6";
			samsung,pin-function = <3>;
			samsung,pin-pud = <3>;
			samsung,pin-drv = <3>;
		};
		part-2 {
			samsung,pins = "gpk1-3", "gpk1-4",
					"gpk1-5", "gpk1-6";
			samsung,pin-function = <4>;
			samsung,pin-pud = <4>;
			samsung,pin-drv = <3>;
		};
	};

Tested on Exynos4210-Trats board and a custom Exynos4212-based one.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Cc: devicetree at vger.kernel.org
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 .../bindings/pinctrl/samsung-pinctrl.txt           |  23 +-
 drivers/pinctrl/pinctrl-samsung.c                  | 613 ++++++++++++---------
 drivers/pinctrl/pinctrl-samsung.h                  |   1 +
 3 files changed, 384 insertions(+), 253 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index 2b32783..464b2bb 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -44,7 +44,11 @@ Required Properties:
 - Pin mux/config groups as child nodes: The pin mux (selecting pin function
   mode) and pin config (pull up/down, driver strength) settings are represented
   as child nodes of the pin-controller node. There should be atleast one
-  child node and there is no limit on the count of these child nodes.
+  child node and there is no limit on the count of these child nodes. It is
+  also possible for a child node to consist of several further child nodes
+  to allow grouping multiple pinctrl groups into one. The format of second
+  level child nodes is exactly the same as for first level ones and is
+  described below.
 
   The child node should contain a list of pin(s) on which a particular pin
   function selection or pin configuration (or both) have to applied. This
@@ -249,6 +253,23 @@ Example 1: A pin-controller node with pin groups.
 			samsung,pin-pud = <3>;
 			samsung,pin-drv = <0>;
 		};
+
+		sd4_bus8: sd4-bus-width8 {
+			part-1 {
+				samsung,pins = "gpk0-3", "gpk0-4",
+						"gpk0-5", "gpk0-6";
+				samsung,pin-function = <3>;
+				samsung,pin-pud = <3>;
+				samsung,pin-drv = <3>;
+			};
+			part-2 {
+				samsung,pins = "gpk1-3", "gpk1-4",
+						"gpk1-5", "gpk1-6";
+				samsung,pin-function = <4>;
+				samsung,pin-pud = <4>;
+				samsung,pin-drv = <3>;
+			};
+		};
 	};
 
 Example 2: A pin-controller node with external wakeup interrupt controller node.
diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
index 6e099d6..caa6dbe 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -40,9 +40,9 @@
 
 /* list of all possible config options supported */
 static struct pin_config {
-	char		*prop_cfg;
-	unsigned int	cfg_type;
-} pcfgs[] = {
+	const char *property;
+	enum pincfg_type param;
+} cfg_params[] = {
 	{ "samsung,pin-pud", PINCFG_TYPE_PUD },
 	{ "samsung,pin-drv", PINCFG_TYPE_DRV },
 	{ "samsung,pin-con-pdn", PINCFG_TYPE_CON_PDN },
@@ -59,163 +59,242 @@ static inline struct samsung_pin_bank *gc_to_pin_bank(struct gpio_chip *gc)
 	return container_of(gc, struct samsung_pin_bank, gpio_chip);
 }
 
-/* check if the selector is a valid pin group selector */
 static int samsung_get_group_count(struct pinctrl_dev *pctldev)
 {
-	struct samsung_pinctrl_drv_data *drvdata;
+	struct samsung_pinctrl_drv_data *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	drvdata = pinctrl_dev_get_drvdata(pctldev);
-	return drvdata->nr_groups;
+	return pmx->nr_groups;
 }
 
-/* return the name of the group selected by the group selector */
 static const char *samsung_get_group_name(struct pinctrl_dev *pctldev,
-						unsigned selector)
+						unsigned group)
 {
-	struct samsung_pinctrl_drv_data *drvdata;
+	struct samsung_pinctrl_drv_data *pmx = pinctrl_dev_get_drvdata(pctldev);
 
-	drvdata = pinctrl_dev_get_drvdata(pctldev);
-	return drvdata->pin_groups[selector].name;
+	return pmx->pin_groups[group].name;
 }
 
-/* return the pin numbers associated with the specified group */
 static int samsung_get_group_pins(struct pinctrl_dev *pctldev,
-		unsigned selector, const unsigned **pins, unsigned *num_pins)
+					unsigned group,
+					const unsigned **pins,
+					unsigned *num_pins)
 {
-	struct samsung_pinctrl_drv_data *drvdata;
+	struct samsung_pinctrl_drv_data *pmx = pinctrl_dev_get_drvdata(pctldev);
+
+	*pins = pmx->pin_groups[group].pins;
+	*num_pins = pmx->pin_groups[group].num_pins;
 
-	drvdata = pinctrl_dev_get_drvdata(pctldev);
-	*pins = drvdata->pin_groups[selector].pins;
-	*num_pins = drvdata->pin_groups[selector].num_pins;
 	return 0;
 }
 
-/* create pinctrl_map entries by parsing device tree nodes */
-static int samsung_dt_node_to_map(struct pinctrl_dev *pctldev,
-			struct device_node *np, struct pinctrl_map **maps,
-			unsigned *nmaps)
+static int reserve_map(struct device *dev, struct pinctrl_map **map,
+		       unsigned *reserved_maps, unsigned *num_maps,
+		       unsigned reserve)
 {
-	struct device *dev = pctldev->dev;
-	struct pinctrl_map *map;
-	unsigned long *cfg = NULL;
-	char *gname, *fname;
-	int cfg_cnt = 0, map_cnt = 0, idx = 0;
-
-	/* count the number of config options specfied in the node */
-	for (idx = 0; idx < ARRAY_SIZE(pcfgs); idx++) {
-		if (of_find_property(np, pcfgs[idx].prop_cfg, NULL))
-			cfg_cnt++;
-	}
+	unsigned old_num = *reserved_maps;
+	unsigned new_num = *num_maps + reserve;
+	struct pinctrl_map *new_map;
 
-	/*
-	 * Find out the number of map entries to create. All the config options
-	 * can be accomadated into a single config map entry.
-	 */
-	if (cfg_cnt)
-		map_cnt = 1;
-	if (of_find_property(np, "samsung,pin-function", NULL))
-		map_cnt++;
-	if (!map_cnt) {
-		dev_err(dev, "node %s does not have either config or function "
-				"configurations\n", np->name);
-		return -EINVAL;
-	}
+	if (old_num >= new_num)
+		return 0;
 
-	/* Allocate memory for pin-map entries */
-	map = kzalloc(sizeof(*map) * map_cnt, GFP_KERNEL);
-	if (!map) {
-		dev_err(dev, "could not alloc memory for pin-maps\n");
+	new_map = krealloc(*map, sizeof(*new_map) * new_num, GFP_KERNEL);
+	if (!new_map) {
+		dev_err(dev, "krealloc(map) failed\n");
 		return -ENOMEM;
 	}
-	*nmaps = 0;
 
-	/*
-	 * Allocate memory for pin group name. The pin group name is derived
-	 * from the node name from which these map entries are be created.
-	 */
-	gname = kzalloc(strlen(np->name) + GSUFFIX_LEN, GFP_KERNEL);
-	if (!gname) {
-		dev_err(dev, "failed to alloc memory for group name\n");
-		goto free_map;
+	memset(new_map + old_num, 0, (new_num - old_num) * sizeof(*new_map));
+
+	*map = new_map;
+	*reserved_maps = new_num;
+
+	return 0;
+}
+
+static int add_map_mux(struct pinctrl_map **map, unsigned *reserved_maps,
+		       unsigned *num_maps, const char *group,
+		       const char *function)
+{
+	if (WARN_ON(*num_maps == *reserved_maps))
+		return -ENOSPC;
+
+	(*map)[*num_maps].type = PIN_MAP_TYPE_MUX_GROUP;
+	(*map)[*num_maps].data.mux.group = group;
+	(*map)[*num_maps].data.mux.function = function;
+	(*num_maps)++;
+
+	return 0;
+}
+
+static int add_map_configs(struct device *dev, struct pinctrl_map **map,
+			   unsigned *reserved_maps, unsigned *num_maps,
+			   const char *group, unsigned long *configs,
+			   unsigned num_configs)
+{
+	unsigned long *dup_configs;
+
+	if (WARN_ON(*num_maps == *reserved_maps))
+		return -ENOSPC;
+
+	dup_configs = kmemdup(configs, num_configs * sizeof(*dup_configs),
+			      GFP_KERNEL);
+	if (!dup_configs) {
+		dev_err(dev, "kmemdup(configs) failed\n");
+		return -ENOMEM;
 	}
-	sprintf(gname, "%s%s", np->name, GROUP_SUFFIX);
 
-	/*
-	 * don't have config options? then skip over to creating function
-	 * map entries.
-	 */
-	if (!cfg_cnt)
-		goto skip_cfgs;
-
-	/* Allocate memory for config entries */
-	cfg = kzalloc(sizeof(*cfg) * cfg_cnt, GFP_KERNEL);
-	if (!cfg) {
-		dev_err(dev, "failed to alloc memory for configs\n");
-		goto free_gname;
+	(*map)[*num_maps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
+	(*map)[*num_maps].data.configs.group_or_pin = group;
+	(*map)[*num_maps].data.configs.configs = dup_configs;
+	(*map)[*num_maps].data.configs.num_configs = num_configs;
+	(*num_maps)++;
+
+	return 0;
+}
+
+static int add_config(struct device *dev, unsigned long **configs,
+		      unsigned *num_configs, unsigned long config)
+{
+	unsigned old_num = *num_configs;
+	unsigned new_num = old_num + 1;
+	unsigned long *new_configs;
+
+	new_configs = krealloc(*configs, sizeof(*new_configs) * new_num,
+			       GFP_KERNEL);
+	if (!new_configs) {
+		dev_err(dev, "krealloc(configs) failed\n");
+		return -ENOMEM;
 	}
 
-	/* Prepare a list of config settings */
-	for (idx = 0, cfg_cnt = 0; idx < ARRAY_SIZE(pcfgs); idx++) {
-		u32 value;
-		if (!of_property_read_u32(np, pcfgs[idx].prop_cfg, &value))
-			cfg[cfg_cnt++] =
-				PINCFG_PACK(pcfgs[idx].cfg_type, value);
+	new_configs[old_num] = config;
+
+	*configs = new_configs;
+	*num_configs = new_num;
+
+	return 0;
+}
+
+static void samsung_dt_free_map(struct pinctrl_dev *pctldev,
+				      struct pinctrl_map *map,
+				      unsigned num_maps)
+{
+	int i;
+
+	for (i = 0; i < num_maps; i++)
+		if (map[i].type == PIN_MAP_TYPE_CONFIGS_GROUP)
+			kfree(map[i].data.configs.configs);
+
+	kfree(map);
+}
+
+static int samsung_dt_subnode_to_map(struct samsung_pinctrl_drv_data *drvdata,
+				     struct device *dev,
+				     struct device_node *np,
+				     struct pinctrl_map **map,
+				     unsigned *reserved_maps,
+				     unsigned *num_maps)
+{
+	int ret, i;
+	u32 val;
+	unsigned long config;
+	unsigned long *configs = NULL;
+	unsigned num_configs = 0;
+	unsigned reserve;
+	struct property *prop;
+	const char *group;
+	bool has_func = false;
+
+	ret = of_property_read_u32(np, "samsung,pin-function", &val);
+	if (!ret)
+		has_func = true;
+
+	for (i = 0; i < ARRAY_SIZE(cfg_params); i++) {
+		ret = of_property_read_u32(np, cfg_params[i].property, &val);
+		if (!ret) {
+			config = PINCFG_PACK(cfg_params[i].param, val);
+			ret = add_config(dev, &configs, &num_configs, config);
+			if (ret < 0)
+				goto exit;
+		/* EINVAL=missing, which is fine since it's optional */
+		} else if (ret != -EINVAL) {
+			dev_err(dev, "could not parse property %s\n",
+				cfg_params[i].property);
+		}
 	}
 
-	/* create the config map entry */
-	map[*nmaps].data.configs.group_or_pin = gname;
-	map[*nmaps].data.configs.configs = cfg;
-	map[*nmaps].data.configs.num_configs = cfg_cnt;
-	map[*nmaps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
-	*nmaps += 1;
-
-skip_cfgs:
-	/* create the function map entry */
-	if (of_find_property(np, "samsung,pin-function", NULL)) {
-		fname = kzalloc(strlen(np->name) + FSUFFIX_LEN,	GFP_KERNEL);
-		if (!fname) {
-			dev_err(dev, "failed to alloc memory for func name\n");
-			goto free_cfg;
+	reserve = 0;
+	if (has_func)
+		reserve++;
+	if (num_configs)
+		reserve++;
+	ret = of_property_count_strings(np, "samsung,pins");
+	if (ret < 0) {
+		dev_err(dev, "could not parse property samsung,pins\n");
+		goto exit;
+	}
+	reserve *= ret;
+
+	ret = reserve_map(dev, map, reserved_maps, num_maps, reserve);
+	if (ret < 0)
+		goto exit;
+
+	of_property_for_each_string(np, "samsung,pins", prop, group) {
+		if (has_func) {
+			ret = add_map_mux(map, reserved_maps,
+						num_maps, group, np->full_name);
+			if (ret < 0)
+				goto exit;
 		}
-		sprintf(fname, "%s%s", np->name, FUNCTION_SUFFIX);
 
-		map[*nmaps].data.mux.group = gname;
-		map[*nmaps].data.mux.function = fname;
-		map[*nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
-		*nmaps += 1;
+		if (num_configs) {
+			ret = add_map_configs(dev, map, reserved_maps,
+					      num_maps, group, configs,
+					      num_configs);
+			if (ret < 0)
+				goto exit;
+		}
 	}
 
-	*maps = map;
-	return 0;
+	ret = 0;
 
-free_cfg:
-	kfree(cfg);
-free_gname:
-	kfree(gname);
-free_map:
-	kfree(map);
-	return -ENOMEM;
+exit:
+	kfree(configs);
+	return ret;
 }
 
-/* free the memory allocated to hold the pin-map table */
-static void samsung_dt_free_map(struct pinctrl_dev *pctldev,
-			     struct pinctrl_map *map, unsigned num_maps)
+static int samsung_dt_node_to_map(struct pinctrl_dev *pctldev,
+					struct device_node *np_config,
+					struct pinctrl_map **map,
+					unsigned *num_maps)
 {
-	int idx;
-
-	for (idx = 0; idx < num_maps; idx++) {
-		if (map[idx].type == PIN_MAP_TYPE_MUX_GROUP) {
-			kfree(map[idx].data.mux.function);
-			if (!idx)
-				kfree(map[idx].data.mux.group);
-		} else if (map->type == PIN_MAP_TYPE_CONFIGS_GROUP) {
-			kfree(map[idx].data.configs.configs);
-			if (!idx)
-				kfree(map[idx].data.configs.group_or_pin);
+	struct samsung_pinctrl_drv_data *drvdata;
+	unsigned reserved_maps;
+	struct device_node *np;
+	int ret;
+
+	drvdata = pinctrl_dev_get_drvdata(pctldev);
+
+	reserved_maps = 0;
+	*map = NULL;
+	*num_maps = 0;
+
+	if (!of_get_child_count(np_config))
+		return samsung_dt_subnode_to_map(drvdata, pctldev->dev,
+							np_config, map,
+							&reserved_maps,
+							num_maps);
+
+	for_each_child_of_node(np_config, np) {
+		ret = samsung_dt_subnode_to_map(drvdata, pctldev->dev, np, map,
+						&reserved_maps, num_maps);
+		if (ret < 0) {
+			samsung_dt_free_map(pctldev, *map, *num_maps);
+			return ret;
 		}
-	};
+	}
 
-	kfree(map);
+	return 0;
 }
 
 /* list of pinctrl callbacks for the pinctrl core */
@@ -286,43 +365,38 @@ static void samsung_pinmux_setup(struct pinctrl_dev *pctldev, unsigned selector,
 					unsigned group, bool enable)
 {
 	struct samsung_pinctrl_drv_data *drvdata;
-	const unsigned int *pins;
+	struct samsung_pin_bank_type *type;
 	struct samsung_pin_bank *bank;
 	void __iomem *reg;
-	u32 mask, shift, data, pin_offset, cnt;
+	u32 mask, shift, data, pin_offset;
 	unsigned long flags;
+	const struct samsung_pmx_func *func;
+	const struct samsung_pin_group *grp;
 
 	drvdata = pinctrl_dev_get_drvdata(pctldev);
-	pins = drvdata->pin_groups[group].pins;
+	func = &drvdata->pmx_functions[selector];
+	grp = &drvdata->pin_groups[group];
 
-	/*
-	 * for each pin in the pin group selected, program the correspoding pin
-	 * pin function number in the config register.
-	 */
-	for (cnt = 0; cnt < drvdata->pin_groups[group].num_pins; cnt++) {
-		struct samsung_pin_bank_type *type;
-
-		pin_to_reg_bank(drvdata, pins[cnt] - drvdata->ctrl->base,
-				&reg, &pin_offset, &bank);
-		type = bank->type;
-		mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1;
-		shift = pin_offset * type->fld_width[PINCFG_TYPE_FUNC];
-		if (shift >= 32) {
-			/* Some banks have two config registers */
-			shift -= 32;
-			reg += 4;
-		}
+	pin_to_reg_bank(drvdata, grp->pins[0] - drvdata->ctrl->base,
+			&reg, &pin_offset, &bank);
+	type = bank->type;
+	mask = (1 << type->fld_width[PINCFG_TYPE_FUNC]) - 1;
+	shift = pin_offset * type->fld_width[PINCFG_TYPE_FUNC];
+	if (shift >= 32) {
+		/* Some banks have two config registers */
+		shift -= 32;
+		reg += 4;
+	}
 
-		spin_lock_irqsave(&bank->slock, flags);
+	spin_lock_irqsave(&bank->slock, flags);
 
-		data = readl(reg + type->reg_offset[PINCFG_TYPE_FUNC]);
-		data &= ~(mask << shift);
-		if (enable)
-			data |= drvdata->pin_groups[group].func << shift;
-		writel(data, reg + type->reg_offset[PINCFG_TYPE_FUNC]);
+	data = readl(reg + type->reg_offset[PINCFG_TYPE_FUNC]);
+	data &= ~(mask << shift);
+	if (enable)
+		data |= func->val << shift;
+	writel(data, reg + type->reg_offset[PINCFG_TYPE_FUNC]);
 
-		spin_unlock_irqrestore(&bank->slock, flags);
-	}
+	spin_unlock_irqrestore(&bank->slock, flags);
 }
 
 /* enable a specified pinmux by writing to registers */
@@ -567,87 +641,115 @@ static int samsung_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
 	return (virq) ? : -ENXIO;
 }
 
-/*
- * Parse the pin names listed in the 'samsung,pins' property and convert it
- * into a list of gpio numbers are create a pin group from it.
- */
-static int samsung_pinctrl_parse_dt_pins(struct platform_device *pdev,
-					 struct device_node *cfg_np,
-					 struct pinctrl_desc *pctl,
-					 unsigned int **pin_list,
-					 unsigned int *npins)
+static struct samsung_pin_group *samsung_pinctrl_create_groups(
+				struct device *dev,
+				struct samsung_pinctrl_drv_data *drvdata,
+				unsigned int *cnt)
 {
-	struct device *dev = &pdev->dev;
-	struct property *prop;
-	struct pinctrl_pin_desc const *pdesc = pctl->pins;
-	unsigned int idx = 0, cnt;
-	const char *pin_name;
+	struct pinctrl_desc *ctrldesc = &drvdata->pctl;
+	struct samsung_pin_group *groups, *grp;
+	const struct pinctrl_pin_desc *pdesc;
+	int i;
+
+	groups = devm_kzalloc(dev, ctrldesc->npins * sizeof(*groups),
+				GFP_KERNEL);
+	if (!groups)
+		return ERR_PTR(-EINVAL);
+	grp = groups;
+
+	pdesc = ctrldesc->pins;
+	for (i = 0; i < ctrldesc->npins; ++i, ++pdesc, ++grp) {
+		grp->name = pdesc->name;
+		grp->pins = &pdesc->number;
+		grp->num_pins = 1;
+	}
+
+	*cnt = ctrldesc->npins;
+	return groups;
+}
 
-	*npins = of_property_count_strings(cfg_np, "samsung,pins");
-	if (IS_ERR_VALUE(*npins)) {
-		dev_err(dev, "invalid pin list in %s node", cfg_np->name);
+static int samsung_pinctrl_create_function(struct device *dev,
+				struct samsung_pinctrl_drv_data *drvdata,
+				struct device_node *func_np,
+				struct samsung_pmx_func *func)
+{
+	int npins;
+	int ret;
+	int i;
+
+	if (of_property_read_u32(func_np, "samsung,pin-function", &func->val))
+		return 0;
+
+	npins = of_property_count_strings(func_np, "samsung,pins");
+	if (npins < 1) {
+		dev_err(dev, "invalid pin list in %s node", func_np->name);
 		return -EINVAL;
 	}
 
-	*pin_list = devm_kzalloc(dev, *npins * sizeof(**pin_list), GFP_KERNEL);
-	if (!*pin_list) {
-		dev_err(dev, "failed to allocate memory for pin list\n");
+	func->name = func_np->full_name;
+
+	func->groups = devm_kzalloc(dev, npins * sizeof(char *), GFP_KERNEL);
+	if (!func->groups)
 		return -ENOMEM;
-	}
 
-	of_property_for_each_string(cfg_np, "samsung,pins", prop, pin_name) {
-		for (cnt = 0; cnt < pctl->npins; cnt++) {
-			if (pdesc[cnt].name) {
-				if (!strcmp(pin_name, pdesc[cnt].name)) {
-					(*pin_list)[idx++] = pdesc[cnt].number;
-					break;
-				}
-			}
-		}
-		if (cnt == pctl->npins) {
-			dev_err(dev, "pin %s not valid in %s node\n",
-					pin_name, cfg_np->name);
-			devm_kfree(dev, *pin_list);
-			return -EINVAL;
+	for (i = 0; i < npins; ++i) {
+		const char *gname;
+
+		ret = of_property_read_string_index(func_np, "samsung,pins",
+							i, &gname);
+		if (ret) {
+			dev_err(dev,
+				"failed to read pin name %d from %s node\n",
+				i, func_np->name);
+			return ret;
 		}
+
+		func->groups[i] = gname;
 	}
 
-	return 0;
+	func->num_groups = npins;
+	return 1;
 }
 
-/*
- * Parse the information about all the available pin groups and pin functions
- * from device node of the pin-controller. A pin group is formed with all
- * the pins listed in the "samsung,pins" property.
- */
-static int samsung_pinctrl_parse_dt(struct platform_device *pdev,
-				    struct samsung_pinctrl_drv_data *drvdata)
+static struct samsung_pmx_func *samsung_pinctrl_create_functions(
+				struct device *dev,
+				struct samsung_pinctrl_drv_data *drvdata,
+				unsigned int *cnt)
 {
-	struct device *dev = &pdev->dev;
+	struct samsung_pmx_func *functions, *func;
 	struct device_node *dev_np = dev->of_node;
 	struct device_node *cfg_np;
-	struct samsung_pin_group *groups, *grp;
-	struct samsung_pmx_func *functions, *func;
-	unsigned *pin_list;
-	unsigned int npins, grp_cnt, func_idx = 0;
-	char *gname, *fname;
+	unsigned int func_cnt = 0;
 	int ret;
 
-	grp_cnt = of_get_child_count(dev_np);
-	if (!grp_cnt)
-		return -EINVAL;
+	/*
+	 * Iterate over all the child nodes of the pin controller node
+	 * and create pin groups and pin function lists.
+	 */
+	for_each_child_of_node(dev_np, cfg_np) {
+		struct device_node *func_np;
 
-	groups = devm_kzalloc(dev, grp_cnt * sizeof(*groups), GFP_KERNEL);
-	if (!groups) {
-		dev_err(dev, "failed allocate memory for ping group list\n");
-		return -EINVAL;
+		if (!of_get_child_count(cfg_np)) {
+			if (!of_find_property(cfg_np,
+			    "samsung,pin-function", NULL))
+				continue;
+			++func_cnt;
+			continue;
+		}
+
+		for_each_child_of_node(cfg_np, func_np) {
+			if (!of_find_property(func_np,
+			    "samsung,pin-function", NULL))
+				continue;
+			++func_cnt;
+		}
 	}
-	grp = groups;
 
-	functions = devm_kzalloc(dev, grp_cnt * sizeof(*functions), GFP_KERNEL);
+	functions = devm_kzalloc(dev, func_cnt * sizeof(*functions),
+					GFP_KERNEL);
 	if (!functions) {
 		dev_err(dev, "failed to allocate memory for function list\n");
-		return -EINVAL;
+		return ERR_PTR(-EINVAL);
 	}
 	func = functions;
 
@@ -655,61 +757,68 @@ static int samsung_pinctrl_parse_dt(struct platform_device *pdev,
 	 * Iterate over all the child nodes of the pin controller node
 	 * and create pin groups and pin function lists.
 	 */
+	func_cnt = 0;
 	for_each_child_of_node(dev_np, cfg_np) {
-		u32 function;
-		if (!of_find_property(cfg_np, "samsung,pins", NULL))
+		struct device_node *func_np;
+
+		if (!of_get_child_count(cfg_np)) {
+			ret = samsung_pinctrl_create_function(dev, drvdata,
+							cfg_np, func);
+			if (ret < 0)
+				return ERR_PTR(ret);
+			if (ret > 0) {
+				++func;
+				++func_cnt;
+			}
 			continue;
+		}
 
-		ret = samsung_pinctrl_parse_dt_pins(pdev, cfg_np,
-					&drvdata->pctl,	&pin_list, &npins);
-		if (ret)
-			return ret;
-
-		/* derive pin group name from the node name */
-		gname = devm_kzalloc(dev, strlen(cfg_np->name) + GSUFFIX_LEN,
-					GFP_KERNEL);
-		if (!gname) {
-			dev_err(dev, "failed to alloc memory for group name\n");
-			return -ENOMEM;
+		for_each_child_of_node(cfg_np, func_np) {
+			ret = samsung_pinctrl_create_function(dev, drvdata,
+						func_np, func);
+			if (ret < 0)
+				return ERR_PTR(ret);
+			if (ret > 0) {
+				++func;
+				++func_cnt;
+			}
 		}
-		sprintf(gname, "%s%s", cfg_np->name, GROUP_SUFFIX);
+	}
 
-		grp->name = gname;
-		grp->pins = pin_list;
-		grp->num_pins = npins;
-		of_property_read_u32(cfg_np, "samsung,pin-function", &function);
-		grp->func = function;
-		grp++;
+	*cnt = func_cnt;
+	return functions;
+}
 
-		if (!of_find_property(cfg_np, "samsung,pin-function", NULL))
-			continue;
+/*
+ * Parse the information about all the available pin groups and pin functions
+ * from device node of the pin-controller. A pin group is formed with all
+ * the pins listed in the "samsung,pins" property.
+ */
 
-		/* derive function name from the node name */
-		fname = devm_kzalloc(dev, strlen(cfg_np->name) + FSUFFIX_LEN,
-					GFP_KERNEL);
-		if (!fname) {
-			dev_err(dev, "failed to alloc memory for func name\n");
-			return -ENOMEM;
-		}
-		sprintf(fname, "%s%s", cfg_np->name, FUNCTION_SUFFIX);
-
-		func->name = fname;
-		func->groups = devm_kzalloc(dev, sizeof(char *), GFP_KERNEL);
-		if (!func->groups) {
-			dev_err(dev, "failed to alloc memory for group list "
-					"in pin function");
-			return -ENOMEM;
-		}
-		func->groups[0] = gname;
-		func->num_groups = 1;
-		func++;
-		func_idx++;
+static int samsung_pinctrl_parse_dt(struct platform_device *pdev,
+				    struct samsung_pinctrl_drv_data *drvdata)
+{
+	struct device *dev = &pdev->dev;
+	struct samsung_pin_group *groups;
+	struct samsung_pmx_func *functions;
+	unsigned int grp_cnt = 0, func_cnt = 0;
+
+	groups = samsung_pinctrl_create_groups(dev, drvdata, &grp_cnt);
+	if (IS_ERR(groups)) {
+		dev_err(dev, "failed to parse pin groups\n");
+		return PTR_ERR(groups);
+	}
+
+	functions = samsung_pinctrl_create_functions(dev, drvdata, &func_cnt);
+	if (IS_ERR(functions)) {
+		dev_err(dev, "failed to parse pin functions\n");
+		return PTR_ERR(groups);
 	}
 
 	drvdata->pin_groups = groups;
 	drvdata->nr_groups = grp_cnt;
 	drvdata->pmx_functions = functions;
-	drvdata->nr_functions = func_idx;
+	drvdata->nr_functions = func_cnt;
 
 	return 0;
 }
diff --git a/drivers/pinctrl/pinctrl-samsung.h b/drivers/pinctrl/pinctrl-samsung.h
index 4d7566a..5cedc9d 100644
--- a/drivers/pinctrl/pinctrl-samsung.h
+++ b/drivers/pinctrl/pinctrl-samsung.h
@@ -232,6 +232,7 @@ struct samsung_pmx_func {
 	const char		*name;
 	const char		**groups;
 	u8			num_groups;
+	u32			val;
 };
 
 /* list of all exported SoC specific data */
-- 
1.9.3

[PATCH 6/6] pinctrl: samsung: Allow pin value to be initialized using pinfunc

[Tomasz Figa]

This patch extends the range of settings configurable via pinfunc API
to cover pin value as well. This allows configuration of default values
of pins, which is useful for pins that are not supposed to be used by
any dedicated driver, but need certain board-specific setting.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 1 +
 drivers/pinctrl/pinctrl-samsung.c                             | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
index 464b2bb..e82aaf4 100644
--- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
+++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
@@ -75,6 +75,7 @@ Required Properties:
   "samsung,pins" property of the child node. The following pin configuration
   properties are supported.
 
+  - samsung,pin-val: Initial value of pin output buffer.
   - samsung,pin-pud: Pull up/down configuration.
   - samsung,pin-drv: Drive strength configuration.
   - samsung,pin-pud-pdn: Pull up/down configuration in power down mode.
diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
index caa6dbe..76de677 100644
--- a/drivers/pinctrl/pinctrl-samsung.c
+++ b/drivers/pinctrl/pinctrl-samsung.c
@@ -47,6 +47,7 @@ static struct pin_config {
 	{ "samsung,pin-drv", PINCFG_TYPE_DRV },
 	{ "samsung,pin-con-pdn", PINCFG_TYPE_CON_PDN },
 	{ "samsung,pin-pud-pdn", PINCFG_TYPE_PUD_PDN },
+	{ "samsung,pin-val", PINCFG_TYPE_DAT },
 };
 
 /* Global list of devices (struct samsung_pinctrl_drv_data) */
-- 
1.9.3

[PATCH 2/6] pinctrl: samsung: Handle GPIO request and free using pinctrl helpers

[Sachin Kamat]

Hi Tomasz,

On Wed, Jul 2, 2014 at 9:11 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> This patch adds .request() and .free() operations to gpio_chip of
> pinctrl-samsung driver, which call pinctrl request and free helpers to
> request and free pinctrl pin along with GPIO pin.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> ---
>  drivers/pinctrl/pinctrl-samsung.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
> index 779c8bc..6e099d6 100644
> --- a/drivers/pinctrl/pinctrl-samsung.c
> +++ b/drivers/pinctrl/pinctrl-samsung.c
> @@ -779,7 +779,8 @@ static int samsung_pinctrl_register(struct platform_device *pdev,
>                 pin_bank = &drvdata->ctrl->pin_banks[bank];
>                 pin_bank->grange.name = pin_bank->name;
>                 pin_bank->grange.id = bank;
> -               pin_bank->grange.pin_base = pin_bank->pin_base;
> +               pin_bank->grange.pin_base = drvdata->ctrl->base
> +                                               + pin_bank->pin_base;

Is this a fix?

-- 
Regards,
Sachin.

[PATCH 2/6] pinctrl: samsung: Handle GPIO request and free using pinctrl helpers

[Tomasz Figa]

On 04.07.2014 11:41, Sachin Kamat wrote:
> Hi Tomasz,
> 
> On Wed, Jul 2, 2014 at 9:11 PM, Tomasz Figa <t.figa@samsung.com> wrote:
>> This patch adds .request() and .free() operations to gpio_chip of
>> pinctrl-samsung driver, which call pinctrl request and free helpers to
>> request and free pinctrl pin along with GPIO pin.
>>
>> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>> ---
>>  drivers/pinctrl/pinctrl-samsung.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-samsung.c b/drivers/pinctrl/pinctrl-samsung.c
>> index 779c8bc..6e099d6 100644
>> --- a/drivers/pinctrl/pinctrl-samsung.c
>> +++ b/drivers/pinctrl/pinctrl-samsung.c
>> @@ -779,7 +779,8 @@ static int samsung_pinctrl_register(struct platform_device *pdev,
>>                 pin_bank = &drvdata->ctrl->pin_banks[bank];
>>                 pin_bank->grange.name = pin_bank->name;
>>                 pin_bank->grange.id = bank;
>> -               pin_bank->grange.pin_base = pin_bank->pin_base;
>> +               pin_bank->grange.pin_base = drvdata->ctrl->base
>> +                                               + pin_bank->pin_base;
> 
> Is this a fix?
> 

Hmm, could be.

I haven't observed any issues due to this without further patches from
this series, so I'm not sure if this needs to be sent as a separate fix,
but I might split this patch into two if necessary.

Best regards,
Tomasz

[PATCH 1/6] pinctrl: samsung: Decouple direction setting from pinctrl

[Linus Walleij]

On Wed, Jul 2, 2014 at 5:40 PM, Tomasz Figa <t.figa@samsung.com> wrote:

> This patch makes the pinctrl-samsung driver configure GPIO direction on
> its own, without using the pinctrl_gpio_direction_*() "helpers". The
> rationale behind this change is as follows:
>  - pinctrl-samsung does not need translation from GPIO namespace to
>    pinctrl namespace to handle GPIO operations - GPIO chip and offset
>    therein are enough to calculate necessary offsets and bit masks in
>    constant time,
>  - the pinctrl_gpio_direction_*() functions do not do anything useful
>    other than translating the pin into pinctrl namespace and calling the
>    .gpio_set_direction() from pinmux_ops of the controller,
>  - the undesirable side effect of using those helpers is losing the
>    ability to change GPIO direction in atomic context, because they
>    explicitly use a mutex for synchronization,
>
> Results of this patch are:
>  - fixed warnings about scheduling while atomic in code that needs to
>    set GPIO direction in atomic context (e.g. interrupt handler),
>  - reduced overhead of bitbanging drivers that use gpio_direction_*(),
>    e.g. i2c-gpio.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>

Well the cross-call function is actually mostly for the case when
the pinctrl and GPIO drivers are in different files, or you combine
two IP blocks arbitrarily. It's a bit messy anyway.

Patch applied.

Yours,
Linus Walleij

[PATCH 2/6] pinctrl: samsung: Handle GPIO request and free using pinctrl helpers

[Linus Walleij]

On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:

> This patch adds .request() and .free() operations to gpio_chip of
> pinctrl-samsung driver, which call pinctrl request and free helpers to
> request and free pinctrl pin along with GPIO pin.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>

Patch applied.

Yours,
Linus Walleij

[PATCH 3/6] pinctrl: exynos: Consolidate irq_chips of GPIO and WKUP EINTs

[Linus Walleij]

On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:

> Handling of irq_chip operations for GPIO and WKUP external interrupts
> is mostly the same, with the difference being offset of registers.
> However currently the driver has all the code duplicated for both EINT
> types, which is undesirable, because changes in irq_chip operations have
> to be done to both instances of the same code.
>
> This patch fixes this by creating exynos_irq_chip struct that has normal
> irq_chip struct embedded and contain differences between particular EINT
> types, which are three register offsets. One instance of code is removed
> and the new structure is used instead to fetch necessary data instead of
> samsung_pin_ctrl struct used previously.
>
> While at it, the patch removes Exynos-specific fields from
> aforementioned structure to improve layering of the driver.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>

Patch applied.

Yours,
Linus Walleij

[PATCH 4/6] pinctrl: exynos: Lock GPIOs as interrupts when used as EINTs

[Linus Walleij]

On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:

> Currently after configuring a GPIO pin as an interrupt related pinmux
> registers are changed, but there is no protection from calling
> gpio_direction_*() in a badly written driver, which would cause the same
> pinmux register to be reconfigured for regular input/output and this
> disabling interrupt capability of the pin.
>
> This patch addresses this issue by moving pinmux reconfiguration to
> .irq_startup() callback of irq_chip and calling gpio_lock_as_irq()
> helper to prevent reconfiguration of pin direction.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
(...)
> +               .irq_startup = exynos_irq_startup,
> +               .irq_shutdown = exynos_irq_shutdown,

I think you should be using the
.irq_request_resources and .irq_release_resources callbacks instead.

The reason is that startup and shutdown cannot really fail (ret code
is unsigned...), so using the other callbacks is safer.

Can you have a quick look at this before I apply any more of the
Samsung patches?

Yours,
Linus Walleij

[PATCH 4/6] pinctrl: exynos: Lock GPIOs as interrupts when used as EINTs

[Tomasz Figa]

Hi Linus,

On 08.07.2014 11:03, Linus Walleij wrote:
> On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> 
>> Currently after configuring a GPIO pin as an interrupt related pinmux
>> registers are changed, but there is no protection from calling
>> gpio_direction_*() in a badly written driver, which would cause the same
>> pinmux register to be reconfigured for regular input/output and this
>> disabling interrupt capability of the pin.
>>
>> This patch addresses this issue by moving pinmux reconfiguration to
>> .irq_startup() callback of irq_chip and calling gpio_lock_as_irq()
>> helper to prevent reconfiguration of pin direction.
>>
>> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> (...)
>> +               .irq_startup = exynos_irq_startup,
>> +               .irq_shutdown = exynos_irq_shutdown,
> 
> I think you should be using the
> .irq_request_resources and .irq_release_resources callbacks instead.
> 
> The reason is that startup and shutdown cannot really fail (ret code
> is unsigned...), so using the other callbacks is safer.

Hmm, I used the at91 pinctrl driver as an example, but I agree that
request/release_resources would be better. I guess it should be changed
there as well. [Ccing Jean-Jacques and Jean-Christophe]

> 
> Can you have a quick look at this before I apply any more of the
> Samsung patches?

The two remaining patches are pretty much independent from this one and
rest of this series so please take a look at them while I prepare new
version of this patch.

Best regards,
Tomasz

[PATCH 5/6] pinctrl: samsung: Allow grouping multiple pinmux/pinconf nodes

[Linus Walleij]

On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:

> One of remaining limitations of current pinctrl-samsung driver was
> the inability to parse multiple pinmux/pinconf group nodes grouped
> inside a single device tree node. It made defining groups of pins for
> single purpose, but with different parameters very inconvenient.
>
> This patch implements Tegra-like support for grouping multiple pinctrl
> groups inside one device tree node, by completely changing the way
> pin groups and functions are parsed from device tree. The code creating
> pinctrl maps from DT nodes has been borrowed from pinctrl-tegra, while
> the initial creation of groups and functions has been completely
> rewritten with following assumptions:
>  - each group consists of just one pin and does not depend on data
>    from device tree,
>  - each function is represented by a device tree child node of the
>    pin controller, which in turn can contain multiple child nodes
>    for pins that need to have different configuration values.
>
> Device Tree bindings are fully backwards compatible. New functionality
> can be used by defining a new pinctrl group consisting of several child
> nodes, as on following example:
>
>         sd4_bus8: sd4-bus-width8 {
>                 part-1 {
>                         samsung,pins = "gpk0-3", "gpk0-4",
>                                         "gpk0-5", "gpk0-6";
>                         samsung,pin-function = <3>;
>                         samsung,pin-pud = <3>;
>                         samsung,pin-drv = <3>;
>                 };
>                 part-2 {
>                         samsung,pins = "gpk1-3", "gpk1-4",
>                                         "gpk1-5", "gpk1-6";
>                         samsung,pin-function = <4>;
>                         samsung,pin-pud = <4>;
>                         samsung,pin-drv = <3>;
>                 };
>         };
>
> Tested on Exynos4210-Trats board and a custom Exynos4212-based one.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
> Cc: devicetree at vger.kernel.org
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>

Patch applied.

(This decision is mainly based on trust, I got lost in the patch :-)

Yours,
Linus Walleij

[PATCH 6/6] pinctrl: samsung: Allow pin value to be initialized using pinfunc

[Linus Walleij]

On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:

> This patch extends the range of settings configurable via pinfunc API
> to cover pin value as well. This allows configuration of default values
> of pins, which is useful for pins that are not supposed to be used by
> any dedicated driver, but need certain board-specific setting.
>
> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>

Patch applied. It always makes me a little bit sad that we didn't get
generic pin config in place before this driver but now we have to live
with it.

Yours,
Linus Walleij

[PATCH 5/6] pinctrl: samsung: Allow grouping multiple pinmux/pinconf nodes

[Tomasz Figa]

On 09.07.2014 09:32, Linus Walleij wrote:
> On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> 
>> One of remaining limitations of current pinctrl-samsung driver was
>> the inability to parse multiple pinmux/pinconf group nodes grouped
>> inside a single device tree node. It made defining groups of pins for
>> single purpose, but with different parameters very inconvenient.
>>
>> This patch implements Tegra-like support for grouping multiple pinctrl
>> groups inside one device tree node, by completely changing the way
>> pin groups and functions are parsed from device tree. The code creating
>> pinctrl maps from DT nodes has been borrowed from pinctrl-tegra, while
>> the initial creation of groups and functions has been completely
>> rewritten with following assumptions:
>>  - each group consists of just one pin and does not depend on data
>>    from device tree,
>>  - each function is represented by a device tree child node of the
>>    pin controller, which in turn can contain multiple child nodes
>>    for pins that need to have different configuration values.
>>
>> Device Tree bindings are fully backwards compatible. New functionality
>> can be used by defining a new pinctrl group consisting of several child
>> nodes, as on following example:
>>
>>         sd4_bus8: sd4-bus-width8 {
>>                 part-1 {
>>                         samsung,pins = "gpk0-3", "gpk0-4",
>>                                         "gpk0-5", "gpk0-6";
>>                         samsung,pin-function = <3>;
>>                         samsung,pin-pud = <3>;
>>                         samsung,pin-drv = <3>;
>>                 };
>>                 part-2 {
>>                         samsung,pins = "gpk1-3", "gpk1-4",
>>                                         "gpk1-5", "gpk1-6";
>>                         samsung,pin-function = <4>;
>>                         samsung,pin-pud = <4>;
>>                         samsung,pin-drv = <3>;
>>                 };
>>         };
>>
>> Tested on Exynos4210-Trats board and a custom Exynos4212-based one.
>>
>> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Reviewed-by: Stephen Warren <swarren@nvidia.com>
>> Cc: devicetree at vger.kernel.org
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
> 
> Patch applied.
> 
> (This decision is mainly based on trust, I got lost in the patch :-)

Thanks Linus.

The patch mostly transplants the way of parsing the DT of Tegra driver
into our driver, so that a logical pin group could consist from multiple
smaller groups in DT. I posted it first time as a stand alone patch long
time ago and I believe it was positively acknowledged, with some minor
comments only, which I unfortunately didn't have time to address until now.

The next step could be trying to make some code shared, although I'm not
sure how much that could be, due to the need to handle subtle
differences between both bindings in a backwards compatible way. I will
think about it.

Best regards,
Tomasz

[PATCH 6/6] pinctrl: samsung: Allow pin value to be initialized using pinfunc

[Tomasz Figa]

On 09.07.2014 09:35, Linus Walleij wrote:
> On Wed, Jul 2, 2014 at 5:41 PM, Tomasz Figa <t.figa@samsung.com> wrote:
> 
>> This patch extends the range of settings configurable via pinfunc API
>> to cover pin value as well. This allows configuration of default values
>> of pins, which is useful for pins that are not supposed to be used by
>> any dedicated driver, but need certain board-specific setting.
>>
>> Signed-off-by: Tomasz Figa <t.figa@samsung.com>
>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
> 
> Patch applied.

Thanks!

> It always makes me a little bit sad that we didn't get
> generic pin config in place before this driver but now we have to live
> with it.

That's true. :(

Best regards,
Tomasz