[PATCH v2 0/3] add Amlogic A1 clock controller driver

[PATCH v2 0/3] add Amlogic A1 clock controller driver

[Jian Hu]

add support for Amlogic A1 clock driver, the clock includes 
three parts: peripheral clocks, pll clocks, CPU clocks.
sys pll and CPU clocks will be sent in next patch.

Changes since v1 at [1]:
-place A1 config alphabetically
-add actual reason for RO ops, CLK_IS_CRITICAL, CLK_IGNORE_UNUSED
-separate the driver into two driver: peripheral and pll driver
-delete CLK_IGNORE_UNUSED flag for pwm b/c/d/e/f clock, dsp clock
-delete the change in Kconfig.platforms, address to Kevin alone
-remove the useless comments
-modify the meson pll driver to support A1 PLLs

[1] https://lkml.kernel.org/r/1569411888-98116-1-git-send-email-jian.hu@amlogic.com

Jian Hu (3):
  dt-bindings: clock: meson: add A1 clock controller bindings
  clk: meson: add support for A1 PLL clock ops
  clk: meson: a1: add support for Amlogic A1 clock driver

 .../devicetree/bindings/clock/amlogic,a1-clkc.yaml |  143 ++
 drivers/clk/meson/Kconfig                          |   10 +
 drivers/clk/meson/Makefile                         |    1 +
 drivers/clk/meson/a1-pll.c                         |  345 +++
 drivers/clk/meson/a1-pll.h                         |   56 +
 drivers/clk/meson/a1.c                             | 2264 ++++++++++++++++++++
 drivers/clk/meson/a1.h                             |  120 ++
 drivers/clk/meson/clk-pll.c                        |   66 +-
 drivers/clk/meson/clk-pll.h                        |    1 +
 include/dt-bindings/clock/a1-clkc.h                |   98 +
 include/dt-bindings/clock/a1-pll-clkc.h            |   16 +
 11 files changed, 3114 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
 create mode 100644 drivers/clk/meson/a1-pll.c
 create mode 100644 drivers/clk/meson/a1-pll.h
 create mode 100644 drivers/clk/meson/a1.c
 create mode 100644 drivers/clk/meson/a1.h
 create mode 100644 include/dt-bindings/clock/a1-clkc.h
 create mode 100644 include/dt-bindings/clock/a1-pll-clkc.h

-- 
1.9.1

[PATCH v2 1/3] dt-bindings: clock: meson: add A1 clock controller bindings

[Jian Hu]

Add the documentation to support Amlogic A1 clock driver,
and add A1 clock controller bindings.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 143 +++++++++++++++++++++
 include/dt-bindings/clock/a1-clkc.h                |  98 ++++++++++++++
 include/dt-bindings/clock/a1-pll-clkc.h            |  16 +++
 3 files changed, 257 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
 create mode 100644 include/dt-bindings/clock/a1-clkc.h
 create mode 100644 include/dt-bindings/clock/a1-pll-clkc.h

diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
new file mode 100644
index 0000000..b382eebe
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
@@ -0,0 +1,143 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Amlogic Meson A/C serials Clock Control Unit Device Tree Bindings
+
+maintainers:
+  - Neil Armstrong <narmstrong@baylibre.com>
+  - Jerome Brunet <jbrunet@baylibre.com>
+  - Jian Hu <jian.hu@jian.hu.com>
+
+description: |+
+  The clock controller node should be the child of a syscon node with the
+  required property:
+
+  - compatible:         Should be one of the following:
+                        "amlogic,meson-a-analog-sysctrl", "syscon", "simple-mfd"
+                        "amlogic,meson-a-periphs-sysctrl", "syscon", "simple-mfd"
+
+  Refer to the the bindings described in
+  Documentation/devicetree/bindings/mfd/syscon.txt
+
+properties:
+  "#clock-cells":
+    const: 1
+  compatible:
+    - enum:
+        - amlogic,a1-periphs-clkc
+        - amlogic,a1-pll-clkc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 6
+
+  clock-names:
+    minItems: 2
+    maxItems: 6
+
+required:
+  - "#clock-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+
+if:
+  properties:
+    compatible:
+      enum:
+        - amlogic,a1-periphs-clkc
+
+then:
+  properties:
+    clocks:
+      minItems: 2
+      maxItems: 2
+    items:
+     - description: fixed pll gate clock
+     - description: hifi pll gate clock
+
+    clock-names:
+      minItems: 2
+      maxItems: 2
+      items:
+        - const: xtal_fixpll
+        - const: xtal_hifipll
+
+else:
+  if:
+    properties:
+      compatible:
+        const: amlogic,a1-pll-clkc
+
+  then:
+    properties:
+      clocks:
+        minItems: 6
+        maxItems: 6
+        items:
+         - description: Input fixed pll div2
+         - description: Input fixed pll div3
+         - description: Input fixed pll div5
+         - description: Input fixed pll div7
+         - description: Periph Hifi pll
+         - description: Input Oscillator (usually at 24MHz)
+
+      clock-names:
+        minItems: 6
+        maxItems: 6
+        items:
+         - const: fclk_div2
+         - const: fclk_div3
+         - const: fclk_div5
+         - const: fclk_div7
+         - const: hifi_pll
+         - const: xtal
+
+
+additionalProperties: false
+
+examples:
+  - |
+    analog: system-controller@0 {
+        compatible = "amlogic,meson-a-analog-sysctrl",
+                     "simple-mfd", "syscon";
+        reg = <0 0x7c00 0 0x21c>;
+
+        clkc_pll: pll-clock-controller {
+                compatible = "amlogic,a1-pll-clkc";
+                #clock-cells = <1>;
+                clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
+                         <&clkc_periphs CLKID_XTAL_HIFIPLL>;
+                clock-names = "xtal_fixpll", "xtal_hifipll";
+        };
+    };
+
+  - |
+    periphs: system-controller@1 {
+        compatible = "amlogic,meson-a-periphs-sysctrl",
+                     "simple-mfd", "syscon";
+        reg = <0 0x800 0 0x104>;
+
+        clkc_periphs: periphs-clock-controller {
+                compatible = "amlogic,a1-periphs-clkc";
+                #clock-cells = <1>;
+                clocks = <&clkc_pll CLKID_FCLK_DIV2>,
+                        <&clkc_pll CLKID_FCLK_DIV3>,
+                        <&clkc_pll CLKID_FCLK_DIV5>,
+                        <&clkc_pll CLKID_FCLK_DIV7>,
+                        <&clkc_pll CLKID_HIFI_PLL>,
+                        <&xtal>;
+                clock-names = "fclk_div2", "fclk_div3", "fclk_div5",
+                              "fclk_div7", "hifi_pll", "xtal";
+        };
+    };
diff --git a/include/dt-bindings/clock/a1-clkc.h b/include/dt-bindings/clock/a1-clkc.h
new file mode 100644
index 0000000..1ba0112
--- /dev/null
+++ b/include/dt-bindings/clock/a1-clkc.h
@@ -0,0 +1,98 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#ifndef __A1_CLKC_H
+#define __A1_CLKC_H
+
+#define CLKID_XTAL_FIXPLL			1
+#define CLKID_XTAL_USB_PHY			2
+#define CLKID_XTAL_USB_CTRL			3
+#define CLKID_XTAL_HIFIPLL			4
+#define CLKID_XTAL_SYSPLL			5
+#define CLKID_XTAL_DDS				6
+#define CLKID_SYS_CLK				7
+#define CLKID_CLKTREE				8
+#define CLKID_RESET_CTRL			9
+#define CLKID_ANALOG_CTRL			10
+#define CLKID_PWR_CTRL				11
+#define CLKID_PAD_CTRL				12
+#define CLKID_SYS_CTRL				13
+#define CLKID_TEMP_SENSOR			14
+#define CLKID_AM2AXI_DIV			15
+#define CLKID_SPICC_B				16
+#define CLKID_SPICC_A				17
+#define CLKID_CLK_MSR				18
+#define CLKID_AUDIO				19
+#define CLKID_JTAG_CTRL				20
+#define CLKID_SARADC				21
+#define CLKID_PWM_EF				22
+#define CLKID_PWM_CD				23
+#define CLKID_PWM_AB				24
+#define CLKID_CEC				25
+#define CLKID_I2C_S				26
+#define CLKID_IR_CTRL				27
+#define CLKID_I2C_M_D				28
+#define CLKID_I2C_M_C				29
+#define CLKID_I2C_M_B				30
+#define CLKID_I2C_M_A				31
+#define CLKID_ACODEC				32
+#define CLKID_OTP				33
+#define CLKID_SD_EMMC_A				34
+#define CLKID_USB_PHY				35
+#define CLKID_USB_CTRL				36
+#define CLKID_SYS_DSPB				37
+#define CLKID_SYS_DSPA				38
+#define CLKID_DMA				39
+#define CLKID_IRQ_CTRL				40
+#define CLKID_NIC				41
+#define CLKID_GIC				42
+#define CLKID_UART_C				43
+#define CLKID_UART_B				44
+#define CLKID_UART_A				45
+#define CLKID_SYS_PSRAM				46
+#define CLKID_RSA				47
+#define CLKID_CORESIGHT				48
+#define CLKID_AM2AXI_VAD			49
+#define CLKID_AUDIO_VAD				50
+#define CLKID_AXI_DMC				51
+#define CLKID_AXI_PSRAM				52
+#define CLKID_RAMB				53
+#define CLKID_RAMA				54
+#define CLKID_AXI_SPIFC				55
+#define CLKID_AXI_NIC				56
+#define CLKID_AXI_DMA				57
+#define CLKID_CPU_CTRL				58
+#define CLKID_ROM				59
+#define CLKID_PROC_I2C				60
+#define CLKID_DSPA_SEL				61
+#define CLKID_DSPB_SEL				62
+#define CLKID_DSPA_EN_DSPA			63
+#define CLKID_DSPA_EN_NIC			64
+#define CLKID_DSPB_EN_DSPB			65
+#define CLKID_DSPB_EN_NIC			66
+#define CLKID_RTC_CLK				67
+#define CLKID_CECA_32K				68
+#define CLKID_CECB_32K				69
+#define CLKID_24M				70
+#define CLKID_12M				71
+#define CLKID_FCLK_DIV2_DIVN			72
+#define CLKID_GEN				73
+#define CLKID_SARADC_SEL			74
+#define CLKID_SARADC_CLK			75
+#define CLKID_PWM_A				76
+#define CLKID_PWM_B				77
+#define CLKID_PWM_C				78
+#define CLKID_PWM_D				79
+#define CLKID_PWM_E				80
+#define CLKID_PWM_F				81
+#define CLKID_SPICC				82
+#define CLKID_TS				83
+#define CLKID_SPIFC				84
+#define CLKID_USB_BUS				85
+#define CLKID_SD_EMMC				86
+#define CLKID_PSRAM				87
+#define CLKID_DMC				88
+
+#endif /* __A1_CLKC_H */
diff --git a/include/dt-bindings/clock/a1-pll-clkc.h b/include/dt-bindings/clock/a1-pll-clkc.h
new file mode 100644
index 0000000..58eae23
--- /dev/null
+++ b/include/dt-bindings/clock/a1-pll-clkc.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ */
+
+#ifndef __A1_PLL_CLKC_H
+#define __A1_PLL_CLKC_H
+
+#define CLKID_FIXED_PLL				1
+#define CLKID_FCLK_DIV2				6
+#define CLKID_FCLK_DIV3				7
+#define CLKID_FCLK_DIV5				8
+#define CLKID_FCLK_DIV7				9
+#define CLKID_HIFI_PLL				10
+
+#endif /* __A1_PLL_CLKC_H */
-- 
1.9.1

[PATCH v2 2/3] clk: meson: add support for A1 PLL clock ops

[Jian Hu]

The A1 PLL design is different with previous SoCs. The PLL
internal analog modules Power-on sequence is different
with previous, and thus requires a strict register sequence to
enable the PLL. Unlike the previous series, the maximum frequency
is 6G in G12A, for A1 the maximum is 1536M.

Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
 drivers/clk/meson/clk-pll.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
 drivers/clk/meson/clk-pll.h |  1 +
 2 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index ddb1e56..b440e62 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -349,6 +349,56 @@ static void meson_clk_pll_disable(struct clk_hw *hw)
 	meson_parm_write(clk->map, &pll->en, 0);
 }
 
