All of lore.kernel.org
 help / color / mirror / Atom feed
* Renesas CPG/MSTP DT Binding Proposal
@ 2015-06-22 19:49 ` Geert Uytterhoeven
  0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-06-22 19:49 UTC (permalink / raw)
  To: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd
  Cc: linux-clk, Linux PM list, devicetree, Linux-sh list

Introduction
------------

On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
MSTP (Module Standby and Software Reset), and APMU (Advanced Power Management
Unit for AP-System Core) blocks are very intimately tied.

  - The CPG clock generates various clocks for LSI internal generation,
  - The MSTP block provides two functions:
      1. Module Standby: "Clock supply to specified modules is stopped by
         setting the module stop control register bits."
         However, the clock supply to a module is not stopped until all CPUs
         in the SoC agree.  Indeed, there are separate MSTP registers for
         application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
      2. Reset Control. to perform a software reset of a specific module.
  - The APMU controls power and clock supply to the AP-system core (e.g.
    CA7, CA15, SCU).

MSTP and CPG registers are mixed in one register block.
The APMU has one or two separate register blocks.

The current DT bindings are split across 4 different binding documents:
  1. Renesas SoC-specific "core" CPG clocks that cannot be represented easily
     in DT in another way (cfr. [1]),
  2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
  3. Renesas-specific "div6" variable factor clocks, also generated by the CPG
     (cfr. [3]).
  3. Generic "fixed-factor-clock" clocks, generated by the CPG, but represented
     in DT using the generic ""fixed-factor-clock" bindings (cfr. [4]),

Note that currently the Reset Control function of the MSTP block is not
supported by DT nor Linux, and that there are no DT bindings for the APMU block
yet.


Issues with current bindings
----------------------------

General:
  - Tight coupling of CPG and MSTP is not represented in DT:
      - There's one CPG node, and separate MSTP nodes (one for each block of
        up to 32 possible MSTP clocks) next to it,
      - CPG Clock Domain requires "power-domain = <...>" properties linking
        individual device nodes to the "CPG/MSTP" clock domain provider.
        For now I use a link to the (single) CPG device node, while the clock
        provider (mostly) controls the (multiple) MSTP clocks (cfr. [6]).
      - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
        requires writing a special value to a specific "Write Protect
        Register" inside the CPG first. Exact details and impact are still
        under investigation (cfr. [7]).

MSTP-specific:
  - The clock hierarchy (parent-child relationship) is represented in a flat
    structure,
  - Multiple arrays have to be kept in sync:
      - "clocks" (parents),
      - "clock-indices" (sparse bit index inside register),
      - "clock-output-names",
  - MSTP clocks are referred to using a reference to a device node and a bit
    index:
       - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h are
         separated from the .dtsi file,
       - There's no protection against using a bit index definition for a
         different MSTP register, e.g.
           - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
           - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),
   - The "reg" properties contain only the SMSTPCRx (control register for
     System CPU) and MSTPSRx (optional status register) registers only.
     However, in reality there are many more registers:
       - Separate control registers for application (Cortex-A, supported) and
         real-time (SH and/or Cortex-R, not supported) cores,
       - Software reset registers (not supported).
     To make matters more complicated:
       - Many registers are optional.
         The current bindings just support one control register and one
         optional status register, but this doesn't scale to more register
         sets,
       - Register sets are not linear and not contiguous in the register space.
         I.e. it's not possible to derive the address of SMSTPCRx from the
         address of SMSTPCRy, or the address of MSTPSRx from the address of
         SMSTPCRx.


Proposal
--------

I'd like to propose the following, to resolve (most of) the issues with the
current bindings:

  - Get rid of the "clocks" node.
    IIRC grouping all clocks under a "clock" node was deprecated, but I can't
    find the email anymore?
      - "fixed-clock" clocks are now at the root node level,
      - The CPG node is now at the root node level,
  - CPG node:
      - Use e.g. "renesas,r8a7791-cpg" as compatible value,
  - MSTP clocks:
      - All MSTP clocks are now grouped inside the CPG node, under a new "mstp"
        subnode, without compatible values,
      - Each individual MSTP clock has its own subnode, referring to the MSTP
        clock number, instead of using one node per register, and arrays of
        "clocks", "clock-output-names", and "clock-indices" properties.
      - The list of available registers is maintained as arrays inside the
        unified CPG/MSTP driver, e.g.

        struct cpg {
                void __iomem *base;
                unsigned int n_mstp;    /* Size of a register set */
                /*
                 * Optional arrays of register offsets for the various
                 * register sets:
                 *   - Array pointer may be NULL, if not supported */
                 *   - Array element may be ~0, if not supported */
                 */
                u16 *smstp;             /* System CPU control registers */
                u16 *rmstp;             /* Realtime CPU control registers */
                u16 *mstpsr;            /* Status registers */
                u16 *srcr;              /* Reset registers */
                ...
        };


Short example
-------------

(from Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt)

For the MSTP part:

        #include <dt-bindings/clock/r8a7790-clock.h>

        mstp3_clks: mstp3_clks@e615013c {
                compatible = "renesas,r8a7790-mstp-clocks",
                             "renesas,cpg-mstp-clocks";
                reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
                clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
                         <&cpg_clocks R8A7790_CLK_SD1>,
                         <&cpg_clocks R8A7790_CLK_SD0>, <&mmc0_clk>;
                #clock-cells = <1>;
                clock-output-names                         "tpu0", "mmcif1", "sdhi3", "sdhi2",
                         "sdhi1", "sdhi0", "mmcif0";
                clock-indices = <
                        R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1 R8A7790_CLK_SDHI3
                        R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
                        R8A7790_CLK_MMCIF0
                >;
        };

would become:

        mstp {
                #address-cells = <1>;
                #size-cells = <0>;
                ...

                /* MSTP3 */
                tpu0_clk: clock@304 {
                        reg = <304>;
                        clocks = <&cp_clk>;
                        clock-output-names = "tpu0";
                };
                mmcif1_clk: clock@305 {
                        reg = <305>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };
                sdhi3_clk: clock@311 {
                        reg = <311>;
                        clocks = <&sd3_clk>;
                        clock-output-names = "sdhi3";
                };
                sdhi2_clk: clock@312 {
                        reg = <312>;
                        clocks = <&sd2_clk>;
                        clock-output-names = "sdhi2";
                };
                sdhi1_clk: clock@313 {
                        reg = <313>;
                        clocks = <&cpg_clocks R8A7790_CLK_SD1>;
                        clock-output-names = "sdhi1";
                };
                sdhi0_clk: clock@314 {
                        reg = <314>;
                        clocks = <&cpg_clocks R8A7790_CLK_SD0>;
                        clock-output-names = "sdhi0";
                };
                mmcif0_clk: clock@315 {
                        reg = <315>;
                        clocks = <&mmc0_clk>;
                        clock-output-names = "mmcif0";
                };

                ...
        }

Devices would no longer use e.g. "clocks = <&mstp3_clks R8A7790_CLK_MMCIF1>",
but "clocks = <&mmcif1_clk>".


Questions/alternatives
----------------------

  - Should the subnode be called "mssr" (Module Standby and Software Reset)
    instead of "mstp"?
  - What with "renesas,*-cpg-clocks" clocks?
      - Should we keep on using "#clock-cells = <1>" and <SOC>_CLK_<NAME>
        indices, or switch to a less error-prone "#clock-cells = <0>"?
  - What with "renesas,cpg-div6-clock" clocks?
      - For now I kept them as a subnode of cpg_clocks, as their registers are
        a subset of the CPG registers.
      - This also means I had to keep the "#address-cells = <2>",
        "#size-cells = <2>", and "ranges" properties under the CPG node.
  - The MSTP number could be split in register and bit offset (and thus use
    "#address-cells = <2>"), e.g.:

                mmcif1_clk: clock@3,5 {
                        reg = <3 5>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };

        vs.

                mmcif1_clk: clock@305 {
                        reg = <305>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };

    I don't think it's clearer, as the datasheet refers to e.g. "MSTP305".


Thanks for your comments!


Long example
------------

All r8a7791 clocks:

        /* External root clock */
        extal_clk: extal_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                /* This value must be overriden by the board. */
                clock-frequency = <0>;
                clock-output-names = "extal";
        };

        /*
         * The external audio clocks are configured as 0 Hz fixed
frequency clocks by
         * default. Boards that provide audio clocks should override them.
         */
        audio_clk_a: audio_clk_a {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_a";
        };
        audio_clk_b: audio_clk_b {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_b";
        };
        audio_clk_c: audio_clk_c {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_c";
        };

        /* External PCIe clock - can be overridden by the board */
        pcie_bus_clk: pcie_bus_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <100000000>;
                clock-output-names = "pcie_bus";
                status = "disabled";
        };

        /* External USB clock - can be overridden by the board */
        usb_extal_clk: usb_extal_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <48000000>;
                clock-output-names = "usb_extal";
        };

        /* External CAN clock */
        can_clk: can_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                /* This value must be overridden by the board. */
                clock-frequency = <0>;
                clock-output-names = "can_clk";
                status = "disabled";
        };

        /* Fixed factor clocks */
        pll1_div2_clk: pll1_div2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "pll1_div2";
        };
        zg_clk: zg_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <3>;
                clock-mult = <1>;
                clock-output-names = "zg";
        };
        zx_clk: zx_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <3>;
                clock-mult = <1>;
                clock-output-names = "zx";
        };
        zs_clk: zs_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <6>;
                clock-mult = <1>;
                clock-output-names = "zs";
        };
        hp_clk: hp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <12>;
                clock-mult = <1>;
                clock-output-names = "hp";
        };
        i_clk: i_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "i";
        };
        b_clk: b_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <12>;
                clock-mult = <1>;
                clock-output-names = "b";
        };
        p_clk: p_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <24>;
                clock-mult = <1>;
                clock-output-names = "p";
        };
        cl_clk: cl_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <48>;
                clock-mult = <1>;
                clock-output-names = "cl";
        };
        m2_clk: m2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "m2";
        };
        imp_clk: imp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <4>;
                clock-mult = <1>;
                clock-output-names = "imp";
        };
        rclk_clk: rclk_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <(48 * 1024)>;
                clock-mult = <1>;
                clock-output-names = "rclk";
        };
        oscclk_clk: oscclk_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <(12 * 1024)>;
                clock-mult = <1>;
                clock-output-names = "oscclk";
        };
        zb3_clk: zb3_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <4>;
                clock-mult = <1>;
                clock-output-names = "zb3";
        };
        zb3d2_clk: zb3d2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "zb3d2";
        };
        ddr_clk: ddr_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "ddr";
        };
        mp_clk: mp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&pll1_div2_clk>;
                #clock-cells = <0>;
                clock-div = <15>;
                clock-mult = <1>;
                clock-output-names = "mp";
        };
        cp_clk: cp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&extal_clk>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "cp";
        };

        /* Special CPG clocks */
        cpg_clocks: cpg_clocks@e6150000 {
                #address-cells = <2>;
                #size-cells = <2>;
                ranges;

                compatible = "renesas,r8a7791-cpg",
                reg = <0 0xe6150000 0 0x1000>;
                clocks = <&extal_clk &usb_extal_clk>;
                #clock-cells = <1>;
                clock-output-names = "main", "pll0", "pll1", "pll3",
                                     "lb", "qspi", "sdh", "sd0", "z",
                                     "rcan", "adsp";
                #power-domain-cells = <0>;

                /* Variable factor clocks */
                sd2_clk: sd2_clk@e6150078 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150078 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "sd2";
                };
                sd3_clk: sd3_clk@e615026c {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe615026c 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "sd3";
                };
                mmc0_clk: mmc0_clk@e6150240 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150240 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "mmc0";
                };
                ssp_clk: ssp_clk@e6150248 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150248 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "ssp";
                };
                ssprs_clk: ssprs_clk@e615024c {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe615024c 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "ssprs";
                };

                /* Module Standby and Software Reset */
                mstp {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        /* MSTP0 */
                        msiof0_clk: clock@000 {
                                reg = <000>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof0";
                        };

                        /* MSTP1 */
                        vcp0_clk: clock@101 {
                                reg = <101>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vcp0";
                        };
                        vpc0_clk: clock@103 {
                                reg = <103>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vpc0";
                        };
                        jpu_clk: clock@106 {
                                reg = <106>;
                                clocks = <&m2_clk>;
                                clock-output-names = "jpu";
                        };
                        ssp1_clk: clock@109 {
                                reg = <109>;
                                clocks = <&zs_clk>;
                                clock-output-names = "ssp1";
                        };
                        tmu1_clk: clock@111 {
                                reg = <111>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu1";
                        };
                        threedg_clk: clock@112 {
                                reg = <112>;
                                clocks = <&zg_clk>;
                                clock-output-names = "3dg";
                        };
                        twoddmac_clk: clock@115 {
                                reg = <115>;
                                clocks = <&zs_clk>;
                                clock-output-names = "2ddmac";
                        };
                        fdp1_1_clk: clock@118 {
                                reg = <118>;
                                clocks = <&zs_clk>;
                                clock-output-names = "fdp1-1";
                        };
                        fdp1_0_clk: clock@119 {
                                reg = <119>;
                                clocks = <&zs_clk>;
                                clock-output-names = "fdp1-0";
                        };
                        tmu3_clk: clock@121 {
                                reg = <121>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu3";
                        };
                        tmu2_clk: clock@122 {
                                reg = <122>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu2";
                        };
                        cmt0_clk: clock@124 {
                                reg = <124>;
                                clocks = <&rclk_clk>;
                                clock-output-names = "cmt0";
                        };
                        tmu0_clk: clock@125 {
                                reg = <125>;
                                clocks = <&cp_clk>;
                                clock-output-names = "tmu0";
                        };
                        vsp1_du1_clk: clock@127 {
                                reg = <127>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-du1";
                        };
                        vsp1_du0_clk: clock@128 {
                                reg = <128>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-du0";
                        };
                        vsp1_sy_clk: clock@131 {
                                reg = <131>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-sy";
                        };

                        /* MSTP2 */
                        scifa2_clk: clock@202 {
                                reg = <202>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa2";
                        };
                        scifa1_clk: clock@203 {
                                reg = <203>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa1";
                        };
                        scifa0_clk: clock@204 {
                                reg = <204>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa0";
                        };
                        msiof2_clk: clock@205 {
                                reg = <205>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof2";
                        };
                        scifb0_clk: clock@206 {
                                reg = <206>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb0";
                        };
                        scifb1_clk: clock@207 {
                                reg = <207>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb1";
                        };
                        msiof1_clk: clock@208 {
                                reg = <208>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof1";
                        };
                        scifb2_clk: clock@216 {
                                reg = <216>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb2";
                        };
                        sys_dmac1_clk: clock@218 {
                                reg = <218>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sys-dmac1";
                        };
                        sys_dmac0_clk: clock@219 {
                                reg = <219>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sys-dmac0";
                        };

                        /* MSTP3 */
                        tpu0_clk: clock@304 {
                                reg = <304>;
                                clocks = <&cp_clk>;
                                clock-output-names = "tpu0";
                        };
                        sdhi2_clk: clock@311 {
                                reg = <311>;
                                clocks = <&sd3_clk>;
                                clock-output-names = "sdhi2";
                        };
                        sdhi1_clk: clock@312 {
                                reg = <312>;
                                clocks = <&sd2_clk>;
                                clock-output-names = "sdhi1";
                        };
                        sdhi0_clk: clock@314 {
                                reg = <314>;
                                clocks = <&cpg_clocks R8A7791_CLK_SD0>;
                                clock-output-names = "sdhi0";
                        };
                        mmcif0_clk: clock@315 {
                                reg = <315>;
                                clocks = <&mmc0_clk>;
                                clock-output-names = "mmcif0";
                        };
                        i2c7_clk: clock@318 {
                                reg = <318>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c7";
                        };
                        pciec_clk: clock@319 {
                                reg = <319>;
                                clocks = <&mp_clk>;
                                clock-output-names = "pciec";
                        };
                        i2c8_clk: clock@323 {
                                reg = <323>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c8";
                        };
                        ssusb_clk: clock@328 {
                                reg = <328>;
                                clocks = <&mp_clk>;
                                clock-output-names = "ssusb";
                        };
                        cmt1_clk: clock@329 {
                                reg = <329>;
                                clocks = <&rclk_clk>;
                                clock-output-names = "cmt1";
                        };
                        usbdmac0_clk: clock@330 {
                                reg = <330>;
                                clocks = <&hp_clk>;
                                clock-output-names = "usbdmac0";
                        };
                        usbdmac1_clk: clock@331 {
                                reg = <331>;
                                clocks = <&hp_clk>;
                                clock-output-names = "usbdmac1";
                        };

                        /* MSTP4 */
                        irqc_clk: clock@407 {
                                reg = <407>;
                                clocks = <&cp_clk>;
                                clock-output-names = "irqc";
                        };
                        intc_sys_clk: clock@408 {
                                reg = <408>;
                                clocks = <&zs_clk>;
                                clock-output-names = "intc-sys";
                        };

                        /* MSTP5 */
                        audmac1_clk: clock@501 {
                                reg = <501>;
                                clocks = <&hp_clk>;
                                clock-output-names = "audmac1";
                        };
                        audmac0_clk: clock@502 {
                                reg = <502>;
                                clocks = <&hp_clk>;
                                clock-output-names = "audmac0";
                        };
                        adsp_mod_clk: clock@506 {
                                reg = <506>;
                                clocks = <&cpg_clocks R8A7791_CLK_ADSP>;
                                clock-output-names = "adsp_mod";
                        };
                        thermal_clk: clock@522 {
                                reg = <522>;
                                clocks = <&extal_clk>;
                                clock-output-names = "thermal";
                        };
                        pwm_clk: clock@523 {
                                reg = <523>;
                                clocks = <&p_clk>;
                                clock-output-names = "pwm";
                        };

                        /* MSTP7 */
                        ehci_clk: clock@703 {
                                reg = <703>;
                                clocks = <&mp_clk>;
                                clock-output-names = "ehci";
                        };
                        hsusb_clk: clock@704 {
                                reg = <704>;
                                clocks = <&hp_clk>;
                                clock-output-names = "hsusb";
                        };
                        hscif2_clk: clock@713 {
                                reg = <713>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif2";
                        };
                        scif5_clk: clock@714 {
                                reg = <714>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif5";
                        };
                        scif4_clk: clock@715 {
                                reg = <715>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif4";
                        };
                        hscif1_clk: clock@716 {
                                reg = <716>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif1";
                        };
                        hscif0_clk: clock@717 {
                                reg = <717>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif0";
                        };
                        scif3_clk: clock@718 {
                                reg = <718>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif3";
                        };
                        scif2_clk: clock@719 {
                                reg = <719>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif2";
                        };
                        scif1_clk: clock@720 {
                                reg = <720>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif1";
                        };
                        scif0_clk: clock@721 {
                                reg = <721>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif0";
                        };
                        du1_clk: clock@723 {
                                reg = <723>;
                                clocks = <&zx_clk>;
                                clock-output-names = "du1";
                        };
                        du0_clk: clock@724 {
                                reg = <724>;
                                clocks = <&zx_clk>;
                                clock-output-names = "du0";
                        };
                        lvds0_clk: clock@726 {
                                reg = <726>;
                                clocks = <&zx_clk>;
                                clock-output-names = "lvds0";
                        };

                        /* MSTP8 */
                        ipmmu_sgx_clk: clock@800 {
                                reg = <800>;
                                clocks = <&zx_clk>;
                                clock-output-names = "ipmmu_sgx";
                        };
                        mlb_clk: clock@802 {
                                reg = <802>;
                                clocks = <&hp_clk>;
                                clock-output-names = "mlb";
                        };
                        vin2_clk: clock@809 {
                                reg = <809>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin2";
                        };
                        vin1_clk: clock@810 {
                                reg = <810>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin1";
                        };
                        vin0_clk: clock@811 {
                                reg = <811>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin0";
                        };
                        ether_clk: clock@813 {
                                reg = <813>;
                                clocks = <&p_clk>;
                                clock-output-names = "ether";
                        };
                        sata1_clk: clock@814 {
                                reg = <814>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sata1";
                        };
                        sata0_clk: clock@815 {
                                reg = <815>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sata0";
                        };

                        /* MSTP9 */
                        gpio7_clk: clock@904 {
                                reg = <904>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio7";
                        };
                        gpio6_clk: clock@905 {
                                reg = <905>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio6";
                        };
                        gpio5_clk: clock@907 {
                                reg = <907>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio5";
                        };
                        gpio4_clk: clock@908 {
                                reg = <908>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio4";
                        };
                        gpio3_clk: clock@909 {
                                reg = <909>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio3";
                        };
                        gpio2_clk: clock@910 {
                                reg = <910>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio2";
                        };
                        gpio1_clk: clock@911 {
                                reg = <911>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio1";
                        };
                        gpio0_clk: clock@912 {
                                reg = <912>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio0";
                        };
                        rcan1_clk: clock@915 {
                                reg = <915>;
                                clocks = <&p_clk>;
                                clock-output-names = "rcan1";
                        };
                        rcan0_clk: clock@916 {
                                reg = <916>;
                                clocks = <&p_clk>;
                                clock-output-names = "rcan0";
                        };
                        qspi_mod_clk: clock@917 {
                                reg = <917>;
                                clocks = <&cpg_clocks R8A7791_CLK_QSPI>;
                                clock-output-names = "qspi_mod";
                        };
                        i2c5_clk: clock@925 {
                                reg = <925>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c5";
                        };
                        i2c6_clk: clock@926 {
                                reg = <926>;
                                clocks = <&cp_clk>;
                                clock-output-names = "i2c6";
                        };
                        i2c4_clk: clock@927 {
                                reg = <927>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c4";
                        };
                        i2c3_clk: clock@928 {
                                reg = <928>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c3";
                        };
                        i2c2_clk: clock@929 {
                                reg = <929>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c2";
                        };
                        i2c1_clk: clock@930 {
                                reg = <930>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c1";
                        };
                        i2c0_clk: clock@931 {
                                reg = <931>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c0";
                        };

                        /* MSTP10 */
                        ssi_all_clk: clock@1005 {
                                reg = <1005>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi-all";
                        };
                        ssi9_clk: clock@1006 {
                                reg = <1006>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi9";
                        };
                        ssi8_clk: clock@1007 {
                                reg = <1007>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi8";
                        };
                        ssi7_clk: clock@1008 {
                                reg = <1008>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi7";
                        };
                        ssi6_clk: clock@1009 {
                                reg = <1009>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi6";
                        };
                        ssi5_clk: clock@1010 {
                                reg = <1010>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi5";
                        };
                        ssi4_clk: clock@1011 {
                                reg = <1011>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi4";
                        };
                        ssi3_clk: clock@1012 {
                                reg = <1012>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi3";
                        };
                        ssi2_clk: clock@1013 {
                                reg = <1013>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi2";
                        };
                        ssi1_clk: clock@1014 {
                                reg = <1014>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi1";
                        };
                        ssi0_clk: clock@1015 {
                                reg = <1015>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi0";
                        };
                        scu_all_clk: clock@1017 {
                                reg = <1017>;
                                clocks = <&p_clk>;
                                clock-output-names = "scu-all";
                        };
                        scu_dvc1_clk: clock@1018 {
                                reg = <1018>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-dvc1";
                        };
                        scu_dvc0_clk: clock@1019 {
                                reg = <1019>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-dvc0";
                        };
                        scu_src9_clk: clock@1022 {
                                reg = <1022>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src9";
                        };
                        scu_src8_clk: clock@1023 {
                                reg = <1023>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src8";
                        };
                        scu_src7_clk: clock@1024 {
                                reg = <1024>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src7";
                        };
                        scu_src6_clk: clock@1025 {
                                reg = <1025>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src6";
                        };
                        scu_src5_clk: clock@1026 {
                                reg = <1026>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src5";
                        };
                        scu_src4_clk: clock@1027 {
                                reg = <1027>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src4";
                        };
                        scu_src3_clk: clock@1028 {
                                reg = <1028>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src3";
                        };
                        scu_src2_clk: clock@1029 {
                                reg = <1029>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src2";
                        };
                        scu_src1_clk: clock@1030 {
                                reg = <1030>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src1";
                        };
                        scu_src0_clk: clock@1031 {
                                reg = <1031>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src0";
                        };

                        /* MSTP11 */
                        scifa3_clk: clock@1106 {
                                reg = <1106>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa3";
                        };
                        scifa4_clk: clock@1107 {
                                reg = <1107>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa4";
                        };
                        scifa5_clk: clock@1108 {
                                reg = <1108>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa5";
                        };
                };
        };


References
----------

  [1] Documentation/devicetree/bindings/clock/renesas,<SoC>-cpg-clocks.txt
      where <SoC> is a single SoC name or a family of SoCs
  [2] Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
  [3] Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
  [4] Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
  [5] arch/arm/boot/dts/{sh7,r7,r8}*.dtsi
  [6] Series "[PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains"
      (https://lkml.org/lkml/2015/5/28/590,
       http://lkml.iu.edu/hypermail/linux/kernel/1505.3/03336.html)
  [7] Series "[PATCH 0/6][RFC] arm64: Renesas Gen3 initial patch"
      (http://www.spinics.net/lists/linux-sh/msg42683.html)

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
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Renesas CPG/MSTP DT Binding Proposal
@ 2015-06-22 19:49 ` Geert Uytterhoeven
  0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-06-22 19:49 UTC (permalink / raw)
  To: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd
  Cc: linux-clk-u79uwXL29TY76Z2rM5mHXA, Linux PM list,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-sh list

