[PATCH 0/2] ARM: shmobile: sh73a0 legacy: Add missing INTCA0 clock

[PATCH 0/2] ARM: shmobile: sh73a0 legacy: Add missing INTCA0 clock

[Geert Uytterhoeven]

	Hi Simon, Magnus,

This patch series adds the missing INTCA0 clock on sh73a0 SoCs, for both
legacy/reference and multi-platform builds.

The INTCA0 clock drives the irqpin controller modules.
Before, it was assumed enabled by the bootloader or reset state.
By making it available to the driver, we make sure it gets enabled when
needed, and allow it to be managed by system or runtime PM.

The first patch can be applied now.
The second patch depends on the series "[PATCH v5 0/6] sh73a0 common clock
framework implementation".

Thanks!

Geert Uytterhoeven (2):
  ARM: shmobile: sh73a0 legacy: Add missing INTCA0 clock for irqpin
    module
  ARM: shmobile: sh73a0 dtsi: Add missing INTCA0 clock for irqpin module

 arch/arm/boot/dts/sh73a0.dtsi            | 15 +++++++++++++++
 arch/arm/mach-shmobile/clock-sh73a0.c    | 10 ++++++++++
 include/dt-bindings/clock/sh73a0-clock.h |  3 +++
 3 files changed, 28 insertions(+)

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/2] ARM: shmobile: sh73a0 legacy/reference: Add missing INTCA0 clock for irqpin module

[Geert Uytterhoeven]

This clock drives the irqpin controller modules.
Before, it was assumed enabled by the bootloader or reset state.
By making it available to the driver, we make sure it gets enabled when
needed, and allow it to be managed by system or runtime PM.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/mach-shmobile/clock-sh73a0.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/mach-shmobile/clock-sh73a0.c b/arch/arm/mach-shmobile/clock-sh73a0.c
index 6b4c1f313cc987c1..3855fb024fdbb1ca 100644
--- a/arch/arm/mach-shmobile/clock-sh73a0.c
+++ b/arch/arm/mach-shmobile/clock-sh73a0.c
@@ -553,6 +553,7 @@ enum { MSTP001,
 	MSTP314, MSTP313, MSTP312, MSTP311,
 	MSTP304, MSTP303, MSTP302, MSTP301, MSTP300,
 	MSTP411, MSTP410, MSTP403,
+	MSTP508,
 	MSTP_NR };
 
 #define MSTP(_parent, _reg, _bit, _flags) \
@@ -597,6 +598,7 @@ static struct clk mstp_clks[MSTP_NR] = {
 	[MSTP411] = MSTP(&div4_clks[DIV4_HP], SMSTPCR4, 11, 0), /* IIC3 */
 	[MSTP410] = MSTP(&div4_clks[DIV4_HP], SMSTPCR4, 10, 0), /* IIC4 */
 	[MSTP403] = MSTP(&r_clk, SMSTPCR4, 3, 0), /* KEYSC */
+	[MSTP508] = MSTP(&div4_clks[DIV4_HP], SMSTPCR5, 8, 0), /* INTCA0 */
 };
 
 /* The lookups structure below includes duplicate entries for some clocks
@@ -677,6 +679,14 @@ static struct clk_lookup lookups[] = {
 	CLKDEV_DEV_ID("i2c-sh_mobile.4", &mstp_clks[MSTP410]), /* I2C4 */
 	CLKDEV_DEV_ID("e6828000.i2c", &mstp_clks[MSTP410]), /* I2C4 */
 	CLKDEV_DEV_ID("sh_keysc.0", &mstp_clks[MSTP403]), /* KEYSC */