+/*
+ * The A1 design is different with previous SoCs.The PLL
+ * internal analog modules Power-on sequence is different with
+ * previous, different PLL has the different sequence, and
+ * thus requires a strict register sequence to enable the PLL.
+ * When set a new target frequency, the sequence should keep
+ * the same with the initial sequence. Unlike the previous series,
+ * the maximum frequency is 6G in G12A, for A1 the maximum
+ * is 1536M.
+ */
+static void meson_params_update_with_init_seq(struct clk_regmap *clk,
+				       struct meson_clk_pll_data *pll,
+				       unsigned int m, unsigned int n,
+				       unsigned int frac)
+{
+	struct parm *pm = &pll->m;
+	struct parm *pn = &pll->n;
+	struct parm *pfrac = &pll->frac;
+	const struct reg_sequence *init_regs = pll->init_regs;
+	unsigned int i, val;
+
+	for (i = 0; i < pll->init_count; i++) {
+		if (pn->reg_off == init_regs[i].reg) {
+			/* Clear M N bits and Update M N value */
+			val = init_regs[i].def;
+			val &= CLRPMASK(pn->width, pn->shift);
+			val &= CLRPMASK(pm->width, pm->shift);
+			val |= n << pn->shift;
+			val |= m << pm->shift;
+			regmap_write(clk->map, pn->reg_off, val);
+		} else if (MESON_PARM_APPLICABLE(&pll->frac) &&
+			   (pfrac->reg_off == init_regs[i].reg)) {
+			/* Clear Frac bits and Update Frac value */
+			val = init_regs[i].def;
+			val &= CLRPMASK(pfrac->width, pfrac->shift);
+			val |= frac << pfrac->shift;
+			regmap_write(clk->map, pfrac->reg_off, val);
+		} else {
+			/*
+			 * According to the PLL hardware constraint,
+			 * the left registers should be setted again.
+			 */
+			val = init_regs[i].def;
+			regmap_write(clk->map, init_regs[i].reg, val);
+		}
+		if (init_regs[i].delay_us)
+			udelay(init_regs[i].delay_us);
+	}
+}
+
 static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 				  unsigned long parent_rate)
 {
@@ -366,16 +416,20 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (ret)
 		return ret;
 
+	if (MESON_PARM_APPLICABLE(&pll->frac))
+		frac = __pll_params_with_frac(rate, parent_rate, m, n, pll);
+
 	enabled = meson_parm_read(clk->map, &pll->en);
 	if (enabled)
 		meson_clk_pll_disable(hw);
 
-	meson_parm_write(clk->map, &pll->n, n);
-	meson_parm_write(clk->map, &pll->m, m);
-
-	if (MESON_PARM_APPLICABLE(&pll->frac)) {
-		frac = __pll_params_with_frac(rate, parent_rate, m, n, pll);
-		meson_parm_write(clk->map, &pll->frac, frac);
+	if (pll->strict_sequence)
+		meson_params_update_with_init_seq(clk, pll, m, n, frac);
+	else {
+		meson_parm_write(clk->map, &pll->n, n);
+		meson_parm_write(clk->map, &pll->m, m);
+		if (MESON_PARM_APPLICABLE(&pll->frac))
+			meson_parm_write(clk->map, &pll->frac, frac);
 	}
 
 	/* If the pll is stopped, bail out now */
diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
index 367efd0..d5789cef 100644
--- a/drivers/clk/meson/clk-pll.h
+++ b/drivers/clk/meson/clk-pll.h
@@ -41,6 +41,7 @@ struct meson_clk_pll_data {
 	const struct pll_params_table *table;
 	const struct pll_mult_range *range;
 	u8 flags;
+	bool strict_sequence;
 };
 
 extern const struct clk_ops meson_clk_pll_ro_ops;
-- 
1.9.1

Re: [PATCH v2 1/3] dt-bindings: clock: meson: add A1 clock controller bindings

[Jerome Brunet]

On Fri 18 Oct 2019 at 09:14, Jian Hu <jian.hu@amlogic.com> wrote:

> Add the documentation to support Amlogic A1 clock driver,
> and add A1 clock controller bindings.
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>  .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 143
> +++++++++++++++++++++

Those are 2 different controllers, not variants.
One description (one file) per controller please

>  include/dt-bindings/clock/a1-clkc.h                |  98 ++++++++++++++
>  include/dt-bindings/clock/a1-pll-clkc.h            |  16 +++
>  3 files changed, 257 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>  create mode 100644 include/dt-bindings/clock/a1-clkc.h
>  create mode 100644 include/dt-bindings/clock/a1-pll-clkc.h
>
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> new file mode 100644
> index 0000000..b382eebe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
> @@ -0,0 +1,143 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: Amlogic Meson A/C serials Clock Control Unit Device Tree Bindings
> +
> +maintainers:
> +  - Neil Armstrong <narmstrong@baylibre.com>
> +  - Jerome Brunet <jbrunet@baylibre.com>
> +  - Jian Hu <jian.hu@jian.hu.com>
> +
> +description: |+
> +  The clock controller node should be the child of a syscon node with the
> +  required property:
> +
> +  - compatible:         Should be one of the following:
> +                        "amlogic,meson-a-analog-sysctrl", "syscon", "simple-mfd"
> +                        "amlogic,meson-a-periphs-sysctrl", "syscon", "simple-mfd"
> +
> +  Refer to the the bindings described in
> +  Documentation/devicetree/bindings/mfd/syscon.txt
> +
> +properties:
> +  "#clock-cells":
> +    const: 1
> +  compatible:
> +    - enum:
> +        - amlogic,a1-periphs-clkc
> +        - amlogic,a1-pll-clkc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 6
> +
> +  clock-names:
> +    minItems: 2
> +    maxItems: 6
> +
> +required:
> +  - "#clock-cells"
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +
> +if:
> +  properties:
> +    compatible:
> +      enum:
> +        - amlogic,a1-periphs-clkc
> +
> +then:
> +  properties:
> +    clocks:
> +      minItems: 2
> +      maxItems: 2
> +    items:
> +     - description: fixed pll gate clock
> +     - description: hifi pll gate clock
> +
> +    clock-names:
> +      minItems: 2
> +      maxItems: 2
> +      items:
> +        - const: xtal_fixpll
> +        - const: xtal_hifipll
> +
> +else:
> +  if:
> +    properties:
> +      compatible:
> +        const: amlogic,a1-pll-clkc
> +
> +  then:
> +    properties:
> +      clocks:
> +        minItems: 6
> +        maxItems: 6
> +        items:
> +         - description: Input fixed pll div2
> +         - description: Input fixed pll div3
> +         - description: Input fixed pll div5
> +         - description: Input fixed pll div7
> +         - description: Periph Hifi pll
> +         - description: Input Oscillator (usually at 24MHz)
> +
> +      clock-names:
> +        minItems: 6
> +        maxItems: 6
> +        items:
> +         - const: fclk_div2
> +         - const: fclk_div3
> +         - const: fclk_div5
> +         - const: fclk_div7
> +         - const: hifi_pll
> +         - const: xtal
> +
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    analog: system-controller@0 {
> +        compatible = "amlogic,meson-a-analog-sysctrl",
> +                     "simple-mfd", "syscon";
> +        reg = <0 0x7c00 0 0x21c>;
> +
> +        clkc_pll: pll-clock-controller {
> +                compatible = "amlogic,a1-pll-clkc";
> +                #clock-cells = <1>;
> +                clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
> +                         <&clkc_periphs CLKID_XTAL_HIFIPLL>;
> +                clock-names = "xtal_fixpll", "xtal_hifipll";
> +        };
> +    };
> +
> +  - |
> +    periphs: system-controller@1 {
> +        compatible = "amlogic,meson-a-periphs-sysctrl",
> +                     "simple-mfd", "syscon";
> +        reg = <0 0x800 0 0x104>;
> +
> +        clkc_periphs: periphs-clock-controller {
> +                compatible = "amlogic,a1-periphs-clkc";
> +                #clock-cells = <1>;
> +                clocks = <&clkc_pll CLKID_FCLK_DIV2>,
> +                        <&clkc_pll CLKID_FCLK_DIV3>,
> +                        <&clkc_pll CLKID_FCLK_DIV5>,
> +                        <&clkc_pll CLKID_FCLK_DIV7>,
> +                        <&clkc_pll CLKID_HIFI_PLL>,
> +                        <&xtal>;
> +                clock-names = "fclk_div2", "fclk_div3", "fclk_div5",
> +                              "fclk_div7", "hifi_pll", "xtal";
> +        };
> +    };
> diff --git a/include/dt-bindings/clock/a1-clkc.h b/include/dt-bindings/clock/a1-clkc.h
> new file mode 100644
> index 0000000..1ba0112
> --- /dev/null
> +++ b/include/dt-bindings/clock/a1-clkc.h
> @@ -0,0 +1,98 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +#ifndef __A1_CLKC_H
> +#define __A1_CLKC_H
> +
> +#define CLKID_XTAL_FIXPLL			1
> +#define CLKID_XTAL_USB_PHY			2
> +#define CLKID_XTAL_USB_CTRL			3
> +#define CLKID_XTAL_HIFIPLL			4
> +#define CLKID_XTAL_SYSPLL			5
> +#define CLKID_XTAL_DDS				6
> +#define CLKID_SYS_CLK				7
> +#define CLKID_CLKTREE				8
> +#define CLKID_RESET_CTRL			9
> +#define CLKID_ANALOG_CTRL			10
> +#define CLKID_PWR_CTRL				11
> +#define CLKID_PAD_CTRL				12
> +#define CLKID_SYS_CTRL				13
> +#define CLKID_TEMP_SENSOR			14
> +#define CLKID_AM2AXI_DIV			15
> +#define CLKID_SPICC_B				16
> +#define CLKID_SPICC_A				17
> +#define CLKID_CLK_MSR				18
> +#define CLKID_AUDIO				19
> +#define CLKID_JTAG_CTRL				20
> +#define CLKID_SARADC				21
> +#define CLKID_PWM_EF				22
> +#define CLKID_PWM_CD				23
> +#define CLKID_PWM_AB				24
> +#define CLKID_CEC				25
> +#define CLKID_I2C_S				26
> +#define CLKID_IR_CTRL				27
> +#define CLKID_I2C_M_D				28
> +#define CLKID_I2C_M_C				29
> +#define CLKID_I2C_M_B				30
> +#define CLKID_I2C_M_A				31
> +#define CLKID_ACODEC				32
> +#define CLKID_OTP				33
> +#define CLKID_SD_EMMC_A				34
> +#define CLKID_USB_PHY				35
> +#define CLKID_USB_CTRL				36
> +#define CLKID_SYS_DSPB				37
> +#define CLKID_SYS_DSPA				38
> +#define CLKID_DMA				39
> +#define CLKID_IRQ_CTRL				40
> +#define CLKID_NIC				41
> +#define CLKID_GIC				42
> +#define CLKID_UART_C				43
> +#define CLKID_UART_B				44
> +#define CLKID_UART_A				45
> +#define CLKID_SYS_PSRAM				46
> +#define CLKID_RSA				47
> +#define CLKID_CORESIGHT				48
> +#define CLKID_AM2AXI_VAD			49
> +#define CLKID_AUDIO_VAD				50
> +#define CLKID_AXI_DMC				51
> +#define CLKID_AXI_PSRAM				52
> +#define CLKID_RAMB				53
> +#define CLKID_RAMA				54
> +#define CLKID_AXI_SPIFC				55
> +#define CLKID_AXI_NIC				56
> +#define CLKID_AXI_DMA				57
> +#define CLKID_CPU_CTRL				58
> +#define CLKID_ROM				59
> +#define CLKID_PROC_I2C				60
> +#define CLKID_DSPA_SEL				61
> +#define CLKID_DSPB_SEL				62
> +#define CLKID_DSPA_EN_DSPA			63
> +#define CLKID_DSPA_EN_NIC			64
> +#define CLKID_DSPB_EN_DSPB			65
> +#define CLKID_DSPB_EN_NIC			66
> +#define CLKID_RTC_CLK				67
> +#define CLKID_CECA_32K				68
> +#define CLKID_CECB_32K				69
> +#define CLKID_24M				70
> +#define CLKID_12M				71
> +#define CLKID_FCLK_DIV2_DIVN			72
> +#define CLKID_GEN				73
> +#define CLKID_SARADC_SEL			74
> +#define CLKID_SARADC_CLK			75
> +#define CLKID_PWM_A				76
> +#define CLKID_PWM_B				77
> +#define CLKID_PWM_C				78
> +#define CLKID_PWM_D				79
> +#define CLKID_PWM_E				80
> +#define CLKID_PWM_F				81
> +#define CLKID_SPICC				82
> +#define CLKID_TS				83
> +#define CLKID_SPIFC				84
> +#define CLKID_USB_BUS				85
> +#define CLKID_SD_EMMC				86
> +#define CLKID_PSRAM				87
> +#define CLKID_DMC				88
> +
> +#endif /* __A1_CLKC_H */
> diff --git a/include/dt-bindings/clock/a1-pll-clkc.h b/include/dt-bindings/clock/a1-pll-clkc.h
> new file mode 100644
> index 0000000..58eae23
> --- /dev/null
> +++ b/include/dt-bindings/clock/a1-pll-clkc.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +#ifndef __A1_PLL_CLKC_H
> +#define __A1_PLL_CLKC_H
> +
> +#define CLKID_FIXED_PLL				1
> +#define CLKID_FCLK_DIV2				6
> +#define CLKID_FCLK_DIV3				7
> +#define CLKID_FCLK_DIV5				8
> +#define CLKID_FCLK_DIV7				9
> +#define CLKID_HIFI_PLL				10
> +
> +#endif /* __A1_PLL_CLKC_H */

Re: [PATCH v2 2/3] clk: meson: add support for A1 PLL clock ops

[Jerome Brunet]

On Fri 18 Oct 2019 at 09:14, Jian Hu <jian.hu@amlogic.com> wrote:

> The A1 PLL design is different with previous SoCs. The PLL
> internal analog modules Power-on sequence is different
> with previous, and thus requires a strict register sequence to
> enable the PLL. Unlike the previous series, the maximum frequency
> is 6G in G12A, for A1 the maximum is 1536M.
>
> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
> ---
>  drivers/clk/meson/clk-pll.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
>  drivers/clk/meson/clk-pll.h |  1 +
>  2 files changed, 61 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> index ddb1e56..b440e62 100644
> --- a/drivers/clk/meson/clk-pll.c
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -349,6 +349,56 @@ static void meson_clk_pll_disable(struct clk_hw *hw)
>  	meson_parm_write(clk->map, &pll->en, 0);
>  }
>  
> +/*
> + * The A1 design is different with previous SoCs.The PLL
> + * internal analog modules Power-on sequence is different with
> + * previous, different PLL has the different sequence, and
> + * thus requires a strict register sequence to enable the PLL.
> + * When set a new target frequency, the sequence should keep
> + * the same with the initial sequence. Unlike the previous series,
> + * the maximum frequency is 6G in G12A, for A1 the maximum
> + * is 1536M.

The comment about the max frequency belongs in your a1 driver, not in
the PLL driver

> + */
> +static void meson_params_update_with_init_seq(struct clk_regmap *clk,
> +				       struct meson_clk_pll_data *pll,
> +				       unsigned int m, unsigned int n,
> +				       unsigned int frac)
> +{
> +	struct parm *pm = &pll->m;
> +	struct parm *pn = &pll->n;
> +	struct parm *pfrac = &pll->frac;
> +	const struct reg_sequence *init_regs = pll->init_regs;
> +	unsigned int i, val;
> +
> +	for (i = 0; i < pll->init_count; i++) {
> +		if (pn->reg_off == init_regs[i].reg) {
> +			/* Clear M N bits and Update M N value */
> +			val = init_regs[i].def;
> +			val &= CLRPMASK(pn->width, pn->shift);
> +			val &= CLRPMASK(pm->width, pm->shift);
> +			val |= n << pn->shift;
> +			val |= m << pm->shift;
> +			regmap_write(clk->map, pn->reg_off, val);
> +		} else if (MESON_PARM_APPLICABLE(&pll->frac) &&
> +			   (pfrac->reg_off == init_regs[i].reg)) {
> +			/* Clear Frac bits and Update Frac value */
> +			val = init_regs[i].def;
> +			val &= CLRPMASK(pfrac->width, pfrac->shift);
> +			val |= frac << pfrac->shift;
> +			regmap_write(clk->map, pfrac->reg_off, val);
> +		} else {
> +			/*
> +			 * According to the PLL hardware constraint,
> +			 * the left registers should be setted again.
> +			 */
> +			val = init_regs[i].def;
> +			regmap_write(clk->map, init_regs[i].reg, val);
> +		}
> +		if (init_regs[i].delay_us)
> +			udelay(init_regs[i].delay_us);
> +	}

So:

1) All the code above this there make the PLL lock, IOW enable the
PLL. It does not belong in the set_rate() callback but in enable() or
prepare() maybe.

2) All the above is works but it is a bit over complicated for what it
does. From the a1_hifi_init_regs I see, all you really need to do is
  * toggle BIT(6) in CTRL2
  * toggle BIT(28) in CTRL0 (enable PARM)
  * toggle BIT(26) in CTRL0