Introduction
------------

On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
MSTP (Module Standby and Software Reset), and APMU (Advanced Power Management
Unit for AP-System Core) blocks are very intimately tied.

  - The CPG clock generates various clocks for LSI internal generation,
  - The MSTP block provides two functions:
      1. Module Standby: "Clock supply to specified modules is stopped by
         setting the module stop control register bits."
         However, the clock supply to a module is not stopped until all CPUs
         in the SoC agree.  Indeed, there are separate MSTP registers for
         application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
      2. Reset Control. to perform a software reset of a specific module.
  - The APMU controls power and clock supply to the AP-system core (e.g.
    CA7, CA15, SCU).

MSTP and CPG registers are mixed in one register block.
The APMU has one or two separate register blocks.

The current DT bindings are split across 4 different binding documents:
  1. Renesas SoC-specific "core" CPG clocks that cannot be represented easily
     in DT in another way (cfr. [1]),
  2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
  3. Renesas-specific "div6" variable factor clocks, also generated by the CPG
     (cfr. [3]).
  3. Generic "fixed-factor-clock" clocks, generated by the CPG, but represented
     in DT using the generic ""fixed-factor-clock" bindings (cfr. [4]),

Note that currently the Reset Control function of the MSTP block is not
supported by DT nor Linux, and that there are no DT bindings for the APMU block
yet.


Issues with current bindings
----------------------------

General:
  - Tight coupling of CPG and MSTP is not represented in DT:
      - There's one CPG node, and separate MSTP nodes (one for each block of
        up to 32 possible MSTP clocks) next to it,
      - CPG Clock Domain requires "power-domain = <...>" properties linking
        individual device nodes to the "CPG/MSTP" clock domain provider.
        For now I use a link to the (single) CPG device node, while the clock
        provider (mostly) controls the (multiple) MSTP clocks (cfr. [6]).
      - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
        requires writing a special value to a specific "Write Protect
        Register" inside the CPG first. Exact details and impact are still
        under investigation (cfr. [7]).

MSTP-specific:
  - The clock hierarchy (parent-child relationship) is represented in a flat
    structure,
  - Multiple arrays have to be kept in sync:
      - "clocks" (parents),
      - "clock-indices" (sparse bit index inside register),
      - "clock-output-names",
  - MSTP clocks are referred to using a reference to a device node and a bit
    index:
       - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h are
         separated from the .dtsi file,
       - There's no protection against using a bit index definition for a
         different MSTP register, e.g.
           - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
           - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),
   - The "reg" properties contain only the SMSTPCRx (control register for
     System CPU) and MSTPSRx (optional status register) registers only.
     However, in reality there are many more registers:
       - Separate control registers for application (Cortex-A, supported) and
         real-time (SH and/or Cortex-R, not supported) cores,
       - Software reset registers (not supported).
     To make matters more complicated:
       - Many registers are optional.
         The current bindings just support one control register and one
         optional status register, but this doesn't scale to more register
         sets,
       - Register sets are not linear and not contiguous in the register space.
         I.e. it's not possible to derive the address of SMSTPCRx from the
         address of SMSTPCRy, or the address of MSTPSRx from the address of
         SMSTPCRx.


Proposal
--------

I'd like to propose the following, to resolve (most of) the issues with the
current bindings:

  - Get rid of the "clocks" node.
    IIRC grouping all clocks under a "clock" node was deprecated, but I can't
    find the email anymore?
      - "fixed-clock" clocks are now at the root node level,
      - The CPG node is now at the root node level,
  - CPG node:
      - Use e.g. "renesas,r8a7791-cpg" as compatible value,
  - MSTP clocks:
      - All MSTP clocks are now grouped inside the CPG node, under a new "mstp"
        subnode, without compatible values,
      - Each individual MSTP clock has its own subnode, referring to the MSTP
        clock number, instead of using one node per register, and arrays of
        "clocks", "clock-output-names", and "clock-indices" properties.
      - The list of available registers is maintained as arrays inside the
        unified CPG/MSTP driver, e.g.

        struct cpg {
                void __iomem *base;
                unsigned int n_mstp;    /* Size of a register set */
                /*
                 * Optional arrays of register offsets for the various
                 * register sets:
                 *   - Array pointer may be NULL, if not supported */
                 *   - Array element may be ~0, if not supported */
                 */
                u16 *smstp;             /* System CPU control registers */
                u16 *rmstp;             /* Realtime CPU control registers */
                u16 *mstpsr;            /* Status registers */
                u16 *srcr;              /* Reset registers */
                ...
        };


Short example
-------------

(from Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt)

For the MSTP part:

        #include <dt-bindings/clock/r8a7790-clock.h>

        mstp3_clks: mstp3_clks@e615013c {
                compatible = "renesas,r8a7790-mstp-clocks",
                             "renesas,cpg-mstp-clocks";
                reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
                clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
                         <&cpg_clocks R8A7790_CLK_SD1>,
                         <&cpg_clocks R8A7790_CLK_SD0>, <&mmc0_clk>;
                #clock-cells = <1>;
                clock-output-names =
                        "tpu0", "mmcif1", "sdhi3", "sdhi2",
                         "sdhi1", "sdhi0", "mmcif0";
                clock-indices = <
                        R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1 R8A7790_CLK_SDHI3
                        R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
                        R8A7790_CLK_MMCIF0
                >;
        };

would become:

        mstp {
                #address-cells = <1>;
                #size-cells = <0>;
                ...

                /* MSTP3 */
                tpu0_clk: clock@304 {
                        reg = <304>;
                        clocks = <&cp_clk>;
                        clock-output-names = "tpu0";
                };
                mmcif1_clk: clock@305 {
                        reg = <305>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };
                sdhi3_clk: clock@311 {
                        reg = <311>;
                        clocks = <&sd3_clk>;
                        clock-output-names = "sdhi3";
                };
                sdhi2_clk: clock@312 {
                        reg = <312>;
                        clocks = <&sd2_clk>;
                        clock-output-names = "sdhi2";
                };
                sdhi1_clk: clock@313 {
                        reg = <313>;
                        clocks = <&cpg_clocks R8A7790_CLK_SD1>;
                        clock-output-names = "sdhi1";
                };
                sdhi0_clk: clock@314 {
                        reg = <314>;
                        clocks = <&cpg_clocks R8A7790_CLK_SD0>;
                        clock-output-names = "sdhi0";
                };
                mmcif0_clk: clock@315 {
                        reg = <315>;
                        clocks = <&mmc0_clk>;
                        clock-output-names = "mmcif0";
                };

                ...
        }

Devices would no longer use e.g. "clocks = <&mstp3_clks R8A7790_CLK_MMCIF1>",
but "clocks = <&mmcif1_clk>".


Questions/alternatives
----------------------

  - Should the subnode be called "mssr" (Module Standby and Software Reset)
    instead of "mstp"?
  - What with "renesas,*-cpg-clocks" clocks?
      - Should we keep on using "#clock-cells = <1>" and <SOC>_CLK_<NAME>
        indices, or switch to a less error-prone "#clock-cells = <0>"?
  - What with "renesas,cpg-div6-clock" clocks?
      - For now I kept them as a subnode of cpg_clocks, as their registers are
        a subset of the CPG registers.
      - This also means I had to keep the "#address-cells = <2>",
        "#size-cells = <2>", and "ranges" properties under the CPG node.
  - The MSTP number could be split in register and bit offset (and thus use
    "#address-cells = <2>"), e.g.:

                mmcif1_clk: clock@3,5 {
                        reg = <3 5>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };

        vs.

                mmcif1_clk: clock@305 {
                        reg = <305>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };

    I don't think it's clearer, as the datasheet refers to e.g. "MSTP305".


Thanks for your comments!


Long example
------------

All r8a7791 clocks:

        /* External root clock */
        extal_clk: extal_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                /* This value must be overriden by the board. */
                clock-frequency = <0>;
                clock-output-names = "extal";
        };

        /*
         * The external audio clocks are configured as 0 Hz fixed
frequency clocks by
         * default. Boards that provide audio clocks should override them.
         */
        audio_clk_a: audio_clk_a {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_a";
        };
        audio_clk_b: audio_clk_b {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_b";
        };
        audio_clk_c: audio_clk_c {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_c";
        };

        /* External PCIe clock - can be overridden by the board */
        pcie_bus_clk: pcie_bus_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <100000000>;
                clock-output-names = "pcie_bus";
                status = "disabled";
        };

        /* External USB clock - can be overridden by the board */
        usb_extal_clk: usb_extal_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <48000000>;
                clock-output-names = "usb_extal";
        };

        /* External CAN clock */
        can_clk: can_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                /* This value must be overridden by the board. */
                clock-frequency = <0>;
                clock-output-names = "can_clk";
                status = "disabled";
        };

        /* Fixed factor clocks */
        pll1_div2_clk: pll1_div2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "pll1_div2";
        };
        zg_clk: zg_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <3>;
                clock-mult = <1>;
                clock-output-names = "zg";
        };
        zx_clk: zx_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <3>;
                clock-mult = <1>;
                clock-output-names = "zx";
        };
        zs_clk: zs_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <6>;
                clock-mult = <1>;
                clock-output-names = "zs";
        };
        hp_clk: hp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <12>;
                clock-mult = <1>;
                clock-output-names = "hp";
        };
        i_clk: i_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "i";
        };
        b_clk: b_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <12>;
                clock-mult = <1>;
                clock-output-names = "b";
        };
        p_clk: p_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <24>;
                clock-mult = <1>;
                clock-output-names = "p";
        };
        cl_clk: cl_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <48>;
                clock-mult = <1>;
                clock-output-names = "cl";
        };
        m2_clk: m2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "m2";
        };
        imp_clk: imp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <4>;
                clock-mult = <1>;
                clock-output-names = "imp";
        };
        rclk_clk: rclk_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <(48 * 1024)>;
                clock-mult = <1>;
                clock-output-names = "rclk";
        };
        oscclk_clk: oscclk_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <(12 * 1024)>;
                clock-mult = <1>;
                clock-output-names = "oscclk";
        };
        zb3_clk: zb3_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <4>;
                clock-mult = <1>;
                clock-output-names = "zb3";
        };
        zb3d2_clk: zb3d2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "zb3d2";
        };
        ddr_clk: ddr_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "ddr";
        };
        mp_clk: mp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&pll1_div2_clk>;
                #clock-cells = <0>;
                clock-div = <15>;
                clock-mult = <1>;
                clock-output-names = "mp";
        };
        cp_clk: cp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&extal_clk>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "cp";
        };

        /* Special CPG clocks */
        cpg_clocks: cpg_clocks@e6150000 {
                #address-cells = <2>;
                #size-cells = <2>;
                ranges;

                compatible = "renesas,r8a7791-cpg",
                reg = <0 0xe6150000 0 0x1000>;
                clocks = <&extal_clk &usb_extal_clk>;
                #clock-cells = <1>;
                clock-output-names = "main", "pll0", "pll1", "pll3",
                                     "lb", "qspi", "sdh", "sd0", "z",
                                     "rcan", "adsp";
                #power-domain-cells = <0>;

                /* Variable factor clocks */
                sd2_clk: sd2_clk@e6150078 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150078 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "sd2";
                };
                sd3_clk: sd3_clk@e615026c {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe615026c 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "sd3";
                };
                mmc0_clk: mmc0_clk@e6150240 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150240 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "mmc0";
                };
                ssp_clk: ssp_clk@e6150248 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150248 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "ssp";
                };
                ssprs_clk: ssprs_clk@e615024c {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe615024c 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "ssprs";
                };

                /* Module Standby and Software Reset */
                mstp {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        /* MSTP0 */
                        msiof0_clk: clock@000 {
                                reg = <000>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof0";
                        };

                        /* MSTP1 */
                        vcp0_clk: clock@101 {
                                reg = <101>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vcp0";
                        };
                        vpc0_clk: clock@103 {
                                reg = <103>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vpc0";
                        };
                        jpu_clk: clock@106 {
                                reg = <106>;
                                clocks = <&m2_clk>;
                                clock-output-names = "jpu";
                        };
                        ssp1_clk: clock@109 {
                                reg = <109>;
                                clocks = <&zs_clk>;
                                clock-output-names = "ssp1";
                        };
                        tmu1_clk: clock@111 {
                                reg = <111>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu1";
                        };
                        threedg_clk: clock@112 {
                                reg = <112>;
                                clocks = <&zg_clk>;
                                clock-output-names = "3dg";
                        };
                        twoddmac_clk: clock@115 {
                                reg = <115>;
                                clocks = <&zs_clk>;
                                clock-output-names = "2ddmac";
                        };
                        fdp1_1_clk: clock@118 {
                                reg = <118>;
                                clocks = <&zs_clk>;
                                clock-output-names = "fdp1-1";
                        };
                        fdp1_0_clk: clock@119 {
                                reg = <119>;
                                clocks = <&zs_clk>;
                                clock-output-names = "fdp1-0";
                        };
                        tmu3_clk: clock@121 {
                                reg = <121>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu3";
                        };
                        tmu2_clk: clock@122 {
                                reg = <122>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu2";
                        };
                        cmt0_clk: clock@124 {
                                reg = <124>;
                                clocks = <&rclk_clk>;
                                clock-output-names = "cmt0";
                        };
                        tmu0_clk: clock@125 {
                                reg = <125>;
                                clocks = <&cp_clk>;
                                clock-output-names = "tmu0";
                        };
                        vsp1_du1_clk: clock@127 {
                                reg = <127>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-du1";
                        };
                        vsp1_du0_clk: clock@128 {
                                reg = <128>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-du0";
                        };
                        vsp1_sy_clk: clock@131 {
                                reg = <131>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-sy";
                        };

                        /* MSTP2 */
                        scifa2_clk: clock@202 {
                                reg = <202>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa2";
                        };
                        scifa1_clk: clock@203 {
                                reg = <203>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa1";
                        };
                        scifa0_clk: clock@204 {
                                reg = <204>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa0";
                        };
                        msiof2_clk: clock@205 {
                                reg = <205>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof2";
                        };
                        scifb0_clk: clock@206 {
                                reg = <206>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb0";
                        };
                        scifb1_clk: clock@207 {
                                reg = <207>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb1";
                        };
                        msiof1_clk: clock@208 {
                                reg = <208>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof1";
                        };
                        scifb2_clk: clock@216 {
                                reg = <216>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb2";
                        };
                        sys_dmac1_clk: clock@218 {
                                reg = <218>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sys-dmac1";
                        };
                        sys_dmac0_clk: clock@219 {
                                reg = <219>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sys-dmac0";
                        };

                        /* MSTP3 */
                        tpu0_clk: clock@304 {
                                reg = <304>;
                                clocks = <&cp_clk>;
                                clock-output-names = "tpu0";
                        };
                        sdhi2_clk: clock@311 {
                                reg = <311>;
                                clocks = <&sd3_clk>;
                                clock-output-names = "sdhi2";
                        };
                        sdhi1_clk: clock@312 {
                                reg = <312>;
                                clocks = <&sd2_clk>;
                                clock-output-names = "sdhi1";
                        };
                        sdhi0_clk: clock@314 {
                                reg = <314>;
                                clocks = <&cpg_clocks R8A7791_CLK_SD0>;
                                clock-output-names = "sdhi0";
                        };
                        mmcif0_clk: clock@315 {
                                reg = <315>;
                                clocks = <&mmc0_clk>;
                                clock-output-names = "mmcif0";
                        };
                        i2c7_clk: clock@318 {
                                reg = <318>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c7";
                        };
                        pciec_clk: clock@319 {
                                reg = <319>;
                                clocks = <&mp_clk>;
                                clock-output-names = "pciec";
                        };
                        i2c8_clk: clock@323 {
                                reg = <323>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c8";
                        };
                        ssusb_clk: clock@328 {
                                reg = <328>;
                                clocks = <&mp_clk>;
                                clock-output-names = "ssusb";
                        };
                        cmt1_clk: clock@329 {
                                reg = <329>;
                                clocks = <&rclk_clk>;
                                clock-output-names = "cmt1";
                        };
                        usbdmac0_clk: clock@330 {
                                reg = <330>;
                                clocks = <&hp_clk>;
                                clock-output-names = "usbdmac0";
                        };
                        usbdmac1_clk: clock@331 {
                                reg = <331>;
                                clocks = <&hp_clk>;
                                clock-output-names = "usbdmac1";
                        };

                        /* MSTP4 */
                        irqc_clk: clock@407 {
                                reg = <407>;
                                clocks = <&cp_clk>;
                                clock-output-names = "irqc";
                        };
                        intc_sys_clk: clock@408 {
                                reg = <408>;
                                clocks = <&zs_clk>;
                                clock-output-names = "intc-sys";
                        };

                        /* MSTP5 */
                        audmac1_clk: clock@501 {
                                reg = <501>;
                                clocks = <&hp_clk>;
                                clock-output-names = "audmac1";
                        };
                        audmac0_clk: clock@502 {
                                reg = <502>;
                                clocks = <&hp_clk>;
                                clock-output-names = "audmac0";
                        };
                        adsp_mod_clk: clock@506 {
                                reg = <506>;
                                clocks = <&cpg_clocks R8A7791_CLK_ADSP>;
                                clock-output-names = "adsp_mod";
                        };
                        thermal_clk: clock@522 {
                                reg = <522>;
                                clocks = <&extal_clk>;
                                clock-output-names = "thermal";
                        };
                        pwm_clk: clock@523 {
                                reg = <523>;
                                clocks = <&p_clk>;
                                clock-output-names = "pwm";
                        };

                        /* MSTP7 */
                        ehci_clk: clock@703 {
                                reg = <703>;
                                clocks = <&mp_clk>;
                                clock-output-names = "ehci";
                        };
                        hsusb_clk: clock@704 {
                                reg = <704>;
                                clocks = <&hp_clk>;
                                clock-output-names = "hsusb";
                        };
                        hscif2_clk: clock@713 {
                                reg = <713>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif2";
                        };
                        scif5_clk: clock@714 {
                                reg = <714>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif5";
                        };
                        scif4_clk: clock@715 {
                                reg = <715>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif4";
                        };
                        hscif1_clk: clock@716 {
                                reg = <716>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif1";
                        };
                        hscif0_clk: clock@717 {
                                reg = <717>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif0";
                        };
                        scif3_clk: clock@718 {
                                reg = <718>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif3";
                        };
                        scif2_clk: clock@719 {
                                reg = <719>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif2";
                        };
                        scif1_clk: clock@720 {
                                reg = <720>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif1";
                        };
                        scif0_clk: clock@721 {
                                reg = <721>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif0";
                        };
                        du1_clk: clock@723 {
                                reg = <723>;
                                clocks = <&zx_clk>;
                                clock-output-names = "du1";
                        };
                        du0_clk: clock@724 {
                                reg = <724>;
                                clocks = <&zx_clk>;
                                clock-output-names = "du0";
                        };
                        lvds0_clk: clock@726 {
                                reg = <726>;
                                clocks = <&zx_clk>;
                                clock-output-names = "lvds0";
                        };

                        /* MSTP8 */
                        ipmmu_sgx_clk: clock@800 {
                                reg = <800>;
                                clocks = <&zx_clk>;
                                clock-output-names = "ipmmu_sgx";
                        };
                        mlb_clk: clock@802 {
                                reg = <802>;
                                clocks = <&hp_clk>;
                                clock-output-names = "mlb";
                        };
                        vin2_clk: clock@809 {
                                reg = <809>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin2";
                        };
                        vin1_clk: clock@810 {
                                reg = <810>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin1";
                        };
                        vin0_clk: clock@811 {
                                reg = <811>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin0";
                        };
                        ether_clk: clock@813 {
                                reg = <813>;
                                clocks = <&p_clk>;
                                clock-output-names = "ether";
                        };
                        sata1_clk: clock@814 {
                                reg = <814>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sata1";
                        };
                        sata0_clk: clock@815 {
                                reg = <815>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sata0";
                        };

                        /* MSTP9 */
                        gpio7_clk: clock@904 {
                                reg = <904>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio7";
                        };
                        gpio6_clk: clock@905 {
                                reg = <905>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio6";
                        };
                        gpio5_clk: clock@907 {
                                reg = <907>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio5";
                        };
                        gpio4_clk: clock@908 {
                                reg = <908>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio4";
                        };
                        gpio3_clk: clock@909 {
                                reg = <909>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio3";
                        };
                        gpio2_clk: clock@910 {
                                reg = <910>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio2";
                        };
                        gpio1_clk: clock@911 {
                                reg = <911>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio1";
                        };
                        gpio0_clk: clock@912 {
                                reg = <912>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio0";
                        };
                        rcan1_clk: clock@915 {
                                reg = <915>;
                                clocks = <&p_clk>;
                                clock-output-names = "rcan1";
                        };
                        rcan0_clk: clock@916 {
                                reg = <916>;
                                clocks = <&p_clk>;
                                clock-output-names = "rcan0";
                        };
                        qspi_mod_clk: clock@917 {
                                reg = <917>;
                                clocks = <&cpg_clocks R8A7791_CLK_QSPI>;
                                clock-output-names = "qspi_mod";
                        };
                        i2c5_clk: clock@925 {
                                reg = <925>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c5";
                        };
                        i2c6_clk: clock@926 {
                                reg = <926>;
                                clocks = <&cp_clk>;
                                clock-output-names = "i2c6";
                        };
                        i2c4_clk: clock@927 {
                                reg = <927>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c4";
                        };
                        i2c3_clk: clock@928 {
                                reg = <928>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c3";
                        };
                        i2c2_clk: clock@929 {
                                reg = <929>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c2";
                        };
                        i2c1_clk: clock@930 {
                                reg = <930>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c1";
                        };
                        i2c0_clk: clock@931 {
                                reg = <931>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c0";
                        };

                        /* MSTP10 */
                        ssi_all_clk: clock@1005 {
                                reg = <1005>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi-all";
                        };
                        ssi9_clk: clock@1006 {
                                reg = <1006>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi9";
                        };
                        ssi8_clk: clock@1007 {
                                reg = <1007>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi8";
                        };
                        ssi7_clk: clock@1008 {
                                reg = <1008>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi7";
                        };
                        ssi6_clk: clock@1009 {
                                reg = <1009>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi6";
                        };
                        ssi5_clk: clock@1010 {
                                reg = <1010>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi5";
                        };
                        ssi4_clk: clock@1011 {
                                reg = <1011>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi4";
                        };
                        ssi3_clk: clock@1012 {
                                reg = <1012>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi3";
                        };
                        ssi2_clk: clock@1013 {
                                reg = <1013>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi2";
                        };
                        ssi1_clk: clock@1014 {
                                reg = <1014>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi1";
                        };
                        ssi0_clk: clock@1015 {
                                reg = <1015>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi0";
                        };
                        scu_all_clk: clock@1017 {
                                reg = <1017>;
                                clocks = <&p_clk>;
                                clock-output-names = "scu-all";
                        };
                        scu_dvc1_clk: clock@1018 {
                                reg = <1018>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-dvc1";
                        };
                        scu_dvc0_clk: clock@1019 {
                                reg = <1019>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-dvc0";
                        };
                        scu_src9_clk: clock@1022 {
                                reg = <1022>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src9";
                        };
                        scu_src8_clk: clock@1023 {
                                reg = <1023>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src8";
                        };
                        scu_src7_clk: clock@1024 {
                                reg = <1024>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src7";
                        };
                        scu_src6_clk: clock@1025 {
                                reg = <1025>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src6";
                        };
                        scu_src5_clk: clock@1026 {
                                reg = <1026>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src5";
                        };
                        scu_src4_clk: clock@1027 {
                                reg = <1027>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src4";
                        };
                        scu_src3_clk: clock@1028 {
                                reg = <1028>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src3";
                        };
                        scu_src2_clk: clock@1029 {
                                reg = <1029>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src2";
                        };
                        scu_src1_clk: clock@1030 {
                                reg = <1030>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src1";
                        };
                        scu_src0_clk: clock@1031 {
                                reg = <1031>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src0";
                        };

                        /* MSTP11 */
                        scifa3_clk: clock@1106 {
                                reg = <1106>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa3";
                        };
                        scifa4_clk: clock@1107 {
                                reg = <1107>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa4";
                        };
                        scifa5_clk: clock@1108 {
                                reg = <1108>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa5";
                        };
                };
        };


