[PATCH v2 0/6] Add PLL clocks driver for StarFive JH7110 SoC

[PATCH v2 0/6] Add PLL clocks driver for StarFive JH7110 SoC

[Xingyu Wu]

This patch serises are to add PLL clocks driver and providers by writing
and reading syscon registers for the StarFive JH7110 RISC-V SoC.

PLL are high speed, low jitter frequency synthesizers in JH7110.
Each PLL clocks work in integer mode or fraction mode by some dividers,
and the dividers are set in several syscon registers.
The formula for calculating frequency is:
Fvco = Fref * (NI + NF) / M / Q1

The first patch adds docunmentation to describe PLL clock bindings,
and the second patch adds driver to support PLL clocks for JH7110.
The patch 3 modifies the syscon dt-bindings and adds optional
pateerProperties about PLL clock controller. The patch 4 modifies the
SYSCRG dibindings and adds PLL clock inputs. The patch 5 modifies the
system clock driver and changes PLL clock source from PLL clock
controller instead of the fixed factor clocks. The last patch adds PLL
clock node and modifies the syscrg node in dts file.

This patchset should be applied after these patchset about JH7110 clock
driver[1] and syscon dt-bindings[2].
[1] https://lore.kernel.org/all/20230221024645.127922-1-hal.feng@starfivetech.com/
[2] https://lore.kernel.org/all/20230315055813.94740-1-william.qiu@starfivetech.com/

Changes since v1:
- Changed PLL clock node to be child of syscon node in dts.
- Modifed the definitions and names of function in PLL clock driver.
- Added commit to update syscon and syscrg dt-bindings.

Xingyu Wu (6):
  dt-bindings: clock: Add StarFive JH7110 PLL clock generator
  clk: starfive: Add StarFive JH7110 PLL clock driver
  dt-bindings: soc: starfive: syscon: Add optional patternProperties
  dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs
  clk: starfive: jh7110-sys: Modify PLL clocks source
  riscv: dts: starfive: jh7110: Add PLL clock node and modify syscrg
    node

 .../bindings/clock/starfive,jh7110-pll.yaml   |  46 ++
 .../clock/starfive,jh7110-syscrg.yaml         |  20 +-
 .../soc/starfive/starfive,jh7110-syscon.yaml  |  39 +-
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  16 +-
 drivers/clk/starfive/Kconfig                  |   9 +
 drivers/clk/starfive/Makefile                 |   1 +
 .../clk/starfive/clk-starfive-jh7110-pll.c    | 420 ++++++++++++++++++
 .../clk/starfive/clk-starfive-jh7110-pll.h    | 293 ++++++++++++
 .../clk/starfive/clk-starfive-jh7110-sys.c    |  35 +-
 .../dt-bindings/clock/starfive,jh7110-crg.h   |   6 +
 10 files changed, 845 insertions(+), 40 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.c
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.h


base-commit: 8ca09d5fa3549d142c2080a72a4c70ce389163cd
prerequisite-patch-id: ebaead89601acf604e83224f4df8d57a7f4331b8
prerequisite-patch-id: 609d5d7c55b0b8e2967966673dab8f62a6fceab9
prerequisite-patch-id: ac150a8c622e858e088df8121093d448df49c245
prerequisite-patch-id: a4255724d4698f1238663443024de56de38d717b
prerequisite-patch-id: 89f049f951e5acf75aab92541992f816fd0acc0d
prerequisite-patch-id: dfb8d5a1fb262127d7a8e1ef3e97f455aaa19509
prerequisite-patch-id: 11b0f5746bbfbf8aa5c5746dcd7b0dce62e7f922
prerequisite-patch-id: 315303931e4b6499de7127a88113763f86e97e16
prerequisite-patch-id: 40cb8212ddb024c20593f73d8b87d9894877e172
prerequisite-patch-id: a1673a9e9f19d6fab5a51abb721e54e36636f067
prerequisite-patch-id: 189a0f41ba4eecd4f3f35c503baac8aed8ccd7de
prerequisite-patch-id: 1117ecaa40a353c667b71802ab34ecf9568d8bb2
prerequisite-patch-id: 25923a0c77e92631ed3cd8a163d789daad35f0f8
prerequisite-patch-id: 6a6f6215f09932e68fdfd294df2e813ec9d2481f
prerequisite-patch-id: 2cc95b47cad25fd9b875d27f4e8e3d84eb70274b
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: e7773c977a7b37692e9792b21cc4f17fa58f9215
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 0a0ac5a8a90655b415f6b62e324f3db083cdaaee
prerequisite-patch-id: 2ffbced093555055b5796c0c0572b3b0216f8938
prerequisite-patch-id: 1be0fb49e0fbe293ca8fa94601e191b13c8c67d9
-- 
2.25.1

[PATCH v2 0/6] Add PLL clocks driver for StarFive JH7110 SoC

[Xingyu Wu]

This patch serises are to add PLL clocks driver and providers by writing
and reading syscon registers for the StarFive JH7110 RISC-V SoC.

PLL are high speed, low jitter frequency synthesizers in JH7110.
Each PLL clocks work in integer mode or fraction mode by some dividers,
and the dividers are set in several syscon registers.
The formula for calculating frequency is:
Fvco = Fref * (NI + NF) / M / Q1

The first patch adds docunmentation to describe PLL clock bindings,
and the second patch adds driver to support PLL clocks for JH7110.
The patch 3 modifies the syscon dt-bindings and adds optional
pateerProperties about PLL clock controller. The patch 4 modifies the
SYSCRG dibindings and adds PLL clock inputs. The patch 5 modifies the
system clock driver and changes PLL clock source from PLL clock
controller instead of the fixed factor clocks. The last patch adds PLL
clock node and modifies the syscrg node in dts file.

This patchset should be applied after these patchset about JH7110 clock
driver[1] and syscon dt-bindings[2].
[1] https://lore.kernel.org/all/20230221024645.127922-1-hal.feng@starfivetech.com/
[2] https://lore.kernel.org/all/20230315055813.94740-1-william.qiu@starfivetech.com/

Changes since v1:
- Changed PLL clock node to be child of syscon node in dts.
- Modifed the definitions and names of function in PLL clock driver.
- Added commit to update syscon and syscrg dt-bindings.

Xingyu Wu (6):
  dt-bindings: clock: Add StarFive JH7110 PLL clock generator
  clk: starfive: Add StarFive JH7110 PLL clock driver
  dt-bindings: soc: starfive: syscon: Add optional patternProperties
  dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs
  clk: starfive: jh7110-sys: Modify PLL clocks source
  riscv: dts: starfive: jh7110: Add PLL clock node and modify syscrg
    node

 .../bindings/clock/starfive,jh7110-pll.yaml   |  46 ++
 .../clock/starfive,jh7110-syscrg.yaml         |  20 +-
 .../soc/starfive/starfive,jh7110-syscon.yaml  |  39 +-
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  16 +-
 drivers/clk/starfive/Kconfig                  |   9 +
 drivers/clk/starfive/Makefile                 |   1 +
 .../clk/starfive/clk-starfive-jh7110-pll.c    | 420 ++++++++++++++++++
 .../clk/starfive/clk-starfive-jh7110-pll.h    | 293 ++++++++++++
 .../clk/starfive/clk-starfive-jh7110-sys.c    |  35 +-
 .../dt-bindings/clock/starfive,jh7110-crg.h   |   6 +
 10 files changed, 845 insertions(+), 40 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.c
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.h


base-commit: 8ca09d5fa3549d142c2080a72a4c70ce389163cd
prerequisite-patch-id: ebaead89601acf604e83224f4df8d57a7f4331b8
prerequisite-patch-id: 609d5d7c55b0b8e2967966673dab8f62a6fceab9
prerequisite-patch-id: ac150a8c622e858e088df8121093d448df49c245
prerequisite-patch-id: a4255724d4698f1238663443024de56de38d717b
prerequisite-patch-id: 89f049f951e5acf75aab92541992f816fd0acc0d
prerequisite-patch-id: dfb8d5a1fb262127d7a8e1ef3e97f455aaa19509
prerequisite-patch-id: 11b0f5746bbfbf8aa5c5746dcd7b0dce62e7f922
prerequisite-patch-id: 315303931e4b6499de7127a88113763f86e97e16
prerequisite-patch-id: 40cb8212ddb024c20593f73d8b87d9894877e172
prerequisite-patch-id: a1673a9e9f19d6fab5a51abb721e54e36636f067
prerequisite-patch-id: 189a0f41ba4eecd4f3f35c503baac8aed8ccd7de
prerequisite-patch-id: 1117ecaa40a353c667b71802ab34ecf9568d8bb2
prerequisite-patch-id: 25923a0c77e92631ed3cd8a163d789daad35f0f8
prerequisite-patch-id: 6a6f6215f09932e68fdfd294df2e813ec9d2481f
prerequisite-patch-id: 2cc95b47cad25fd9b875d27f4e8e3d84eb70274b
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: dbb0c0151b8bdf093e6ce79fd2fe3f60791a6e0b
prerequisite-patch-id: e7773c977a7b37692e9792b21cc4f17fa58f9215
prerequisite-patch-id: d57e95d31686772abc4c4d5aa1cadc344dc293cd
prerequisite-patch-id: 0a0ac5a8a90655b415f6b62e324f3db083cdaaee
prerequisite-patch-id: 2ffbced093555055b5796c0c0572b3b0216f8938
prerequisite-patch-id: 1be0fb49e0fbe293ca8fa94601e191b13c8c67d9
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 1/6] dt-bindings: clock: Add StarFive JH7110 PLL clock generator

[Xingyu Wu]

Add bindings for the PLL clock generator on the JH7110 RISC-V SoC.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../bindings/clock/starfive,jh7110-pll.yaml   | 46 +++++++++++++++++++
 .../dt-bindings/clock/starfive,jh7110-crg.h   |  6 +++
 2 files changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml

diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml
new file mode 100644
index 000000000000..9397516f60ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 PLL Clock Generator
+
+description:
+  This PLL are high speed, low jitter frequency synthesizers in JH7110.
+  Each PLL clocks work in integer mode or fraction mode by some dividers,
+  and the configuration registers and dividers are set in several syscon
+  registers. So pll node should be a child of SYS-SYSCON node.
+  The formula for calculating frequency is that,
+  Fvco = Fref * (NI + NF) / M / Q1
+
+maintainers:
+  - Xingyu Wu <xingyu.wu@starfivetech.com>
+
+properties:
+  compatible:
+    const: starfive,jh7110-pll
+
+  clocks:
+    maxItems: 1
+    description: Main Oscillator (24 MHz)
+
+  '#clock-cells':
+    const: 1
+    description:
+      See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices.
+
+required:
+  - compatible
+  - clocks
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    pllclk: pll-clock-controller {
+      compatible = "starfive,jh7110-pll";
+      clocks = <&osc>;
+      #clock-cells = <1>;
+    };
diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h
index 06257bfd9ac1..086a6ddcf380 100644
--- a/include/dt-bindings/clock/starfive,jh7110-crg.h
+++ b/include/dt-bindings/clock/starfive,jh7110-crg.h
@@ -6,6 +6,12 @@
 #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
 #define __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
 
+/* PLL clocks */
+#define JH7110_CLK_PLL0_OUT			0
+#define JH7110_CLK_PLL1_OUT			1
+#define JH7110_CLK_PLL2_OUT			2
+#define JH7110_PLLCLK_END			3
+
 /* SYSCRG clocks */
 #define JH7110_SYSCLK_CPU_ROOT			0
 #define JH7110_SYSCLK_CPU_CORE			1
-- 
2.25.1

[PATCH v2 1/6] dt-bindings: clock: Add StarFive JH7110 PLL clock generator

[Xingyu Wu]

Add bindings for the PLL clock generator on the JH7110 RISC-V SoC.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../bindings/clock/starfive,jh7110-pll.yaml   | 46 +++++++++++++++++++
 .../dt-bindings/clock/starfive,jh7110-crg.h   |  6 +++
 2 files changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml

diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml
new file mode 100644
index 000000000000..9397516f60ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-pll.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/starfive,jh7110-pll.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7110 PLL Clock Generator
+
+description:
+  This PLL are high speed, low jitter frequency synthesizers in JH7110.
+  Each PLL clocks work in integer mode or fraction mode by some dividers,
+  and the configuration registers and dividers are set in several syscon
+  registers. So pll node should be a child of SYS-SYSCON node.
+  The formula for calculating frequency is that,
+  Fvco = Fref * (NI + NF) / M / Q1
+
+maintainers:
+  - Xingyu Wu <xingyu.wu@starfivetech.com>
+
+properties:
+  compatible:
+    const: starfive,jh7110-pll
+
+  clocks:
+    maxItems: 1
+    description: Main Oscillator (24 MHz)
+
+  '#clock-cells':
+    const: 1
+    description:
+      See <dt-bindings/clock/starfive,jh7110-crg.h> for valid indices.
+
+required:
+  - compatible
+  - clocks
+  - '#clock-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    pllclk: pll-clock-controller {
+      compatible = "starfive,jh7110-pll";
+      clocks = <&osc>;
+      #clock-cells = <1>;
+    };
diff --git a/include/dt-bindings/clock/starfive,jh7110-crg.h b/include/dt-bindings/clock/starfive,jh7110-crg.h
index 06257bfd9ac1..086a6ddcf380 100644
--- a/include/dt-bindings/clock/starfive,jh7110-crg.h
+++ b/include/dt-bindings/clock/starfive,jh7110-crg.h
@@ -6,6 +6,12 @@
 #ifndef __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
 #define __DT_BINDINGS_CLOCK_STARFIVE_JH7110_CRG_H__
 
+/* PLL clocks */
+#define JH7110_CLK_PLL0_OUT			0
+#define JH7110_CLK_PLL1_OUT			1
+#define JH7110_CLK_PLL2_OUT			2
+#define JH7110_PLLCLK_END			3
+
 /* SYSCRG clocks */
 #define JH7110_SYSCLK_CPU_ROOT			0
 #define JH7110_SYSCLK_CPU_CORE			1
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 2/6] clk: starfive: Add StarFive JH7110 PLL clock driver

[Xingyu Wu]

Add driver for the StarFive JH7110 PLL clock controller.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 drivers/clk/starfive/Kconfig                  |   8 +
 drivers/clk/starfive/Makefile                 |   1 +
 .../clk/starfive/clk-starfive-jh7110-pll.c    | 420 ++++++++++++++++++
 .../clk/starfive/clk-starfive-jh7110-pll.h    | 293 ++++++++++++
 4 files changed, 722 insertions(+)
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.c
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.h

diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
index 71c1148ee5f6..e306edf4defa 100644
--- a/drivers/clk/starfive/Kconfig
+++ b/drivers/clk/starfive/Kconfig
@@ -21,6 +21,14 @@ config CLK_STARFIVE_JH7100_AUDIO
 	  Say Y or M here to support the audio clocks on the StarFive JH7100
 	  SoC.
 
+config CLK_STARFIVE_JH7110_PLL
+	bool "StarFive JH7110 PLL clock support"
+	depends on ARCH_STARFIVE || COMPILE_TEST
+	default ARCH_STARFIVE
+	help
+	  Say yes here to support the PLL clock controller on the
+	  StarFive JH7110 SoC.
+
 config CLK_STARFIVE_JH7110_SYS
 	bool "StarFive JH7110 system clock support"
 	depends on ARCH_STARFIVE || COMPILE_TEST
diff --git a/drivers/clk/starfive/Makefile b/drivers/clk/starfive/Makefile
index f3df7d957b1e..b48e539e52b0 100644
--- a/drivers/clk/starfive/Makefile
+++ b/drivers/clk/starfive/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_CLK_STARFIVE_JH71X0)	+= clk-starfive-jh71x0.o
 obj-$(CONFIG_CLK_STARFIVE_JH7100)	+= clk-starfive-jh7100.o
 obj-$(CONFIG_CLK_STARFIVE_JH7100_AUDIO)	+= clk-starfive-jh7100-audio.o
 
