[PATCH v4 0/4] clk: visconti: Add support common clock driver and reset driver

[PATCH v4 0/4] clk: visconti: Add support common clock driver and reset driver

[Nobuhiro Iwamatsu]

Hi,

This series is PLL, clock and reset driver for Toshiba's ARM SoC, Visconti[0].
Since the clock driver and reset driver are provided as one function, they are
provided in cooperation with the clock driver.

This provides DT binding documentation, device driver, MAINTAINER files.

Best regards,
  Nobuhiro

[0]:
https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html

  clk: visconti: Add support common clock driver and reset driver
    v3 -> v4:
       - Remove osc2 clock's code. Move to DT.
    v2 -> v3:
       - Fix return value in visconti_register_pll().
       - Remove initialization of flags used by spin_lock_irqsave().
       - - Change function name from *7708* to *770x*.
       - Fix some coding style.
    v1 -> v2:
       - Fix warning with W=1

  dt-bindings: clock: Add DT bindings for PLL of Toshiba Visconti TMPV7708 SoC
    v3 -> v4:
       - Fix node name to clock-controller.
       - Remove osc2-clk-frequency, and this defines to DT as fixed-clock.
       - Add clocks.
    v2 -> v3:
       - Change file name.
    v1 -> v2:
       - Update subject.

  dt-bindings: clock: Add DT bindings for SMU of Toshiba Visconti TMPV7708 SoC
    v3 -> v4:
      - Move reset dt-binding header file.
      - Move clock dt-binding header file.
      - Change dt-binding header's License to 'GPL-2.0-only OR BSD-2-Clause'
      - Fix node name to clock-controller.
    v2 -> v3:
       - Change file name.
    v1 -> v2:
       - Update subject.

  MAINTAINERS: Add entries for Toshiba Visconti PLL and clock controller
    v3 -> v4:
       - no update.
    v2 -> v3:
       - Change path of DT binding files.
    v1 -> v2:
       - no update.

Nobuhiro Iwamatsu (4):
  dt-bindings: clock: Add DT bindings for PLL of Toshiba Visconti TMPV770x SoC
  dt-bindings: clock: Add DT bindings for SMU of Toshiba Visconti TMPV770x SoC
  clk: visconti: Add support common clock driver and reset driver
  MAINTAINERS: Add entries for Toshiba Visconti PLL and clock controller

 .../clock/toshiba,tmpv770x-pipllct.yaml       |  57 +++
 .../clock/toshiba,tmpv770x-pismu.yaml         |  50 +++
 MAINTAINERS                                   |   3 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/visconti/Makefile                 |   5 +
 drivers/clk/visconti/clkc-tmpv770x.c          | 232 +++++++++++
 drivers/clk/visconti/clkc.c                   | 220 +++++++++++
 drivers/clk/visconti/clkc.h                   |  75 ++++
 drivers/clk/visconti/pll-tmpv770x.c           |  85 ++++
 drivers/clk/visconti/pll.c                    | 369 ++++++++++++++++++
 drivers/clk/visconti/pll.h                    |  63 +++
 drivers/clk/visconti/reset.c                  | 111 ++++++
 drivers/clk/visconti/reset.h                  |  35 ++
 include/dt-bindings/clock/toshiba,tmpv770x.h  | 181 +++++++++
 include/dt-bindings/reset/toshiba,tmpv770x.h  |  41 ++
 15 files changed, 1528 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pismu.yaml
 create mode 100644 drivers/clk/visconti/Makefile
 create mode 100644 drivers/clk/visconti/clkc-tmpv770x.c
 create mode 100644 drivers/clk/visconti/clkc.c
 create mode 100644 drivers/clk/visconti/clkc.h
 create mode 100644 drivers/clk/visconti/pll-tmpv770x.c
 create mode 100644 drivers/clk/visconti/pll.c
 create mode 100644 drivers/clk/visconti/pll.h
 create mode 100644 drivers/clk/visconti/reset.c
 create mode 100644 drivers/clk/visconti/reset.h
 create mode 100644 include/dt-bindings/clock/toshiba,tmpv770x.h
 create mode 100644 include/dt-bindings/reset/toshiba,tmpv770x.h

-- 
2.32.0

[PATCH v4 1/4] dt-bindings: clock: Add DT bindings for PLL of Toshiba Visconti TMPV770x SoC

[Nobuhiro Iwamatsu]

Add device tree bindings for PLL of Toshiba Visconti TMPV770x SoC series.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
 .../clock/toshiba,tmpv770x-pipllct.yaml       | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml

diff --git a/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
new file mode 100644
index 000000000000..7b7300ce96d6
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/toshiba,tmpv770x-pipllct.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba Visconti5 TMPV770X PLL Controller Device Tree Bindings
+
+maintainers:
+  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+
+description:
+  Toshia Visconti5 PLL controller which supports the PLLs on TMPV770X.
+
+properties:
+  compatible:
+    const: toshiba,tmpv7708-pipllct
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  clocks:
+    description: External reference clock (OSC2)
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+
+    osc2_clk: osc2-clk {
+      compatible = "fixed-clock";
+      clock-frequency = <20000000>;
+      #clock-cells = <0>;
+    };
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pipllct: clock-controller@24220000 {
+            compatible = "toshiba,tmpv7708-pipllct";
+            reg = <0 0x24220000 0 0x820>;
+            #clock-cells = <1>;
+            clocks = <&osc2_clk>;
+        };
+    };
+...
-- 
2.32.0

Re: [PATCH v4 1/4] dt-bindings: clock: Add DT bindings for PLL of Toshiba Visconti TMPV770x SoC

[Rob Herring]

On Wed, 04 Aug 2021 18:22:41 +0900, Nobuhiro Iwamatsu wrote:
> Add device tree bindings for PLL of Toshiba Visconti TMPV770x SoC series.
> 
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> ---
>  .../clock/toshiba,tmpv770x-pipllct.yaml       | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 1/4] dt-bindings: clock: Add DT bindings for PLL of Toshiba Visconti TMPV770x SoC

[Stephen Boyd]

Quoting Nobuhiro Iwamatsu (2021-08-04 02:22:41)
> Add device tree bindings for PLL of Toshiba Visconti TMPV770x SoC series.
> 
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> ---
>  .../clock/toshiba,tmpv770x-pipllct.yaml       | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
> 
> diff --git a/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
> new file mode 100644
> index 000000000000..7b7300ce96d6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/clock/toshiba,tmpv770x-pipllct.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Toshiba Visconti5 TMPV770X PLL Controller Device Tree Bindings
> +
> +maintainers:
> +  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> +
> +description:
> +  Toshia Visconti5 PLL controller which supports the PLLs on TMPV770X.
> +
> +properties:
> +  compatible:
> +    const: toshiba,tmpv7708-pipllct
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  clocks:
> +    description: External reference clock (OSC2)
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#clock-cells"
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +
> +    osc2_clk: osc2-clk {
> +      compatible = "fixed-clock";
> +      clock-frequency = <20000000>;
> +      #clock-cells = <0>;
> +    };
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pipllct: clock-controller@24220000 {
> +            compatible = "toshiba,tmpv7708-pipllct";

The driver makes it look like this is actually part of a syscon node. Is
that right? It's not clear to me that this is a syscon. But then looking
at the binding it seems that one device has been split up into PLL and
"not PLL" parts sort of arbitrarily.

> +            reg = <0 0x24220000 0 0x820>;
> +            #clock-cells = <1>;
> +            clocks = <&osc2_clk>;
> +        };
> +    };
> +...

Re: [PATCH v4 1/4] dt-bindings: clock: Add DT bindings for PLL of Toshiba Visconti TMPV770x SoC

[Nobuhiro Iwamatsu]

Hi,

Thanks for your review.

On Sat, Aug 28, 2021 at 10:09:54PM -0700, Stephen Boyd wrote:
> Quoting Nobuhiro Iwamatsu (2021-08-04 02:22:41)
> > Add device tree bindings for PLL of Toshiba Visconti TMPV770x SoC series.
> > 
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > ---
> >  .../clock/toshiba,tmpv770x-pipllct.yaml       | 57 +++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
> > new file mode 100644
> > index 000000000000..7b7300ce96d6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
> > @@ -0,0 +1,57 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/toshiba,tmpv770x-pipllct.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Toshiba Visconti5 TMPV770X PLL Controller Device Tree Bindings
> > +
> > +maintainers:
> > +  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > +
> > +description:
> > +  Toshia Visconti5 PLL controller which supports the PLLs on TMPV770X.
> > +
> > +properties:
> > +  compatible:
> > +    const: toshiba,tmpv7708-pipllct
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#clock-cells':
> > +    const: 1
> > +
> > +  clocks:
> > +    description: External reference clock (OSC2)
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#clock-cells"
> > +  - clocks
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +
> > +    osc2_clk: osc2-clk {
> > +      compatible = "fixed-clock";
> > +      clock-frequency = <20000000>;
> > +      #clock-cells = <0>;
> > +    };
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        pipllct: clock-controller@24220000 {
> > +            compatible = "toshiba,tmpv7708-pipllct";
> 
> The driver makes it look like this is actually part of a syscon node. Is
> that right? It's not clear to me that this is a syscon. But then looking
> at the binding it seems that one device has been split up into PLL and
> "not PLL" parts sort of arbitrarily.

This is the driver that controls the PIPLLCT device that produces the
PLL. This device only has the ability to generate his PLL, no other
features.

I have received similar comments in the driver patch from you, so I will
check that as well.

> 
> > +            reg = <0 0x24220000 0 0x820>;
> > +            #clock-cells = <1>;
> > +            clocks = <&osc2_clk>;
> > +        };
> > +    };
> > +...
> 

Best regards,
  Nobuhiro

[PATCH v4 2/4] dt-bindings: clock: Add DT bindings for SMU of Toshiba Visconti TMPV770x SoC

[Nobuhiro Iwamatsu]

Add device tree bindings for SMU (System Management Unit) controller of
Toshiba Visconti TMPV770x SoC series.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
 .../clock/toshiba,tmpv770x-pismu.yaml         |  50 +++++
 include/dt-bindings/clock/toshiba,tmpv770x.h  | 181 ++++++++++++++++++
 include/dt-bindings/reset/toshiba,tmpv770x.h  |  41 ++++
 3 files changed, 272 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pismu.yaml
 create mode 100644 include/dt-bindings/clock/toshiba,tmpv770x.h
 create mode 100644 include/dt-bindings/reset/toshiba,tmpv770x.h

diff --git a/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pismu.yaml b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pismu.yaml
new file mode 100644
index 000000000000..845a8d277965
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pismu.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/toshiba,tmpv770x-pismu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba Visconti5 TMPV770x SMU controller Device Tree Bindings
+
+maintainers:
+  - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+
+description:
+  Toshia Visconti5 SMU (System Management Unit) which supports the clock
+  and resets on TMPV770x.
+
+properties:
+  compatible:
+    const: toshiba,tmpv7708-pismu
+
+  reg:
+    maxItems: 1
+
+  '#clock-cells':
+    const: 1
+
+  '#reset-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - "#reset-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pismu: clock-controller@24200000 {
+            compatible = "toshiba,tmpv7708-pismu";
+            reg = <0 0x24200000 0 0x2140>;
+            #clock-cells = <1>;
+            #reset-cells = <1>;
+        };
+    };
+...
diff --git a/include/dt-bindings/clock/toshiba,tmpv770x.h b/include/dt-bindings/clock/toshiba,tmpv770x.h
new file mode 100644
index 000000000000..5fce713001fd
--- /dev/null
+++ b/include/dt-bindings/clock/toshiba,tmpv770x.h
@@ -0,0 +1,181 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_CLOCK_TOSHIBA_TMPV770X_H_
+#define _DT_BINDINGS_CLOCK_TOSHIBA_TMPV770X_H_
+
+/* PLL */
+#define TMPV770X_PLL_PIPLL0		0
+#define TMPV770X_PLL_PIPLL1		1
+#define TMPV770X_PLL_PIDNNPLL		2
+#define TMPV770X_PLL_PIETHERPLL		3
+#define TMPV770X_PLL_PIDDRCPLL		4
+#define TMPV770X_PLL_PIVOIFPLL		5
+#define TMPV770X_PLL_PIIMGERPLL		6
+#define TMPV770X_NR_PLL		7
+
+/* Clocks */
+#define TMPV770X_CLK_PIPLL1_DIV1	0
+#define TMPV770X_CLK_PIPLL1_DIV2	1
+#define TMPV770X_CLK_PIPLL1_DIV4	2
+#define TMPV770X_CLK_PIDNNPLL_DIV1	3
+#define TMPV770X_CLK_DDRC_PHY_PLL0	4
+#define TMPV770X_CLK_DDRC_PHY_PLL1	5
+#define TMPV770X_CLK_D_PHYPLL		6
+#define TMPV770X_CLK_PHY_PCIEPLL	7
+#define TMPV770X_CLK_CA53CL0		8
+#define TMPV770X_CLK_CA53CL1		9
+#define TMPV770X_CLK_PISDMAC		10
+#define TMPV770X_CLK_PIPDMAC0		11
+#define TMPV770X_CLK_PIPDMAC1		12
+#define TMPV770X_CLK_PIWRAM		13
+#define TMPV770X_CLK_DDRC0		14
+#define TMPV770X_CLK_DDRC0_SCLK		15
+#define TMPV770X_CLK_DDRC0_NCLK		16
+#define TMPV770X_CLK_DDRC0_MCLK		17
+#define TMPV770X_CLK_DDRC0_APBCLK	18
+#define TMPV770X_CLK_DDRC1		19
+#define TMPV770X_CLK_DDRC1_SCLK		20
+#define TMPV770X_CLK_DDRC1_NCLK		21
+#define TMPV770X_CLK_DDRC1_MCLK		22
+#define TMPV770X_CLK_DDRC1_APBCLK	23
+#define TMPV770X_CLK_HOX		24
+#define TMPV770X_CLK_PCIE_MSTR		25
+#define TMPV770X_CLK_PCIE_AUX		26
+#define TMPV770X_CLK_PIINTC		27
+#define TMPV770X_CLK_PIETHER_BUS	28
+#define TMPV770X_CLK_PISPI0		29
+#define TMPV770X_CLK_PISPI1		30
+#define TMPV770X_CLK_PISPI2		31
+#define TMPV770X_CLK_PISPI3		32
+#define TMPV770X_CLK_PISPI4		33
+#define TMPV770X_CLK_PISPI5		34
+#define TMPV770X_CLK_PISPI6		35
+#define TMPV770X_CLK_PIUART0		36
+#define TMPV770X_CLK_PIUART1		37
+#define TMPV770X_CLK_PIUART2		38
+#define TMPV770X_CLK_PIUART3		39
+#define TMPV770X_CLK_PII2C0		40
+#define TMPV770X_CLK_PII2C1		41
+#define TMPV770X_CLK_PII2C2		42
+#define TMPV770X_CLK_PII2C3		43
+#define TMPV770X_CLK_PII2C4		44
+#define TMPV770X_CLK_PII2C5		45
+#define TMPV770X_CLK_PII2C6		46
+#define TMPV770X_CLK_PII2C7		47
+#define TMPV770X_CLK_PII2C8		48
+#define TMPV770X_CLK_PIGPIO		49
+#define TMPV770X_CLK_PIPGM		50
+#define TMPV770X_CLK_PIPCMIF		51
+#define TMPV770X_CLK_PIPCMIF_AUDIO_O	52
+#define TMPV770X_CLK_PIPCMIF_AUDIO_I	53
+#define TMPV770X_CLK_PICMPT0		54
+#define TMPV770X_CLK_PICMPT1		55
+#define TMPV770X_CLK_PITSC		56
+#define TMPV770X_CLK_PIUWDT		57
+#define TMPV770X_CLK_PISWDT		58
+#define TMPV770X_CLK_WDTCLK		59
+#define TMPV770X_CLK_PISUBUS_150M	60
+#define TMPV770X_CLK_PISUBUS_300M	61
+#define TMPV770X_CLK_PIPMU		62
+#define TMPV770X_CLK_PIGPMU		63
+#define TMPV770X_CLK_PITMU		64
+#define TMPV770X_CLK_WRCK		65
+#define TMPV770X_CLK_PIEMM		66
+#define TMPV770X_CLK_PIMISC		67
+#define TMPV770X_CLK_PIGCOMM		68
+#define TMPV770X_CLK_PIDCOMM		69
+#define TMPV770X_CLK_PICKMON		70
+#define TMPV770X_CLK_PIMBUS		71
+#define TMPV770X_CLK_SBUSCLK		72
+#define TMPV770X_CLK_DDR0_APBCLKCLK	73
+#define TMPV770X_CLK_DDR1_APBCLKCLK	74
+#define TMPV770X_CLK_DSP0_PBCLK		75
+#define TMPV770X_CLK_DSP1_PBCLK		76
+#define TMPV770X_CLK_DSP2_PBCLK		77
+#define TMPV770X_CLK_DSP3_PBCLK		78
+#define TMPV770X_CLK_DSVIIF0_APBCLK	79
+#define TMPV770X_CLK_VIIF0_APBCLK	80
+#define TMPV770X_CLK_VIIF0_CFGCLK	81
+#define TMPV770X_CLK_VIIF1_APBCLK	82
+#define TMPV770X_CLK_VIIF1_CFGCLK	83
+#define TMPV770X_CLK_VIIF2_APBCLK	84
+#define TMPV770X_CLK_VIIF2_CFGCLK	85
+#define TMPV770X_CLK_VIIF3_APBCLK	86
+#define TMPV770X_CLK_VIIF3_CFGCLK	87
+#define TMPV770X_CLK_VIIF4_APBCLK	88
+#define TMPV770X_CLK_VIIF4_CFGCLK	89
+#define TMPV770X_CLK_VIIF5_APBCLK	90
+#define TMPV770X_CLK_VIIF5_CFGCLK	91
+#define TMPV770X_CLK_VOIF_SBUSCLK	92
+#define TMPV770X_CLK_VOIF_PROCCLK	93
+#define TMPV770X_CLK_VOIF_DPHYCFGCLK	94
+#define TMPV770X_CLK_DNN0		95
+#define TMPV770X_CLK_STMAT		96
+#define TMPV770X_CLK_HWA0		97
+#define TMPV770X_CLK_AFFINE0		98
+#define TMPV770X_CLK_HAMAT		99
+#define TMPV770X_CLK_SMLDB		100
+#define TMPV770X_CLK_HWA0_ASYNC		101
+#define TMPV770X_CLK_HWA2		102
+#define TMPV770X_CLK_FLMAT		103
+#define TMPV770X_CLK_PYRAMID		104
+#define TMPV770X_CLK_HWA2_ASYNC		105
+#define TMPV770X_CLK_DSP0		106
+#define TMPV770X_CLK_VIIFBS0		107
+#define TMPV770X_CLK_VIIFBS0_L2ISP	108
+#define TMPV770X_CLK_VIIFBS0_L1ISP	109
+#define TMPV770X_CLK_VIIFBS0_PROC	110
+#define TMPV770X_CLK_VIIFBS1		111
+#define TMPV770X_CLK_VIIFBS2		112
+#define TMPV770X_CLK_VIIFOP_MBUS	113
+#define TMPV770X_CLK_VIIFOP0_PROC	114
+#define TMPV770X_CLK_PIETHER_2P5M	115
+#define TMPV770X_CLK_PIETHER_25M	116
+#define TMPV770X_CLK_PIETHER_50M	117
+#define TMPV770X_CLK_PIETHER_125M	118
+#define TMPV770X_CLK_VOIF0_DPHYCFG	119
+#define TMPV770X_CLK_VOIF0_PROC		120
+#define TMPV770X_CLK_VOIF0_SBUS		121
+#define TMPV770X_CLK_VOIF0_DSIREF	122
+#define TMPV770X_CLK_VOIF0_PIXEL	123
+#define TMPV770X_CLK_PIREFCLK		124
+#define TMPV770X_CLK_SBUS		125
+#define TMPV770X_CLK_BUSLCK		126
+#define TMPV770X_NR_CLK			127
+
+/* Reset */
+#define TMPV770X_RESET_PIETHER_2P5M	0
+#define TMPV770X_RESET_PIETHER_25M	1
+#define TMPV770X_RESET_PIETHER_50M	2
+#define TMPV770X_RESET_PIETHER_125M	3
+#define TMPV770X_RESET_HOX		4
+#define TMPV770X_RESET_PCIE_MSTR	5
+#define TMPV770X_RESET_PCIE_AUX		6
+#define TMPV770X_RESET_PIINTC		7
+#define TMPV770X_RESET_PIETHER_BUS	8
+#define TMPV770X_RESET_PISPI0		9
+#define TMPV770X_RESET_PISPI1		10
+#define TMPV770X_RESET_PISPI2		11
+#define TMPV770X_RESET_PISPI3		12
+#define TMPV770X_RESET_PISPI4		13
+#define TMPV770X_RESET_PISPI5		14
+#define TMPV770X_RESET_PISPI6		15
+#define TMPV770X_RESET_PIUART0		16
+#define TMPV770X_RESET_PIUART1		17
+#define TMPV770X_RESET_PIUART2		18
+#define TMPV770X_RESET_PIUART3		19
+#define TMPV770X_RESET_PII2C0		20
+#define TMPV770X_RESET_PII2C1		21
+#define TMPV770X_RESET_PII2C2		22
+#define TMPV770X_RESET_PII2C3		23
+#define TMPV770X_RESET_PII2C4		24
+#define TMPV770X_RESET_PII2C5		25
+#define TMPV770X_RESET_PII2C6		26
+#define TMPV770X_RESET_PII2C7		27
+#define TMPV770X_RESET_PII2C8		28
+#define TMPV770X_RESET_PIPCMIF		29
+#define TMPV770X_RESET_PICKMON		30
+#define TMPV770X_RESET_SBUSCLK		31
+#define TMPV770X_NR_RESET		32
+
+#endif /*_DT_BINDINGS_CLOCK_TOSHIBA_TMPV770X_H_ */
diff --git a/include/dt-bindings/reset/toshiba,tmpv770x.h b/include/dt-bindings/reset/toshiba,tmpv770x.h
new file mode 100644
index 000000000000..c1007acb1941
--- /dev/null
+++ b/include/dt-bindings/reset/toshiba,tmpv770x.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+
+#ifndef _DT_BINDINGS_RESET_TOSHIBA_TMPV770X_H_
+#define _DT_BINDINGS_RESET_TOSHIBA_TMPV770X_H_
+
+/* Reset */
+#define TMPV770X_RESET_PIETHER_2P5M	0
+#define TMPV770X_RESET_PIETHER_25M	1
+#define TMPV770X_RESET_PIETHER_50M	2
+#define TMPV770X_RESET_PIETHER_125M	3
+#define TMPV770X_RESET_HOX		4
+#define TMPV770X_RESET_PCIE_MSTR	5
+#define TMPV770X_RESET_PCIE_AUX		6
+#define TMPV770X_RESET_PIINTC		7
+#define TMPV770X_RESET_PIETHER_BUS	8
+#define TMPV770X_RESET_PISPI0		9
+#define TMPV770X_RESET_PISPI1		10
+#define TMPV770X_RESET_PISPI2		11
+#define TMPV770X_RESET_PISPI3		12
+#define TMPV770X_RESET_PISPI4		13
+#define TMPV770X_RESET_PISPI5		14
+#define TMPV770X_RESET_PISPI6		15
+#define TMPV770X_RESET_PIUART0		16
+#define TMPV770X_RESET_PIUART1		17
+#define TMPV770X_RESET_PIUART2		18
+#define TMPV770X_RESET_PIUART3		19
+#define TMPV770X_RESET_PII2C0		20
+#define TMPV770X_RESET_PII2C1		21
+#define TMPV770X_RESET_PII2C2		22
+#define TMPV770X_RESET_PII2C3		23
+#define TMPV770X_RESET_PII2C4		24
+#define TMPV770X_RESET_PII2C5		25
+#define TMPV770X_RESET_PII2C6		26
+#define TMPV770X_RESET_PII2C7		27
+#define TMPV770X_RESET_PII2C8		28
+#define TMPV770X_RESET_PIPCMIF		29
+#define TMPV770X_RESET_PICKMON		30
+#define TMPV770X_RESET_SBUSCLK		31
+#define TMPV770X_NR_RESET		32
+
+#endif /*_DT_BINDINGS_RESET_TOSHIBA_TMPV770X_H_ */
-- 
2.32.0

