pinctrl: samsung: Various fixes

PATCH In review Tomasz Figa 3 participants 13 messages Sep 20, 2012 – Sep 24, 2012

Subsystems (2)

Arm Samsung Exynos Pin control

lore

Share & tools

lore

Tomasz Figa

author

2012-09-20 08:51 UTC #0 lore raw

cover pinctrl: samsung: Various fixes

This patch series contains various fixes of problems and nitpicks spotted when working on adding support for Exynos4x12. See each patch for more detailed information.

TODO:
- setting WEINT pins to EINT function in exynos_wkup_irq_set_type

Tomasz Figa (6):
pinctrl: exynos: Set pin function to EINT in irq_set_type of GPIO
EINT
pinctrl: exynos: Fix typos in gpio/wkup _irq_mask
pinctrl: exynos: Handle only unmasked wakeup interrupts
pinctrl: exynos: Mark exynos_irq_demux_eint as inline
pinctrl: exynos: Correct the detection of wakeup-eint node
pinctrl: samsung: Uninline samsung_pinctrl_get_soc_data

 drivers/pinctrl/pinctrl-exynos.c  | 39 +++++++++++++++++++++++++++++----------
 drivers/pinctrl/pinctrl-exynos.h  |  1 +
 drivers/pinctrl/pinctrl-samsung.c |  2 +-
 3 files changed, 31 insertions(+), 11 deletions(-)

Signature · 1 line

1.7.12

Tomasz Figa

author

2012-09-20 08:51 UTC #1 lore raw

PATCH 1/6 pinctrl: exynos: Set pin function to EINT in irq_set_type of GPIO EINTa

Pins used as GPIO interrupts need to be configured as EINTs. This patch adds the required configuration code to exynos_gpio_irq_set_type, to set the pin as EINT when its interrupt trigger is configured.

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c | 12 ++++++++++++
 drivers/pinctrl/pinctrl-exynos.h |  1 +
 2 files changed, 13 insertions(+)

drivers/pinctrl/pinctrl-exynos.c +12 -0

index 447818d..c2fa85f 100644
@@ -76,9 +76,11 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
 	struct samsung_pinctrl_drv_data *d = irqd->domain->host_data;
 	struct samsung_pin_ctrl *ctrl = d->ctrl;
 	struct exynos_geint_data *edata = irq_data_get_irq_handler_data(irqd);
+	struct samsung_pin_bank *bank = edata->bank;
 	unsigned int shift = EXYNOS_EINT_CON_LEN * edata->pin;
 	unsigned int con, trig_type;
 	unsigned long reg_con = ctrl->geint_con + edata->eint_offset;
+	unsigned int mask;
 
 	switch (type) {
 	case IRQ_TYPE_EDGE_RISING:

@@ -110,6 +112,16 @@ static int exynos_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
 	con &= ~(EXYNOS_EINT_CON_MASK << shift);
 	con |= trig_type << shift;
 	writel(con, d->virt_base + reg_con);
+
+	reg_con = bank->pctl_offset;
+	shift = edata->pin * bank->func_width;
+	mask = (1 << bank->func_width) - 1;
+
+	con = readl(d->virt_base + reg_con);
+	con &= ~(mask << shift);
+	con |= EXYNOS_EINT_FUNC << shift;
+	writel(con, d->virt_base + reg_con);
+
 	return 0;
 }

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

