Various improvements for samsung-pinctrl driver

PATCH New Tomasz Figa 4 participants 18 messages Jul 2, 2014 – Jul 9, 2014

Subsystems (3)

Device tree bindings Arm Samsung Exynos Pin control

lore

Share & tools

lore

Tomasz Figa

author

2014-07-02 15:40 UTC #0 lore raw

cover Various improvements for samsung-pinctrl driver

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(-)

Signature · 1 line

1.9.3

Tomasz Figa

author

2014-07-02 15:40 UTC #1 lore raw

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

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 <[email protected]>

---
 drivers/pinctrl/pinctrl-samsung.c | 99 +++++++++++++++++----------------------
 1 file changed, 44 insertions(+), 55 deletions(-)

drivers/pinctrl/pinctrl-samsung.c +44 ~~-55~~

index 3e61d0f..779c8bc 100644
@@ -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);
 }
 
 /*

Signature · 1 line

1.9.3

Linus Walleij

2014-07-08 08:54 UTC #2 lore raw

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

Quoted 14 lines · 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,

Quoted 7 lines

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 <[email protected]>

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

Tomasz Figa

author

2014-07-02 15:41 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/pinctrl/pinctrl-samsung.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

drivers/pinctrl/pinctrl-samsung.c +14 -1

index 779c8bc..6e099d6 100644
@@ -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,

Signature · 1 line

1.9.3

Sachin Kamat

2014-07-04 09:41 UTC #4 lore raw

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

Hi Tomasz,

Quoted 14 lines · 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 <[email protected]>
---
 drivers/pinctrl/pinctrl-samsung.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
drivers/pinctrl/pinctrl-samsung.c +0 -0
index 779c8bc..6e099d6 100644

drivers/pinctrl/pinctrl-samsung.c static int samsung_pinctrl_register(struct platform_device *pdev, · line 783

Quoted 6 lines

drivers/pinctrl/pinctrl-samsung.c +2 -1

@@ -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?

Signature · 2 lines

Regards,
Sachin.

Tomasz Figa

author

2014-07-04 11:00 UTC #5 lore raw

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

Quoted 12 lines · Sachin Kamat

Hi Tomasz,
Quoted 9 lines · 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 <[email protected]>
---
 drivers/pinctrl/pinctrl-samsung.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Quoted 14 lines

drivers/pinctrl/pinctrl-samsung.c static int samsung_pinctrl_register(struct platform_device *pdev, · line 783

Quoted 11 lines

drivers/pinctrl/pinctrl-samsung.c +2 -1

index 779c8bc..6e099d6 100644
@@ -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

Linus Walleij

2014-07-08 08:57 UTC #6 lore raw

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

In reply to Tomasz Figa

Quoted 5 lines · 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 <[email protected]>

Patch applied.

Yours,
Linus Walleij

Tomasz Figa

author

2014-07-02 15:41 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c  | 307 ++++++++------------------------------
 drivers/pinctrl/pinctrl-samsung.h |  17 ---
 2 files changed, 60 insertions(+), 264 deletions(-)

drivers/pinctrl/pinctrl-exynos.c +60 ~~-247~~

index 9609c23..003bfd8 100644
@@ -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",
 	},

drivers/pinctrl/pinctrl-samsung.h +0 ~~-17~~

index b3e41fa..e2dce47 100644
@@ -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 *);

Signature · 1 line

1.9.3

Linus Walleij

2014-07-08 08:59 UTC #8 lore raw

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

Quoted 12 lines · 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.

Quoted 4 lines

While at it, the patch removes Exynos-specific fields from aforementioned structure to improve layering of the driver.
Signed-off-by Tomasz Figa <[email protected]>

Patch applied.

Yours,
Linus Walleij

Tomasz Figa

author

2014-07-02 15:41 UTC #9 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c  | 69 +++++++++++++++++++++++++++++++++++----
 drivers/pinctrl/pinctrl-samsung.h |  1 +
 2 files changed, 64 insertions(+), 6 deletions(-)

drivers/pinctrl/pinctrl-exynos.c +63 -6

index 003bfd8..a209cb4 100644
@@ -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,

drivers/pinctrl/pinctrl-samsung.h +1 -0

index e2dce47..4d7566a 100644
@@ -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
 
 /**

Signature · 1 line

1.9.3

Linus Walleij

2014-07-08 09:03 UTC #10 lore raw

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

Quoted 11 lines · 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 <[email protected]>

(...)

Review

Quoted 2 lines

(quoted diff) +2 -0

+               .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

Tomasz Figa

author

2014-07-08 10:50 UTC #11 lore raw

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

Hi Linus,

Quoted 14 lines · Linus Walleij

Quoted 11 lines · 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 <[email protected]>
(...)

Quoted 8 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+               .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]

Quoted 3 lines

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

Tomasz Figa

author

2014-07-02 15:41 UTC #12 lore raw

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

In reply to 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 <[email protected]>

Acked-by Kyungmin Park <[email protected]>

Reviewed-by Stephen Warren <[email protected]>

Cc devicetree at vger.kernel.org

Cc Rob Herring <[email protected]>

Cc Mark Rutland <[email protected]>

---
 .../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(-)

Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +22 -1

index 2b32783..464b2bb 100644
@@ -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.

drivers/pinctrl/pinctrl-samsung.c +361 ~~-252~~

724 lines not on this page · raw message

drivers/pinctrl/pinctrl-samsung.h +1 -0

index 4d7566a..5cedc9d 100644
@@ -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 */

