[PATCH/RFC 00/11] ARM: shmobile: Let CPG use syscon for MD pin values

[PATCH/RFC 00/11] ARM: shmobile: Let CPG use syscon for MD pin values

[Geert Uytterhoeven]

	Hi,

Currently the R-Car Gen2 Clock Pulse Generator (CPG) driver obtains the
values of the MD pins by a direct call from the R-Car Gen2 platform
code. This is a bit messy, and creates a dependency between driver and
platform code.

This RFC patch series makes the CPG driver obtain the MD pin values by
reading the Mode Monitoring Register (MODEMR) using syscon and regmap
instead.  The syscon device and register offset to use are obtained from
the "renesas,modemr" property in DT, which contains a phandle to the Reset
Controller module, and the register offset of the MODEMR register inside the
RST module's register block.

For backward-compatibility with old DTs, a fallback is added to call
rcar_gen2_read_mode_pins() in the R-Car Gen2 platform code when needed.
Once backward-compatibility is deemed no longer necessary, the call to
rcar_gen2_read_mode_pins() can be removed.

Patches:
  - Patch 1 adds syscon/regmap support to the CPG driver,
  - Patches 2-5 add device nodes for the RST modules on all R-Car Gen2
    SoCs,
  - Patches 6-9 add "renesas,modemr" links on all R-Car Gen2 SoCs,
  - Patch 10 removes the old call from platform code to CPG driver,
  - Patch 11 removes the obsolete code called by the platform code from
    the CPG driver.

TODO:
  - Write DT binding docs for the RST Reset Controller Module,
  - Document the "renesas,modemr" DT property in the R-Car Gen2 CPG DT
    binding docs,
  - Extend this technique to R-Car Gen1 (r8a7778/r8a7779) and RZ
    (r7s72100) SoCs.

Dependencies:
  - This patch series is against my local tree, which a.o. contains the
    R-Car Gen2 CPG/MSTP Clock Domain and R-Car Gen2 SYSC PM Domain
    patches.  This mostly impacts the .dtsi patches.  But there's no
    functional dependency, only contextually.
  - Patch 10 (run-time) depends on patches 1-9,
  - Patch 11 (build-time) depends on patch 10.

Geert Uytterhoeven (11):
  clk: shmobile: rcar-gen2: Obtain MD pin values using syscon/regmap
  ARM: shmobile: r8a7790 dtsi: Add device node for RST module
  ARM: shmobile: r8a7791 dtsi: Add device node for RST module
  ARM: shmobile: r8a7793 dtsi: Add device node for RST module
  ARM: shmobile: r8a7794 dtsi: Add device node for RST module
  ARM: shmobile: r8a7790: Link CPG to RST using "renesas,modemr"
  ARM: shmobile: r8a7791: Link CPG to RST using "renesas,modemr"
  ARM: shmobile: r8a7793: Link CPG to RST using "renesas,modemr"
  ARM: shmobile: r8a7794: Link CPG to RST using "renesas,modemr"
  ARM: shmobile: rcar-gen2: Use syscon to configure the CPG
  clk: shmobile: rcar-gen2: Remove obsolete rcar_gen2_clocks_init()

 arch/arm/boot/dts/r8a7790.dtsi           |  6 ++++++
 arch/arm/boot/dts/r8a7791.dtsi           |  6 ++++++
 arch/arm/boot/dts/r8a7793.dtsi           |  6 ++++++
 arch/arm/boot/dts/r8a7794.dtsi           |  6 ++++++
 arch/arm/mach-shmobile/Kconfig           |  1 +
 arch/arm/mach-shmobile/setup-rcar-gen2.c |  3 ++-
 drivers/clk/shmobile/clk-rcar-gen2.c     | 22 +++++++++++++++-------
 include/linux/clk/shmobile.h             |  1 -
 8 files changed, 42 insertions(+), 9 deletions(-)

-- 
1.9.1

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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/RFC 01/11] clk: shmobile: rcar-gen2: Obtain MD pin values using syscon/regmap

[Geert Uytterhoeven]