Re: [PATCH v4 2/4] dt-bindings: clock: Add DT bindings for SMU of Toshiba Visconti TMPV770x SoC

[Rob Herring]

On Wed, 04 Aug 2021 18:22:42 +0900, Nobuhiro Iwamatsu wrote:
> Add device tree bindings for SMU (System Management Unit) controller of
> Toshiba Visconti TMPV770x SoC series.
> 
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> ---
>  .../clock/toshiba,tmpv770x-pismu.yaml         |  50 +++++
>  include/dt-bindings/clock/toshiba,tmpv770x.h  | 181 ++++++++++++++++++
>  include/dt-bindings/reset/toshiba,tmpv770x.h  |  41 ++++
>  3 files changed, 272 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pismu.yaml
>  create mode 100644 include/dt-bindings/clock/toshiba,tmpv770x.h
>  create mode 100644 include/dt-bindings/reset/toshiba,tmpv770x.h
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH v4 3/4] clk: visconti: Add support common clock driver and reset driver

[Nobuhiro Iwamatsu]

Add support for common interface of the common clock and reset driver
for Toshiba Visconti5 and its SoC, TMPV7708. The PIPLLCT provides the PLL,
and the PISMU provides clock and reset functionality.
Each drivers are provided in this patch.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
 drivers/clk/Makefile                 |   1 +
 drivers/clk/visconti/Makefile        |   5 +
 drivers/clk/visconti/clkc-tmpv770x.c | 232 +++++++++++++++++
 drivers/clk/visconti/clkc.c          | 220 ++++++++++++++++
 drivers/clk/visconti/clkc.h          |  75 ++++++
 drivers/clk/visconti/pll-tmpv770x.c  |  85 ++++++
 drivers/clk/visconti/pll.c           | 369 +++++++++++++++++++++++++++
 drivers/clk/visconti/pll.h           |  63 +++++
 drivers/clk/visconti/reset.c         | 111 ++++++++
 drivers/clk/visconti/reset.h         |  35 +++
 10 files changed, 1196 insertions(+)
 create mode 100644 drivers/clk/visconti/Makefile
 create mode 100644 drivers/clk/visconti/clkc-tmpv770x.c
 create mode 100644 drivers/clk/visconti/clkc.c
 create mode 100644 drivers/clk/visconti/clkc.h
 create mode 100644 drivers/clk/visconti/pll-tmpv770x.c
 create mode 100644 drivers/clk/visconti/pll.c
 create mode 100644 drivers/clk/visconti/pll.h
 create mode 100644 drivers/clk/visconti/reset.c
 create mode 100644 drivers/clk/visconti/reset.h

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 5f06879d7fe9..dbf70fdecfc8 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -116,6 +116,7 @@ obj-y					+= ti/
 obj-$(CONFIG_CLK_UNIPHIER)		+= uniphier/
 obj-$(CONFIG_ARCH_U8500)		+= ux500/
 obj-y					+= versatile/
+obj-$(CONFIG_ARCH_VISCONTI)		+= visconti/
 ifeq ($(CONFIG_COMMON_CLK), y)
 obj-$(CONFIG_X86)			+= x86/
 endif