+	CLKDEV_DEV_ID("renesas_intc_irqpin.0",	&mstp_clks[MSTP508]), /* INTCA0 */
+	CLKDEV_DEV_ID("e6900000.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
+	CLKDEV_DEV_ID("renesas_intc_irqpin.1",	&mstp_clks[MSTP508]), /* INTCA0 */
+	CLKDEV_DEV_ID("e6900004.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
+	CLKDEV_DEV_ID("renesas_intc_irqpin.2",	&mstp_clks[MSTP508]), /* INTCA0 */
+	CLKDEV_DEV_ID("e6900008.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
+	CLKDEV_DEV_ID("renesas_intc_irqpin.3",	&mstp_clks[MSTP508]), /* INTCA0 */
+	CLKDEV_DEV_ID("e690000c.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
 
 	/* ICK */
 	CLKDEV_ICK_ID("dsit_clk", "sh-mipi-dsi.0", &div6_clks[DIV6_DSIT]),
-- 
1.9.1

[PATCH 2/2] ARM: shmobile: sh73a0 dtsi: Add missing INTCA0 clock for irqpin module

[Geert Uytterhoeven]

This clock drives the irqpin controller modules.
Before, it was assumed enabled by the bootloader or reset state.
By making it available to the driver, we make sure it gets enabled when
needed, and allow it to be managed by system or runtime PM.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: devicetree@vger.kernel.org
---
This depends on the series "[PATCH v5 0/6] sh73a0 common clock framework
implementation".
---
 arch/arm/boot/dts/sh73a0.dtsi            | 15 +++++++++++++++
 include/dt-bindings/clock/sh73a0-clock.h |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/arch/arm/boot/dts/sh73a0.dtsi b/arch/arm/boot/dts/sh73a0.dtsi
index 0853a6fe2209a48a..c7ab55e55e23f8cb 100644
--- a/arch/arm/boot/dts/sh73a0.dtsi
+++ b/arch/arm/boot/dts/sh73a0.dtsi
@@ -78,6 +78,7 @@
 			      0 6 IRQ_TYPE_LEVEL_HIGH
 			      0 7 IRQ_TYPE_LEVEL_HIGH
 			      0 8 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp5_clks SH73A0_CLK_INTCA0>;
 	};
 
 	irqpin1: irqpin@e6900004 {
@@ -97,6 +98,7 @@
 			      0 14 IRQ_TYPE_LEVEL_HIGH
 			      0 15 IRQ_TYPE_LEVEL_HIGH
 			      0 16 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp5_clks SH73A0_CLK_INTCA0>;
 		control-parent;
 	};
 
@@ -117,6 +119,7 @@
 			      0 22 IRQ_TYPE_LEVEL_HIGH
 			      0 23 IRQ_TYPE_LEVEL_HIGH
 			      0 24 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp5_clks SH73A0_CLK_INTCA0>;
 	};
 
 	irqpin3: irqpin@e690000c {
@@ -136,6 +139,7 @@
 			      0 30 IRQ_TYPE_LEVEL_HIGH
 			      0 31 IRQ_TYPE_LEVEL_HIGH
 			      0 32 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp5_clks SH73A0_CLK_INTCA0>;
 	};
 
 	i2c0: i2c@e6820000 {
@@ -705,5 +709,16 @@
 			clock-output-names  				"iic3", "iic4", "keysc";
 		};
+		mstp5_clks: mstp5_clks@e6150144 {
+			compatible = "renesas,sh73a0-mstp-clocks", "renesas,cpg-mstp-clocks";
+			reg = <0xe6150144 4>, <0xe615003c 4>;
+			clocks = <&cpg_clocks SH73A0_CLK_HP>;
+			#clock-cells = <1>;
+			clock-indices = <
+				SH73A0_CLK_INTCA0
+			>;
+			clock-output-names +				"intca0";
+		};
 	};
 };
diff --git a/include/dt-bindings/clock/sh73a0-clock.h b/include/dt-bindings/clock/sh73a0-clock.h
index 1dd3eb2b7d902afd..53369568c24c5dc6 100644
--- a/include/dt-bindings/clock/sh73a0-clock.h
+++ b/include/dt-bindings/clock/sh73a0-clock.h
@@ -76,4 +76,7 @@
 #define SH73A0_CLK_IIC4		10
 #define SH73A0_CLK_KEYSC	3
 