References
----------

  [1] Documentation/devicetree/bindings/clock/renesas,<SoC>-cpg-clocks.txt
      where <SoC> is a single SoC name or a family of SoCs
  [2] Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
  [3] Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
  [4] Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
  [5] arch/arm/boot/dts/{sh7,r7,r8}*.dtsi
  [6] Series "[PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains"
      (https://lkml.org/lkml/2015/5/28/590,
       http://lkml.iu.edu/hypermail/linux/kernel/1505.3/03336.html)
  [7] Series "[PATCH 0/6][RFC] arm64: Renesas Gen3 initial patch"
      (http://www.spinics.net/lists/linux-sh/msg42683.html)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Renesas CPG/MSTP DT Binding Proposal
@ 2015-06-22 19:49 ` Geert Uytterhoeven
  0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-06-22 19:49 UTC (permalink / raw)
  To: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd
  Cc: linux-clk, Linux PM list, devicetree, Linux-sh list

Introduction
------------

On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
MSTP (Module Standby and Software Reset), and APMU (Advanced Power Management
Unit for AP-System Core) blocks are very intimately tied.

  - The CPG clock generates various clocks for LSI internal generation,
  - The MSTP block provides two functions:
      1. Module Standby: "Clock supply to specified modules is stopped by
         setting the module stop control register bits."
         However, the clock supply to a module is not stopped until all CPUs
         in the SoC agree.  Indeed, there are separate MSTP registers for
         application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
      2. Reset Control. to perform a software reset of a specific module.
  - The APMU controls power and clock supply to the AP-system core (e.g.
    CA7, CA15, SCU).

MSTP and CPG registers are mixed in one register block.
The APMU has one or two separate register blocks.

The current DT bindings are split across 4 different binding documents:
  1. Renesas SoC-specific "core" CPG clocks that cannot be represented easily
     in DT in another way (cfr. [1]),
  2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
  3. Renesas-specific "div6" variable factor clocks, also generated by the CPG
     (cfr. [3]).
  3. Generic "fixed-factor-clock" clocks, generated by the CPG, but represented
     in DT using the generic ""fixed-factor-clock" bindings (cfr. [4]),

Note that currently the Reset Control function of the MSTP block is not
supported by DT nor Linux, and that there are no DT bindings for the APMU block
yet.


Issues with current bindings
----------------------------

General:
  - Tight coupling of CPG and MSTP is not represented in DT:
      - There's one CPG node, and separate MSTP nodes (one for each block of
        up to 32 possible MSTP clocks) next to it,
      - CPG Clock Domain requires "power-domain = <...>" properties linking
        individual device nodes to the "CPG/MSTP" clock domain provider.
        For now I use a link to the (single) CPG device node, while the clock
        provider (mostly) controls the (multiple) MSTP clocks (cfr. [6]).
      - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
        requires writing a special value to a specific "Write Protect
        Register" inside the CPG first. Exact details and impact are still
        under investigation (cfr. [7]).

MSTP-specific:
  - The clock hierarchy (parent-child relationship) is represented in a flat
    structure,
  - Multiple arrays have to be kept in sync:
      - "clocks" (parents),
      - "clock-indices" (sparse bit index inside register),
      - "clock-output-names",
  - MSTP clocks are referred to using a reference to a device node and a bit
    index:
       - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h are
         separated from the .dtsi file,
       - There's no protection against using a bit index definition for a
         different MSTP register, e.g.
           - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
           - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),
   - The "reg" properties contain only the SMSTPCRx (control register for
     System CPU) and MSTPSRx (optional status register) registers only.
     However, in reality there are many more registers:
       - Separate control registers for application (Cortex-A, supported) and
         real-time (SH and/or Cortex-R, not supported) cores,
       - Software reset registers (not supported).
     To make matters more complicated:
       - Many registers are optional.
         The current bindings just support one control register and one
         optional status register, but this doesn't scale to more register
         sets,
       - Register sets are not linear and not contiguous in the register space.
         I.e. it's not possible to derive the address of SMSTPCRx from the
         address of SMSTPCRy, or the address of MSTPSRx from the address of
         SMSTPCRx.


Proposal
--------

I'd like to propose the following, to resolve (most of) the issues with the
current bindings:

  - Get rid of the "clocks" node.
    IIRC grouping all clocks under a "clock" node was deprecated, but I can't
    find the email anymore?
      - "fixed-clock" clocks are now at the root node level,
      - The CPG node is now at the root node level,
  - CPG node:
      - Use e.g. "renesas,r8a7791-cpg" as compatible value,
  - MSTP clocks:
      - All MSTP clocks are now grouped inside the CPG node, under a new "mstp"
        subnode, without compatible values,
      - Each individual MSTP clock has its own subnode, referring to the MSTP
        clock number, instead of using one node per register, and arrays of
        "clocks", "clock-output-names", and "clock-indices" properties.
      - The list of available registers is maintained as arrays inside the
        unified CPG/MSTP driver, e.g.

        struct cpg {
                void __iomem *base;
                unsigned int n_mstp;    /* Size of a register set */
                /*
                 * Optional arrays of register offsets for the various
                 * register sets:
                 *   - Array pointer may be NULL, if not supported */
                 *   - Array element may be ~0, if not supported */
                 */
                u16 *smstp;             /* System CPU control registers */
                u16 *rmstp;             /* Realtime CPU control registers */
                u16 *mstpsr;            /* Status registers */
                u16 *srcr;              /* Reset registers */
                ...
        };


Short example
-------------

(from Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt)

For the MSTP part:

        #include <dt-bindings/clock/r8a7790-clock.h>

        mstp3_clks: mstp3_clks@e615013c {
                compatible = "renesas,r8a7790-mstp-clocks",
                             "renesas,cpg-mstp-clocks";
                reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
                clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
                         <&cpg_clocks R8A7790_CLK_SD1>,
                         <&cpg_clocks R8A7790_CLK_SD0>, <&mmc0_clk>;
                #clock-cells = <1>;
                clock-output-names =
                        "tpu0", "mmcif1", "sdhi3", "sdhi2",
                         "sdhi1", "sdhi0", "mmcif0";
                clock-indices = <
                        R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1 R8A7790_CLK_SDHI3
                        R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
                        R8A7790_CLK_MMCIF0
                >;
        };

would become:

        mstp {
                #address-cells = <1>;
                #size-cells = <0>;
                ...

                /* MSTP3 */
                tpu0_clk: clock@304 {
                        reg = <304>;
                        clocks = <&cp_clk>;
                        clock-output-names = "tpu0";
                };
                mmcif1_clk: clock@305 {
                        reg = <305>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };
                sdhi3_clk: clock@311 {
                        reg = <311>;
                        clocks = <&sd3_clk>;
                        clock-output-names = "sdhi3";
                };
                sdhi2_clk: clock@312 {
                        reg = <312>;
                        clocks = <&sd2_clk>;
                        clock-output-names = "sdhi2";
                };
                sdhi1_clk: clock@313 {
                        reg = <313>;
                        clocks = <&cpg_clocks R8A7790_CLK_SD1>;
                        clock-output-names = "sdhi1";
                };
                sdhi0_clk: clock@314 {
                        reg = <314>;
                        clocks = <&cpg_clocks R8A7790_CLK_SD0>;
                        clock-output-names = "sdhi0";
                };
                mmcif0_clk: clock@315 {
                        reg = <315>;
                        clocks = <&mmc0_clk>;
                        clock-output-names = "mmcif0";
                };

                ...
        }

Devices would no longer use e.g. "clocks = <&mstp3_clks R8A7790_CLK_MMCIF1>",
but "clocks = <&mmcif1_clk>".


Questions/alternatives
----------------------

  - Should the subnode be called "mssr" (Module Standby and Software Reset)
    instead of "mstp"?
  - What with "renesas,*-cpg-clocks" clocks?
      - Should we keep on using "#clock-cells = <1>" and <SOC>_CLK_<NAME>
        indices, or switch to a less error-prone "#clock-cells = <0>"?
  - What with "renesas,cpg-div6-clock" clocks?
      - For now I kept them as a subnode of cpg_clocks, as their registers are
        a subset of the CPG registers.
      - This also means I had to keep the "#address-cells = <2>",
        "#size-cells = <2>", and "ranges" properties under the CPG node.
  - The MSTP number could be split in register and bit offset (and thus use
    "#address-cells = <2>"), e.g.:

                mmcif1_clk: clock@3,5 {
                        reg = <3 5>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };

        vs.

                mmcif1_clk: clock@305 {
                        reg = <305>;
                        clocks = <&mmc1_clk>;
                        clock-output-names = "mmcif1";
                };

    I don't think it's clearer, as the datasheet refers to e.g. "MSTP305".


Thanks for your comments!


Long example
------------

All r8a7791 clocks:

        /* External root clock */
        extal_clk: extal_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                /* This value must be overriden by the board. */
                clock-frequency = <0>;
                clock-output-names = "extal";
        };

        /*
         * The external audio clocks are configured as 0 Hz fixed
frequency clocks by
         * default. Boards that provide audio clocks should override them.
         */
        audio_clk_a: audio_clk_a {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_a";
        };
        audio_clk_b: audio_clk_b {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_b";
        };
        audio_clk_c: audio_clk_c {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <0>;
                clock-output-names = "audio_clk_c";
        };

        /* External PCIe clock - can be overridden by the board */
        pcie_bus_clk: pcie_bus_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <100000000>;
                clock-output-names = "pcie_bus";
                status = "disabled";
        };

        /* External USB clock - can be overridden by the board */
        usb_extal_clk: usb_extal_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                clock-frequency = <48000000>;
                clock-output-names = "usb_extal";
        };

        /* External CAN clock */
        can_clk: can_clk {
                compatible = "fixed-clock";
                #clock-cells = <0>;
                /* This value must be overridden by the board. */
                clock-frequency = <0>;
                clock-output-names = "can_clk";
                status = "disabled";
        };

        /* Fixed factor clocks */
        pll1_div2_clk: pll1_div2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "pll1_div2";
        };
        zg_clk: zg_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <3>;
                clock-mult = <1>;
                clock-output-names = "zg";
        };
        zx_clk: zx_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <3>;
                clock-mult = <1>;
                clock-output-names = "zx";
        };
        zs_clk: zs_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <6>;
                clock-mult = <1>;
                clock-output-names = "zs";
        };
        hp_clk: hp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <12>;
                clock-mult = <1>;
                clock-output-names = "hp";
        };
        i_clk: i_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "i";
        };
        b_clk: b_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <12>;
                clock-mult = <1>;
                clock-output-names = "b";
        };
        p_clk: p_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <24>;
                clock-mult = <1>;
                clock-output-names = "p";
        };
        cl_clk: cl_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <48>;
                clock-mult = <1>;
                clock-output-names = "cl";
        };
        m2_clk: m2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "m2";
        };
        imp_clk: imp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <4>;
                clock-mult = <1>;
                clock-output-names = "imp";
        };
        rclk_clk: rclk_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <(48 * 1024)>;
                clock-mult = <1>;
                clock-output-names = "rclk";
        };
        oscclk_clk: oscclk_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
                #clock-cells = <0>;
                clock-div = <(12 * 1024)>;
                clock-mult = <1>;
                clock-output-names = "oscclk";
        };
        zb3_clk: zb3_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <4>;
                clock-mult = <1>;
                clock-output-names = "zb3";
        };
        zb3d2_clk: zb3d2_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "zb3d2";
        };
        ddr_clk: ddr_clk {
                compatible = "fixed-factor-clock";
                clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
                #clock-cells = <0>;
                clock-div = <8>;
                clock-mult = <1>;
                clock-output-names = "ddr";
        };
        mp_clk: mp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&pll1_div2_clk>;
                #clock-cells = <0>;
                clock-div = <15>;
                clock-mult = <1>;
                clock-output-names = "mp";
        };
        cp_clk: cp_clk {
                compatible = "fixed-factor-clock";
                clocks = <&extal_clk>;
                #clock-cells = <0>;
                clock-div = <2>;
                clock-mult = <1>;
                clock-output-names = "cp";
        };

        /* Special CPG clocks */
        cpg_clocks: cpg_clocks@e6150000 {
                #address-cells = <2>;
                #size-cells = <2>;
                ranges;

                compatible = "renesas,r8a7791-cpg",
                reg = <0 0xe6150000 0 0x1000>;
                clocks = <&extal_clk &usb_extal_clk>;
                #clock-cells = <1>;
                clock-output-names = "main", "pll0", "pll1", "pll3",
                                     "lb", "qspi", "sdh", "sd0", "z",
                                     "rcan", "adsp";
                #power-domain-cells = <0>;

                /* Variable factor clocks */
                sd2_clk: sd2_clk@e6150078 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150078 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "sd2";
                };
                sd3_clk: sd3_clk@e615026c {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe615026c 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "sd3";
                };
                mmc0_clk: mmc0_clk@e6150240 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150240 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "mmc0";
                };
                ssp_clk: ssp_clk@e6150248 {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe6150248 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "ssp";
                };
                ssprs_clk: ssprs_clk@e615024c {
                        compatible = "renesas,r8a7791-div6-clock",
                                     "renesas,cpg-div6-clock";
                        reg = <0 0xe615024c 0 4>;
                        clocks = <&pll1_div2_clk>;
                        #clock-cells = <0>;
                        clock-output-names = "ssprs";
                };

                /* Module Standby and Software Reset */
                mstp {
                        #address-cells = <1>;
                        #size-cells = <0>;

                        /* MSTP0 */
                        msiof0_clk: clock@000 {
                                reg = <000>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof0";
                        };

                        /* MSTP1 */
                        vcp0_clk: clock@101 {
                                reg = <101>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vcp0";
                        };
                        vpc0_clk: clock@103 {
                                reg = <103>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vpc0";
                        };
                        jpu_clk: clock@106 {
                                reg = <106>;
                                clocks = <&m2_clk>;
                                clock-output-names = "jpu";
                        };
                        ssp1_clk: clock@109 {
                                reg = <109>;
                                clocks = <&zs_clk>;
                                clock-output-names = "ssp1";
                        };
                        tmu1_clk: clock@111 {
                                reg = <111>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu1";
                        };
                        threedg_clk: clock@112 {
                                reg = <112>;
                                clocks = <&zg_clk>;
                                clock-output-names = "3dg";
                        };
                        twoddmac_clk: clock@115 {
                                reg = <115>;
                                clocks = <&zs_clk>;
                                clock-output-names = "2ddmac";
                        };
                        fdp1_1_clk: clock@118 {
                                reg = <118>;
                                clocks = <&zs_clk>;
                                clock-output-names = "fdp1-1";
                        };
                        fdp1_0_clk: clock@119 {
                                reg = <119>;
                                clocks = <&zs_clk>;
                                clock-output-names = "fdp1-0";
                        };
                        tmu3_clk: clock@121 {
                                reg = <121>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu3";
                        };
                        tmu2_clk: clock@122 {
                                reg = <122>;
                                clocks = <&p_clk>;
                                clock-output-names = "tmu2";
                        };
                        cmt0_clk: clock@124 {
                                reg = <124>;
                                clocks = <&rclk_clk>;
                                clock-output-names = "cmt0";
                        };
                        tmu0_clk: clock@125 {
                                reg = <125>;
                                clocks = <&cp_clk>;
                                clock-output-names = "tmu0";
                        };
                        vsp1_du1_clk: clock@127 {
                                reg = <127>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-du1";
                        };
                        vsp1_du0_clk: clock@128 {
                                reg = <128>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-du0";
                        };
                        vsp1_sy_clk: clock@131 {
                                reg = <131>;
                                clocks = <&zs_clk>;
                                clock-output-names = "vsp1-sy";
                        };

                        /* MSTP2 */
                        scifa2_clk: clock@202 {
                                reg = <202>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa2";
                        };
                        scifa1_clk: clock@203 {
                                reg = <203>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa1";
                        };
                        scifa0_clk: clock@204 {
                                reg = <204>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa0";
                        };
                        msiof2_clk: clock@205 {
                                reg = <205>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof2";
                        };
                        scifb0_clk: clock@206 {
                                reg = <206>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb0";
                        };
                        scifb1_clk: clock@207 {
                                reg = <207>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb1";
                        };
                        msiof1_clk: clock@208 {
                                reg = <208>;
                                clocks = <&mp_clk>;
                                clock-output-names = "msiof1";
                        };
                        scifb2_clk: clock@216 {
                                reg = <216>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifb2";
                        };
                        sys_dmac1_clk: clock@218 {
                                reg = <218>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sys-dmac1";
                        };
                        sys_dmac0_clk: clock@219 {
                                reg = <219>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sys-dmac0";
                        };

                        /* MSTP3 */
                        tpu0_clk: clock@304 {
                                reg = <304>;
                                clocks = <&cp_clk>;
                                clock-output-names = "tpu0";
                        };
                        sdhi2_clk: clock@311 {
                                reg = <311>;
                                clocks = <&sd3_clk>;
                                clock-output-names = "sdhi2";
                        };
                        sdhi1_clk: clock@312 {
                                reg = <312>;
                                clocks = <&sd2_clk>;
                                clock-output-names = "sdhi1";
                        };
                        sdhi0_clk: clock@314 {
                                reg = <314>;
                                clocks = <&cpg_clocks R8A7791_CLK_SD0>;
                                clock-output-names = "sdhi0";
                        };
                        mmcif0_clk: clock@315 {
                                reg = <315>;
                                clocks = <&mmc0_clk>;
                                clock-output-names = "mmcif0";
                        };
                        i2c7_clk: clock@318 {
                                reg = <318>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c7";
                        };
                        pciec_clk: clock@319 {
                                reg = <319>;
                                clocks = <&mp_clk>;
                                clock-output-names = "pciec";
                        };
                        i2c8_clk: clock@323 {
                                reg = <323>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c8";
                        };
                        ssusb_clk: clock@328 {
                                reg = <328>;
                                clocks = <&mp_clk>;
                                clock-output-names = "ssusb";
                        };
                        cmt1_clk: clock@329 {
                                reg = <329>;
                                clocks = <&rclk_clk>;
                                clock-output-names = "cmt1";
                        };
                        usbdmac0_clk: clock@330 {
                                reg = <330>;
                                clocks = <&hp_clk>;
                                clock-output-names = "usbdmac0";
                        };
                        usbdmac1_clk: clock@331 {
                                reg = <331>;
                                clocks = <&hp_clk>;
                                clock-output-names = "usbdmac1";
                        };

                        /* MSTP4 */
                        irqc_clk: clock@407 {
                                reg = <407>;
                                clocks = <&cp_clk>;
                                clock-output-names = "irqc";
                        };
                        intc_sys_clk: clock@408 {
                                reg = <408>;
                                clocks = <&zs_clk>;
                                clock-output-names = "intc-sys";
                        };

                        /* MSTP5 */
                        audmac1_clk: clock@501 {
                                reg = <501>;
                                clocks = <&hp_clk>;
                                clock-output-names = "audmac1";
                        };
                        audmac0_clk: clock@502 {
                                reg = <502>;
                                clocks = <&hp_clk>;
                                clock-output-names = "audmac0";
                        };
                        adsp_mod_clk: clock@506 {
                                reg = <506>;
                                clocks = <&cpg_clocks R8A7791_CLK_ADSP>;
                                clock-output-names = "adsp_mod";
                        };
                        thermal_clk: clock@522 {
                                reg = <522>;
                                clocks = <&extal_clk>;
                                clock-output-names = "thermal";
                        };
                        pwm_clk: clock@523 {
                                reg = <523>;
                                clocks = <&p_clk>;
                                clock-output-names = "pwm";
                        };

                        /* MSTP7 */
                        ehci_clk: clock@703 {
                                reg = <703>;
                                clocks = <&mp_clk>;
                                clock-output-names = "ehci";
                        };
                        hsusb_clk: clock@704 {
                                reg = <704>;
                                clocks = <&hp_clk>;
                                clock-output-names = "hsusb";
                        };
                        hscif2_clk: clock@713 {
                                reg = <713>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif2";
                        };
                        scif5_clk: clock@714 {
                                reg = <714>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif5";
                        };
                        scif4_clk: clock@715 {
                                reg = <715>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif4";
                        };
                        hscif1_clk: clock@716 {
                                reg = <716>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif1";
                        };
                        hscif0_clk: clock@717 {
                                reg = <717>;
                                clocks = <&zs_clk>;
                                clock-output-names = "hscif0";
                        };
                        scif3_clk: clock@718 {
                                reg = <718>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif3";
                        };
                        scif2_clk: clock@719 {
                                reg = <719>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif2";
                        };
                        scif1_clk: clock@720 {
                                reg = <720>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif1";
                        };
                        scif0_clk: clock@721 {
                                reg = <721>;
                                clocks = <&p_clk>;
                                clock-output-names = "scif0";
                        };
                        du1_clk: clock@723 {
                                reg = <723>;
                                clocks = <&zx_clk>;
                                clock-output-names = "du1";
                        };
                        du0_clk: clock@724 {
                                reg = <724>;
                                clocks = <&zx_clk>;
                                clock-output-names = "du0";
                        };
                        lvds0_clk: clock@726 {
                                reg = <726>;
                                clocks = <&zx_clk>;
                                clock-output-names = "lvds0";
                        };

                        /* MSTP8 */
                        ipmmu_sgx_clk: clock@800 {
                                reg = <800>;
                                clocks = <&zx_clk>;
                                clock-output-names = "ipmmu_sgx";
                        };
                        mlb_clk: clock@802 {
                                reg = <802>;
                                clocks = <&hp_clk>;
                                clock-output-names = "mlb";
                        };
                        vin2_clk: clock@809 {
                                reg = <809>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin2";
                        };
                        vin1_clk: clock@810 {
                                reg = <810>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin1";
                        };
                        vin0_clk: clock@811 {
                                reg = <811>;
                                clocks = <&zg_clk>;
                                clock-output-names = "vin0";
                        };
                        ether_clk: clock@813 {
                                reg = <813>;
                                clocks = <&p_clk>;
                                clock-output-names = "ether";
                        };
                        sata1_clk: clock@814 {
                                reg = <814>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sata1";
                        };
                        sata0_clk: clock@815 {
                                reg = <815>;
                                clocks = <&zs_clk>;
                                clock-output-names = "sata0";
                        };

                        /* MSTP9 */
                        gpio7_clk: clock@904 {
                                reg = <904>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio7";
                        };
                        gpio6_clk: clock@905 {
                                reg = <905>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio6";
                        };
                        gpio5_clk: clock@907 {
                                reg = <907>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio5";
                        };
                        gpio4_clk: clock@908 {
                                reg = <908>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio4";
                        };
                        gpio3_clk: clock@909 {
                                reg = <909>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio3";
                        };
                        gpio2_clk: clock@910 {
                                reg = <910>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio2";
                        };
                        gpio1_clk: clock@911 {
                                reg = <911>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio1";
                        };
                        gpio0_clk: clock@912 {
                                reg = <912>;
                                clocks = <&cp_clk>;
                                clock-output-names = "gpio0";
                        };
                        rcan1_clk: clock@915 {
                                reg = <915>;
                                clocks = <&p_clk>;
                                clock-output-names = "rcan1";
                        };
                        rcan0_clk: clock@916 {
                                reg = <916>;
                                clocks = <&p_clk>;
                                clock-output-names = "rcan0";
                        };
                        qspi_mod_clk: clock@917 {
                                reg = <917>;
                                clocks = <&cpg_clocks R8A7791_CLK_QSPI>;
                                clock-output-names = "qspi_mod";
                        };
                        i2c5_clk: clock@925 {
                                reg = <925>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c5";
                        };
                        i2c6_clk: clock@926 {
                                reg = <926>;
                                clocks = <&cp_clk>;
                                clock-output-names = "i2c6";
                        };
                        i2c4_clk: clock@927 {
                                reg = <927>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c4";
                        };
                        i2c3_clk: clock@928 {
                                reg = <928>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c3";
                        };
                        i2c2_clk: clock@929 {
                                reg = <929>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c2";
                        };
                        i2c1_clk: clock@930 {
                                reg = <930>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c1";
                        };
                        i2c0_clk: clock@931 {
                                reg = <931>;
                                clocks = <&hp_clk>;
                                clock-output-names = "i2c0";
                        };

                        /* MSTP10 */
                        ssi_all_clk: clock@1005 {
                                reg = <1005>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi-all";
                        };
                        ssi9_clk: clock@1006 {
                                reg = <1006>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi9";
                        };
                        ssi8_clk: clock@1007 {
                                reg = <1007>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi8";
                        };
                        ssi7_clk: clock@1008 {
                                reg = <1008>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi7";
                        };
                        ssi6_clk: clock@1009 {
                                reg = <1009>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi6";
                        };
                        ssi5_clk: clock@1010 {
                                reg = <1010>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi5";
                        };
                        ssi4_clk: clock@1011 {
                                reg = <1011>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi4";
                        };
                        ssi3_clk: clock@1012 {
                                reg = <1012>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi3";
                        };
                        ssi2_clk: clock@1013 {
                                reg = <1013>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi2";
                        };
                        ssi1_clk: clock@1014 {
                                reg = <1014>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi1";
                        };
                        ssi0_clk: clock@1015 {
                                reg = <1015>;
                                clocks = <&p_clk>;
                                clock-output-names = "ssi0";
                        };
                        scu_all_clk: clock@1017 {
                                reg = <1017>;
                                clocks = <&p_clk>;
                                clock-output-names = "scu-all";
                        };
                        scu_dvc1_clk: clock@1018 {
                                reg = <1018>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-dvc1";
                        };
                        scu_dvc0_clk: clock@1019 {
                                reg = <1019>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-dvc0";
                        };
                        scu_src9_clk: clock@1022 {
                                reg = <1022>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src9";
                        };
                        scu_src8_clk: clock@1023 {
                                reg = <1023>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src8";
                        };
                        scu_src7_clk: clock@1024 {
                                reg = <1024>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src7";
                        };
                        scu_src6_clk: clock@1025 {
                                reg = <1025>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src6";
                        };
                        scu_src5_clk: clock@1026 {
                                reg = <1026>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src5";
                        };
                        scu_src4_clk: clock@1027 {
                                reg = <1027>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src4";
                        };
                        scu_src3_clk: clock@1028 {
                                reg = <1028>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src3";
                        };
                        scu_src2_clk: clock@1029 {
                                reg = <1029>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src2";
                        };
                        scu_src1_clk: clock@1030 {
                                reg = <1030>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src1";
                        };
                        scu_src0_clk: clock@1031 {
                                reg = <1031>;
                                clocks = <&scu_all_clk>;
                                clock-output-names = "scu-src0";
                        };

                        /* MSTP11 */
                        scifa3_clk: clock@1106 {
                                reg = <1106>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa3";
                        };
                        scifa4_clk: clock@1107 {
                                reg = <1107>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa4";
                        };
                        scifa5_clk: clock@1108 {
                                reg = <1108>;
                                clocks = <&mp_clk>;
                                clock-output-names = "scifa5";
                        };
                };
        };


References
----------

  [1] Documentation/devicetree/bindings/clock/renesas,<SoC>-cpg-clocks.txt
      where <SoC> is a single SoC name or a family of SoCs
  [2] Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
  [3] Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
  [4] Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
  [5] arch/arm/boot/dts/{sh7,r7,r8}*.dtsi
  [6] Series "[PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains"
      (https://lkml.org/lkml/2015/5/28/590,
       http://lkml.iu.edu/hypermail/linux/kernel/1505.3/03336.html)
  [7] Series "[PATCH 0/6][RFC] arm64: Renesas Gen3 initial patch"
      (http://www.spinics.net/lists/linux-sh/msg42683.html)

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Renesas CPG/MSTP DT Binding Proposal
  2015-06-22 19:49 ` Geert Uytterhoeven
  (?)
@ 2015-06-22 20:36   ` Laurent Pinchart
  -1 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2015-06-22 20:36 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd, linux-clk, Linux PM list, devicetree,
	Linux-sh list

Hi Geert,

On Monday 22 June 2015 21:49:23 Geert Uytterhoeven wrote:
> Introduction
> ------------
> 
> On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
> MSTP (Module Standby and Software Reset), and APMU (Advanced Power
> Management Unit for AP-System Core) blocks are very intimately tied.
> 
>   - The CPG clock generates various clocks for LSI internal generation,
>   - The MSTP block provides two functions:
>       1. Module Standby: "Clock supply to specified modules is stopped by
>          setting the module stop control register bits."
>          However, the clock supply to a module is not stopped until all CPUs
>          in the SoC agree.  Indeed, there are separate MSTP registers for
>          application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
>       2. Reset Control. to perform a software reset of a specific module.
>   - The APMU controls power and clock supply to the AP-system core (e.g.
>   CA7, CA15, SCU).
> 
> MSTP and CPG registers are mixed in one register block.
> The APMU has one or two separate register blocks.
> 
> The current DT bindings are split across 4 different binding documents:
>   1. Renesas SoC-specific "core" CPG clocks that cannot be represented
> easily in DT in another way (cfr. [1]),
>   2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
>   3. Renesas-specific "div6" variable factor clocks, also generated by the
> CPG (cfr. [3]).
>   3. Generic "fixed-factor-clock" clocks, generated by the CPG, but
> represented in DT using the generic ""fixed-factor-clock" bindings (cfr.
> [4]),
> 
> Note that currently the Reset Control function of the MSTP block is not
> supported by DT nor Linux, and that there are no DT bindings for the APMU
> block yet.
> 
> 
> Issues with current bindings
> ----------------------------
> 
> General:
>   - Tight coupling of CPG and MSTP is not represented in DT:
>       - There's one CPG node, and separate MSTP nodes (one for each block of
>         up to 32 possible MSTP clocks) next to it,
>       - CPG Clock Domain requires "power-domain = <...>" properties linking
>         individual device nodes to the "CPG/MSTP" clock domain provider.
>         For now I use a link to the (single) CPG device node, while the
>         clock provider (mostly) controls the (multiple) MSTP clocks (cfr.
>         [6]).
>       - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
>         requires writing a special value to a specific "Write Protect
>         Register" inside the CPG first. Exact details and impact are still
>         under investigation (cfr. [7]).
> 
> MSTP-specific:
>   - The clock hierarchy (parent-child relationship) is represented in a flat
>     structure,

What do you mean exactly here ?

>   - Multiple arrays have to be kept in sync:
>       - "clocks" (parents),
>       - "clock-indices" (sparse bit index inside register),
>       - "clock-output-names",
>   - MSTP clocks are referred to using a reference to a device node and a bit
>     index:
>        - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h
>          are separated from the .dtsi file,
>        - There's no protection against using a bit index definition for a
>          different MSTP register, e.g.
>            - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
>            - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),

If this concerns you, we could add the MSTP number to the macro name, making 
it easier to catch such issues.

>    - The "reg" properties contain only the SMSTPCRx (control register for
>      System CPU) and MSTPSRx (optional status register) registers only.
>      However, in reality there are many more registers:
>        - Separate control registers for application (Cortex-A, supported)
>          and real-time (SH and/or Cortex-R, not supported) cores,
>        - Software reset registers (not supported).
>      To make matters more complicated:
>        - Many registers are optional.
>          The current bindings just support one control register and one
>          optional status register, but this doesn't scale to more register
>          sets,

We could just use named registers for that.

>        - Register sets are not linear and not contiguous in the register
>          space. I.e. it's not possible to derive the address of SMSTPCRx
>          from the address of SMSTPCRy, or the address of MSTPSRx from the
>          address of SMSTPCRx.

When the hardware is to be blamed we seem to all agree easily :-)

> Proposal
> --------
> 
> I'd like to propose the following, to resolve (most of) the issues with the
> current bindings:
> 
>   - Get rid of the "clocks" node.
>     IIRC grouping all clocks under a "clock" node was deprecated, but I
>     can't find the email anymore?
>       - "fixed-clock" clocks are now at the root node level,

I wonder whether we should move all clocks to a separate .dtsi, they take 
quite a bit of space.

>       - The CPG node is now at the root node level,

The CPG node should actually be a child of a bus, not a child of the root 
node. That's a separate issue though.

>   - CPG node:
>       - Use e.g. "renesas,r8a7791-cpg" as compatible value,
>   - MSTP clocks:
>       - All MSTP clocks are now grouped inside the CPG node, under a new
>         "mstp" subnode, without compatible values,

We've discussed this several times in the past and agreed, the MSTP nodes 
should be subnodes of the CPG node.

>       - Each individual MSTP clock has its own subnode, referring to the
>         MSTP clock number, instead of using one node per register, and
>         arrays of "clocks", "clock-output-names", and "clock-indices"
>         properties.

That I don't really like. It would result in more than a hundred of clock 
nodes. I have discussed this option with Mike when designing the clock DT 
bindings and he didn't like it either.

>       - The list of available registers is maintained as arrays inside the
>         unified CPG/MSTP driver, e.g.
> 
>         struct cpg {
>                 void __iomem *base;
>                 unsigned int n_mstp;    /* Size of a register set */
>                 /*
>                  * Optional arrays of register offsets for the various
>                  * register sets:
>                  *   - Array pointer may be NULL, if not supported */
>                  *   - Array element may be ~0, if not supported */
>                  */
>                 u16 *smstp;             /* System CPU control registers */
>                 u16 *rmstp;             /* Realtime CPU control registers */
>                 u16 *mstpsr;            /* Status registers */
>                 u16 *srcr;              /* Reset registers */
>                 ...

I'd create a struct to group smstp, rmstp, mstpsr, srcr & co and have a single 
array of those structures.

>         };
> 
> 
> Short example
> -------------
> 
> (from Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt)
> 
> For the MSTP part:
> 
>         #include <dt-bindings/clock/r8a7790-clock.h>
> 
>         mstp3_clks: mstp3_clks@e615013c {
>                 compatible = "renesas,r8a7790-mstp-clocks",
>                              "renesas,cpg-mstp-clocks";
>                 reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
>                 clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
>                          <&cpg_clocks R8A7790_CLK_SD1>,
>                          <&cpg_clocks R8A7790_CLK_SD0>, <&mmc0_clk>;
>                 #clock-cells = <1>;
>                 clock-output-names >                         "tpu0", "mmcif1", "sdhi3", "sdhi2",
>                          "sdhi1", "sdhi0", "mmcif0";
>                 clock-indices = <
>                         R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1
> R8A7790_CLK_SDHI3 R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
> R8A7790_CLK_MMCIF0
> 
>                 >;
> 
>         };
> 
> would become:
> 
>         mstp {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 ...
> 
>                 /* MSTP3 */
>                 tpu0_clk: clock@304 {
>                         reg = <304>;
>                         clocks = <&cp_clk>;
>                         clock-output-names = "tpu0";
>                 };
>                 mmcif1_clk: clock@305 {
>                         reg = <305>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
>                 sdhi3_clk: clock@311 {
>                         reg = <311>;
>                         clocks = <&sd3_clk>;
>                         clock-output-names = "sdhi3";
>                 };
>                 sdhi2_clk: clock@312 {
>                         reg = <312>;
>                         clocks = <&sd2_clk>;
>                         clock-output-names = "sdhi2";
>                 };
>                 sdhi1_clk: clock@313 {
>                         reg = <313>;
>                         clocks = <&cpg_clocks R8A7790_CLK_SD1>;
>                         clock-output-names = "sdhi1";
>                 };
>                 sdhi0_clk: clock@314 {
>                         reg = <314>;
>                         clocks = <&cpg_clocks R8A7790_CLK_SD0>;
>                         clock-output-names = "sdhi0";
>                 };
>                 mmcif0_clk: clock@315 {
>                         reg = <315>;
>                         clocks = <&mmc0_clk>;
>                         clock-output-names = "mmcif0";
>                 };
> 
>                 ...
>         }
> 
> Devices would no longer use e.g. "clocks = <&mstp3_clks
> R8A7790_CLK_MMCIF1>", but "clocks = <&mmcif1_clk>".
> 
> 
> Questions/alternatives
> ----------------------
> 
>   - Should the subnode be called "mssr" (Module Standby and Software Reset)
>     instead of "mstp"?
>   - What with "renesas,*-cpg-clocks" clocks?
>       - Should we keep on using "#clock-cells = <1>" and <SOC>_CLK_<NAME>
>         indices, or switch to a less error-prone "#clock-cells = <0>"?
>   - What with "renesas,cpg-div6-clock" clocks?
>       - For now I kept them as a subnode of cpg_clocks, as their registers
>         are a subset of the CPG registers.

If you want to keep DT nodes for them I see no other location. The other 
option would be to merge them into the CPG node and add them to the list of 
clocks exposed by the CPG.

>       - This also means I had to keep the "#address-cells = <2>",
>         "#size-cells = <2>", and "ranges" properties under the CPG node.
>   - The MSTP number could be split in register and bit offset (and thus use
>     "#address-cells = <2>"), e.g.:
> 
>                 mmcif1_clk: clock@3,5 {
>                         reg = <3 5>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
> 
>         vs.
> 
>                 mmcif1_clk: clock@305 {
>                         reg = <305>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
> 
>     I don't think it's clearer, as the datasheet refers to e.g. "MSTP305".

You forgot the favourite topic of DT maintainers : how will this handle 
backward compatibility ?

> Thanks for your comments!
> 
> 
> Long example
> ------------
> 
> All r8a7791 clocks:
> 
>         /* External root clock */
>         extal_clk: extal_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 /* This value must be overriden by the board. */
>                 clock-frequency = <0>;
>                 clock-output-names = "extal";
>         };
> 
>         /*
>          * The external audio clocks are configured as 0 Hz fixed
> frequency clocks by
>          * default. Boards that provide audio clocks should override them.
>          */
>         audio_clk_a: audio_clk_a {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_a";
>         };
>         audio_clk_b: audio_clk_b {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_b";
>         };
>         audio_clk_c: audio_clk_c {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_c";
>         };
> 
>         /* External PCIe clock - can be overridden by the board */
>         pcie_bus_clk: pcie_bus_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <100000000>;
>                 clock-output-names = "pcie_bus";
>                 status = "disabled";
>         };
> 
>         /* External USB clock - can be overridden by the board */
>         usb_extal_clk: usb_extal_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <48000000>;
>                 clock-output-names = "usb_extal";
>         };
> 
>         /* External CAN clock */
>         can_clk: can_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 /* This value must be overridden by the board. */
>                 clock-frequency = <0>;
>                 clock-output-names = "can_clk";
>                 status = "disabled";
>         };
> 
>         /* Fixed factor clocks */
>         pll1_div2_clk: pll1_div2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "pll1_div2";
>         };
>         zg_clk: zg_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <3>;
>                 clock-mult = <1>;
>                 clock-output-names = "zg";
>         };
>         zx_clk: zx_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <3>;
>                 clock-mult = <1>;
>                 clock-output-names = "zx";
>         };
>         zs_clk: zs_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <6>;
>                 clock-mult = <1>;
>                 clock-output-names = "zs";
>         };
>         hp_clk: hp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <12>;
>                 clock-mult = <1>;
>                 clock-output-names = "hp";
>         };
>         i_clk: i_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "i";
>         };
>         b_clk: b_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <12>;
>                 clock-mult = <1>;
>                 clock-output-names = "b";
>         };
>         p_clk: p_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <24>;
>                 clock-mult = <1>;
>                 clock-output-names = "p";
>         };
>         cl_clk: cl_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <48>;
>                 clock-mult = <1>;
>                 clock-output-names = "cl";
>         };
>         m2_clk: m2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "m2";
>         };
>         imp_clk: imp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <4>;
>                 clock-mult = <1>;
>                 clock-output-names = "imp";
>         };
>         rclk_clk: rclk_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <(48 * 1024)>;
>                 clock-mult = <1>;
>                 clock-output-names = "rclk";
>         };
>         oscclk_clk: oscclk_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <(12 * 1024)>;
>                 clock-mult = <1>;
>                 clock-output-names = "oscclk";
>         };
>         zb3_clk: zb3_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <4>;
>                 clock-mult = <1>;
>                 clock-output-names = "zb3";
>         };
>         zb3d2_clk: zb3d2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "zb3d2";
>         };
>         ddr_clk: ddr_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "ddr";
>         };
>         mp_clk: mp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&pll1_div2_clk>;
>                 #clock-cells = <0>;
>                 clock-div = <15>;
>                 clock-mult = <1>;
>                 clock-output-names = "mp";
>         };
>         cp_clk: cp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&extal_clk>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "cp";
>         };
> 
>         /* Special CPG clocks */
>         cpg_clocks: cpg_clocks@e6150000 {
>                 #address-cells = <2>;
>                 #size-cells = <2>;
>                 ranges;
> 
>                 compatible = "renesas,r8a7791-cpg",
>                 reg = <0 0xe6150000 0 0x1000>;
>                 clocks = <&extal_clk &usb_extal_clk>;
>                 #clock-cells = <1>;
>                 clock-output-names = "main", "pll0", "pll1", "pll3",
>                                      "lb", "qspi", "sdh", "sd0", "z",
>                                      "rcan", "adsp";
>                 #power-domain-cells = <0>;
> 
>                 /* Variable factor clocks */
>                 sd2_clk: sd2_clk@e6150078 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150078 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "sd2";
>                 };
>                 sd3_clk: sd3_clk@e615026c {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe615026c 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "sd3";
>                 };
>                 mmc0_clk: mmc0_clk@e6150240 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150240 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "mmc0";
>                 };
>                 ssp_clk: ssp_clk@e6150248 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150248 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "ssp";
>                 };
>                 ssprs_clk: ssprs_clk@e615024c {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe615024c 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "ssprs";
>                 };
> 
>                 /* Module Standby and Software Reset */
>                 mstp {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         /* MSTP0 */
>                         msiof0_clk: clock@000 {
>                                 reg = <000>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof0";
>                         };
> 
>                         /* MSTP1 */
>                         vcp0_clk: clock@101 {
>                                 reg = <101>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vcp0";
>                         };
>                         vpc0_clk: clock@103 {
>                                 reg = <103>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vpc0";
>                         };
>                         jpu_clk: clock@106 {
>                                 reg = <106>;
>                                 clocks = <&m2_clk>;
>                                 clock-output-names = "jpu";
>                         };
>                         ssp1_clk: clock@109 {
>                                 reg = <109>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "ssp1";
>                         };
>                         tmu1_clk: clock@111 {
>                                 reg = <111>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu1";
>                         };
>                         threedg_clk: clock@112 {
>                                 reg = <112>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "3dg";
>                         };
>                         twoddmac_clk: clock@115 {
>                                 reg = <115>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "2ddmac";
>                         };
>                         fdp1_1_clk: clock@118 {
>                                 reg = <118>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "fdp1-1";
>                         };
>                         fdp1_0_clk: clock@119 {
>                                 reg = <119>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "fdp1-0";
>                         };
>                         tmu3_clk: clock@121 {
>                                 reg = <121>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu3";
>                         };
>                         tmu2_clk: clock@122 {
>                                 reg = <122>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu2";
>                         };
>                         cmt0_clk: clock@124 {
>                                 reg = <124>;
>                                 clocks = <&rclk_clk>;
>                                 clock-output-names = "cmt0";
>                         };
>                         tmu0_clk: clock@125 {
>                                 reg = <125>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "tmu0";
>                         };
>                         vsp1_du1_clk: clock@127 {
>                                 reg = <127>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-du1";
>                         };
>                         vsp1_du0_clk: clock@128 {
>                                 reg = <128>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-du0";
>                         };
>                         vsp1_sy_clk: clock@131 {
>                                 reg = <131>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-sy";
>                         };
> 
>                         /* MSTP2 */
>                         scifa2_clk: clock@202 {
>                                 reg = <202>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa2";
>                         };
>                         scifa1_clk: clock@203 {
>                                 reg = <203>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa1";
>                         };
>                         scifa0_clk: clock@204 {
>                                 reg = <204>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa0";
>                         };
>                         msiof2_clk: clock@205 {
>                                 reg = <205>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof2";
>                         };
>                         scifb0_clk: clock@206 {
>                                 reg = <206>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb0";
>                         };
>                         scifb1_clk: clock@207 {
>                                 reg = <207>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb1";
>                         };
>                         msiof1_clk: clock@208 {
>                                 reg = <208>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof1";
>                         };
>                         scifb2_clk: clock@216 {
>                                 reg = <216>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb2";
>                         };
>                         sys_dmac1_clk: clock@218 {
>                                 reg = <218>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sys-dmac1";
>                         };
>                         sys_dmac0_clk: clock@219 {
>                                 reg = <219>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sys-dmac0";
>                         };
> 
>                         /* MSTP3 */
>                         tpu0_clk: clock@304 {
>                                 reg = <304>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "tpu0";
>                         };
>                         sdhi2_clk: clock@311 {
>                                 reg = <311>;
>                                 clocks = <&sd3_clk>;
>                                 clock-output-names = "sdhi2";
>                         };
>                         sdhi1_clk: clock@312 {
>                                 reg = <312>;
>                                 clocks = <&sd2_clk>;
>                                 clock-output-names = "sdhi1";
>                         };
>                         sdhi0_clk: clock@314 {
>                                 reg = <314>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_SD0>;
>                                 clock-output-names = "sdhi0";
>                         };
>                         mmcif0_clk: clock@315 {
>                                 reg = <315>;
>                                 clocks = <&mmc0_clk>;
>                                 clock-output-names = "mmcif0";
>                         };
>                         i2c7_clk: clock@318 {
>                                 reg = <318>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c7";
>                         };
>                         pciec_clk: clock@319 {
>                                 reg = <319>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "pciec";
>                         };
>                         i2c8_clk: clock@323 {
>                                 reg = <323>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c8";
>                         };
>                         ssusb_clk: clock@328 {
>                                 reg = <328>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "ssusb";
>                         };
>                         cmt1_clk: clock@329 {
>                                 reg = <329>;
>                                 clocks = <&rclk_clk>;
>                                 clock-output-names = "cmt1";
>                         };
>                         usbdmac0_clk: clock@330 {
>                                 reg = <330>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "usbdmac0";
>                         };
>                         usbdmac1_clk: clock@331 {
>                                 reg = <331>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "usbdmac1";
>                         };
> 
>                         /* MSTP4 */
>                         irqc_clk: clock@407 {
>                                 reg = <407>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "irqc";
>                         };
>                         intc_sys_clk: clock@408 {
>                                 reg = <408>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "intc-sys";
>                         };
> 
>                         /* MSTP5 */
>                         audmac1_clk: clock@501 {
>                                 reg = <501>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "audmac1";
>                         };
>                         audmac0_clk: clock@502 {
>                                 reg = <502>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "audmac0";
>                         };
>                         adsp_mod_clk: clock@506 {
>                                 reg = <506>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_ADSP>;
>                                 clock-output-names = "adsp_mod";
>                         };
>                         thermal_clk: clock@522 {
>                                 reg = <522>;
>                                 clocks = <&extal_clk>;
>                                 clock-output-names = "thermal";
>                         };
>                         pwm_clk: clock@523 {
>                                 reg = <523>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "pwm";
>                         };
> 
>                         /* MSTP7 */
>                         ehci_clk: clock@703 {
>                                 reg = <703>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "ehci";
>                         };
>                         hsusb_clk: clock@704 {
>                                 reg = <704>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "hsusb";
>                         };
>                         hscif2_clk: clock@713 {
>                                 reg = <713>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif2";
>                         };
>                         scif5_clk: clock@714 {
>                                 reg = <714>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif5";
>                         };
>                         scif4_clk: clock@715 {
>                                 reg = <715>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif4";
>                         };
>                         hscif1_clk: clock@716 {
>                                 reg = <716>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif1";
>                         };
>                         hscif0_clk: clock@717 {
>                                 reg = <717>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif0";
>                         };
>                         scif3_clk: clock@718 {
>                                 reg = <718>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif3";
>                         };
>                         scif2_clk: clock@719 {
>                                 reg = <719>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif2";
>                         };
>                         scif1_clk: clock@720 {
>                                 reg = <720>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif1";
>                         };
>                         scif0_clk: clock@721 {
>                                 reg = <721>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif0";
>                         };
>                         du1_clk: clock@723 {
>                                 reg = <723>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "du1";
>                         };
>                         du0_clk: clock@724 {
>                                 reg = <724>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "du0";
>                         };
>                         lvds0_clk: clock@726 {
>                                 reg = <726>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "lvds0";
>                         };
> 
>                         /* MSTP8 */
>                         ipmmu_sgx_clk: clock@800 {
>                                 reg = <800>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "ipmmu_sgx";
>                         };
>                         mlb_clk: clock@802 {
>                                 reg = <802>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "mlb";
>                         };
>                         vin2_clk: clock@809 {
>                                 reg = <809>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin2";
>                         };
>                         vin1_clk: clock@810 {
>                                 reg = <810>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin1";
>                         };
>                         vin0_clk: clock@811 {
>                                 reg = <811>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin0";
>                         };
>                         ether_clk: clock@813 {
>                                 reg = <813>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ether";
>                         };
>                         sata1_clk: clock@814 {
>                                 reg = <814>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sata1";
>                         };
>                         sata0_clk: clock@815 {
>                                 reg = <815>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sata0";
>                         };
> 
>                         /* MSTP9 */
>                         gpio7_clk: clock@904 {
>                                 reg = <904>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio7";
>                         };
>                         gpio6_clk: clock@905 {
>                                 reg = <905>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio6";
>                         };
>                         gpio5_clk: clock@907 {
>                                 reg = <907>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio5";
>                         };
>                         gpio4_clk: clock@908 {
>                                 reg = <908>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio4";
>                         };
>                         gpio3_clk: clock@909 {
>                                 reg = <909>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio3";
>                         };
>                         gpio2_clk: clock@910 {
>                                 reg = <910>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio2";
>                         };
>                         gpio1_clk: clock@911 {
>                                 reg = <911>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio1";
>                         };
>                         gpio0_clk: clock@912 {
>                                 reg = <912>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio0";
>                         };
>                         rcan1_clk: clock@915 {
>                                 reg = <915>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "rcan1";
>                         };
>                         rcan0_clk: clock@916 {
>                                 reg = <916>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "rcan0";
>                         };
>                         qspi_mod_clk: clock@917 {
>                                 reg = <917>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_QSPI>;
>                                 clock-output-names = "qspi_mod";
>                         };
>                         i2c5_clk: clock@925 {
>                                 reg = <925>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c5";
>                         };
>                         i2c6_clk: clock@926 {
>                                 reg = <926>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "i2c6";
>                         };
>                         i2c4_clk: clock@927 {
>                                 reg = <927>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c4";
>                         };
>                         i2c3_clk: clock@928 {
>                                 reg = <928>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c3";
>                         };
>                         i2c2_clk: clock@929 {
>                                 reg = <929>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c2";
>                         };
>                         i2c1_clk: clock@930 {
>                                 reg = <930>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c1";
>                         };
>                         i2c0_clk: clock@931 {
>                                 reg = <931>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c0";
>                         };
> 
>                         /* MSTP10 */
>                         ssi_all_clk: clock@1005 {
>                                 reg = <1005>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi-all";
>                         };
>                         ssi9_clk: clock@1006 {
>                                 reg = <1006>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi9";
>                         };
>                         ssi8_clk: clock@1007 {
>                                 reg = <1007>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi8";
>                         };
>                         ssi7_clk: clock@1008 {
>                                 reg = <1008>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi7";
>                         };
>                         ssi6_clk: clock@1009 {
>                                 reg = <1009>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi6";
>                         };
>                         ssi5_clk: clock@1010 {
>                                 reg = <1010>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi5";
>                         };
>                         ssi4_clk: clock@1011 {
>                                 reg = <1011>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi4";
>                         };
>                         ssi3_clk: clock@1012 {
>                                 reg = <1012>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi3";
>                         };
>                         ssi2_clk: clock@1013 {
>                                 reg = <1013>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi2";
>                         };
>                         ssi1_clk: clock@1014 {
>                                 reg = <1014>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi1";
>                         };
>                         ssi0_clk: clock@1015 {
>                                 reg = <1015>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi0";
>                         };
>                         scu_all_clk: clock@1017 {
>                                 reg = <1017>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scu-all";
>                         };
>                         scu_dvc1_clk: clock@1018 {
>                                 reg = <1018>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-dvc1";
>                         };
>                         scu_dvc0_clk: clock@1019 {
>                                 reg = <1019>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-dvc0";
>                         };
>                         scu_src9_clk: clock@1022 {
>                                 reg = <1022>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src9";
>                         };
>                         scu_src8_clk: clock@1023 {
>                                 reg = <1023>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src8";
>                         };
>                         scu_src7_clk: clock@1024 {
>                                 reg = <1024>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src7";
>                         };
>                         scu_src6_clk: clock@1025 {
>                                 reg = <1025>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src6";
>                         };
>                         scu_src5_clk: clock@1026 {
>                                 reg = <1026>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src5";
>                         };
>                         scu_src4_clk: clock@1027 {
>                                 reg = <1027>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src4";
>                         };
>                         scu_src3_clk: clock@1028 {
>                                 reg = <1028>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src3";
>                         };
>                         scu_src2_clk: clock@1029 {
>                                 reg = <1029>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src2";
>                         };
>                         scu_src1_clk: clock@1030 {
>                                 reg = <1030>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src1";
>                         };
>                         scu_src0_clk: clock@1031 {
>                                 reg = <1031>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src0";
>                         };
> 
>                         /* MSTP11 */
>                         scifa3_clk: clock@1106 {
>                                 reg = <1106>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa3";
>                         };
>                         scifa4_clk: clock@1107 {
>                                 reg = <1107>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa4";
>                         };
>                         scifa5_clk: clock@1108 {
>                                 reg = <1108>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa5";
>                         };
>                 };
>         };
> 
> 
> References
> ----------
> 
>   [1] Documentation/devicetree/bindings/clock/renesas,<SoC>-cpg-clocks.txt
>       where <SoC> is a single SoC name or a family of SoCs
>   [2] Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
>   [3] Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
>   [4] Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
>   [5] arch/arm/boot/dts/{sh7,r7,r8}*.dtsi
>   [6] Series "[PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains"
>       (https://lkml.org/lkml/2015/5/28/590,
>        http://lkml.iu.edu/hypermail/linux/kernel/1505.3/03336.html)
>   [7] Series "[PATCH 0/6][RFC] arm64: Renesas Gen3 initial patch"
>       (http://www.spinics.net/lists/linux-sh/msg42683.html)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Renesas CPG/MSTP DT Binding Proposal
@ 2015-06-22 20:36   ` Laurent Pinchart
  0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2015-06-22 20:36 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd, linux-clk, Linux PM list, devicetree,
	Linux-sh list

Hi Geert,

On Monday 22 June 2015 21:49:23 Geert Uytterhoeven wrote:
> Introduction
> ------------
> 
> On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
> MSTP (Module Standby and Software Reset), and APMU (Advanced Power
> Management Unit for AP-System Core) blocks are very intimately tied.
> 
>   - The CPG clock generates various clocks for LSI internal generation,
>   - The MSTP block provides two functions:
>       1. Module Standby: "Clock supply to specified modules is stopped by
>          setting the module stop control register bits."
>          However, the clock supply to a module is not stopped until all CPUs
>          in the SoC agree.  Indeed, there are separate MSTP registers for
>          application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
>       2. Reset Control. to perform a software reset of a specific module.
>   - The APMU controls power and clock supply to the AP-system core (e.g.
>   CA7, CA15, SCU).
> 
> MSTP and CPG registers are mixed in one register block.
> The APMU has one or two separate register blocks.
> 
> The current DT bindings are split across 4 different binding documents:
>   1. Renesas SoC-specific "core" CPG clocks that cannot be represented
> easily in DT in another way (cfr. [1]),
>   2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
>   3. Renesas-specific "div6" variable factor clocks, also generated by the
> CPG (cfr. [3]).
>   3. Generic "fixed-factor-clock" clocks, generated by the CPG, but
> represented in DT using the generic ""fixed-factor-clock" bindings (cfr.
> [4]),
> 
> Note that currently the Reset Control function of the MSTP block is not
> supported by DT nor Linux, and that there are no DT bindings for the APMU
> block yet.
> 
> 
> Issues with current bindings
> ----------------------------
> 
> General:
>   - Tight coupling of CPG and MSTP is not represented in DT:
>       - There's one CPG node, and separate MSTP nodes (one for each block of
>         up to 32 possible MSTP clocks) next to it,
>       - CPG Clock Domain requires "power-domain = <...>" properties linking
>         individual device nodes to the "CPG/MSTP" clock domain provider.
>         For now I use a link to the (single) CPG device node, while the
>         clock provider (mostly) controls the (multiple) MSTP clocks (cfr.
>         [6]).
>       - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
>         requires writing a special value to a specific "Write Protect
>         Register" inside the CPG first. Exact details and impact are still
>         under investigation (cfr. [7]).
> 
> MSTP-specific:
>   - The clock hierarchy (parent-child relationship) is represented in a flat
>     structure,

What do you mean exactly here ?

>   - Multiple arrays have to be kept in sync:
>       - "clocks" (parents),
>       - "clock-indices" (sparse bit index inside register),
>       - "clock-output-names",
>   - MSTP clocks are referred to using a reference to a device node and a bit
>     index:
>        - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h
>          are separated from the .dtsi file,
>        - There's no protection against using a bit index definition for a
>          different MSTP register, e.g.
>            - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
>            - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),

If this concerns you, we could add the MSTP number to the macro name, making 
it easier to catch such issues.

>    - The "reg" properties contain only the SMSTPCRx (control register for
>      System CPU) and MSTPSRx (optional status register) registers only.
>      However, in reality there are many more registers:
>        - Separate control registers for application (Cortex-A, supported)
>          and real-time (SH and/or Cortex-R, not supported) cores,
>        - Software reset registers (not supported).
>      To make matters more complicated:
>        - Many registers are optional.
>          The current bindings just support one control register and one
>          optional status register, but this doesn't scale to more register
>          sets,

We could just use named registers for that.

>        - Register sets are not linear and not contiguous in the register
>          space. I.e. it's not possible to derive the address of SMSTPCRx
>          from the address of SMSTPCRy, or the address of MSTPSRx from the
>          address of SMSTPCRx.