Obtain the values of the MD pins by reading the Mode Monitoring Register
(MODEMR) using syscon and regmap. The syscon device and register offset
to use are obtained from the "renesas,modemr" property in DT.

Fall back to calling rcar_gen2_read_mode_pins() in the R-Car Gen2
platform code for backward-compatibility with old DTs.

TODO Document "renesas,modemr" property in DT binding doc.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/shmobile/clk-rcar-gen2.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c b/drivers/clk/shmobile/clk-rcar-gen2.c
index 745496f7ee9c7402..b121a49f7575a97b 100644
--- a/drivers/clk/shmobile/clk-rcar-gen2.c
+++ b/drivers/clk/shmobile/clk-rcar-gen2.c
@@ -15,8 +15,10 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/math64.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
@@ -368,9 +370,22 @@ static void __init rcar_gen2_cpg_clocks_init(struct device_node *np)
 {
 	const struct cpg_pll_config *config;
 	struct rcar_gen2_cpg *cpg;
+	struct regmap *regmap;
 	struct clk **clks;
 	unsigned int i;
 	int num_clks;
+	u32 reg;
+
+	regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr");
+	if (IS_ERR(regmap) ||
+	    of_property_read_u32_index(np, "renesas,modemr", 1, &reg) ||
+	    regmap_read(regmap, reg, &cpg_mode)) {
+		/* Backward-compatibility with old DT */
+		extern u32 rcar_gen2_read_mode_pins(void);
+
+		pr_warn("%s: failed to parse renesas,modemr\n", np->full_name);
+		cpg_mode = rcar_gen2_read_mode_pins();
+	}
 
 	num_clks = of_property_count_strings(np, "clock-output-names");
 	if (num_clks < 0) {
-- 
1.9.1

[PATCH/RFC 02/11] ARM: shmobile: r8a7790 dtsi: Add device node for RST module

[Geert Uytterhoeven]

Add a device node for the RST module, which provides reset control.
Declare it to be a system controller (syscon), so other device nodes can
refer to its registers.

TODO Add DT binding documentation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7790.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 837090ec9b4fb987..08067ae177643b8f 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -1495,6 +1495,11 @@
 		};
 	};
 
+	rst: reset-controller at e616000 {
+		compatible = "renesas,rst-r8a7790", "syscon";
+		reg = <0 0xe6160000 0 0x0100>;
+	};
+
 	sysc: system-controller at e6180000 {
 		compatible = "renesas,sysc-r8a7790", "renesas,sysc-rcar";
 		reg = <0 0xe6180000 0 0x0200>;
-- 
1.9.1

[PATCH/RFC 03/11] ARM: shmobile: r8a7791 dtsi: Add device node for RST module

[Geert Uytterhoeven]

Add a device node for the RST module, which provides reset control.
Declare it to be a system controller (syscon), so other device nodes can
refer to its registers.

TODO Add DT binding documentation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7791.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index b83f63bd2b1ae6e9..17a6e0fd779fb7a4 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -1405,6 +1405,11 @@
 		};
 	};
 
+	rst: reset-controller at e616000 {
+		compatible = "renesas,rst-r8a7791", "syscon";
+		reg = <0 0xe6160000 0 0x0100>;
+	};
+
 	sysc: system-controller at e6180000 {
 		compatible = "renesas,sysc-r8a7791", "renesas,sysc-rcar";
 		reg = <0 0xe6180000 0 0x0200>;
-- 
1.9.1

[PATCH/RFC 04/11] ARM: shmobile: r8a7793 dtsi: Add device node for RST module

[Geert Uytterhoeven]

Add a device node for the RST module, which provides reset control.
Declare it to be a system controller (syscon), so other device nodes can
refer to its registers.

TODO Add DT binding documentation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7793.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7793.dtsi b/arch/arm/boot/dts/r8a7793.dtsi
index 420fafda442b8a0e..fbb1aa3740e2b55e 100644
--- a/arch/arm/boot/dts/r8a7793.dtsi
+++ b/arch/arm/boot/dts/r8a7793.dtsi
@@ -394,6 +394,11 @@
 		};
 	};
 
+	rst: reset-controller at e616000 {
+		compatible = "renesas,rst-r8a7793", "syscon";
+		reg = <0 0xe6160000 0 0x0100>;
+	};
+
 	sysc: system-controller at e6180000 {
 		compatible = "renesas,sysc-r8a7793", "renesas,sysc-rcar";
 		reg = <0 0xe6180000 0 0x0200>;
-- 
1.9.1

[PATCH/RFC 05/11] ARM: shmobile: r8a7794 dtsi: Add device node for RST module

[Geert Uytterhoeven]

Add a device node for the RST module, which provides reset control.
Declare it to be a system controller (syscon), so other device nodes can
refer to its registers.

TODO Add DT binding documentation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7794.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi
index b1505d2537772e82..bd37ec229329ed9c 100644
--- a/arch/arm/boot/dts/r8a7794.dtsi
+++ b/arch/arm/boot/dts/r8a7794.dtsi
@@ -786,6 +786,11 @@
 		};
 	};
 
+	rst: reset-controller at e616000 {
+		compatible = "renesas,rst-r8a7794", "syscon";
+		reg = <0 0xe6160000 0 0x0100>;
+	};
+
 	sysc: system-controller at e6180000 {
 		compatible = "renesas,sysc-r8a7794", "renesas,sysc-rcar";
 		reg = <0 0xe6180000 0 0x0200>;
-- 
1.9.1

[PATCH/RFC 06/11] ARM: shmobile: r8a7790: Link CPG to RST using "renesas, modemr"

[Geert Uytterhoeven]

Add a link from the Clock Pulse Generator node to the Reset Controller
node, so the CPG can read the Mode Monitoring Register (MODEMR) to
obtain the MD pin values.x

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7790.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 08067ae177643b8f..4ee5523fc3e13e12 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -1115,6 +1115,7 @@
 					     "lb", "qspi", "sdh", "sd0", "sd1",
 					     "z", "rcan", "adsp";
 			#power-domain-cells = <0>;
+			renesas,modemr = <&rst 0x60>;
 		};
 
 		/* Variable factor clocks */
-- 
1.9.1

[PATCH/RFC 06/11] ARM: shmobile: r8a7790: Link CPG to RST using "renesas, modemr"

[Laurent Pinchart]

Hi Geert,

Thank you for the patches.

On Tuesday 07 July 2015 16:10:12 Geert Uytterhoeven wrote:
> Add a link from the Clock Pulse Generator node to the Reset Controller
> node, so the CPG can read the Mode Monitoring Register (MODEMR) to
> obtain the MD pin values.x
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm/boot/dts/r8a7790.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
> index 08067ae177643b8f..4ee5523fc3e13e12 100644
> --- a/arch/arm/boot/dts/r8a7790.dtsi
> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> @@ -1115,6 +1115,7 @@
>  					     "lb", "qspi", "sdh", "sd0", "sd1",
>  					     "z", "rcan", "adsp";
>  			#power-domain-cells = <0>;
> +			renesas,modemr = <&rst 0x60>;

I have mixed feelings about this as I don't think it really describes the 
hardware.

Wouldn't it make sense to instead add a standard kernel API to retrieve the 
boot mode value ? It seems to be a pretty common feature of SoCs across all 
vendors.

>  		};
> 
>  		/* Variable factor clocks */

-- 
Regards,

Laurent Pinchart

[PATCH/RFC 06/11] ARM: shmobile: r8a7790: Link CPG to RST using "renesas, modemr"

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Jul 7, 2015 at 4:23 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday 07 July 2015 16:10:12 Geert Uytterhoeven wrote:
>> Add a link from the Clock Pulse Generator node to the Reset Controller
>> node, so the CPG can read the Mode Monitoring Register (MODEMR) to
>> obtain the MD pin values.x
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  arch/arm/boot/dts/r8a7790.dtsi | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
>> index 08067ae177643b8f..4ee5523fc3e13e12 100644
>> --- a/arch/arm/boot/dts/r8a7790.dtsi
>> +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> @@ -1115,6 +1115,7 @@
>>                                            "lb", "qspi", "sdh", "sd0", "sd1",
>>                                            "z", "rcan", "adsp";
>>                       #power-domain-cells = <0>;
>> +                     renesas,modemr = <&rst 0x60>;
>
> I have mixed feelings about this as I don't think it really describes the
> hardware.