+/* MSTP5 */
+#define SH73A0_CLK_INTCA0	8
+
 #endif
-- 
1.9.1

Re: [PATCH 1/2] ARM: shmobile: sh73a0 legacy/reference: Add missing INTCA0 clock for irqpin module

[Simon Horman]

On Wed, Nov 19, 2014 at 04:26:59PM +0100, Geert Uytterhoeven wrote:
> This clock drives the irqpin controller modules.
> Before, it was assumed enabled by the bootloader or reset state.
> By making it available to the driver, we make sure it gets enabled when
> needed, and allow it to be managed by system or runtime PM.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Hi Geert,

should this be considered a fix?

If so could you cook up some text describing which patch introduced
this problem in which kernel version.

> ---
>  arch/arm/mach-shmobile/clock-sh73a0.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/mach-shmobile/clock-sh73a0.c b/arch/arm/mach-shmobile/clock-sh73a0.c
> index 6b4c1f313cc987c1..3855fb024fdbb1ca 100644
> --- a/arch/arm/mach-shmobile/clock-sh73a0.c
> +++ b/arch/arm/mach-shmobile/clock-sh73a0.c
> @@ -553,6 +553,7 @@ enum { MSTP001,
>  	MSTP314, MSTP313, MSTP312, MSTP311,
>  	MSTP304, MSTP303, MSTP302, MSTP301, MSTP300,
>  	MSTP411, MSTP410, MSTP403,
> +	MSTP508,
>  	MSTP_NR };
>  
>  #define MSTP(_parent, _reg, _bit, _flags) \
> @@ -597,6 +598,7 @@ static struct clk mstp_clks[MSTP_NR] = {
>  	[MSTP411] = MSTP(&div4_clks[DIV4_HP], SMSTPCR4, 11, 0), /* IIC3 */
>  	[MSTP410] = MSTP(&div4_clks[DIV4_HP], SMSTPCR4, 10, 0), /* IIC4 */
>  	[MSTP403] = MSTP(&r_clk, SMSTPCR4, 3, 0), /* KEYSC */
> +	[MSTP508] = MSTP(&div4_clks[DIV4_HP], SMSTPCR5, 8, 0), /* INTCA0 */
>  };
>  
>  /* The lookups structure below includes duplicate entries for some clocks
> @@ -677,6 +679,14 @@ static struct clk_lookup lookups[] = {
>  	CLKDEV_DEV_ID("i2c-sh_mobile.4", &mstp_clks[MSTP410]), /* I2C4 */
>  	CLKDEV_DEV_ID("e6828000.i2c", &mstp_clks[MSTP410]), /* I2C4 */
>  	CLKDEV_DEV_ID("sh_keysc.0", &mstp_clks[MSTP403]), /* KEYSC */
> +	CLKDEV_DEV_ID("renesas_intc_irqpin.0",	&mstp_clks[MSTP508]), /* INTCA0 */
> +	CLKDEV_DEV_ID("e6900000.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
> +	CLKDEV_DEV_ID("renesas_intc_irqpin.1",	&mstp_clks[MSTP508]), /* INTCA0 */
> +	CLKDEV_DEV_ID("e6900004.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
> +	CLKDEV_DEV_ID("renesas_intc_irqpin.2",	&mstp_clks[MSTP508]), /* INTCA0 */
> +	CLKDEV_DEV_ID("e6900008.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
> +	CLKDEV_DEV_ID("renesas_intc_irqpin.3",	&mstp_clks[MSTP508]), /* INTCA0 */
> +	CLKDEV_DEV_ID("e690000c.irqpin",	&mstp_clks[MSTP508]), /* INTCA0 */
>  
>  	/* ICK */
>  	CLKDEV_ICK_ID("dsit_clk", "sh-mipi-dsi.0", &div6_clks[DIV6_DSIT]),
> -- 
> 1.9.1
> 

Re: [PATCH 1/2] ARM: shmobile: sh73a0 legacy/reference: Add missing INTCA0 clock for irqpin module

[Geert Uytterhoeven]

Hi Simon,

On Tue, Nov 25, 2014 at 3:44 AM, Simon Horman <horms@verge.net.au> wrote:
> On Wed, Nov 19, 2014 at 04:26:59PM +0100, Geert Uytterhoeven wrote:
>> This clock drives the irqpin controller modules.
>> Before, it was assumed enabled by the bootloader or reset state.
>> By making it available to the driver, we make sure it gets enabled when
>> needed, and allow it to be managed by system or runtime PM.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Hi Geert,
>
> should this be considered a fix?

In some sense this is a fix. But I don't think anyone is suffering from it.
Just poor guys like me who disable all MSTP clocks in early boot code,
to find more missing clock declarations and more drivers that don't manage
their clocks ;-)

> If so could you cook up some text describing which patch introduced
> this problem in which kernel version.

I guess that would be:

commit 341eb5465f67437ad37ef2f6302b581beda4614a
Author: Magnus Damm <damm@opensource.se>
Date:   Tue Feb 26 12:01:09 2013 +0900

    ARM: shmobile: INTC External IRQ pin driver on sh73a0

But I don't think it's worth adding that, or do you plan to backport
this to LTSI?

Note that you'd also have to backport the following to make this work:

commit705bc96c2c15313c0677607f6e81800f4d2b4534
Author: Geert Uytterhoeven <geert+renesas@glider.be>
Date:   Fri Sep 12 15:15:18 2014 +0200

    irqchip: renesas-intc-irqpin: Add minimal runtime PM support

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] ARM: shmobile: sh73a0 legacy/reference: Add missing INTCA0 clock for irqpin module

[Geert Uytterhoeven]

On Tue, Nov 25, 2014 at 10:31 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Tue, Nov 25, 2014 at 3:44 AM, Simon Horman <horms@verge.net.au> wrote:
>> On Wed, Nov 19, 2014 at 04:26:59PM +0100, Geert Uytterhoeven wrote:
>>> This clock drives the irqpin controller modules.
>>> Before, it was assumed enabled by the bootloader or reset state.
>>> By making it available to the driver, we make sure it gets enabled when
>>> needed, and allow it to be managed by system or runtime PM.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> Hi Geert,
>>
>> should this be considered a fix?
>
> In some sense this is a fix. But I don't think anyone is suffering from it.
> Just poor guys like me who disable all MSTP clocks in early boot code,
> to find more missing clock declarations and more drivers that don't manage
> their clocks ;-)
>
>> If so could you cook up some text describing which patch introduced
>> this problem in which kernel version.
>
> I guess that would be:
>
> commit 341eb5465f67437ad37ef2f6302b581beda4614a
> Author: Magnus Damm <damm@opensource.se>
> Date:   Tue Feb 26 12:01:09 2013 +0900
>
>     ARM: shmobile: INTC External IRQ pin driver on sh73a0
>
> But I don't think it's worth adding that, or do you plan to backport
> this to LTSI?