When the hardware is to be blamed we seem to all agree easily :-)

> Proposal
> --------
> 
> I'd like to propose the following, to resolve (most of) the issues with the
> current bindings:
> 
>   - Get rid of the "clocks" node.
>     IIRC grouping all clocks under a "clock" node was deprecated, but I
>     can't find the email anymore?
>       - "fixed-clock" clocks are now at the root node level,

I wonder whether we should move all clocks to a separate .dtsi, they take 
quite a bit of space.

>       - The CPG node is now at the root node level,

The CPG node should actually be a child of a bus, not a child of the root 
node. That's a separate issue though.

>   - CPG node:
>       - Use e.g. "renesas,r8a7791-cpg" as compatible value,
>   - MSTP clocks:
>       - All MSTP clocks are now grouped inside the CPG node, under a new
>         "mstp" subnode, without compatible values,

We've discussed this several times in the past and agreed, the MSTP nodes 
should be subnodes of the CPG node.

>       - Each individual MSTP clock has its own subnode, referring to the
>         MSTP clock number, instead of using one node per register, and
>         arrays of "clocks", "clock-output-names", and "clock-indices"
>         properties.

That I don't really like. It would result in more than a hundred of clock 
nodes. I have discussed this option with Mike when designing the clock DT 
bindings and he didn't like it either.