+obj-$(CONFIG_CLK_STARFIVE_JH7110_PLL)	+= clk-starfive-jh7110-pll.o
 obj-$(CONFIG_CLK_STARFIVE_JH7110_SYS)	+= clk-starfive-jh7110-sys.o
 obj-$(CONFIG_CLK_STARFIVE_JH7110_AON)	+= clk-starfive-jh7110-aon.o
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-pll.c b/drivers/clk/starfive/clk-starfive-jh7110-pll.c
new file mode 100644
index 000000000000..b947861065db
--- /dev/null
+++ b/drivers/clk/starfive/clk-starfive-jh7110-pll.c
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * StarFive JH7110 PLL Clock Generator Driver
+ *
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Xingyu Wu <xingyu.wu@starfivetech.com>
+ *
+ * This driver is about to register JH7110 PLL clock generator and support ops.
+ * The JH7110 have three PLL clock, PLL0, PLL1 and PLL2.
+ * Each PLL clocks work in integer mode or fraction mode by some dividers,
+ * and the configuration registers and dividers are set in several syscon registers.
+ * The formula for calculating frequency is:
+ * Fvco = Fref * (NI + NF) / M / Q1
+ * Fref: OSC source clock rate
+ * NI: integer frequency dividing ratio of feedback divider, set by fbdiv[11:0].
+ * NF: fractional frequency dividing ratio, set by frac[23:0]. NF = frac[23:0] / 2^24 = 0 ~ 0.999.
+ * M: frequency dividing ratio of pre-divider, set by prediv[5:0].
+ * Q1: frequency dividing ratio of post divider, set by postdiv1[1:0], Q1= 1,2,4,8.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/debugfs.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+
+#include "clk-starfive-jh7110-pll.h"
+
+static struct jh7110_clk_pll_data *jh7110_pll_data_from(struct clk_hw *hw)
+{
+	return container_of(hw, struct jh7110_clk_pll_data, hw);
+}
+
+static struct jh7110_clk_pll_priv *jh7110_pll_priv_from(struct jh7110_clk_pll_data *data)
+{
+	return container_of(data, struct jh7110_clk_pll_priv, data[data->idx]);
+}
+
+/* Read register value from syscon and calculate PLL(x) frequency */
+static unsigned long jh7110_pll_get_freq(struct jh7110_clk_pll_data *data,
+					 unsigned long parent_rate)
+{
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	struct jh7110_pll_syscon_offset *offset = &data->offset;
+	struct jh7110_pll_syscon_mask *mask = &data->mask;
+	struct jh7110_pll_syscon_shift *shift = &data->shift;
+	unsigned long freq = 0;
+	unsigned long frac_cal;
+	u32 dacpd;
+	u32 dsmpd;
+	u32 fbdiv;
+	u32 prediv;
+	u32 postdiv1;
+	u32 frac;
+	u32 reg_value;
+
+	if (regmap_read(priv->syscon_regmap, offset->dacpd, &reg_value))
+		goto read_register_error;
+	dacpd = (reg_value & mask->dacpd) >> shift->dacpd;
+
+	if (regmap_read(priv->syscon_regmap, offset->dsmpd, &reg_value))
+		goto read_register_error;
+	dsmpd = (reg_value & mask->dsmpd) >> shift->dsmpd;
+
+	if (regmap_read(priv->syscon_regmap, offset->fbdiv, &reg_value))
+		goto read_register_error;
+	fbdiv = (reg_value & mask->fbdiv) >> shift->fbdiv;
+	/* fbdiv value should be 8 to 4095 */
+	if (fbdiv < 8)
+		goto read_register_error;
+
+	if (regmap_read(priv->syscon_regmap, offset->prediv, &reg_value))
+		goto read_register_error;
+	prediv = (reg_value & mask->prediv) >> shift->prediv;
+
+	if (regmap_read(priv->syscon_regmap, offset->postdiv1, &reg_value))
+		goto read_register_error;
+	/* postdiv1 = 2 ^ reg_value */
+	postdiv1 = 1 << ((reg_value & mask->postdiv1) >> shift->postdiv1);
+
+	if (regmap_read(priv->syscon_regmap, offset->frac, &reg_value))
+		goto read_register_error;
+	frac = (reg_value & mask->frac) >> shift->frac;
+
+	/*
+	 * Integer Mode (Both 1) or Fraction Mode (Both 0).
+	 * And the decimal places are counted by expanding them by
+	 * a factor of STARFIVE_PLL_FRAC_PATR_SIZE.
+	 */
+	if (dacpd == 1 && dsmpd == 1)
+		frac_cal = 0;
+	else if (dacpd == 0 && dsmpd == 0)
+		frac_cal = (unsigned long)frac * STARFIVE_PLL_FRAC_PATR_SIZE / (1 << 24);
+	else
+		goto read_register_error;
+
+	if (frac_cal)
+		/* fraction mode: Fvco = Fref * (NI + NF) / M / Q1 */
+		freq = parent_rate / STARFIVE_PLL_FRAC_PATR_SIZE *
+		       (fbdiv * STARFIVE_PLL_FRAC_PATR_SIZE + frac_cal) / prediv / postdiv1;
+	else
+		/* integer mode: Fvco = Fref * NI / M / Q1 */
+		freq = parent_rate * fbdiv / prediv / postdiv1;
+
+read_register_error:
+	return freq;
+}
+
+/* Select the appropriate frequency from the already configured registers value */
+static int jh7110_pll_select_freq_syscon(struct jh7110_clk_pll_data *data,
+					 unsigned long target_rate)
+{
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	const struct starfive_pll_syscon_value *syscon_value;
+	unsigned int id;
+	unsigned int pll_arry_size;
+
+	if (data->idx == JH7110_CLK_PLL0_OUT)
+		pll_arry_size = ARRAY_SIZE(jh7110_pll0_syscon_freq);
+	else if (data->idx == JH7110_CLK_PLL1_OUT)
+		pll_arry_size = ARRAY_SIZE(jh7110_pll1_syscon_freq);
+	else
+		pll_arry_size = ARRAY_SIZE(jh7110_pll2_syscon_freq);
+
+	for (id = 0; id < pll_arry_size; id++) {
+		if (data->idx == JH7110_CLK_PLL0_OUT)
+			syscon_value = &jh7110_pll0_syscon_freq[id];
+		else if (data->idx == JH7110_CLK_PLL1_OUT)
+			syscon_value = &jh7110_pll1_syscon_freq[id];
+		else
+			syscon_value = &jh7110_pll2_syscon_freq[id];
+
+		if (target_rate == syscon_value->freq)
+			goto select_succeed;
+	}
+
+	dev_err(priv->dev, "pll%d frequency:%ld do not match, please check it.\n",
+		data->idx, target_rate);
+	return -EINVAL;
+
+select_succeed:
+	data->freq_select_idx = id;
+	return 0;
+}
+
+static int jh7110_pll_set_freq_syscon(struct jh7110_clk_pll_data *data)
+{
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	struct jh7110_pll_syscon_offset *offset = &data->offset;
+	struct jh7110_pll_syscon_mask *mask = &data->mask;
+	struct jh7110_pll_syscon_shift *shift = &data->shift;
+	unsigned int freq_idx = data->freq_select_idx;
+	const struct starfive_pll_syscon_value *syscon_value;
+	int ret;
+
+	if (data->idx == JH7110_CLK_PLL0_OUT)
+		syscon_value = &jh7110_pll0_syscon_freq[freq_idx];
+	else if (data->idx == JH7110_CLK_PLL1_OUT)
+		syscon_value = &jh7110_pll1_syscon_freq[freq_idx];
+	else
+		syscon_value = &jh7110_pll2_syscon_freq[freq_idx];
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->dacpd, mask->dacpd,
+				 (syscon_value->dacpd << shift->dacpd));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->dsmpd, mask->dsmpd,
+				 (syscon_value->dsmpd << shift->dsmpd));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->prediv, mask->prediv,
+				 (syscon_value->prediv << shift->prediv));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->fbdiv, mask->fbdiv,
+				 (syscon_value->fbdiv << shift->fbdiv));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->postdiv1, mask->postdiv1,
+				 ((syscon_value->postdiv1 >> 1) << shift->postdiv1));
+	if (ret)
+		goto set_failed;
+
+	/* frac: Integer Mode (Both 1) or Fraction Mode (Both 0) */
+	if (syscon_value->dacpd == 0 && syscon_value->dsmpd == 0)
+		ret = regmap_update_bits(priv->syscon_regmap, offset->frac, mask->frac,
+					 (syscon_value->frac << shift->frac));
+	else if (syscon_value->dacpd != syscon_value->dsmpd)
+		ret = -EINVAL;
+
+set_failed:
+	return ret;
+}
+
+static unsigned long jh7110_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+
+	return jh7110_pll_get_freq(data, parent_rate);
+}
+
+static int jh7110_pll_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
+{
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+	int ret = jh7110_pll_select_freq_syscon(data, req->rate);
+
+	if (ret)
+		return ret;
+
+	if (data->idx == JH7110_CLK_PLL0_OUT)
+		req->rate = jh7110_pll0_syscon_freq[data->freq_select_idx].freq;
+	else if (data->idx == JH7110_CLK_PLL1_OUT)
+		req->rate = jh7110_pll1_syscon_freq[data->freq_select_idx].freq;
+	else
+		req->rate = jh7110_pll2_syscon_freq[data->freq_select_idx].freq;
+
+	return 0;
+}
+
+static int jh7110_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long parent_rate)
+{
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+
+	return jh7110_pll_set_freq_syscon(data);
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void jh7110_pll_debug_init(struct clk_hw *hw, struct dentry *dentry)
+{
+	static const struct debugfs_reg32 jh7110_clk_pll_reg = {
+		.name = "CTRL",
+		.offset = 0,
+	};
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	struct debugfs_regset32 *regset;
+
+	regset = devm_kzalloc(priv->dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->regs = &jh7110_clk_pll_reg;
+	regset->nregs = 1;
+
+	debugfs_create_regset32("registers", 0400, dentry, regset);
+}
+#else
+#define jh7110_pll_debug_init NULL
+#endif
+
+static const struct clk_ops jh7110_pll_ops = {
+	.recalc_rate = jh7110_pll_recalc_rate,
+	.determine_rate = jh7110_pll_determine_rate,
+	.set_rate = jh7110_pll_set_rate,
+	.debug_init = jh7110_pll_debug_init,
+};
+
+/* get offset, mask and shift of PLL(x) syscon */
+static int jh7110_pll_data_get(struct jh7110_clk_pll_data *data, int index)
+{
+	struct jh7110_pll_syscon_offset *offset = &data->offset;
+	struct jh7110_pll_syscon_mask *mask = &data->mask;
+	struct jh7110_pll_syscon_shift *shift = &data->shift;
+
+	if (index == JH7110_CLK_PLL0_OUT) {
+		offset->dacpd = STARFIVE_JH7110_PLL0_DACPD_OFFSET;
+		offset->dsmpd = STARFIVE_JH7110_PLL0_DSMPD_OFFSET;
+		offset->fbdiv = STARFIVE_JH7110_PLL0_FBDIV_OFFSET;
+		offset->frac = STARFIVE_JH7110_PLL0_FRAC_OFFSET;
+		offset->prediv = STARFIVE_JH7110_PLL0_PREDIV_OFFSET;
+		offset->postdiv1 = STARFIVE_JH7110_PLL0_POSTDIV1_OFFSET;
+
+		mask->dacpd = STARFIVE_JH7110_PLL0_DACPD_MASK;
+		mask->dsmpd = STARFIVE_JH7110_PLL0_DSMPD_MASK;
+		mask->fbdiv = STARFIVE_JH7110_PLL0_FBDIV_MASK;
+		mask->frac = STARFIVE_JH7110_PLL0_FRAC_MASK;
+		mask->prediv = STARFIVE_JH7110_PLL0_PREDIV_MASK;
+		mask->postdiv1 = STARFIVE_JH7110_PLL0_POSTDIV1_MASK;
+
+		shift->dacpd = STARFIVE_JH7110_PLL0_DACPD_SHIFT;
+		shift->dsmpd = STARFIVE_JH7110_PLL0_DSMPD_SHIFT;
+		shift->fbdiv = STARFIVE_JH7110_PLL0_FBDIV_SHIFT;
+		shift->frac = STARFIVE_JH7110_PLL0_FRAC_SHIFT;
+		shift->prediv = STARFIVE_JH7110_PLL0_PREDIV_SHIFT;
+		shift->postdiv1 = STARFIVE_JH7110_PLL0_POSTDIV1_SHIFT;
+
+	} else if (index == JH7110_CLK_PLL1_OUT) {
+		offset->dacpd = STARFIVE_JH7110_PLL1_DACPD_OFFSET;
+		offset->dsmpd = STARFIVE_JH7110_PLL1_DSMPD_OFFSET;
+		offset->fbdiv = STARFIVE_JH7110_PLL1_FBDIV_OFFSET;
+		offset->frac = STARFIVE_JH7110_PLL1_FRAC_OFFSET;
+		offset->prediv = STARFIVE_JH7110_PLL1_PREDIV_OFFSET;
+		offset->postdiv1 = STARFIVE_JH7110_PLL1_POSTDIV1_OFFSET;
+
+		mask->dacpd = STARFIVE_JH7110_PLL1_DACPD_MASK;
+		mask->dsmpd = STARFIVE_JH7110_PLL1_DSMPD_MASK;
+		mask->fbdiv = STARFIVE_JH7110_PLL1_FBDIV_MASK;
+		mask->frac = STARFIVE_JH7110_PLL1_FRAC_MASK;
+		mask->prediv = STARFIVE_JH7110_PLL1_PREDIV_MASK;
+		mask->postdiv1 = STARFIVE_JH7110_PLL1_POSTDIV1_MASK;
+
+		shift->dacpd = STARFIVE_JH7110_PLL1_DACPD_SHIFT;
+		shift->dsmpd = STARFIVE_JH7110_PLL1_DSMPD_SHIFT;
+		shift->fbdiv = STARFIVE_JH7110_PLL1_FBDIV_SHIFT;
+		shift->frac = STARFIVE_JH7110_PLL1_FRAC_SHIFT;
+		shift->prediv = STARFIVE_JH7110_PLL1_PREDIV_SHIFT;
+		shift->postdiv1 = STARFIVE_JH7110_PLL1_POSTDIV1_SHIFT;
+
+	} else if (index == JH7110_CLK_PLL2_OUT) {
+		offset->dacpd = STARFIVE_JH7110_PLL2_DACPD_OFFSET;
+		offset->dsmpd = STARFIVE_JH7110_PLL2_DSMPD_OFFSET;
+		offset->fbdiv = STARFIVE_JH7110_PLL2_FBDIV_OFFSET;
+		offset->frac = STARFIVE_JH7110_PLL2_FRAC_OFFSET;
+		offset->prediv = STARFIVE_JH7110_PLL2_PREDIV_OFFSET;
+		offset->postdiv1 = STARFIVE_JH7110_PLL2_POSTDIV1_OFFSET;
+
+		mask->dacpd = STARFIVE_JH7110_PLL2_DACPD_MASK;
+		mask->dsmpd = STARFIVE_JH7110_PLL2_DSMPD_MASK;
+		mask->fbdiv = STARFIVE_JH7110_PLL2_FBDIV_MASK;
+		mask->frac = STARFIVE_JH7110_PLL2_FRAC_MASK;
+		mask->prediv = STARFIVE_JH7110_PLL2_PREDIV_MASK;
+		mask->postdiv1 = STARFIVE_JH7110_PLL2_POSTDIV1_MASK;
+
+		shift->dacpd = STARFIVE_JH7110_PLL2_DACPD_SHIFT;
+		shift->dsmpd = STARFIVE_JH7110_PLL2_DSMPD_SHIFT;
+		shift->fbdiv = STARFIVE_JH7110_PLL2_FBDIV_SHIFT;
+		shift->frac = STARFIVE_JH7110_PLL2_FRAC_SHIFT;
+		shift->prediv = STARFIVE_JH7110_PLL2_PREDIV_SHIFT;
+		shift->postdiv1 = STARFIVE_JH7110_PLL2_POSTDIV1_SHIFT;
+
+	} else {
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
+static struct clk_hw *jh7110_pll_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct jh7110_clk_pll_priv *priv = data;
+	unsigned int idx = clkspec->args[0];
+
+	if (idx < JH7110_PLLCLK_END)
+		return &priv->data[idx].hw;
+
+	return ERR_PTR(-EINVAL);
+}
+
+static int jh7110_pll_probe(struct platform_device *pdev)
+{
+	const char *pll_name[JH7110_PLLCLK_END] = {
+		"pll0_out",
+		"pll1_out",
+		"pll2_out"
+	};
+	struct jh7110_clk_pll_priv *priv;
+	struct jh7110_clk_pll_data *data;
+	int ret;
+	unsigned int idx;
+
+	priv = devm_kzalloc(&pdev->dev, struct_size(priv, data, JH7110_PLLCLK_END),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+	priv->syscon_regmap = syscon_node_to_regmap(priv->dev->of_node->parent);
+	if (IS_ERR(priv->syscon_regmap))
+		return PTR_ERR(priv->syscon_regmap);
+
+	for (idx = 0; idx < JH7110_PLLCLK_END; idx++) {
+		struct clk_parent_data parents = {
+			.index = 0,
+		};
+		struct clk_init_data init = {
+			.name = pll_name[idx],
+			.ops = &jh7110_pll_ops,
+			.parent_data = &parents,
+			.num_parents = 1,
+			.flags = 0,
+		};
+
+		data = &priv->data[idx];
+
+		ret = jh7110_pll_data_get(data, idx);
+		if (ret)
+			return ret;
+
+		data->hw.init = &init;
+		data->idx = idx;
+
+		ret = devm_clk_hw_register(&pdev->dev, &data->hw);
+		if (ret)
+			return ret;
+	}
+
+	return devm_of_clk_add_hw_provider(&pdev->dev, jh7110_pll_get, priv);
+}
+
+static const struct of_device_id jh7110_pll_match[] = {
+	{ .compatible = "starfive,jh7110-pll" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, jh7110_pll_match);
+
+static struct platform_driver jh7110_pll_driver = {
+	.driver = {
+		.name = "clk-starfive-jh7110-pll",
+		.of_match_table = jh7110_pll_match,
+	},
+};
+builtin_platform_driver_probe(jh7110_pll_driver, jh7110_pll_probe);
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-pll.h b/drivers/clk/starfive/clk-starfive-jh7110-pll.h
new file mode 100644
index 000000000000..3deb35f144dc
--- /dev/null
+++ b/drivers/clk/starfive/clk-starfive-jh7110-pll.h
@@ -0,0 +1,293 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/*
+ * StarFive JH7110 PLL Clock Generator Driver
+ *
+ * Copyright (C) 2022 Xingyu Wu <xingyu.wu@starfivetech.com>
+ */
+
+#ifndef _CLK_STARFIVE_JH7110_PLL_H_
+#define _CLK_STARFIVE_JH7110_PLL_H_
+
+#include <linux/bits.h>
+
+/* The decimal places are counted by expanding them by a factor of STARFIVE_PLL_FRAC_PATR_SIZE. */
+#define STARFIVE_PLL_FRAC_PATR_SIZE		1000
+
+#define STARFIVE_JH7110_PLL0_DACPD_OFFSET	0x18
+#define STARFIVE_JH7110_PLL0_DACPD_SHIFT	24
+#define STARFIVE_JH7110_PLL0_DACPD_MASK		BIT(24)
+#define STARFIVE_JH7110_PLL0_DSMPD_OFFSET	0x18
+#define STARFIVE_JH7110_PLL0_DSMPD_SHIFT	25
+#define STARFIVE_JH7110_PLL0_DSMPD_MASK		BIT(25)
+#define STARFIVE_JH7110_PLL0_FBDIV_OFFSET	0x1c
+#define STARFIVE_JH7110_PLL0_FBDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL0_FBDIV_MASK		GENMASK(11, 0)
+#define STARFIVE_JH7110_PLL0_FRAC_OFFSET	0x20
+#define STARFIVE_JH7110_PLL0_FRAC_SHIFT		0
+#define STARFIVE_JH7110_PLL0_FRAC_MASK		GENMASK(23, 0)
+#define STARFIVE_JH7110_PLL0_POSTDIV1_OFFSET	0x20
+#define STARFIVE_JH7110_PLL0_POSTDIV1_SHIFT	28
+#define STARFIVE_JH7110_PLL0_POSTDIV1_MASK	GENMASK(29, 28)
+#define STARFIVE_JH7110_PLL0_PREDIV_OFFSET	0x24
+#define STARFIVE_JH7110_PLL0_PREDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL0_PREDIV_MASK	GENMASK(5, 0)
+
+#define STARFIVE_JH7110_PLL1_DACPD_OFFSET	0x24
+#define STARFIVE_JH7110_PLL1_DACPD_SHIFT	15
+#define STARFIVE_JH7110_PLL1_DACPD_MASK		BIT(15)
+#define STARFIVE_JH7110_PLL1_DSMPD_OFFSET	0x24
+#define STARFIVE_JH7110_PLL1_DSMPD_SHIFT	16
+#define STARFIVE_JH7110_PLL1_DSMPD_MASK		BIT(16)
+#define STARFIVE_JH7110_PLL1_FBDIV_OFFSET	0x24
+#define STARFIVE_JH7110_PLL1_FBDIV_SHIFT	17
+#define STARFIVE_JH7110_PLL1_FBDIV_MASK		GENMASK(28, 17)
+#define STARFIVE_JH7110_PLL1_FRAC_OFFSET	0x28
+#define STARFIVE_JH7110_PLL1_FRAC_SHIFT		0
+#define STARFIVE_JH7110_PLL1_FRAC_MASK		GENMASK(23, 0)
+#define STARFIVE_JH7110_PLL1_POSTDIV1_OFFSET	0x28
+#define STARFIVE_JH7110_PLL1_POSTDIV1_SHIFT	28
+#define STARFIVE_JH7110_PLL1_POSTDIV1_MASK	GENMASK(29, 28)
+#define STARFIVE_JH7110_PLL1_PREDIV_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL1_PREDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL1_PREDIV_MASK	GENMASK(5, 0)
+
+#define STARFIVE_JH7110_PLL2_DACPD_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL2_DACPD_SHIFT	15
+#define STARFIVE_JH7110_PLL2_DACPD_MASK		BIT(15)
+#define STARFIVE_JH7110_PLL2_DSMPD_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL2_DSMPD_SHIFT	16
+#define STARFIVE_JH7110_PLL2_DSMPD_MASK		BIT(16)
+#define STARFIVE_JH7110_PLL2_FBDIV_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL2_FBDIV_SHIFT	17
+#define STARFIVE_JH7110_PLL2_FBDIV_MASK		GENMASK(28, 17)
+#define STARFIVE_JH7110_PLL2_FRAC_OFFSET	0x30
+#define STARFIVE_JH7110_PLL2_FRAC_SHIFT		0
+#define STARFIVE_JH7110_PLL2_FRAC_MASK		GENMASK(23, 0)
+#define STARFIVE_JH7110_PLL2_POSTDIV1_OFFSET	0x30
+#define STARFIVE_JH7110_PLL2_POSTDIV1_SHIFT	28
+#define STARFIVE_JH7110_PLL2_POSTDIV1_MASK	GENMASK(29, 28)
+#define STARFIVE_JH7110_PLL2_PREDIV_OFFSET	0x34
+#define STARFIVE_JH7110_PLL2_PREDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL2_PREDIV_MASK	GENMASK(5, 0)
+
+struct jh7110_pll_syscon_offset {
+	unsigned int dacpd;
+	unsigned int dsmpd;
+	unsigned int fbdiv;
+	unsigned int frac;
+	unsigned int prediv;
+	unsigned int postdiv1;
+};
+
+struct jh7110_pll_syscon_mask {
+	u32 dacpd;
+	u32 dsmpd;
+	u32 fbdiv;
+	u32 frac;
+	u32 prediv;
+	u32 postdiv1;
+};
+
+struct jh7110_pll_syscon_shift {
+	char dacpd;
+	char dsmpd;
+	char fbdiv;
+	char frac;
+	char prediv;
+	char postdiv1;
+};
+
+struct jh7110_clk_pll_data {
+	struct clk_hw hw;
+	unsigned int idx;
+	unsigned int freq_select_idx;
+
+	struct jh7110_pll_syscon_offset offset;
+	struct jh7110_pll_syscon_mask mask;
+	struct jh7110_pll_syscon_shift shift;
+};
+
+struct jh7110_clk_pll_priv {
+	struct device *dev;
+	struct regmap *syscon_regmap;
+	struct jh7110_clk_pll_data data[];
+};
+
+struct starfive_pll_syscon_value {
+	unsigned long freq;
+	u32 prediv;
+	u32 fbdiv;
+	u32 postdiv1;
+/* Both daxpd and dsmpd set 1 while integer multiple mode */
+/* Both daxpd and dsmpd set 0 while fraction multiple mode */
+	u32 dacpd;
+	u32 dsmpd;
+/* frac value should be decimals multiplied by 2^24 */
+	u32 frac;
+};
+
+enum starfive_pll0_freq_index {
+	PLL0_FREQ_375 = 0,
+	PLL0_FREQ_500,
+	PLL0_FREQ_625,
+	PLL0_FREQ_750,
+	PLL0_FREQ_875,
+	PLL0_FREQ_1000,
+	PLL0_FREQ_1250,
+	PLL0_FREQ_1375,
+	PLL0_FREQ_1500,
+	PLL0_FREQ_MAX
+};
+
+enum starfive_pll1_freq_index {
+	PLL1_FREQ_1066 = 0,
+	PLL1_FREQ_1200,
+	PLL1_FREQ_1400,
+	PLL1_FREQ_1600,
+	PLL1_FREQ_MAX
+};
+
+enum starfive_pll2_freq_index {
+	PLL2_FREQ_1188 = 0,
+	PLL2_FREQ_12288,
+	PLL2_FREQ_MAX
+};
+
+/*
+ * Because the pll frequency is relatively fixed,
+ * it cannot be set arbitrarily, so it needs a specific configuration.
+ * PLL0 frequency should be multiple of 125MHz (USB frequency).
+ */
+static const struct starfive_pll_syscon_value
+	jh7110_pll0_syscon_freq[] = {
+	[PLL0_FREQ_375] = {
+		.freq = 375000000,
+		.prediv = 8,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_500] = {
+		.freq = 500000000,
+		.prediv = 6,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_625] = {
+		.freq = 625000000,
+		.prediv = 24,
+		.fbdiv = 625,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_750] = {
+		.freq = 750000000,
+		.prediv = 4,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_875] = {
+		.freq = 875000000,
+		.prediv = 24,
+		.fbdiv = 875,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1000] = {
+		.freq = 1000000000,
+		.prediv = 3,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1250] = {
+		.freq = 1250000000,
+		.prediv = 12,
+		.fbdiv = 625,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1375] = {
+		.freq = 1375000000,
+		.prediv = 24,
+		.fbdiv = 1375,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1500] = {
+		.freq = 1500000000,
+		.prediv = 2,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+};
+
+static const struct starfive_pll_syscon_value
+	jh7110_pll1_syscon_freq[] = {
+	[PLL1_FREQ_1066] = {
+		.freq = 1066000000,
+		.prediv = 12,
+		.fbdiv = 533,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL1_FREQ_1200] = {
+		.freq = 1200000000,
+		.prediv = 1,
+		.fbdiv = 50,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL1_FREQ_1400] = {
+		.freq = 1400000000,
+		.prediv = 6,
+		.fbdiv = 350,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL1_FREQ_1600] = {
+		.freq = 1600000000,
+		.prediv = 3,
+		.fbdiv = 200,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+};
+
+static const struct starfive_pll_syscon_value
+	jh7110_pll2_syscon_freq[] = {
+	[PLL2_FREQ_1188] = {
+		.freq = 1188000000,
+		.prediv = 2,
+		.fbdiv = 99,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL2_FREQ_12288] = {
+		.freq = 1228800000,
+		.prediv = 5,
+		.fbdiv = 256,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+};
+
+#endif
-- 
2.25.1

[PATCH v2 2/6] clk: starfive: Add StarFive JH7110 PLL clock driver

[Xingyu Wu]

Add driver for the StarFive JH7110 PLL clock controller.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 drivers/clk/starfive/Kconfig                  |   8 +
 drivers/clk/starfive/Makefile                 |   1 +
 .../clk/starfive/clk-starfive-jh7110-pll.c    | 420 ++++++++++++++++++
 .../clk/starfive/clk-starfive-jh7110-pll.h    | 293 ++++++++++++
 4 files changed, 722 insertions(+)
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.c
 create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-pll.h

diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
index 71c1148ee5f6..e306edf4defa 100644
--- a/drivers/clk/starfive/Kconfig
+++ b/drivers/clk/starfive/Kconfig
@@ -21,6 +21,14 @@ config CLK_STARFIVE_JH7100_AUDIO
 	  Say Y or M here to support the audio clocks on the StarFive JH7100
 	  SoC.
 
+config CLK_STARFIVE_JH7110_PLL
+	bool "StarFive JH7110 PLL clock support"
+	depends on ARCH_STARFIVE || COMPILE_TEST
+	default ARCH_STARFIVE
+	help
+	  Say yes here to support the PLL clock controller on the
+	  StarFive JH7110 SoC.
+
 config CLK_STARFIVE_JH7110_SYS
 	bool "StarFive JH7110 system clock support"
 	depends on ARCH_STARFIVE || COMPILE_TEST
diff --git a/drivers/clk/starfive/Makefile b/drivers/clk/starfive/Makefile
index f3df7d957b1e..b48e539e52b0 100644
--- a/drivers/clk/starfive/Makefile
+++ b/drivers/clk/starfive/Makefile
@@ -4,5 +4,6 @@ obj-$(CONFIG_CLK_STARFIVE_JH71X0)	+= clk-starfive-jh71x0.o
 obj-$(CONFIG_CLK_STARFIVE_JH7100)	+= clk-starfive-jh7100.o
 obj-$(CONFIG_CLK_STARFIVE_JH7100_AUDIO)	+= clk-starfive-jh7100-audio.o
 
+obj-$(CONFIG_CLK_STARFIVE_JH7110_PLL)	+= clk-starfive-jh7110-pll.o
 obj-$(CONFIG_CLK_STARFIVE_JH7110_SYS)	+= clk-starfive-jh7110-sys.o
 obj-$(CONFIG_CLK_STARFIVE_JH7110_AON)	+= clk-starfive-jh7110-aon.o
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-pll.c b/drivers/clk/starfive/clk-starfive-jh7110-pll.c
new file mode 100644
index 000000000000..b947861065db
--- /dev/null
+++ b/drivers/clk/starfive/clk-starfive-jh7110-pll.c
@@ -0,0 +1,420 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * StarFive JH7110 PLL Clock Generator Driver
+ *
+ * Copyright (C) 2022 StarFive Technology Co., Ltd.
+ * Copyright (C) 2022 Xingyu Wu <xingyu.wu@starfivetech.com>
+ *
+ * This driver is about to register JH7110 PLL clock generator and support ops.
+ * The JH7110 have three PLL clock, PLL0, PLL1 and PLL2.
+ * Each PLL clocks work in integer mode or fraction mode by some dividers,
+ * and the configuration registers and dividers are set in several syscon registers.
+ * The formula for calculating frequency is:
+ * Fvco = Fref * (NI + NF) / M / Q1
+ * Fref: OSC source clock rate
+ * NI: integer frequency dividing ratio of feedback divider, set by fbdiv[11:0].
+ * NF: fractional frequency dividing ratio, set by frac[23:0]. NF = frac[23:0] / 2^24 = 0 ~ 0.999.
+ * M: frequency dividing ratio of pre-divider, set by prediv[5:0].
+ * Q1: frequency dividing ratio of post divider, set by postdiv1[1:0], Q1= 1,2,4,8.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/debugfs.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/starfive,jh7110-crg.h>
+
+#include "clk-starfive-jh7110-pll.h"
+
+static struct jh7110_clk_pll_data *jh7110_pll_data_from(struct clk_hw *hw)
+{
+	return container_of(hw, struct jh7110_clk_pll_data, hw);
+}
+
+static struct jh7110_clk_pll_priv *jh7110_pll_priv_from(struct jh7110_clk_pll_data *data)
+{
+	return container_of(data, struct jh7110_clk_pll_priv, data[data->idx]);
+}
+
+/* Read register value from syscon and calculate PLL(x) frequency */
+static unsigned long jh7110_pll_get_freq(struct jh7110_clk_pll_data *data,
+					 unsigned long parent_rate)
+{
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	struct jh7110_pll_syscon_offset *offset = &data->offset;
+	struct jh7110_pll_syscon_mask *mask = &data->mask;
+	struct jh7110_pll_syscon_shift *shift = &data->shift;
+	unsigned long freq = 0;
+	unsigned long frac_cal;
+	u32 dacpd;
+	u32 dsmpd;
+	u32 fbdiv;
+	u32 prediv;
+	u32 postdiv1;
+	u32 frac;
+	u32 reg_value;
+
+	if (regmap_read(priv->syscon_regmap, offset->dacpd, &reg_value))
+		goto read_register_error;
+	dacpd = (reg_value & mask->dacpd) >> shift->dacpd;
+
+	if (regmap_read(priv->syscon_regmap, offset->dsmpd, &reg_value))
+		goto read_register_error;
+	dsmpd = (reg_value & mask->dsmpd) >> shift->dsmpd;
+
+	if (regmap_read(priv->syscon_regmap, offset->fbdiv, &reg_value))
+		goto read_register_error;
+	fbdiv = (reg_value & mask->fbdiv) >> shift->fbdiv;
+	/* fbdiv value should be 8 to 4095 */
+	if (fbdiv < 8)
+		goto read_register_error;
+
+	if (regmap_read(priv->syscon_regmap, offset->prediv, &reg_value))
+		goto read_register_error;
+	prediv = (reg_value & mask->prediv) >> shift->prediv;
+
+	if (regmap_read(priv->syscon_regmap, offset->postdiv1, &reg_value))
+		goto read_register_error;
+	/* postdiv1 = 2 ^ reg_value */
+	postdiv1 = 1 << ((reg_value & mask->postdiv1) >> shift->postdiv1);
+
+	if (regmap_read(priv->syscon_regmap, offset->frac, &reg_value))
+		goto read_register_error;
+	frac = (reg_value & mask->frac) >> shift->frac;
+
+	/*
+	 * Integer Mode (Both 1) or Fraction Mode (Both 0).
+	 * And the decimal places are counted by expanding them by
+	 * a factor of STARFIVE_PLL_FRAC_PATR_SIZE.
+	 */
+	if (dacpd == 1 && dsmpd == 1)
+		frac_cal = 0;
+	else if (dacpd == 0 && dsmpd == 0)
+		frac_cal = (unsigned long)frac * STARFIVE_PLL_FRAC_PATR_SIZE / (1 << 24);
+	else
+		goto read_register_error;
+
+	if (frac_cal)
+		/* fraction mode: Fvco = Fref * (NI + NF) / M / Q1 */
+		freq = parent_rate / STARFIVE_PLL_FRAC_PATR_SIZE *
+		       (fbdiv * STARFIVE_PLL_FRAC_PATR_SIZE + frac_cal) / prediv / postdiv1;
+	else
+		/* integer mode: Fvco = Fref * NI / M / Q1 */
+		freq = parent_rate * fbdiv / prediv / postdiv1;
+
+read_register_error:
+	return freq;
+}
+
+/* Select the appropriate frequency from the already configured registers value */
+static int jh7110_pll_select_freq_syscon(struct jh7110_clk_pll_data *data,
+					 unsigned long target_rate)
+{
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	const struct starfive_pll_syscon_value *syscon_value;
+	unsigned int id;
+	unsigned int pll_arry_size;
+
+	if (data->idx == JH7110_CLK_PLL0_OUT)
+		pll_arry_size = ARRAY_SIZE(jh7110_pll0_syscon_freq);
+	else if (data->idx == JH7110_CLK_PLL1_OUT)
+		pll_arry_size = ARRAY_SIZE(jh7110_pll1_syscon_freq);
+	else
+		pll_arry_size = ARRAY_SIZE(jh7110_pll2_syscon_freq);
+
+	for (id = 0; id < pll_arry_size; id++) {
+		if (data->idx == JH7110_CLK_PLL0_OUT)
+			syscon_value = &jh7110_pll0_syscon_freq[id];
+		else if (data->idx == JH7110_CLK_PLL1_OUT)
+			syscon_value = &jh7110_pll1_syscon_freq[id];
+		else
+			syscon_value = &jh7110_pll2_syscon_freq[id];
+
+		if (target_rate == syscon_value->freq)
+			goto select_succeed;
+	}
+
+	dev_err(priv->dev, "pll%d frequency:%ld do not match, please check it.\n",
+		data->idx, target_rate);
+	return -EINVAL;
+
+select_succeed:
+	data->freq_select_idx = id;
+	return 0;
+}
+
+static int jh7110_pll_set_freq_syscon(struct jh7110_clk_pll_data *data)
+{
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	struct jh7110_pll_syscon_offset *offset = &data->offset;
+	struct jh7110_pll_syscon_mask *mask = &data->mask;
+	struct jh7110_pll_syscon_shift *shift = &data->shift;
+	unsigned int freq_idx = data->freq_select_idx;
+	const struct starfive_pll_syscon_value *syscon_value;
+	int ret;
+
+	if (data->idx == JH7110_CLK_PLL0_OUT)
+		syscon_value = &jh7110_pll0_syscon_freq[freq_idx];
+	else if (data->idx == JH7110_CLK_PLL1_OUT)
+		syscon_value = &jh7110_pll1_syscon_freq[freq_idx];
+	else
+		syscon_value = &jh7110_pll2_syscon_freq[freq_idx];
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->dacpd, mask->dacpd,
+				 (syscon_value->dacpd << shift->dacpd));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->dsmpd, mask->dsmpd,
+				 (syscon_value->dsmpd << shift->dsmpd));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->prediv, mask->prediv,
+				 (syscon_value->prediv << shift->prediv));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->fbdiv, mask->fbdiv,
+				 (syscon_value->fbdiv << shift->fbdiv));
+	if (ret)
+		goto set_failed;
+
+	ret = regmap_update_bits(priv->syscon_regmap, offset->postdiv1, mask->postdiv1,
+				 ((syscon_value->postdiv1 >> 1) << shift->postdiv1));
+	if (ret)
+		goto set_failed;
+
+	/* frac: Integer Mode (Both 1) or Fraction Mode (Both 0) */
+	if (syscon_value->dacpd == 0 && syscon_value->dsmpd == 0)
+		ret = regmap_update_bits(priv->syscon_regmap, offset->frac, mask->frac,
+					 (syscon_value->frac << shift->frac));
+	else if (syscon_value->dacpd != syscon_value->dsmpd)
+		ret = -EINVAL;
+
+set_failed:
+	return ret;
+}
+
+static unsigned long jh7110_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+
+	return jh7110_pll_get_freq(data, parent_rate);
+}
+
+static int jh7110_pll_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
+{
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+	int ret = jh7110_pll_select_freq_syscon(data, req->rate);
+
+	if (ret)
+		return ret;
+
+	if (data->idx == JH7110_CLK_PLL0_OUT)
+		req->rate = jh7110_pll0_syscon_freq[data->freq_select_idx].freq;
+	else if (data->idx == JH7110_CLK_PLL1_OUT)
+		req->rate = jh7110_pll1_syscon_freq[data->freq_select_idx].freq;
+	else
+		req->rate = jh7110_pll2_syscon_freq[data->freq_select_idx].freq;
+
+	return 0;
+}
+
+static int jh7110_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long parent_rate)
+{
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+
+	return jh7110_pll_set_freq_syscon(data);
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void jh7110_pll_debug_init(struct clk_hw *hw, struct dentry *dentry)
+{
+	static const struct debugfs_reg32 jh7110_clk_pll_reg = {
+		.name = "CTRL",
+		.offset = 0,
+	};
+	struct jh7110_clk_pll_data *data = jh7110_pll_data_from(hw);
+	struct jh7110_clk_pll_priv *priv = jh7110_pll_priv_from(data);
+	struct debugfs_regset32 *regset;
+
+	regset = devm_kzalloc(priv->dev, sizeof(*regset), GFP_KERNEL);
+	if (!regset)
+		return;
+
+	regset->regs = &jh7110_clk_pll_reg;
+	regset->nregs = 1;
+
+	debugfs_create_regset32("registers", 0400, dentry, regset);
+}
+#else
+#define jh7110_pll_debug_init NULL
+#endif
+
+static const struct clk_ops jh7110_pll_ops = {
+	.recalc_rate = jh7110_pll_recalc_rate,
+	.determine_rate = jh7110_pll_determine_rate,
+	.set_rate = jh7110_pll_set_rate,
+	.debug_init = jh7110_pll_debug_init,
+};
+
+/* get offset, mask and shift of PLL(x) syscon */
+static int jh7110_pll_data_get(struct jh7110_clk_pll_data *data, int index)
+{
+	struct jh7110_pll_syscon_offset *offset = &data->offset;
+	struct jh7110_pll_syscon_mask *mask = &data->mask;
+	struct jh7110_pll_syscon_shift *shift = &data->shift;
+
+	if (index == JH7110_CLK_PLL0_OUT) {
+		offset->dacpd = STARFIVE_JH7110_PLL0_DACPD_OFFSET;
+		offset->dsmpd = STARFIVE_JH7110_PLL0_DSMPD_OFFSET;
+		offset->fbdiv = STARFIVE_JH7110_PLL0_FBDIV_OFFSET;
+		offset->frac = STARFIVE_JH7110_PLL0_FRAC_OFFSET;
+		offset->prediv = STARFIVE_JH7110_PLL0_PREDIV_OFFSET;
+		offset->postdiv1 = STARFIVE_JH7110_PLL0_POSTDIV1_OFFSET;
+
+		mask->dacpd = STARFIVE_JH7110_PLL0_DACPD_MASK;
+		mask->dsmpd = STARFIVE_JH7110_PLL0_DSMPD_MASK;
+		mask->fbdiv = STARFIVE_JH7110_PLL0_FBDIV_MASK;
+		mask->frac = STARFIVE_JH7110_PLL0_FRAC_MASK;
+		mask->prediv = STARFIVE_JH7110_PLL0_PREDIV_MASK;
+		mask->postdiv1 = STARFIVE_JH7110_PLL0_POSTDIV1_MASK;
+
+		shift->dacpd = STARFIVE_JH7110_PLL0_DACPD_SHIFT;
+		shift->dsmpd = STARFIVE_JH7110_PLL0_DSMPD_SHIFT;
+		shift->fbdiv = STARFIVE_JH7110_PLL0_FBDIV_SHIFT;
+		shift->frac = STARFIVE_JH7110_PLL0_FRAC_SHIFT;
+		shift->prediv = STARFIVE_JH7110_PLL0_PREDIV_SHIFT;
+		shift->postdiv1 = STARFIVE_JH7110_PLL0_POSTDIV1_SHIFT;
+
+	} else if (index == JH7110_CLK_PLL1_OUT) {
+		offset->dacpd = STARFIVE_JH7110_PLL1_DACPD_OFFSET;
+		offset->dsmpd = STARFIVE_JH7110_PLL1_DSMPD_OFFSET;
+		offset->fbdiv = STARFIVE_JH7110_PLL1_FBDIV_OFFSET;
+		offset->frac = STARFIVE_JH7110_PLL1_FRAC_OFFSET;
+		offset->prediv = STARFIVE_JH7110_PLL1_PREDIV_OFFSET;
+		offset->postdiv1 = STARFIVE_JH7110_PLL1_POSTDIV1_OFFSET;
+
+		mask->dacpd = STARFIVE_JH7110_PLL1_DACPD_MASK;
+		mask->dsmpd = STARFIVE_JH7110_PLL1_DSMPD_MASK;
+		mask->fbdiv = STARFIVE_JH7110_PLL1_FBDIV_MASK;
+		mask->frac = STARFIVE_JH7110_PLL1_FRAC_MASK;
+		mask->prediv = STARFIVE_JH7110_PLL1_PREDIV_MASK;
+		mask->postdiv1 = STARFIVE_JH7110_PLL1_POSTDIV1_MASK;
+
+		shift->dacpd = STARFIVE_JH7110_PLL1_DACPD_SHIFT;
+		shift->dsmpd = STARFIVE_JH7110_PLL1_DSMPD_SHIFT;
+		shift->fbdiv = STARFIVE_JH7110_PLL1_FBDIV_SHIFT;
+		shift->frac = STARFIVE_JH7110_PLL1_FRAC_SHIFT;
+		shift->prediv = STARFIVE_JH7110_PLL1_PREDIV_SHIFT;
+		shift->postdiv1 = STARFIVE_JH7110_PLL1_POSTDIV1_SHIFT;
+
+	} else if (index == JH7110_CLK_PLL2_OUT) {
+		offset->dacpd = STARFIVE_JH7110_PLL2_DACPD_OFFSET;
+		offset->dsmpd = STARFIVE_JH7110_PLL2_DSMPD_OFFSET;
+		offset->fbdiv = STARFIVE_JH7110_PLL2_FBDIV_OFFSET;
+		offset->frac = STARFIVE_JH7110_PLL2_FRAC_OFFSET;
+		offset->prediv = STARFIVE_JH7110_PLL2_PREDIV_OFFSET;
+		offset->postdiv1 = STARFIVE_JH7110_PLL2_POSTDIV1_OFFSET;
+
+		mask->dacpd = STARFIVE_JH7110_PLL2_DACPD_MASK;
+		mask->dsmpd = STARFIVE_JH7110_PLL2_DSMPD_MASK;
+		mask->fbdiv = STARFIVE_JH7110_PLL2_FBDIV_MASK;
+		mask->frac = STARFIVE_JH7110_PLL2_FRAC_MASK;
+		mask->prediv = STARFIVE_JH7110_PLL2_PREDIV_MASK;
+		mask->postdiv1 = STARFIVE_JH7110_PLL2_POSTDIV1_MASK;
+
+		shift->dacpd = STARFIVE_JH7110_PLL2_DACPD_SHIFT;
+		shift->dsmpd = STARFIVE_JH7110_PLL2_DSMPD_SHIFT;
+		shift->fbdiv = STARFIVE_JH7110_PLL2_FBDIV_SHIFT;
+		shift->frac = STARFIVE_JH7110_PLL2_FRAC_SHIFT;
+		shift->prediv = STARFIVE_JH7110_PLL2_PREDIV_SHIFT;
+		shift->postdiv1 = STARFIVE_JH7110_PLL2_POSTDIV1_SHIFT;
+
+	} else {
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
+static struct clk_hw *jh7110_pll_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct jh7110_clk_pll_priv *priv = data;
+	unsigned int idx = clkspec->args[0];
+
+	if (idx < JH7110_PLLCLK_END)
+		return &priv->data[idx].hw;
+
+	return ERR_PTR(-EINVAL);
+}
+
+static int jh7110_pll_probe(struct platform_device *pdev)
+{
+	const char *pll_name[JH7110_PLLCLK_END] = {
+		"pll0_out",
+		"pll1_out",
+		"pll2_out"
+	};
+	struct jh7110_clk_pll_priv *priv;
+	struct jh7110_clk_pll_data *data;
+	int ret;
+	unsigned int idx;
+
+	priv = devm_kzalloc(&pdev->dev, struct_size(priv, data, JH7110_PLLCLK_END),
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+	priv->syscon_regmap = syscon_node_to_regmap(priv->dev->of_node->parent);
+	if (IS_ERR(priv->syscon_regmap))
+		return PTR_ERR(priv->syscon_regmap);
+
+	for (idx = 0; idx < JH7110_PLLCLK_END; idx++) {
+		struct clk_parent_data parents = {
+			.index = 0,
+		};
+		struct clk_init_data init = {
+			.name = pll_name[idx],
+			.ops = &jh7110_pll_ops,
+			.parent_data = &parents,
+			.num_parents = 1,
+			.flags = 0,
+		};
+
+		data = &priv->data[idx];
+
+		ret = jh7110_pll_data_get(data, idx);
+		if (ret)
+			return ret;
+
+		data->hw.init = &init;
+		data->idx = idx;
+
+		ret = devm_clk_hw_register(&pdev->dev, &data->hw);
+		if (ret)
+			return ret;
+	}
+
+	return devm_of_clk_add_hw_provider(&pdev->dev, jh7110_pll_get, priv);
+}
+
+static const struct of_device_id jh7110_pll_match[] = {
+	{ .compatible = "starfive,jh7110-pll" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, jh7110_pll_match);
+
+static struct platform_driver jh7110_pll_driver = {
+	.driver = {
+		.name = "clk-starfive-jh7110-pll",
+		.of_match_table = jh7110_pll_match,
+	},
+};
+builtin_platform_driver_probe(jh7110_pll_driver, jh7110_pll_probe);
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-pll.h b/drivers/clk/starfive/clk-starfive-jh7110-pll.h
new file mode 100644
index 000000000000..3deb35f144dc
--- /dev/null
+++ b/drivers/clk/starfive/clk-starfive-jh7110-pll.h
@@ -0,0 +1,293 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+/*
+ * StarFive JH7110 PLL Clock Generator Driver
+ *
+ * Copyright (C) 2022 Xingyu Wu <xingyu.wu@starfivetech.com>
+ */
+
+#ifndef _CLK_STARFIVE_JH7110_PLL_H_
+#define _CLK_STARFIVE_JH7110_PLL_H_
+
+#include <linux/bits.h>
+
+/* The decimal places are counted by expanding them by a factor of STARFIVE_PLL_FRAC_PATR_SIZE. */
+#define STARFIVE_PLL_FRAC_PATR_SIZE		1000
+
+#define STARFIVE_JH7110_PLL0_DACPD_OFFSET	0x18
+#define STARFIVE_JH7110_PLL0_DACPD_SHIFT	24
+#define STARFIVE_JH7110_PLL0_DACPD_MASK		BIT(24)
+#define STARFIVE_JH7110_PLL0_DSMPD_OFFSET	0x18
+#define STARFIVE_JH7110_PLL0_DSMPD_SHIFT	25
+#define STARFIVE_JH7110_PLL0_DSMPD_MASK		BIT(25)
+#define STARFIVE_JH7110_PLL0_FBDIV_OFFSET	0x1c
+#define STARFIVE_JH7110_PLL0_FBDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL0_FBDIV_MASK		GENMASK(11, 0)
+#define STARFIVE_JH7110_PLL0_FRAC_OFFSET	0x20
+#define STARFIVE_JH7110_PLL0_FRAC_SHIFT		0
+#define STARFIVE_JH7110_PLL0_FRAC_MASK		GENMASK(23, 0)
+#define STARFIVE_JH7110_PLL0_POSTDIV1_OFFSET	0x20
+#define STARFIVE_JH7110_PLL0_POSTDIV1_SHIFT	28
+#define STARFIVE_JH7110_PLL0_POSTDIV1_MASK	GENMASK(29, 28)
+#define STARFIVE_JH7110_PLL0_PREDIV_OFFSET	0x24
+#define STARFIVE_JH7110_PLL0_PREDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL0_PREDIV_MASK	GENMASK(5, 0)
+
+#define STARFIVE_JH7110_PLL1_DACPD_OFFSET	0x24
+#define STARFIVE_JH7110_PLL1_DACPD_SHIFT	15
+#define STARFIVE_JH7110_PLL1_DACPD_MASK		BIT(15)
+#define STARFIVE_JH7110_PLL1_DSMPD_OFFSET	0x24
+#define STARFIVE_JH7110_PLL1_DSMPD_SHIFT	16
+#define STARFIVE_JH7110_PLL1_DSMPD_MASK		BIT(16)
+#define STARFIVE_JH7110_PLL1_FBDIV_OFFSET	0x24
+#define STARFIVE_JH7110_PLL1_FBDIV_SHIFT	17
+#define STARFIVE_JH7110_PLL1_FBDIV_MASK		GENMASK(28, 17)
+#define STARFIVE_JH7110_PLL1_FRAC_OFFSET	0x28
+#define STARFIVE_JH7110_PLL1_FRAC_SHIFT		0
+#define STARFIVE_JH7110_PLL1_FRAC_MASK		GENMASK(23, 0)
+#define STARFIVE_JH7110_PLL1_POSTDIV1_OFFSET	0x28
+#define STARFIVE_JH7110_PLL1_POSTDIV1_SHIFT	28
+#define STARFIVE_JH7110_PLL1_POSTDIV1_MASK	GENMASK(29, 28)
+#define STARFIVE_JH7110_PLL1_PREDIV_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL1_PREDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL1_PREDIV_MASK	GENMASK(5, 0)
+
+#define STARFIVE_JH7110_PLL2_DACPD_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL2_DACPD_SHIFT	15
+#define STARFIVE_JH7110_PLL2_DACPD_MASK		BIT(15)
+#define STARFIVE_JH7110_PLL2_DSMPD_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL2_DSMPD_SHIFT	16
+#define STARFIVE_JH7110_PLL2_DSMPD_MASK		BIT(16)
+#define STARFIVE_JH7110_PLL2_FBDIV_OFFSET	0x2c
+#define STARFIVE_JH7110_PLL2_FBDIV_SHIFT	17
+#define STARFIVE_JH7110_PLL2_FBDIV_MASK		GENMASK(28, 17)
+#define STARFIVE_JH7110_PLL2_FRAC_OFFSET	0x30
+#define STARFIVE_JH7110_PLL2_FRAC_SHIFT		0
+#define STARFIVE_JH7110_PLL2_FRAC_MASK		GENMASK(23, 0)
+#define STARFIVE_JH7110_PLL2_POSTDIV1_OFFSET	0x30
+#define STARFIVE_JH7110_PLL2_POSTDIV1_SHIFT	28
+#define STARFIVE_JH7110_PLL2_POSTDIV1_MASK	GENMASK(29, 28)
+#define STARFIVE_JH7110_PLL2_PREDIV_OFFSET	0x34
+#define STARFIVE_JH7110_PLL2_PREDIV_SHIFT	0
+#define STARFIVE_JH7110_PLL2_PREDIV_MASK	GENMASK(5, 0)
+
+struct jh7110_pll_syscon_offset {
+	unsigned int dacpd;
+	unsigned int dsmpd;
+	unsigned int fbdiv;
+	unsigned int frac;
+	unsigned int prediv;
+	unsigned int postdiv1;
+};
+
+struct jh7110_pll_syscon_mask {
+	u32 dacpd;
+	u32 dsmpd;
+	u32 fbdiv;
+	u32 frac;
+	u32 prediv;
+	u32 postdiv1;
+};
+
+struct jh7110_pll_syscon_shift {
+	char dacpd;
+	char dsmpd;
+	char fbdiv;
+	char frac;
+	char prediv;
+	char postdiv1;
+};
+
+struct jh7110_clk_pll_data {
+	struct clk_hw hw;
+	unsigned int idx;
+	unsigned int freq_select_idx;
+
+	struct jh7110_pll_syscon_offset offset;
+	struct jh7110_pll_syscon_mask mask;
+	struct jh7110_pll_syscon_shift shift;
+};
+
+struct jh7110_clk_pll_priv {
+	struct device *dev;
+	struct regmap *syscon_regmap;
+	struct jh7110_clk_pll_data data[];
+};
+
+struct starfive_pll_syscon_value {
+	unsigned long freq;
+	u32 prediv;
+	u32 fbdiv;
+	u32 postdiv1;
+/* Both daxpd and dsmpd set 1 while integer multiple mode */
+/* Both daxpd and dsmpd set 0 while fraction multiple mode */
+	u32 dacpd;
+	u32 dsmpd;
+/* frac value should be decimals multiplied by 2^24 */
+	u32 frac;
+};
+
+enum starfive_pll0_freq_index {
+	PLL0_FREQ_375 = 0,
+	PLL0_FREQ_500,
+	PLL0_FREQ_625,
+	PLL0_FREQ_750,
+	PLL0_FREQ_875,
+	PLL0_FREQ_1000,
+	PLL0_FREQ_1250,
+	PLL0_FREQ_1375,
+	PLL0_FREQ_1500,
+	PLL0_FREQ_MAX
+};
+
+enum starfive_pll1_freq_index {
+	PLL1_FREQ_1066 = 0,
+	PLL1_FREQ_1200,
+	PLL1_FREQ_1400,
+	PLL1_FREQ_1600,
+	PLL1_FREQ_MAX
+};
+
+enum starfive_pll2_freq_index {
+	PLL2_FREQ_1188 = 0,
+	PLL2_FREQ_12288,
+	PLL2_FREQ_MAX
+};
+
+/*
+ * Because the pll frequency is relatively fixed,
+ * it cannot be set arbitrarily, so it needs a specific configuration.
+ * PLL0 frequency should be multiple of 125MHz (USB frequency).
+ */
+static const struct starfive_pll_syscon_value
+	jh7110_pll0_syscon_freq[] = {
+	[PLL0_FREQ_375] = {
+		.freq = 375000000,
+		.prediv = 8,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_500] = {
+		.freq = 500000000,
+		.prediv = 6,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_625] = {
+		.freq = 625000000,
+		.prediv = 24,
+		.fbdiv = 625,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_750] = {
+		.freq = 750000000,
+		.prediv = 4,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_875] = {
+		.freq = 875000000,
+		.prediv = 24,
+		.fbdiv = 875,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1000] = {
+		.freq = 1000000000,
+		.prediv = 3,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1250] = {
+		.freq = 1250000000,
+		.prediv = 12,
+		.fbdiv = 625,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1375] = {
+		.freq = 1375000000,
+		.prediv = 24,
+		.fbdiv = 1375,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL0_FREQ_1500] = {
+		.freq = 1500000000,
+		.prediv = 2,
+		.fbdiv = 125,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+};
+
+static const struct starfive_pll_syscon_value
+	jh7110_pll1_syscon_freq[] = {
+	[PLL1_FREQ_1066] = {
+		.freq = 1066000000,
+		.prediv = 12,
+		.fbdiv = 533,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL1_FREQ_1200] = {
+		.freq = 1200000000,
+		.prediv = 1,
+		.fbdiv = 50,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL1_FREQ_1400] = {
+		.freq = 1400000000,
+		.prediv = 6,
+		.fbdiv = 350,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL1_FREQ_1600] = {
+		.freq = 1600000000,
+		.prediv = 3,
+		.fbdiv = 200,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+};
+
+static const struct starfive_pll_syscon_value
+	jh7110_pll2_syscon_freq[] = {
+	[PLL2_FREQ_1188] = {
+		.freq = 1188000000,
+		.prediv = 2,
+		.fbdiv = 99,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+	[PLL2_FREQ_12288] = {
+		.freq = 1228800000,
+		.prediv = 5,
+		.fbdiv = 256,
+		.postdiv1 = 1,
+		.dacpd = 1,
+		.dsmpd = 1,
+	},
+};
+
+#endif
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

Add optional compatible and patternProperties.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
index ae7f1d6916af..b61d8921ef42 100644
--- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
+++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
@@ -15,16 +15,31 @@ description: |
 
 properties:
   compatible:
-    items:
-      - enum:
-          - starfive,jh7110-aon-syscon
-          - starfive,jh7110-stg-syscon
-          - starfive,jh7110-sys-syscon
-      - const: syscon
+    oneOf:
+      - items:
+          - enum:
+              - starfive,jh7110-aon-syscon
+              - starfive,jh7110-stg-syscon
+              - starfive,jh7110-sys-syscon
+          - const: syscon
+      - items:
+          - enum:
+              - starfive,jh7110-aon-syscon
+              - starfive,jh7110-stg-syscon
+              - starfive,jh7110-sys-syscon
+          - const: syscon
+          - const: simple-mfd
 
   reg:
     maxItems: 1
 
+patternProperties:
+  # Optional children
+  "pll-clock-controller":
+    type: object
+    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
+    description: Clock provider for PLL.
+
 required:
   - compatible
   - reg
@@ -38,4 +53,16 @@ examples:
         reg = <0x10240000 0x1000>;
     };
 
+  - |
+    syscon@13030000 {
+        compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
+        reg = <0x13030000 0x1000>;
+
+        pll-clock-controller {
+            compatible = "starfive,jh7110-pll";
+            clocks = <&osc>;
+            #clock-cells = <1>;
+        };
+    };
+
 ...
-- 
2.25.1

[PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

Add optional compatible and patternProperties.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
index ae7f1d6916af..b61d8921ef42 100644
--- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
+++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
@@ -15,16 +15,31 @@ description: |
 
 properties:
   compatible:
-    items:
-      - enum:
-          - starfive,jh7110-aon-syscon
-          - starfive,jh7110-stg-syscon
-          - starfive,jh7110-sys-syscon
-      - const: syscon
+    oneOf:
+      - items:
+          - enum:
+              - starfive,jh7110-aon-syscon
+              - starfive,jh7110-stg-syscon
+              - starfive,jh7110-sys-syscon
+          - const: syscon
+      - items:
+          - enum:
+              - starfive,jh7110-aon-syscon
+              - starfive,jh7110-stg-syscon
+              - starfive,jh7110-sys-syscon
+          - const: syscon
+          - const: simple-mfd
 
   reg:
     maxItems: 1
 
+patternProperties:
+  # Optional children
+  "pll-clock-controller":
+    type: object
+    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
+    description: Clock provider for PLL.
+
 required:
   - compatible
   - reg
@@ -38,4 +53,16 @@ examples:
         reg = <0x10240000 0x1000>;
     };
 
+  - |
+    syscon@13030000 {
+        compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
+        reg = <0x13030000 0x1000>;
+
+        pll-clock-controller {
+            compatible = "starfive,jh7110-pll";
+            clocks = <&osc>;
+            #clock-cells = <1>;
+        };
+    };
+
 ...
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 4/6] dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs

[Xingyu Wu]

Add PLL clock inputs from PLL clock generator.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../clock/starfive,jh7110-syscrg.yaml         | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml
index 84373ae31644..55d4e7f09cd5 100644
--- a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml
+++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml
@@ -27,6 +27,9 @@ properties:
           - description: External I2S RX left/right channel clock
           - description: External TDM clock
           - description: External audio master clock
+          - description: PLL0
+          - description: PLL1
+          - description: PLL2
 
       - items:
           - description: Main Oscillator (24 MHz)
@@ -38,6 +41,9 @@ properties:
           - description: External I2S RX left/right channel clock
           - description: External TDM clock
           - description: External audio master clock
+          - description: PLL0
+          - description: PLL1
+          - description: PLL2
 
   clock-names:
     oneOf:
@@ -52,6 +58,9 @@ properties:
           - const: i2srx_lrck_ext
           - const: tdm_ext
           - const: mclk_ext
+          - const: pll0_out
+          - const: pll1_out
+          - const: pll2_out
 
       - items:
           - const: osc
@@ -63,6 +72,9 @@ properties:
           - const: i2srx_lrck_ext
           - const: tdm_ext
           - const: mclk_ext
+          - const: pll0_out
+          - const: pll1_out
+          - const: pll2_out
 
   '#clock-cells':
     const: 1
@@ -93,12 +105,16 @@ examples:
                  <&gmac1_rgmii_rxin>,
                  <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
                  <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
-                 <&tdm_ext>, <&mclk_ext>;
+                 <&tdm_ext>, <&mclk_ext>,
+                 <&pllclk JH7110_CLK_PLL0_OUT>,
+                 <&pllclk JH7110_CLK_PLL1_OUT>,
+                 <&pllclk JH7110_CLK_PLL2_OUT>;
         clock-names = "osc", "gmac1_rmii_refin",
                       "gmac1_rgmii_rxin",
                       "i2stx_bclk_ext", "i2stx_lrck_ext",
                       "i2srx_bclk_ext", "i2srx_lrck_ext",
-                      "tdm_ext", "mclk_ext";
+                      "tdm_ext", "mclk_ext",
+                      "pll0_out", "pll1_out", "pll2_out";
         #clock-cells = <1>;
         #reset-cells = <1>;
     };
-- 
2.25.1

[PATCH v2 4/6] dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs

[Xingyu Wu]

Add PLL clock inputs from PLL clock generator.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 .../clock/starfive,jh7110-syscrg.yaml         | 20 +++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml
index 84373ae31644..55d4e7f09cd5 100644
--- a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml
+++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml
@@ -27,6 +27,9 @@ properties:
           - description: External I2S RX left/right channel clock
           - description: External TDM clock
           - description: External audio master clock
+          - description: PLL0
+          - description: PLL1
+          - description: PLL2
 
       - items:
           - description: Main Oscillator (24 MHz)
@@ -38,6 +41,9 @@ properties:
           - description: External I2S RX left/right channel clock
           - description: External TDM clock
           - description: External audio master clock
+          - description: PLL0
+          - description: PLL1
+          - description: PLL2
 
   clock-names:
     oneOf:
@@ -52,6 +58,9 @@ properties:
           - const: i2srx_lrck_ext
           - const: tdm_ext
           - const: mclk_ext
+          - const: pll0_out
+          - const: pll1_out
+          - const: pll2_out
 
       - items:
           - const: osc
@@ -63,6 +72,9 @@ properties:
           - const: i2srx_lrck_ext
           - const: tdm_ext
           - const: mclk_ext
+          - const: pll0_out
+          - const: pll1_out
+          - const: pll2_out
 
   '#clock-cells':
     const: 1
@@ -93,12 +105,16 @@ examples:
                  <&gmac1_rgmii_rxin>,
                  <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
                  <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
-                 <&tdm_ext>, <&mclk_ext>;
+                 <&tdm_ext>, <&mclk_ext>,
+                 <&pllclk JH7110_CLK_PLL0_OUT>,
+                 <&pllclk JH7110_CLK_PLL1_OUT>,
+                 <&pllclk JH7110_CLK_PLL2_OUT>;
         clock-names = "osc", "gmac1_rmii_refin",
                       "gmac1_rgmii_rxin",
                       "i2stx_bclk_ext", "i2stx_lrck_ext",
                       "i2srx_bclk_ext", "i2srx_lrck_ext",
-                      "tdm_ext", "mclk_ext";
+                      "tdm_ext", "mclk_ext",
+                      "pll0_out", "pll1_out", "pll2_out";
         #clock-cells = <1>;
         #reset-cells = <1>;
     };
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 5/6] clk: starfive: jh7110-sys: Modify PLL clocks source

[Xingyu Wu]

Modify PLL clocks source to be got from dts instead of
the fixed factor clocks.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 drivers/clk/starfive/Kconfig                  |  1 +
 .../clk/starfive/clk-starfive-jh7110-sys.c    | 35 ++++---------------
 2 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
index e306edf4defa..903a5097c642 100644
--- a/drivers/clk/starfive/Kconfig
+++ b/drivers/clk/starfive/Kconfig
@@ -35,6 +35,7 @@ config CLK_STARFIVE_JH7110_SYS
 	select AUXILIARY_BUS
 	select CLK_STARFIVE_JH71X0
 	select RESET_STARFIVE_JH7110
+	select CLK_STARFIVE_JH7110_PLL
 	default ARCH_STARFIVE
 	help
 	  Say yes here to support the system clock controller on the
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-sys.c b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
index b90d8035ba18..4bd8ff5ff912 100644
--- a/drivers/clk/starfive/clk-starfive-jh7110-sys.c
+++ b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
@@ -329,9 +329,6 @@ static struct clk_hw *jh7110_sysclk_get(struct of_phandle_args *clkspec, void *d
 	if (idx < JH7110_SYSCLK_END)
 		return &priv->reg[idx].hw;
 
-	if (idx >= JH7110_SYSCLK_PLL0_OUT && idx <= JH7110_SYSCLK_PLL2_OUT)
-		return priv->pll[idx - JH7110_SYSCLK_PLL0_OUT];
-
 	return ERR_PTR(-EINVAL);
 }
 
@@ -355,29 +352,6 @@ static int __init jh7110_syscrg_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(priv->dev, (void *)(&priv->base));
 
-	/*
-	 * These PLL clocks are not actually fixed factor clocks and can be
-	 * controlled by the syscon registers of JH7110. They will be dropped
-	 * and registered in the PLL clock driver instead.
-	 */
-	/* 24MHz -> 1000.0MHz */
-	priv->pll[0] = devm_clk_hw_register_fixed_factor(priv->dev, "pll0_out",
-							 "osc", 0, 125, 3);
-	if (IS_ERR(priv->pll[0]))
-		return PTR_ERR(priv->pll[0]);
-
-	/* 24MHz -> 1066.0MHz */
-	priv->pll[1] = devm_clk_hw_register_fixed_factor(priv->dev, "pll1_out",
-							 "osc", 0, 533, 12);
-	if (IS_ERR(priv->pll[1]))
-		return PTR_ERR(priv->pll[1]);
-
-	/* 24MHz -> 1188.0MHz */
-	priv->pll[2] = devm_clk_hw_register_fixed_factor(priv->dev, "pll2_out",
-							 "osc", 0, 99, 2);
-	if (IS_ERR(priv->pll[2]))
-		return PTR_ERR(priv->pll[2]);
-
 	for (idx = 0; idx < JH7110_SYSCLK_END; idx++) {
 		u32 max = jh7110_sysclk_data[idx].max;
 		struct clk_parent_data parents[4] = {};
@@ -415,9 +389,12 @@ static int __init jh7110_syscrg_probe(struct platform_device *pdev)
 				parents[i].fw_name = "tdm_ext";
 			else if (pidx == JH7110_SYSCLK_MCLK_EXT)
 				parents[i].fw_name = "mclk_ext";
-			else if (pidx >= JH7110_SYSCLK_PLL0_OUT &&
-				 pidx <= JH7110_SYSCLK_PLL2_OUT)
-				parents[i].hw = priv->pll[pidx - JH7110_SYSCLK_PLL0_OUT];
+			else if (pidx == JH7110_SYSCLK_PLL0_OUT)
+				parents[i].fw_name = "pll0_out";
+			else if (pidx == JH7110_SYSCLK_PLL1_OUT)
+				parents[i].fw_name = "pll1_out";
+			else if (pidx == JH7110_SYSCLK_PLL2_OUT)
+				parents[i].fw_name = "pll2_out";
 		}
 
 		clk->hw.init = &init;
-- 
2.25.1

[PATCH v2 5/6] clk: starfive: jh7110-sys: Modify PLL clocks source

[Xingyu Wu]

Modify PLL clocks source to be got from dts instead of
the fixed factor clocks.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 drivers/clk/starfive/Kconfig                  |  1 +
 .../clk/starfive/clk-starfive-jh7110-sys.c    | 35 ++++---------------
 2 files changed, 7 insertions(+), 29 deletions(-)

diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
index e306edf4defa..903a5097c642 100644
--- a/drivers/clk/starfive/Kconfig
+++ b/drivers/clk/starfive/Kconfig
@@ -35,6 +35,7 @@ config CLK_STARFIVE_JH7110_SYS
 	select AUXILIARY_BUS
 	select CLK_STARFIVE_JH71X0
 	select RESET_STARFIVE_JH7110
+	select CLK_STARFIVE_JH7110_PLL
 	default ARCH_STARFIVE
 	help
 	  Say yes here to support the system clock controller on the
diff --git a/drivers/clk/starfive/clk-starfive-jh7110-sys.c b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
index b90d8035ba18..4bd8ff5ff912 100644
--- a/drivers/clk/starfive/clk-starfive-jh7110-sys.c
+++ b/drivers/clk/starfive/clk-starfive-jh7110-sys.c
@@ -329,9 +329,6 @@ static struct clk_hw *jh7110_sysclk_get(struct of_phandle_args *clkspec, void *d
 	if (idx < JH7110_SYSCLK_END)
 		return &priv->reg[idx].hw;
 
-	if (idx >= JH7110_SYSCLK_PLL0_OUT && idx <= JH7110_SYSCLK_PLL2_OUT)
-		return priv->pll[idx - JH7110_SYSCLK_PLL0_OUT];
-
 	return ERR_PTR(-EINVAL);
 }
 
@@ -355,29 +352,6 @@ static int __init jh7110_syscrg_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(priv->dev, (void *)(&priv->base));
 
-	/*
-	 * These PLL clocks are not actually fixed factor clocks and can be
-	 * controlled by the syscon registers of JH7110. They will be dropped
-	 * and registered in the PLL clock driver instead.
-	 */
-	/* 24MHz -> 1000.0MHz */
-	priv->pll[0] = devm_clk_hw_register_fixed_factor(priv->dev, "pll0_out",
-							 "osc", 0, 125, 3);
-	if (IS_ERR(priv->pll[0]))
-		return PTR_ERR(priv->pll[0]);
-
-	/* 24MHz -> 1066.0MHz */
-	priv->pll[1] = devm_clk_hw_register_fixed_factor(priv->dev, "pll1_out",
-							 "osc", 0, 533, 12);
-	if (IS_ERR(priv->pll[1]))
-		return PTR_ERR(priv->pll[1]);
-
-	/* 24MHz -> 1188.0MHz */
-	priv->pll[2] = devm_clk_hw_register_fixed_factor(priv->dev, "pll2_out",
-							 "osc", 0, 99, 2);
-	if (IS_ERR(priv->pll[2]))
-		return PTR_ERR(priv->pll[2]);
-
 	for (idx = 0; idx < JH7110_SYSCLK_END; idx++) {
 		u32 max = jh7110_sysclk_data[idx].max;
 		struct clk_parent_data parents[4] = {};
@@ -415,9 +389,12 @@ static int __init jh7110_syscrg_probe(struct platform_device *pdev)
 				parents[i].fw_name = "tdm_ext";
 			else if (pidx == JH7110_SYSCLK_MCLK_EXT)
 				parents[i].fw_name = "mclk_ext";
-			else if (pidx >= JH7110_SYSCLK_PLL0_OUT &&
-				 pidx <= JH7110_SYSCLK_PLL2_OUT)
-				parents[i].hw = priv->pll[pidx - JH7110_SYSCLK_PLL0_OUT];
+			else if (pidx == JH7110_SYSCLK_PLL0_OUT)
+				parents[i].fw_name = "pll0_out";
+			else if (pidx == JH7110_SYSCLK_PLL1_OUT)
+				parents[i].fw_name = "pll1_out";
+			else if (pidx == JH7110_SYSCLK_PLL2_OUT)
+				parents[i].fw_name = "pll2_out";
 		}
 
 		clk->hw.init = &init;
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 6/6] riscv: dts: starfive: jh7110: Add PLL clock node and modify syscrg node

[Xingyu Wu]

Add the PLL clock node for the Starfive JH7110 SoC and
modify the SYSCRG node to add PLL clocks.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 49dd62276b0d..37ccd4600da8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -461,19 +461,29 @@ syscrg: clock-controller@13020000 {
 				 <&gmac1_rgmii_rxin>,
 				 <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
 				 <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
-				 <&tdm_ext>, <&mclk_ext>;
+				 <&tdm_ext>, <&mclk_ext>,
+				 <&pllclk JH7110_CLK_PLL0_OUT>,
+				 <&pllclk JH7110_CLK_PLL1_OUT>,
+				 <&pllclk JH7110_CLK_PLL2_OUT>;
 			clock-names = "osc", "gmac1_rmii_refin",
 				      "gmac1_rgmii_rxin",
 				      "i2stx_bclk_ext", "i2stx_lrck_ext",
 				      "i2srx_bclk_ext", "i2srx_lrck_ext",
-				      "tdm_ext", "mclk_ext";
+				      "tdm_ext", "mclk_ext",
+				      "pll0_out", "pll1_out", "pll2_out";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
 
 		sys_syscon: syscon@13030000 {
-			compatible = "starfive,jh7110-sys-syscon", "syscon";
+			compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
 			reg = <0x0 0x13030000 0x0 0x1000>;
+
+			pllclk: pll-clock-controller {
+				compatible = "starfive,jh7110-pll";
+				clocks = <&osc>;
+				#clock-cells = <1>;
+			};
 		};
 
 		sysgpio: pinctrl@13040000 {
-- 
2.25.1

[PATCH v2 6/6] riscv: dts: starfive: jh7110: Add PLL clock node and modify syscrg node

[Xingyu Wu]

Add the PLL clock node for the Starfive JH7110 SoC and
modify the SYSCRG node to add PLL clocks.

Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index 49dd62276b0d..37ccd4600da8 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -461,19 +461,29 @@ syscrg: clock-controller@13020000 {
 				 <&gmac1_rgmii_rxin>,
 				 <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
 				 <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
-				 <&tdm_ext>, <&mclk_ext>;
+				 <&tdm_ext>, <&mclk_ext>,
+				 <&pllclk JH7110_CLK_PLL0_OUT>,
+				 <&pllclk JH7110_CLK_PLL1_OUT>,
+				 <&pllclk JH7110_CLK_PLL2_OUT>;
 			clock-names = "osc", "gmac1_rmii_refin",
 				      "gmac1_rgmii_rxin",
 				      "i2stx_bclk_ext", "i2stx_lrck_ext",
 				      "i2srx_bclk_ext", "i2srx_lrck_ext",
-				      "tdm_ext", "mclk_ext";
+				      "tdm_ext", "mclk_ext",
+				      "pll0_out", "pll1_out", "pll2_out";
 			#clock-cells = <1>;
 			#reset-cells = <1>;
 		};
 
 		sys_syscon: syscon@13030000 {
-			compatible = "starfive,jh7110-sys-syscon", "syscon";
+			compatible = "starfive,jh7110-sys-syscon", "syscon", "simple-mfd";
 			reg = <0x0 0x13030000 0x0 0x1000>;
+
+			pllclk: pll-clock-controller {
+				compatible = "starfive,jh7110-pll";
+				clocks = <&osc>;
+				#clock-cells = <1>;
+			};
 		};
 
 		sysgpio: pinctrl@13040000 {
-- 
2.25.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/6] dt-bindings: clock: Add StarFive JH7110 PLL clock generator

[Krzysztof Kozlowski]

On 16/03/2023 04:05, Xingyu Wu wrote:
> Add bindings for the PLL clock generator on the JH7110 RISC-V SoC.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>


> +
> +examples:
> +  - |
> +    pllclk: pll-clock-controller {

This should be just "clock-controller" (and drop the label).

With above
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 1/6] dt-bindings: clock: Add StarFive JH7110 PLL clock generator

[Krzysztof Kozlowski]

On 16/03/2023 04:05, Xingyu Wu wrote:
> Add bindings for the PLL clock generator on the JH7110 RISC-V SoC.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>


> +
> +examples:
> +  - |
> +    pllclk: pll-clock-controller {

This should be just "clock-controller" (and drop the label).

With above
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 4/6] dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs

[Krzysztof Kozlowski]

On 16/03/2023 04:05, Xingyu Wu wrote:
> Add PLL clock inputs from PLL clock generator.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 4/6] dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs

[Krzysztof Kozlowski]

On 16/03/2023 04:05, Xingyu Wu wrote:
> Add PLL clock inputs from PLL clock generator.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 16/03/2023 04:05, Xingyu Wu wrote:
> Add optional compatible and patternProperties.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---
>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> index ae7f1d6916af..b61d8921ef42 100644
> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -15,16 +15,31 @@ description: |
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - starfive,jh7110-aon-syscon
> -          - starfive,jh7110-stg-syscon
> -          - starfive,jh7110-sys-syscon
> -      - const: syscon
> +    oneOf:
> +      - items:
> +          - enum:
> +              - starfive,jh7110-aon-syscon
> +              - starfive,jh7110-stg-syscon
> +              - starfive,jh7110-sys-syscon
> +          - const: syscon
> +      - items:
> +          - enum:
> +              - starfive,jh7110-aon-syscon
> +              - starfive,jh7110-stg-syscon
> +              - starfive,jh7110-sys-syscon
> +          - const: syscon
> +          - const: simple-mfd
>  
>    reg:
>      maxItems: 1
>  
> +patternProperties:
> +  # Optional children
> +  "pll-clock-controller":

It's not a pattern.

Anyway should be clock-controller

> +    type: object
> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> +    description: Clock provider for PLL.
> +

You just added these bindings! So the initial submission was incomplete
on purpose?

No, add complete bindings.

>  required:
>    - compatible
>    - reg
> @@ -38,4 +53,16 @@ examples:
>          reg = <0x10240000 0x1000>;
>      };
>  
> +  - |
> +    syscon@13030000 {

No need for new example... Just put it in existing one.


Best regards,
Krzysztof

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 16/03/2023 04:05, Xingyu Wu wrote:
> Add optional compatible and patternProperties.
> 
> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> ---
>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>  1 file changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> index ae7f1d6916af..b61d8921ef42 100644
> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -15,16 +15,31 @@ description: |
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - starfive,jh7110-aon-syscon
> -          - starfive,jh7110-stg-syscon
> -          - starfive,jh7110-sys-syscon
> -      - const: syscon
> +    oneOf:
> +      - items:
> +          - enum:
> +              - starfive,jh7110-aon-syscon
> +              - starfive,jh7110-stg-syscon
> +              - starfive,jh7110-sys-syscon
> +          - const: syscon
> +      - items:
> +          - enum:
> +              - starfive,jh7110-aon-syscon
> +              - starfive,jh7110-stg-syscon
> +              - starfive,jh7110-sys-syscon
> +          - const: syscon
> +          - const: simple-mfd
>  
>    reg:
>      maxItems: 1
>  
> +patternProperties:
> +  # Optional children
> +  "pll-clock-controller":

It's not a pattern.

Anyway should be clock-controller

> +    type: object
> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> +    description: Clock provider for PLL.
> +

You just added these bindings! So the initial submission was incomplete
on purpose?

No, add complete bindings.

>  required:
>    - compatible
>    - reg
> @@ -38,4 +53,16 @@ examples:
>          reg = <0x10240000 0x1000>;
>      };
>  
> +  - |
> +    syscon@13030000 {

No need for new example... Just put it in existing one.


Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/6] dt-bindings: clock: Add StarFive JH7110 PLL clock generator

[Xingyu Wu]

On 2023/3/19 20:25, Krzysztof Kozlowski wrote:
> On 16/03/2023 04:05, Xingyu Wu wrote:
>> Add bindings for the PLL clock generator on the JH7110 RISC-V SoC.
>> 
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> 
> 
>> +
>> +examples:
>> +  - |
>> +    pllclk: pll-clock-controller {
> 
> This should be just "clock-controller" (and drop the label).

Will modify to "clock-controller" and drop the label.
Thanks.

> 
> With above
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 

Best regards,
Xingyu Wu

Re: [PATCH v2 1/6] dt-bindings: clock: Add StarFive JH7110 PLL clock generator

[Xingyu Wu]

On 2023/3/19 20:25, Krzysztof Kozlowski wrote:
> On 16/03/2023 04:05, Xingyu Wu wrote:
>> Add bindings for the PLL clock generator on the JH7110 RISC-V SoC.
>> 
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
> 
> 
>> +
>> +examples:
>> +  - |
>> +    pllclk: pll-clock-controller {
> 
> This should be just "clock-controller" (and drop the label).

Will modify to "clock-controller" and drop the label.
Thanks.

> 
> With above
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 

Best regards,
Xingyu Wu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
> On 16/03/2023 04:05, Xingyu Wu wrote:
>> Add optional compatible and patternProperties.
>> 
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> ---
>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>  1 file changed, 33 insertions(+), 6 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> index ae7f1d6916af..b61d8921ef42 100644
>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> @@ -15,16 +15,31 @@ description: |
>>  
>>  properties:
>>    compatible:
>> -    items:
>> -      - enum:
>> -          - starfive,jh7110-aon-syscon
>> -          - starfive,jh7110-stg-syscon
>> -          - starfive,jh7110-sys-syscon
>> -      - const: syscon
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - starfive,jh7110-aon-syscon
>> +              - starfive,jh7110-stg-syscon
>> +              - starfive,jh7110-sys-syscon
>> +          - const: syscon
>> +      - items:
>> +          - enum:
>> +              - starfive,jh7110-aon-syscon
>> +              - starfive,jh7110-stg-syscon
>> +              - starfive,jh7110-sys-syscon
>> +          - const: syscon
>> +          - const: simple-mfd
>>  
>>    reg:
>>      maxItems: 1
>>  
>> +patternProperties:
>> +  # Optional children
>> +  "pll-clock-controller":
> 
> It's not a pattern.

Does it use 'properties' instead of 'patternProperties'?

> 
> Anyway should be clock-controller

Will fix.

> 
>> +    type: object
>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>> +    description: Clock provider for PLL.
>> +
> 
> You just added these bindings! So the initial submission was incomplete
> on purpose?
> 
> No, add complete bindings.

Does you mean that it should drop the 'description', or add complete 'description',
or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?

> 
>>  required:
>>    - compatible
>>    - reg
>> @@ -38,4 +53,16 @@ examples:
>>          reg = <0x10240000 0x1000>;
>>      };
>>  
>> +  - |
>> +    syscon@13030000 {
> 
> No need for new example... Just put it in existing one.
> 

Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.

Best regards,
Xingyu Wu

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
> On 16/03/2023 04:05, Xingyu Wu wrote:
>> Add optional compatible and patternProperties.
>> 
>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>> ---
>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>  1 file changed, 33 insertions(+), 6 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> index ae7f1d6916af..b61d8921ef42 100644
>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>> @@ -15,16 +15,31 @@ description: |
>>  
>>  properties:
>>    compatible:
>> -    items:
>> -      - enum:
>> -          - starfive,jh7110-aon-syscon
>> -          - starfive,jh7110-stg-syscon
>> -          - starfive,jh7110-sys-syscon
>> -      - const: syscon
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - starfive,jh7110-aon-syscon
>> +              - starfive,jh7110-stg-syscon
>> +              - starfive,jh7110-sys-syscon
>> +          - const: syscon
>> +      - items:
>> +          - enum:
>> +              - starfive,jh7110-aon-syscon
>> +              - starfive,jh7110-stg-syscon
>> +              - starfive,jh7110-sys-syscon
>> +          - const: syscon
>> +          - const: simple-mfd
>>  
>>    reg:
>>      maxItems: 1
>>  
>> +patternProperties:
>> +  # Optional children
>> +  "pll-clock-controller":
> 
> It's not a pattern.

Does it use 'properties' instead of 'patternProperties'?

> 
> Anyway should be clock-controller

Will fix.

> 
>> +    type: object
>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>> +    description: Clock provider for PLL.
>> +
> 
> You just added these bindings! So the initial submission was incomplete
> on purpose?
> 
> No, add complete bindings.

Does you mean that it should drop the 'description', or add complete 'description',
or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?

> 
>>  required:
>>    - compatible
>>    - reg
>> @@ -38,4 +53,16 @@ examples:
>>          reg = <0x10240000 0x1000>;
>>      };
>>  
>> +  - |
>> +    syscon@13030000 {
> 
> No need for new example... Just put it in existing one.
> 

Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.

Best regards,
Xingyu Wu

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 20/03/2023 04:54, Xingyu Wu wrote:
> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>> Add optional compatible and patternProperties.
>>>
>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>> ---
>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>> index ae7f1d6916af..b61d8921ef42 100644
>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>> @@ -15,16 +15,31 @@ description: |
>>>  
>>>  properties:
>>>    compatible:
>>> -    items:
>>> -      - enum:
>>> -          - starfive,jh7110-aon-syscon
>>> -          - starfive,jh7110-stg-syscon
>>> -          - starfive,jh7110-sys-syscon
>>> -      - const: syscon
>>> +    oneOf:
>>> +      - items:
>>> +          - enum:
>>> +              - starfive,jh7110-aon-syscon
>>> +              - starfive,jh7110-stg-syscon
>>> +              - starfive,jh7110-sys-syscon
>>> +          - const: syscon
>>> +      - items:
>>> +          - enum:
>>> +              - starfive,jh7110-aon-syscon
>>> +              - starfive,jh7110-stg-syscon
>>> +              - starfive,jh7110-sys-syscon
>>> +          - const: syscon
>>> +          - const: simple-mfd
>>>  
>>>    reg:
>>>      maxItems: 1
>>>  
>>> +patternProperties:
>>> +  # Optional children
>>> +  "pll-clock-controller":
>>
>> It's not a pattern.
> 
> Does it use 'properties' instead of 'patternProperties'?

Yes.

> 
>>
>> Anyway should be clock-controller
> 
> Will fix.
> 
>>
>>> +    type: object
>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>> +    description: Clock provider for PLL.
>>> +
>>
>> You just added these bindings! So the initial submission was incomplete
>> on purpose?
>>
>> No, add complete bindings.
> 
> Does you mean that it should drop the 'description', or add complete 'description',
> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?

It means it should be squashed with the patch which adds it.

> 
>>
>>>  required:
>>>    - compatible
>>>    - reg
>>> @@ -38,4 +53,16 @@ examples:
>>>          reg = <0x10240000 0x1000>;
>>>      };
>>>  
>>> +  - |
>>> +    syscon@13030000 {
>>
>> No need for new example... Just put it in existing one.
>>
> 
> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.

So why having other examples if they are included here? Drop them.



Best regards,
Krzysztof

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 20/03/2023 04:54, Xingyu Wu wrote:
> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>> Add optional compatible and patternProperties.
>>>
>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>> ---
>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>> index ae7f1d6916af..b61d8921ef42 100644
>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>> @@ -15,16 +15,31 @@ description: |
>>>  
>>>  properties:
>>>    compatible:
>>> -    items:
>>> -      - enum:
>>> -          - starfive,jh7110-aon-syscon
>>> -          - starfive,jh7110-stg-syscon
>>> -          - starfive,jh7110-sys-syscon
>>> -      - const: syscon
>>> +    oneOf:
>>> +      - items:
>>> +          - enum:
>>> +              - starfive,jh7110-aon-syscon
>>> +              - starfive,jh7110-stg-syscon
>>> +              - starfive,jh7110-sys-syscon
>>> +          - const: syscon
>>> +      - items:
>>> +          - enum:
>>> +              - starfive,jh7110-aon-syscon
>>> +              - starfive,jh7110-stg-syscon
>>> +              - starfive,jh7110-sys-syscon
>>> +          - const: syscon
>>> +          - const: simple-mfd
>>>  
>>>    reg:
>>>      maxItems: 1
>>>  
>>> +patternProperties:
>>> +  # Optional children
>>> +  "pll-clock-controller":
>>
>> It's not a pattern.
> 
> Does it use 'properties' instead of 'patternProperties'?

Yes.

> 
>>
>> Anyway should be clock-controller
> 
> Will fix.
> 
>>
>>> +    type: object
>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>> +    description: Clock provider for PLL.
>>> +
>>
>> You just added these bindings! So the initial submission was incomplete
>> on purpose?
>>
>> No, add complete bindings.
> 
> Does you mean that it should drop the 'description', or add complete 'description',
> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?

It means it should be squashed with the patch which adds it.

> 
>>
>>>  required:
>>>    - compatible
>>>    - reg
>>> @@ -38,4 +53,16 @@ examples:
>>>          reg = <0x10240000 0x1000>;
>>>      };
>>>  
>>> +  - |
>>> +    syscon@13030000 {
>>
>> No need for new example... Just put it in existing one.
>>
> 
> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.

So why having other examples if they are included here? Drop them.



Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
> On 20/03/2023 04:54, Xingyu Wu wrote:
>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>> Add optional compatible and patternProperties.
>>>>
>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>> ---
>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>> @@ -15,16 +15,31 @@ description: |
>>>>  
>>>>  properties:
>>>>    compatible:
>>>> -    items:
>>>> -      - enum:
>>>> -          - starfive,jh7110-aon-syscon
>>>> -          - starfive,jh7110-stg-syscon
>>>> -          - starfive,jh7110-sys-syscon
>>>> -      - const: syscon
>>>> +    oneOf:
>>>> +      - items:
>>>> +          - enum:
>>>> +              - starfive,jh7110-aon-syscon
>>>> +              - starfive,jh7110-stg-syscon
>>>> +              - starfive,jh7110-sys-syscon
>>>> +          - const: syscon
>>>> +      - items:
>>>> +          - enum:
>>>> +              - starfive,jh7110-aon-syscon
>>>> +              - starfive,jh7110-stg-syscon
>>>> +              - starfive,jh7110-sys-syscon
>>>> +          - const: syscon
>>>> +          - const: simple-mfd
>>>>  
>>>>    reg:
>>>>      maxItems: 1
>>>>  
>>>> +patternProperties:
>>>> +  # Optional children
>>>> +  "pll-clock-controller":
>>>
>>> It's not a pattern.
>> 
>> Does it use 'properties' instead of 'patternProperties'?
> 
> Yes.
> 
>> 
>>>
>>> Anyway should be clock-controller
>> 
>> Will fix.
>> 
>>>
>>>> +    type: object
>>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>>> +    description: Clock provider for PLL.
>>>> +
>>>
>>> You just added these bindings! So the initial submission was incomplete
>>> on purpose?
>>>
>>> No, add complete bindings.
>> 
>> Does you mean that it should drop the 'description', or add complete 'description',
>> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?
> 
> It means it should be squashed with the patch which adds it.

Should I drop the 'decription' here and keep the 'decription' in patch1?

> 
>> 
>>>
>>>>  required:
>>>>    - compatible
>>>>    - reg
>>>> @@ -38,4 +53,16 @@ examples:
>>>>          reg = <0x10240000 0x1000>;
>>>>      };
>>>>  
>>>> +  - |
>>>> +    syscon@13030000 {
>>>
>>> No need for new example... Just put it in existing one.
>>>
>> 
>> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
>> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.
> 
> So why having other examples if they are included here? Drop them.
> 

Should I drop the old example of stg-syscon and add a new example of sys-syscon which
include clock-controller child node?

Best regards,
Xingyu Wu

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
> On 20/03/2023 04:54, Xingyu Wu wrote:
>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>> Add optional compatible and patternProperties.
>>>>
>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>> ---
>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>> @@ -15,16 +15,31 @@ description: |
>>>>  
>>>>  properties:
>>>>    compatible:
>>>> -    items:
>>>> -      - enum:
>>>> -          - starfive,jh7110-aon-syscon
>>>> -          - starfive,jh7110-stg-syscon
>>>> -          - starfive,jh7110-sys-syscon
>>>> -      - const: syscon
>>>> +    oneOf:
>>>> +      - items:
>>>> +          - enum:
>>>> +              - starfive,jh7110-aon-syscon
>>>> +              - starfive,jh7110-stg-syscon
>>>> +              - starfive,jh7110-sys-syscon
>>>> +          - const: syscon
>>>> +      - items:
>>>> +          - enum:
>>>> +              - starfive,jh7110-aon-syscon
>>>> +              - starfive,jh7110-stg-syscon
>>>> +              - starfive,jh7110-sys-syscon
>>>> +          - const: syscon
>>>> +          - const: simple-mfd
>>>>  
>>>>    reg:
>>>>      maxItems: 1
>>>>  
>>>> +patternProperties:
>>>> +  # Optional children
>>>> +  "pll-clock-controller":
>>>
>>> It's not a pattern.
>> 
>> Does it use 'properties' instead of 'patternProperties'?
> 
> Yes.
> 
>> 
>>>
>>> Anyway should be clock-controller
>> 
>> Will fix.
>> 
>>>
>>>> +    type: object
>>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>>> +    description: Clock provider for PLL.
>>>> +
>>>
>>> You just added these bindings! So the initial submission was incomplete
>>> on purpose?
>>>
>>> No, add complete bindings.
>> 
>> Does you mean that it should drop the 'description', or add complete 'description',
>> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?
> 
> It means it should be squashed with the patch which adds it.

Should I drop the 'decription' here and keep the 'decription' in patch1?

> 
>> 
>>>
>>>>  required:
>>>>    - compatible
>>>>    - reg
>>>> @@ -38,4 +53,16 @@ examples:
>>>>          reg = <0x10240000 0x1000>;
>>>>      };
>>>>  
>>>> +  - |
>>>> +    syscon@13030000 {
>>>
>>> No need for new example... Just put it in existing one.
>>>
>> 
>> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
>> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.
> 
> So why having other examples if they are included here? Drop them.
> 

Should I drop the old example of stg-syscon and add a new example of sys-syscon which
include clock-controller child node?

Best regards,
Xingyu Wu

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 20/03/2023 08:29, Xingyu Wu wrote:
> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>> Add optional compatible and patternProperties.
>>>>>
>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>> ---
>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>  
>>>>>  properties:
>>>>>    compatible:
>>>>> -    items:
>>>>> -      - enum:
>>>>> -          - starfive,jh7110-aon-syscon
>>>>> -          - starfive,jh7110-stg-syscon
>>>>> -          - starfive,jh7110-sys-syscon
>>>>> -      - const: syscon
>>>>> +    oneOf:
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - starfive,jh7110-aon-syscon
>>>>> +              - starfive,jh7110-stg-syscon
>>>>> +              - starfive,jh7110-sys-syscon
>>>>> +          - const: syscon
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - starfive,jh7110-aon-syscon
>>>>> +              - starfive,jh7110-stg-syscon
>>>>> +              - starfive,jh7110-sys-syscon
>>>>> +          - const: syscon
>>>>> +          - const: simple-mfd

BTW, this also looks wrong. You just said that clock controller exists
only in few variants. Also, why sometimes the same device  goes with
simple-mfd and sometimies without? It's the same device.

>>>>>  
>>>>>    reg:
>>>>>      maxItems: 1
>>>>>  
>>>>> +patternProperties:
>>>>> +  # Optional children
>>>>> +  "pll-clock-controller":
>>>>
>>>> It's not a pattern.
>>>
>>> Does it use 'properties' instead of 'patternProperties'?
>>
>> Yes.
>>
>>>
>>>>
>>>> Anyway should be clock-controller
>>>
>>> Will fix.
>>>
>>>>
>>>>> +    type: object
>>>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>>>> +    description: Clock provider for PLL.
>>>>> +
>>>>
>>>> You just added these bindings! So the initial submission was incomplete
>>>> on purpose?
>>>>
>>>> No, add complete bindings.
>>>
>>> Does you mean that it should drop the 'description', or add complete 'description',
>>> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?
>>
>> It means it should be squashed with the patch which adds it.
> 
> Should I drop the 'decription' here and keep the 'decription' in patch1?

There should be no this patch at all. However I do not understand what
you want to do with description. What's wrong with description?
> 
>>
>>>
>>>>
>>>>>  required:
>>>>>    - compatible
>>>>>    - reg
>>>>> @@ -38,4 +53,16 @@ examples:
>>>>>          reg = <0x10240000 0x1000>;
>>>>>      };
>>>>>  
>>>>> +  - |
>>>>> +    syscon@13030000 {
>>>>
>>>> No need for new example... Just put it in existing one.
>>>>
>>>
>>> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
>>> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.
>>
>> So why having other examples if they are included here? Drop them.
>>
> 
> Should I drop the old example of stg-syscon and add a new example of sys-syscon which
> include clock-controller child node?

No, there should be no stg-syscon example, it's useless.

Best regards,
Krzysztof

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 20/03/2023 08:29, Xingyu Wu wrote:
> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>> Add optional compatible and patternProperties.
>>>>>
>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>> ---
>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>  
>>>>>  properties:
>>>>>    compatible:
>>>>> -    items:
>>>>> -      - enum:
>>>>> -          - starfive,jh7110-aon-syscon
>>>>> -          - starfive,jh7110-stg-syscon
>>>>> -          - starfive,jh7110-sys-syscon
>>>>> -      - const: syscon
>>>>> +    oneOf:
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - starfive,jh7110-aon-syscon
>>>>> +              - starfive,jh7110-stg-syscon
>>>>> +              - starfive,jh7110-sys-syscon
>>>>> +          - const: syscon
>>>>> +      - items:
>>>>> +          - enum:
>>>>> +              - starfive,jh7110-aon-syscon
>>>>> +              - starfive,jh7110-stg-syscon
>>>>> +              - starfive,jh7110-sys-syscon
>>>>> +          - const: syscon
>>>>> +          - const: simple-mfd

BTW, this also looks wrong. You just said that clock controller exists
only in few variants. Also, why sometimes the same device  goes with
simple-mfd and sometimies without? It's the same device.

>>>>>  
>>>>>    reg:
>>>>>      maxItems: 1
>>>>>  
>>>>> +patternProperties:
>>>>> +  # Optional children
>>>>> +  "pll-clock-controller":
>>>>
>>>> It's not a pattern.
>>>
>>> Does it use 'properties' instead of 'patternProperties'?
>>
>> Yes.
>>
>>>
>>>>
>>>> Anyway should be clock-controller
>>>
>>> Will fix.
>>>
>>>>
>>>>> +    type: object
>>>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>>>> +    description: Clock provider for PLL.
>>>>> +
>>>>
>>>> You just added these bindings! So the initial submission was incomplete
>>>> on purpose?
>>>>
>>>> No, add complete bindings.
>>>
>>> Does you mean that it should drop the 'description', or add complete 'description',
>>> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?
>>
>> It means it should be squashed with the patch which adds it.
> 
> Should I drop the 'decription' here and keep the 'decription' in patch1?

There should be no this patch at all. However I do not understand what
you want to do with description. What's wrong with description?
> 
>>
>>>
>>>>
>>>>>  required:
>>>>>    - compatible
>>>>>    - reg
>>>>> @@ -38,4 +53,16 @@ examples:
>>>>>          reg = <0x10240000 0x1000>;
>>>>>      };
>>>>>  
>>>>> +  - |
>>>>> +    syscon@13030000 {
>>>>
>>>> No need for new example... Just put it in existing one.
>>>>
>>>
>>> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
>>> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.
>>
>> So why having other examples if they are included here? Drop them.
>>
> 
> Should I drop the old example of stg-syscon and add a new example of sys-syscon which
> include clock-controller child node?

No, there should be no stg-syscon example, it's useless.

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/20 15:40, Krzysztof Kozlowski wrote:
> On 20/03/2023 08:29, Xingyu Wu wrote:
>> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>>> Add optional compatible and patternProperties.
>>>>>>
>>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>>> ---
>>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>>  
>>>>>>  properties:
>>>>>>    compatible:
>>>>>> -    items:
>>>>>> -      - enum:
>>>>>> -          - starfive,jh7110-aon-syscon
>>>>>> -          - starfive,jh7110-stg-syscon
>>>>>> -          - starfive,jh7110-sys-syscon
>>>>>> -      - const: syscon
>>>>>> +    oneOf:
>>>>>> +      - items:
>>>>>> +          - enum:
>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>> +          - const: syscon
>>>>>> +      - items:
>>>>>> +          - enum:
>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>> +          - const: syscon
>>>>>> +          - const: simple-mfd
> 
> BTW, this also looks wrong. You just said that clock controller exists
> only in few variants. Also, why sometimes the same device  goes with
> simple-mfd and sometimies without? It's the same device.

Oh yes, If modified to:

oneOf:
      - items:
          - enum:
              - starfive,jh7110-aon-syscon
              - starfive,jh7110-stg-syscon
          - const: syscon
      - items:
          - const: starfive,jh7110-sys-syscon
          - const: syscon
          - const: simple-mfd

Or:

     - minItems: 2
       items:
         - enum:
             - starfive,jh7110-aon-syscon
             - starfive,jh7110-stg-syscon
             - starfive,jh7110-sys-syscon
         - const: syscon
         - const: simple-mfd


Which one is better?

> 
>>>>>>  
>>>>>>    reg:
>>>>>>      maxItems: 1
>>>>>>  
>>>>>> +patternProperties:
>>>>>> +  # Optional children
>>>>>> +  "pll-clock-controller":
>>>>>
>>>>> It's not a pattern.
>>>>
>>>> Does it use 'properties' instead of 'patternProperties'?
>>>
>>> Yes.
>>>
>>>>
>>>>>
>>>>> Anyway should be clock-controller
>>>>
>>>> Will fix.
>>>>
>>>>>
>>>>>> +    type: object
>>>>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>>>>> +    description: Clock provider for PLL.
>>>>>> +
>>>>>
>>>>> You just added these bindings! So the initial submission was incomplete
>>>>> on purpose?
>>>>>
>>>>> No, add complete bindings.
>>>>
>>>> Does you mean that it should drop the 'description', or add complete 'description',
>>>> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?
>>>
>>> It means it should be squashed with the patch which adds it.
>> 
>> Should I drop the 'decription' here and keep the 'decription' in patch1?
> 
> There should be no this patch at all. However I do not understand what
> you want to do with description. What's wrong with description?

I thought you were commenting under description, saying a conflict with pll dtbindings' description.
Is that mean I should add it in the syscon patch fo william not this patchset?

>> 
>>>
>>>>
>>>>>
>>>>>>  required:
>>>>>>    - compatible
>>>>>>    - reg
>>>>>> @@ -38,4 +53,16 @@ examples:
>>>>>>          reg = <0x10240000 0x1000>;
>>>>>>      };
>>>>>>  
>>>>>> +  - |
>>>>>> +    syscon@13030000 {
>>>>>
>>>>> No need for new example... Just put it in existing one.
>>>>>
>>>>
>>>> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
>>>> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.
>>>
>>> So why having other examples if they are included here? Drop them.
>>>
>> 
>> Should I drop the old example of stg-syscon and add a new example of sys-syscon which
>> include clock-controller child node?
> 
> No, there should be no stg-syscon example, it's useless.
> 

Thanks. I will remind william to use sys-syscon example instead.

Best regards,
Xingyu Wu

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/20 15:40, Krzysztof Kozlowski wrote:
> On 20/03/2023 08:29, Xingyu Wu wrote:
>> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>>> Add optional compatible and patternProperties.
>>>>>>
>>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>>> ---
>>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>>  
>>>>>>  properties:
>>>>>>    compatible:
>>>>>> -    items:
>>>>>> -      - enum:
>>>>>> -          - starfive,jh7110-aon-syscon
>>>>>> -          - starfive,jh7110-stg-syscon
>>>>>> -          - starfive,jh7110-sys-syscon
>>>>>> -      - const: syscon
>>>>>> +    oneOf:
>>>>>> +      - items:
>>>>>> +          - enum:
>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>> +          - const: syscon
>>>>>> +      - items:
>>>>>> +          - enum:
>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>> +          - const: syscon
>>>>>> +          - const: simple-mfd
> 
> BTW, this also looks wrong. You just said that clock controller exists
> only in few variants. Also, why sometimes the same device  goes with
> simple-mfd and sometimies without? It's the same device.

Oh yes, If modified to:

oneOf:
      - items:
          - enum:
              - starfive,jh7110-aon-syscon
              - starfive,jh7110-stg-syscon
          - const: syscon
      - items:
          - const: starfive,jh7110-sys-syscon
          - const: syscon
          - const: simple-mfd

Or:

     - minItems: 2
       items:
         - enum:
             - starfive,jh7110-aon-syscon
             - starfive,jh7110-stg-syscon
             - starfive,jh7110-sys-syscon
         - const: syscon
         - const: simple-mfd


Which one is better?

> 
>>>>>>  
>>>>>>    reg:
>>>>>>      maxItems: 1
>>>>>>  
>>>>>> +patternProperties:
>>>>>> +  # Optional children
>>>>>> +  "pll-clock-controller":
>>>>>
>>>>> It's not a pattern.
>>>>
>>>> Does it use 'properties' instead of 'patternProperties'?
>>>
>>> Yes.
>>>
>>>>
>>>>>
>>>>> Anyway should be clock-controller
>>>>
>>>> Will fix.
>>>>
>>>>>
>>>>>> +    type: object
>>>>>> +    $ref: /schemas/clock/starfive,jh7110-pll.yaml#
>>>>>> +    description: Clock provider for PLL.
>>>>>> +
>>>>>
>>>>> You just added these bindings! So the initial submission was incomplete
>>>>> on purpose?
>>>>>
>>>>> No, add complete bindings.
>>>>
>>>> Does you mean that it should drop the 'description', or add complete 'description',
>>>> or add 'compatible', 'clocks' and 'clock-cells' of complete clock-controller bindings?
>>>
>>> It means it should be squashed with the patch which adds it.
>> 
>> Should I drop the 'decription' here and keep the 'decription' in patch1?
> 
> There should be no this patch at all. However I do not understand what
> you want to do with description. What's wrong with description?

I thought you were commenting under description, saying a conflict with pll dtbindings' description.
Is that mean I should add it in the syscon patch fo william not this patchset?

>> 
>>>
>>>>
>>>>>
>>>>>>  required:
>>>>>>    - compatible
>>>>>>    - reg
>>>>>> @@ -38,4 +53,16 @@ examples:
>>>>>>          reg = <0x10240000 0x1000>;
>>>>>>      };
>>>>>>  
>>>>>> +  - |
>>>>>> +    syscon@13030000 {
>>>>>
>>>>> No need for new example... Just put it in existing one.
>>>>>
>>>>
>>>> Actually, the PLL clock-controller are just set in sys-syscon resgisters. The stg-syscon and
>>>> aon-syscon don't need it. So PLL clock-controller node only is added in sys-syscon node.
>>>
>>> So why having other examples if they are included here? Drop them.
>>>
>> 
>> Should I drop the old example of stg-syscon and add a new example of sys-syscon which
>> include clock-controller child node?
> 
> No, there should be no stg-syscon example, it's useless.
> 

Thanks. I will remind william to use sys-syscon example instead.

Best regards,
Xingyu Wu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 20/03/2023 09:26, Xingyu Wu wrote:
> On 2023/3/20 15:40, Krzysztof Kozlowski wrote:
>> On 20/03/2023 08:29, Xingyu Wu wrote:
>>> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>>>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>>>> Add optional compatible and patternProperties.
>>>>>>>
>>>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>>>> ---
>>>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>>>  
>>>>>>>  properties:
>>>>>>>    compatible:
>>>>>>> -    items:
>>>>>>> -      - enum:
>>>>>>> -          - starfive,jh7110-aon-syscon
>>>>>>> -          - starfive,jh7110-stg-syscon
>>>>>>> -          - starfive,jh7110-sys-syscon
>>>>>>> -      - const: syscon
>>>>>>> +    oneOf:
>>>>>>> +      - items:
>>>>>>> +          - enum:
>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>> +          - const: syscon
>>>>>>> +      - items:
>>>>>>> +          - enum:
>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>> +          - const: syscon
>>>>>>> +          - const: simple-mfd
>>
>> BTW, this also looks wrong. You just said that clock controller exists
>> only in few variants. Also, why sometimes the same device  goes with
>> simple-mfd and sometimies without? It's the same device.
> 
> Oh yes, If modified to:
> 
> oneOf:
>       - items:
>           - enum:
>               - starfive,jh7110-aon-syscon
>               - starfive,jh7110-stg-syscon
>           - const: syscon
>       - items:
>           - const: starfive,jh7110-sys-syscon
>           - const: syscon
>           - const: simple-mfd
> 
> Or:
> 
>      - minItems: 2
>        items:
>          - enum:
>              - starfive,jh7110-aon-syscon
>              - starfive,jh7110-stg-syscon
>              - starfive,jh7110-sys-syscon
>          - const: syscon
>          - const: simple-mfd
> 
> 
> Which one is better?

If aon and stg are not supposed to have children, then only the first is
correct. It's not which is better, the second is not really correct in
such case.

Best regards,
Krzysztof

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Krzysztof Kozlowski]

On 20/03/2023 09:26, Xingyu Wu wrote:
> On 2023/3/20 15:40, Krzysztof Kozlowski wrote:
>> On 20/03/2023 08:29, Xingyu Wu wrote:
>>> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>>>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>>>> Add optional compatible and patternProperties.
>>>>>>>
>>>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>>>> ---
>>>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>>>  
>>>>>>>  properties:
>>>>>>>    compatible:
>>>>>>> -    items:
>>>>>>> -      - enum:
>>>>>>> -          - starfive,jh7110-aon-syscon
>>>>>>> -          - starfive,jh7110-stg-syscon
>>>>>>> -          - starfive,jh7110-sys-syscon
>>>>>>> -      - const: syscon
>>>>>>> +    oneOf:
>>>>>>> +      - items:
>>>>>>> +          - enum:
>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>> +          - const: syscon
>>>>>>> +      - items:
>>>>>>> +          - enum:
>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>> +          - const: syscon
>>>>>>> +          - const: simple-mfd
>>
>> BTW, this also looks wrong. You just said that clock controller exists
>> only in few variants. Also, why sometimes the same device  goes with
>> simple-mfd and sometimies without? It's the same device.
> 
> Oh yes, If modified to:
> 
> oneOf:
>       - items:
>           - enum:
>               - starfive,jh7110-aon-syscon
>               - starfive,jh7110-stg-syscon
>           - const: syscon
>       - items:
>           - const: starfive,jh7110-sys-syscon
>           - const: syscon
>           - const: simple-mfd
> 
> Or:
> 
>      - minItems: 2
>        items:
>          - enum:
>              - starfive,jh7110-aon-syscon
>              - starfive,jh7110-stg-syscon
>              - starfive,jh7110-sys-syscon
>          - const: syscon
>          - const: simple-mfd
> 
> 
> Which one is better?

If aon and stg are not supposed to have children, then only the first is
correct. It's not which is better, the second is not really correct in
such case.

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/20 16:36, Krzysztof Kozlowski wrote:
> On 20/03/2023 09:26, Xingyu Wu wrote:
>> On 2023/3/20 15:40, Krzysztof Kozlowski wrote:
>>> On 20/03/2023 08:29, Xingyu Wu wrote:
>>>> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>>>>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>>>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>>>>> Add optional compatible and patternProperties.
>>>>>>>>
>>>>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>>>>> ---
>>>>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>>>>  
>>>>>>>>  properties:
>>>>>>>>    compatible:
>>>>>>>> -    items:
>>>>>>>> -      - enum:
>>>>>>>> -          - starfive,jh7110-aon-syscon
>>>>>>>> -          - starfive,jh7110-stg-syscon
>>>>>>>> -          - starfive,jh7110-sys-syscon
>>>>>>>> -      - const: syscon
>>>>>>>> +    oneOf:
>>>>>>>> +      - items:
>>>>>>>> +          - enum:
>>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>>> +          - const: syscon
>>>>>>>> +      - items:
>>>>>>>> +          - enum:
>>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>>> +          - const: syscon
>>>>>>>> +          - const: simple-mfd
>>>
>>> BTW, this also looks wrong. You just said that clock controller exists
>>> only in few variants. Also, why sometimes the same device  goes with
>>> simple-mfd and sometimies without? It's the same device.
>> 
>> Oh yes, If modified to:
>> 
>> oneOf:
>>       - items:
>>           - enum:
>>               - starfive,jh7110-aon-syscon
>>               - starfive,jh7110-stg-syscon
>>           - const: syscon
>>       - items:
>>           - const: starfive,jh7110-sys-syscon
>>           - const: syscon
>>           - const: simple-mfd
>> 
>> Or:
>> 
>>      - minItems: 2
>>        items:
>>          - enum:
>>              - starfive,jh7110-aon-syscon
>>              - starfive,jh7110-stg-syscon
>>              - starfive,jh7110-sys-syscon
>>          - const: syscon
>>          - const: simple-mfd
>> 
>> 
>> Which one is better?
> 
> If aon and stg are not supposed to have children, then only the first is
> correct. It's not which is better, the second is not really correct in
> such case.
> 

OK, will modify it in next version. Thanks.

Best regards,
Xingyu Wu

Re: [PATCH v2 3/6] dt-bindings: soc: starfive: syscon: Add optional patternProperties

[Xingyu Wu]

On 2023/3/20 16:36, Krzysztof Kozlowski wrote:
> On 20/03/2023 09:26, Xingyu Wu wrote:
>> On 2023/3/20 15:40, Krzysztof Kozlowski wrote:
>>> On 20/03/2023 08:29, Xingyu Wu wrote:
>>>> On 2023/3/20 14:37, Krzysztof Kozlowski wrote:
>>>>> On 20/03/2023 04:54, Xingyu Wu wrote:
>>>>>> On 2023/3/19 20:28, Krzysztof Kozlowski wrote:
>>>>>>> On 16/03/2023 04:05, Xingyu Wu wrote:
>>>>>>>> Add optional compatible and patternProperties.
>>>>>>>>
>>>>>>>> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com>
>>>>>>>> ---
>>>>>>>>  .../soc/starfive/starfive,jh7110-syscon.yaml  | 39 ++++++++++++++++---
>>>>>>>>  1 file changed, 33 insertions(+), 6 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>>> index ae7f1d6916af..b61d8921ef42 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>>> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
>>>>>>>> @@ -15,16 +15,31 @@ description: |
>>>>>>>>  
>>>>>>>>  properties:
>>>>>>>>    compatible:
>>>>>>>> -    items:
>>>>>>>> -      - enum:
>>>>>>>> -          - starfive,jh7110-aon-syscon
>>>>>>>> -          - starfive,jh7110-stg-syscon
>>>>>>>> -          - starfive,jh7110-sys-syscon
>>>>>>>> -      - const: syscon
>>>>>>>> +    oneOf:
>>>>>>>> +      - items:
>>>>>>>> +          - enum:
>>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>>> +          - const: syscon
>>>>>>>> +      - items:
>>>>>>>> +          - enum:
>>>>>>>> +              - starfive,jh7110-aon-syscon
>>>>>>>> +              - starfive,jh7110-stg-syscon
>>>>>>>> +              - starfive,jh7110-sys-syscon
>>>>>>>> +          - const: syscon
>>>>>>>> +          - const: simple-mfd
>>>
>>> BTW, this also looks wrong. You just said that clock controller exists
>>> only in few variants. Also, why sometimes the same device  goes with
>>> simple-mfd and sometimies without? It's the same device.
>> 
>> Oh yes, If modified to:
>> 
>> oneOf:
>>       - items:
>>           - enum:
>>               - starfive,jh7110-aon-syscon
>>               - starfive,jh7110-stg-syscon
>>           - const: syscon
>>       - items:
>>           - const: starfive,jh7110-sys-syscon
>>           - const: syscon
>>           - const: simple-mfd
>> 
>> Or:
>> 
>>      - minItems: 2
>>        items:
>>          - enum:
>>              - starfive,jh7110-aon-syscon
>>              - starfive,jh7110-stg-syscon
>>              - starfive,jh7110-sys-syscon
>>          - const: syscon
>>          - const: simple-mfd
>> 
>> 
>> Which one is better?
> 
> If aon and stg are not supposed to have children, then only the first is
> correct. It's not which is better, the second is not really correct in
> such case.
> 

OK, will modify it in next version. Thanks.

Best regards,
Xingyu Wu


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv