2021-08-04 10:07:32

by Nobuhiro Iwamatsu

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

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




2021-08-04 10:07:53

by Nobuhiro Iwamatsu

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

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

Signed-off-by: Nobuhiro Iwamatsu <[email protected]>
---
.../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 <[email protected]>
+
+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



2021-08-04 10:08:11

by Nobuhiro Iwamatsu

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

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 <[email protected]>
---
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 <[email protected]>
+ */
+
+#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 <[email protected]>
+ */
+#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 <[email protected]>
+ */
+
+#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 <[email protected]>
+ */
+
+#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 <[email protected]>
+ */
+
+#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 <[email protected]>
+ */
+
+#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 <[email protected]>
+ */
+#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 <[email protected]>
+ */
+
+#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



2021-08-04 10:08:15

by Nobuhiro Iwamatsu

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

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

Signed-off-by: Nobuhiro Iwamatsu <[email protected]>
---
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: [email protected] (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



2021-08-04 10:45:51

by Nobuhiro Iwamatsu

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

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

Signed-off-by: Nobuhiro Iwamatsu <[email protected]>
---
.../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 <[email protected]>
+
+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



2021-08-11 18:55:47

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> .../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 <[email protected]>

2021-08-11 18:56:23

by Rob Herring (Arm)

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

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 <[email protected]>
> ---
> .../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 <[email protected]>

2021-08-29 05:11:52

by Stephen Boyd

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

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 <[email protected]>
> + */
> +
> +#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 <[email protected]>
> + */
> +#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 <[email protected]>
> + */
> +
> +#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 <[email protected]>
> + */
> +
> +#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 <[email protected]>
> + */
> +
> +#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 <[email protected]>
> + */
> +
> +#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_ */

2021-08-29 05:13:04

by Stephen Boyd

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

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 <[email protected]>
> ---
> .../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 <[email protected]>
> +
> +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>;
> + };
> + };
> +...

2021-08-31 02:16:39

by Nobuhiro Iwamatsu

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

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 <[email protected]>
> > ---
> > .../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 <[email protected]>
> > +
> > +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

2021-10-25 00:58:01

by Nobuhiro Iwamatsu

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

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 <[email protected]>
> > + */
> > +
> > +#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 <[email protected]>
> > + */
> > +#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 <[email protected]>
> > + */
> > +
> > +#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 <[email protected]>
> > + */
> > +
> > +#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 <[email protected]>
> > + */
> > +
> > +#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 <[email protected]>
> > + */
> > +
> > +#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