>       - The list of available registers is maintained as arrays inside the
>         unified CPG/MSTP driver, e.g.
> 
>         struct cpg {
>                 void __iomem *base;
>                 unsigned int n_mstp;    /* Size of a register set */
>                 /*
>                  * Optional arrays of register offsets for the various
>                  * register sets:
>                  *   - Array pointer may be NULL, if not supported */
>                  *   - Array element may be ~0, if not supported */
>                  */
>                 u16 *smstp;             /* System CPU control registers */
>                 u16 *rmstp;             /* Realtime CPU control registers */
>                 u16 *mstpsr;            /* Status registers */
>                 u16 *srcr;              /* Reset registers */
>                 ...

I'd create a struct to group smstp, rmstp, mstpsr, srcr & co and have a single 
array of those structures.

>         };
> 
> 
> Short example
> -------------
> 
> (from Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt)
> 
> For the MSTP part:
> 
>         #include <dt-bindings/clock/r8a7790-clock.h>
> 
>         mstp3_clks: mstp3_clks@e615013c {
>                 compatible = "renesas,r8a7790-mstp-clocks",
>                              "renesas,cpg-mstp-clocks";
>                 reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
>                 clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
>                          <&cpg_clocks R8A7790_CLK_SD1>,
>                          <&cpg_clocks R8A7790_CLK_SD0>, <&mmc0_clk>;
>                 #clock-cells = <1>;
>                 clock-output-names =
>                         "tpu0", "mmcif1", "sdhi3", "sdhi2",
>                          "sdhi1", "sdhi0", "mmcif0";
>                 clock-indices = <
>                         R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1
> R8A7790_CLK_SDHI3 R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
> R8A7790_CLK_MMCIF0
> 
>                 >;
> 
>         };
> 
> would become:
> 
>         mstp {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 ...
> 
>                 /* MSTP3 */
>                 tpu0_clk: clock@304 {
>                         reg = <304>;
>                         clocks = <&cp_clk>;
>                         clock-output-names = "tpu0";
>                 };
>                 mmcif1_clk: clock@305 {
>                         reg = <305>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
>                 sdhi3_clk: clock@311 {
>                         reg = <311>;
>                         clocks = <&sd3_clk>;
>                         clock-output-names = "sdhi3";
>                 };
>                 sdhi2_clk: clock@312 {
>                         reg = <312>;
>                         clocks = <&sd2_clk>;
>                         clock-output-names = "sdhi2";
>                 };
>                 sdhi1_clk: clock@313 {
>                         reg = <313>;
>                         clocks = <&cpg_clocks R8A7790_CLK_SD1>;
>                         clock-output-names = "sdhi1";
>                 };
>                 sdhi0_clk: clock@314 {
>                         reg = <314>;
>                         clocks = <&cpg_clocks R8A7790_CLK_SD0>;
>                         clock-output-names = "sdhi0";
>                 };
>                 mmcif0_clk: clock@315 {
>                         reg = <315>;
>                         clocks = <&mmc0_clk>;
>                         clock-output-names = "mmcif0";
>                 };
> 
>                 ...
>         }
> 
> Devices would no longer use e.g. "clocks = <&mstp3_clks
> R8A7790_CLK_MMCIF1>", but "clocks = <&mmcif1_clk>".
> 
> 
> Questions/alternatives
> ----------------------
> 
>   - Should the subnode be called "mssr" (Module Standby and Software Reset)
>     instead of "mstp"?
>   - What with "renesas,*-cpg-clocks" clocks?
>       - Should we keep on using "#clock-cells = <1>" and <SOC>_CLK_<NAME>
>         indices, or switch to a less error-prone "#clock-cells = <0>"?
>   - What with "renesas,cpg-div6-clock" clocks?
>       - For now I kept them as a subnode of cpg_clocks, as their registers
>         are a subset of the CPG registers.

If you want to keep DT nodes for them I see no other location. The other 
option would be to merge them into the CPG node and add them to the list of 
clocks exposed by the CPG.

>       - This also means I had to keep the "#address-cells = <2>",
>         "#size-cells = <2>", and "ranges" properties under the CPG node.
>   - The MSTP number could be split in register and bit offset (and thus use
>     "#address-cells = <2>"), e.g.:
> 
>                 mmcif1_clk: clock@3,5 {
>                         reg = <3 5>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
> 
>         vs.
> 
>                 mmcif1_clk: clock@305 {
>                         reg = <305>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
> 
>     I don't think it's clearer, as the datasheet refers to e.g. "MSTP305".

You forgot the favourite topic of DT maintainers : how will this handle 
backward compatibility ?

> Thanks for your comments!
> 
> 
> Long example
> ------------
> 
> All r8a7791 clocks:
> 
>         /* External root clock */
>         extal_clk: extal_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 /* This value must be overriden by the board. */
>                 clock-frequency = <0>;
>                 clock-output-names = "extal";
>         };
> 
>         /*
>          * The external audio clocks are configured as 0 Hz fixed
> frequency clocks by
>          * default. Boards that provide audio clocks should override them.
>          */
>         audio_clk_a: audio_clk_a {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_a";
>         };
>         audio_clk_b: audio_clk_b {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_b";
>         };
>         audio_clk_c: audio_clk_c {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_c";
>         };
> 
>         /* External PCIe clock - can be overridden by the board */
>         pcie_bus_clk: pcie_bus_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <100000000>;
>                 clock-output-names = "pcie_bus";
>                 status = "disabled";
>         };
> 
>         /* External USB clock - can be overridden by the board */
>         usb_extal_clk: usb_extal_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <48000000>;
>                 clock-output-names = "usb_extal";
>         };
> 
>         /* External CAN clock */
>         can_clk: can_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 /* This value must be overridden by the board. */
>                 clock-frequency = <0>;
>                 clock-output-names = "can_clk";
>                 status = "disabled";
>         };
> 
>         /* Fixed factor clocks */
>         pll1_div2_clk: pll1_div2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "pll1_div2";
>         };
>         zg_clk: zg_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <3>;
>                 clock-mult = <1>;
>                 clock-output-names = "zg";
>         };
>         zx_clk: zx_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <3>;
>                 clock-mult = <1>;
>                 clock-output-names = "zx";
>         };
>         zs_clk: zs_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <6>;
>                 clock-mult = <1>;
>                 clock-output-names = "zs";
>         };
>         hp_clk: hp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <12>;
>                 clock-mult = <1>;
>                 clock-output-names = "hp";
>         };
>         i_clk: i_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "i";
>         };
>         b_clk: b_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <12>;
>                 clock-mult = <1>;
>                 clock-output-names = "b";
>         };
>         p_clk: p_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <24>;
>                 clock-mult = <1>;
>                 clock-output-names = "p";
>         };
>         cl_clk: cl_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <48>;
>                 clock-mult = <1>;
>                 clock-output-names = "cl";
>         };
>         m2_clk: m2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "m2";
>         };
>         imp_clk: imp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <4>;
>                 clock-mult = <1>;
>                 clock-output-names = "imp";
>         };
>         rclk_clk: rclk_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <(48 * 1024)>;
>                 clock-mult = <1>;
>                 clock-output-names = "rclk";
>         };
>         oscclk_clk: oscclk_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <(12 * 1024)>;
>                 clock-mult = <1>;
>                 clock-output-names = "oscclk";
>         };
>         zb3_clk: zb3_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <4>;
>                 clock-mult = <1>;
>                 clock-output-names = "zb3";
>         };
>         zb3d2_clk: zb3d2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "zb3d2";
>         };
>         ddr_clk: ddr_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "ddr";
>         };
>         mp_clk: mp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&pll1_div2_clk>;
>                 #clock-cells = <0>;
>                 clock-div = <15>;
>                 clock-mult = <1>;
>                 clock-output-names = "mp";
>         };
>         cp_clk: cp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&extal_clk>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "cp";
>         };
> 
>         /* Special CPG clocks */
>         cpg_clocks: cpg_clocks@e6150000 {
>                 #address-cells = <2>;
>                 #size-cells = <2>;
>                 ranges;
> 
>                 compatible = "renesas,r8a7791-cpg",
>                 reg = <0 0xe6150000 0 0x1000>;
>                 clocks = <&extal_clk &usb_extal_clk>;
>                 #clock-cells = <1>;
>                 clock-output-names = "main", "pll0", "pll1", "pll3",
>                                      "lb", "qspi", "sdh", "sd0", "z",
>                                      "rcan", "adsp";
>                 #power-domain-cells = <0>;
> 
>                 /* Variable factor clocks */
>                 sd2_clk: sd2_clk@e6150078 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150078 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "sd2";
>                 };
>                 sd3_clk: sd3_clk@e615026c {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe615026c 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "sd3";
>                 };
>                 mmc0_clk: mmc0_clk@e6150240 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150240 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "mmc0";
>                 };
>                 ssp_clk: ssp_clk@e6150248 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150248 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "ssp";
>                 };
>                 ssprs_clk: ssprs_clk@e615024c {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe615024c 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "ssprs";
>                 };
> 
>                 /* Module Standby and Software Reset */
>                 mstp {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         /* MSTP0 */
>                         msiof0_clk: clock@000 {
>                                 reg = <000>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof0";
>                         };
> 
>                         /* MSTP1 */
>                         vcp0_clk: clock@101 {
>                                 reg = <101>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vcp0";
>                         };
>                         vpc0_clk: clock@103 {
>                                 reg = <103>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vpc0";
>                         };
>                         jpu_clk: clock@106 {
>                                 reg = <106>;
>                                 clocks = <&m2_clk>;
>                                 clock-output-names = "jpu";
>                         };
>                         ssp1_clk: clock@109 {
>                                 reg = <109>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "ssp1";
>                         };
>                         tmu1_clk: clock@111 {
>                                 reg = <111>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu1";
>                         };
>                         threedg_clk: clock@112 {
>                                 reg = <112>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "3dg";
>                         };
>                         twoddmac_clk: clock@115 {
>                                 reg = <115>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "2ddmac";
>                         };
>                         fdp1_1_clk: clock@118 {
>                                 reg = <118>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "fdp1-1";
>                         };
>                         fdp1_0_clk: clock@119 {
>                                 reg = <119>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "fdp1-0";
>                         };
>                         tmu3_clk: clock@121 {
>                                 reg = <121>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu3";
>                         };
>                         tmu2_clk: clock@122 {
>                                 reg = <122>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu2";
>                         };
>                         cmt0_clk: clock@124 {
>                                 reg = <124>;
>                                 clocks = <&rclk_clk>;
>                                 clock-output-names = "cmt0";
>                         };
>                         tmu0_clk: clock@125 {
>                                 reg = <125>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "tmu0";
>                         };
>                         vsp1_du1_clk: clock@127 {
>                                 reg = <127>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-du1";
>                         };
>                         vsp1_du0_clk: clock@128 {
>                                 reg = <128>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-du0";
>                         };
>                         vsp1_sy_clk: clock@131 {
>                                 reg = <131>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-sy";
>                         };
> 
>                         /* MSTP2 */
>                         scifa2_clk: clock@202 {
>                                 reg = <202>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa2";
>                         };
>                         scifa1_clk: clock@203 {
>                                 reg = <203>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa1";
>                         };
>                         scifa0_clk: clock@204 {
>                                 reg = <204>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa0";
>                         };
>                         msiof2_clk: clock@205 {
>                                 reg = <205>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof2";
>                         };
>                         scifb0_clk: clock@206 {
>                                 reg = <206>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb0";
>                         };
>                         scifb1_clk: clock@207 {
>                                 reg = <207>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb1";
>                         };
>                         msiof1_clk: clock@208 {
>                                 reg = <208>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof1";
>                         };
>                         scifb2_clk: clock@216 {
>                                 reg = <216>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb2";
>                         };
>                         sys_dmac1_clk: clock@218 {
>                                 reg = <218>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sys-dmac1";
>                         };
>                         sys_dmac0_clk: clock@219 {
>                                 reg = <219>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sys-dmac0";
>                         };
> 
>                         /* MSTP3 */
>                         tpu0_clk: clock@304 {
>                                 reg = <304>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "tpu0";
>                         };
>                         sdhi2_clk: clock@311 {
>                                 reg = <311>;
>                                 clocks = <&sd3_clk>;
>                                 clock-output-names = "sdhi2";
>                         };
>                         sdhi1_clk: clock@312 {
>                                 reg = <312>;
>                                 clocks = <&sd2_clk>;
>                                 clock-output-names = "sdhi1";
>                         };
>                         sdhi0_clk: clock@314 {
>                                 reg = <314>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_SD0>;
>                                 clock-output-names = "sdhi0";
>                         };
>                         mmcif0_clk: clock@315 {
>                                 reg = <315>;
>                                 clocks = <&mmc0_clk>;
>                                 clock-output-names = "mmcif0";
>                         };
>                         i2c7_clk: clock@318 {
>                                 reg = <318>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c7";
>                         };
>                         pciec_clk: clock@319 {
>                                 reg = <319>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "pciec";
>                         };
>                         i2c8_clk: clock@323 {
>                                 reg = <323>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c8";
>                         };
>                         ssusb_clk: clock@328 {
>                                 reg = <328>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "ssusb";
>                         };
>                         cmt1_clk: clock@329 {
>                                 reg = <329>;
>                                 clocks = <&rclk_clk>;
>                                 clock-output-names = "cmt1";
>                         };
>                         usbdmac0_clk: clock@330 {
>                                 reg = <330>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "usbdmac0";
>                         };
>                         usbdmac1_clk: clock@331 {
>                                 reg = <331>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "usbdmac1";
>                         };
> 
>                         /* MSTP4 */
>                         irqc_clk: clock@407 {
>                                 reg = <407>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "irqc";
>                         };
>                         intc_sys_clk: clock@408 {
>                                 reg = <408>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "intc-sys";
>                         };
> 
>                         /* MSTP5 */
>                         audmac1_clk: clock@501 {
>                                 reg = <501>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "audmac1";
>                         };
>                         audmac0_clk: clock@502 {
>                                 reg = <502>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "audmac0";
>                         };
>                         adsp_mod_clk: clock@506 {
>                                 reg = <506>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_ADSP>;
>                                 clock-output-names = "adsp_mod";
>                         };
>                         thermal_clk: clock@522 {
>                                 reg = <522>;
>                                 clocks = <&extal_clk>;
>                                 clock-output-names = "thermal";
>                         };
>                         pwm_clk: clock@523 {
>                                 reg = <523>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "pwm";
>                         };
> 
>                         /* MSTP7 */
>                         ehci_clk: clock@703 {
>                                 reg = <703>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "ehci";
>                         };
>                         hsusb_clk: clock@704 {
>                                 reg = <704>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "hsusb";
>                         };
>                         hscif2_clk: clock@713 {
>                                 reg = <713>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif2";
>                         };
>                         scif5_clk: clock@714 {
>                                 reg = <714>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif5";
>                         };
>                         scif4_clk: clock@715 {
>                                 reg = <715>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif4";
>                         };
>                         hscif1_clk: clock@716 {
>                                 reg = <716>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif1";
>                         };
>                         hscif0_clk: clock@717 {
>                                 reg = <717>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif0";
>                         };
>                         scif3_clk: clock@718 {
>                                 reg = <718>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif3";
>                         };
>                         scif2_clk: clock@719 {
>                                 reg = <719>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif2";
>                         };
>                         scif1_clk: clock@720 {
>                                 reg = <720>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif1";
>                         };
>                         scif0_clk: clock@721 {
>                                 reg = <721>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif0";
>                         };
>                         du1_clk: clock@723 {
>                                 reg = <723>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "du1";
>                         };
>                         du0_clk: clock@724 {
>                                 reg = <724>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "du0";
>                         };
>                         lvds0_clk: clock@726 {
>                                 reg = <726>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "lvds0";
>                         };
> 
>                         /* MSTP8 */
>                         ipmmu_sgx_clk: clock@800 {
>                                 reg = <800>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "ipmmu_sgx";
>                         };
>                         mlb_clk: clock@802 {
>                                 reg = <802>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "mlb";
>                         };
>                         vin2_clk: clock@809 {
>                                 reg = <809>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin2";
>                         };
>                         vin1_clk: clock@810 {
>                                 reg = <810>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin1";
>                         };
>                         vin0_clk: clock@811 {
>                                 reg = <811>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin0";
>                         };
>                         ether_clk: clock@813 {
>                                 reg = <813>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ether";
>                         };
>                         sata1_clk: clock@814 {
>                                 reg = <814>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sata1";
>                         };
>                         sata0_clk: clock@815 {
>                                 reg = <815>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sata0";
>                         };
> 
>                         /* MSTP9 */
>                         gpio7_clk: clock@904 {
>                                 reg = <904>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio7";
>                         };
>                         gpio6_clk: clock@905 {
>                                 reg = <905>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio6";
>                         };
>                         gpio5_clk: clock@907 {
>                                 reg = <907>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio5";
>                         };
>                         gpio4_clk: clock@908 {
>                                 reg = <908>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio4";
>                         };
>                         gpio3_clk: clock@909 {
>                                 reg = <909>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio3";
>                         };
>                         gpio2_clk: clock@910 {
>                                 reg = <910>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio2";
>                         };
>                         gpio1_clk: clock@911 {
>                                 reg = <911>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio1";
>                         };
>                         gpio0_clk: clock@912 {
>                                 reg = <912>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio0";
>                         };
>                         rcan1_clk: clock@915 {
>                                 reg = <915>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "rcan1";
>                         };
>                         rcan0_clk: clock@916 {
>                                 reg = <916>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "rcan0";
>                         };
>                         qspi_mod_clk: clock@917 {
>                                 reg = <917>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_QSPI>;
>                                 clock-output-names = "qspi_mod";
>                         };
>                         i2c5_clk: clock@925 {
>                                 reg = <925>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c5";
>                         };
>                         i2c6_clk: clock@926 {
>                                 reg = <926>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "i2c6";
>                         };
>                         i2c4_clk: clock@927 {
>                                 reg = <927>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c4";
>                         };
>                         i2c3_clk: clock@928 {
>                                 reg = <928>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c3";
>                         };
>                         i2c2_clk: clock@929 {
>                                 reg = <929>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c2";
>                         };
>                         i2c1_clk: clock@930 {
>                                 reg = <930>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c1";
>                         };
>                         i2c0_clk: clock@931 {
>                                 reg = <931>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c0";
>                         };
> 
>                         /* MSTP10 */
>                         ssi_all_clk: clock@1005 {
>                                 reg = <1005>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi-all";
>                         };
>                         ssi9_clk: clock@1006 {
>                                 reg = <1006>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi9";
>                         };
>                         ssi8_clk: clock@1007 {
>                                 reg = <1007>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi8";
>                         };
>                         ssi7_clk: clock@1008 {
>                                 reg = <1008>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi7";
>                         };
>                         ssi6_clk: clock@1009 {
>                                 reg = <1009>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi6";
>                         };
>                         ssi5_clk: clock@1010 {
>                                 reg = <1010>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi5";
>                         };
>                         ssi4_clk: clock@1011 {
>                                 reg = <1011>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi4";
>                         };
>                         ssi3_clk: clock@1012 {
>                                 reg = <1012>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi3";
>                         };
>                         ssi2_clk: clock@1013 {
>                                 reg = <1013>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi2";
>                         };
>                         ssi1_clk: clock@1014 {
>                                 reg = <1014>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi1";
>                         };
>                         ssi0_clk: clock@1015 {
>                                 reg = <1015>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi0";
>                         };
>                         scu_all_clk: clock@1017 {
>                                 reg = <1017>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scu-all";
>                         };
>                         scu_dvc1_clk: clock@1018 {
>                                 reg = <1018>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-dvc1";
>                         };
>                         scu_dvc0_clk: clock@1019 {
>                                 reg = <1019>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-dvc0";
>                         };
>                         scu_src9_clk: clock@1022 {
>                                 reg = <1022>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src9";
>                         };
>                         scu_src8_clk: clock@1023 {
>                                 reg = <1023>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src8";
>                         };
>                         scu_src7_clk: clock@1024 {
>                                 reg = <1024>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src7";
>                         };
>                         scu_src6_clk: clock@1025 {
>                                 reg = <1025>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src6";
>                         };
>                         scu_src5_clk: clock@1026 {
>                                 reg = <1026>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src5";
>                         };
>                         scu_src4_clk: clock@1027 {
>                                 reg = <1027>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src4";
>                         };
>                         scu_src3_clk: clock@1028 {
>                                 reg = <1028>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src3";
>                         };
>                         scu_src2_clk: clock@1029 {
>                                 reg = <1029>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src2";
>                         };
>                         scu_src1_clk: clock@1030 {
>                                 reg = <1030>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src1";
>                         };
>                         scu_src0_clk: clock@1031 {
>                                 reg = <1031>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src0";
>                         };
> 
>                         /* MSTP11 */
>                         scifa3_clk: clock@1106 {
>                                 reg = <1106>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa3";
>                         };
>                         scifa4_clk: clock@1107 {
>                                 reg = <1107>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa4";
>                         };
>                         scifa5_clk: clock@1108 {
>                                 reg = <1108>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa5";
>                         };
>                 };
>         };
> 
> 
> References
> ----------
> 
>   [1] Documentation/devicetree/bindings/clock/renesas,<SoC>-cpg-clocks.txt
>       where <SoC> is a single SoC name or a family of SoCs
>   [2] Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
>   [3] Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
>   [4] Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
>   [5] arch/arm/boot/dts/{sh7,r7,r8}*.dtsi
>   [6] Series "[PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains"
>       (https://lkml.org/lkml/2015/5/28/590,
>        http://lkml.iu.edu/hypermail/linux/kernel/1505.3/03336.html)
>   [7] Series "[PATCH 0/6][RFC] arm64: Renesas Gen3 initial patch"
>       (http://www.spinics.net/lists/linux-sh/msg42683.html)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Renesas CPG/MSTP DT Binding Proposal
@ 2015-06-22 20:36   ` Laurent Pinchart
  0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2015-06-22 20:36 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd, linux-clk, Linux PM list, devicetree,
	Linux-sh list

Hi Geert,

On Monday 22 June 2015 21:49:23 Geert Uytterhoeven wrote:
> Introduction
> ------------
> 
> On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
> MSTP (Module Standby and Software Reset), and APMU (Advanced Power
> Management Unit for AP-System Core) blocks are very intimately tied.
> 
>   - The CPG clock generates various clocks for LSI internal generation,
>   - The MSTP block provides two functions:
>       1. Module Standby: "Clock supply to specified modules is stopped by
>          setting the module stop control register bits."
>          However, the clock supply to a module is not stopped until all CPUs
>          in the SoC agree.  Indeed, there are separate MSTP registers for
>          application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
>       2. Reset Control. to perform a software reset of a specific module.
>   - The APMU controls power and clock supply to the AP-system core (e.g.
>   CA7, CA15, SCU).
> 
> MSTP and CPG registers are mixed in one register block.
> The APMU has one or two separate register blocks.
> 
> The current DT bindings are split across 4 different binding documents:
>   1. Renesas SoC-specific "core" CPG clocks that cannot be represented
> easily in DT in another way (cfr. [1]),
>   2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
>   3. Renesas-specific "div6" variable factor clocks, also generated by the
> CPG (cfr. [3]).
>   3. Generic "fixed-factor-clock" clocks, generated by the CPG, but
> represented in DT using the generic ""fixed-factor-clock" bindings (cfr.
> [4]),
> 
> Note that currently the Reset Control function of the MSTP block is not
> supported by DT nor Linux, and that there are no DT bindings for the APMU
> block yet.
> 
> 
> Issues with current bindings
> ----------------------------
> 
> General:
>   - Tight coupling of CPG and MSTP is not represented in DT:
>       - There's one CPG node, and separate MSTP nodes (one for each block of
>         up to 32 possible MSTP clocks) next to it,
>       - CPG Clock Domain requires "power-domain = <...>" properties linking
>         individual device nodes to the "CPG/MSTP" clock domain provider.
>         For now I use a link to the (single) CPG device node, while the
>         clock provider (mostly) controls the (multiple) MSTP clocks (cfr.
>         [6]).
>       - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
>         requires writing a special value to a specific "Write Protect
>         Register" inside the CPG first. Exact details and impact are still
>         under investigation (cfr. [7]).
> 
> MSTP-specific:
>   - The clock hierarchy (parent-child relationship) is represented in a flat
>     structure,

What do you mean exactly here ?

>   - Multiple arrays have to be kept in sync:
>       - "clocks" (parents),
>       - "clock-indices" (sparse bit index inside register),
>       - "clock-output-names",
>   - MSTP clocks are referred to using a reference to a device node and a bit
>     index:
>        - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h
>          are separated from the .dtsi file,
>        - There's no protection against using a bit index definition for a
>          different MSTP register, e.g.
>            - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
>            - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),

If this concerns you, we could add the MSTP number to the macro name, making 
it easier to catch such issues.

>    - The "reg" properties contain only the SMSTPCRx (control register for
>      System CPU) and MSTPSRx (optional status register) registers only.
>      However, in reality there are many more registers:
>        - Separate control registers for application (Cortex-A, supported)
>          and real-time (SH and/or Cortex-R, not supported) cores,
>        - Software reset registers (not supported).
>      To make matters more complicated:
>        - Many registers are optional.
>          The current bindings just support one control register and one
>          optional status register, but this doesn't scale to more register
>          sets,

We could just use named registers for that.

>        - Register sets are not linear and not contiguous in the register
>          space. I.e. it's not possible to derive the address of SMSTPCRx
>          from the address of SMSTPCRy, or the address of MSTPSRx from the
>          address of SMSTPCRx.

When the hardware is to be blamed we seem to all agree easily :-)

> Proposal
> --------
> 
> I'd like to propose the following, to resolve (most of) the issues with the
> current bindings:
> 
>   - Get rid of the "clocks" node.
>     IIRC grouping all clocks under a "clock" node was deprecated, but I
>     can't find the email anymore?
>       - "fixed-clock" clocks are now at the root node level,

I wonder whether we should move all clocks to a separate .dtsi, they take 
quite a bit of space.

>       - The CPG node is now at the root node level,

The CPG node should actually be a child of a bus, not a child of the root 
node. That's a separate issue though.

>   - CPG node:
>       - Use e.g. "renesas,r8a7791-cpg" as compatible value,
>   - MSTP clocks:
>       - All MSTP clocks are now grouped inside the CPG node, under a new
>         "mstp" subnode, without compatible values,

We've discussed this several times in the past and agreed, the MSTP nodes 
should be subnodes of the CPG node.

>       - Each individual MSTP clock has its own subnode, referring to the
>         MSTP clock number, instead of using one node per register, and
>         arrays of "clocks", "clock-output-names", and "clock-indices"
>         properties.

That I don't really like. It would result in more than a hundred of clock 
nodes. I have discussed this option with Mike when designing the clock DT 
bindings and he didn't like it either.

>       - The list of available registers is maintained as arrays inside the
>         unified CPG/MSTP driver, e.g.
> 
>         struct cpg {
>                 void __iomem *base;
>                 unsigned int n_mstp;    /* Size of a register set */
>                 /*
>                  * Optional arrays of register offsets for the various
>                  * register sets:
>                  *   - Array pointer may be NULL, if not supported */
>                  *   - Array element may be ~0, if not supported */
>                  */
>                 u16 *smstp;             /* System CPU control registers */
>                 u16 *rmstp;             /* Realtime CPU control registers */
>                 u16 *mstpsr;            /* Status registers */
>                 u16 *srcr;              /* Reset registers */
>                 ...

I'd create a struct to group smstp, rmstp, mstpsr, srcr & co and have a single 
array of those structures.

>         };
> 
> 
> Short example
> -------------
> 
> (from Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt)
> 
> For the MSTP part:
> 
>         #include <dt-bindings/clock/r8a7790-clock.h>
> 
>         mstp3_clks: mstp3_clks@e615013c {
>                 compatible = "renesas,r8a7790-mstp-clocks",
>                              "renesas,cpg-mstp-clocks";
>                 reg = <0 0xe615013c 0 4>, <0 0xe6150048 0 4>;
>                 clocks = <&cp_clk>, <&mmc1_clk>, <&sd3_clk>, <&sd2_clk>,
>                          <&cpg_clocks R8A7790_CLK_SD1>,
>                          <&cpg_clocks R8A7790_CLK_SD0>, <&mmc0_clk>;
>                 #clock-cells = <1>;
>                 clock-output-names =
>                         "tpu0", "mmcif1", "sdhi3", "sdhi2",
>                          "sdhi1", "sdhi0", "mmcif0";
>                 clock-indices = <
>                         R8A7790_CLK_TPU0 R8A7790_CLK_MMCIF1
> R8A7790_CLK_SDHI3 R8A7790_CLK_SDHI2 R8A7790_CLK_SDHI1 R8A7790_CLK_SDHI0
> R8A7790_CLK_MMCIF0
> 
>                 >;
> 
>         };
> 
> would become:
> 
>         mstp {
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>                 ...
> 
>                 /* MSTP3 */
>                 tpu0_clk: clock@304 {
>                         reg = <304>;
>                         clocks = <&cp_clk>;
>                         clock-output-names = "tpu0";
>                 };
>                 mmcif1_clk: clock@305 {
>                         reg = <305>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
>                 sdhi3_clk: clock@311 {
>                         reg = <311>;
>                         clocks = <&sd3_clk>;
>                         clock-output-names = "sdhi3";
>                 };
>                 sdhi2_clk: clock@312 {
>                         reg = <312>;
>                         clocks = <&sd2_clk>;
>                         clock-output-names = "sdhi2";
>                 };
>                 sdhi1_clk: clock@313 {
>                         reg = <313>;
>                         clocks = <&cpg_clocks R8A7790_CLK_SD1>;
>                         clock-output-names = "sdhi1";
>                 };
>                 sdhi0_clk: clock@314 {
>                         reg = <314>;
>                         clocks = <&cpg_clocks R8A7790_CLK_SD0>;
>                         clock-output-names = "sdhi0";
>                 };
>                 mmcif0_clk: clock@315 {
>                         reg = <315>;
>                         clocks = <&mmc0_clk>;
>                         clock-output-names = "mmcif0";
>                 };
> 
>                 ...
>         }
> 
> Devices would no longer use e.g. "clocks = <&mstp3_clks
> R8A7790_CLK_MMCIF1>", but "clocks = <&mmcif1_clk>".
> 
> 
> Questions/alternatives
> ----------------------
> 
>   - Should the subnode be called "mssr" (Module Standby and Software Reset)
>     instead of "mstp"?
>   - What with "renesas,*-cpg-clocks" clocks?
>       - Should we keep on using "#clock-cells = <1>" and <SOC>_CLK_<NAME>
>         indices, or switch to a less error-prone "#clock-cells = <0>"?
>   - What with "renesas,cpg-div6-clock" clocks?
>       - For now I kept them as a subnode of cpg_clocks, as their registers
>         are a subset of the CPG registers.

If you want to keep DT nodes for them I see no other location. The other 
option would be to merge them into the CPG node and add them to the list of 
clocks exposed by the CPG.

>       - This also means I had to keep the "#address-cells = <2>",
>         "#size-cells = <2>", and "ranges" properties under the CPG node.
>   - The MSTP number could be split in register and bit offset (and thus use
>     "#address-cells = <2>"), e.g.:
> 
>                 mmcif1_clk: clock@3,5 {
>                         reg = <3 5>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
> 
>         vs.
> 
>                 mmcif1_clk: clock@305 {
>                         reg = <305>;
>                         clocks = <&mmc1_clk>;
>                         clock-output-names = "mmcif1";
>                 };
> 
>     I don't think it's clearer, as the datasheet refers to e.g. "MSTP305".

You forgot the favourite topic of DT maintainers : how will this handle 
backward compatibility ?

> Thanks for your comments!
> 
> 
> Long example
> ------------
> 
> All r8a7791 clocks:
> 
>         /* External root clock */
>         extal_clk: extal_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 /* This value must be overriden by the board. */
>                 clock-frequency = <0>;
>                 clock-output-names = "extal";
>         };
> 
>         /*
>          * The external audio clocks are configured as 0 Hz fixed
> frequency clocks by
>          * default. Boards that provide audio clocks should override them.
>          */
>         audio_clk_a: audio_clk_a {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_a";
>         };
>         audio_clk_b: audio_clk_b {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_b";
>         };
>         audio_clk_c: audio_clk_c {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <0>;
>                 clock-output-names = "audio_clk_c";
>         };
> 
>         /* External PCIe clock - can be overridden by the board */
>         pcie_bus_clk: pcie_bus_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <100000000>;
>                 clock-output-names = "pcie_bus";
>                 status = "disabled";
>         };
> 
>         /* External USB clock - can be overridden by the board */
>         usb_extal_clk: usb_extal_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 clock-frequency = <48000000>;
>                 clock-output-names = "usb_extal";
>         };
> 
>         /* External CAN clock */
>         can_clk: can_clk {
>                 compatible = "fixed-clock";
>                 #clock-cells = <0>;
>                 /* This value must be overridden by the board. */
>                 clock-frequency = <0>;
>                 clock-output-names = "can_clk";
>                 status = "disabled";
>         };
> 
>         /* Fixed factor clocks */
>         pll1_div2_clk: pll1_div2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "pll1_div2";
>         };
>         zg_clk: zg_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <3>;
>                 clock-mult = <1>;
>                 clock-output-names = "zg";
>         };
>         zx_clk: zx_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <3>;
>                 clock-mult = <1>;
>                 clock-output-names = "zx";
>         };
>         zs_clk: zs_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <6>;
>                 clock-mult = <1>;
>                 clock-output-names = "zs";
>         };
>         hp_clk: hp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <12>;
>                 clock-mult = <1>;
>                 clock-output-names = "hp";
>         };
>         i_clk: i_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "i";
>         };
>         b_clk: b_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <12>;
>                 clock-mult = <1>;
>                 clock-output-names = "b";
>         };
>         p_clk: p_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <24>;
>                 clock-mult = <1>;
>                 clock-output-names = "p";
>         };
>         cl_clk: cl_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <48>;
>                 clock-mult = <1>;
>                 clock-output-names = "cl";
>         };
>         m2_clk: m2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "m2";
>         };
>         imp_clk: imp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <4>;
>                 clock-mult = <1>;
>                 clock-output-names = "imp";
>         };
>         rclk_clk: rclk_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <(48 * 1024)>;
>                 clock-mult = <1>;
>                 clock-output-names = "rclk";
>         };
>         oscclk_clk: oscclk_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL1>;
>                 #clock-cells = <0>;
>                 clock-div = <(12 * 1024)>;
>                 clock-mult = <1>;
>                 clock-output-names = "oscclk";
>         };
>         zb3_clk: zb3_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <4>;
>                 clock-mult = <1>;
>                 clock-output-names = "zb3";
>         };
>         zb3d2_clk: zb3d2_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "zb3d2";
>         };
>         ddr_clk: ddr_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&cpg_clocks R8A7791_CLK_PLL3>;
>                 #clock-cells = <0>;
>                 clock-div = <8>;
>                 clock-mult = <1>;
>                 clock-output-names = "ddr";
>         };
>         mp_clk: mp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&pll1_div2_clk>;
>                 #clock-cells = <0>;
>                 clock-div = <15>;
>                 clock-mult = <1>;
>                 clock-output-names = "mp";
>         };
>         cp_clk: cp_clk {
>                 compatible = "fixed-factor-clock";
>                 clocks = <&extal_clk>;
>                 #clock-cells = <0>;
>                 clock-div = <2>;
>                 clock-mult = <1>;
>                 clock-output-names = "cp";
>         };
> 
>         /* Special CPG clocks */
>         cpg_clocks: cpg_clocks@e6150000 {
>                 #address-cells = <2>;
>                 #size-cells = <2>;
>                 ranges;
> 
>                 compatible = "renesas,r8a7791-cpg",
>                 reg = <0 0xe6150000 0 0x1000>;
>                 clocks = <&extal_clk &usb_extal_clk>;
>                 #clock-cells = <1>;
>                 clock-output-names = "main", "pll0", "pll1", "pll3",
>                                      "lb", "qspi", "sdh", "sd0", "z",
>                                      "rcan", "adsp";
>                 #power-domain-cells = <0>;
> 
>                 /* Variable factor clocks */
>                 sd2_clk: sd2_clk@e6150078 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150078 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "sd2";
>                 };
>                 sd3_clk: sd3_clk@e615026c {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe615026c 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "sd3";
>                 };
>                 mmc0_clk: mmc0_clk@e6150240 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150240 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "mmc0";
>                 };
>                 ssp_clk: ssp_clk@e6150248 {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe6150248 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "ssp";
>                 };
>                 ssprs_clk: ssprs_clk@e615024c {
>                         compatible = "renesas,r8a7791-div6-clock",
>                                      "renesas,cpg-div6-clock";
>                         reg = <0 0xe615024c 0 4>;
>                         clocks = <&pll1_div2_clk>;
>                         #clock-cells = <0>;
>                         clock-output-names = "ssprs";
>                 };
> 
>                 /* Module Standby and Software Reset */
>                 mstp {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> 
>                         /* MSTP0 */
>                         msiof0_clk: clock@000 {
>                                 reg = <000>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof0";
>                         };
> 
>                         /* MSTP1 */
>                         vcp0_clk: clock@101 {
>                                 reg = <101>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vcp0";
>                         };
>                         vpc0_clk: clock@103 {
>                                 reg = <103>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vpc0";
>                         };
>                         jpu_clk: clock@106 {
>                                 reg = <106>;
>                                 clocks = <&m2_clk>;
>                                 clock-output-names = "jpu";
>                         };
>                         ssp1_clk: clock@109 {
>                                 reg = <109>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "ssp1";
>                         };
>                         tmu1_clk: clock@111 {
>                                 reg = <111>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu1";
>                         };
>                         threedg_clk: clock@112 {
>                                 reg = <112>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "3dg";
>                         };
>                         twoddmac_clk: clock@115 {
>                                 reg = <115>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "2ddmac";
>                         };
>                         fdp1_1_clk: clock@118 {
>                                 reg = <118>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "fdp1-1";
>                         };
>                         fdp1_0_clk: clock@119 {
>                                 reg = <119>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "fdp1-0";
>                         };
>                         tmu3_clk: clock@121 {
>                                 reg = <121>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu3";
>                         };
>                         tmu2_clk: clock@122 {
>                                 reg = <122>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "tmu2";
>                         };
>                         cmt0_clk: clock@124 {
>                                 reg = <124>;
>                                 clocks = <&rclk_clk>;
>                                 clock-output-names = "cmt0";
>                         };
>                         tmu0_clk: clock@125 {
>                                 reg = <125>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "tmu0";
>                         };
>                         vsp1_du1_clk: clock@127 {
>                                 reg = <127>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-du1";
>                         };
>                         vsp1_du0_clk: clock@128 {
>                                 reg = <128>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-du0";
>                         };
>                         vsp1_sy_clk: clock@131 {
>                                 reg = <131>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "vsp1-sy";
>                         };
> 
>                         /* MSTP2 */
>                         scifa2_clk: clock@202 {
>                                 reg = <202>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa2";
>                         };
>                         scifa1_clk: clock@203 {
>                                 reg = <203>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa1";
>                         };
>                         scifa0_clk: clock@204 {
>                                 reg = <204>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa0";
>                         };
>                         msiof2_clk: clock@205 {
>                                 reg = <205>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof2";
>                         };
>                         scifb0_clk: clock@206 {
>                                 reg = <206>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb0";
>                         };
>                         scifb1_clk: clock@207 {
>                                 reg = <207>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb1";
>                         };
>                         msiof1_clk: clock@208 {
>                                 reg = <208>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "msiof1";
>                         };
>                         scifb2_clk: clock@216 {
>                                 reg = <216>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifb2";
>                         };
>                         sys_dmac1_clk: clock@218 {
>                                 reg = <218>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sys-dmac1";
>                         };
>                         sys_dmac0_clk: clock@219 {
>                                 reg = <219>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sys-dmac0";
>                         };
> 
>                         /* MSTP3 */
>                         tpu0_clk: clock@304 {
>                                 reg = <304>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "tpu0";
>                         };
>                         sdhi2_clk: clock@311 {
>                                 reg = <311>;
>                                 clocks = <&sd3_clk>;
>                                 clock-output-names = "sdhi2";
>                         };
>                         sdhi1_clk: clock@312 {
>                                 reg = <312>;
>                                 clocks = <&sd2_clk>;
>                                 clock-output-names = "sdhi1";
>                         };
>                         sdhi0_clk: clock@314 {
>                                 reg = <314>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_SD0>;
>                                 clock-output-names = "sdhi0";
>                         };
>                         mmcif0_clk: clock@315 {
>                                 reg = <315>;
>                                 clocks = <&mmc0_clk>;
>                                 clock-output-names = "mmcif0";
>                         };
>                         i2c7_clk: clock@318 {
>                                 reg = <318>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c7";
>                         };
>                         pciec_clk: clock@319 {
>                                 reg = <319>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "pciec";
>                         };
>                         i2c8_clk: clock@323 {
>                                 reg = <323>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c8";
>                         };
>                         ssusb_clk: clock@328 {
>                                 reg = <328>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "ssusb";
>                         };
>                         cmt1_clk: clock@329 {
>                                 reg = <329>;
>                                 clocks = <&rclk_clk>;
>                                 clock-output-names = "cmt1";
>                         };
>                         usbdmac0_clk: clock@330 {
>                                 reg = <330>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "usbdmac0";
>                         };
>                         usbdmac1_clk: clock@331 {
>                                 reg = <331>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "usbdmac1";
>                         };
> 
>                         /* MSTP4 */
>                         irqc_clk: clock@407 {
>                                 reg = <407>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "irqc";
>                         };
>                         intc_sys_clk: clock@408 {
>                                 reg = <408>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "intc-sys";
>                         };
> 
>                         /* MSTP5 */
>                         audmac1_clk: clock@501 {
>                                 reg = <501>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "audmac1";
>                         };
>                         audmac0_clk: clock@502 {
>                                 reg = <502>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "audmac0";
>                         };
>                         adsp_mod_clk: clock@506 {
>                                 reg = <506>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_ADSP>;
>                                 clock-output-names = "adsp_mod";
>                         };
>                         thermal_clk: clock@522 {
>                                 reg = <522>;
>                                 clocks = <&extal_clk>;
>                                 clock-output-names = "thermal";
>                         };
>                         pwm_clk: clock@523 {
>                                 reg = <523>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "pwm";
>                         };
> 
>                         /* MSTP7 */
>                         ehci_clk: clock@703 {
>                                 reg = <703>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "ehci";
>                         };
>                         hsusb_clk: clock@704 {
>                                 reg = <704>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "hsusb";
>                         };
>                         hscif2_clk: clock@713 {
>                                 reg = <713>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif2";
>                         };
>                         scif5_clk: clock@714 {
>                                 reg = <714>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif5";
>                         };
>                         scif4_clk: clock@715 {
>                                 reg = <715>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif4";
>                         };
>                         hscif1_clk: clock@716 {
>                                 reg = <716>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif1";
>                         };
>                         hscif0_clk: clock@717 {
>                                 reg = <717>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "hscif0";
>                         };
>                         scif3_clk: clock@718 {
>                                 reg = <718>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif3";
>                         };
>                         scif2_clk: clock@719 {
>                                 reg = <719>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif2";
>                         };
>                         scif1_clk: clock@720 {
>                                 reg = <720>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif1";
>                         };
>                         scif0_clk: clock@721 {
>                                 reg = <721>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scif0";
>                         };
>                         du1_clk: clock@723 {
>                                 reg = <723>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "du1";
>                         };
>                         du0_clk: clock@724 {
>                                 reg = <724>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "du0";
>                         };
>                         lvds0_clk: clock@726 {
>                                 reg = <726>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "lvds0";
>                         };
> 
>                         /* MSTP8 */
>                         ipmmu_sgx_clk: clock@800 {
>                                 reg = <800>;
>                                 clocks = <&zx_clk>;
>                                 clock-output-names = "ipmmu_sgx";
>                         };
>                         mlb_clk: clock@802 {
>                                 reg = <802>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "mlb";
>                         };
>                         vin2_clk: clock@809 {
>                                 reg = <809>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin2";
>                         };
>                         vin1_clk: clock@810 {
>                                 reg = <810>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin1";
>                         };
>                         vin0_clk: clock@811 {
>                                 reg = <811>;
>                                 clocks = <&zg_clk>;
>                                 clock-output-names = "vin0";
>                         };
>                         ether_clk: clock@813 {
>                                 reg = <813>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ether";
>                         };
>                         sata1_clk: clock@814 {
>                                 reg = <814>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sata1";
>                         };
>                         sata0_clk: clock@815 {
>                                 reg = <815>;
>                                 clocks = <&zs_clk>;
>                                 clock-output-names = "sata0";
>                         };
> 
>                         /* MSTP9 */
>                         gpio7_clk: clock@904 {
>                                 reg = <904>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio7";
>                         };
>                         gpio6_clk: clock@905 {
>                                 reg = <905>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio6";
>                         };
>                         gpio5_clk: clock@907 {
>                                 reg = <907>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio5";
>                         };
>                         gpio4_clk: clock@908 {
>                                 reg = <908>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio4";
>                         };
>                         gpio3_clk: clock@909 {
>                                 reg = <909>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio3";
>                         };
>                         gpio2_clk: clock@910 {
>                                 reg = <910>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio2";
>                         };
>                         gpio1_clk: clock@911 {
>                                 reg = <911>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio1";
>                         };
>                         gpio0_clk: clock@912 {
>                                 reg = <912>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "gpio0";
>                         };
>                         rcan1_clk: clock@915 {
>                                 reg = <915>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "rcan1";
>                         };
>                         rcan0_clk: clock@916 {
>                                 reg = <916>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "rcan0";
>                         };
>                         qspi_mod_clk: clock@917 {
>                                 reg = <917>;
>                                 clocks = <&cpg_clocks R8A7791_CLK_QSPI>;
>                                 clock-output-names = "qspi_mod";
>                         };
>                         i2c5_clk: clock@925 {
>                                 reg = <925>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c5";
>                         };
>                         i2c6_clk: clock@926 {
>                                 reg = <926>;
>                                 clocks = <&cp_clk>;
>                                 clock-output-names = "i2c6";
>                         };
>                         i2c4_clk: clock@927 {
>                                 reg = <927>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c4";
>                         };
>                         i2c3_clk: clock@928 {
>                                 reg = <928>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c3";
>                         };
>                         i2c2_clk: clock@929 {
>                                 reg = <929>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c2";
>                         };
>                         i2c1_clk: clock@930 {
>                                 reg = <930>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c1";
>                         };
>                         i2c0_clk: clock@931 {
>                                 reg = <931>;
>                                 clocks = <&hp_clk>;
>                                 clock-output-names = "i2c0";
>                         };
> 
>                         /* MSTP10 */
>                         ssi_all_clk: clock@1005 {
>                                 reg = <1005>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi-all";
>                         };
>                         ssi9_clk: clock@1006 {
>                                 reg = <1006>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi9";
>                         };
>                         ssi8_clk: clock@1007 {
>                                 reg = <1007>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi8";
>                         };
>                         ssi7_clk: clock@1008 {
>                                 reg = <1008>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi7";
>                         };
>                         ssi6_clk: clock@1009 {
>                                 reg = <1009>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi6";
>                         };
>                         ssi5_clk: clock@1010 {
>                                 reg = <1010>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi5";
>                         };
>                         ssi4_clk: clock@1011 {
>                                 reg = <1011>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi4";
>                         };
>                         ssi3_clk: clock@1012 {
>                                 reg = <1012>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi3";
>                         };
>                         ssi2_clk: clock@1013 {
>                                 reg = <1013>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi2";
>                         };
>                         ssi1_clk: clock@1014 {
>                                 reg = <1014>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi1";
>                         };
>                         ssi0_clk: clock@1015 {
>                                 reg = <1015>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "ssi0";
>                         };
>                         scu_all_clk: clock@1017 {
>                                 reg = <1017>;
>                                 clocks = <&p_clk>;
>                                 clock-output-names = "scu-all";
>                         };
>                         scu_dvc1_clk: clock@1018 {
>                                 reg = <1018>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-dvc1";
>                         };
>                         scu_dvc0_clk: clock@1019 {
>                                 reg = <1019>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-dvc0";
>                         };
>                         scu_src9_clk: clock@1022 {
>                                 reg = <1022>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src9";
>                         };
>                         scu_src8_clk: clock@1023 {
>                                 reg = <1023>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src8";
>                         };
>                         scu_src7_clk: clock@1024 {
>                                 reg = <1024>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src7";
>                         };
>                         scu_src6_clk: clock@1025 {
>                                 reg = <1025>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src6";
>                         };
>                         scu_src5_clk: clock@1026 {
>                                 reg = <1026>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src5";
>                         };
>                         scu_src4_clk: clock@1027 {
>                                 reg = <1027>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src4";
>                         };
>                         scu_src3_clk: clock@1028 {
>                                 reg = <1028>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src3";
>                         };
>                         scu_src2_clk: clock@1029 {
>                                 reg = <1029>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src2";
>                         };
>                         scu_src1_clk: clock@1030 {
>                                 reg = <1030>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src1";
>                         };
>                         scu_src0_clk: clock@1031 {
>                                 reg = <1031>;
>                                 clocks = <&scu_all_clk>;
>                                 clock-output-names = "scu-src0";
>                         };
> 
>                         /* MSTP11 */
>                         scifa3_clk: clock@1106 {
>                                 reg = <1106>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa3";
>                         };
>                         scifa4_clk: clock@1107 {
>                                 reg = <1107>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa4";
>                         };
>                         scifa5_clk: clock@1108 {
>                                 reg = <1108>;
>                                 clocks = <&mp_clk>;
>                                 clock-output-names = "scifa5";
>                         };
>                 };
>         };
> 
> 
> References
> ----------
> 
>   [1] Documentation/devicetree/bindings/clock/renesas,<SoC>-cpg-clocks.txt
>       where <SoC> is a single SoC name or a family of SoCs
>   [2] Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt
>   [3] Documentation/devicetree/bindings/clock/renesas,cpg-div6-clocks.txt
>   [4] Documentation/devicetree/bindings/clock/fixed-factor-clock.txt
>   [5] arch/arm/boot/dts/{sh7,r7,r8}*.dtsi
>   [6] Series "[PATCH v2 00/14] ARM: shmobile: Add CPG Clock Domains"
>       (https://lkml.org/lkml/2015/5/28/590,
>        http://lkml.iu.edu/hypermail/linux/kernel/1505.3/03336.html)
>   [7] Series "[PATCH 0/6][RFC] arm64: Renesas Gen3 initial patch"
>       (http://www.spinics.net/lists/linux-sh/msg42683.html)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Renesas CPG/MSTP DT Binding Proposal
  2015-06-22 20:36   ` Laurent Pinchart
@ 2015-06-23  8:33     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-06-23  8:33 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd, linux-clk, Linux PM list, devicetree,
	Linux-sh list

Hi Laurent,

Thanks for your comments!

On Mon, Jun 22, 2015 at 10:36 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 22 June 2015 21:49:23 Geert Uytterhoeven wrote:
>> On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
>> MSTP (Module Standby and Software Reset), and APMU (Advanced Power
>> Management Unit for AP-System Core) blocks are very intimately tied.
>>
>>   - The CPG clock generates various clocks for LSI internal generation,
>>   - The MSTP block provides two functions:
>>       1. Module Standby: "Clock supply to specified modules is stopped by
>>          setting the module stop control register bits."
>>          However, the clock supply to a module is not stopped until all CPUs
>>          in the SoC agree.  Indeed, there are separate MSTP registers for
>>          application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
>>       2. Reset Control. to perform a software reset of a specific module.
>>   - The APMU controls power and clock supply to the AP-system core (e.g.
>>   CA7, CA15, SCU).
>>
>> MSTP and CPG registers are mixed in one register block.
>> The APMU has one or two separate register blocks.
>>
>> The current DT bindings are split across 4 different binding documents:
>>   1. Renesas SoC-specific "core" CPG clocks that cannot be represented
>> easily in DT in another way (cfr. [1]),
>>   2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
>>   3. Renesas-specific "div6" variable factor clocks, also generated by the
>> CPG (cfr. [3]).
>>   3. Generic "fixed-factor-clock" clocks, generated by the CPG, but
>> represented in DT using the generic ""fixed-factor-clock" bindings (cfr.
>> [4]),
>>
>> Note that currently the Reset Control function of the MSTP block is not
>> supported by DT nor Linux, and that there are no DT bindings for the APMU
>> block yet.
>>
>>
>> Issues with current bindings
>> ----------------------------
>>
>> General:
>>   - Tight coupling of CPG and MSTP is not represented in DT:
>>       - There's one CPG node, and separate MSTP nodes (one for each block of
>>         up to 32 possible MSTP clocks) next to it,
>>       - CPG Clock Domain requires "power-domain = <...>" properties linking
>>         individual device nodes to the "CPG/MSTP" clock domain provider.
>>         For now I use a link to the (single) CPG device node, while the
>>         clock provider (mostly) controls the (multiple) MSTP clocks (cfr.
>>         [6]).
>>       - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
>>         requires writing a special value to a specific "Write Protect
>>         Register" inside the CPG first. Exact details and impact are still
>>         under investigation (cfr. [7]).
>>
>> MSTP-specific:
>>   - The clock hierarchy (parent-child relationship) is represented in a flat
>>     structure,
>
> What do you mean exactly here ?