>From the R-Car Gen2 manual:

8. Reset (RST)
8.1 Features
The following functions are implemented by RST.
[...]
Latching of the levels on mode pins when PRESET# is negated
Mode monitoring register

7. Clock Pulse Generator (CPG)
7.2 Input/Output Pins
Table 7.1 lists the CPG pin configuration.
Table 7.1 Pin Configuration and Functions of CPG
Pin Name Function I/O Description
[...]
MD0 Mode 0 ...

Hence there definitely is a link between the (latched) values in the RST module
and CPG configuration. This link is expressed using the "renesas,modemr"
property, where the phandle provides the link to the RST block, and the
register offset provides a way for software to read the configuration.

> Wouldn't it make sense to instead add a standard kernel API to retrieve the
> boot mode value ? It seems to be a pretty common feature of SoCs across all
> vendors.

What format would the boot mode value be in? One u32 word, an array
of u32s?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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/RFC 06/11] ARM: shmobile: r8a7790: Link CPG to RST using "renesas, modemr"

[Laurent Pinchart]

Hi Geert,

On Tuesday 07 July 2015 16:58:21 Geert Uytterhoeven wrote:
> On Tue, Jul 7, 2015 at 4:23 PM, Laurent Pinchart wrote:
> > On Tuesday 07 July 2015 16:10:12 Geert Uytterhoeven wrote:
> >> Add a link from the Clock Pulse Generator node to the Reset Controller
> >> node, so the CPG can read the Mode Monitoring Register (MODEMR) to
> >> obtain the MD pin values.x
> >> 
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >> 
> >>  arch/arm/boot/dts/r8a7790.dtsi | 1 +
> >>  1 file changed, 1 insertion(+)
> >> 
> >> diff --git a/arch/arm/boot/dts/r8a7790.dtsi
> >> b/arch/arm/boot/dts/r8a7790.dtsi index
> >> 08067ae177643b8f..4ee5523fc3e13e12 100644
> >> --- a/arch/arm/boot/dts/r8a7790.dtsi
> >> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> >> @@ -1115,6 +1115,7 @@
> >>                                            "lb", "qspi", "sdh", "sd0",
> >>                                            "sd1",
> >>                                            "z", "rcan", "adsp";
> >>                       #power-domain-cells = <0>;
> >> +                     renesas,modemr = <&rst 0x60>;
> > 
> > I have mixed feelings about this as I don't think it really describes the
> > hardware.
> 
> From the R-Car Gen2 manual:
> 
> 8. Reset (RST)
> 8.1 Features
> The following functions are implemented by RST.
> [...]
> Latching of the levels on mode pins when PRESET# is negated
> Mode monitoring register
> 
> 7. Clock Pulse Generator (CPG)
> 7.2 Input/Output Pins
> Table 7.1 lists the CPG pin configuration.
> Table 7.1 Pin Configuration and Functions of CPG
> Pin Name Function I/O Description
> [...]
> MD0 Mode 0 ...
> 
> Hence there definitely is a link between the (latched) values in the RST
> module and CPG configuration. This link is expressed using the
> "renesas,modemr" property, where the phandle provides the link to the RST
> block, and the register offset provides a way for software to read the
> configuration.

The mode bits of course influence the CPG (otherwise we wouldn't be having 
this discussion :-)), but to me it looks more like a configuration broadcast 
through the whole SoC than a real link between two IP cores. It's obviously 
subject to interpretation.

> > Wouldn't it make sense to instead add a standard kernel API to retrieve
> > the boot mode value ? It seems to be a pretty common feature of SoCs
> > across all vendors.
> 
> What format would the boot mode value be in? One u32 word, an array
> of u32s?