Seeing the similar fix for r8a7740 has landed in backports, I now assume
you will ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] ARM: shmobile: sh73a0 legacy/reference: Add missing INTCA0 clock for irqpin module

[Simon Horman]

On Tue, Nov 25, 2014 at 11:16:40AM +0100, Geert Uytterhoeven wrote:
> On Tue, Nov 25, 2014 at 10:31 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Tue, Nov 25, 2014 at 3:44 AM, Simon Horman <horms@verge.net.au> wrote:
> >> On Wed, Nov 19, 2014 at 04:26:59PM +0100, Geert Uytterhoeven wrote:
> >>> This clock drives the irqpin controller modules.
> >>> Before, it was assumed enabled by the bootloader or reset state.
> >>> By making it available to the driver, we make sure it gets enabled when
> >>> needed, and allow it to be managed by system or runtime PM.
> >>>
> >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >>
> >> Hi Geert,
> >>
> >> should this be considered a fix?
> >
> > In some sense this is a fix. But I don't think anyone is suffering from it.
> > Just poor guys like me who disable all MSTP clocks in early boot code,
> > to find more missing clock declarations and more drivers that don't manage
> > their clocks ;-)
> >
> >> If so could you cook up some text describing which patch introduced
> >> this problem in which kernel version.
> >
> > I guess that would be:
> >
> > commit 341eb5465f67437ad37ef2f6302b581beda4614a
> > Author: Magnus Damm <damm@opensource.se>
> > Date:   Tue Feb 26 12:01:09 2013 +0900
> >
> >     ARM: shmobile: INTC External IRQ pin driver on sh73a0
> >
> > But I don't think it's worth adding that, or do you plan to backport
> > this to LTSI?