If all clocks are structured in a tree topology, it would be nice to have a
tree topology in DT, too.
I agree this is impossible with clocks, as they can have multiple parents,
but it worked nicely with hierarchical SYSC power domains on R-Mobile.

>>   - Multiple arrays have to be kept in sync:
>>       - "clocks" (parents),
>>       - "clock-indices" (sparse bit index inside register),
>>       - "clock-output-names",
>>   - MSTP clocks are referred to using a reference to a device node and a bit
>>     index:
>>        - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h
>>          are separated from the .dtsi file,
>>        - There's no protection against using a bit index definition for a
>>          different MSTP register, e.g.
>>            - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
>>            - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),
>
> If this concerns you, we could add the MSTP number to the macro name, making
> it easier to catch such issues.

That's one solution. But this doesn't solve the multiple arrays that have to be
kept in sync, which is where most bugs crept in in the past.

>>    - The "reg" properties contain only the SMSTPCRx (control register for
>>      System CPU) and MSTPSRx (optional status register) registers only.
>>      However, in reality there are many more registers:
>>        - Separate control registers for application (Cortex-A, supported)
>>          and real-time (SH and/or Cortex-R, not supported) cores,
>>        - Software reset registers (not supported).
>>      To make matters more complicated:
>>        - Many registers are optional.
>>          The current bindings just support one control register and one
>>          optional status register, but this doesn't scale to more register
>>          sets,
>
> We could just use named registers for that.