I'd go for a u32 as that's what all the vendors I came across use. It could 
easily be extended to a u64 or an array of u32/u64 later if needed. The format 
of the boot mode value will be SoC-specific, but that's also true of the mode 
register that would be read through syscon.

-- 
Regards,

Laurent Pinchart

[PATCH/RFC 06/11] ARM: shmobile: r8a7790: Link CPG to RST using "renesas, modemr"

[Geert Uytterhoeven]

Hi Laurent,

On Tue, Jul 7, 2015 at 6:20 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> >> --- a/arch/arm/boot/dts/r8a7790.dtsi
>> >> +++ b/arch/arm/boot/dts/r8a7790.dtsi
>> >> @@ -1115,6 +1115,7 @@
>> >>                                            "lb", "qspi", "sdh", "sd0",
>> >>                                            "sd1",
>> >>                                            "z", "rcan", "adsp";
>> >>                       #power-domain-cells = <0>;
>> >> +                     renesas,modemr = <&rst 0x60>;
>> >
>> > I have mixed feelings about this as I don't think it really describes the
>> > hardware.
>>
>> From the R-Car Gen2 manual:
>>
>> 8. Reset (RST)
>> 8.1 Features
>> The following functions are implemented by RST.
>> [...]
>> Latching of the levels on mode pins when PRESET# is negated
>> Mode monitoring register
>>
>> 7. Clock Pulse Generator (CPG)
>> 7.2 Input/Output Pins
>> Table 7.1 lists the CPG pin configuration.
>> Table 7.1 Pin Configuration and Functions of CPG
>> Pin Name Function I/O Description
>> [...]
>> MD0 Mode 0 ...
>>
>> Hence there definitely is a link between the (latched) values in the RST
>> module and CPG configuration. This link is expressed using the
>> "renesas,modemr" property, where the phandle provides the link to the RST
>> block, and the register offset provides a way for software to read the
>> configuration.
>
> The mode bits of course influence the CPG (otherwise we wouldn't be having
> this discussion :-)), but to me it looks more like a configuration broadcast
> through the whole SoC than a real link between two IP cores. It's obviously
> subject to interpretation.

Yep.

Broadcast? Like a bus clock? For that we also have properties with phandles...

Syscon is the Hot New Abstraction for a module that provides a bunch of
registers needed by drivers for other modules. Those other modules need
some way to refer to the appropriate syscon register.

The RST module definitely falls in that category: CPG needs the MODEMR
register, watchdog (RWDT/SWDT) needs the Watchdog Timer Reset Control Register,
and APMU probably needs the CA15/7 reset control registers.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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/RFC 06/11] ARM: shmobile: r8a7790: Link CPG to RST using "renesas, modemr"

[Laurent Pinchart]

Hi Geert,

On Wednesday 08 July 2015 10:29:56 Geert Uytterhoeven wrote:
> On Tue, Jul 7, 2015 at 6:20 PM, Laurent Pinchart wrote:
> >>>> --- a/arch/arm/boot/dts/r8a7790.dtsi
> >>>> +++ b/arch/arm/boot/dts/r8a7790.dtsi
> >>>> @@ -1115,6 +1115,7 @@
> >>>>                                            "lb", "qspi", "sdh", "sd0",
> >>>>                                            "sd1",
> >>>>                                            "z", "rcan", "adsp";
> >>>>                       #power-domain-cells = <0>;
> >>>> +                     renesas,modemr = <&rst 0x60>;
> >>> 
> >>> I have mixed feelings about this as I don't think it really describes
> >>> the hardware.
> >> 
> >> From the R-Car Gen2 manual:
> >> 
> >> 8. Reset (RST)
> >> 8.1 Features
> >> The following functions are implemented by RST.
> >> [...]
> >> Latching of the levels on mode pins when PRESET# is negated
> >> Mode monitoring register
> >> 
> >> 7. Clock Pulse Generator (CPG)
> >> 7.2 Input/Output Pins
> >> Table 7.1 lists the CPG pin configuration.
> >> Table 7.1 Pin Configuration and Functions of CPG
> >> Pin Name Function I/O Description
> >> [...]
> >> MD0 Mode 0 ...
> >> 
> >> Hence there definitely is a link between the (latched) values in the RST
> >> module and CPG configuration. This link is expressed using the
> >> "renesas,modemr" property, where the phandle provides the link to the RST
> >> block, and the register offset provides a way for software to read the
> >> configuration.
> > 
> > The mode bits of course influence the CPG (otherwise we wouldn't be having
> > this discussion :-)), but to me it looks more like a configuration
> > broadcast through the whole SoC than a real link between two IP cores.
> > It's obviously subject to interpretation.
> 
> Yep.
> 
> Broadcast? Like a bus clock? For that we also have properties with
> phandles...

