Re: [PATCH v2 14/21] ARM: imx6: convert GPC to stacked domains

2015-01-10 Thread Stefan Agner 
On 2015-01-10 14:34, Marc Zyngier wrote:
 On 2015-01-09 17:40, Stefan Agner wrote:
 Hi Marc,

 On 2015-01-07 18:42, Marc Zyngier wrote:
 IMX6 has been (ab)using the gic_arch_extn to provide
 wakeup from suspend, and it makes a lot of sense to convert
 this code to use stacked domains instead.

 This patch does just this, updating the DT files to actually
 reflect what the HW provides.

 BIG FAT WARNING: because the DTs were so far lying by not
 exposing the fact that the GPC block is actually the first
 interrupt controller in the chain, kernels with this patch
 applied wont have any suspend-resume facility when booted
 with old DTs, and old kernels with updated DTs won't even boot.

 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm/boot/dts/imx6qdl.dtsi  |   6 +-
  arch/arm/boot/dts/imx6sl.dtsi   |   5 +-
  arch/arm/boot/dts/imx6sx.dtsi   |   5 +-
  arch/arm/mach-imx/common.h  |   1 -
  arch/arm/mach-imx/gpc.c | 127 
 
  arch/arm/mach-imx/mach-imx6q.c  |   1 -
  arch/arm/mach-imx/mach-imx6sl.c |   1 -
  arch/arm/mach-imx/mach-imx6sx.c |   1 -
  8 files changed, 116 insertions(+), 31 deletions(-)

 diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
 index 4fc03b7..c16d428 100644
 --- a/arch/arm/boot/dts/imx6qdl.dtsi
 +++ b/arch/arm/boot/dts/imx6qdl.dtsi
 @@ -53,6 +53,7 @@
 interrupt-controller;
 reg = 0x00a01000 0x1000,
   0x00a00100 0x100;
 +   interrupt-parent = intc;
 };

 clocks {
 @@ -82,7 +83,7 @@
 #address-cells = 1;
 #size-cells = 1;
 compatible = simple-bus;
 -   interrupt-parent = intc;
 +   interrupt-parent = gpc;
 ranges;

 dma_apbh: dma-apbh@0011 {
 @@ -122,6 +123,7 @@
 compatible = arm,cortex-a9-twd-timer;
 reg = 0x00a00600 0x20;
 interrupts = 1 13 0xf01;
 +   interrupt-parent = intc;
 clocks = clks IMX6QDL_CLK_TWD;
 };

 @@ -694,8 +696,10 @@
 gpc: gpc@020dc000 {
 compatible = fsl,imx6q-gpc;
 reg = 0x020dc000 0x4000;
 +   interrupt-controller;

 #interrupt-cells = 3; is missing here.
 
 Ah, nice catch!
 
 I tested the patchset on a Colibri iMX6, but the module stopped booting
 at some point. No error, no warn, but it looked like IRQ's are not
 working:

 [1.623939] platform sound: Driver imx-sgtl5000 requests probe
 deferral
 [1.630677] backlight supply power not found, using dummy regulator
 [1.637067] pwm-backlight backlight: unable to request PWM, trying
 legacy API
 [1.644271] pwm-backlight backlight: unable to request legacy PWM
 [1.650534] platform backlight: Driver pwm-backlight requests probe
 deferral
 [1.658080] platform 2028000.ssi: Driver fsl-ssi-dai requests probe
 deferral
 [1.665441] fec 2188000.ethernet eth0: Freescale FEC PHY driver
 [Micrel KSZ8041] (mii_bus:phy_addr=2188000.ethernet:00, irq=-1)
 [1.677157] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
 [freeze]

 I figured out that the GPC code did not get called. After digging
 through the parsing code, I found the reason: irq_find_host always opted
 to intc because this was missing... So, interrupt-cells mandatory for
 all interrupt-controller? Maybe we could add a warn somewhere..?
 
 interrupt-cells has a default of 1, I believe. I suppose I could add a
 WARN_ON in the xlate/alloc functions...
 

I think the problem was in of_irq_find_parent (of drivers/of/irq.c). In
that while, interrupt-controller without interrupt-cells just get
ignored.

 With that in place, it worked fine:

 Tested-by: Stefan Agner ste...@agner.ch
 
 Thanks!
 

 interrupts = 0 89 IRQ_TYPE_LEVEL_HIGH,
  0 90 IRQ_TYPE_LEVEL_HIGH;
 +   interrupt-parent = intc;
 };

 gpr: iomuxc-gpr@020e {
 diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
 index 36ab8e0..35099b7 100644
 --- a/arch/arm/boot/dts/imx6sl.dtsi
 +++ b/arch/arm/boot/dts/imx6sl.dtsi
 @@ -72,6 +72,7 @@
 interrupt-controller;
 reg = 0x00a01000 0x1000,
   0x00a00100 0x100;
 +   interrupt-parent = intc;
 };

 clocks {
 @@ -95,7 +96,7 @@
 #address-cells = 1;
 #size-cells = 1;
 compatible = simple-bus;
 -   interrupt-parent = intc;
 +   interrupt-parent = gpc;
 ranges;

 ocram: sram@0090 {
 @@ -603,7 +604,9 @@
 gpc: gpc@020dc000 {
 compatible = fsl,imx6sl-gpc, fsl,imx6q-gpc;
 reg = 0x020dc000 0x4000;
 +   interrupt-controller;

Re: [PATCH v2 14/21] ARM: imx6: convert GPC to stacked domains

2015-01-10 Thread Marc Zyngier 

On 2015-01-09 17:40, Stefan Agner wrote:

Hi Marc,

On 2015-01-07 18:42, Marc Zyngier wrote:

IMX6 has been (ab)using the gic_arch_extn to provide
wakeup from suspend, and it makes a lot of sense to convert
this code to use stacked domains instead.

This patch does just this, updating the DT files to actually
reflect what the HW provides.

BIG FAT WARNING: because the DTs were so far lying by not
exposing the fact that the GPC block is actually the first
interrupt controller in the chain, kernels with this patch
applied wont have any suspend-resume facility when booted
with old DTs, and old kernels with updated DTs won't even boot.

Signed-off-by: Marc Zyngier marc.zyng...@arm.com
---
 arch/arm/boot/dts/imx6qdl.dtsi  |   6 +-
 arch/arm/boot/dts/imx6sl.dtsi   |   5 +-
 arch/arm/boot/dts/imx6sx.dtsi   |   5 +-
 arch/arm/mach-imx/common.h  |   1 -
 arch/arm/mach-imx/gpc.c | 127 


 arch/arm/mach-imx/mach-imx6q.c  |   1 -
 arch/arm/mach-imx/mach-imx6sl.c |   1 -
 arch/arm/mach-imx/mach-imx6sx.c |   1 -
 8 files changed, 116 insertions(+), 31 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi 
b/arch/arm/boot/dts/imx6qdl.dtsi

index 4fc03b7..c16d428 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -53,6 +53,7 @@
interrupt-controller;
reg = 0x00a01000 0x1000,
  0x00a00100 0x100;
+   interrupt-parent = intc;
};

clocks {
@@ -82,7 +83,7 @@
#address-cells = 1;
#size-cells = 1;
compatible = simple-bus;
-   interrupt-parent = intc;
+   interrupt-parent = gpc;
ranges;

dma_apbh: dma-apbh@0011 {
@@ -122,6 +123,7 @@
compatible = arm,cortex-a9-twd-timer;
reg = 0x00a00600 0x20;
interrupts = 1 13 0xf01;
+   interrupt-parent = intc;
clocks = clks IMX6QDL_CLK_TWD;
};

@@ -694,8 +696,10 @@
gpc: gpc@020dc000 {
compatible = fsl,imx6q-gpc;
reg = 0x020dc000 0x4000;
+   interrupt-controller;


#interrupt-cells = 3; is missing here.


Ah, nice catch!

I tested the patchset on a Colibri iMX6, but the module stopped 
booting

at some point. No error, no warn, but it looked like IRQ's are not
working:

[1.623939] platform sound: Driver imx-sgtl5000 requests probe
deferral
[1.630677] backlight supply power not found, using dummy 
regulator

[1.637067] pwm-backlight backlight: unable to request PWM, trying
legacy API
[1.644271] pwm-backlight backlight: unable to request legacy PWM
[1.650534] platform backlight: Driver pwm-backlight requests 
probe

deferral
[1.658080] platform 2028000.ssi: Driver fsl-ssi-dai requests 
probe

deferral
[1.665441] fec 2188000.ethernet eth0: Freescale FEC PHY driver
[Micrel KSZ8041] (mii_bus:phy_addr=2188000.ethernet:00, irq=-1)
[1.677157] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[freeze]

I figured out that the GPC code did not get called. After digging
through the parsing code, I found the reason: irq_find_host always 
opted

to intc because this was missing... So, interrupt-cells mandatory for
all interrupt-controller? Maybe we could add a warn somewhere..?


interrupt-cells has a default of 1, I believe. I suppose I could add a 
WARN_ON in the xlate/alloc functions...



With that in place, it worked fine:

Tested-by: Stefan Agner ste...@agner.ch


Thanks!




interrupts = 0 89 IRQ_TYPE_LEVEL_HIGH,
 0 90 IRQ_TYPE_LEVEL_HIGH;
+   interrupt-parent = intc;
};

gpr: iomuxc-gpr@020e {
diff --git a/arch/arm/boot/dts/imx6sl.dtsi 
b/arch/arm/boot/dts/imx6sl.dtsi

index 36ab8e0..35099b7 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -72,6 +72,7 @@
interrupt-controller;
reg = 0x00a01000 0x1000,
  0x00a00100 0x100;
+   interrupt-parent = intc;
};

clocks {
@@ -95,7 +96,7 @@
#address-cells = 1;
#size-cells = 1;
compatible = simple-bus;
-   interrupt-parent = intc;
+   interrupt-parent = gpc;
ranges;

ocram: sram@0090 {
@@ -603,7 +604,9 @@
gpc: gpc@020dc000 {
compatible = fsl,imx6sl-gpc, fsl,imx6q-gpc;
reg = 0x020dc000 0x4000;
+   interrupt-controller;
interrupts = 0 89 IRQ_TYPE_LEVEL_HIGH;
+

Re: [PATCH v2 14/21] ARM: imx6: convert GPC to stacked domains

2015-01-09 Thread Stefan Agner 
Hi Marc,

On 2015-01-07 18:42, Marc Zyngier wrote:
 IMX6 has been (ab)using the gic_arch_extn to provide
 wakeup from suspend, and it makes a lot of sense to convert
 this code to use stacked domains instead.
 
 This patch does just this, updating the DT files to actually
 reflect what the HW provides.
 
 BIG FAT WARNING: because the DTs were so far lying by not
 exposing the fact that the GPC block is actually the first
 interrupt controller in the chain, kernels with this patch
 applied wont have any suspend-resume facility when booted
 with old DTs, and old kernels with updated DTs won't even boot.
 
 Signed-off-by: Marc Zyngier marc.zyng...@arm.com
 ---
  arch/arm/boot/dts/imx6qdl.dtsi  |   6 +-
  arch/arm/boot/dts/imx6sl.dtsi   |   5 +-
  arch/arm/boot/dts/imx6sx.dtsi   |   5 +-
  arch/arm/mach-imx/common.h  |   1 -
  arch/arm/mach-imx/gpc.c | 127 
 
  arch/arm/mach-imx/mach-imx6q.c  |   1 -
  arch/arm/mach-imx/mach-imx6sl.c |   1 -
  arch/arm/mach-imx/mach-imx6sx.c |   1 -
  8 files changed, 116 insertions(+), 31 deletions(-)
 
 diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
 index 4fc03b7..c16d428 100644
 --- a/arch/arm/boot/dts/imx6qdl.dtsi
 +++ b/arch/arm/boot/dts/imx6qdl.dtsi
 @@ -53,6 +53,7 @@
   interrupt-controller;
   reg = 0x00a01000 0x1000,
 0x00a00100 0x100;
 + interrupt-parent = intc;
   };
  
   clocks {
 @@ -82,7 +83,7 @@
   #address-cells = 1;
   #size-cells = 1;
   compatible = simple-bus;
 - interrupt-parent = intc;
 + interrupt-parent = gpc;
   ranges;
  
   dma_apbh: dma-apbh@0011 {
 @@ -122,6 +123,7 @@
   compatible = arm,cortex-a9-twd-timer;
   reg = 0x00a00600 0x20;
   interrupts = 1 13 0xf01;
 + interrupt-parent = intc;
   clocks = clks IMX6QDL_CLK_TWD;
   };
  
 @@ -694,8 +696,10 @@
   gpc: gpc@020dc000 {
   compatible = fsl,imx6q-gpc;
   reg = 0x020dc000 0x4000;
 + interrupt-controller;

#interrupt-cells = 3; is missing here.

I tested the patchset on a Colibri iMX6, but the module stopped booting
at some point. No error, no warn, but it looked like IRQ's are not
working:

[1.623939] platform sound: Driver imx-sgtl5000 requests probe
deferral
[1.630677] backlight supply power not found, using dummy regulator
[1.637067] pwm-backlight backlight: unable to request PWM, trying
legacy API
[1.644271] pwm-backlight backlight: unable to request legacy PWM
[1.650534] platform backlight: Driver pwm-backlight requests probe
deferral
[1.658080] platform 2028000.ssi: Driver fsl-ssi-dai requests probe
deferral
[1.665441] fec 2188000.ethernet eth0: Freescale FEC PHY driver
[Micrel KSZ8041] (mii_bus:phy_addr=2188000.ethernet:00, irq=-1)
[1.677157] IPv6: ADDRCONF(NETDEV_UP): eth0: link is not ready
[freeze]

I figured out that the GPC code did not get called. After digging
through the parsing code, I found the reason: irq_find_host always opted
to intc because this was missing... So, interrupt-cells mandatory for
all interrupt-controller? Maybe we could add a warn somewhere..?

With that in place, it worked fine:

Tested-by: Stefan Agner ste...@agner.ch

   interrupts = 0 89 IRQ_TYPE_LEVEL_HIGH,
0 90 IRQ_TYPE_LEVEL_HIGH;
 + interrupt-parent = intc;
   };
  
   gpr: iomuxc-gpr@020e {
 diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
 index 36ab8e0..35099b7 100644
 --- a/arch/arm/boot/dts/imx6sl.dtsi
 +++ b/arch/arm/boot/dts/imx6sl.dtsi
 @@ -72,6 +72,7 @@
   interrupt-controller;
   reg = 0x00a01000 0x1000,
 0x00a00100 0x100;
 + interrupt-parent = intc;
   };
  
   clocks {
 @@ -95,7 +96,7 @@
   #address-cells = 1;
   #size-cells = 1;
   compatible = simple-bus;
 - interrupt-parent = intc;
 + interrupt-parent = gpc;
   ranges;
  
   ocram: sram@0090 {
 @@ -603,7 +604,9 @@
   gpc: gpc@020dc000 {
   compatible = fsl,imx6sl-gpc, fsl,imx6q-gpc;
   reg = 0x020dc000 0x4000;
 + interrupt-controller;
   interrupts = 0 89 IRQ_TYPE_LEVEL_HIGH;
 + interrupt-parent = intc;
   };
  
   gpr: iomuxc-gpr@020e {
 diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
 index 7a24fee..c476e67 100644
 ---

Re: [PATCH v2 14/21] ARM: imx6: convert GPC to stacked domains

2015-01-08 Thread Nishanth Menon 
On 17:42-20150107, Marc Zyngier wrote:
 IMX6 has been (ab)using the gic_arch_extn to provide
 wakeup from suspend, and it makes a lot of sense to convert
 this code to use stacked domains instead.
 
 This patch does just this, updating the DT files to actually
 reflect what the HW provides.
 
minor checkpatch warnings below:

@@ -0,0 +1,14 @@
+CHECK: Alignment should match open parenthesis
+#249: FILE: arch/arm/mach-imx/gpc.c:158:
++static int imx_gpc_domain_alloc(struct irq_domain *domain,
++unsigned int virq,
+WARNING: line over 80 characters
+#272: FILE: arch/arm/mach-imx/gpc.c:181:
++  return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, 
parent_args);
+ERROR: code indent should use tabs where possible
+#305: FILE: arch/arm/mach-imx/gpc.c:209:
++^Ireturn -ENOMEM;$
+NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
+  scripts/cleanfile
-- 
Regards,
Nishanth Menon
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 14/21] ARM: imx6: convert GPC to stacked domains

2015-01-07 Thread Marc Zyngier 
IMX6 has been (ab)using the gic_arch_extn to provide
wakeup from suspend, and it makes a lot of sense to convert
this code to use stacked domains instead.

This patch does just this, updating the DT files to actually
reflect what the HW provides.

BIG FAT WARNING: because the DTs were so far lying by not
exposing the fact that the GPC block is actually the first
interrupt controller in the chain, kernels with this patch
applied wont have any suspend-resume facility when booted
with old DTs, and old kernels with updated DTs won't even boot.

Signed-off-by: Marc Zyngier marc.zyng...@arm.com
---
 arch/arm/boot/dts/imx6qdl.dtsi  |   6 +-
 arch/arm/boot/dts/imx6sl.dtsi   |   5 +-
 arch/arm/boot/dts/imx6sx.dtsi   |   5 +-
 arch/arm/mach-imx/common.h  |   1 -
 arch/arm/mach-imx/gpc.c | 127 
 arch/arm/mach-imx/mach-imx6q.c  |   1 -
 arch/arm/mach-imx/mach-imx6sl.c |   1 -
 arch/arm/mach-imx/mach-imx6sx.c |   1 -
 8 files changed, 116 insertions(+), 31 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 4fc03b7..c16d428 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -53,6 +53,7 @@
interrupt-controller;
reg = 0x00a01000 0x1000,
  0x00a00100 0x100;
+   interrupt-parent = intc;
};
 
clocks {
@@ -82,7 +83,7 @@
#address-cells = 1;
#size-cells = 1;
compatible = simple-bus;
-   interrupt-parent = intc;
+   interrupt-parent = gpc;
ranges;
 
dma_apbh: dma-apbh@0011 {
@@ -122,6 +123,7 @@
compatible = arm,cortex-a9-twd-timer;
reg = 0x00a00600 0x20;
interrupts = 1 13 0xf01;
+   interrupt-parent = intc;
clocks = clks IMX6QDL_CLK_TWD;
};
 
@@ -694,8 +696,10 @@
gpc: gpc@020dc000 {
compatible = fsl,imx6q-gpc;
reg = 0x020dc000 0x4000;
+   interrupt-controller;
interrupts = 0 89 IRQ_TYPE_LEVEL_HIGH,
 0 90 IRQ_TYPE_LEVEL_HIGH;
+   interrupt-parent = intc;
};
 
gpr: iomuxc-gpr@020e {
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 36ab8e0..35099b7 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -72,6 +72,7 @@
interrupt-controller;
reg = 0x00a01000 0x1000,
  0x00a00100 0x100;
+   interrupt-parent = intc;
};
 
clocks {
@@ -95,7 +96,7 @@
#address-cells = 1;
#size-cells = 1;
compatible = simple-bus;
-   interrupt-parent = intc;
+   interrupt-parent = gpc;
ranges;
 
ocram: sram@0090 {
@@ -603,7 +604,9 @@
gpc: gpc@020dc000 {
compatible = fsl,imx6sl-gpc, fsl,imx6q-gpc;
reg = 0x020dc000 0x4000;
+   interrupt-controller;
interrupts = 0 89 IRQ_TYPE_LEVEL_HIGH;
+   interrupt-parent = intc;
};
 
gpr: iomuxc-gpr@020e {
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 7a24fee..c476e67 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -88,6 +88,7 @@
interrupt-controller;
reg = 0x00a01000 0x1000,
  0x00a00100 0x100;
+   interrupt-parent = intc;
};
 
clocks {
@@ -131,7 +132,7 @@
#address-cells = 1;
#size-cells = 1;
compatible = simple-bus;
-   interrupt-parent = intc;
+   interrupt-parent = gpc;
ranges;
 
pmu {
@@ -700,7 +701,9 @@
gpc: gpc@020dc000 {
compatible = fsl,imx6sx-gpc, fsl,imx6q-gpc;
reg = 0x020dc000 0x4000;
+   interrupt-controller;
interrupts = GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH;
+   interrupt-parent = intc;
};
 
iomuxc: iomuxc@020e {
diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h
index cfcdb62..7052302 100644
--- a/arch/arm/mach-imx/common.h
+++ b/arch/arm/mach-imx/common.h
@@ -102,7 +102,6 @@ static inline void imx_scu_map_io(void) {}
 static inline void