Sure, up to 6 resources, for up to 6 out of 7 types of registers.
Sounds like overkill to me.

>>        - Register sets are not linear and not contiguous in the register
>>          space. I.e. it's not possible to derive the address of SMSTPCRx
>>          from the address of SMSTPCRy, or the address of MSTPSRx from the
>>          address of SMSTPCRx.
>
> When the hardware is to be blamed we seem to all agree easily :-)
>
>> Proposal
>> --------
>>
>> I'd like to propose the following, to resolve (most of) the issues with the
>> current bindings:
>>
>>   - Get rid of the "clocks" node.
>>     IIRC grouping all clocks under a "clock" node was deprecated, but I
>>     can't find the email anymore?
>>       - "fixed-clock" clocks are now at the root node level,
>
> I wonder whether we should move all clocks to a separate .dtsi, they take
> quite a bit of space.

Perhaps.
Should we share more .dtsis for similar parts in the same SoC family,
cfr. arch/arm/boot/dts/am*?
Our SoC-specific compatible values complicate that, though.

>>       - The CPG node is now at the root node level,
>
> The CPG node should actually be a child of a bus, not a child of the root
> node. That's a separate issue though.

Yep. We don't have a bus topology in DT for on-SoC peripherals (yet).

>>   - CPG node:
>>       - Use e.g. "renesas,r8a7791-cpg" as compatible value,
>>   - MSTP clocks:
>>       - All MSTP clocks are now grouped inside the CPG node, under a new
>>         "mstp" subnode, without compatible values,
>
> We've discussed this several times in the past and agreed, the MSTP nodes
> should be subnodes of the CPG node.

OK.

>>       - Each individual MSTP clock has its own subnode, referring to the
>>         MSTP clock number, instead of using one node per register, and
>>         arrays of "clocks", "clock-output-names", and "clock-indices"
>>         properties.
>
> That I don't really like. It would result in more than a hundred of clock
> nodes. I have discussed this option with Mike when designing the clock DT
> bindings and he didn't like it either.

I missed that discussion. Was it on a mailing list, with an archive?

If each clock node would have been a full-blown device node, with its own
compatible value, I can understand, as that would lead to one platform_device
+ resources per clock, i.e. quite some memory.

With my proposal, there would be only one platform device, for the CPG
device node (and possibly one for each div6 clock), i.e. much less memory
consumption.

>>       - The list of available registers is maintained as arrays inside the
>>         unified CPG/MSTP driver, e.g.
>>
>>         struct cpg {
>>                 void __iomem *base;
>>                 unsigned int n_mstp;    /* Size of a register set */
>>                 /*
>>                  * Optional arrays of register offsets for the various
>>                  * register sets:
>>                  *   - Array pointer may be NULL, if not supported */
>>                  *   - Array element may be ~0, if not supported */
>>                  */
>>                 u16 *smstp;             /* System CPU control registers */
>>                 u16 *rmstp;             /* Realtime CPU control registers */
>>                 u16 *mstpsr;            /* Status registers */
>>                 u16 *srcr;              /* Reset registers */
>>                 ...
>
> I'd create a struct to group smstp, rmstp, mstpsr, srcr & co and have a single
> array of those structures.

I deliberately chose multiple arrays, as over the entire SoC range, 2-6 register
types (out of 7) are available. All other array pointers can be NULL.
In addition, not all MSTP blocks have the same number of register types
(e.g. R-Car Gen1 doesn't have status registers for all MSTP blocks).

> You forgot the favourite topic of DT maintainers : how will this handle
> backward compatibility ?

The new CPG node is compatible with e.g. "renesas,r8a7791-cpg", which is a
brand new value. Hence as long as you compile in the old driver, it'll be
compatible with old DTs.

Thanks!

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Renesas CPG/MSTP DT Binding Proposal
@ 2015-06-23  8:33     ` Geert Uytterhoeven
  0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2015-06-23  8:33 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Laurent Pinchart, Magnus Damm, Ulrich Hecht, Michael Turquette,
	Stephen Boyd, linux-clk, Linux PM list, devicetree,
	Linux-sh list

Hi Laurent,

Thanks for your comments!

On Mon, Jun 22, 2015 at 10:36 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Monday 22 June 2015 21:49:23 Geert Uytterhoeven wrote:
>> On Renesas SoCs (SH/R-Mobile, R-Car, RZ), the CPG (Clock Pulse Generator),
>> MSTP (Module Standby and Software Reset), and APMU (Advanced Power
>> Management Unit for AP-System Core) blocks are very intimately tied.
>>
>>   - The CPG clock generates various clocks for LSI internal generation,
>>   - The MSTP block provides two functions:
>>       1. Module Standby: "Clock supply to specified modules is stopped by
>>          setting the module stop control register bits."
>>          However, the clock supply to a module is not stopped until all CPUs
>>          in the SoC agree.  Indeed, there are separate MSTP registers for
>>          application (Cortex-A) and real-time (SH and/or Cortex-R) cores.
>>       2. Reset Control. to perform a software reset of a specific module.
>>   - The APMU controls power and clock supply to the AP-system core (e.g.
>>   CA7, CA15, SCU).
>>
>> MSTP and CPG registers are mixed in one register block.
>> The APMU has one or two separate register blocks.
>>
>> The current DT bindings are split across 4 different binding documents:
>>   1. Renesas SoC-specific "core" CPG clocks that cannot be represented
>> easily in DT in another way (cfr. [1]),
>>   2. Renesas-specific MSTP "Module Standby" clocks (cfr. [2]),
>>   3. Renesas-specific "div6" variable factor clocks, also generated by the
>> CPG (cfr. [3]).
>>   3. Generic "fixed-factor-clock" clocks, generated by the CPG, but
>> represented in DT using the generic ""fixed-factor-clock" bindings (cfr.
>> [4]),
>>
>> Note that currently the Reset Control function of the MSTP block is not
>> supported by DT nor Linux, and that there are no DT bindings for the APMU
>> block yet.
>>
>>
>> Issues with current bindings
>> ----------------------------
>>
>> General:
>>   - Tight coupling of CPG and MSTP is not represented in DT:
>>       - There's one CPG node, and separate MSTP nodes (one for each block of
>>         up to 32 possible MSTP clocks) next to it,
>>       - CPG Clock Domain requires "power-domain = <...>" properties linking
>>         individual device nodes to the "CPG/MSTP" clock domain provider.
>>         For now I use a link to the (single) CPG device node, while the
>>         clock provider (mostly) controls the (multiple) MSTP clocks (cfr.
>>         [6]).
>>       - On R-Car Gen3, any CPG/MSTP (and APMU) register write access
>>         requires writing a special value to a specific "Write Protect
>>         Register" inside the CPG first. Exact details and impact are still
>>         under investigation (cfr. [7]).
>>
>> MSTP-specific:
>>   - The clock hierarchy (parent-child relationship) is represented in a flat
>>     structure,
>
> What do you mean exactly here ?

If all clocks are structured in a tree topology, it would be nice to have a
tree topology in DT, too.
I agree this is impossible with clocks, as they can have multiple parents,
but it worked nicely with hierarchical SYSC power domains on R-Mobile.

>>   - Multiple arrays have to be kept in sync:
>>       - "clocks" (parents),
>>       - "clock-indices" (sparse bit index inside register),
>>       - "clock-output-names",
>>   - MSTP clocks are referred to using a reference to a device node and a bit
>>     index:
>>        - Bit index definitions in include/dt-bindings/clock/<soc>-clock.h
>>          are separated from the .dtsi file,
>>        - There's no protection against using a bit index definition for a
>>          different MSTP register, e.g.
>>            - clocks = <&mstp8_clks R8A7791_CLK_QSPI_MOD> (wrong),
>>            - clocks = <&mstp9_clks R8A7791_CLK_QSPI_MOD> (correct),
>
> If this concerns you, we could add the MSTP number to the macro name, making
> it easier to catch such issues.

That's one solution. But this doesn't solve the multiple arrays that have to be
kept in sync, which is where most bugs crept in in the past.

>>    - The "reg" properties contain only the SMSTPCRx (control register for
>>      System CPU) and MSTPSRx (optional status register) registers only.
>>      However, in reality there are many more registers:
>>        - Separate control registers for application (Cortex-A, supported)
>>          and real-time (SH and/or Cortex-R, not supported) cores,
>>        - Software reset registers (not supported).
>>      To make matters more complicated:
>>        - Many registers are optional.
>>          The current bindings just support one control register and one
>>          optional status register, but this doesn't scale to more register
>>          sets,
>
> We could just use named registers for that.

Sure, up to 6 resources, for up to 6 out of 7 types of registers.
Sounds like overkill to me.

>>        - Register sets are not linear and not contiguous in the register
>>          space. I.e. it's not possible to derive the address of SMSTPCRx
>>          from the address of SMSTPCRy, or the address of MSTPSRx from the
>>          address of SMSTPCRx.
>
> When the hardware is to be blamed we seem to all agree easily :-)
>
>> Proposal
>> --------
>>
>> I'd like to propose the following, to resolve (most of) the issues with the
>> current bindings:
>>
>>   - Get rid of the "clocks" node.
>>     IIRC grouping all clocks under a "clock" node was deprecated, but I
>>     can't find the email anymore?
>>       - "fixed-clock" clocks are now at the root node level,
>
> I wonder whether we should move all clocks to a separate .dtsi, they take
> quite a bit of space.

Perhaps.
Should we share more .dtsis for similar parts in the same SoC family,
cfr. arch/arm/boot/dts/am*?
Our SoC-specific compatible values complicate that, though.

>>       - The CPG node is now at the root node level,
>
> The CPG node should actually be a child of a bus, not a child of the root
> node. That's a separate issue though.

Yep. We don't have a bus topology in DT for on-SoC peripherals (yet).

>>   - CPG node:
>>       - Use e.g. "renesas,r8a7791-cpg" as compatible value,
>>   - MSTP clocks:
>>       - All MSTP clocks are now grouped inside the CPG node, under a new
>>         "mstp" subnode, without compatible values,
>
> We've discussed this several times in the past and agreed, the MSTP nodes
> should be subnodes of the CPG node.

OK.

>>       - Each individual MSTP clock has its own subnode, referring to the
>>         MSTP clock number, instead of using one node per register, and
>>         arrays of "clocks", "clock-output-names", and "clock-indices"
>>         properties.
>
> That I don't really like. It would result in more than a hundred of clock
> nodes. I have discussed this option with Mike when designing the clock DT
> bindings and he didn't like it either.

I missed that discussion. Was it on a mailing list, with an archive?

If each clock node would have been a full-blown device node, with its own
compatible value, I can understand, as that would lead to one platform_device
+ resources per clock, i.e. quite some memory.

With my proposal, there would be only one platform device, for the CPG
device node (and possibly one for each div6 clock), i.e. much less memory
consumption.

>>       - The list of available registers is maintained as arrays inside the
>>         unified CPG/MSTP driver, e.g.
>>
>>         struct cpg {
>>                 void __iomem *base;
>>                 unsigned int n_mstp;    /* Size of a register set */
>>                 /*
>>                  * Optional arrays of register offsets for the various
>>                  * register sets:
>>                  *   - Array pointer may be NULL, if not supported */
>>                  *   - Array element may be ~0, if not supported */
>>                  */
>>                 u16 *smstp;             /* System CPU control registers */
>>                 u16 *rmstp;             /* Realtime CPU control registers */
>>                 u16 *mstpsr;            /* Status registers */
>>                 u16 *srcr;              /* Reset registers */
>>                 ...
>
> I'd create a struct to group smstp, rmstp, mstpsr, srcr & co and have a single
> array of those structures.

I deliberately chose multiple arrays, as over the entire SoC range, 2-6 register
types (out of 7) are available. All other array pointers can be NULL.
In addition, not all MSTP blocks have the same number of register types
(e.g. R-Car Gen1 doesn't have status registers for all MSTP blocks).

> You forgot the favourite topic of DT maintainers : how will this handle
> backward compatibility ?

The new CPG node is compatible with e.g. "renesas,r8a7791-cpg", which is a
brand new value. Hence as long as you compile in the old driver, it'll be
compatible with old DTs.

Thanks!

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2015-06-23  8:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-22 19:49 Renesas CPG/MSTP DT Binding Proposal Geert Uytterhoeven
2015-06-22 19:49 ` Geert Uytterhoeven
2015-06-22 19:49 ` Geert Uytterhoeven
2015-06-22 20:36 ` Laurent Pinchart
2015-06-22 20:36   ` Laurent Pinchart
2015-06-22 20:36   ` Laurent Pinchart
2015-06-23  8:33   ` Geert Uytterhoeven
2015-06-23  8:33     ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.