I knew someone would raise that topic ;-)

> Syscon is the Hot New Abstraction for a module that provides a bunch of
> registers needed by drivers for other modules. Those other modules need
> some way to refer to the appropriate syscon register.
> 
> The RST module definitely falls in that category: CPG needs the MODEMR
> register, watchdog (RWDT/SWDT) needs the Watchdog Timer Reset Control
> Register, and APMU probably needs the CA15/7 reset control registers.

syscon is a quick hack to be used when no clean solution exists. Sometimes 
implementing a proper API is just overkill, especially when the "system 
controller" aggregates registers that really belong to individual IP cores.

Proper abstract kernel APIs should of course be used wherever possible. Some 
system controllers are already supported in such a way, using the MFD 
subsystem and exposing proper APIs for part of their features, and letting 
drivers access registers directly for other features.

In the boot mode case I believe adding a new API would be both useful and 
quite simple, so I'd like to explore that option.

By the way, regarding the CA15/7 reset control registers, shouldn't they be 
exposed through the reset controller API ?

-- 
Regards,

Laurent Pinchart

[PATCH/RFC 07/11] ARM: shmobile: r8a7791: Link CPG to RST using "renesas, modemr"

[Geert Uytterhoeven]

Add a link from the Clock Pulse Generator node to the Reset Controller
node, so the CPG can read the Mode Monitoring Register (MODEMR) to
obtain the MD pin values.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7791.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 17a6e0fd779fb7a4..fe29b67be73be5ae 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -1033,6 +1033,7 @@
 					     "lb", "qspi", "sdh", "sd0", "z",
 					     "rcan", "adsp";
 			#power-domain-cells = <0>;
+			renesas,modemr = <&rst 0x60>;
 		};
 
 		/* Variable factor clocks */
-- 
1.9.1

[PATCH/RFC 08/11] ARM: shmobile: r8a7793: Link CPG to RST using "renesas, modemr"

[Geert Uytterhoeven]

Add a link from the Clock Pulse Generator node to the Reset Controller
node, so the CPG can read the Mode Monitoring Register (MODEMR) to
obtain the MD pin values.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7793.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7793.dtsi b/arch/arm/boot/dts/r8a7793.dtsi
index fbb1aa3740e2b55e..4bddaf2fbd0b2e1c 100644
--- a/arch/arm/boot/dts/r8a7793.dtsi
+++ b/arch/arm/boot/dts/r8a7793.dtsi
@@ -192,6 +192,7 @@
 					     "lb", "qspi", "sdh", "sd0", "z",
 					     "rcan", "adsp";
 			#power-domain-cells = <0>;
+			renesas,modemr = <&rst 0x60>;
 		};
 
 		/* Variable factor clocks */
-- 
1.9.1

[PATCH/RFC 09/11] ARM: shmobile: r8a7794: Link CPG to RST using "renesas, modemr"

[Geert Uytterhoeven]

Add a link from the Clock Pulse Generator node to the Reset Controller
node, so the CPG can read the Mode Monitoring Register (MODEMR) to
obtain MD pin values.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/boot/dts/r8a7794.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/r8a7794.dtsi b/arch/arm/boot/dts/r8a7794.dtsi
index bd37ec229329ed9c..e4cddf164330e77b 100644
--- a/arch/arm/boot/dts/r8a7794.dtsi
+++ b/arch/arm/boot/dts/r8a7794.dtsi
@@ -484,6 +484,7 @@
 			clock-output-names = "main", "pll0", "pll1", "pll3",
 					     "lb", "qspi", "sdh", "sd0", "z";
 			#power-domain-cells = <0>;