Signature · 1 line

1.9.3

Linus Walleij

2014-07-09 07:32 UTC #13 lore raw

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

Quoted 38 lines · 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>;
                };
        };

Quoted 8 lines

Tested on Exynos4210-Trats board and a custom Exynos4212-based one.
Signed-off-by Tomasz Figa <[email protected]>
Acked-by Kyungmin Park <[email protected]>
Reviewed-by Stephen Warren <[email protected]>
Cc devicetree at vger.kernel.org
Cc Rob Herring <[email protected]>
Cc Mark Rutland <[email protected]>

Patch applied.

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

Yours,
Linus Walleij

Tomasz Figa

2014-07-09 08:06 UTC #14 lore raw

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

Quoted 49 lines · Linus Walleij

Quoted 38 lines · 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>;
                };
        };
Quoted 8 lines
Tested on Exynos4210-Trats board and a custom Exynos4212-based one.
Signed-off-by Tomasz Figa <[email protected]>
Acked-by Kyungmin Park <[email protected]>
Reviewed-by Stephen Warren <[email protected]>
Cc devicetree at vger.kernel.org
Cc Rob Herring <[email protected]>
Cc Mark Rutland <[email protected]>

Quoted 3 lines

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

Tomasz Figa

author

2014-07-02 15:41 UTC #15 lore raw

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

In reply to 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 <[email protected]>

Acked-by Kyungmin Park <[email protected]>

---
 Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 1 +
 drivers/pinctrl/pinctrl-samsung.c                             | 1 +
 2 files changed, 2 insertions(+)

Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt +1 -0

index 464b2bb..e82aaf4 100644
@@ -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.

drivers/pinctrl/pinctrl-samsung.c +1 -0

index caa6dbe..76de677 100644
@@ -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) */

Signature · 1 line

1.9.3

Linus Walleij

2014-07-09 07:35 UTC #16 lore raw

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

Quoted 7 lines · 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 <[email protected]>
Acked-by Kyungmin Park <[email protected]>

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

Tomasz Figa

2014-07-09 08:07 UTC #17 lore raw

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

Quoted 11 lines · Linus Walleij

Quoted 7 lines · 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 <[email protected]>
Acked-by Kyungmin Park <[email protected]>
Patch applied.

Thanks!

Quoted 3 lines

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