diff --git a/drivers/clk/visconti/Makefile b/drivers/clk/visconti/Makefile
new file mode 100644
index 000000000000..c1254fd52b31
--- /dev/null
+++ b/drivers/clk/visconti/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# Makefile for Toshiba Visconti clock
+
+obj-y	+= clkc.o pll.o reset.o
+obj-y	+= pll-tmpv770x.o clkc-tmpv770x.o
diff --git a/drivers/clk/visconti/clkc-tmpv770x.c b/drivers/clk/visconti/clkc-tmpv770x.c
new file mode 100644
index 000000000000..6ee5c5179f7c
--- /dev/null
+++ b/drivers/clk/visconti/clkc-tmpv770x.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Toshiba Visconti clock controller
+ *
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#include <dt-bindings/clock/toshiba,tmpv770x.h>
+#include <dt-bindings/reset/toshiba,tmpv770x.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+
+#include "clkc.h"
+#include "reset.h"
+
+static DEFINE_SPINLOCK(tmpv770x_clk_lock);
+static DEFINE_SPINLOCK(tmpv770x_rst_lock);
+
+static const struct visconti_fixed_clk fixed_clk_tables[] = {
+	/* PLL1 */
+	/* PICMPT0/1, PITSC, PIUWDT, PISWDT, PISBUS, PIPMU, PIGPMU, PITMU */
+	/* PIEMM, PIMISC, PIGCOMM, PIDCOMM, PIMBUS, PIGPIO, PIPGM */
+	{ TMPV770X_CLK_PIPLL1_DIV4, "pipll1_div4", "pipll1", 0, 1, 4, },
+	/* PISBUS */
+	{ TMPV770X_CLK_PIPLL1_DIV2, "pipll1_div2", "pipll1", 0, 1, 2, },
+	/* PICOBUS_CLK */
+	{ TMPV770X_CLK_PIPLL1_DIV1, "pipll1_div1", "pipll1", 0, 1, 1, },
+	/* PIDNNPLL */
+	/* CONN_CLK, PIMBUS, PICRC0/1 */
+	{ TMPV770X_CLK_PIDNNPLL_DIV1, "pidnnpll_div1", "pidnnpll", 0, 1, 1, },
+	{ TMPV770X_CLK_PIREFCLK, "pirefclk", "osc2-clk", 0, 1, 1, },
+	{ TMPV770X_CLK_WDTCLK, "wdtclk", "osc2-clk", 0, 1, 1, },
+};
+
+static const struct visconti_clk_gate_table pietherpll_clk_gate_tables[] = {
+	/* pietherpll */
+	{ TMPV770X_CLK_PIETHER_2P5M, "piether_2p5m", "pietherpll",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 4, 200,
+		TMPV770X_RESET_PIETHER_2P5M, },
+	{ TMPV770X_CLK_PIETHER_25M, "piether_25m", "pietherpll",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 5, 20,
+		TMPV770X_RESET_PIETHER_25M, },
+	{ TMPV770X_CLK_PIETHER_50M, "piether_50m", "pietherpll",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 6, 10,
+		TMPV770X_RESET_PIETHER_50M, },
+	{ TMPV770X_CLK_PIETHER_125M, "piether_125m", "pietherpll",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 7, 4,
+		TMPV770X_RESET_PIETHER_125M, },
+};
+
+static const struct visconti_clk_gate_table clk_gate_tables[] = {
+	{ TMPV770X_CLK_HOX, "hox", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x4C, 0x14C, 0, 1,
+		TMPV770X_RESET_HOX, },
+	{ TMPV770X_CLK_PCIE_MSTR, "pcie_mstr", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x38, 0x138, 0, 1,
+		TMPV770X_RESET_PCIE_MSTR, },
+	{ TMPV770X_CLK_PCIE_AUX, "pcie_aux", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x38, 0x138, 1, 24,
+		TMPV770X_RESET_PCIE_AUX, },
+	{ TMPV770X_CLK_PIINTC, "piintc", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x8, 0x108, 0, 2,
+		TMPV770X_RESET_PIINTC,},
+	{ TMPV770X_CLK_PIETHER_BUS, "piether_bus", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 0, 2,
+		TMPV770X_RESET_PIETHER_BUS, }, /* BUS_CLK */
+	{ TMPV770X_CLK_PISPI0, "pispi0", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 0, 2,
+		TMPV770X_RESET_PISPI0, },
+	{ TMPV770X_CLK_PISPI1, "pispi1", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 1, 2,
+		TMPV770X_RESET_PISPI1, },
+	{ TMPV770X_CLK_PISPI2, "pispi2", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 2, 2,
+		TMPV770X_RESET_PISPI2, },
+	{ TMPV770X_CLK_PISPI3, "pispi3", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 3, 2,
+		TMPV770X_RESET_PISPI3,},
+	{ TMPV770X_CLK_PISPI4, "pispi4", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 4, 2,
+		TMPV770X_RESET_PISPI4, },
+	{ TMPV770X_CLK_PISPI5, "pispi5", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 5, 2,
+		TMPV770X_RESET_PISPI5},
+	{ TMPV770X_CLK_PISPI6, "pispi6", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 6, 2,
+		TMPV770X_RESET_PISPI6,},
+	{ TMPV770X_CLK_PIUART0, "piuart0", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 0, 4,
+		TMPV770X_RESET_PIUART0,},
+	{ TMPV770X_CLK_PIUART1, "piuart1", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 1, 4,
+		TMPV770X_RESET_PIUART1, },
+	{ TMPV770X_CLK_PIUART2, "piuart2", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 2, 4,
+		TMPV770X_RESET_PIUART2, },
+	{ TMPV770X_CLK_PIUART3, "piuart3", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 3, 4,
+		TMPV770X_RESET_PIUART3, },
+	{ TMPV770X_CLK_PII2C0, "pii2c0", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 0, 4,
+		TMPV770X_RESET_PII2C0, },
+	{ TMPV770X_CLK_PII2C1, "pii2c1", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 1, 4,
+		TMPV770X_RESET_PII2C1, },
+	{ TMPV770X_CLK_PII2C2, "pii2c2", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 2, 4,
+		TMPV770X_RESET_PII2C2, },
+	{ TMPV770X_CLK_PII2C3, "pii2c3", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 3, 4,
+		TMPV770X_RESET_PII2C3,},
+	{ TMPV770X_CLK_PII2C4, "pii2c4", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 4, 4,
+		TMPV770X_RESET_PII2C4, },
+	{ TMPV770X_CLK_PII2C5, "pii2c5", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 5, 4,
+		TMPV770X_RESET_PII2C5, },
+	{ TMPV770X_CLK_PII2C6, "pii2c6", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 6, 4,
+		TMPV770X_RESET_PII2C6, },
+	{ TMPV770X_CLK_PII2C7, "pii2c7", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 7, 4,
+		TMPV770X_RESET_PII2C7, },
+	{ TMPV770X_CLK_PII2C8, "pii2c8", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 8, 4,
+		TMPV770X_RESET_PII2C8, },
+	/* PIPCMIF */
+	{ TMPV770X_CLK_PIPCMIF, "pipcmif", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x64, 0x164, 0, 4,
+		TMPV770X_RESET_PIPCMIF, },
+	/* PISYSTEM */
+	{ TMPV770X_CLK_WRCK, "wrck", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x68, 0x168, 9, 32,
+		-1, }, /* No reset */
+	{ TMPV770X_CLK_PICKMON, "pickmon", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x10, 0x110, 8, 4,
+		TMPV770X_RESET_PICKMON, },
+	{ TMPV770X_CLK_SBUSCLK, "sbusclk", "pipll1",
+		CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x14, 0x114, 0, 4,
+		TMPV770X_RESET_SBUSCLK, },
+};
+
+static const struct visconti_reset_data clk_reset_data[] = {
+	[TMPV770X_RESET_PIETHER_2P5M]	= { 0x434, 0x534, 4, },
+	[TMPV770X_RESET_PIETHER_25M]	= { 0x434, 0x534, 5, },
+	[TMPV770X_RESET_PIETHER_50M]	= { 0x434, 0x534, 6, },
+	[TMPV770X_RESET_PIETHER_125M]	= { 0x434, 0x534, 7, },
+	[TMPV770X_RESET_HOX]		= { 0x44C, 0x54C, 0, },
+	[TMPV770X_RESET_PCIE_MSTR]	= { 0x438, 0x538, 0, },
+	[TMPV770X_RESET_PCIE_AUX]	= { 0x438, 0x538, 1, },
+	[TMPV770X_RESET_PIINTC]		= { 0x408, 0x508, 0, },
+	[TMPV770X_RESET_PIETHER_BUS]	= { 0x434, 0x534, 0, },
+	[TMPV770X_RESET_PISPI0]		= { 0x428, 0x528, 0, },
+	[TMPV770X_RESET_PISPI1]		= { 0x428, 0x528, 1, },
+	[TMPV770X_RESET_PISPI2]		= { 0x428, 0x528, 2, },
+	[TMPV770X_RESET_PISPI3]		= { 0x428, 0x528, 3, },
+	[TMPV770X_RESET_PISPI4]		= { 0x428, 0x528, 4, },
+	[TMPV770X_RESET_PISPI5]		= { 0x428, 0x528, 5, },
+	[TMPV770X_RESET_PISPI6]		= { 0x428, 0x528, 6, },
+	[TMPV770X_RESET_PIUART0]	= { 0x42C, 0x52C, 0, },
+	[TMPV770X_RESET_PIUART1]	= { 0x42C, 0x52C, 1, },
+	[TMPV770X_RESET_PIUART2]	= { 0x42C, 0x52C, 2, },
+	[TMPV770X_RESET_PIUART3]	= { 0x42C, 0x52C, 3, },
+	[TMPV770X_RESET_PII2C0]		= { 0x430, 0x530, 0, },
+	[TMPV770X_RESET_PII2C1]		= { 0x430, 0x530, 1, },
+	[TMPV770X_RESET_PII2C2]		= { 0x430, 0x530, 2, },
+	[TMPV770X_RESET_PII2C3]		= { 0x430, 0x530, 3, },
+	[TMPV770X_RESET_PII2C4]		= { 0x430, 0x530, 4, },
+	[TMPV770X_RESET_PII2C5]		= { 0x430, 0x530, 5, },
+	[TMPV770X_RESET_PII2C6]		= { 0x430, 0x530, 6, },
+	[TMPV770X_RESET_PII2C7]		= { 0x430, 0x530, 7, },
+	[TMPV770X_RESET_PII2C8]		= { 0x430, 0x530, 8, },
+	[TMPV770X_RESET_PIPCMIF]	= { 0x464, 0x564, 0, },
+	[TMPV770X_RESET_PICKMON]	= { 0x410, 0x510, 8, },
+	[TMPV770X_RESET_SBUSCLK]	= { 0x414, 0x514, 0, },
+};
+
+static void __init tmpv770x_clkc_setup_clks(struct device_node *np)
+{
+	struct visconti_clk_provider *ctx;
+	struct regmap *regmap;
+	int ret, i;
+
+	regmap = device_node_to_regmap(np);
+	if (IS_ERR(regmap))
+		return;
+
+	ctx = visconti_init_clk(np, regmap, TMPV770X_NR_CLK);
+	if (IS_ERR(ctx))
+		return;
+
+	ret = visconti_register_reset_controller(np, regmap, clk_reset_data,
+						 TMPV770X_NR_RESET,
+						 &visconti_reset_ops,
+						 &tmpv770x_rst_lock);
+	if (ret) {
+		pr_err("Failed to register reset controller: %d\n", ret);
+		return;
+	}
+
+	for (i = 0; i < (ARRAY_SIZE(fixed_clk_tables)); i++)
+		ctx->clk_data.clks[fixed_clk_tables[i].id] =
+			clk_register_fixed_factor(NULL,
+						fixed_clk_tables[i].name,
+						fixed_clk_tables[i].parent,
+						fixed_clk_tables[i].flag,
+						fixed_clk_tables[i].mult,
+						fixed_clk_tables[i].div);
+
+	ret = visconti_clk_register_gates(ctx, clk_gate_tables,
+				    ARRAY_SIZE(clk_gate_tables), clk_reset_data,
+				    &tmpv770x_clk_lock);
+	if (ret) {
+		pr_err("Failed to register main clock gate: %d\n", ret);
+		return;
+	}
+
+	ret = visconti_clk_register_gates(ctx, pietherpll_clk_gate_tables,
+				    ARRAY_SIZE(pietherpll_clk_gate_tables),
+				    clk_reset_data, &tmpv770x_clk_lock);
+	if (ret) {
+		pr_err("Failed to register pietherpll clock gate: %d\n", ret);
+		return;
+	}
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
+}
+
+CLK_OF_DECLARE_DRIVER(tmpv770x_clkc, "toshiba,tmpv7708-pismu", tmpv770x_clkc_setup_clks);
diff --git a/drivers/clk/visconti/clkc.c b/drivers/clk/visconti/clkc.c
new file mode 100644
index 000000000000..32b6f6266154
--- /dev/null
+++ b/drivers/clk/visconti/clkc.c
@@ -0,0 +1,220 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Toshiba Visconti clock controller
+ *
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include "clkc.h"
+
+static inline struct visconti_clk_gate *to_visconti_clk_gate(struct clk_hw *hw)
+{
+	return container_of(hw, struct visconti_clk_gate, hw);
+}
+
+static int visconti_gate_clk_is_enabled(struct clk_hw *hw)
+{
+	struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
+	u32 clk = BIT(gate->ck_idx);
+	u32 val;
+
+	regmap_read(gate->regmap, gate->ckon_offset, &val);
+	return (val & clk) ? 1 : 0;
+}
+
+static void visconti_gate_clk_disable(struct clk_hw *hw)
+{
+	struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
+	u32 clk = BIT(gate->ck_idx);
+	u32 rst = BIT(gate->rs_idx);
+	unsigned long flags;
+
+	spin_lock_irqsave(gate->lock, flags);
+
+	if (visconti_gate_clk_is_enabled(hw)) {
+		spin_unlock_irqrestore(gate->lock, flags);
+		return;
+	}
+
+	/* Reset release */
+	regmap_update_bits(gate->regmap, gate->rson_offset, rst, 1);
+
+	udelay(100);
+
+	/* Disable clock */
+	regmap_update_bits(gate->regmap, gate->ckoff_offset, clk, 1);
+	spin_unlock_irqrestore(gate->lock, flags);
+}
+
+static int visconti_gate_clk_enable(struct clk_hw *hw)
+{
+	struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
+	u32 clk = BIT(gate->ck_idx);
+	u32 rst = BIT(gate->rs_idx);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(gate->lock, flags);
+
+	if (visconti_gate_clk_is_enabled(hw)) {
+		spin_unlock_irqrestore(gate->lock, flags);
+		return 0;
+	}
+
+	regmap_update_bits(gate->regmap, gate->ckon_offset, clk, 1);
+	/* Need read back */
+	regmap_read(gate->regmap, gate->ckon_offset, &val);
+
+	udelay(100);
+	/* Reset release */
+	regmap_update_bits(gate->regmap, gate->rsoff_offset, rst, 1);
+	/* Need read back */
+	regmap_read(gate->regmap, gate->ckoff_offset, &val);
+	spin_unlock_irqrestore(gate->lock, flags);
+
+	return 0;
+}
+
+static const struct clk_ops visconti_clk_gate_ops = {
+	.enable = visconti_gate_clk_enable,
+	.disable = visconti_gate_clk_disable,
+	.is_enabled = visconti_gate_clk_is_enabled,
+};
+
+static struct clk_hw *visconti_clk_register_gate(struct device *dev,
+						 const char *name,
+						 const char *parent_name,
+						 struct regmap *regmap,
+						 const struct visconti_clk_gate_table *clks,
+						 u32	rson_offset,
+						 u32	rsoff_offset,
+						 u8	rs_idx,
+						 spinlock_t *lock)
+{
+	struct visconti_clk_gate *gate;
+	struct clk_init_data init;
+	struct clk_hw *hw;
+	int ret;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &visconti_clk_gate_ops;
+	init.flags = clks->flags;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	init.num_parents = parent_name ? 1 : 0;
+
+	gate->regmap = regmap;
+	gate->ckon_offset = clks->ckon_offset;
+	gate->ckoff_offset = clks->ckoff_offset;
+	gate->ck_idx = clks->ck_idx;
+	gate->rson_offset = rson_offset;
+	gate->rsoff_offset = rsoff_offset;
+	gate->rs_idx = rs_idx;
+	gate->lock = lock;
+	gate->hw.init = &init;
+
+	hw = &gate->hw;
+	ret = clk_hw_register(dev, hw);
+	if (ret) {
+		kfree(gate);
+		hw = ERR_PTR(ret);
+	}
+
+	return hw;
+}
+
+int visconti_clk_register_gates(struct visconti_clk_provider *data,
+				 const struct visconti_clk_gate_table *clks,
+				 int num_gate, const struct visconti_reset_data *reset,
+				 spinlock_t *lock)
+{
+	u32 rson_offset, rsoff_offset;
+	struct clk_hw *hw_clk;
+	u8 rs_idx;
+	int i;
+
+	for (i = 0; i < num_gate; i++) {
+		struct clk *clk;
+		char *div_name;
+
+		div_name = kasprintf(GFP_KERNEL, "%s_div", clks[i].name);
+		if (!div_name)
+			return -ENOMEM;
+
+		if (clks[i].rs_id >= 0) {
+			rson_offset = reset[clks[i].rs_id].rson_offset;
+			rsoff_offset = reset[clks[i].rs_id].rsoff_offset;
+			rs_idx = reset[clks[i].rs_id].rs_idx;
+		} else {
+			rson_offset = rsoff_offset = rs_idx = -1;
+		}
+
+		clk = clk_register_fixed_factor(NULL, div_name, clks[i].parent,
+						0, 1, clks[i].div);
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+
+		hw_clk = visconti_clk_register_gate(NULL,
+						clks[i].name,
+						div_name,
+						data->regmap,
+						&clks[i],
+						rson_offset,
+						rsoff_offset,
+						rs_idx,
+						lock);
+		kfree(div_name);
+		if (IS_ERR(hw_clk)) {
+			pr_err("%s: failed to register clock %s\n",
+			       __func__, clks[i].name);
+			return PTR_ERR(hw_clk);
+		}
+
+		data->clk_data.clks[clks[i].id] = hw_clk->clk;
+	}
+
+	return 0;
+}
+
+struct visconti_clk_provider *visconti_init_clk(struct device_node *np,
+						struct regmap *regmap,
+						unsigned long nr_clks)
+{
+	struct visconti_clk_provider *ctx;
+	struct clk **clk_table;
+	int i;
+
+	ctx = kzalloc(sizeof(struct visconti_clk_provider), GFP_KERNEL);
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	clk_table = kcalloc(nr_clks, sizeof(struct clk *), GFP_KERNEL);
+	if (!clk_table)
+		goto err;
+
+	for (i = 0; i < nr_clks; ++i)
+		clk_table[i] = ERR_PTR(-ENOENT);
+	ctx->node = np;
+	ctx->regmap = regmap;
+	ctx->clk_data.clks = clk_table;
+	ctx->clk_data.clk_num = nr_clks;
+
+	return ctx;
+err:
+	kfree(ctx);
+	return ERR_PTR(-ENOMEM);
+}
diff --git a/drivers/clk/visconti/clkc.h b/drivers/clk/visconti/clkc.h
new file mode 100644
index 000000000000..4df8788d635a
--- /dev/null
+++ b/drivers/clk/visconti/clkc.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Toshiba Visconti clock controller
+ *
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#ifndef _VISCONTI_CLKC_H_
+#define _VISCONTI_CLKC_H_
+
+#include <linux/mfd/syscon.h>
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/io.h>
+#include <linux/spinlock.h>
+
+#include "reset.h"
+
+struct visconti_clk_provider {
+	struct device_node	*node;
+	struct regmap		*regmap;
+	struct clk_onecell_data clk_data;
+};
+
+struct visconti_clk_gate_table {
+	unsigned int	id;
+	const char	*name;
+	const char	*parent;
+	u8		flags;
+	u32		ckon_offset;
+	u32		ckoff_offset;
+	u8		ck_idx;
+	unsigned int	div;
+	u8		rs_id;
+};
+
+struct visconti_fixed_clk {
+	unsigned int	id;
+	const char	*name;
+	const char	*parent;
+	unsigned long	flag;
+	unsigned int	mult;
+	unsigned int	div;
+};
+
+struct visconti_clk_gate {
+	struct clk_hw	hw;
+	struct regmap	*regmap;
+	u32		ckon_offset;
+	u32		ckoff_offset;
+	u8		ck_idx;
+	u8		flags;
+	u32		rson_offset;
+	u32		rsoff_offset;
+	u8		rs_idx;
+	spinlock_t	*lock;
+};
+
+struct visconti_clk_provider *visconti_init_clk(struct device_node *np,
+						struct regmap *regmap,
+						unsigned long nr_clks);
+int visconti_clk_register_gates(struct visconti_clk_provider *data,
+				 const struct visconti_clk_gate_table *clks,
+				 int num_gate,
+				 const struct visconti_reset_data *reset,
+				 spinlock_t *lock);
+#endif /* _VISCONTI_CLKC_H_ */
diff --git a/drivers/clk/visconti/pll-tmpv770x.c b/drivers/clk/visconti/pll-tmpv770x.c
new file mode 100644
index 000000000000..a0874cb6a67b
--- /dev/null
+++ b/drivers/clk/visconti/pll-tmpv770x.c
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Toshiba Visconti PLL controller
+ *
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#include <dt-bindings/clock/toshiba,tmpv770x.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include "pll.h"
+
+static DEFINE_SPINLOCK(tmpv770x_pll_lock);
+
+static const struct visconti_pll_rate_table pipll0_rates[] __initconst = {
+	VISCONTI_PLL_RATE(840000000, 0x1, 0x0, 0x1, 0x54, 0x000000, 0x2, 0x1),
+	VISCONTI_PLL_RATE(780000000, 0x1, 0x0, 0x1, 0x4E, 0x000000, 0x2, 0x1),
+	VISCONTI_PLL_RATE(600000000, 0x1, 0x0, 0x1, 0x3C, 0x000000, 0x2, 0x1),
+	{ /* sentinel */ },
+};
+
+static const struct visconti_pll_rate_table piddrcpll_rates[] __initconst = {
+	VISCONTI_PLL_RATE(780000000, 0x1, 0x0, 0x1, 0x4E, 0x000000, 0x2, 0x1),
+	VISCONTI_PLL_RATE(760000000, 0x1, 0x0, 0x1, 0x4C, 0x000000, 0x2, 0x1),
+	{ /* sentinel */ },
+};
+
+static const struct visconti_pll_rate_table pivoifpll_rates[] __initconst = {
+	VISCONTI_PLL_RATE(165000000, 0x1, 0x0, 0x1, 0x42, 0x000000, 0x4, 0x2),
+	VISCONTI_PLL_RATE(148500000, 0x1, 0x1, 0x1, 0x3B, 0x666666, 0x4, 0x2),
+	VISCONTI_PLL_RATE(96000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x2),
+	VISCONTI_PLL_RATE(74250000, 0x1, 0x1, 0x1, 0x3B, 0x666666, 0x4, 0x4),
+	VISCONTI_PLL_RATE(54000000, 0x1, 0x0, 0x1, 0x36, 0x000000, 0x5, 0x4),
+	VISCONTI_PLL_RATE(48000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x4),
+	VISCONTI_PLL_RATE(35750000, 0x1, 0x1, 0x1, 0x32, 0x0CCCCC, 0x7, 0x4),
+	{ /* sentinel */ },
+};
+
+static const struct visconti_pll_rate_table piimgerpll_rates[] __initconst = {
+	VISCONTI_PLL_RATE(165000000, 0x1, 0x0, 0x1, 0x42, 0x000000, 0x4, 0x2),
+	VISCONTI_PLL_RATE(96000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x2),
+	VISCONTI_PLL_RATE(54000000, 0x1, 0x0, 0x1, 0x36, 0x000000, 0x5, 0x4),
+	VISCONTI_PLL_RATE(48000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x4),
+	{ /* sentinel */ },
+};
+
+static const struct visconti_pll_info pll_info[] __initconst = {
+	{ TMPV770X_PLL_PIPLL0, "pipll0", "osc2-clk", 0, 0x0, pipll0_rates },
+	{ TMPV770X_PLL_PIDDRCPLL, "piddrcpll", "osc2-clk", 0, 0x500, piddrcpll_rates },
+	{ TMPV770X_PLL_PIVOIFPLL, "pivoifpll", "osc2-clk", 0, 0x600, pivoifpll_rates },
+	{ TMPV770X_PLL_PIIMGERPLL, "piimgerpll", "osc2-clk", 0, 0x700, piimgerpll_rates },
+};
+
+static void __init tmpv770x_setup_plls(struct device_node *np)
+{
+	struct visconti_pll_provider *ctx;
+	void __iomem *reg_base;
+
+	reg_base = of_iomap(np, 0);
+	if (!reg_base)
+		return;
+
+	ctx = visconti_init_pll(np, reg_base, TMPV770X_NR_PLL);
+	if (IS_ERR(ctx)) {
+		iounmap(reg_base);
+		return;
+	}
+
+	ctx->clk_data.clks[TMPV770X_PLL_PIPLL1] =
+		clk_register_fixed_rate(NULL, "pipll1", NULL, 0, 600000000);
+	ctx->clk_data.clks[TMPV770X_PLL_PIDNNPLL] =
+		clk_register_fixed_rate(NULL, "pidnnpll", NULL, 0, 500000000);
+	ctx->clk_data.clks[TMPV770X_PLL_PIETHERPLL] =
+		clk_register_fixed_rate(NULL, "pietherpll", NULL, 0, 500000000);
+
+	visconti_register_plls(ctx, pll_info, ARRAY_SIZE(pll_info), &tmpv770x_pll_lock);
+}
+
+CLK_OF_DECLARE_DRIVER(tmpv770x_plls, "toshiba,tmpv7708-pipllct", tmpv770x_setup_plls);
diff --git a/drivers/clk/visconti/pll.c b/drivers/clk/visconti/pll.c
new file mode 100644
index 000000000000..7baa9512fad9
--- /dev/null
+++ b/drivers/clk/visconti/pll.c
@@ -0,0 +1,369 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Toshiba Visconti PLL driver
+ *
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+
+#include "pll.h"
+
+struct visconti_pll {
+	struct clk_hw	hw;
+	void __iomem	*pll_base;
+	spinlock_t	*lock;
+	int rate_count;
+	unsigned long flags;
+	const struct visconti_pll_rate_table *rate_table;
+	struct visconti_pll_provider *ctx;
+};
+
+#define PLL_CONF_REG		0x0000
+#define PLL_CTRL_REG		0x0004
+#define PLL_FRACMODE_REG	0x0010
+#define PLL_INTIN_REG		0x0014
+#define PLL_FRACIN_REG		0x0018
+#define PLL_REFDIV_REG		0x001C
+#define PLL_POSTDIV_REG		0x0020
+
+#define PLL_CONFIG_SEL		BIT(0)
+#define PLL_PLLEN		BIT(4)
+#define PLL_BYPASS		BIT(16)
+#define PLL_INTIN_MASK		GENMASK(11, 0)
+#define PLL_FRACIN_MASK		GENMASK(23, 0)
+#define PLL_REFDIV_MASK		GENMASK(5, 0)
+#define PLL_POSTDIV_MASK	GENMASK(2, 0)
+
+static inline struct visconti_pll *to_visconti_pll(struct clk_hw *hw)
+{
+	return container_of(hw, struct visconti_pll, hw);
+}
+
+static void visconti_pll_get_params(struct visconti_pll *pll,
+				    struct visconti_pll_rate_table *rate_table)
+{
+	u32 postdiv, val;
+
+	val = readl(pll->pll_base + PLL_FRACMODE_REG);
+	rate_table->dacen = (val >> 4) & 0x1;
+	rate_table->dsmen = val & 0x1;
+	rate_table->fracin = readl(pll->pll_base + PLL_FRACIN_REG) & PLL_FRACIN_MASK;
+	rate_table->intin = readl(pll->pll_base + PLL_INTIN_REG) & PLL_INTIN_MASK;
+	rate_table->refdiv = readl(pll->pll_base + PLL_REFDIV_REG) & PLL_REFDIV_MASK;
+
+	postdiv = readl(pll->pll_base + PLL_POSTDIV_REG);
+	rate_table->postdiv1 = postdiv & PLL_POSTDIV_MASK;
+	rate_table->postdiv2 = (postdiv >> 4) & PLL_POSTDIV_MASK;
+}
+
+static const struct visconti_pll_rate_table *visconti_get_pll_settings(struct visconti_pll *pll,
+								       unsigned long rate)
+{
+	const struct visconti_pll_rate_table *rate_table = pll->rate_table;
+	int i;
+
+	for (i = 0; i < pll->rate_count; i++) {
+		if (rate == rate_table[i].rate)
+			return &rate_table[i];
+	}
+
+	return NULL;
+}
+
+static unsigned long visconti_get_pll_rate_from_data(struct visconti_pll *pll,
+						     struct visconti_pll_rate_table *rate)
+{
+	const struct visconti_pll_rate_table *rate_table = pll->rate_table;
+	int i;
+
+	for (i = 0; i < pll->rate_count; i++) {
+		if (rate_table[i].dacen == rate->dacen &&
+			rate_table[i].dsmen == rate->dsmen &&
+			rate_table[i].fracin == rate->fracin &&
+			rate_table[i].intin == rate->intin &&
+			rate_table[i].refdiv == rate->refdiv &&
+			rate_table[i].postdiv1 == rate->postdiv1 &&
+			rate_table[i].postdiv2 == rate->postdiv2)
+			return rate_table[i].rate;
+	}
+
+	/* set default */
+	return rate_table[0].rate;
+}
+
+static long visconti_pll_round_rate(struct clk_hw *hw,
+			    unsigned long rate, unsigned long *prate)
+{
+	struct visconti_pll *pll = to_visconti_pll(hw);
+	const struct visconti_pll_rate_table *rate_table = pll->rate_table;
+	int i;
+
+	/* Assumming rate_table is in descending order */
+	for (i = 0; i < pll->rate_count; i++) {
+		if (rate >= rate_table[i].rate)
+			return rate_table[i].rate;
+	}
+
+	/* return minimum supported value */
+	return rate_table[i - 1].rate;
+}
+
+static unsigned long visconti_pll_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent_rate)
+{
+	struct visconti_pll *pll = to_visconti_pll(hw);
+	struct visconti_pll_rate_table rate_table;
+
+	memset(&rate_table, 0, sizeof(rate_table));
+	visconti_pll_get_params(pll, &rate_table);
+
+	return visconti_get_pll_rate_from_data(pll, &rate_table);
+}
+
+static int visconti_pll_set_params(struct visconti_pll *pll,
+				const struct visconti_pll_rate_table *rate_table)
+{
+	/* update pll values */
+	writel(((rate_table->dacen << 4) | rate_table->dsmen),
+			pll->pll_base + PLL_FRACMODE_REG);
+	writel(((rate_table->postdiv2 << 4) | rate_table->postdiv1),
+			pll->pll_base + PLL_POSTDIV_REG);
+	writel(rate_table->intin, pll->pll_base + PLL_INTIN_REG);
+	writel(rate_table->fracin, pll->pll_base + PLL_FRACIN_REG);
+	writel(rate_table->refdiv, pll->pll_base + PLL_REFDIV_REG);
+
+	return 0;
+}
+
+static int visconti_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+			    unsigned long parent_rate)
+{
+	struct visconti_pll *pll = to_visconti_pll(hw);
+	const struct visconti_pll_rate_table *rate_table;
+
+	/* Get required rate settings from table */
+	rate_table = visconti_get_pll_settings(pll, rate);
+	if (!rate_table) {
+		pr_err("Invalid rate : %lu for pll clk %s\n",
+			parent_rate, __clk_get_name(hw->clk));
+		return -EINVAL;
+	}
+
+	return visconti_pll_set_params(pll, rate_table);
+}
+
+static int visconti_pll_is_enabled(struct clk_hw *hw)
+{
+	struct visconti_pll *pll = to_visconti_pll(hw);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(pll->lock, flags);
+	reg = readl(pll->pll_base + PLL_CTRL_REG);
+	spin_unlock_irqrestore(pll->lock, flags);
+
+	return (reg & PLL_PLLEN);
+}
+
+static int visconti_pll_enable(struct clk_hw *hw)
+{
+	struct visconti_pll *pll = to_visconti_pll(hw);
+	const struct visconti_pll_rate_table *rate_table = pll->rate_table;
+	unsigned long flags;
+	u32 reg;
+
+	if (visconti_pll_is_enabled(hw))
+		return 0;
+
+	spin_lock_irqsave(pll->lock, flags);
+
+	/* Change to access via register */
+	writel(PLL_CONFIG_SEL, pll->pll_base + PLL_CONF_REG);
+
+	/* Change to BYPASS mode */
+	reg = readl(pll->pll_base + PLL_CTRL_REG);
+	reg |= PLL_BYPASS;
+	writel(reg, pll->pll_base + PLL_CTRL_REG);
+
+	/* Set DIV register .....*/
+	visconti_pll_set_params(pll, &rate_table[0]);
+
+	/* Disable  PLL register */
+	reg = readl(pll->pll_base + PLL_CTRL_REG);
+	reg &= ~PLL_PLLEN;
+	writel(reg, pll->pll_base + PLL_CTRL_REG);
+
+	/* Wait 1us */
+	udelay(1);
+
+	/* Enable PLL register */
+	reg = readl(pll->pll_base + PLL_CTRL_REG);
+	reg |= PLL_PLLEN;
+	writel(reg, pll->pll_base + PLL_CTRL_REG);
+
+	/* Wait 40us */
+	udelay(40);
+
+	/* Change to PLL mode */
+	reg = readl(pll->pll_base + PLL_CTRL_REG);
+	reg &= ~PLL_BYPASS;
+	writel(reg, pll->pll_base + PLL_CTRL_REG);
+
+	spin_unlock_irqrestore(pll->lock, flags);
+
+	return 0;
+}
+
+static void visconti_pll_disable(struct clk_hw *hw)
+{
+	struct visconti_pll *pll = to_visconti_pll(hw);
+	unsigned long flags;
+	u32 reg;
+
+	if (!visconti_pll_is_enabled(hw))
+		return;
+
+	spin_lock_irqsave(pll->lock, flags);
+
+	/* Change to access via register */
+	writel(PLL_CONFIG_SEL, pll->pll_base + PLL_CONF_REG);
+
+	/* Change to BYPASS mode */
+	reg = readl(pll->pll_base + PLL_CTRL_REG);
+	reg |= PLL_BYPASS;
+	writel(reg, pll->pll_base + PLL_CTRL_REG);
+
+	/* PLL disable */
+	reg = readl(pll->pll_base + PLL_CTRL_REG);
+	reg &= ~PLL_PLLEN;
+	writel(reg, pll->pll_base + PLL_CTRL_REG);
+
+	spin_unlock_irqrestore(pll->lock, flags);
+}
+
+static const struct clk_ops visconti_pll_ops = {
+	.enable = visconti_pll_enable,
+	.disable = visconti_pll_disable,
+	.is_enabled = visconti_pll_is_enabled,
+	.round_rate = visconti_pll_round_rate,
+	.recalc_rate = visconti_pll_recalc_rate,
+	.set_rate = visconti_pll_set_rate,
+};
+
+static struct clk *visconti_register_pll(struct visconti_pll_provider *ctx,
+					 const char *name,
+					 const char *parent_name,
+					 int offset,
+					 const struct visconti_pll_rate_table *rate_table,
+					 u8 clk_pll_flags,
+					 spinlock_t *lock)
+{
+	struct clk_init_data init;
+	struct visconti_pll *pll;
+	struct clk *pll_clk;
+	int len;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (!pll)
+		return ERR_PTR(-ENOMEM);
+
+	/* Create the actual pll */
+	init.name = name;
+	init.flags = CLK_IGNORE_UNUSED;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	for (len = 0; rate_table[len].rate != 0; )
+		len++;
+	pll->rate_count = len;
+	pll->rate_table = kmemdup(rate_table,
+				  pll->rate_count * sizeof(struct visconti_pll_rate_table),
+				  GFP_KERNEL);
+	WARN(!pll->rate_table, "%s: could not allocate rate table for %s\n", __func__, name);
+
+	init.ops = &visconti_pll_ops;
+	pll->hw.init = &init;
+	pll->pll_base = ctx->reg_base + offset;
+	pll->flags = clk_pll_flags;
+	pll->lock = lock;
+	pll->ctx = ctx;
+
+	pll_clk = clk_register(NULL, &pll->hw);
+	if (IS_ERR(pll_clk)) {
+		pr_err("failed to register pll clock %s : %ld\n", name, PTR_ERR(pll_clk));
+		kfree(pll);
+	}
+
+	return pll_clk;
+}
+
+static void visconti_pll_add_lookup(struct visconti_pll_provider *ctx, struct clk *clk,
+				    unsigned int id)
+{
+	if (ctx->clk_data.clks && id)
+		ctx->clk_data.clks[id] = clk;
+}
+
+void __init visconti_register_plls(struct visconti_pll_provider *ctx,
+				   const struct visconti_pll_info *list,
+				   unsigned int nr_plls,
+				   spinlock_t *lock)
+{
+	int idx;
+
+	for (idx = 0; idx < nr_plls; idx++, list++) {
+		struct clk *clk;
+
+		clk = visconti_register_pll(ctx,
+					    list->name,
+					    list->parent,
+					    list->base_reg,
+					    list->rate_table,
+					    list->flags,
+					    lock);
+		if (IS_ERR(clk)) {
+			pr_err("failed to register clock %s\n", list->name);
+			continue;
+		}
+
+		visconti_pll_add_lookup(ctx, clk, list->id);
+	}
+}
+
+struct visconti_pll_provider * __init visconti_init_pll(struct device_node *np,
+							void __iomem *base,
+							unsigned long nr_plls)
+{
+	struct visconti_pll_provider *ctx;
+	struct clk **clk_table;
+	int i;
+
+	ctx = kzalloc(sizeof(struct visconti_pll_provider), GFP_KERNEL);
+	if (!ctx)
+		return ERR_PTR(-ENOMEM);
+
+	clk_table = kcalloc(nr_plls, sizeof(struct clk *), GFP_KERNEL);
+	if (!clk_table)
+		goto err;
+
+	for (i = 0; i < nr_plls; ++i)
+		clk_table[i] = ERR_PTR(-ENOENT);
+
+	ctx->node = np;
+	ctx->reg_base = base;
+	ctx->clk_data.clks = clk_table;
+	ctx->clk_data.clk_num = nr_plls;
+
+	return ctx;
+
+err:
+	kfree(ctx);
+	return ERR_PTR(-ENOMEM);
+}
diff --git a/drivers/clk/visconti/pll.h b/drivers/clk/visconti/pll.h
new file mode 100644
index 000000000000..ce0ce1d8cd88
--- /dev/null
+++ b/drivers/clk/visconti/pll.h
@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#ifndef _VISCONTI_PLL_H_
+#define _VISCONTI_PLL_H_
+
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+#include <linux/spinlock.h>
+
+struct visconti_pll_provider {
+	void __iomem *reg_base;
+	struct regmap *regmap;
+	struct clk_onecell_data clk_data;
+	struct device_node *node;
+};
+
+#define VISCONTI_PLL_RATE(_rate, _dacen, _dsmen, \
+	_refdiv, _intin, _fracin, _postdiv1, _postdiv2) \
+{				\
+	.rate = _rate,		\
+	.dacen = _dacen,	\
+	.dsmen = _dsmen,	\
+	.refdiv = _refdiv,	\
+	.intin = _intin,	\
+	.fracin = _fracin,	\
+	.postdiv1 = _postdiv1,	\
+	.postdiv2 = _postdiv2	\
+}
+
+struct visconti_pll_rate_table {
+	unsigned long rate;
+	unsigned int dacen;
+	unsigned int dsmen;
+	unsigned int refdiv;
+	unsigned long intin;
+	unsigned long fracin;
+	unsigned int postdiv1;
+	unsigned int postdiv2;
+};
+
+struct visconti_pll_info {
+	unsigned int id;
+	const char *name;
+	const char *parent;
+	u8 flags;
+	unsigned long base_reg;
+	const struct visconti_pll_rate_table *rate_table;
+};
+
+struct visconti_pll_provider * __init visconti_init_pll(struct device_node *np,
+							void __iomem *base,
+							unsigned long nr_plls);
+void __init visconti_register_plls(struct visconti_pll_provider *ctx,
+				   const struct visconti_pll_info *list,
+				   unsigned int nr_plls, spinlock_t *lock);
+
+#endif /* _VISCONTI_PLL_H_ */
diff --git a/drivers/clk/visconti/reset.c b/drivers/clk/visconti/reset.c
new file mode 100644
index 000000000000..cfe8fde5f749
--- /dev/null
+++ b/drivers/clk/visconti/reset.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Toshiba Visconti ARM SoC reset controller
+ *
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+#include <linux/delay.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include "reset.h"
+
+static inline struct visconti_reset *to_visconti_reset(struct reset_controller_dev *rcdev)
+{
+	return container_of(rcdev, struct visconti_reset, rcdev);
+}
+
+static int visconti_reset_assert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct visconti_reset *reset = to_visconti_reset(rcdev);
+	const struct visconti_reset_data *data = &reset->resets[id];
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(reset->lock, flags);
+	ret = regmap_update_bits(reset->regmap, data->rson_offset, data->rs_idx, 1);
+	spin_unlock_irqrestore(reset->lock, flags);
+
+	return ret;
+}
+
+static int visconti_reset_deassert(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct visconti_reset *reset = to_visconti_reset(rcdev);
+	const struct visconti_reset_data *data = &reset->resets[id];
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(reset->lock, flags);
+	ret = regmap_update_bits(reset->regmap, data->rsoff_offset, data->rs_idx, 1);
+	spin_unlock_irqrestore(reset->lock, flags);
+
+	return ret;
+}
+
+static int visconti_reset_reset(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	visconti_reset_assert(rcdev, id);
+	udelay(1);
+	visconti_reset_deassert(rcdev, id);
+
+	return 0;
+}
+
+static int visconti_reset_status(struct reset_controller_dev *rcdev, unsigned long id)
+{
+	struct visconti_reset *reset = to_visconti_reset(rcdev);
+	const struct visconti_reset_data *data = &reset->resets[id];
+	unsigned long flags;
+	u32 reg;
+	int ret;
+
+	spin_lock_irqsave(reset->lock, flags);
+	ret = regmap_read(reset->regmap, data->rson_offset, &reg);
+	spin_unlock_irqrestore(reset->lock, flags);
+	if (ret)
+		return ret;
+
+	return !(reg & data->rs_idx);
+}
+
+const struct reset_control_ops visconti_reset_ops = {
+	.assert		= visconti_reset_assert,
+	.deassert	= visconti_reset_deassert,
+	.reset		= visconti_reset_reset,
+	.status		= visconti_reset_status,
+};
+
+int visconti_register_reset_controller(struct device_node *np, struct regmap *regmap,
+				       const struct visconti_reset_data *resets,
+				       unsigned int num_resets,
+				       const struct reset_control_ops *reset_ops,
+				       spinlock_t *lock)
+{
+	struct visconti_reset *reset;
+	int ret;
+
+	reset = kzalloc(sizeof(*reset), GFP_KERNEL);
+	if (!reset)
+		return -ENOMEM;
+
+	reset->regmap = regmap;
+	reset->resets = resets;
+	reset->rcdev.ops = reset_ops;
+	reset->rcdev.nr_resets = num_resets;
+	reset->rcdev.of_node = np;
+	reset->lock = lock;
+
+	ret = reset_controller_register(&reset->rcdev);
+	if (ret) {
+		pr_err("could not register reset controller: %d\n", ret);
+		kfree(reset);
+		return ret;
+	}
+
+	return 0;
+}
diff --git a/drivers/clk/visconti/reset.h b/drivers/clk/visconti/reset.h
new file mode 100644
index 000000000000..521305caa6fa
--- /dev/null
+++ b/drivers/clk/visconti/reset.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Toshiba ARM SoC reset controller driver
+ *
+ * Copyright (c) 2021 TOSHIBA CORPORATION
+ *
+ * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ */
+
+#ifndef _VISCONTI_RESET_H_
+#define _VISCONTI_RESET_H_
+
+#include <linux/reset-controller.h>
+
+struct visconti_reset_data {
+	u32	rson_offset;
+	u32	rsoff_offset;
+	u8	rs_idx;
+};
+
+struct visconti_reset {
+	struct reset_controller_dev rcdev;
+	struct regmap *regmap;
+	const struct visconti_reset_data *resets;
+	spinlock_t *lock;
+};
+
+extern const struct reset_control_ops visconti_reset_ops;
+
+int visconti_register_reset_controller(struct device_node *np, struct regmap *regmap,
+				       const struct visconti_reset_data *resets,
+				       unsigned int num_resets,
+				       const struct reset_control_ops *reset_ops,
+				       spinlock_t *lock);
+#endif /* _VISCONTI_RESET_H_ */
-- 
2.32.0