Thanks. My main motivation for asking is that Olof asked about some
recent patches that looked like fixes. So I am trying to be a bit
better at breaking out fixes and documenting them appropriately.

> Seeing the similar fix for r8a7740 has landed in backports, I now assume
> you will ;-)

Yes, I plan to backport this at some point.

Re: [PATCH 1/2] ARM: shmobile: sh73a0 legacy/reference: Add missing INTCA0 clock for irqpin module

[Simon Horman]

Hi Geert,

On Wed, Nov 26, 2014 at 10:31:56AM +0900, Simon Horman wrote:
> On Tue, Nov 25, 2014 at 11:16:40AM +0100, Geert Uytterhoeven wrote:
> > On Tue, Nov 25, 2014 at 10:31 AM, Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Tue, Nov 25, 2014 at 3:44 AM, Simon Horman <horms@verge.net.au> wrote:
> > >> On Wed, Nov 19, 2014 at 04:26:59PM +0100, Geert Uytterhoeven wrote:
> > >>> This clock drives the irqpin controller modules.
> > >>> Before, it was assumed enabled by the bootloader or reset state.
> > >>> By making it available to the driver, we make sure it gets enabled when
> > >>> needed, and allow it to be managed by system or runtime PM.
> > >>>
> > >>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >>
> > >> Hi Geert,
> > >>
> > >> should this be considered a fix?
> > >
> > > In some sense this is a fix. But I don't think anyone is suffering from it.
> > > Just poor guys like me who disable all MSTP clocks in early boot code,
> > > to find more missing clock declarations and more drivers that don't manage
> > > their clocks ;-)
> > >
> > >> If so could you cook up some text describing which patch introduced
> > >> this problem in which kernel version.
> > >
> > > I guess that would be:
> > >
> > > commit 341eb5465f67437ad37ef2f6302b581beda4614a
> > > Author: Magnus Damm <damm@opensource.se>
> > > Date:   Tue Feb 26 12:01:09 2013 +0900
> > >
> > >     ARM: shmobile: INTC External IRQ pin driver on sh73a0
> > >
> > > But I don't think it's worth adding that, or do you plan to backport
> > > this to LTSI?
> 
> Thanks. My main motivation for asking is that Olof asked about some
> recent patches that looked like fixes. So I am trying to be a bit
> better at breaking out fixes and documenting them appropriately.
> 
> > Seeing the similar fix for r8a7740 has landed in backports, I now assume
> > you will ;-)
> 
> Yes, I plan to backport this at some point.

Thanks for all your effort with this change.
After all our discussion of fixes I have decided
just to classify it as a regular change and have queued it up for v3.20.

I expect the discussion earlier in this thread will be useful when
explaining things to the arm-soc maintainers, so thanks for that material.