[PATCH 1/2] GPIO: gpio-pxa: simplify pxa_gpio_to_irq() and pxa_irq_to_chip()

Simplify the code in gpio-pxa.c and make them based on irq_base.
When not probed from devicetree, initialize irq_base from
PXA_GPIO_TO_IRQ() or MMP_GPIO_TO_IRQ(), respectively, so the non-DT case
still works.

Only tested on PXA3xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/gpio/gpio-pxa.c |   70 +++++++++++------------------------------------
 1 file changed, 16 insertions(+), 54 deletions(-)

diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
index 58a6a63..6d0cb9d 100644
--- a/drivers/gpio/gpio-pxa.c
+++ b/drivers/gpio/gpio-pxa.c

@@ -59,6 +59,7 @@
 #define BANK_OFF(n)	(((n) < 3) ? (n) << 2 : 0x100 + (((n) - 3) << 2))
 
 int pxa_last_gpio;
+static int irq_base;
 
 #ifdef CONFIG_OF
 static struct irq_domain *domain;

@@ -166,63 +167,14 @@ static inline int __gpio_is_occupied(unsigned gpio)
 	return ret;
 }
 
-#ifdef CONFIG_ARCH_PXA
-static inline int __pxa_gpio_to_irq(int gpio)
-{
-	if (gpio_is_pxa_type(gpio_type))
-		return PXA_GPIO_TO_IRQ(gpio);
-	return -1;
-}
-
-static inline int __pxa_irq_to_gpio(int irq)
-{
-	if (gpio_is_pxa_type(gpio_type))
-		return irq - PXA_GPIO_TO_IRQ(0);
-	return -1;
-}
-#else
-static inline int __pxa_gpio_to_irq(int gpio) { return -1; }
-static inline int __pxa_irq_to_gpio(int irq) { return -1; }
-#endif
-
-#ifdef CONFIG_ARCH_MMP
-static inline int __mmp_gpio_to_irq(int gpio)
-{
-	if (gpio_is_mmp_type(gpio_type))
-		return MMP_GPIO_TO_IRQ(gpio);
-	return -1;
-}
-
-static inline int __mmp_irq_to_gpio(int irq)
-{
-	if (gpio_is_mmp_type(gpio_type))
-		return irq - MMP_GPIO_TO_IRQ(0);
-	return -1;
-}
-#else
-static inline int __mmp_gpio_to_irq(int gpio) { return -1; }
-static inline int __mmp_irq_to_gpio(int irq) { return -1; }
-#endif
-
 static int pxa_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
 {
-	int gpio, ret;
-
-	gpio = chip->base + offset;
-	ret = __pxa_gpio_to_irq(gpio);
-	if (ret >= 0)
-		return ret;
-	return __mmp_gpio_to_irq(gpio);
+	return chip->base + offset + irq_base;
 }
 
 int pxa_irq_to_gpio(int irq)
 {
-	int ret;
-
-	ret = __pxa_irq_to_gpio(irq);
-	if (ret >= 0)
-		return ret;
-	return __mmp_irq_to_gpio(irq);
+	return irq - irq_base;
 }
 
 static int pxa_gpio_direction_input(struct gpio_chip *chip, unsigned offset)

@@ -510,7 +462,7 @@ const struct irq_domain_ops pxa_irq_domain_ops = {
 #ifdef CONFIG_OF
 static int __devinit pxa_gpio_probe_dt(struct platform_device *pdev)
 {
-	int ret, nr_banks, nr_gpios, irq_base;
+	int ret, nr_banks, nr_gpios;
 	struct device_node *prev, *next, *np = pdev->dev.of_node;
 	const struct of_device_id *of_id =
 				of_match_device(pxa_gpio_dt_ids, &pdev->dev);

@@ -564,10 +516,20 @@ static int __devinit pxa_gpio_probe(struct platform_device *pdev)
 	int irq0 = 0, irq1 = 0, irq_mux, gpio_offset = 0;
 
 	ret = pxa_gpio_probe_dt(pdev);
-	if (ret < 0)
+	if (ret < 0) {
 		pxa_last_gpio = pxa_gpio_nums();
-	else
+#ifdef CONFIG_ARCH_PXA
+		if (gpio_is_pxa_type(gpio_type))
+			irq_base = PXA_GPIO_TO_IRQ(0);
+#endif
+#ifdef CONFIG_ARCH_MMP
+		if (gpio_is_mmp_type(gpio_type))
+			irq_base = MMP_GPIO_TO_IRQ(0);
+#endif
+	} else {
 		use_of = 1;
+	}
+
 	if (!pxa_last_gpio)
 		return -EINVAL;
 

-- 
1.7.10.4

Provide an of_xlate function for the PXA GPIO chips and make it work for
devicetree environments.

Successfully tested on a PXA3xx board.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/gpio/gpio-pxa.c |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
index 6d0cb9d..f47e6b3 100644
--- a/drivers/gpio/gpio-pxa.c
+++ b/drivers/gpio/gpio-pxa.c

@@ -63,6 +63,7 @@ static int irq_base;
 
 #ifdef CONFIG_OF
 static struct irq_domain *domain;
+static struct device_node *pxa_gpio_of_node;
 #endif
 
 struct pxa_gpio_chip {

@@ -229,6 +230,24 @@ static void pxa_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 				(value ? GPSR_OFFSET : GPCR_OFFSET));
 }
 
+#ifdef CONFIG_OF_GPIO
+static int pxa_gpio_of_xlate(struct gpio_chip *gc,
+			     const struct of_phandle_args *gpiospec,
+			     u32 *flags)
+{
+	if (gpiospec->args[0] > pxa_last_gpio)
+		return -EINVAL;
+
+	if (gc != &pxa_gpio_chips[gpiospec->args[0] / 32].chip)
+		return -EINVAL;
+
+	if (flags)
+		*flags = gpiospec->args[1];
+
+	return gpiospec->args[0] % 32;
+}
+#endif
+
 static int __devinit pxa_init_gpio_chip(int gpio_end,
 					int (*set_wake)(unsigned int, unsigned int))
 {

@@ -256,6 +275,11 @@ static int __devinit pxa_init_gpio_chip(int gpio_end,
 		c->get = pxa_gpio_get;
 		c->set = pxa_gpio_set;
 		c->to_irq = pxa_gpio_to_irq;
+#ifdef CONFIG_OF_GPIO
+		c->of_node = pxa_gpio_of_node;
+		c->of_xlate = pxa_gpio_of_xlate;
+		c->of_gpio_n_cells = 2;
+#endif
 
 		/* number of GPIOs on last bank may be less than 32 */
 		c->ngpio = (gpio + 31 > gpio_end) ? (gpio_end - gpio + 1) : 32;

@@ -457,6 +481,7 @@ static int pxa_irq_domain_map(struct irq_domain *d, unsigned int irq,
 
 const struct irq_domain_ops pxa_irq_domain_ops = {
 	.map	= pxa_irq_domain_map,
+	.xlate	= irq_domain_xlate_twocell,
 };
 
 #ifdef CONFIG_OF

@@ -497,6 +522,7 @@ static int __devinit pxa_gpio_probe_dt(struct platform_device *pdev)
 	}
 	domain = irq_domain_add_legacy(np, nr_gpios, irq_base, 0,
 				       &pxa_irq_domain_ops, NULL);
+	pxa_gpio_of_node = np;
 	return 0;
 err:
 	iounmap(gpio_reg_base);

-- 
1.7.10.4

On Wednesday 25 July 2012, Daniel Mack wrote:

Simplify the code in gpio-pxa.c and make them based on irq_base.
When not probed from devicetree, initialize irq_base from
PXA_GPIO_TO_IRQ() or MMP_GPIO_TO_IRQ(), respectively, so the non-DT case
still works.

Only tested on PXA3xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>

Both look good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

On 25.07.2012 18:17, Arnd Bergmann wrote:

On Wednesday 25 July 2012, Daniel Mack wrote:

quoted

Simplify the code in gpio-pxa.c and make them based on irq_base.
When not probed from devicetree, initialize irq_base from
PXA_GPIO_TO_IRQ() or MMP_GPIO_TO_IRQ(), respectively, so the non-DT case
still works.

Only tested on PXA3xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>

Both look good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks. I somehow lost track about the upstream pathes recently. Who
would queue them up now?


Daniel

On Wednesday 25 July 2012, Daniel Mack wrote:

On 25.07.2012 18:17, Arnd Bergmann wrote:

quoted

On Wednesday 25 July 2012, Daniel Mack wrote:

quoted

Simplify the code in gpio-pxa.c and make them based on irq_base.
When not probed from devicetree, initialize irq_base from
PXA_GPIO_TO_IRQ() or MMP_GPIO_TO_IRQ(), respectively, so the non-DT case
still works.

Only tested on PXA3xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>

Both look good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>

Thanks. I somehow lost track about the upstream pathes recently. Who
would queue them up now?

Haojian Zhuang is usually the person who picks up PXA patches these days.
If he prefers, you can also prepare a series and send them directly to
arm at kernel.org (that's Olof and me, for all practical purposes) for
integration into the arm-soc tree. I have not received many pxa patches
recently, so if he has no other stuff then this might be the more convenient
way.

	Arnd

On Wed, Jul 25, 2012 at 5:35 PM, Daniel Mack [off-list ref] wrote:

Provide an of_xlate function for the PXA GPIO chips and make it work for
devicetree environments.

Successfully tested on a PXA3xx board.

Signed-off-by: Daniel Mack <zonque@gmail.com>

Could the PXA maintainers pls have a look at this patch, if you're working
on DT support especially...

BTW: I guess this is not a fix for issues in the current kernel HEAD?
Is it OK to have this for the next (v3.7) cycle?

Yours,
Linus Walleij

On Wed, Jul 25, 2012 at 9:10 PM, Arnd Bergmann [off-list ref] wrote:

On Wednesday 25 July 2012, Daniel Mack wrote:

quoted

Thanks. I somehow lost track about the upstream pathes recently. Who
would queue them up now?

Haojian Zhuang is usually the person who picks up PXA patches these days.
If he prefers, you can also prepare a series and send them directly to
arm at kernel.org (that's Olof and me, for all practical purposes) for
integration into the arm-soc tree.

Since these two are in drivers/gpio I can take them through the GPIO
tree if desired, but still need some PXA maintainer to ACK them,
however they haven't really listed themselves as maintainers for the
GPIO driver in MAINTAINERS which would be preferable.

Yours,
Linus Walleij

On Sun, Aug 5, 2012 at 7:16 AM, Linus Walleij [off-list ref] wrote:

On Wed, Jul 25, 2012 at 5:35 PM, Daniel Mack [off-list ref] wrote:

quoted

Provide an of_xlate function for the PXA GPIO chips and make it work for
devicetree environments.

Successfully tested on a PXA3xx board.

Signed-off-by: Daniel Mack <zonque@gmail.com>

Could the PXA maintainers pls have a look at this patch, if you're working
on DT support especially...

BTW: I guess this is not a fix for issues in the current kernel HEAD?
Is it OK to have this for the next (v3.7) cycle?

It's a real fix. Since of_get_named_gpio_flags() needs to get flags of gpio.
This function is widely used in mmc/usb/... drivers. So it requires
vendor's gpio
driver to support parse flags from DT script.

gpio-pxa driver doesn't support flags before. Now Daniel's patch can
fix this issue.
So I'm OK on this patch.

Regards
Haojian

Hi Daniel,

On 07/25/12 18:35, Daniel Mack wrote:

[...]

The ifdes above do not look essential.
Can't we drop them and just have else if ... else if?

+	} else {
 		use_of = 1;
+	}
+
 	if (!pxa_last_gpio)
 		return -EINVAL;
 

-- 
Regards,
Igor.

On Sun, Aug 5, 2012 at 5:52 AM, Haojian Zhuang [off-list ref] wrote:

On Sun, Aug 5, 2012 at 7:16 AM, Linus Walleij [off-list ref] wrote:

quoted

On Wed, Jul 25, 2012 at 5:35 PM, Daniel Mack [off-list ref] wrote:

quoted

Provide an of_xlate function for the PXA GPIO chips and make it work for
devicetree environments.

Successfully tested on a PXA3xx board.

Signed-off-by: Daniel Mack <zonque@gmail.com>

Could the PXA maintainers pls have a look at this patch, if you're working
on DT support especially...

BTW: I guess this is not a fix for issues in the current kernel HEAD?
Is it OK to have this for the next (v3.7) cycle?

It's a real fix. Since of_get_named_gpio_flags() needs to get flags of gpio.
This function is widely used in mmc/usb/... drivers. So it requires
vendor's gpio
driver to support parse flags from DT script.

OK I take that as an ACK so I've put it on my fixes branch with
your Acked-by.

Thanks,
Linus Walleij

On 05.08.2012 10:31, Igor Grinberg wrote:

Hi Daniel,

On 07/25/12 18:35, Daniel Mack wrote:

quoted

Simplify the code in gpio-pxa.c and make them based on irq_base.
When not probed from devicetree, initialize irq_base from
PXA_GPIO_TO_IRQ() or MMP_GPIO_TO_IRQ(), respectively, so the non-DT case
still works.

Only tested on PXA3xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/gpio/gpio-pxa.c |   70 +++++++++++------------------------------------
 1 file changed, 16 insertions(+), 54 deletions(-)

diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
index 58a6a63..6d0cb9d 100644
--- a/drivers/gpio/gpio-pxa.c
+++ b/drivers/gpio/gpio-pxa.c

[...]

quoted

@@ -564,10 +516,20 @@ static int __devinit pxa_gpio_probe(struct platform_device *pdev)
 	int irq0 = 0, irq1 = 0, irq_mux, gpio_offset = 0;
 
 	ret = pxa_gpio_probe_dt(pdev);
-	if (ret < 0)
+	if (ret < 0) {
 		pxa_last_gpio = pxa_gpio_nums();
-	else
+#ifdef CONFIG_ARCH_PXA
+		if (gpio_is_pxa_type(gpio_type))
+			irq_base = PXA_GPIO_TO_IRQ(0);
+#endif
+#ifdef CONFIG_ARCH_MMP
+		if (gpio_is_mmp_type(gpio_type))
+			irq_base = MMP_GPIO_TO_IRQ(0);
+#endif

The ifdes above do not look essential.

But they are. In case of !CONFIG_ARCH_MMP, MMP_GPIO_TO_IRQ is undefined.
Same problem for !CONFIG_ARC_PXA.

Can't we drop them and just have else if ... else if?

We can't do that either, as we might have a hybrid kernel that works on
both PXA and MMP platforms, and then the condition is a runtime thing.


Daniel

On Sun, Aug 5, 2012 at 5:40 PM, Linus Walleij [off-list ref] wrote:

On Sun, Aug 5, 2012 at 5:52 AM, Haojian Zhuang [off-list ref] wrote:

quoted

On Sun, Aug 5, 2012 at 7:16 AM, Linus Walleij [off-list ref] wrote:

quoted

On Wed, Jul 25, 2012 at 5:35 PM, Daniel Mack [off-list ref] wrote:

quoted

Provide an of_xlate function for the PXA GPIO chips and make it work for
devicetree environments.

Successfully tested on a PXA3xx board.

Signed-off-by: Daniel Mack <zonque@gmail.com>

Could the PXA maintainers pls have a look at this patch, if you're working
on DT support especially...

BTW: I guess this is not a fix for issues in the current kernel HEAD?
Is it OK to have this for the next (v3.7) cycle?

It's a real fix. Since of_get_named_gpio_flags() needs to get flags of gpio.
This function is widely used in mmc/usb/... drivers. So it requires
vendor's gpio
driver to support parse flags from DT script.

OK I take that as an ACK so I've put it on my fixes branch with
your Acked-by.

Thanks. Let me revert it from pxa git tree.

Regards
Haojian

On 08/05/12 14:56, Daniel Mack wrote:

On 05.08.2012 10:31, Igor Grinberg wrote:

quoted

Hi Daniel,

On 07/25/12 18:35, Daniel Mack wrote:

quoted

Simplify the code in gpio-pxa.c and make them based on irq_base.
When not probed from devicetree, initialize irq_base from
PXA_GPIO_TO_IRQ() or MMP_GPIO_TO_IRQ(), respectively, so the non-DT case
still works.

Only tested on PXA3xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 drivers/gpio/gpio-pxa.c |   70 +++++++++++------------------------------------
 1 file changed, 16 insertions(+), 54 deletions(-)

diff --git a/drivers/gpio/gpio-pxa.c b/drivers/gpio/gpio-pxa.c
index 58a6a63..6d0cb9d 100644
--- a/drivers/gpio/gpio-pxa.c
+++ b/drivers/gpio/gpio-pxa.c

[...]

quoted

@@ -564,10 +516,20 @@ static int __devinit pxa_gpio_probe(struct platform_device *pdev)
 	int irq0 = 0, irq1 = 0, irq_mux, gpio_offset = 0;
 
 	ret = pxa_gpio_probe_dt(pdev);
-	if (ret < 0)
+	if (ret < 0) {
 		pxa_last_gpio = pxa_gpio_nums();
-	else
+#ifdef CONFIG_ARCH_PXA
+		if (gpio_is_pxa_type(gpio_type))
+			irq_base = PXA_GPIO_TO_IRQ(0);
+#endif
+#ifdef CONFIG_ARCH_MMP
+		if (gpio_is_mmp_type(gpio_type))
+			irq_base = MMP_GPIO_TO_IRQ(0);
+#endif

The ifdes above do not look essential.

But they are. In case of !CONFIG_ARCH_MMP, MMP_GPIO_TO_IRQ is undefined.
Same problem for !CONFIG_ARC_PXA.

I see. Ok then. I'm not a huge fan of having the #ifdes inside
the code (functions) and thinking of moving those outside of the function,
I get pretty much the same solution it was before your patch.

I really think that PXA|MMP_GPIO_TO_IRQ should be moved to some common
location (say arch/arm/plat-pxa/) so both are available regardless
of CONFIG_ARCH_MMP|PXA. That will simplify the code even more.
But probably it is too much to ask for that simple patch...
So, I'm fine with the patch.

Thanks

quoted

Can't we drop them and just have else if ... else if?

We can't do that either, as we might have a hybrid kernel that works on
both PXA and MMP platforms, and then the condition is a runtime thing.

Well, that is exactly what I mean...
I mean, the if ... else ... if ... else without the #ifdefs...
But again, it requires some more changes.

-- 
Regards,
Igor.

On Monday 06 August 2012, Igor Grinberg wrote:

I see. Ok then. I'm not a huge fan of having the #ifdes inside
the code (functions) and thinking of moving those outside of the function,
I get pretty much the same solution it was before your patch.

I really think that PXA|MMP_GPIO_TO_IRQ should be moved to some common
location (say arch/arm/plat-pxa/) so both are available regardless
of CONFIG_ARCH_MMP|PXA. That will simplify the code even more.
But probably it is too much to ask for that simple patch...
So, I'm fine with the patch.

Right. I would also not try to spend too much work on making pxa and
mmp coexist better when the plan is to merge them eventually. At that
point, a couple of these hacks can just be removed.

	Arnd