Re: [PATCH v4 3/4] clk: visconti: Add support common clock driver and reset driver

[Stephen Boyd]

Quoting Nobuhiro Iwamatsu (2021-08-04 02:22:43)
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 5f06879d7fe9..dbf70fdecfc8 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -116,6 +116,7 @@ obj-y                                       += ti/
>  obj-$(CONFIG_CLK_UNIPHIER)             += uniphier/
>  obj-$(CONFIG_ARCH_U8500)               += ux500/
>  obj-y                                  += versatile/
> +obj-$(CONFIG_ARCH_VISCONTI)            += visconti/

Can we have an actual Kconfig symbol for this so we can compile test the
driver?

>  ifeq ($(CONFIG_COMMON_CLK), y)
>  obj-$(CONFIG_X86)                      += x86/
>  endif
> diff --git a/drivers/clk/visconti/Makefile b/drivers/clk/visconti/Makefile
> new file mode 100644
> index 000000000000..c1254fd52b31
> --- /dev/null
> +++ b/drivers/clk/visconti/Makefile
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +# Makefile for Toshiba Visconti clock
> +
> +obj-y  += clkc.o pll.o reset.o
> +obj-y  += pll-tmpv770x.o clkc-tmpv770x.o
> diff --git a/drivers/clk/visconti/clkc-tmpv770x.c b/drivers/clk/visconti/clkc-tmpv770x.c
> new file mode 100644
> index 000000000000..6ee5c5179f7c
> --- /dev/null
> +++ b/drivers/clk/visconti/clkc-tmpv770x.c
> @@ -0,0 +1,232 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Toshiba Visconti clock controller
> + *
> + * Copyright (c) 2021 TOSHIBA CORPORATION
> + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> + *
> + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + */
> +
> +#include <dt-bindings/clock/toshiba,tmpv770x.h>
> +#include <dt-bindings/reset/toshiba,tmpv770x.h>

Please include these after linux headers.

> +#include <linux/clk.h>

Is it a clk provider or a clk consumer? Hopefully it's a clk provider,
and thus this should be clk-provider.h instead of clk.h for the
consumer.

> +#include <linux/platform_device.h>
> +
> +#include "clkc.h"
> +#include "reset.h"
> +
> +static DEFINE_SPINLOCK(tmpv770x_clk_lock);
> +static DEFINE_SPINLOCK(tmpv770x_rst_lock);
> +
> +static const struct visconti_fixed_clk fixed_clk_tables[] = {
> +       /* PLL1 */
> +       /* PICMPT0/1, PITSC, PIUWDT, PISWDT, PISBUS, PIPMU, PIGPMU, PITMU */
> +       /* PIEMM, PIMISC, PIGCOMM, PIDCOMM, PIMBUS, PIGPIO, PIPGM */
> +       { TMPV770X_CLK_PIPLL1_DIV4, "pipll1_div4", "pipll1", 0, 1, 4, },
> +       /* PISBUS */
> +       { TMPV770X_CLK_PIPLL1_DIV2, "pipll1_div2", "pipll1", 0, 1, 2, },
> +       /* PICOBUS_CLK */
> +       { TMPV770X_CLK_PIPLL1_DIV1, "pipll1_div1", "pipll1", 0, 1, 1, },
> +       /* PIDNNPLL */
> +       /* CONN_CLK, PIMBUS, PICRC0/1 */
> +       { TMPV770X_CLK_PIDNNPLL_DIV1, "pidnnpll_div1", "pidnnpll", 0, 1, 1, },
> +       { TMPV770X_CLK_PIREFCLK, "pirefclk", "osc2-clk", 0, 1, 1, },
> +       { TMPV770X_CLK_WDTCLK, "wdtclk", "osc2-clk", 0, 1, 1, },
> +};
> +
> +static const struct visconti_clk_gate_table pietherpll_clk_gate_tables[] = {
> +       /* pietherpll */
> +       { TMPV770X_CLK_PIETHER_2P5M, "piether_2p5m", "pietherpll",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 4, 200,

There's quite a lot of CLK_IGNORE_UNUSED. Should these be
CLK_IS_CRITICAL instead, and then only the ones that are truly critical
to the system still running should be marked as critical along with a
comment explaining what they're clocking and why that device isn't
probing and enabling the clks itself.

> +               TMPV770X_RESET_PIETHER_2P5M, },
> +       { TMPV770X_CLK_PIETHER_25M, "piether_25m", "pietherpll",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 5, 20,
> +               TMPV770X_RESET_PIETHER_25M, },
> +       { TMPV770X_CLK_PIETHER_50M, "piether_50m", "pietherpll",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 6, 10,
> +               TMPV770X_RESET_PIETHER_50M, },
> +       { TMPV770X_CLK_PIETHER_125M, "piether_125m", "pietherpll",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 7, 4,
> +               TMPV770X_RESET_PIETHER_125M, },
> +};
> +
> +static const struct visconti_clk_gate_table clk_gate_tables[] = {
> +       { TMPV770X_CLK_HOX, "hox", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x4C, 0x14C, 0, 1,

Lowercase hex please.

> +               TMPV770X_RESET_HOX, },
> +       { TMPV770X_CLK_PCIE_MSTR, "pcie_mstr", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x38, 0x138, 0, 1,
> +               TMPV770X_RESET_PCIE_MSTR, },
> +       { TMPV770X_CLK_PCIE_AUX, "pcie_aux", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x38, 0x138, 1, 24,
> +               TMPV770X_RESET_PCIE_AUX, },
> +       { TMPV770X_CLK_PIINTC, "piintc", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x8, 0x108, 0, 2,
> +               TMPV770X_RESET_PIINTC,},
> +       { TMPV770X_CLK_PIETHER_BUS, "piether_bus", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 0, 2,
> +               TMPV770X_RESET_PIETHER_BUS, }, /* BUS_CLK */
> +       { TMPV770X_CLK_PISPI0, "pispi0", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 0, 2,
> +               TMPV770X_RESET_PISPI0, },
> +       { TMPV770X_CLK_PISPI1, "pispi1", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 1, 2,
> +               TMPV770X_RESET_PISPI1, },
> +       { TMPV770X_CLK_PISPI2, "pispi2", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 2, 2,
> +               TMPV770X_RESET_PISPI2, },
> +       { TMPV770X_CLK_PISPI3, "pispi3", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 3, 2,
> +               TMPV770X_RESET_PISPI3,},
> +       { TMPV770X_CLK_PISPI4, "pispi4", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 4, 2,
> +               TMPV770X_RESET_PISPI4, },
> +       { TMPV770X_CLK_PISPI5, "pispi5", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 5, 2,
> +               TMPV770X_RESET_PISPI5},
> +       { TMPV770X_CLK_PISPI6, "pispi6", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 6, 2,
> +               TMPV770X_RESET_PISPI6,},
> +       { TMPV770X_CLK_PIUART0, "piuart0", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 0, 4,
> +               TMPV770X_RESET_PIUART0,},
> +       { TMPV770X_CLK_PIUART1, "piuart1", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 1, 4,
> +               TMPV770X_RESET_PIUART1, },
> +       { TMPV770X_CLK_PIUART2, "piuart2", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 2, 4,
> +               TMPV770X_RESET_PIUART2, },
> +       { TMPV770X_CLK_PIUART3, "piuart3", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 3, 4,
> +               TMPV770X_RESET_PIUART3, },
> +       { TMPV770X_CLK_PII2C0, "pii2c0", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 0, 4,
> +               TMPV770X_RESET_PII2C0, },
> +       { TMPV770X_CLK_PII2C1, "pii2c1", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 1, 4,
> +               TMPV770X_RESET_PII2C1, },
> +       { TMPV770X_CLK_PII2C2, "pii2c2", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 2, 4,
> +               TMPV770X_RESET_PII2C2, },
> +       { TMPV770X_CLK_PII2C3, "pii2c3", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 3, 4,
> +               TMPV770X_RESET_PII2C3,},
> +       { TMPV770X_CLK_PII2C4, "pii2c4", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 4, 4,
> +               TMPV770X_RESET_PII2C4, },
> +       { TMPV770X_CLK_PII2C5, "pii2c5", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 5, 4,
> +               TMPV770X_RESET_PII2C5, },
> +       { TMPV770X_CLK_PII2C6, "pii2c6", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 6, 4,
> +               TMPV770X_RESET_PII2C6, },
> +       { TMPV770X_CLK_PII2C7, "pii2c7", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 7, 4,
> +               TMPV770X_RESET_PII2C7, },
> +       { TMPV770X_CLK_PII2C8, "pii2c8", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 8, 4,
> +               TMPV770X_RESET_PII2C8, },
> +       /* PIPCMIF */
> +       { TMPV770X_CLK_PIPCMIF, "pipcmif", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x64, 0x164, 0, 4,
> +               TMPV770X_RESET_PIPCMIF, },
> +       /* PISYSTEM */
> +       { TMPV770X_CLK_WRCK, "wrck", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x68, 0x168, 9, 32,
> +               -1, }, /* No reset */
> +       { TMPV770X_CLK_PICKMON, "pickmon", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x10, 0x110, 8, 4,
> +               TMPV770X_RESET_PICKMON, },
> +       { TMPV770X_CLK_SBUSCLK, "sbusclk", "pipll1",
> +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x14, 0x114, 0, 4,
> +               TMPV770X_RESET_SBUSCLK, },
> +};
> +
> +static const struct visconti_reset_data clk_reset_data[] = {
> +       [TMPV770X_RESET_PIETHER_2P5M]   = { 0x434, 0x534, 4, },
> +       [TMPV770X_RESET_PIETHER_25M]    = { 0x434, 0x534, 5, },
> +       [TMPV770X_RESET_PIETHER_50M]    = { 0x434, 0x534, 6, },
> +       [TMPV770X_RESET_PIETHER_125M]   = { 0x434, 0x534, 7, },
> +       [TMPV770X_RESET_HOX]            = { 0x44C, 0x54C, 0, },
> +       [TMPV770X_RESET_PCIE_MSTR]      = { 0x438, 0x538, 0, },
> +       [TMPV770X_RESET_PCIE_AUX]       = { 0x438, 0x538, 1, },
> +       [TMPV770X_RESET_PIINTC]         = { 0x408, 0x508, 0, },
> +       [TMPV770X_RESET_PIETHER_BUS]    = { 0x434, 0x534, 0, },
> +       [TMPV770X_RESET_PISPI0]         = { 0x428, 0x528, 0, },
> +       [TMPV770X_RESET_PISPI1]         = { 0x428, 0x528, 1, },
> +       [TMPV770X_RESET_PISPI2]         = { 0x428, 0x528, 2, },
> +       [TMPV770X_RESET_PISPI3]         = { 0x428, 0x528, 3, },
> +       [TMPV770X_RESET_PISPI4]         = { 0x428, 0x528, 4, },
> +       [TMPV770X_RESET_PISPI5]         = { 0x428, 0x528, 5, },
> +       [TMPV770X_RESET_PISPI6]         = { 0x428, 0x528, 6, },
> +       [TMPV770X_RESET_PIUART0]        = { 0x42C, 0x52C, 0, },
> +       [TMPV770X_RESET_PIUART1]        = { 0x42C, 0x52C, 1, },
> +       [TMPV770X_RESET_PIUART2]        = { 0x42C, 0x52C, 2, },
> +       [TMPV770X_RESET_PIUART3]        = { 0x42C, 0x52C, 3, },
> +       [TMPV770X_RESET_PII2C0]         = { 0x430, 0x530, 0, },
> +       [TMPV770X_RESET_PII2C1]         = { 0x430, 0x530, 1, },
> +       [TMPV770X_RESET_PII2C2]         = { 0x430, 0x530, 2, },
> +       [TMPV770X_RESET_PII2C3]         = { 0x430, 0x530, 3, },
> +       [TMPV770X_RESET_PII2C4]         = { 0x430, 0x530, 4, },
> +       [TMPV770X_RESET_PII2C5]         = { 0x430, 0x530, 5, },
> +       [TMPV770X_RESET_PII2C6]         = { 0x430, 0x530, 6, },
> +       [TMPV770X_RESET_PII2C7]         = { 0x430, 0x530, 7, },
> +       [TMPV770X_RESET_PII2C8]         = { 0x430, 0x530, 8, },
> +       [TMPV770X_RESET_PIPCMIF]        = { 0x464, 0x564, 0, },
> +       [TMPV770X_RESET_PICKMON]        = { 0x410, 0x510, 8, },
> +       [TMPV770X_RESET_SBUSCLK]        = { 0x414, 0x514, 0, },
> +};
> +
> +static void __init tmpv770x_clkc_setup_clks(struct device_node *np)
> +{
> +       struct visconti_clk_provider *ctx;
> +       struct regmap *regmap;
> +       int ret, i;
> +
> +       regmap = device_node_to_regmap(np);

Is it inside a syscon?

> +       if (IS_ERR(regmap))
> +               return;
> +
> +       ctx = visconti_init_clk(np, regmap, TMPV770X_NR_CLK);
> +       if (IS_ERR(ctx))
> +               return;
> +
> +       ret = visconti_register_reset_controller(np, regmap, clk_reset_data,
> +                                                TMPV770X_NR_RESET,
> +                                                &visconti_reset_ops,
> +                                                &tmpv770x_rst_lock);
> +       if (ret) {
> +               pr_err("Failed to register reset controller: %d\n", ret);
> +               return;
> +       }
> +
> +       for (i = 0; i < (ARRAY_SIZE(fixed_clk_tables)); i++)
> +               ctx->clk_data.clks[fixed_clk_tables[i].id] =
> +                       clk_register_fixed_factor(NULL,
> +                                               fixed_clk_tables[i].name,
> +                                               fixed_clk_tables[i].parent,
> +                                               fixed_clk_tables[i].flag,
> +                                               fixed_clk_tables[i].mult,
> +                                               fixed_clk_tables[i].div);
> +
> +       ret = visconti_clk_register_gates(ctx, clk_gate_tables,
> +                                   ARRAY_SIZE(clk_gate_tables), clk_reset_data,
> +                                   &tmpv770x_clk_lock);
> +       if (ret) {
> +               pr_err("Failed to register main clock gate: %d\n", ret);
> +               return;
> +       }
> +
> +       ret = visconti_clk_register_gates(ctx, pietherpll_clk_gate_tables,
> +                                   ARRAY_SIZE(pietherpll_clk_gate_tables),
> +                                   clk_reset_data, &tmpv770x_clk_lock);
> +       if (ret) {
> +               pr_err("Failed to register pietherpll clock gate: %d\n", ret);
> +               return;
> +       }
> +
> +       of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
> +}
> +
> +CLK_OF_DECLARE_DRIVER(tmpv770x_clkc, "toshiba,tmpv7708-pismu", tmpv770x_clkc_setup_clks);

Any chance this can be a platform driver?

> diff --git a/drivers/clk/visconti/clkc.c b/drivers/clk/visconti/clkc.c
> new file mode 100644
> index 000000000000..32b6f6266154
> --- /dev/null
> +++ b/drivers/clk/visconti/clkc.c
> @@ -0,0 +1,220 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Toshiba Visconti clock controller
> + *
> + * Copyright (c) 2021 TOSHIBA CORPORATION
> + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> + *
> + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + */
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +
> +#include "clkc.h"
> +
> +static inline struct visconti_clk_gate *to_visconti_clk_gate(struct clk_hw *hw)
> +{
> +       return container_of(hw, struct visconti_clk_gate, hw);
> +}
> +
> +static int visconti_gate_clk_is_enabled(struct clk_hw *hw)
> +{
> +       struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
> +       u32 clk = BIT(gate->ck_idx);
> +       u32 val;
> +
> +       regmap_read(gate->regmap, gate->ckon_offset, &val);
> +       return (val & clk) ? 1 : 0;
> +}
> +
> +static void visconti_gate_clk_disable(struct clk_hw *hw)
> +{
> +       struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
> +       u32 clk = BIT(gate->ck_idx);
> +       u32 rst = BIT(gate->rs_idx);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(gate->lock, flags);
> +
> +       if (visconti_gate_clk_is_enabled(hw)) {

So we never disable the clk?

> +               spin_unlock_irqrestore(gate->lock, flags);
> +               return;
> +       }
> +
> +       /* Reset release */
> +       regmap_update_bits(gate->regmap, gate->rson_offset, rst, 1);
> +
> +       udelay(100);
> +
> +       /* Disable clock */
> +       regmap_update_bits(gate->regmap, gate->ckoff_offset, clk, 1);
> +       spin_unlock_irqrestore(gate->lock, flags);
> +}
> +
> +static int visconti_gate_clk_enable(struct clk_hw *hw)
> +{
> +       struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
> +       u32 clk = BIT(gate->ck_idx);
> +       u32 rst = BIT(gate->rs_idx);
> +       unsigned long flags;
> +       u32 val;
> +
> +       spin_lock_irqsave(gate->lock, flags);
> +
> +       if (visconti_gate_clk_is_enabled(hw)) {

The core framework should only call down to here if the clk is actually
disabled. This check can be removed.

> +               spin_unlock_irqrestore(gate->lock, flags);
> +               return 0;
> +       }
> +
> +       regmap_update_bits(gate->regmap, gate->ckon_offset, clk, 1);
> +       /* Need read back */

Why? Please indicate why instead of just saying what the code is doing.

> +       regmap_read(gate->regmap, gate->ckon_offset, &val);
> +
> +       udelay(100);
> +       /* Reset release */
> +       regmap_update_bits(gate->regmap, gate->rsoff_offset, rst, 1);
> +       /* Need read back */
> +       regmap_read(gate->regmap, gate->ckoff_offset, &val);
> +       spin_unlock_irqrestore(gate->lock, flags);
> +
> +       return 0;
> +}
> +
> +static const struct clk_ops visconti_clk_gate_ops = {
> +       .enable = visconti_gate_clk_enable,
> +       .disable = visconti_gate_clk_disable,
> +       .is_enabled = visconti_gate_clk_is_enabled,
> +};
> +
> +static struct clk_hw *visconti_clk_register_gate(struct device *dev,
> +                                                const char *name,
> +                                                const char *parent_name,
> +                                                struct regmap *regmap,
> +                                                const struct visconti_clk_gate_table *clks,
> +                                                u32    rson_offset,
> +                                                u32    rsoff_offset,
> +                                                u8     rs_idx,
> +                                                spinlock_t *lock)
> +{
> +       struct visconti_clk_gate *gate;
> +       struct clk_init_data init;
> +       struct clk_hw *hw;
> +       int ret;
> +
> +       gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +       if (!gate)
> +               return ERR_PTR(-ENOMEM);
> +
> +       init.name = name;
> +       init.ops = &visconti_clk_gate_ops;
> +       init.flags = clks->flags;
> +       init.parent_names = parent_name ? &parent_name : NULL;
> +       init.num_parents = parent_name ? 1 : 0;

Do some clks have parents and other clks don't? Or do they always have
parents? Preferably it's one or the other. And if there are parents,
please use clk_parent_data instead of parent_names.

> +
> +       gate->regmap = regmap;
> +       gate->ckon_offset = clks->ckon_offset;
> +       gate->ckoff_offset = clks->ckoff_offset;
> +       gate->ck_idx = clks->ck_idx;
> +       gate->rson_offset = rson_offset;
> +       gate->rsoff_offset = rsoff_offset;
> +       gate->rs_idx = rs_idx;
> +       gate->lock = lock;
> +       gate->hw.init = &init;
> +
> +       hw = &gate->hw;
> +       ret = clk_hw_register(dev, hw);

Is the dev non-NULL? If yes then can devm be used?

> +       if (ret) {
> +               kfree(gate);
> +               hw = ERR_PTR(ret);
> +       }
> +
> +       return hw;
> +}
> +
> +int visconti_clk_register_gates(struct visconti_clk_provider *data,
> +                                const struct visconti_clk_gate_table *clks,
> +                                int num_gate, const struct visconti_reset_data *reset,
> +                                spinlock_t *lock)
> +{
> +       u32 rson_offset, rsoff_offset;
> +       struct clk_hw *hw_clk;
> +       u8 rs_idx;
> +       int i;
> +
> +       for (i = 0; i < num_gate; i++) {
> +               struct clk *clk;
> +               char *div_name;
> +
> +               div_name = kasprintf(GFP_KERNEL, "%s_div", clks[i].name);
> +               if (!div_name)
> +                       return -ENOMEM;
> +
> +               if (clks[i].rs_id >= 0) {
> +                       rson_offset = reset[clks[i].rs_id].rson_offset;
> +                       rsoff_offset = reset[clks[i].rs_id].rsoff_offset;
> +                       rs_idx = reset[clks[i].rs_id].rs_idx;
> +               } else {
> +                       rson_offset = rsoff_offset = rs_idx = -1;
> +               }
> +
> +               clk = clk_register_fixed_factor(NULL, div_name, clks[i].parent,
> +                                               0, 1, clks[i].div);
> +               if (IS_ERR(clk))
> +                       return PTR_ERR(clk);
> +
> +               hw_clk = visconti_clk_register_gate(NULL,

Oh it's always NULL? Sad

> +                                               clks[i].name,
> +                                               div_name,
> +                                               data->regmap,
> +                                               &clks[i],
> +                                               rson_offset,
> +                                               rsoff_offset,
> +                                               rs_idx,
> +                                               lock);
> +               kfree(div_name);
> +               if (IS_ERR(hw_clk)) {
> +                       pr_err("%s: failed to register clock %s\n",
> +                              __func__, clks[i].name);
> +                       return PTR_ERR(hw_clk);
> +               }
> +
> +               data->clk_data.clks[clks[i].id] = hw_clk->clk;
> +       }
> +
> +       return 0;
> +}
> +
> +struct visconti_clk_provider *visconti_init_clk(struct device_node *np,
> +                                               struct regmap *regmap,
> +                                               unsigned long nr_clks)
> +{
> +       struct visconti_clk_provider *ctx;
> +       struct clk **clk_table;
> +       int i;
> +
> +       ctx = kzalloc(sizeof(struct visconti_clk_provider), GFP_KERNEL);

sizeof(*ctx)?

> +       if (!ctx)
> +               return ERR_PTR(-ENOMEM);
> +
> +       clk_table = kcalloc(nr_clks, sizeof(struct clk *), GFP_KERNEL);

sizeof(*clk_table)?

> +       if (!clk_table)
> +               goto err;
> +
> +       for (i = 0; i < nr_clks; ++i)
> +               clk_table[i] = ERR_PTR(-ENOENT);
> +       ctx->node = np;
> +       ctx->regmap = regmap;
> +       ctx->clk_data.clks = clk_table;
> +       ctx->clk_data.clk_num = nr_clks;
> +
> +       return ctx;
> +err:
> +       kfree(ctx);
> +       return ERR_PTR(-ENOMEM);
> +}
> diff --git a/drivers/clk/visconti/clkc.h b/drivers/clk/visconti/clkc.h
> new file mode 100644
> index 000000000000..4df8788d635a
> --- /dev/null
> +++ b/drivers/clk/visconti/clkc.h
> @@ -0,0 +1,75 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Toshiba Visconti clock controller
> + *
> + * Copyright (c) 2021 TOSHIBA CORPORATION
> + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> + *
> + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + */
> +
> +#ifndef _VISCONTI_CLKC_H_
> +#define _VISCONTI_CLKC_H_
> +
> +#include <linux/mfd/syscon.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/delay.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/io.h>
> +#include <linux/spinlock.h>
> +
> +#include "reset.h"
> +
> +struct visconti_clk_provider {
> +       struct device_node      *node;
> +       struct regmap           *regmap;
> +       struct clk_onecell_data clk_data;
> +};
> +
> +struct visconti_clk_gate_table {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       u8              flags;
> +       u32             ckon_offset;
> +       u32             ckoff_offset;
> +       u8              ck_idx;
> +       unsigned int    div;
> +       u8              rs_id;
> +};
> +
> +struct visconti_fixed_clk {
> +       unsigned int    id;
> +       const char      *name;
> +       const char      *parent;
> +       unsigned long   flag;
> +       unsigned int    mult;
> +       unsigned int    div;
> +};
> +
> +struct visconti_clk_gate {
> +       struct clk_hw   hw;
> +       struct regmap   *regmap;
> +       u32             ckon_offset;
> +       u32             ckoff_offset;
> +       u8              ck_idx;
> +       u8              flags;
> +       u32             rson_offset;
> +       u32             rsoff_offset;
> +       u8              rs_idx;
> +       spinlock_t      *lock;
> +};
> +
> +struct visconti_clk_provider *visconti_init_clk(struct device_node *np,
> +                                               struct regmap *regmap,
> +                                               unsigned long nr_clks);
> +int visconti_clk_register_gates(struct visconti_clk_provider *data,
> +                                const struct visconti_clk_gate_table *clks,
> +                                int num_gate,
> +                                const struct visconti_reset_data *reset,
> +                                spinlock_t *lock);
> +#endif /* _VISCONTI_CLKC_H_ */
> diff --git a/drivers/clk/visconti/pll-tmpv770x.c b/drivers/clk/visconti/pll-tmpv770x.c
> new file mode 100644
> index 000000000000..a0874cb6a67b
> --- /dev/null
> +++ b/drivers/clk/visconti/pll-tmpv770x.c
> @@ -0,0 +1,85 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Toshiba Visconti PLL controller
> + *
> + * Copyright (c) 2021 TOSHIBA CORPORATION
> + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> + *
> + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + */
> +
> +#include <dt-bindings/clock/toshiba,tmpv770x.h>
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +
> +#include "pll.h"
> +
> +static DEFINE_SPINLOCK(tmpv770x_pll_lock);
> +
> +static const struct visconti_pll_rate_table pipll0_rates[] __initconst = {
> +       VISCONTI_PLL_RATE(840000000, 0x1, 0x0, 0x1, 0x54, 0x000000, 0x2, 0x1),
> +       VISCONTI_PLL_RATE(780000000, 0x1, 0x0, 0x1, 0x4E, 0x000000, 0x2, 0x1),
> +       VISCONTI_PLL_RATE(600000000, 0x1, 0x0, 0x1, 0x3C, 0x000000, 0x2, 0x1),
> +       { /* sentinel */ },
> +};
> +
> +static const struct visconti_pll_rate_table piddrcpll_rates[] __initconst = {
> +       VISCONTI_PLL_RATE(780000000, 0x1, 0x0, 0x1, 0x4E, 0x000000, 0x2, 0x1),
> +       VISCONTI_PLL_RATE(760000000, 0x1, 0x0, 0x1, 0x4C, 0x000000, 0x2, 0x1),
> +       { /* sentinel */ },
> +};
> +
> +static const struct visconti_pll_rate_table pivoifpll_rates[] __initconst = {
> +       VISCONTI_PLL_RATE(165000000, 0x1, 0x0, 0x1, 0x42, 0x000000, 0x4, 0x2),
> +       VISCONTI_PLL_RATE(148500000, 0x1, 0x1, 0x1, 0x3B, 0x666666, 0x4, 0x2),
> +       VISCONTI_PLL_RATE(96000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x2),
> +       VISCONTI_PLL_RATE(74250000, 0x1, 0x1, 0x1, 0x3B, 0x666666, 0x4, 0x4),
> +       VISCONTI_PLL_RATE(54000000, 0x1, 0x0, 0x1, 0x36, 0x000000, 0x5, 0x4),
> +       VISCONTI_PLL_RATE(48000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x4),
> +       VISCONTI_PLL_RATE(35750000, 0x1, 0x1, 0x1, 0x32, 0x0CCCCC, 0x7, 0x4),
> +       { /* sentinel */ },
> +};
> +
> +static const struct visconti_pll_rate_table piimgerpll_rates[] __initconst = {
> +       VISCONTI_PLL_RATE(165000000, 0x1, 0x0, 0x1, 0x42, 0x000000, 0x4, 0x2),
> +       VISCONTI_PLL_RATE(96000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x2),
> +       VISCONTI_PLL_RATE(54000000, 0x1, 0x0, 0x1, 0x36, 0x000000, 0x5, 0x4),
> +       VISCONTI_PLL_RATE(48000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x4),
> +       { /* sentinel */ },
> +};
> +
> +static const struct visconti_pll_info pll_info[] __initconst = {
> +       { TMPV770X_PLL_PIPLL0, "pipll0", "osc2-clk", 0, 0x0, pipll0_rates },
> +       { TMPV770X_PLL_PIDDRCPLL, "piddrcpll", "osc2-clk", 0, 0x500, piddrcpll_rates },
> +       { TMPV770X_PLL_PIVOIFPLL, "pivoifpll", "osc2-clk", 0, 0x600, pivoifpll_rates },
> +       { TMPV770X_PLL_PIIMGERPLL, "piimgerpll", "osc2-clk", 0, 0x700, piimgerpll_rates },
> +};
> +
> +static void __init tmpv770x_setup_plls(struct device_node *np)
> +{
> +       struct visconti_pll_provider *ctx;
> +       void __iomem *reg_base;
> +
> +       reg_base = of_iomap(np, 0);
> +       if (!reg_base)
> +               return;
> +
> +       ctx = visconti_init_pll(np, reg_base, TMPV770X_NR_PLL);
> +       if (IS_ERR(ctx)) {
> +               iounmap(reg_base);
> +               return;
> +       }
> +
> +       ctx->clk_data.clks[TMPV770X_PLL_PIPLL1] =
> +               clk_register_fixed_rate(NULL, "pipll1", NULL, 0, 600000000);
> +       ctx->clk_data.clks[TMPV770X_PLL_PIDNNPLL] =
> +               clk_register_fixed_rate(NULL, "pidnnpll", NULL, 0, 500000000);
> +       ctx->clk_data.clks[TMPV770X_PLL_PIETHERPLL] =
> +               clk_register_fixed_rate(NULL, "pietherpll", NULL, 0, 500000000);
> +
> +       visconti_register_plls(ctx, pll_info, ARRAY_SIZE(pll_info), &tmpv770x_pll_lock);
> +}
> +
> +CLK_OF_DECLARE_DRIVER(tmpv770x_plls, "toshiba,tmpv7708-pipllct", tmpv770x_setup_plls);

DECLARE_DRIVER is for when more than one device needs to probe the node
in DT. Is that happening here? If so, please add a comment indicating
what the other driver is.

> diff --git a/drivers/clk/visconti/pll.c b/drivers/clk/visconti/pll.c
> new file mode 100644
> index 000000000000..7baa9512fad9
> --- /dev/null
> +++ b/drivers/clk/visconti/pll.c
> @@ -0,0 +1,369 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Toshiba Visconti PLL driver
> + *
> + * Copyright (c) 2021 TOSHIBA CORPORATION
> + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> + *
> + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +
> +#include "pll.h"
> +
> +struct visconti_pll {
> +       struct clk_hw   hw;
> +       void __iomem    *pll_base;
> +       spinlock_t      *lock;
> +       int rate_count;

size_t? And then place next to rate_table so we know they're related
please.

> +       unsigned long flags;
> +       const struct visconti_pll_rate_table *rate_table;
> +       struct visconti_pll_provider *ctx;
> +};
> +
> +#define PLL_CONF_REG           0x0000
> +#define PLL_CTRL_REG           0x0004
> +#define PLL_FRACMODE_REG       0x0010
> +#define PLL_INTIN_REG          0x0014
> +#define PLL_FRACIN_REG         0x0018
> +#define PLL_REFDIV_REG         0x001C
> +#define PLL_POSTDIV_REG                0x0020
> +
> +#define PLL_CONFIG_SEL         BIT(0)
> +#define PLL_PLLEN              BIT(4)
> +#define PLL_BYPASS             BIT(16)
> +#define PLL_INTIN_MASK         GENMASK(11, 0)
> +#define PLL_FRACIN_MASK                GENMASK(23, 0)
> +#define PLL_REFDIV_MASK                GENMASK(5, 0)
> +#define PLL_POSTDIV_MASK       GENMASK(2, 0)
> +
> +static inline struct visconti_pll *to_visconti_pll(struct clk_hw *hw)
> +{
> +       return container_of(hw, struct visconti_pll, hw);
> +}
> +
> +static void visconti_pll_get_params(struct visconti_pll *pll,
> +                                   struct visconti_pll_rate_table *rate_table)
> +{
> +       u32 postdiv, val;
> +
> +       val = readl(pll->pll_base + PLL_FRACMODE_REG);
> +       rate_table->dacen = (val >> 4) & 0x1;

What is 0x1? Some bitfield? Can it be a #define at least? Same for 4.
Better yet, use GENMASK and FIELD_PREP() and FIELD_GET() macros.

> +       rate_table->dsmen = val & 0x1;
> +       rate_table->fracin = readl(pll->pll_base + PLL_FRACIN_REG) & PLL_FRACIN_MASK;
> +       rate_table->intin = readl(pll->pll_base + PLL_INTIN_REG) & PLL_INTIN_MASK;
> +       rate_table->refdiv = readl(pll->pll_base + PLL_REFDIV_REG) & PLL_REFDIV_MASK;
> +
> +       postdiv = readl(pll->pll_base + PLL_POSTDIV_REG);
> +       rate_table->postdiv1 = postdiv & PLL_POSTDIV_MASK;
> +       rate_table->postdiv2 = (postdiv >> 4) & PLL_POSTDIV_MASK;
> +}
> +
> +static const struct visconti_pll_rate_table *visconti_get_pll_settings(struct visconti_pll *pll,
> +                                                                      unsigned long rate)
> +{
> +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> +       int i;
> +
> +       for (i = 0; i < pll->rate_count; i++) {
> +               if (rate == rate_table[i].rate)
> +                       return &rate_table[i];
> +       }

Drop braces please.

> +
> +       return NULL;
> +}
> +
> +static unsigned long visconti_get_pll_rate_from_data(struct visconti_pll *pll,
> +                                                    struct visconti_pll_rate_table *rate)

const pointers?

> +{
> +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> +       int i;
> +
> +       for (i = 0; i < pll->rate_count; i++) {
> +               if (rate_table[i].dacen == rate->dacen &&
> +                       rate_table[i].dsmen == rate->dsmen &&
> +                       rate_table[i].fracin == rate->fracin &&
> +                       rate_table[i].intin == rate->intin &&
> +                       rate_table[i].refdiv == rate->refdiv &&
> +                       rate_table[i].postdiv1 == rate->postdiv1 &&
> +                       rate_table[i].postdiv2 == rate->postdiv2)

Is it not the same table? Can we just compare the two pointers instead
of each member?

> +                       return rate_table[i].rate;
> +       }
> +
> +       /* set default */
> +       return rate_table[0].rate;
> +}
> +
> +static long visconti_pll_round_rate(struct clk_hw *hw,
> +                           unsigned long rate, unsigned long *prate)
> +{
> +       struct visconti_pll *pll = to_visconti_pll(hw);
> +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> +       int i;
> +
> +       /* Assumming rate_table is in descending order */
> +       for (i = 0; i < pll->rate_count; i++) {
> +               if (rate >= rate_table[i].rate)
> +                       return rate_table[i].rate;
> +       }

Drop braces please.

> +
> +       /* return minimum supported value */
> +       return rate_table[i - 1].rate;
> +}
> +
> +static unsigned long visconti_pll_recalc_rate(struct clk_hw *hw,
> +                                             unsigned long parent_rate)
> +{
> +       struct visconti_pll *pll = to_visconti_pll(hw);
> +       struct visconti_pll_rate_table rate_table;
> +
> +       memset(&rate_table, 0, sizeof(rate_table));
> +       visconti_pll_get_params(pll, &rate_table);
> +
> +       return visconti_get_pll_rate_from_data(pll, &rate_table);

Oh. Can we do math instead?

> +}
> +
> +static int visconti_pll_set_params(struct visconti_pll *pll,
> +                               const struct visconti_pll_rate_table *rate_table)
> +{
> +       /* update pll values */

This comment is worthless. Please remove.

> +       writel(((rate_table->dacen << 4) | rate_table->dsmen),

One too many parenthesis here. Please clean it up.

> +                       pll->pll_base + PLL_FRACMODE_REG);
> +       writel(((rate_table->postdiv2 << 4) | rate_table->postdiv1),

Same.

> +                       pll->pll_base + PLL_POSTDIV_REG);
> +       writel(rate_table->intin, pll->pll_base + PLL_INTIN_REG);
> +       writel(rate_table->fracin, pll->pll_base + PLL_FRACIN_REG);
> +       writel(rate_table->refdiv, pll->pll_base + PLL_REFDIV_REG);
> +
> +       return 0;
> +}
> +
> +static int visconti_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> +                           unsigned long parent_rate)
> +{
> +       struct visconti_pll *pll = to_visconti_pll(hw);
> +       const struct visconti_pll_rate_table *rate_table;
> +
> +       /* Get required rate settings from table */

This comment is worthless, please remove.

> +       rate_table = visconti_get_pll_settings(pll, rate);
> +       if (!rate_table) {
> +               pr_err("Invalid rate : %lu for pll clk %s\n",

Please don't print anything during set_rate. It can be quite noisy if
something like a cpufreq driver keeps calling this with the incorrect
frequency.

> +                       parent_rate, __clk_get_name(hw->clk));
> +               return -EINVAL;
> +       }
> +
> +       return visconti_pll_set_params(pll, rate_table);
> +}
> +
> +static int visconti_pll_is_enabled(struct clk_hw *hw)
> +{
> +       struct visconti_pll *pll = to_visconti_pll(hw);
> +       unsigned long flags;
> +       u32 reg;
> +
> +       spin_lock_irqsave(pll->lock, flags);
> +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> +       spin_unlock_irqrestore(pll->lock, flags);

Don't think we need a spinlock to read a register. After this unlocks it
can be disabled anyway, so what's the point to locking here?

> +
> +       return (reg & PLL_PLLEN);
> +}
> +
> +static int visconti_pll_enable(struct clk_hw *hw)
> +{
> +       struct visconti_pll *pll = to_visconti_pll(hw);
> +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> +       unsigned long flags;
> +       u32 reg;
> +
> +       if (visconti_pll_is_enabled(hw))
> +               return 0;

Drop.

> +
> +       spin_lock_irqsave(pll->lock, flags);
> +
> +       /* Change to access via register */
> +       writel(PLL_CONFIG_SEL, pll->pll_base + PLL_CONF_REG);
> +
> +       /* Change to BYPASS mode */
> +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> +       reg |= PLL_BYPASS;
> +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> +
> +       /* Set DIV register .....*/
> +       visconti_pll_set_params(pll, &rate_table[0]);
> +
> +       /* Disable  PLL register */
> +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> +       reg &= ~PLL_PLLEN;
> +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> +
> +       /* Wait 1us */
> +       udelay(1);
> +
> +       /* Enable PLL register */
> +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> +       reg |= PLL_PLLEN;
> +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> +
> +       /* Wait 40us */
> +       udelay(40);
> +
> +       /* Change to PLL mode */

Every comment in here is saying what the code is doing. Please remove
them.

> +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> +       reg &= ~PLL_BYPASS;
> +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> +
> +       spin_unlock_irqrestore(pll->lock, flags);
> +
> +       return 0;
> +}
> +
> +static void visconti_pll_disable(struct clk_hw *hw)
> +{
> +       struct visconti_pll *pll = to_visconti_pll(hw);
> +       unsigned long flags;
> +       u32 reg;
> +
> +       if (!visconti_pll_is_enabled(hw))
> +               return;
> +
> +       spin_lock_irqsave(pll->lock, flags);
> +
> +       /* Change to access via register */

This comment is almost useful, except I don't know why we need to change
access, which is the important part.

> +       writel(PLL_CONFIG_SEL, pll->pll_base + PLL_CONF_REG);
> +
> +       /* Change to BYPASS mode */
> +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> +       reg |= PLL_BYPASS;
> +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> +
> +       /* PLL disable */
> +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> +       reg &= ~PLL_PLLEN;
> +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> +
> +       spin_unlock_irqrestore(pll->lock, flags);
> +}
> +
> +static const struct clk_ops visconti_pll_ops = {
> +       .enable = visconti_pll_enable,
> +       .disable = visconti_pll_disable,
> +       .is_enabled = visconti_pll_is_enabled,
> +       .round_rate = visconti_pll_round_rate,
> +       .recalc_rate = visconti_pll_recalc_rate,
> +       .set_rate = visconti_pll_set_rate,
> +};
> +
> +static struct clk *visconti_register_pll(struct visconti_pll_provider *ctx,
> +                                        const char *name,
> +                                        const char *parent_name,
> +                                        int offset,
> +                                        const struct visconti_pll_rate_table *rate_table,
> +                                        u8 clk_pll_flags,

Are the flags used?

> +                                        spinlock_t *lock)
> +{
> +       struct clk_init_data init;
> +       struct visconti_pll *pll;
> +       struct clk *pll_clk;
> +       int len;
> +
> +       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +       if (!pll)
> +               return ERR_PTR(-ENOMEM);
> +
> +       /* Create the actual pll */

Worthless comment.

> +       init.name = name;
> +       init.flags = CLK_IGNORE_UNUSED;
> +       init.parent_names = &parent_name;
> +       init.num_parents = 1;
> +
> +       for (len = 0; rate_table[len].rate != 0; )
> +               len++;
> +       pll->rate_count = len;
> +       pll->rate_table = kmemdup(rate_table,
> +                                 pll->rate_count * sizeof(struct visconti_pll_rate_table),
> +                                 GFP_KERNEL);

Do we need to duplicate the table? Can't that be const?

> +       WARN(!pll->rate_table, "%s: could not allocate rate table for %s\n", __func__, name);
> +
> +       init.ops = &visconti_pll_ops;
> +       pll->hw.init = &init;
> +       pll->pll_base = ctx->reg_base + offset;
> +       pll->flags = clk_pll_flags;
> +       pll->lock = lock;
> +       pll->ctx = ctx;
> +
> +       pll_clk = clk_register(NULL, &pll->hw);

Please use clk_hw_register().

> +       if (IS_ERR(pll_clk)) {
> +               pr_err("failed to register pll clock %s : %ld\n", name, PTR_ERR(pll_clk));
> +               kfree(pll);
> +       }
> +
> +       return pll_clk;
> +}
> +
> +static void visconti_pll_add_lookup(struct visconti_pll_provider *ctx, struct clk *clk,
> +                                   unsigned int id)
> +{
> +       if (ctx->clk_data.clks && id)
> +               ctx->clk_data.clks[id] = clk;
> +}
> +
> +void __init visconti_register_plls(struct visconti_pll_provider *ctx,
> +                                  const struct visconti_pll_info *list,
> +                                  unsigned int nr_plls,
> +                                  spinlock_t *lock)
> +{
> +       int idx;
> +
> +       for (idx = 0; idx < nr_plls; idx++, list++) {
> +               struct clk *clk;
> +
> +               clk = visconti_register_pll(ctx,
> +                                           list->name,
> +                                           list->parent,
> +                                           list->base_reg,
> +                                           list->rate_table,
> +                                           list->flags,
> +                                           lock);
> +               if (IS_ERR(clk)) {
> +                       pr_err("failed to register clock %s\n", list->name);
> +                       continue;
> +               }
> +
> +               visconti_pll_add_lookup(ctx, clk, list->id);
> +       }
> +}
> +
> +struct visconti_pll_provider * __init visconti_init_pll(struct device_node *np,
> +                                                       void __iomem *base,
> +                                                       unsigned long nr_plls)
> +{
> +       struct visconti_pll_provider *ctx;
> +       struct clk **clk_table;
> +       int i;
> +
> +       ctx = kzalloc(sizeof(struct visconti_pll_provider), GFP_KERNEL);
> +       if (!ctx)
> +               return ERR_PTR(-ENOMEM);
> +
> +       clk_table = kcalloc(nr_plls, sizeof(struct clk *), GFP_KERNEL);
> +       if (!clk_table)
> +               goto err;
> +
> +       for (i = 0; i < nr_plls; ++i)
> +               clk_table[i] = ERR_PTR(-ENOENT);
> +
> +       ctx->node = np;
> +       ctx->reg_base = base;
> +       ctx->clk_data.clks = clk_table;
> +       ctx->clk_data.clk_num = nr_plls;
> +
> +       return ctx;
> +
> +err:
> +       kfree(ctx);
> +       return ERR_PTR(-ENOMEM);
> +}
> diff --git a/drivers/clk/visconti/pll.h b/drivers/clk/visconti/pll.h
> new file mode 100644
> index 000000000000..ce0ce1d8cd88
> --- /dev/null
> +++ b/drivers/clk/visconti/pll.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2021 TOSHIBA CORPORATION
> + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> + *
> + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + */
> +
> +#ifndef _VISCONTI_PLL_H_
> +#define _VISCONTI_PLL_H_
> +
> +#include <linux/clk-provider.h>
> +#include <linux/regmap.h>
> +#include <linux/spinlock.h>
> +
> +struct visconti_pll_provider {
> +       void __iomem *reg_base;
> +       struct regmap *regmap;
> +       struct clk_onecell_data clk_data;
> +       struct device_node *node;
> +};
> +
> +#define VISCONTI_PLL_RATE(_rate, _dacen, _dsmen, \
> +       _refdiv, _intin, _fracin, _postdiv1, _postdiv2) \
> +{                              \
> +       .rate = _rate,          \
> +       .dacen = _dacen,        \
> +       .dsmen = _dsmen,        \
> +       .refdiv = _refdiv,      \
> +       .intin = _intin,        \
> +       .fracin = _fracin,      \
> +       .postdiv1 = _postdiv1,  \
> +       .postdiv2 = _postdiv2   \
> +}
> +
> +struct visconti_pll_rate_table {
> +       unsigned long rate;
> +       unsigned int dacen;
> +       unsigned int dsmen;
> +       unsigned int refdiv;
> +       unsigned long intin;
> +       unsigned long fracin;
> +       unsigned int postdiv1;
> +       unsigned int postdiv2;
> +};
> +
> +struct visconti_pll_info {
> +       unsigned int id;
> +       const char *name;
> +       const char *parent;
> +       u8 flags;
> +       unsigned long base_reg;
> +       const struct visconti_pll_rate_table *rate_table;
> +};
> +
> +struct visconti_pll_provider * __init visconti_init_pll(struct device_node *np,
> +                                                       void __iomem *base,
> +                                                       unsigned long nr_plls);
> +void __init visconti_register_plls(struct visconti_pll_provider *ctx,

__init can be dropped from header files. It serves no purpose.

> +                                  const struct visconti_pll_info *list,
> +                                  unsigned int nr_plls, spinlock_t *lock);
> +
> +#endif /* _VISCONTI_PLL_H_ */

Re: [PATCH v4 3/4] clk: visconti: Add support common clock driver and reset driver

[Nobuhiro Iwamatsu]

Hi,

Thanks for your review.

On Sat, Aug 28, 2021 at 10:08:20PM -0700, Stephen Boyd wrote:
> Quoting Nobuhiro Iwamatsu (2021-08-04 02:22:43)
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index 5f06879d7fe9..dbf70fdecfc8 100644
> > --- a/drivers/clk/Makefile
> > +++ b/drivers/clk/Makefile
> > @@ -116,6 +116,7 @@ obj-y                                       += ti/
> >  obj-$(CONFIG_CLK_UNIPHIER)             += uniphier/
> >  obj-$(CONFIG_ARCH_U8500)               += ux500/
> >  obj-y                                  += versatile/
> > +obj-$(CONFIG_ARCH_VISCONTI)            += visconti/
> 
> Can we have an actual Kconfig symbol for this so we can compile test the
> driver?


I will add in next version.

> 
> >  ifeq ($(CONFIG_COMMON_CLK), y)
> >  obj-$(CONFIG_X86)                      += x86/
> >  endif
> > diff --git a/drivers/clk/visconti/Makefile b/drivers/clk/visconti/Makefile
> > new file mode 100644
> > index 000000000000..c1254fd52b31
> > --- /dev/null
> > +++ b/drivers/clk/visconti/Makefile
> > @@ -0,0 +1,5 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +# Makefile for Toshiba Visconti clock
> > +
> > +obj-y  += clkc.o pll.o reset.o
> > +obj-y  += pll-tmpv770x.o clkc-tmpv770x.o
> > diff --git a/drivers/clk/visconti/clkc-tmpv770x.c b/drivers/clk/visconti/clkc-tmpv770x.c
> > new file mode 100644
> > index 000000000000..6ee5c5179f7c
> > --- /dev/null
> > +++ b/drivers/clk/visconti/clkc-tmpv770x.c
> > @@ -0,0 +1,232 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Toshiba Visconti clock controller
> > + *
> > + * Copyright (c) 2021 TOSHIBA CORPORATION
> > + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> > + *
> > + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > + */
> > +
> > +#include <dt-bindings/clock/toshiba,tmpv770x.h>
> > +#include <dt-bindings/reset/toshiba,tmpv770x.h>
> 
> Please include these after linux headers.


I will move this.

> 
> > +#include <linux/clk.h>
> 
> Is it a clk provider or a clk consumer? Hopefully it's a clk provider,
> and thus this should be clk-provider.h instead of clk.h for the
> consumer.

I will change to use clk-provider.h.

> 
> > +#include <linux/platform_device.h>
> > +
> > +#include "clkc.h"
> > +#include "reset.h"
> > +
> > +static DEFINE_SPINLOCK(tmpv770x_clk_lock);
> > +static DEFINE_SPINLOCK(tmpv770x_rst_lock);
> > +
> > +static const struct visconti_fixed_clk fixed_clk_tables[] = {
> > +       /* PLL1 */
> > +       /* PICMPT0/1, PITSC, PIUWDT, PISWDT, PISBUS, PIPMU, PIGPMU, PITMU */
> > +       /* PIEMM, PIMISC, PIGCOMM, PIDCOMM, PIMBUS, PIGPIO, PIPGM */
> > +       { TMPV770X_CLK_PIPLL1_DIV4, "pipll1_div4", "pipll1", 0, 1, 4, },
> > +       /* PISBUS */
> > +       { TMPV770X_CLK_PIPLL1_DIV2, "pipll1_div2", "pipll1", 0, 1, 2, },
> > +       /* PICOBUS_CLK */
> > +       { TMPV770X_CLK_PIPLL1_DIV1, "pipll1_div1", "pipll1", 0, 1, 1, },
> > +       /* PIDNNPLL */
> > +       /* CONN_CLK, PIMBUS, PICRC0/1 */
> > +       { TMPV770X_CLK_PIDNNPLL_DIV1, "pidnnpll_div1", "pidnnpll", 0, 1, 1, },
> > +       { TMPV770X_CLK_PIREFCLK, "pirefclk", "osc2-clk", 0, 1, 1, },
> > +       { TMPV770X_CLK_WDTCLK, "wdtclk", "osc2-clk", 0, 1, 1, },
> > +};
> > +
> > +static const struct visconti_clk_gate_table pietherpll_clk_gate_tables[] = {
> > +       /* pietherpll */
> > +       { TMPV770X_CLK_PIETHER_2P5M, "piether_2p5m", "pietherpll",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 4, 200,
> 
> There's quite a lot of CLK_IGNORE_UNUSED. Should these be
> CLK_IS_CRITICAL instead, and then only the ones that are truly critical
> to the system still running should be marked as critical along with a
> comment explaining what they're clocking and why that device isn't
> probing and enabling the clks itself.


I see. I will recheck and update this table.


> 
> > +               TMPV770X_RESET_PIETHER_2P5M, },
> > +       { TMPV770X_CLK_PIETHER_25M, "piether_25m", "pietherpll",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 5, 20,
> > +               TMPV770X_RESET_PIETHER_25M, },
> > +       { TMPV770X_CLK_PIETHER_50M, "piether_50m", "pietherpll",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 6, 10,
> > +               TMPV770X_RESET_PIETHER_50M, },
> > +       { TMPV770X_CLK_PIETHER_125M, "piether_125m", "pietherpll",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 7, 4,
> > +               TMPV770X_RESET_PIETHER_125M, },
> > +};
> > +
> > +static const struct visconti_clk_gate_table clk_gate_tables[] = {
> > +       { TMPV770X_CLK_HOX, "hox", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x4C, 0x14C, 0, 1,
> 
> Lowercase hex please.


I will fix in next version.

> 
> > +               TMPV770X_RESET_HOX, },
> > +       { TMPV770X_CLK_PCIE_MSTR, "pcie_mstr", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x38, 0x138, 0, 1,
> > +               TMPV770X_RESET_PCIE_MSTR, },
> > +       { TMPV770X_CLK_PCIE_AUX, "pcie_aux", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x38, 0x138, 1, 24,
> > +               TMPV770X_RESET_PCIE_AUX, },
> > +       { TMPV770X_CLK_PIINTC, "piintc", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x8, 0x108, 0, 2,
> > +               TMPV770X_RESET_PIINTC,},
> > +       { TMPV770X_CLK_PIETHER_BUS, "piether_bus", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x34, 0x134, 0, 2,
> > +               TMPV770X_RESET_PIETHER_BUS, }, /* BUS_CLK */
> > +       { TMPV770X_CLK_PISPI0, "pispi0", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 0, 2,
> > +               TMPV770X_RESET_PISPI0, },
> > +       { TMPV770X_CLK_PISPI1, "pispi1", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 1, 2,
> > +               TMPV770X_RESET_PISPI1, },
> > +       { TMPV770X_CLK_PISPI2, "pispi2", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 2, 2,
> > +               TMPV770X_RESET_PISPI2, },
> > +       { TMPV770X_CLK_PISPI3, "pispi3", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 3, 2,
> > +               TMPV770X_RESET_PISPI3,},
> > +       { TMPV770X_CLK_PISPI4, "pispi4", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 4, 2,
> > +               TMPV770X_RESET_PISPI4, },
> > +       { TMPV770X_CLK_PISPI5, "pispi5", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 5, 2,
> > +               TMPV770X_RESET_PISPI5},
> > +       { TMPV770X_CLK_PISPI6, "pispi6", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x28, 0x128, 6, 2,
> > +               TMPV770X_RESET_PISPI6,},
> > +       { TMPV770X_CLK_PIUART0, "piuart0", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 0, 4,
> > +               TMPV770X_RESET_PIUART0,},
> > +       { TMPV770X_CLK_PIUART1, "piuart1", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 1, 4,
> > +               TMPV770X_RESET_PIUART1, },
> > +       { TMPV770X_CLK_PIUART2, "piuart2", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 2, 4,
> > +               TMPV770X_RESET_PIUART2, },
> > +       { TMPV770X_CLK_PIUART3, "piuart3", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x2C, 0x12C, 3, 4,
> > +               TMPV770X_RESET_PIUART3, },
> > +       { TMPV770X_CLK_PII2C0, "pii2c0", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 0, 4,
> > +               TMPV770X_RESET_PII2C0, },
> > +       { TMPV770X_CLK_PII2C1, "pii2c1", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 1, 4,
> > +               TMPV770X_RESET_PII2C1, },
> > +       { TMPV770X_CLK_PII2C2, "pii2c2", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 2, 4,
> > +               TMPV770X_RESET_PII2C2, },
> > +       { TMPV770X_CLK_PII2C3, "pii2c3", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 3, 4,
> > +               TMPV770X_RESET_PII2C3,},
> > +       { TMPV770X_CLK_PII2C4, "pii2c4", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 4, 4,
> > +               TMPV770X_RESET_PII2C4, },
> > +       { TMPV770X_CLK_PII2C5, "pii2c5", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 5, 4,
> > +               TMPV770X_RESET_PII2C5, },
> > +       { TMPV770X_CLK_PII2C6, "pii2c6", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 6, 4,
> > +               TMPV770X_RESET_PII2C6, },
> > +       { TMPV770X_CLK_PII2C7, "pii2c7", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 7, 4,
> > +               TMPV770X_RESET_PII2C7, },
> > +       { TMPV770X_CLK_PII2C8, "pii2c8", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x30, 0x130, 8, 4,
> > +               TMPV770X_RESET_PII2C8, },
> > +       /* PIPCMIF */
> > +       { TMPV770X_CLK_PIPCMIF, "pipcmif", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x64, 0x164, 0, 4,
> > +               TMPV770X_RESET_PIPCMIF, },
> > +       /* PISYSTEM */
> > +       { TMPV770X_CLK_WRCK, "wrck", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x68, 0x168, 9, 32,
> > +               -1, }, /* No reset */
> > +       { TMPV770X_CLK_PICKMON, "pickmon", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x10, 0x110, 8, 4,
> > +               TMPV770X_RESET_PICKMON, },
> > +       { TMPV770X_CLK_SBUSCLK, "sbusclk", "pipll1",
> > +               CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, 0x14, 0x114, 0, 4,
> > +               TMPV770X_RESET_SBUSCLK, },
> > +};
> > +
> > +static const struct visconti_reset_data clk_reset_data[] = {
> > +       [TMPV770X_RESET_PIETHER_2P5M]   = { 0x434, 0x534, 4, },
> > +       [TMPV770X_RESET_PIETHER_25M]    = { 0x434, 0x534, 5, },
> > +       [TMPV770X_RESET_PIETHER_50M]    = { 0x434, 0x534, 6, },
> > +       [TMPV770X_RESET_PIETHER_125M]   = { 0x434, 0x534, 7, },
> > +       [TMPV770X_RESET_HOX]            = { 0x44C, 0x54C, 0, },
> > +       [TMPV770X_RESET_PCIE_MSTR]      = { 0x438, 0x538, 0, },
> > +       [TMPV770X_RESET_PCIE_AUX]       = { 0x438, 0x538, 1, },
> > +       [TMPV770X_RESET_PIINTC]         = { 0x408, 0x508, 0, },
> > +       [TMPV770X_RESET_PIETHER_BUS]    = { 0x434, 0x534, 0, },
> > +       [TMPV770X_RESET_PISPI0]         = { 0x428, 0x528, 0, },
> > +       [TMPV770X_RESET_PISPI1]         = { 0x428, 0x528, 1, },
> > +       [TMPV770X_RESET_PISPI2]         = { 0x428, 0x528, 2, },
> > +       [TMPV770X_RESET_PISPI3]         = { 0x428, 0x528, 3, },
> > +       [TMPV770X_RESET_PISPI4]         = { 0x428, 0x528, 4, },
> > +       [TMPV770X_RESET_PISPI5]         = { 0x428, 0x528, 5, },
> > +       [TMPV770X_RESET_PISPI6]         = { 0x428, 0x528, 6, },
> > +       [TMPV770X_RESET_PIUART0]        = { 0x42C, 0x52C, 0, },
> > +       [TMPV770X_RESET_PIUART1]        = { 0x42C, 0x52C, 1, },
> > +       [TMPV770X_RESET_PIUART2]        = { 0x42C, 0x52C, 2, },
> > +       [TMPV770X_RESET_PIUART3]        = { 0x42C, 0x52C, 3, },
> > +       [TMPV770X_RESET_PII2C0]         = { 0x430, 0x530, 0, },
> > +       [TMPV770X_RESET_PII2C1]         = { 0x430, 0x530, 1, },
> > +       [TMPV770X_RESET_PII2C2]         = { 0x430, 0x530, 2, },
> > +       [TMPV770X_RESET_PII2C3]         = { 0x430, 0x530, 3, },
> > +       [TMPV770X_RESET_PII2C4]         = { 0x430, 0x530, 4, },
> > +       [TMPV770X_RESET_PII2C5]         = { 0x430, 0x530, 5, },
> > +       [TMPV770X_RESET_PII2C6]         = { 0x430, 0x530, 6, },
> > +       [TMPV770X_RESET_PII2C7]         = { 0x430, 0x530, 7, },
> > +       [TMPV770X_RESET_PII2C8]         = { 0x430, 0x530, 8, },
> > +       [TMPV770X_RESET_PIPCMIF]        = { 0x464, 0x564, 0, },
> > +       [TMPV770X_RESET_PICKMON]        = { 0x410, 0x510, 8, },
> > +       [TMPV770X_RESET_SBUSCLK]        = { 0x414, 0x514, 0, },
> > +};
> > +
> > +static void __init tmpv770x_clkc_setup_clks(struct device_node *np)
> > +{
> > +       struct visconti_clk_provider *ctx;
> > +       struct regmap *regmap;
> > +       int ret, i;
> > +
> > +       regmap = device_node_to_regmap(np);
> 
> Is it inside a syscon?

Yes, this IP works as syscon. I will fix to use syscon.

> 
> > +       if (IS_ERR(regmap))
> > +               return;
> > +
> > +       ctx = visconti_init_clk(np, regmap, TMPV770X_NR_CLK);
> > +       if (IS_ERR(ctx))
> > +               return;
> > +
> > +       ret = visconti_register_reset_controller(np, regmap, clk_reset_data,
> > +                                                TMPV770X_NR_RESET,
> > +                                                &visconti_reset_ops,
> > +                                                &tmpv770x_rst_lock);
> > +       if (ret) {
> > +               pr_err("Failed to register reset controller: %d\n", ret);
> > +               return;
> > +       }
> > +
> > +       for (i = 0; i < (ARRAY_SIZE(fixed_clk_tables)); i++)
> > +               ctx->clk_data.clks[fixed_clk_tables[i].id] =
> > +                       clk_register_fixed_factor(NULL,
> > +                                               fixed_clk_tables[i].name,
> > +                                               fixed_clk_tables[i].parent,
> > +                                               fixed_clk_tables[i].flag,
> > +                                               fixed_clk_tables[i].mult,
> > +                                               fixed_clk_tables[i].div);
> > +
> > +       ret = visconti_clk_register_gates(ctx, clk_gate_tables,
> > +                                   ARRAY_SIZE(clk_gate_tables), clk_reset_data,
> > +                                   &tmpv770x_clk_lock);
> > +       if (ret) {
> > +               pr_err("Failed to register main clock gate: %d\n", ret);
> > +               return;
> > +       }
> > +
> > +       ret = visconti_clk_register_gates(ctx, pietherpll_clk_gate_tables,
> > +                                   ARRAY_SIZE(pietherpll_clk_gate_tables),
> > +                                   clk_reset_data, &tmpv770x_clk_lock);
> > +       if (ret) {
> > +               pr_err("Failed to register pietherpll clock gate: %d\n", ret);
> > +               return;
> > +       }
> > +
> > +       of_clk_add_provider(np, of_clk_src_onecell_get, &ctx->clk_data);
> > +}
> > +
> > +CLK_OF_DECLARE_DRIVER(tmpv770x_clkc, "toshiba,tmpv7708-pismu", tmpv770x_clkc_setup_clks);
> 
> Any chance this can be a platform driver?
> 

