arm/imx: fix return type of callback passed to of_irq_init()

PATCH New Shawn Guo 3 participants 4 messages Dec 1, 2011 – Dec 2, 2011

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape

lore

Share & tools

lore

Shawn Guo

author

2011-12-01 08:19 UTC #0 lore raw

PATCH 1/2 arm/imx: fix return type of callback passed to of_irq_init()

The of_irq_init() expects the callback passed by .data of of_device_id return 'int' instead of 'void'. This patch fixes it to have irq_init_cb() return the correct value, and in turn have the secondary interrupt controller (gpio in this case) initialized properly and also eliminate the error message 'of_irq_init: children remain, but no parents' which was overlooked before.

Signed-off-by Shawn Guo <[email protected]>

---
 arch/arm/mach-imx/mach-imx6q.c |    4 +++-
 arch/arm/mach-mx5/imx51-dt.c   |    7 +++++--
 arch/arm/mach-mx5/imx53-dt.c   |    7 +++++--
 3 files changed, 13 insertions(+), 5 deletions(-)

arch/arm/mach-imx/mach-imx6q.c +3 -1

index 8bf5fa3..eb7531b 100644
@@ -36,7 +36,7 @@ static void __init imx6q_map_io(void)
 	imx_scu_map_io();
 }
 
-static void __init imx6q_gpio_add_irq_domain(struct device_node *np,
+static int __init imx6q_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -

@@ -44,6 +44,8 @@ static void __init imx6q_gpio_add_irq_domain(struct device_node *np,
 
 	irq_domain_add_simple(np, gpio_irq_base);
 	gpio_irq_base += 32;
+
+	return 0;
 }
 
 static const struct of_device_id imx6q_irq_match[] __initconst = {

arch/arm/mach-mx5/imx51-dt.c +5 -2

index ccc6158..3451a46 100644
@@ -44,13 +44,14 @@ static const struct of_dev_auxdata imx51_auxdata_lookup[] __initconst = {
 	{ /* sentinel */ }
 };
 
-static void __init imx51_tzic_add_irq_domain(struct device_node *np,
+static int __init imx51_tzic_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	irq_domain_add_simple(np, 0);
+	return 0;
 }
 
-static void __init imx51_gpio_add_irq_domain(struct device_node *np,
+static int __init imx51_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -

@@ -58,6 +59,8 @@ static void __init imx51_gpio_add_irq_domain(struct device_node *np,
 
 	irq_domain_add_simple(np, gpio_irq_base);
 	gpio_irq_base += 32;
+
+	return 0;
 }
 
 static const struct of_device_id imx51_irq_match[] __initconst = {

arch/arm/mach-mx5/imx53-dt.c +5 -2

index ccaa0b8..022bc03 100644
@@ -48,13 +48,14 @@ static const struct of_dev_auxdata imx53_auxdata_lookup[] __initconst = {
 	{ /* sentinel */ }
 };
 
-static void __init imx53_tzic_add_irq_domain(struct device_node *np,
+static int __init imx53_tzic_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	irq_domain_add_simple(np, 0);
+	return 0;
 }
 
-static void __init imx53_gpio_add_irq_domain(struct device_node *np,
+static int __init imx53_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
 	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -

@@ -62,6 +63,8 @@ static void __init imx53_gpio_add_irq_domain(struct device_node *np,
 
 	irq_domain_add_simple(np, gpio_irq_base);
 	gpio_irq_base += 32;
+
+	return 0;
 }
 
 static const struct of_device_id imx53_irq_match[] __initconst = {

Signature · 1 line

1.7.4.1

Shawn Guo

author

2011-12-01 08:19 UTC #1 lore raw

PATCH 2/2 arm/imx: fix irq_base for gpio

When gpio core dynamically allocate gpio number for a port, it starts from the end of the total range, 0 ~ ARCH_NR_GPIOS. That said, the earlier a port gets probed, the bigger gpio number it gets assigned. To match this, the irq_base for gpio should be assigned from 'MXC_GPIO_IRQ_START + ARCH_NR_GPIOS' decreasingly.

Signed-off-by Shawn Guo <[email protected]>

---
 arch/arm/mach-imx/mach-imx6q.c |    5 ++---
 arch/arm/mach-mx5/imx51-dt.c   |    5 ++---
 arch/arm/mach-mx5/imx53-dt.c   |    5 ++---
 3 files changed, 6 insertions(+), 9 deletions(-)

arch/arm/mach-imx/mach-imx6q.c +2 -3

index eb7531b..22aa54a 100644
@@ -39,11 +39,10 @@ static void __init imx6q_map_io(void)
 static int __init imx6q_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
-	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -
-				   32 * 7; /* imx6q gets 7 gpio ports */
+	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
 
+	gpio_irq_base -= 32;
 	irq_domain_add_simple(np, gpio_irq_base);
-	gpio_irq_base += 32;
 
 	return 0;
 }

arch/arm/mach-mx5/imx51-dt.c +2 -3

index 3451a46..596edd9 100644
@@ -54,11 +54,10 @@ static int __init imx51_tzic_add_irq_domain(struct device_node *np,
 static int __init imx51_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
-	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -
-				   32 * 4; /* imx51 gets 4 gpio ports */
+	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
 
+	gpio_irq_base -= 32;
 	irq_domain_add_simple(np, gpio_irq_base);
-	gpio_irq_base += 32;
 
 	return 0;
 }

arch/arm/mach-mx5/imx53-dt.c +2 -3

index 022bc03..85bfd5f 100644
@@ -58,11 +58,10 @@ static int __init imx53_tzic_add_irq_domain(struct device_node *np,
 static int __init imx53_gpio_add_irq_domain(struct device_node *np,
 				struct device_node *interrupt_parent)
 {
-	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -
-				   32 * 7; /* imx53 gets 7 gpio ports */
+	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
 
+	gpio_irq_base -= 32;
 	irq_domain_add_simple(np, gpio_irq_base);
-	gpio_irq_base += 32;
 
 	return 0;
 }

Signature · 1 line

1.7.4.1

Troy Kisky

2011-12-01 18:26 UTC #2 lore raw

PATCH 2/2 arm/imx: fix irq_base for gpio

Quoted 6 lines · Shawn Guo

When gpio core dynamically allocate gpio number for a port, it starts from the end of the total range, 0 ~ ARCH_NR_GPIOS. That said, the earlier a port gets probed, the bigger gpio number it gets assigned. To match this, the irq_base for gpio should be assigned from 'MXC_GPIO_IRQ_START + ARCH_NR_GPIOS' decreasingly.

Quoted 6 lines

Signed-off-by Shawn Guo<[email protected]>

---
  arch/arm/mach-imx/mach-imx6q.c |    5 ++---
  arch/arm/mach-mx5/imx51-dt.c   |    5 ++---
  arch/arm/mach-mx5/imx53-dt.c   |    5 ++---
  3 files changed, 6 insertions(+), 9 deletions(-)

I thought the rationale for GPIOLIB to start at the end was to more quickly find a free group. If this isn't important, then I'd rather see GPIOLIB changed
to be increasing. Although I do like that gpio_irq_base is always set the same
way with your patch. Would

static int gpio_irq_base = MXC_GPIO_IRQ_START;

work as well, if you kept it increasing instead ?

Thanks
Troy

Quoted 54 lines

arch/arm/mach-imx/mach-imx6q.c +2 -3

index eb7531b..22aa54a 100644
@@ -39,11 +39,10 @@ static void __init imx6q_map_io(void)
  static int __init imx6q_gpio_add_irq_domain(struct device_node *np,
  				struct device_node *interrupt_parent)
  {
-	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -
-				   32 * 7; /* imx6q gets 7 gpio ports */
+	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
 
+	gpio_irq_base -= 32;
  	irq_domain_add_simple(np, gpio_irq_base);
-	gpio_irq_base += 32;
 
  	return 0;
  }

arch/arm/mach-mx5/imx51-dt.c +2 -3

index 3451a46..596edd9 100644
@@ -54,11 +54,10 @@ static int __init imx51_tzic_add_irq_domain(struct device_node *np,
  static int __init imx51_gpio_add_irq_domain(struct device_node *np,
  				struct device_node *interrupt_parent)
  {
-	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -
-				   32 * 4; /* imx51 gets 4 gpio ports */
+	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
 
+	gpio_irq_base -= 32;
  	irq_domain_add_simple(np, gpio_irq_base);
-	gpio_irq_base += 32;
 
  	return 0;
  }

arch/arm/mach-mx5/imx53-dt.c +2 -3

index 022bc03..85bfd5f 100644
@@ -58,11 +58,10 @@ static int __init imx53_tzic_add_irq_domain(struct device_node *np,
  static int __init imx53_gpio_add_irq_domain(struct device_node *np,
  				struct device_node *interrupt_parent)
  {
-	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS -
-				   32 * 7; /* imx53 gets 7 gpio ports */
+	static int gpio_irq_base = MXC_GPIO_IRQ_START + ARCH_NR_GPIOS;
 
+	gpio_irq_base -= 32;
  	irq_domain_add_simple(np, gpio_irq_base);
-	gpio_irq_base += 32;
 
  	return 0;
  }

Shawn Guo

2011-12-02 03:45 UTC #3 lore raw

PATCH 2/2 arm/imx: fix irq_base for gpio

Quoted 21 lines · Troy Kisky

Quoted 6 lines · Shawn Guo
When gpio core dynamically allocate gpio number for a port, it starts from the end of the total range, 0 ~ ARCH_NR_GPIOS. That said, the earlier a port gets probed, the bigger gpio number it gets assigned. To match this, the irq_base for gpio should be assigned from 'MXC_GPIO_IRQ_START + ARCH_NR_GPIOS' decreasingly.
Quoted 6 lines
Signed-off-by Shawn Guo<[email protected]>
---
 arch/arm/mach-imx/mach-imx6q.c |    5 ++---
 arch/arm/mach-mx5/imx51-dt.c   |    5 ++---
 arch/arm/mach-mx5/imx53-dt.c   |    5 ++---
 3 files changed, 6 insertions(+), 9 deletions(-)
I thought the rationale for GPIOLIB to start at the end was to more quickly find a free group. If this isn't important, then I'd rather see GPIOLIB changed
to be increasing. Although I do like that gpio_irq_base is always set the same
way with your patch. Would

Quoted 4 lines

static int gpio_irq_base = MXC_GPIO_IRQ_START;
work as well, if you kept it increasing instead ?

Inspired by gpio-tegra patches from Stephen, I will send similar patches for gpio-mxc to clean up the use of MXC_GPIO_IRQ_START, which will in turn remove the whole dependency between gpio number and gpio irq. So, yes, it deserves a thorough fix instead of this temporary one.

Signature · 2 lines

Regards,
Shawn