+			renesas,modemr = <&rst 0x60>;
 		};
 		/* Variable factor clocks */
 		sd2_clk: sd2_clk at e6150078 {
-- 
1.9.1

[PATCH/RFC 10/11] ARM: shmobile: rcar-gen2: Use syscon to configure the CPG

[Geert Uytterhoeven]

Let the R-Car Gen2 CPG clock driver rely on syscon to read the Mode
Monitoring Register (MODEMR), instead of passing the register value
directly.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/arm/mach-shmobile/Kconfig           | 1 +
 arch/arm/mach-shmobile/setup-rcar-gen2.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/Kconfig b/arch/arm/mach-shmobile/Kconfig
index 26a036c3ab522128..37faf6e18d26344f 100644
--- a/arch/arm/mach-shmobile/Kconfig
+++ b/arch/arm/mach-shmobile/Kconfig
@@ -23,6 +23,7 @@ config ARCH_RCAR_GEN2
 	select SYS_SUPPORTS_SH_CMT
 	select PCI_DOMAINS if PCI
 	select HAVE_ARM_ARCH_TIMER
+	select MFD_SYSCON
 
 config ARCH_RMOBILE
 	bool
diff --git a/arch/arm/mach-shmobile/setup-rcar-gen2.c b/arch/arm/mach-shmobile/setup-rcar-gen2.c
index 570d6bd3784f7464..7a845f4829081cd9 100644
--- a/arch/arm/mach-shmobile/setup-rcar-gen2.c
+++ b/arch/arm/mach-shmobile/setup-rcar-gen2.c
@@ -15,6 +15,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/clk-provider.h>
 #include <linux/clk/shmobile.h>
 #include <linux/clocksource.h>
 #include <linux/device.h>
@@ -125,7 +126,7 @@ void __init rcar_gen2_timer_init(void)
 
 	iounmap(base);
 
-	rcar_gen2_clocks_init(mode);
+	of_clk_init(NULL);
 	clocksource_of_init();
 }
 
-- 
1.9.1

[PATCH/RFC 11/11] clk: shmobile: rcar-gen2: Remove obsolete rcar_gen2_clocks_init()

[Geert Uytterhoeven]

Since "ARM: shmobile: rcar-gen2: Use syscon to configure the CPG",
rcar_gen2_clocks_init() is no longer called by the R-Car Gen2 platform
code. Remove it.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/clk/shmobile/clk-rcar-gen2.c | 7 -------
 include/linux/clk/shmobile.h         | 1 -
 2 files changed, 8 deletions(-)

diff --git a/drivers/clk/shmobile/clk-rcar-gen2.c b/drivers/clk/shmobile/clk-rcar-gen2.c
index b121a49f7575a97b..3f02b40b4911831c 100644
--- a/drivers/clk/shmobile/clk-rcar-gen2.c
+++ b/drivers/clk/shmobile/clk-rcar-gen2.c
@@ -435,10 +435,3 @@ static void __init rcar_gen2_cpg_clocks_init(struct device_node *np)
 }
 CLK_OF_DECLARE(rcar_gen2_cpg_clks, "renesas,rcar-gen2-cpg-clocks",
 	       rcar_gen2_cpg_clocks_init);
-
-void __init rcar_gen2_clocks_init(u32 mode)
-{
-	cpg_mode = mode;
-
-	of_clk_init(NULL);
-}
diff --git a/include/linux/clk/shmobile.h b/include/linux/clk/shmobile.h
index cb19cc1865ca5cfb..95357cdde185d518 100644
--- a/include/linux/clk/shmobile.h
+++ b/include/linux/clk/shmobile.h
@@ -22,7 +22,6 @@ struct generic_pm_domain;
 
 void r8a7778_clocks_init(u32 mode);
 void r8a7779_clocks_init(u32 mode);
-void rcar_gen2_clocks_init(u32 mode);
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 void cpg_mstp_add_clk_domain(struct device_node *np);
-- 
1.9.1