OK , I will update to platform driver.

> > diff --git a/drivers/clk/visconti/clkc.c b/drivers/clk/visconti/clkc.c
> > new file mode 100644
> > index 000000000000..32b6f6266154
> > --- /dev/null
> > +++ b/drivers/clk/visconti/clkc.c
> > @@ -0,0 +1,220 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Toshiba Visconti clock controller
> > + *
> > + * Copyright (c) 2021 TOSHIBA CORPORATION
> > + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> > + *
> > + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > + */
> > +#include <linux/clk-provider.h>
> > +#include <linux/delay.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +
> > +#include "clkc.h"
> > +
> > +static inline struct visconti_clk_gate *to_visconti_clk_gate(struct clk_hw *hw)
> > +{
> > +       return container_of(hw, struct visconti_clk_gate, hw);
> > +}
> > +
> > +static int visconti_gate_clk_is_enabled(struct clk_hw *hw)
> > +{
> > +       struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
> > +       u32 clk = BIT(gate->ck_idx);
> > +       u32 val;
> > +
> > +       regmap_read(gate->regmap, gate->ckon_offset, &val);
> > +       return (val & clk) ? 1 : 0;
> > +}
> > +
> > +static void visconti_gate_clk_disable(struct clk_hw *hw)
> > +{
> > +       struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
> > +       u32 clk = BIT(gate->ck_idx);
> > +       u32 rst = BIT(gate->rs_idx);
> > +       unsigned long flags;
> > +
> > +       spin_lock_irqsave(gate->lock, flags);
> > +
> > +       if (visconti_gate_clk_is_enabled(hw)) {
> 
> So we never disable the clk?

Thanks. I will fix.


> 
> > +               spin_unlock_irqrestore(gate->lock, flags);
> > +               return;
> > +       }
> > +
> > +       /* Reset release */
> > +       regmap_update_bits(gate->regmap, gate->rson_offset, rst, 1);
> > +
> > +       udelay(100);
> > +
> > +       /* Disable clock */
> > +       regmap_update_bits(gate->regmap, gate->ckoff_offset, clk, 1);
> > +       spin_unlock_irqrestore(gate->lock, flags);
> > +}
> > +
> > +static int visconti_gate_clk_enable(struct clk_hw *hw)
> > +{
> > +       struct visconti_clk_gate *gate = to_visconti_clk_gate(hw);
> > +       u32 clk = BIT(gate->ck_idx);
> > +       u32 rst = BIT(gate->rs_idx);
> > +       unsigned long flags;
> > +       u32 val;
> > +
> > +       spin_lock_irqsave(gate->lock, flags);
> > +
> > +       if (visconti_gate_clk_is_enabled(hw)) {
> 
> The core framework should only call down to here if the clk is actually
> disabled. This check can be removed.
> 

OK, I will remove this check.

> > +               spin_unlock_irqrestore(gate->lock, flags);
> > +               return 0;
> > +       }
> > +
> > +       regmap_update_bits(gate->regmap, gate->ckon_offset, clk, 1);
> > +       /* Need read back */
> 
> Why? Please indicate why instead of just saying what the code is doing.
> 

This was an unnecessary operation. I will delete.

> > +       regmap_read(gate->regmap, gate->ckon_offset, &val);
> > +
> > +       udelay(100);
> > +       /* Reset release */
> > +       regmap_update_bits(gate->regmap, gate->rsoff_offset, rst, 1);
> > +       /* Need read back */
> > +       regmap_read(gate->regmap, gate->ckoff_offset, &val);
> > +       spin_unlock_irqrestore(gate->lock, flags);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct clk_ops visconti_clk_gate_ops = {
> > +       .enable = visconti_gate_clk_enable,
> > +       .disable = visconti_gate_clk_disable,
> > +       .is_enabled = visconti_gate_clk_is_enabled,
> > +};
> > +
> > +static struct clk_hw *visconti_clk_register_gate(struct device *dev,
> > +                                                const char *name,
> > +                                                const char *parent_name,
> > +                                                struct regmap *regmap,
> > +                                                const struct visconti_clk_gate_table *clks,
> > +                                                u32    rson_offset,
> > +                                                u32    rsoff_offset,
> > +                                                u8     rs_idx,
> > +                                                spinlock_t *lock)
> > +{
> > +       struct visconti_clk_gate *gate;
> > +       struct clk_init_data init;
> > +       struct clk_hw *hw;
> > +       int ret;
> > +
> > +       gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> > +       if (!gate)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       init.name = name;
> > +       init.ops = &visconti_clk_gate_ops;
> > +       init.flags = clks->flags;
> > +       init.parent_names = parent_name ? &parent_name : NULL;
> > +       init.num_parents = parent_name ? 1 : 0;
> 
> Do some clks have parents and other clks don't? Or do they always have
> parents? Preferably it's one or the other. And if there are parents,
> please use clk_parent_data instead of parent_names.
> 

OK, I will update to use clk_parent_data.


> > +
> > +       gate->regmap = regmap;
> > +       gate->ckon_offset = clks->ckon_offset;
> > +       gate->ckoff_offset = clks->ckoff_offset;
> > +       gate->ck_idx = clks->ck_idx;
> > +       gate->rson_offset = rson_offset;
> > +       gate->rsoff_offset = rsoff_offset;
> > +       gate->rs_idx = rs_idx;
> > +       gate->lock = lock;
> > +       gate->hw.init = &init;
> > +
> > +       hw = &gate->hw;
> > +       ret = clk_hw_register(dev, hw);
> 
> Is the dev non-NULL? If yes then can devm be used?
> 

I will update to use devm.

> > +       if (ret) {
> > +               kfree(gate);
> > +               hw = ERR_PTR(ret);
> > +       }
> > +
> > +       return hw;
> > +}
> > +
> > +int visconti_clk_register_gates(struct visconti_clk_provider *data,
> > +                                const struct visconti_clk_gate_table *clks,
> > +                                int num_gate, const struct visconti_reset_data *reset,
> > +                                spinlock_t *lock)
> > +{
> > +       u32 rson_offset, rsoff_offset;
> > +       struct clk_hw *hw_clk;
> > +       u8 rs_idx;
> > +       int i;
> > +
> > +       for (i = 0; i < num_gate; i++) {
> > +               struct clk *clk;
> > +               char *div_name;
> > +
> > +               div_name = kasprintf(GFP_KERNEL, "%s_div", clks[i].name);
> > +               if (!div_name)
> > +                       return -ENOMEM;
> > +
> > +               if (clks[i].rs_id >= 0) {
> > +                       rson_offset = reset[clks[i].rs_id].rson_offset;
> > +                       rsoff_offset = reset[clks[i].rs_id].rsoff_offset;
> > +                       rs_idx = reset[clks[i].rs_id].rs_idx;
> > +               } else {
> > +                       rson_offset = rsoff_offset = rs_idx = -1;
> > +               }
> > +
> > +               clk = clk_register_fixed_factor(NULL, div_name, clks[i].parent,
> > +                                               0, 1, clks[i].div);
> > +               if (IS_ERR(clk))
> > +                       return PTR_ERR(clk);
> > +
> > +               hw_clk = visconti_clk_register_gate(NULL,
> 
> Oh it's always NULL? Sad

I will update this fuction, and avoid using NULL.

> 
> > +                                               clks[i].name,
> > +                                               div_name,
> > +                                               data->regmap,
> > +                                               &clks[i],
> > +                                               rson_offset,
> > +                                               rsoff_offset,
> > +                                               rs_idx,
> > +                                               lock);
> > +               kfree(div_name);
> > +               if (IS_ERR(hw_clk)) {
> > +                       pr_err("%s: failed to register clock %s\n",
> > +                              __func__, clks[i].name);
> > +                       return PTR_ERR(hw_clk);
> > +               }
> > +
> > +               data->clk_data.clks[clks[i].id] = hw_clk->clk;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +struct visconti_clk_provider *visconti_init_clk(struct device_node *np,
> > +                                               struct regmap *regmap,
> > +                                               unsigned long nr_clks)
> > +{
> > +       struct visconti_clk_provider *ctx;
> > +       struct clk **clk_table;
> > +       int i;
> > +
> > +       ctx = kzalloc(sizeof(struct visconti_clk_provider), GFP_KERNEL);
> 
> sizeof(*ctx)?

I will fix.

> 
> > +       if (!ctx)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       clk_table = kcalloc(nr_clks, sizeof(struct clk *), GFP_KERNEL);
> 
> sizeof(*clk_table)?
> 

I will fix.


> > +       if (!clk_table)
> > +               goto err;
> > +
> > +       for (i = 0; i < nr_clks; ++i)
> > +               clk_table[i] = ERR_PTR(-ENOENT);
> > +       ctx->node = np;
> > +       ctx->regmap = regmap;
> > +       ctx->clk_data.clks = clk_table;
> > +       ctx->clk_data.clk_num = nr_clks;
> > +
> > +       return ctx;
> > +err:
> > +       kfree(ctx);
> > +       return ERR_PTR(-ENOMEM);
> > +}
> > diff --git a/drivers/clk/visconti/clkc.h b/drivers/clk/visconti/clkc.h
> > new file mode 100644
> > index 000000000000..4df8788d635a
> > --- /dev/null
> > +++ b/drivers/clk/visconti/clkc.h
> > @@ -0,0 +1,75 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Toshiba Visconti clock controller
> > + *
> > + * Copyright (c) 2021 TOSHIBA CORPORATION
> > + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> > + *
> > + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > + */
> > +
> > +#ifndef _VISCONTI_CLKC_H_
> > +#define _VISCONTI_CLKC_H_
> > +
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/delay.h>
> > +#include <linux/regmap.h>
> > +#include <linux/slab.h>
> > +#include <linux/string.h>
> > +#include <linux/io.h>
> > +#include <linux/spinlock.h>
> > +
> > +#include "reset.h"
> > +
> > +struct visconti_clk_provider {
> > +       struct device_node      *node;
> > +       struct regmap           *regmap;
> > +       struct clk_onecell_data clk_data;
> > +};
> > +
> > +struct visconti_clk_gate_table {
> > +       unsigned int    id;
> > +       const char      *name;
> > +       const char      *parent;
> > +       u8              flags;
> > +       u32             ckon_offset;
> > +       u32             ckoff_offset;
> > +       u8              ck_idx;
> > +       unsigned int    div;
> > +       u8              rs_id;
> > +};
> > +
> > +struct visconti_fixed_clk {
> > +       unsigned int    id;
> > +       const char      *name;
> > +       const char      *parent;
> > +       unsigned long   flag;
> > +       unsigned int    mult;
> > +       unsigned int    div;
> > +};
> > +
> > +struct visconti_clk_gate {
> > +       struct clk_hw   hw;
> > +       struct regmap   *regmap;
> > +       u32             ckon_offset;
> > +       u32             ckoff_offset;
> > +       u8              ck_idx;
> > +       u8              flags;
> > +       u32             rson_offset;
> > +       u32             rsoff_offset;
> > +       u8              rs_idx;
> > +       spinlock_t      *lock;
> > +};
> > +
> > +struct visconti_clk_provider *visconti_init_clk(struct device_node *np,
> > +                                               struct regmap *regmap,
> > +                                               unsigned long nr_clks);
> > +int visconti_clk_register_gates(struct visconti_clk_provider *data,
> > +                                const struct visconti_clk_gate_table *clks,
> > +                                int num_gate,
> > +                                const struct visconti_reset_data *reset,
> > +                                spinlock_t *lock);
> > +#endif /* _VISCONTI_CLKC_H_ */
> > diff --git a/drivers/clk/visconti/pll-tmpv770x.c b/drivers/clk/visconti/pll-tmpv770x.c
> > new file mode 100644
> > index 000000000000..a0874cb6a67b
> > --- /dev/null
> > +++ b/drivers/clk/visconti/pll-tmpv770x.c
> > @@ -0,0 +1,85 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Toshiba Visconti PLL controller
> > + *
> > + * Copyright (c) 2021 TOSHIBA CORPORATION
> > + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> > + *
> > + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > + */
> > +
> > +#include <dt-bindings/clock/toshiba,tmpv770x.h>
> > +#include <linux/clk.h>
> > +#include <linux/clk-provider.h>
> > +#include <linux/of_address.h>
> > +#include <linux/slab.h>
> > +
> > +#include "pll.h"
> > +
> > +static DEFINE_SPINLOCK(tmpv770x_pll_lock);
> > +
> > +static const struct visconti_pll_rate_table pipll0_rates[] __initconst = {
> > +       VISCONTI_PLL_RATE(840000000, 0x1, 0x0, 0x1, 0x54, 0x000000, 0x2, 0x1),
> > +       VISCONTI_PLL_RATE(780000000, 0x1, 0x0, 0x1, 0x4E, 0x000000, 0x2, 0x1),
> > +       VISCONTI_PLL_RATE(600000000, 0x1, 0x0, 0x1, 0x3C, 0x000000, 0x2, 0x1),
> > +       { /* sentinel */ },
> > +};
> > +
> > +static const struct visconti_pll_rate_table piddrcpll_rates[] __initconst = {
> > +       VISCONTI_PLL_RATE(780000000, 0x1, 0x0, 0x1, 0x4E, 0x000000, 0x2, 0x1),
> > +       VISCONTI_PLL_RATE(760000000, 0x1, 0x0, 0x1, 0x4C, 0x000000, 0x2, 0x1),
> > +       { /* sentinel */ },
> > +};
> > +
> > +static const struct visconti_pll_rate_table pivoifpll_rates[] __initconst = {
> > +       VISCONTI_PLL_RATE(165000000, 0x1, 0x0, 0x1, 0x42, 0x000000, 0x4, 0x2),
> > +       VISCONTI_PLL_RATE(148500000, 0x1, 0x1, 0x1, 0x3B, 0x666666, 0x4, 0x2),
> > +       VISCONTI_PLL_RATE(96000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x2),
> > +       VISCONTI_PLL_RATE(74250000, 0x1, 0x1, 0x1, 0x3B, 0x666666, 0x4, 0x4),
> > +       VISCONTI_PLL_RATE(54000000, 0x1, 0x0, 0x1, 0x36, 0x000000, 0x5, 0x4),
> > +       VISCONTI_PLL_RATE(48000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x4),
> > +       VISCONTI_PLL_RATE(35750000, 0x1, 0x1, 0x1, 0x32, 0x0CCCCC, 0x7, 0x4),
> > +       { /* sentinel */ },
> > +};
> > +
> > +static const struct visconti_pll_rate_table piimgerpll_rates[] __initconst = {
> > +       VISCONTI_PLL_RATE(165000000, 0x1, 0x0, 0x1, 0x42, 0x000000, 0x4, 0x2),
> > +       VISCONTI_PLL_RATE(96000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x2),
> > +       VISCONTI_PLL_RATE(54000000, 0x1, 0x0, 0x1, 0x36, 0x000000, 0x5, 0x4),
> > +       VISCONTI_PLL_RATE(48000000, 0x1, 0x0, 0x1, 0x30, 0x000000, 0x5, 0x4),
> > +       { /* sentinel */ },
> > +};
> > +
> > +static const struct visconti_pll_info pll_info[] __initconst = {
> > +       { TMPV770X_PLL_PIPLL0, "pipll0", "osc2-clk", 0, 0x0, pipll0_rates },
> > +       { TMPV770X_PLL_PIDDRCPLL, "piddrcpll", "osc2-clk", 0, 0x500, piddrcpll_rates },
> > +       { TMPV770X_PLL_PIVOIFPLL, "pivoifpll", "osc2-clk", 0, 0x600, pivoifpll_rates },
> > +       { TMPV770X_PLL_PIIMGERPLL, "piimgerpll", "osc2-clk", 0, 0x700, piimgerpll_rates },
> > +};
> > +
> > +static void __init tmpv770x_setup_plls(struct device_node *np)
> > +{
> > +       struct visconti_pll_provider *ctx;
> > +       void __iomem *reg_base;
> > +
> > +       reg_base = of_iomap(np, 0);
> > +       if (!reg_base)
> > +               return;
> > +
> > +       ctx = visconti_init_pll(np, reg_base, TMPV770X_NR_PLL);
> > +       if (IS_ERR(ctx)) {
> > +               iounmap(reg_base);
> > +               return;
> > +       }
> > +
> > +       ctx->clk_data.clks[TMPV770X_PLL_PIPLL1] =
> > +               clk_register_fixed_rate(NULL, "pipll1", NULL, 0, 600000000);
> > +       ctx->clk_data.clks[TMPV770X_PLL_PIDNNPLL] =
> > +               clk_register_fixed_rate(NULL, "pidnnpll", NULL, 0, 500000000);
> > +       ctx->clk_data.clks[TMPV770X_PLL_PIETHERPLL] =
> > +               clk_register_fixed_rate(NULL, "pietherpll", NULL, 0, 500000000);
> > +
> > +       visconti_register_plls(ctx, pll_info, ARRAY_SIZE(pll_info), &tmpv770x_pll_lock);
> > +}
> > +
> > +CLK_OF_DECLARE_DRIVER(tmpv770x_plls, "toshiba,tmpv7708-pipllct", tmpv770x_setup_plls);
> 
> DECLARE_DRIVER is for when more than one device needs to probe the node
> in DT. Is that happening here? If so, please add a comment indicating
> what the other driver is.


I see. This driver does not need to use DECLARE_DRIVER. I had the wrong understanding. I will switch to the platform driver.

> 
> > diff --git a/drivers/clk/visconti/pll.c b/drivers/clk/visconti/pll.c
> > new file mode 100644
> > index 000000000000..7baa9512fad9
> > --- /dev/null
> > +++ b/drivers/clk/visconti/pll.c
> > @@ -0,0 +1,369 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Toshiba Visconti PLL driver
> > + *
> > + * Copyright (c) 2021 TOSHIBA CORPORATION
> > + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> > + *
> > + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/delay.h>
> > +#include <linux/slab.h>
> > +#include <linux/io.h>
> > +
> > +#include "pll.h"
> > +
> > +struct visconti_pll {
> > +       struct clk_hw   hw;
> > +       void __iomem    *pll_base;
> > +       spinlock_t      *lock;
> > +       int rate_count;
> 
> size_t? And then place next to rate_table so we know they're related
> please.

OK, I will fix to use size_t, and move.

> 
> > +       unsigned long flags;
> > +       const struct visconti_pll_rate_table *rate_table;
> > +       struct visconti_pll_provider *ctx;
> > +};
> > +
> > +#define PLL_CONF_REG           0x0000
> > +#define PLL_CTRL_REG           0x0004
> > +#define PLL_FRACMODE_REG       0x0010
> > +#define PLL_INTIN_REG          0x0014
> > +#define PLL_FRACIN_REG         0x0018
> > +#define PLL_REFDIV_REG         0x001C
> > +#define PLL_POSTDIV_REG                0x0020
> > +
> > +#define PLL_CONFIG_SEL         BIT(0)
> > +#define PLL_PLLEN              BIT(4)
> > +#define PLL_BYPASS             BIT(16)
> > +#define PLL_INTIN_MASK         GENMASK(11, 0)
> > +#define PLL_FRACIN_MASK                GENMASK(23, 0)
> > +#define PLL_REFDIV_MASK                GENMASK(5, 0)
> > +#define PLL_POSTDIV_MASK       GENMASK(2, 0)
> > +
> > +static inline struct visconti_pll *to_visconti_pll(struct clk_hw *hw)
> > +{
> > +       return container_of(hw, struct visconti_pll, hw);
> > +}
> > +
> > +static void visconti_pll_get_params(struct visconti_pll *pll,
> > +                                   struct visconti_pll_rate_table *rate_table)
> > +{
> > +       u32 postdiv, val;
> > +
> > +       val = readl(pll->pll_base + PLL_FRACMODE_REG);
> > +       rate_table->dacen = (val >> 4) & 0x1;
> 
> What is 0x1? Some bitfield? Can it be a #define at least? Same for 4.
> Better yet, use GENMASK and FIELD_PREP() and FIELD_GET() macros.

I will fix to use macros.

> 
> > +       rate_table->dsmen = val & 0x1;
> > +       rate_table->fracin = readl(pll->pll_base + PLL_FRACIN_REG) & PLL_FRACIN_MASK;
> > +       rate_table->intin = readl(pll->pll_base + PLL_INTIN_REG) & PLL_INTIN_MASK;
> > +       rate_table->refdiv = readl(pll->pll_base + PLL_REFDIV_REG) & PLL_REFDIV_MASK;
> > +
> > +       postdiv = readl(pll->pll_base + PLL_POSTDIV_REG);
> > +       rate_table->postdiv1 = postdiv & PLL_POSTDIV_MASK;
> > +       rate_table->postdiv2 = (postdiv >> 4) & PLL_POSTDIV_MASK;
> > +}
> > +
> > +static const struct visconti_pll_rate_table *visconti_get_pll_settings(struct visconti_pll *pll,
> > +                                                                      unsigned long rate)
> > +{
> > +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> > +       int i;
> > +
> > +       for (i = 0; i < pll->rate_count; i++) {
> > +               if (rate == rate_table[i].rate)
> > +                       return &rate_table[i];
> > +       }
> 
> Drop braces please.
> 

I will drop.

> > +
> > +       return NULL;
> > +}
> > +
> > +static unsigned long visconti_get_pll_rate_from_data(struct visconti_pll *pll,
> > +                                                    struct visconti_pll_rate_table *rate)
> 
> const pointers?

I will fix to use const pointer.

> 
> > +{
> > +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> > +       int i;
> > +
> > +       for (i = 0; i < pll->rate_count; i++) {
> > +               if (rate_table[i].dacen == rate->dacen &&
> > +                       rate_table[i].dsmen == rate->dsmen &&
> > +                       rate_table[i].fracin == rate->fracin &&
> > +                       rate_table[i].intin == rate->intin &&
> > +                       rate_table[i].refdiv == rate->refdiv &&
> > +                       rate_table[i].postdiv1 == rate->postdiv1 &&
> > +                       rate_table[i].postdiv2 == rate->postdiv2)
> 
> Is it not the same table? Can we just compare the two pointers instead
> of each member?

OK, I will update to use memcmp or others.


> 
> > +                       return rate_table[i].rate;
> > +       }
> > +
> > +       /* set default */
> > +       return rate_table[0].rate;
> > +}
> > +
> > +static long visconti_pll_round_rate(struct clk_hw *hw,
> > +                           unsigned long rate, unsigned long *prate)
> > +{
> > +       struct visconti_pll *pll = to_visconti_pll(hw);
> > +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> > +       int i;
> > +
> > +       /* Assumming rate_table is in descending order */
> > +       for (i = 0; i < pll->rate_count; i++) {
> > +               if (rate >= rate_table[i].rate)
> > +                       return rate_table[i].rate;
> > +       }
> 
> Drop braces please.

I will drop.

> 
> > +
> > +       /* return minimum supported value */
> > +       return rate_table[i - 1].rate;
> > +}
> > +
> > +static unsigned long visconti_pll_recalc_rate(struct clk_hw *hw,
> > +                                             unsigned long parent_rate)
> > +{
> > +       struct visconti_pll *pll = to_visconti_pll(hw);
> > +       struct visconti_pll_rate_table rate_table;
> > +
> > +       memset(&rate_table, 0, sizeof(rate_table));
> > +       visconti_pll_get_params(pll, &rate_table);
> > +
> > +       return visconti_get_pll_rate_from_data(pll, &rate_table);
> 
> Oh. Can we do math instead?
> 
> > +}
> > +
> > +static int visconti_pll_set_params(struct visconti_pll *pll,
> > +                               const struct visconti_pll_rate_table *rate_table)
> > +{
> > +       /* update pll values */
> 
> This comment is worthless. Please remove.

I will drop.

> 
> > +       writel(((rate_table->dacen << 4) | rate_table->dsmen),
> 
> One too many parenthesis here. Please clean it up.
> 
> > +                       pll->pll_base + PLL_FRACMODE_REG);
> > +       writel(((rate_table->postdiv2 << 4) | rate_table->postdiv1),
> 
> Same.
> 
OK, I will fix to use macros.


> > +                       pll->pll_base + PLL_POSTDIV_REG);
> > +       writel(rate_table->intin, pll->pll_base + PLL_INTIN_REG);
> > +       writel(rate_table->fracin, pll->pll_base + PLL_FRACIN_REG);
> > +       writel(rate_table->refdiv, pll->pll_base + PLL_REFDIV_REG);
> > +
> > +       return 0;
> > +}
> > +
> > +static int visconti_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> > +                           unsigned long parent_rate)
> > +{
> > +       struct visconti_pll *pll = to_visconti_pll(hw);
> > +       const struct visconti_pll_rate_table *rate_table;
> > +
> > +       /* Get required rate settings from table */
> 
> This comment is worthless, please remove.
> 

I will drop.


> > +       rate_table = visconti_get_pll_settings(pll, rate);
> > +       if (!rate_table) {
> > +               pr_err("Invalid rate : %lu for pll clk %s\n",
> 
> Please don't print anything during set_rate. It can be quite noisy if
> something like a cpufreq driver keeps calling this with the incorrect
> frequency.

OK, I will drop this.

> 
> > +                       parent_rate, __clk_get_name(hw->clk));
> > +               return -EINVAL;
> > +       }
> > +
> > +       return visconti_pll_set_params(pll, rate_table);
> > +}
> > +
> > +static int visconti_pll_is_enabled(struct clk_hw *hw)
> > +{
> > +       struct visconti_pll *pll = to_visconti_pll(hw);
> > +       unsigned long flags;
> > +       u32 reg;
> > +
> > +       spin_lock_irqsave(pll->lock, flags);
> > +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> > +       spin_unlock_irqrestore(pll->lock, flags);
> 
> Don't think we need a spinlock to read a register. After this unlocks it
> can be disabled anyway, so what's the point to locking here?
> 

Indeed. I will drop lock and unlock.

> > +
> > +       return (reg & PLL_PLLEN);
> > +}
> > +
> > +static int visconti_pll_enable(struct clk_hw *hw)
> > +{
> > +       struct visconti_pll *pll = to_visconti_pll(hw);
> > +       const struct visconti_pll_rate_table *rate_table = pll->rate_table;
> > +       unsigned long flags;
> > +       u32 reg;
> > +
> > +       if (visconti_pll_is_enabled(hw))
> > +               return 0;
> 
> Drop.


I will drop.

> 
> > +
> > +       spin_lock_irqsave(pll->lock, flags);
> > +
> > +       /* Change to access via register */
> > +       writel(PLL_CONFIG_SEL, pll->pll_base + PLL_CONF_REG);
> > +
> > +       /* Change to BYPASS mode */
> > +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> > +       reg |= PLL_BYPASS;
> > +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> > +
> > +       /* Set DIV register .....*/
> > +       visconti_pll_set_params(pll, &rate_table[0]);
> > +
> > +       /* Disable  PLL register */
> > +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> > +       reg &= ~PLL_PLLEN;
> > +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> > +
> > +       /* Wait 1us */
> > +       udelay(1);
> > +
> > +       /* Enable PLL register */
> > +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> > +       reg |= PLL_PLLEN;
> > +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> > +
> > +       /* Wait 40us */
> > +       udelay(40);
> > +
> > +       /* Change to PLL mode */
> 
> Every comment in here is saying what the code is doing. Please remove
> them.

OK, I will drop.

> 
> > +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> > +       reg &= ~PLL_BYPASS;
> > +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> > +
> > +       spin_unlock_irqrestore(pll->lock, flags);
> > +
> > +       return 0;
> > +}
> > +
> > +static void visconti_pll_disable(struct clk_hw *hw)
> > +{
> > +       struct visconti_pll *pll = to_visconti_pll(hw);
> > +       unsigned long flags;
> > +       u32 reg;
> > +
> > +       if (!visconti_pll_is_enabled(hw))
> > +               return;
> > +
> > +       spin_lock_irqsave(pll->lock, flags);
> > +
> > +       /* Change to access via register */
> 
> This comment is almost useful, except I don't know why we need to change
> access, which is the important part.
> 

The content of this comment was unclear.
This operation unlocks the parameter setting via the register.
I will update.

> > +       writel(PLL_CONFIG_SEL, pll->pll_base + PLL_CONF_REG);
> > +
> > +       /* Change to BYPASS mode */
> > +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> > +       reg |= PLL_BYPASS;
> > +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> > +
> > +       /* PLL disable */
> > +       reg = readl(pll->pll_base + PLL_CTRL_REG);
> > +       reg &= ~PLL_PLLEN;
> > +       writel(reg, pll->pll_base + PLL_CTRL_REG);
> > +
> > +       spin_unlock_irqrestore(pll->lock, flags);
> > +}
> > +
> > +static const struct clk_ops visconti_pll_ops = {
> > +       .enable = visconti_pll_enable,
> > +       .disable = visconti_pll_disable,
> > +       .is_enabled = visconti_pll_is_enabled,
> > +       .round_rate = visconti_pll_round_rate,
> > +       .recalc_rate = visconti_pll_recalc_rate,
> > +       .set_rate = visconti_pll_set_rate,
> > +};
> > +
> > +static struct clk *visconti_register_pll(struct visconti_pll_provider *ctx,
> > +                                        const char *name,
> > +                                        const char *parent_name,
> > +                                        int offset,
> > +                                        const struct visconti_pll_rate_table *rate_table,
> > +                                        u8 clk_pll_flags,
> 
> Are the flags used?
> 

Not use. I will drop.

> > +                                        spinlock_t *lock)
> > +{
> > +       struct clk_init_data init;
> > +       struct visconti_pll *pll;
> > +       struct clk *pll_clk;
> > +       int len;
> > +
> > +       pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> > +       if (!pll)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       /* Create the actual pll */
> 
> Worthless comment.
> 

I will drop.

> > +       init.name = name;
> > +       init.flags = CLK_IGNORE_UNUSED;
> > +       init.parent_names = &parent_name;
> > +       init.num_parents = 1;
> > +
> > +       for (len = 0; rate_table[len].rate != 0; )
> > +               len++;
> > +       pll->rate_count = len;
> > +       pll->rate_table = kmemdup(rate_table,
> > +                                 pll->rate_count * sizeof(struct visconti_pll_rate_table),
> > +                                 GFP_KERNEL);
> 
> Do we need to duplicate the table? Can't that be const?
> 

I see. I will fix this.

> > +       WARN(!pll->rate_table, "%s: could not allocate rate table for %s\n", __func__, name);
> > +
> > +       init.ops = &visconti_pll_ops;
> > +       pll->hw.init = &init;
> > +       pll->pll_base = ctx->reg_base + offset;
> > +       pll->flags = clk_pll_flags;
> > +       pll->lock = lock;
> > +       pll->ctx = ctx;
> > +
> > +       pll_clk = clk_register(NULL, &pll->hw);
> 
> Please use clk_hw_register().

OK, I will update to use clk_hw_register.


> 
> > +       if (IS_ERR(pll_clk)) {
> > +               pr_err("failed to register pll clock %s : %ld\n", name, PTR_ERR(pll_clk));
> > +               kfree(pll);
> > +       }
> > +
> > +       return pll_clk;
> > +}
> > +
> > +static void visconti_pll_add_lookup(struct visconti_pll_provider *ctx, struct clk *clk,
> > +                                   unsigned int id)
> > +{
> > +       if (ctx->clk_data.clks && id)
> > +               ctx->clk_data.clks[id] = clk;
> > +}
> > +
> > +void __init visconti_register_plls(struct visconti_pll_provider *ctx,
> > +                                  const struct visconti_pll_info *list,
> > +                                  unsigned int nr_plls,
> > +                                  spinlock_t *lock)
> > +{
> > +       int idx;
> > +
> > +       for (idx = 0; idx < nr_plls; idx++, list++) {
> > +               struct clk *clk;
> > +
> > +               clk = visconti_register_pll(ctx,
> > +                                           list->name,
> > +                                           list->parent,
> > +                                           list->base_reg,
> > +                                           list->rate_table,
> > +                                           list->flags,
> > +                                           lock);
> > +               if (IS_ERR(clk)) {
> > +                       pr_err("failed to register clock %s\n", list->name);
> > +                       continue;
> > +               }
> > +
> > +               visconti_pll_add_lookup(ctx, clk, list->id);
> > +       }
> > +}
> > +
> > +struct visconti_pll_provider * __init visconti_init_pll(struct device_node *np,
> > +                                                       void __iomem *base,
> > +                                                       unsigned long nr_plls)
> > +{
> > +       struct visconti_pll_provider *ctx;
> > +       struct clk **clk_table;
> > +       int i;
> > +
> > +       ctx = kzalloc(sizeof(struct visconti_pll_provider), GFP_KERNEL);
> > +       if (!ctx)
> > +               return ERR_PTR(-ENOMEM);
> > +
> > +       clk_table = kcalloc(nr_plls, sizeof(struct clk *), GFP_KERNEL);
> > +       if (!clk_table)
> > +               goto err;
> > +
> > +       for (i = 0; i < nr_plls; ++i)
> > +               clk_table[i] = ERR_PTR(-ENOENT);
> > +
> > +       ctx->node = np;
> > +       ctx->reg_base = base;
> > +       ctx->clk_data.clks = clk_table;
> > +       ctx->clk_data.clk_num = nr_plls;
> > +
> > +       return ctx;
> > +
> > +err:
> > +       kfree(ctx);
> > +       return ERR_PTR(-ENOMEM);
> > +}
> > diff --git a/drivers/clk/visconti/pll.h b/drivers/clk/visconti/pll.h
> > new file mode 100644
> > index 000000000000..ce0ce1d8cd88
> > --- /dev/null
> > +++ b/drivers/clk/visconti/pll.h
> > @@ -0,0 +1,63 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (c) 2021 TOSHIBA CORPORATION
> > + * Copyright (c) 2021 Toshiba Electronic Devices & Storage Corporation
> > + *
> > + * Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> > + */
> > +
> > +#ifndef _VISCONTI_PLL_H_
> > +#define _VISCONTI_PLL_H_
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/regmap.h>
> > +#include <linux/spinlock.h>
> > +
> > +struct visconti_pll_provider {
> > +       void __iomem *reg_base;
> > +       struct regmap *regmap;
> > +       struct clk_onecell_data clk_data;
> > +       struct device_node *node;
> > +};
> > +
> > +#define VISCONTI_PLL_RATE(_rate, _dacen, _dsmen, \
> > +       _refdiv, _intin, _fracin, _postdiv1, _postdiv2) \
> > +{                              \
> > +       .rate = _rate,          \
> > +       .dacen = _dacen,        \
> > +       .dsmen = _dsmen,        \
> > +       .refdiv = _refdiv,      \
> > +       .intin = _intin,        \
> > +       .fracin = _fracin,      \
> > +       .postdiv1 = _postdiv1,  \
> > +       .postdiv2 = _postdiv2   \
> > +}
> > +
> > +struct visconti_pll_rate_table {
> > +       unsigned long rate;
> > +       unsigned int dacen;
> > +       unsigned int dsmen;
> > +       unsigned int refdiv;
> > +       unsigned long intin;
> > +       unsigned long fracin;
> > +       unsigned int postdiv1;
> > +       unsigned int postdiv2;
> > +};
> > +
> > +struct visconti_pll_info {
> > +       unsigned int id;
> > +       const char *name;
> > +       const char *parent;
> > +       u8 flags;
> > +       unsigned long base_reg;
> > +       const struct visconti_pll_rate_table *rate_table;
> > +};
> > +
> > +struct visconti_pll_provider * __init visconti_init_pll(struct device_node *np,
> > +                                                       void __iomem *base,
> > +                                                       unsigned long nr_plls);
> > +void __init visconti_register_plls(struct visconti_pll_provider *ctx,
> 
> __init can be dropped from header files. It serves no purpose.
> 

OK, I will drop.


> > +                                  const struct visconti_pll_info *list,
> > +                                  unsigned int nr_plls, spinlock_t *lock);
> > +
> > +#endif /* _VISCONTI_PLL_H_ */
> 


Best regards,
  Nobuhiro

[PATCH v4 4/4] MAINTAINERS: Add entries for Toshiba Visconti PLL and clock controller

[Nobuhiro Iwamatsu]

Add entries for Toshiba Visconti PLL and clock controller binding and
driver.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bd7aff0c120f..334d482810e7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2661,11 +2661,14 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/iwamatsu/linux-visconti.git
 F:	Documentation/devicetree/bindings/arm/toshiba.yaml
+F:	Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pipllct.yaml
+F:	Documentation/devicetree/bindings/clock/toshiba,tmpv770x-pismu.yaml
 F:	Documentation/devicetree/bindings/net/toshiba,visconti-dwmac.yaml
 F:	Documentation/devicetree/bindings/gpio/toshiba,gpio-visconti.yaml
 F:	Documentation/devicetree/bindings/pinctrl/toshiba,tmpv7700-pinctrl.yaml
 F:	Documentation/devicetree/bindings/watchdog/toshiba,visconti-wdt.yaml
 F:	arch/arm64/boot/dts/toshiba/
+F:	drivers/clk/visconti/
 F:	drivers/net/ethernet/stmicro/stmmac/dwmac-visconti.c
 F:	drivers/gpio/gpio-visconti.c
 F:	drivers/pinctrl/visconti/
-- 
2.32.0