You could use PARM 'rst' for one them and introduce another parm for the
other one. You would not need to repoke the whole sequence this way.

> +}
> +
>  static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>  				  unsigned long parent_rate)
>  {
> @@ -366,16 +416,20 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>  	if (ret)
>  		return ret;
>  
> +	if (MESON_PARM_APPLICABLE(&pll->frac))
> +		frac = __pll_params_with_frac(rate, parent_rate, m, n, pll);
> +
>  	enabled = meson_parm_read(clk->map, &pll->en);
>  	if (enabled)
>  		meson_clk_pll_disable(hw);
>  
> -	meson_parm_write(clk->map, &pll->n, n);
> -	meson_parm_write(clk->map, &pll->m, m);
> -
> -	if (MESON_PARM_APPLICABLE(&pll->frac)) {
> -		frac = __pll_params_with_frac(rate, parent_rate, m, n, pll);
> -		meson_parm_write(clk->map, &pll->frac, frac);
> +	if (pll->strict_sequence)
> +		meson_params_update_with_init_seq(clk, pll, m, n, frac);
> +	else {
> +		meson_parm_write(clk->map, &pll->n, n);
> +		meson_parm_write(clk->map, &pll->m, m);
> +		if (MESON_PARM_APPLICABLE(&pll->frac))
> +			meson_parm_write(clk->map, &pll->frac, frac);
>  	}
>  
>  	/* If the pll is stopped, bail out now */
> diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
> index 367efd0..d5789cef 100644
> --- a/drivers/clk/meson/clk-pll.h
> +++ b/drivers/clk/meson/clk-pll.h
> @@ -41,6 +41,7 @@ struct meson_clk_pll_data {
>  	const struct pll_params_table *table;
>  	const struct pll_mult_range *range;
>  	u8 flags;
> +	bool strict_sequence;

Don't introduce parameter for this We have ops to tune the behavior of
the clock driver. Properly refactor the code if some of it is common.

>  };
>  
>  extern const struct clk_ops meson_clk_pll_ro_ops;

Re: [PATCH v2 1/3] dt-bindings: clock: meson: add A1 clock controller bindings

[Jian Hu]

Hi, Jerome

Thanks for your review.

On 2019/10/21 18:43, Jerome Brunet wrote:
> 
> On Fri 18 Oct 2019 at 09:14, Jian Hu <jian.hu@amlogic.com> wrote:
> 
>> Add the documentation to support Amlogic A1 clock driver,
>> and add A1 clock controller bindings.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   .../devicetree/bindings/clock/amlogic,a1-clkc.yaml | 143
>> +++++++++++++++++++++
> 
> Those are 2 different controllers, not variants.
> One description (one file) per controller please
OK, I will describe for periphs and PLLs controller separately.
> 
>>   include/dt-bindings/clock/a1-clkc.h                |  98 ++++++++++++++
>>   include/dt-bindings/clock/a1-pll-clkc.h            |  16 +++
>>   3 files changed, 257 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>>   create mode 100644 include/dt-bindings/clock/a1-clkc.h
>>   create mode 100644 include/dt-bindings/clock/a1-pll-clkc.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>> new file mode 100644
>> index 0000000..b382eebe
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml
>> @@ -0,0 +1,143 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/clock/amlogic,a1-clkc.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: Amlogic Meson A/C serials Clock Control Unit Device Tree Bindings
>> +
>> +maintainers:
>> +  - Neil Armstrong <narmstrong@baylibre.com>
>> +  - Jerome Brunet <jbrunet@baylibre.com>
>> +  - Jian Hu <jian.hu@jian.hu.com>
>> +
>> +description: |+
>> +  The clock controller node should be the child of a syscon node with the
>> +  required property:
>> +
>> +  - compatible:         Should be one of the following:
>> +                        "amlogic,meson-a-analog-sysctrl", "syscon", "simple-mfd"
>> +                        "amlogic,meson-a-periphs-sysctrl", "syscon", "simple-mfd"
>> +
>> +  Refer to the the bindings described in
>> +  Documentation/devicetree/bindings/mfd/syscon.txt
>> +
>> +properties:
>> +  "#clock-cells":
>> +    const: 1
>> +  compatible:
>> +    - enum:
>> +        - amlogic,a1-periphs-clkc
>> +        - amlogic,a1-pll-clkc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    minItems: 2
>> +    maxItems: 6
>> +
>> +  clock-names:
>> +    minItems: 2
>> +    maxItems: 6
>> +
>> +required:
>> +  - "#clock-cells"
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +
>> +if:
>> +  properties:
>> +    compatible:
>> +      enum:
>> +        - amlogic,a1-periphs-clkc
>> +
>> +then:
>> +  properties:
>> +    clocks:
>> +      minItems: 2
>> +      maxItems: 2
>> +    items:
>> +     - description: fixed pll gate clock
>> +     - description: hifi pll gate clock
>> +
>> +    clock-names:
>> +      minItems: 2
>> +      maxItems: 2
>> +      items:
>> +        - const: xtal_fixpll
>> +        - const: xtal_hifipll
>> +
>> +else:
>> +  if:
>> +    properties:
>> +      compatible:
>> +        const: amlogic,a1-pll-clkc
>> +
>> +  then:
>> +    properties:
>> +      clocks:
>> +        minItems: 6
>> +        maxItems: 6
>> +        items:
>> +         - description: Input fixed pll div2
>> +         - description: Input fixed pll div3
>> +         - description: Input fixed pll div5
>> +         - description: Input fixed pll div7
>> +         - description: Periph Hifi pll
>> +         - description: Input Oscillator (usually at 24MHz)
>> +
>> +      clock-names:
>> +        minItems: 6
>> +        maxItems: 6
>> +        items:
>> +         - const: fclk_div2
>> +         - const: fclk_div3
>> +         - const: fclk_div5
>> +         - const: fclk_div7
>> +         - const: hifi_pll
>> +         - const: xtal
>> +
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    analog: system-controller@0 {
>> +        compatible = "amlogic,meson-a-analog-sysctrl",
>> +                     "simple-mfd", "syscon";
>> +        reg = <0 0x7c00 0 0x21c>;
>> +
>> +        clkc_pll: pll-clock-controller {
>> +                compatible = "amlogic,a1-pll-clkc";
>> +                #clock-cells = <1>;
>> +                clocks = <&clkc_periphs CLKID_XTAL_FIXPLL>,
>> +                         <&clkc_periphs CLKID_XTAL_HIFIPLL>;
>> +                clock-names = "xtal_fixpll", "xtal_hifipll";
>> +        };
>> +    };
>> +
>> +  - |
>> +    periphs: system-controller@1 {
>> +        compatible = "amlogic,meson-a-periphs-sysctrl",
>> +                     "simple-mfd", "syscon";
>> +        reg = <0 0x800 0 0x104>;
>> +
>> +        clkc_periphs: periphs-clock-controller {
>> +                compatible = "amlogic,a1-periphs-clkc";
>> +                #clock-cells = <1>;
>> +                clocks = <&clkc_pll CLKID_FCLK_DIV2>,
>> +                        <&clkc_pll CLKID_FCLK_DIV3>,
>> +                        <&clkc_pll CLKID_FCLK_DIV5>,
>> +                        <&clkc_pll CLKID_FCLK_DIV7>,
>> +                        <&clkc_pll CLKID_HIFI_PLL>,
>> +                        <&xtal>;
>> +                clock-names = "fclk_div2", "fclk_div3", "fclk_div5",
>> +                              "fclk_div7", "hifi_pll", "xtal";
>> +        };
>> +    };
>> diff --git a/include/dt-bindings/clock/a1-clkc.h b/include/dt-bindings/clock/a1-clkc.h
>> new file mode 100644
>> index 0000000..1ba0112
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/a1-clkc.h
>> @@ -0,0 +1,98 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef __A1_CLKC_H
>> +#define __A1_CLKC_H
>> +
>> +#define CLKID_XTAL_FIXPLL			1
>> +#define CLKID_XTAL_USB_PHY			2
>> +#define CLKID_XTAL_USB_CTRL			3
>> +#define CLKID_XTAL_HIFIPLL			4
>> +#define CLKID_XTAL_SYSPLL			5
>> +#define CLKID_XTAL_DDS				6
>> +#define CLKID_SYS_CLK				7
>> +#define CLKID_CLKTREE				8
>> +#define CLKID_RESET_CTRL			9
>> +#define CLKID_ANALOG_CTRL			10
>> +#define CLKID_PWR_CTRL				11
>> +#define CLKID_PAD_CTRL				12
>> +#define CLKID_SYS_CTRL				13
>> +#define CLKID_TEMP_SENSOR			14
>> +#define CLKID_AM2AXI_DIV			15
>> +#define CLKID_SPICC_B				16
>> +#define CLKID_SPICC_A				17
>> +#define CLKID_CLK_MSR				18
>> +#define CLKID_AUDIO				19
>> +#define CLKID_JTAG_CTRL				20
>> +#define CLKID_SARADC				21
>> +#define CLKID_PWM_EF				22
>> +#define CLKID_PWM_CD				23
>> +#define CLKID_PWM_AB				24
>> +#define CLKID_CEC				25
>> +#define CLKID_I2C_S				26
>> +#define CLKID_IR_CTRL				27
>> +#define CLKID_I2C_M_D				28
>> +#define CLKID_I2C_M_C				29
>> +#define CLKID_I2C_M_B				30
>> +#define CLKID_I2C_M_A				31
>> +#define CLKID_ACODEC				32
>> +#define CLKID_OTP				33
>> +#define CLKID_SD_EMMC_A				34
>> +#define CLKID_USB_PHY				35
>> +#define CLKID_USB_CTRL				36
>> +#define CLKID_SYS_DSPB				37
>> +#define CLKID_SYS_DSPA				38
>> +#define CLKID_DMA				39
>> +#define CLKID_IRQ_CTRL				40
>> +#define CLKID_NIC				41
>> +#define CLKID_GIC				42
>> +#define CLKID_UART_C				43
>> +#define CLKID_UART_B				44
>> +#define CLKID_UART_A				45
>> +#define CLKID_SYS_PSRAM				46
>> +#define CLKID_RSA				47
>> +#define CLKID_CORESIGHT				48
>> +#define CLKID_AM2AXI_VAD			49
>> +#define CLKID_AUDIO_VAD				50
>> +#define CLKID_AXI_DMC				51
>> +#define CLKID_AXI_PSRAM				52
>> +#define CLKID_RAMB				53
>> +#define CLKID_RAMA				54
>> +#define CLKID_AXI_SPIFC				55
>> +#define CLKID_AXI_NIC				56
>> +#define CLKID_AXI_DMA				57
>> +#define CLKID_CPU_CTRL				58
>> +#define CLKID_ROM				59
>> +#define CLKID_PROC_I2C				60
>> +#define CLKID_DSPA_SEL				61
>> +#define CLKID_DSPB_SEL				62
>> +#define CLKID_DSPA_EN_DSPA			63
>> +#define CLKID_DSPA_EN_NIC			64
>> +#define CLKID_DSPB_EN_DSPB			65
>> +#define CLKID_DSPB_EN_NIC			66
>> +#define CLKID_RTC_CLK				67
>> +#define CLKID_CECA_32K				68
>> +#define CLKID_CECB_32K				69
>> +#define CLKID_24M				70
>> +#define CLKID_12M				71
>> +#define CLKID_FCLK_DIV2_DIVN			72
>> +#define CLKID_GEN				73
>> +#define CLKID_SARADC_SEL			74
>> +#define CLKID_SARADC_CLK			75
>> +#define CLKID_PWM_A				76
>> +#define CLKID_PWM_B				77
>> +#define CLKID_PWM_C				78
>> +#define CLKID_PWM_D				79
>> +#define CLKID_PWM_E				80
>> +#define CLKID_PWM_F				81
>> +#define CLKID_SPICC				82
>> +#define CLKID_TS				83
>> +#define CLKID_SPIFC				84
>> +#define CLKID_USB_BUS				85
>> +#define CLKID_SD_EMMC				86
>> +#define CLKID_PSRAM				87
>> +#define CLKID_DMC				88
>> +
>> +#endif /* __A1_CLKC_H */
>> diff --git a/include/dt-bindings/clock/a1-pll-clkc.h b/include/dt-bindings/clock/a1-pll-clkc.h
>> new file mode 100644
>> index 0000000..58eae23
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/a1-pll-clkc.h
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#ifndef __A1_PLL_CLKC_H
>> +#define __A1_PLL_CLKC_H
>> +
>> +#define CLKID_FIXED_PLL				1
>> +#define CLKID_FCLK_DIV2				6
>> +#define CLKID_FCLK_DIV3				7
>> +#define CLKID_FCLK_DIV5				8
>> +#define CLKID_FCLK_DIV7				9
>> +#define CLKID_HIFI_PLL				10
>> +
>> +#endif /* __A1_PLL_CLKC_H */
> 
> .
> 

Re: [PATCH v2 2/3] clk: meson: add support for A1 PLL clock ops

[Jian Hu]

Hi, Jerome

On 2019/10/21 19:31, Jerome Brunet wrote:
> 
> On Fri 18 Oct 2019 at 09:14, Jian Hu <jian.hu@amlogic.com> wrote:
> 
>> The A1 PLL design is different with previous SoCs. The PLL
>> internal analog modules Power-on sequence is different
>> with previous, and thus requires a strict register sequence to
>> enable the PLL. Unlike the previous series, the maximum frequency
>> is 6G in G12A, for A1 the maximum is 1536M.
>>
>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>> ---
>>   drivers/clk/meson/clk-pll.c | 66 ++++++++++++++++++++++++++++++++++++++++-----
>>   drivers/clk/meson/clk-pll.h |  1 +
>>   2 files changed, 61 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>> index ddb1e56..b440e62 100644
>> --- a/drivers/clk/meson/clk-pll.c
>> +++ b/drivers/clk/meson/clk-pll.c
>> @@ -349,6 +349,56 @@ static void meson_clk_pll_disable(struct clk_hw *hw)
>>   	meson_parm_write(clk->map, &pll->en, 0);
>>   }
>>   
>> +/*
>> + * The A1 design is different with previous SoCs.The PLL
>> + * internal analog modules Power-on sequence is different with
>> + * previous, different PLL has the different sequence, and
>> + * thus requires a strict register sequence to enable the PLL.
>> + * When set a new target frequency, the sequence should keep
>> + * the same with the initial sequence. Unlike the previous series,
>> + * the maximum frequency is 6G in G12A, for A1 the maximum
>> + * is 1536M.
> 
> The comment about the max frequency belongs in your a1 driver, not in
> the PLL driver
> 
OK, I will remove the max frequency comments
>> + */
>> +static void meson_params_update_with_init_seq(struct clk_regmap *clk,
>> +				       struct meson_clk_pll_data *pll,
>> +				       unsigned int m, unsigned int n,
>> +				       unsigned int frac)
>> +{
>> +	struct parm *pm = &pll->m;
>> +	struct parm *pn = &pll->n;
>> +	struct parm *pfrac = &pll->frac;
>> +	const struct reg_sequence *init_regs = pll->init_regs;
>> +	unsigned int i, val;
>> +
>> +	for (i = 0; i < pll->init_count; i++) {
>> +		if (pn->reg_off == init_regs[i].reg) {
>> +			/* Clear M N bits and Update M N value */
>> +			val = init_regs[i].def;
>> +			val &= CLRPMASK(pn->width, pn->shift);
>> +			val &= CLRPMASK(pm->width, pm->shift);
>> +			val |= n << pn->shift;
>> +			val |= m << pm->shift;
>> +			regmap_write(clk->map, pn->reg_off, val);
>> +		} else if (MESON_PARM_APPLICABLE(&pll->frac) &&
>> +			   (pfrac->reg_off == init_regs[i].reg)) {
>> +			/* Clear Frac bits and Update Frac value */
>> +			val = init_regs[i].def;
>> +			val &= CLRPMASK(pfrac->width, pfrac->shift);
>> +			val |= frac << pfrac->shift;
>> +			regmap_write(clk->map, pfrac->reg_off, val);
>> +		} else {
>> +			/*
>> +			 * According to the PLL hardware constraint,
>> +			 * the left registers should be setted again.
>> +			 */
>> +			val = init_regs[i].def;
>> +			regmap_write(clk->map, init_regs[i].reg, val);
>> +		}
>> +		if (init_regs[i].delay_us)
>> +			udelay(init_regs[i].delay_us);
>> +	}
> 
> So:
> 
> 1) All the code above this there make the PLL lock, IOW enable the
> PLL. It does not belong in the set_rate() callback but in enable() or
> prepare() maybe.
> 
> 2) All the above is works but it is a bit over complicated for what it
> does. From the a1_hifi_init_regs I see, all you really need to do is
>    * toggle BIT(6) in CTRL2
>    * toggle BIT(28) in CTRL0 (enable PARM)
>    * toggle BIT(26) in CTRL0
> 
> You could use PARM 'rst' for one them and introduce another parm for the
> other one. You would not need to repoke the whole sequence this way.
> 
OK, I have realized as you suggested. I will send it in the V3 patch.
>> +}
>> +
>>   static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>>   				  unsigned long parent_rate)
>>   {
>> @@ -366,16 +416,20 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>>   	if (ret)
>>   		return ret;
>>   
>> +	if (MESON_PARM_APPLICABLE(&pll->frac))
>> +		frac = __pll_params_with_frac(rate, parent_rate, m, n, pll);
>> +
>>   	enabled = meson_parm_read(clk->map, &pll->en);
>>   	if (enabled)
>>   		meson_clk_pll_disable(hw);
>>   
>> -	meson_parm_write(clk->map, &pll->n, n);
>> -	meson_parm_write(clk->map, &pll->m, m);
>> -
>> -	if (MESON_PARM_APPLICABLE(&pll->frac)) {
>> -		frac = __pll_params_with_frac(rate, parent_rate, m, n, pll);
>> -		meson_parm_write(clk->map, &pll->frac, frac);
>> +	if (pll->strict_sequence)
>> +		meson_params_update_with_init_seq(clk, pll, m, n, frac);
>> +	else {
>> +		meson_parm_write(clk->map, &pll->n, n);
>> +		meson_parm_write(clk->map, &pll->m, m);
>> +		if (MESON_PARM_APPLICABLE(&pll->frac))
>> +			meson_parm_write(clk->map, &pll->frac, frac);
>>   	}
>>   
>>   	/* If the pll is stopped, bail out now */
>> diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
>> index 367efd0..d5789cef 100644
>> --- a/drivers/clk/meson/clk-pll.h
>> +++ b/drivers/clk/meson/clk-pll.h
>> @@ -41,6 +41,7 @@ struct meson_clk_pll_data {
>>   	const struct pll_params_table *table;
>>   	const struct pll_mult_range *range;
>>   	u8 flags;
>> +	bool strict_sequence;
> 
> Don't introduce parameter for this We have ops to tune the behavior of
> the clock driver. Properly refactor the code if some of it is common.
> 
remove the strict_sequence.
>>   };
>>   
>>   extern const struct clk_ops meson_clk_pll_ro_ops;
> 
> .
>