index 5f27ba9..31d0a06 100644
@@ -144,6 +144,7 @@ enum exynos4210_gpio_xc_start {
 #define EXYNOS_WKUP_EMASK_OFFSET	0xF00
 #define EXYNOS_WKUP_EPEND_OFFSET	0xF40
 #define EXYNOS_SVC_OFFSET		0xB08
+#define EXYNOS_EINT_FUNC		0xF
 
 /* helpers to access interrupt service register */
 #define EXYNOS_SVC_GROUP_SHIFT		3

Signature · 1 line

1.7.12

Tomasz Figa

author

2012-09-20 08:51 UTC #2 lore raw

PATCH 2/6 pinctrl: exynos: Fix typos in gpio/wkup _irq_mask

In reply to Tomasz Figa

To mask GPIO/wakeup IRQ, the corresponding bit in mask register has to be set.

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/pinctrl/pinctrl-exynos.c +2 -2

index c2fa85f..46cd637 100644
@@ -58,7 +58,7 @@ static void exynos_gpio_irq_mask(struct irq_data *irqd)
 	unsigned long mask;
 
 	mask = readl(d->virt_base + reg_mask);
-	mask |= ~(1 << edata->pin);
+	mask |= 1 << edata->pin;
 	writel(mask, d->virt_base + reg_mask);
 }

@@ -290,7 +290,7 @@ static void exynos_wkup_irq_mask(struct irq_data *irqd)
 	unsigned long mask;
 
 	mask = readl(d->virt_base + reg_mask);
-	mask &= ~(1 << pin);
+	mask |= 1 << pin;
 	writel(mask, d->virt_base + reg_mask);
 }

Signature · 1 line

1.7.12

Tomasz Figa

author

2012-09-20 08:51 UTC #3 lore raw

PATCH 3/6 pinctrl: exynos: Handle only unmasked wakeup interrupts

In reply to Tomasz Figa

A bit in EINTxx_PEND register is set regardless of interrupt mask, which causes spurious interrupts. To avoid them, the read value of pending register must be masked with current interrupt mask manually.

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

drivers/pinctrl/pinctrl-exynos.c +5 -2

index 46cd637..5648971 100644
@@ -395,12 +395,15 @@ static void exynos_irq_demux_eint16_31(unsigned int irq, struct irq_desc *desc)
 	struct exynos_weint_data *eintd = irq_get_handler_data(irq);
 	struct samsung_pinctrl_drv_data *d = eintd->domain->host_data;
 	unsigned long pend;
+	unsigned long mask;
 
 	chained_irq_enter(chip, desc);
 	pend = readl(d->virt_base + d->ctrl->weint_pend + 0x8);
-	exynos_irq_demux_eint(16, pend, eintd->domain);
+	mask = readl(d->virt_base + d->ctrl->weint_mask + 0x8);
+	exynos_irq_demux_eint(16, pend & ~mask, eintd->domain);
 	pend = readl(d->virt_base + d->ctrl->weint_pend + 0xC);
-	exynos_irq_demux_eint(24, pend, eintd->domain);
+	mask = readl(d->virt_base + d->ctrl->weint_mask + 0xC);
+	exynos_irq_demux_eint(24, pend & ~mask, eintd->domain);
 	chained_irq_exit(chip, desc);
 }

Signature · 1 line

1.7.12

Tomasz Figa

author

2012-09-20 08:51 UTC #4 lore raw

PATCH 4/6 pinctrl: exynos: Mark exynos_irq_demux_eint as inline

In reply to Tomasz Figa

The exynos_irq_demux_eint utility function is used in chained IRQ handler for EINT16-31 to handle multiplexed interrupts. Inlining it should improve the performance a bit.

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/pinctrl/pinctrl-exynos.c +1 -1

index 5648971..49ef5a2 100644
@@ -376,7 +376,7 @@ static void exynos_irq_eint0_15(unsigned int irq, struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
-static void exynos_irq_demux_eint(int irq_base, unsigned long pend,
+static inline void exynos_irq_demux_eint(int irq_base, unsigned long pend,
 					struct irq_domain *domain)
 {
 	unsigned int irq;

Signature · 1 line

1.7.12

Tomasz Figa

author

2012-09-20 08:51 UTC #5 lore raw

PATCH 5/6 pinctrl: exynos: Correct the detection of wakeup-eint node

In reply to Tomasz Figa

Current way of finding the wakeup-eint node scans the whole device tree not only children of the pinctrl node, so it might detect a wakeup-eint node of another pinctrl device.

This patch limits the scope of looking for nodes only to subnodes of the pinctrl node.

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

drivers/pinctrl/pinctrl-exynos.c +9 -5

index 49ef5a2..0d01d19 100644
@@ -431,15 +431,19 @@ static const struct irq_domain_ops exynos_wkup_irqd_ops = {
 static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
 {
 	struct device *dev = d->dev;
-	struct device_node *wkup_np;
+	struct device_node *wkup_np = NULL;
+	struct device_node *np;
 	struct exynos_weint_data *weint_data;
 	int idx, irq;
 
-	wkup_np = of_find_matching_node(dev->of_node, exynos_wkup_irq_ids);
-	if (!wkup_np) {
-		dev_err(dev, "wakeup controller node not found\n");
-		return -ENODEV;
+	for_each_child_of_node(dev->of_node, np) {
+		if (of_match_node(exynos_wkup_irq_ids, np)) {
+			wkup_np = np;
+			break;
+		}
 	}
+	if (!wkup_np)
+		return -ENODEV;
 
 	d->wkup_irqd = irq_domain_add_linear(wkup_np, d->ctrl->nr_wint,
 				&exynos_wkup_irqd_ops, d);

Signature · 1 line

1.7.12

Tomasz Figa

author

2012-09-20 08:51 UTC #6 lore raw

PATCH 6/6 pinctrl: samsung: Uninline samsung_pinctrl_get_soc_data

In reply to Tomasz Figa

Although the function is used only a single time, it is not performance critical and it is pretty heavy, so let the compiler decide whether to inline it instead.

Signed-off-by Tomasz Figa <[email protected]>

Signed-off-by Kyungmin Park <[email protected]>

---
 drivers/pinctrl/pinctrl-samsung.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index 8a24223..dd108a9 100644
@@ -776,7 +776,7 @@ static int __init samsung_gpiolib_unregister(struct platform_device *pdev,
 static const struct of_device_id samsung_pinctrl_dt_match[];
 
 /* retrieve the soc specific data */
-static inline struct samsung_pin_ctrl *samsung_pinctrl_get_soc_data(
+static struct samsung_pin_ctrl *samsung_pinctrl_get_soc_data(
 				struct platform_device *pdev)
 {
 	int id;

Signature · 1 line

1.7.12

Linus Walleij

2012-09-20 10:25 UTC #7 lore raw

cover pinctrl: samsung: Various fixes

In reply to Tomasz Figa

Quoted 3 lines · Tomasz Figa

This patch series contains various fixes of problems and nitpicks spotted when working on adding support for Exynos4x12. See each patch for more detailed information.

This all looks good, do you want me to take this into the pinctrl tree or will it go through Samsung ARM SoC (Kyungmin)?

In the latter case, Acked-by: Linus Walleij <[email protected]> for all, else poke me and I'll apply them.

Yours,
Linus Walleij

Kukjin Kim

2012-09-20 14:29 UTC #8 lore raw

cover pinctrl: samsung: Various fixes

Quoted 6 lines · Linus Walleij

Quoted 3 lines · Tomasz Figa
This patch series contains various fixes of problems and nitpicks spotted when working on adding support for Exynos4x12. See each patch for more detailed information.

Quoted 6 lines

This all looks good, do you want me to take this into the pinctrl tree or will it go through Samsung ARM SoC (Kyungmin)?
In the latter case, Acked-by: Linus Walleij<[email protected]> for all, else poke me and I'll apply them.

Linus, thanks for your ack and looks OK to me. Since now pinctrl-exynos driver is in my tree so let me pick this series on top of it into Samsung tree with your ack.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Tomasz Figa

author

2012-09-20 14:55 UTC #9 lore raw

cover pinctrl: samsung: Various fixes

Hi Kukjin,

Quoted 2 lines · On Thursday 20 of September 2012 23:29:13 Kukjin Kim wrote:

Quoted 1 line · Linus Walleij
On Thu, Sep 20, 2012 at 10:51 AM, Tomasz Figa<[email protected]>

Quoted 7 lines · wrote:

Quoted 7 lines
Quoted 3 lines
This patch series contains various fixes of problems and nitpicks spotted when working on adding support for Exynos4x12. See each patch for more detailed information.
This all looks good, do you want me to take this into the pinctrl tree or will it go through Samsung ARM SoC (Kyungmin)?

Quoted 6 lines

Quoted 2 lines
In the latter case, Acked-by: Linus Walleij<[email protected]> for all, else poke me and I'll apply them.
Linus, thanks for your ack and looks OK to me. Since now pinctrl-exynos driver is in my tree so let me pick this series on top of it into Samsung tree with your ack.

I just posted an additional last minute trivial patch which fixes another typo I found.

Could you take it with this series?

Best regards,

Signature · 2 lines

Tomasz Figa
Samsung Poland R&D Center

Kukjin Kim

2012-09-20 22:13 UTC #10 lore raw

cover pinctrl: samsung: Various fixes

Quoted 3 lines · Tomasz Figa

Hi Kukjin,

Hi,

Quoted 14 lines · On Thursday 20 of September 2012 23:29:13 Kukjin Kim wrote:

Quoted 2 lines · Linus Walleij
Quoted 1 line
On Thu, Sep 20, 2012 at 10:51 AM, Tomasz Figa<[email protected]>
Quoted 10 lines · wrote:
Quoted 9 lines
Quoted 3 lines
This patch series contains various fixes of problems and nitpicks spotted when working on adding support for Exynos4x12. See each patch for more detailed information.
This all looks good, do you want me to take this into the pinctrl tree or will it go through Samsung ARM SoC (Kyungmin)?
In the latter case, Acked-by: Linus Walleij<[email protected]> for all, else poke me and I'll apply them.

Quoted 9 lines

Quoted 3 lines
Linus, thanks for your ack and looks OK to me. Since now pinctrl-exynos driver is in my tree so let me pick this series on top of it into Samsung tree with your ack.
I just posted an additional last minute trivial patch which fixes another typo I found.
Could you take it with this series?

Sure, your "[PATCH 7/6] pinctrl: exynos: Fix wakeup IRQ domain registration check" is correct. I will pick that up with this series.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Tomasz Figa

author

2012-09-20 14:53 UTC #11 lore raw

[PATCH 7/6] pinctrl: exynos: Fix wakeup IRQ domain registration check

In reply to Tomasz Figa

Because of a typo, incorrect field of a structure was being checked. This patch fixes the check to use correct field.

Signed-off-by Tomasz Figa <[email protected]>

---
 drivers/pinctrl/pinctrl-exynos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/pinctrl/pinctrl-exynos.c +1 -1

index 0d01d19..21362f4 100644
@@ -447,7 +447,7 @@ static int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
 
 	d->wkup_irqd = irq_domain_add_linear(wkup_np, d->ctrl->nr_wint,
 				&exynos_wkup_irqd_ops, d);
-	if (!d->gpio_irqd) {
+	if (!d->wkup_irqd) {
 		dev_err(dev, "wakeup irq domain allocation failed\n");
 		return -ENXIO;
 	}

Signature · 1 line

1.7.12

Linus Walleij

Acked-by

2012-09-24 19:57 UTC #12 lore raw

[PATCH 7/6] pinctrl: exynos: Fix wakeup IRQ domain registration check

Quoted 4 lines · Tomasz Figa

Because of a typo, incorrect field of a structure was being checked. This patch fixes the check to use correct field.
Signed-off-by Tomasz Figa <[email protected]>

Acked-by Linus Walleij <[email protected]>

I assume you'll take this through the Samsung tree.

Yours,
Linus Walleij