Re: [PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

Re: [PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

[Sean Anderson]

On 6/11/21 11:10 AM, Zhengxun wrote:
 > Add the devicetree binding for the xilinx clocking wizard.
 >
 > Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
 > ---
 >   .../clock/xlnx,clocking-wizard.txt            | 43 +++++++++++++++++++
 >   1 file changed, 43 insertions(+)
 >   create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt

Should this be .yml? In general, I believe we only need to document
bindings in U-Boot if they differ from Linux. In Linux, I see that
Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.txt exists.
Will you be upstreaming this conversion to yaml?

--Sean

 >
 > diff --git a/doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt b/doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
 > new file mode 100644
 > index 0000000000..e0e73ec723
 > --- /dev/null
 > +++ b/doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
 > @@ -0,0 +1,43 @@
 > +Xilinx clocking wizard bindings
 > +
 > +description:
 > +  The clocking wizard is a soft ip clocking block of Xilinx versal. It
 > +  reads required input clock frequencies from the devicetree and acts as
 > +  clock output.
 > +
 > +properties:
 > +  compatible:
 > +    const: xlnx,clocking-wizard
 > +
 > +  reg:
 > +    maxItems: 1
 > +
 > +  "#clock-cells":
 > +    const: 1
 > +
 > +  clocks:
 > +    items:
 > +      - description: clock input
 > +
 > +  clock-names:
 > +    items:
 > +      - const: clk_in1
 > +
 > +required:
 > +  - compatible
 > +  - reg
 > +  - "#clock-cells"
 > +  - clocks
 > +  - clock-names
 > +
 > +additionalProperties: false
 > +
 > +examples:
 > +  - |
 > +    clkwizard@43c20000  {
 > +        compatible = "xlnx,clocking-wizard";
 > +        reg = <0x43c20000 0x10000>;
 > +        #clock-cells = <1>;
 > +        clock-names = "clk_in1";
 > +        clocks = <&clkc 18>;
 > +    };
 >

Re: [PATCH v6 1/2] clk: zynq: Add clock wizard driver

[Sean Anderson]

On 6/11/21 11:10 AM, Zhengxun wrote:
> The Clocking Wizard IP supports clock circuits customized
> to your clocking requirements. The wizard support for
> dynamically reconfiguring the clocking primitives for
> Multiply, Divide, Phase Shift/Offset, or Duty Cycle.
> 
> Limited by U-Boot clk uclass without set_phase API, this
> patch only provides set_rate to modify the frequency.
> 
> Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
> ---
>   drivers/clk/Kconfig                 |  11 ++
>   drivers/clk/Makefile                |   1 +
>   drivers/clk/clk-xlnx-clock-wizard.c | 186 ++++++++++++++++++++++++++++
>   3 files changed, 198 insertions(+)
>   create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 40a5a5dd88..a0ac661f28 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -128,6 +128,17 @@ config CLK_ZYNQ
>   	  This clock driver adds support for clock related settings for
>   	  Zynq platform.
>   
> +config CLK_XLNX_CLKWZRD
> +	bool "Xilinx Clocking Wizard"
> +	depends on CLK
> +	help
> +	  Support for the Xilinx Clocking Wizard IP core clock generator.
> +	  The wizard support for dynamically reconfiguring the clocking
> +	  primitives for Multiply, Divide, Phase Shift/Offset, or Duty
> +	  Cycle. Limited by U-Boot clk uclass without set_phase API and
> +	  set_duty_cycle API, this driver only supports set_rate to modify
> +	  the frequency.
> +
>   config CLK_ZYNQMP
>   	bool "Enable clock driver support for ZynqMP"
>   	depends on ARCH_ZYNQMP
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 645709b855..4fcc33953a 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
>   obj-$(CONFIG_CLK_VEXPRESS_OSC) += clk_vexpress_osc.o
>   obj-$(CONFIG_CLK_ZYNQ) += clk_zynq.o
>   obj-$(CONFIG_CLK_ZYNQMP) += clk_zynqmp.o
> +obj-$(CONFIG_CLK_XLNX_CLKWZRD) += clk-xlnx-clock-wizard.o
>   obj-$(CONFIG_ICS8N3QV01) += ics8n3qv01.o
>   obj-$(CONFIG_MACH_PIC32) += clk_pic32.o
>   obj-$(CONFIG_SANDBOX) += clk_sandbox.o
> diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> new file mode 100644
> index 0000000000..70ee3af107
> --- /dev/null
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx 'Clocking Wizard' driver
> + *
> + * Copyright (c) 2021 Macronix Inc.
> + *
> + * Author: Zhengxun Li <zhengxunli@mxic.com.tw>
> + */
> +
> +#include <common.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <div64.h>
> +#include <dm/device_compat.h>
> +#include <linux/iopoll.h>
> +
> +#include <linux/bitfield.h>
> +
> +#define SRR			0x0
> +
> +#define SR			0x4
> +#define SR_LOCKED		BIT(0)
> +
> +#define CCR(x)			(0x200 + ((x) * 4))
> +
> +#define FBOUT_CFG		CCR(0)
> +#define FBOUT_DIV(x)		(x)
> +#define FBOUT_DIV_MASK		GENMASK(7, 0)
> +#define FBOUT_GET_DIV(x)	FIELD_GET(FBOUT_DIV_MASK, x)
> +#define FBOUT_MUL(x)		((x) << 8)
> +#define FBOUT_MUL_MASK		GENMASK(15, 8)
> +#define FBOUT_GET_MUL(x)	FIELD_GET(FBOUT_MUL_MASK, x)
> +#define FBOUT_FRAC(x)		((x) << 16)
> +#define FBOUT_FRAC_MASK		GENMASK(25, 16)
> +#define FBOUT_GET_FRAC(x)	FIELD_GET(FBOUT_FRAC_MASK, x)
> +#define FBOUT_FRAC_EN		BIT(26)
> +
> +#define FBOUT_PHASE		CCR(1)
> +
> +#define OUT_CFG(x)		CCR(2 + ((x) * 3))
> +#define OUT_DIV(x)		(x)
> +#define OUT_DIV_MASK		GENMASK(7, 0)
> +#define OUT_GET_DIV(x)		FIELD_GET(OUT_DIV_MASK, x)
> +#define OUT_FRAC(x)		((x) << 8)
> +#define OUT_GET_MASK		GENMASK(17, 8)
> +#define OUT_GET_FRAC(x)		FIELD_GET(OUT_GET_MASK, x)
> +#define OUT_FRAC_EN		BIT(18)
> +
> +#define OUT_PHASE(x)		CCR(3 + ((x) * 3))
> +#define OUT_DUTY(x)		CCR(4 + ((x) * 3))
> +
> +#define CTRL			CCR(23)
> +#define CTRL_SEN		BIT(2)
> +#define CTRL_SADDR		BIT(1)
> +#define CTRL_LOAD		BIT(0)
> +
> +/**
> + * struct clkwzrd - Clock wizard private data structure
> + *
> + * @base:		memory base
> + * @vco_clk:		voltage-controlled oscillator frequency
> + *
> + */
> +struct clkwzd {
> +	void *base;
> +	u64 vco_clk;
> +};
> +
> +struct clkwzd_plat {
> +	fdt_addr_t addr;
> +};
> +
> +static int clk_wzrd_enable(struct clk *clk)
> +{
> +	struct clkwzd *priv = dev_get_priv(clk->dev);
> +	int ret;
> +	u32 val;
> +
> +	ret = readl_poll_sleep_timeout(priv->base + SR, val, val & SR_LOCKED,
> +				       1, 100);
> +	if (!ret) {
> +		writel(CTRL_SEN | CTRL_SADDR | CTRL_LOAD, priv->base + CTRL);
> +		writel(CTRL_SADDR, priv->base + CTRL);
> +		ret = readl_poll_sleep_timeout(priv->base + SR, val,
> +					       val & SR_LOCKED, 1, 100);
> +	}
> +
> +	return ret;
> +}
> +
> +static unsigned long clk_wzrd_set_rate(struct clk *clk, ulong rate)
> +{
> +	struct clkwzd *priv = dev_get_priv(clk->dev);
> +	u64 div;
> +	u32 cfg;
> +
> +	/* Get output clock divide value */
> +	div = DIV_ROUND_DOWN_ULL(priv->vco_clk * 1000, rate);
> +	if (div < 1000 || div > 255999)
> +		return -EINVAL;
> +
> +	cfg = OUT_DIV((u32)div / 1000);
> +
> +	writel(cfg, priv->base + OUT_CFG(clk->id));
> +
> +	return 0;
> +}
> +
> +static struct clk_ops clk_wzrd_ops = {
> +	.enable = clk_wzrd_enable,
> +	.set_rate = clk_wzrd_set_rate,
> +};
> +
> +static int clk_wzrd_probe(struct udevice *dev)
> +{
> +	struct clkwzd_plat *plat = dev_get_plat(dev);
> +	struct clkwzd *priv = dev_get_priv(dev);
> +	struct clk clk_in1;
> +	u64 clock, vco_clk;
> +	u32 cfg;
> +	int ret;
> +
> +	priv->base = (void *)plat->addr;
> +
> +	ret = clk_get_by_name(dev, "clk_in1", &clk_in1);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get clock\n");
> +		return ret;
> +	}
> +
> +	clock = clk_get_rate(&clk_in1);
> +	if (IS_ERR_VALUE(clock)) {
> +		dev_err(dev, "failed to get rate\n");
> +		return clock;
> +	}
> +
> +	ret = clk_enable(&clk_in1);
> +	if (ret) {
> +		dev_err(dev, "failed to enable clock\n");
> +		clk_free(&clk_in1);
> +		return ret;
> +	}
> +
> +	/* Read clock configuration registers */
> +	cfg = readl(priv->base + FBOUT_CFG);
> +
> +	/* Recalculate VCO rate */
> +	if (cfg & FBOUT_FRAC_EN)
> +		vco_clk = DIV_ROUND_DOWN_ULL(clock *
> +					     ((FBOUT_GET_MUL(cfg) * 1000) +
> +					      FBOUT_GET_FRAC(cfg)),
> +					     1000);
> +	else
> +		vco_clk = clock * FBOUT_GET_MUL(cfg);
> +
> +	priv->vco_clk = DIV_ROUND_DOWN_ULL(vco_clk, FBOUT_GET_DIV(cfg));
> +
> +	return 0;
> +}
> +
> +static int clk_wzrd_of_to_plat(struct udevice *dev)
> +{
> +	struct clkwzd_plat *plat = dev_get_plat(dev);
> +
> +	plat->addr = dev_read_addr(dev);
> +	if (plat->addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static const struct udevice_id clk_wzrd_ids[] = {
> +	{ .compatible = "xlnx,clocking-wizard" },
> +	{ /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(clk_wzrd) = {
> +	.name = "zynq-clk-wizard",
> +	.id = UCLASS_CLK,
> +	.of_match = clk_wzrd_ids,
> +	.ops = &clk_wzrd_ops,
> +	.probe = clk_wzrd_probe,
> +	.of_to_plat = clk_wzrd_of_to_plat,
> +	.priv_auto = sizeof(struct clkwzd),
> +	.plat_auto = sizeof(struct clkwzd_plat),
> +};
> 

Reviewed-by: Sean Anderson <sean.anderson@seco.com>

[PATCH v6 0/2] Add Xilinx clock wizard driver support

[Zhengxun]

Changes in v6:
- modify the description in Kconfig

Changes in v5: 
- add devicetree binding
- use FIELD_PREP and FIELD_GET
- use clk_get_by_name()
- add clk_free()

Changes in v4: 
- rearrange functions
- remove Superfluous words
- check kernel-doc comments

Changes in v3: 
- remove incorrect usage in of_to_plat
- get frequencies via clock framework
- delete incorrect naming

Changes in v2: 
- naming is aligned with linux
- delete inappropriate description and code

Zhengxun (2):
  clk: zynq: Add clock wizard driver
  dt-bindings: add xilinx clocking wizard bindings

 .../clock/xlnx,clocking-wizard.txt            |  43 ++++
 drivers/clk/Kconfig                           |  11 ++
 drivers/clk/Makefile                          |   1 +
 drivers/clk/clk-xlnx-clock-wizard.c           | 186 ++++++++++++++++++
 4 files changed, 241 insertions(+)
 create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
 create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c

-- 
2.17.1

[PATCH v6 1/2] clk: zynq: Add clock wizard driver

[Zhengxun]

The Clocking Wizard IP supports clock circuits customized
to your clocking requirements. The wizard support for
dynamically reconfiguring the clocking primitives for
Multiply, Divide, Phase Shift/Offset, or Duty Cycle.

Limited by U-Boot clk uclass without set_phase API, this
patch only provides set_rate to modify the frequency.

Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
---
 drivers/clk/Kconfig                 |  11 ++
 drivers/clk/Makefile                |   1 +
 drivers/clk/clk-xlnx-clock-wizard.c | 186 ++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+)
 create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 40a5a5dd88..a0ac661f28 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -128,6 +128,17 @@ config CLK_ZYNQ
 	  This clock driver adds support for clock related settings for
 	  Zynq platform.
 
+config CLK_XLNX_CLKWZRD
+	bool "Xilinx Clocking Wizard"
+	depends on CLK
+	help
+	  Support for the Xilinx Clocking Wizard IP core clock generator.
+	  The wizard support for dynamically reconfiguring the clocking
+	  primitives for Multiply, Divide, Phase Shift/Offset, or Duty
+	  Cycle. Limited by U-Boot clk uclass without set_phase API and
+	  set_duty_cycle API, this driver only supports set_rate to modify
+	  the frequency.
+
 config CLK_ZYNQMP
 	bool "Enable clock driver support for ZynqMP"
 	depends on ARCH_ZYNQMP
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 645709b855..4fcc33953a 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -43,6 +43,7 @@ obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
 obj-$(CONFIG_CLK_VEXPRESS_OSC) += clk_vexpress_osc.o
 obj-$(CONFIG_CLK_ZYNQ) += clk_zynq.o
 obj-$(CONFIG_CLK_ZYNQMP) += clk_zynqmp.o
+obj-$(CONFIG_CLK_XLNX_CLKWZRD) += clk-xlnx-clock-wizard.o
 obj-$(CONFIG_ICS8N3QV01) += ics8n3qv01.o
 obj-$(CONFIG_MACH_PIC32) += clk_pic32.o
 obj-$(CONFIG_SANDBOX) += clk_sandbox.o
diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
new file mode 100644
index 0000000000..70ee3af107
--- /dev/null
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx 'Clocking Wizard' driver
+ *
+ * Copyright (c) 2021 Macronix Inc.
+ *
+ * Author: Zhengxun Li <zhengxunli@mxic.com.tw>
+ */
+
+#include <common.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <div64.h>
+#include <dm/device_compat.h>
+#include <linux/iopoll.h>
+
+#include <linux/bitfield.h>
+
+#define SRR			0x0
+
+#define SR			0x4
+#define SR_LOCKED		BIT(0)
+
+#define CCR(x)			(0x200 + ((x) * 4))
+
+#define FBOUT_CFG		CCR(0)
+#define FBOUT_DIV(x)		(x)
+#define FBOUT_DIV_MASK		GENMASK(7, 0)
+#define FBOUT_GET_DIV(x)	FIELD_GET(FBOUT_DIV_MASK, x)
+#define FBOUT_MUL(x)		((x) << 8)
+#define FBOUT_MUL_MASK		GENMASK(15, 8)
+#define FBOUT_GET_MUL(x)	FIELD_GET(FBOUT_MUL_MASK, x)
+#define FBOUT_FRAC(x)		((x) << 16)
+#define FBOUT_FRAC_MASK		GENMASK(25, 16)
+#define FBOUT_GET_FRAC(x)	FIELD_GET(FBOUT_FRAC_MASK, x)
+#define FBOUT_FRAC_EN		BIT(26)
+
+#define FBOUT_PHASE		CCR(1)
+
+#define OUT_CFG(x)		CCR(2 + ((x) * 3))
+#define OUT_DIV(x)		(x)
+#define OUT_DIV_MASK		GENMASK(7, 0)
+#define OUT_GET_DIV(x)		FIELD_GET(OUT_DIV_MASK, x)
+#define OUT_FRAC(x)		((x) << 8)
+#define OUT_GET_MASK		GENMASK(17, 8)
+#define OUT_GET_FRAC(x)		FIELD_GET(OUT_GET_MASK, x)
+#define OUT_FRAC_EN		BIT(18)
+
+#define OUT_PHASE(x)		CCR(3 + ((x) * 3))
+#define OUT_DUTY(x)		CCR(4 + ((x) * 3))
+
+#define CTRL			CCR(23)
+#define CTRL_SEN		BIT(2)
+#define CTRL_SADDR		BIT(1)
+#define CTRL_LOAD		BIT(0)
+
+/**
+ * struct clkwzrd - Clock wizard private data structure
+ *
+ * @base:		memory base
+ * @vco_clk:		voltage-controlled oscillator frequency
+ *
+ */
+struct clkwzd {
+	void *base;
+	u64 vco_clk;
+};
+
+struct clkwzd_plat {
+	fdt_addr_t addr;
+};
+
+static int clk_wzrd_enable(struct clk *clk)
+{
+	struct clkwzd *priv = dev_get_priv(clk->dev);
+	int ret;
+	u32 val;
+
+	ret = readl_poll_sleep_timeout(priv->base + SR, val, val & SR_LOCKED,
+				       1, 100);
+	if (!ret) {
+		writel(CTRL_SEN | CTRL_SADDR | CTRL_LOAD, priv->base + CTRL);
+		writel(CTRL_SADDR, priv->base + CTRL);
+		ret = readl_poll_sleep_timeout(priv->base + SR, val,
+					       val & SR_LOCKED, 1, 100);
+	}
+
+	return ret;
+}
+
+static unsigned long clk_wzrd_set_rate(struct clk *clk, ulong rate)
+{
+	struct clkwzd *priv = dev_get_priv(clk->dev);
+	u64 div;
+	u32 cfg;
+
+	/* Get output clock divide value */
+	div = DIV_ROUND_DOWN_ULL(priv->vco_clk * 1000, rate);
+	if (div < 1000 || div > 255999)
+		return -EINVAL;
+
+	cfg = OUT_DIV((u32)div / 1000);
+
+	writel(cfg, priv->base + OUT_CFG(clk->id));
+
+	return 0;
+}
+
+static struct clk_ops clk_wzrd_ops = {
+	.enable = clk_wzrd_enable,
+	.set_rate = clk_wzrd_set_rate,
+};
+
+static int clk_wzrd_probe(struct udevice *dev)
+{
+	struct clkwzd_plat *plat = dev_get_plat(dev);
+	struct clkwzd *priv = dev_get_priv(dev);
+	struct clk clk_in1;
+	u64 clock, vco_clk;
+	u32 cfg;
+	int ret;
+
+	priv->base = (void *)plat->addr;
+
+	ret = clk_get_by_name(dev, "clk_in1", &clk_in1);
+	if (ret < 0) {
+		dev_err(dev, "failed to get clock\n");
+		return ret;
+	}
+
+	clock = clk_get_rate(&clk_in1);
+	if (IS_ERR_VALUE(clock)) {
+		dev_err(dev, "failed to get rate\n");
+		return clock;
+	}
+
+	ret = clk_enable(&clk_in1);
+	if (ret) {
+		dev_err(dev, "failed to enable clock\n");
+		clk_free(&clk_in1);
+		return ret;
+	}
+
+	/* Read clock configuration registers */
+	cfg = readl(priv->base + FBOUT_CFG);
+
+	/* Recalculate VCO rate */
+	if (cfg & FBOUT_FRAC_EN)
+		vco_clk = DIV_ROUND_DOWN_ULL(clock *
+					     ((FBOUT_GET_MUL(cfg) * 1000) +
+					      FBOUT_GET_FRAC(cfg)),
+					     1000);
+	else
+		vco_clk = clock * FBOUT_GET_MUL(cfg);
+
+	priv->vco_clk = DIV_ROUND_DOWN_ULL(vco_clk, FBOUT_GET_DIV(cfg));
+
+	return 0;
+}
+
+static int clk_wzrd_of_to_plat(struct udevice *dev)
+{
+	struct clkwzd_plat *plat = dev_get_plat(dev);
+
+	plat->addr = dev_read_addr(dev);
+	if (plat->addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	return 0;
+}
+
+static const struct udevice_id clk_wzrd_ids[] = {
+	{ .compatible = "xlnx,clocking-wizard" },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(clk_wzrd) = {
+	.name = "zynq-clk-wizard",
+	.id = UCLASS_CLK,
+	.of_match = clk_wzrd_ids,
+	.ops = &clk_wzrd_ops,
+	.probe = clk_wzrd_probe,
+	.of_to_plat = clk_wzrd_of_to_plat,
+	.priv_auto = sizeof(struct clkwzd),
+	.plat_auto = sizeof(struct clkwzd_plat),
+};
-- 
2.17.1

[PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

[Zhengxun]

Add the devicetree binding for the xilinx clocking wizard.

Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
---
 .../clock/xlnx,clocking-wizard.txt            | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt

diff --git a/doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt b/doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
new file mode 100644
index 0000000000..e0e73ec723
--- /dev/null
+++ b/doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
@@ -0,0 +1,43 @@
+Xilinx clocking wizard bindings
+
+description:
+  The clocking wizard is a soft ip clocking block of Xilinx versal. It
+  reads required input clock frequencies from the devicetree and acts as
+  clock output.
+
+properties:
+  compatible:
+    const: xlnx,clocking-wizard
+
+  reg:
+    maxItems: 1
+
+  "#clock-cells":
+    const: 1
+
+  clocks:
+    items:
+      - description: clock input
+
+  clock-names:
+    items:
+      - const: clk_in1
+
+required:
+  - compatible
+  - reg
+  - "#clock-cells"
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    clkwizard@43c20000  {
+        compatible = "xlnx,clocking-wizard";
+        reg = <0x43c20000 0x10000>;
+        #clock-cells = <1>;
+        clock-names = "clk_in1";
+        clocks = <&clkc 18>;
+    };
-- 
2.17.1

Re: [PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

[Sean Anderson]

On 6/15/21 1:21 PM, Zhengxun Li wrote:
 > Hi Michal,
 >
 > Sean Anderson <sean.anderson@seco.com> 於 2021年6月11日 週五 下午3:07寫道：
 >>
 >>
 >>
 >> On 6/11/21 11:10 AM, Zhengxun wrote:
 >>   > Add the devicetree binding for the xilinx clocking wizard.
 >>   >
 >>   > Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
 >>   > ---
 >>   >   .../clock/xlnx,clocking-wizard.txt            | 43 +++++++++++++++++++
 >>   >   1 file changed, 43 insertions(+)
 >>   >   create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
 >>
 >> Should this be .yml? In general, I believe we only need to document
 >> bindings in U-Boot if they differ from Linux. In Linux, I see that
 >> Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.txt exists.

Because the binding is documented in Linux, we do not need to document
it in U-Boot.

 >> Will you be upstreaming this conversion to yaml?

This patch documents the binding in yaml. However, the version in Linux
is in old-style txt. Will you be sending a patch to Linux to convert
this binding to yaml?

--Sean

 >
 > Can you give some suggestions?
 >
 > Thanks,
 > Zhengxun
 >

Re: [PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

[Zhengxun Li]

Hi Michal,

Sean Anderson <sean.anderson@seco.com> 於 2021年6月11日 週五 下午3:07寫道：
>
>
>
> On 6/11/21 11:10 AM, Zhengxun wrote:
>  > Add the devicetree binding for the xilinx clocking wizard.
>  >
>  > Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
>  > ---
>  >   .../clock/xlnx,clocking-wizard.txt            | 43 +++++++++++++++++++
>  >   1 file changed, 43 insertions(+)
>  >   create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
>
> Should this be .yml? In general, I believe we only need to document
> bindings in U-Boot if they differ from Linux. In Linux, I see that
> Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.txt exists.
> Will you be upstreaming this conversion to yaml?

Can you give some suggestions?

Thanks,
Zhengxun

Re: [PATCH v6 1/2] clk: zynq: Add clock wizard driver

[Michal Simek]

On 6/11/21 5:10 PM, Zhengxun wrote:
> The Clocking Wizard IP supports clock circuits customized
> to your clocking requirements. The wizard support for
> dynamically reconfiguring the clocking primitives for
> Multiply, Divide, Phase Shift/Offset, or Duty Cycle.
> 
> Limited by U-Boot clk uclass without set_phase API, this
> patch only provides set_rate to modify the frequency.
> 
> Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
> ---
>  drivers/clk/Kconfig                 |  11 ++
>  drivers/clk/Makefile                |   1 +
>  drivers/clk/clk-xlnx-clock-wizard.c | 186 ++++++++++++++++++++++++++++
>  3 files changed, 198 insertions(+)
>  create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 40a5a5dd88..a0ac661f28 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -128,6 +128,17 @@ config CLK_ZYNQ
>  	  This clock driver adds support for clock related settings for
>  	  Zynq platform.
>  
> +config CLK_XLNX_CLKWZRD
> +	bool "Xilinx Clocking Wizard"
> +	depends on CLK
> +	help
> +	  Support for the Xilinx Clocking Wizard IP core clock generator.
> +	  The wizard support for dynamically reconfiguring the clocking
> +	  primitives for Multiply, Divide, Phase Shift/Offset, or Duty
> +	  Cycle. Limited by U-Boot clk uclass without set_phase API and
> +	  set_duty_cycle API, this driver only supports set_rate to modify
> +	  the frequency.
> +
>  config CLK_ZYNQMP
>  	bool "Enable clock driver support for ZynqMP"
>  	depends on ARCH_ZYNQMP
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 645709b855..4fcc33953a 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_CLK_UNIPHIER) += uniphier/
>  obj-$(CONFIG_CLK_VEXPRESS_OSC) += clk_vexpress_osc.o
>  obj-$(CONFIG_CLK_ZYNQ) += clk_zynq.o
>  obj-$(CONFIG_CLK_ZYNQMP) += clk_zynqmp.o
> +obj-$(CONFIG_CLK_XLNX_CLKWZRD) += clk-xlnx-clock-wizard.o
>  obj-$(CONFIG_ICS8N3QV01) += ics8n3qv01.o
>  obj-$(CONFIG_MACH_PIC32) += clk_pic32.o
>  obj-$(CONFIG_SANDBOX) += clk_sandbox.o
> diff --git a/drivers/clk/clk-xlnx-clock-wizard.c b/drivers/clk/clk-xlnx-clock-wizard.c
> new file mode 100644
> index 0000000000..70ee3af107
> --- /dev/null
> +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx 'Clocking Wizard' driver
> + *
> + * Copyright (c) 2021 Macronix Inc.
> + *
> + * Author: Zhengxun Li <zhengxunli@mxic.com.tw>
> + */
> +
> +#include <common.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <div64.h>
> +#include <dm/device_compat.h>
> +#include <linux/iopoll.h>
> +
> +#include <linux/bitfield.h>
> +
> +#define SRR			0x0
> +
> +#define SR			0x4
> +#define SR_LOCKED		BIT(0)
> +
> +#define CCR(x)			(0x200 + ((x) * 4))
> +
> +#define FBOUT_CFG		CCR(0)
> +#define FBOUT_DIV(x)		(x)
> +#define FBOUT_DIV_MASK		GENMASK(7, 0)
> +#define FBOUT_GET_DIV(x)	FIELD_GET(FBOUT_DIV_MASK, x)
> +#define FBOUT_MUL(x)		((x) << 8)
> +#define FBOUT_MUL_MASK		GENMASK(15, 8)
> +#define FBOUT_GET_MUL(x)	FIELD_GET(FBOUT_MUL_MASK, x)
> +#define FBOUT_FRAC(x)		((x) << 16)
> +#define FBOUT_FRAC_MASK		GENMASK(25, 16)
> +#define FBOUT_GET_FRAC(x)	FIELD_GET(FBOUT_FRAC_MASK, x)
> +#define FBOUT_FRAC_EN		BIT(26)
> +
> +#define FBOUT_PHASE		CCR(1)
> +
> +#define OUT_CFG(x)		CCR(2 + ((x) * 3))
> +#define OUT_DIV(x)		(x)
> +#define OUT_DIV_MASK		GENMASK(7, 0)
> +#define OUT_GET_DIV(x)		FIELD_GET(OUT_DIV_MASK, x)
> +#define OUT_FRAC(x)		((x) << 8)
> +#define OUT_GET_MASK		GENMASK(17, 8)
> +#define OUT_GET_FRAC(x)		FIELD_GET(OUT_GET_MASK, x)
> +#define OUT_FRAC_EN		BIT(18)
> +
> +#define OUT_PHASE(x)		CCR(3 + ((x) * 3))
> +#define OUT_DUTY(x)		CCR(4 + ((x) * 3))
> +
> +#define CTRL			CCR(23)
> +#define CTRL_SEN		BIT(2)
> +#define CTRL_SADDR		BIT(1)
> +#define CTRL_LOAD		BIT(0)
> +
> +/**
> + * struct clkwzrd - Clock wizard private data structure
> + *
> + * @base:		memory base
> + * @vco_clk:		voltage-controlled oscillator frequency
> + *
> + */
> +struct clkwzd {
> +	void *base;
> +	u64 vco_clk;
> +};
> +
> +struct clkwzd_plat {
> +	fdt_addr_t addr;
> +};
> +
> +static int clk_wzrd_enable(struct clk *clk)
> +{
> +	struct clkwzd *priv = dev_get_priv(clk->dev);
> +	int ret;
> +	u32 val;
> +
> +	ret = readl_poll_sleep_timeout(priv->base + SR, val, val & SR_LOCKED,
> +				       1, 100);
> +	if (!ret) {
> +		writel(CTRL_SEN | CTRL_SADDR | CTRL_LOAD, priv->base + CTRL);
> +		writel(CTRL_SADDR, priv->base + CTRL);
> +		ret = readl_poll_sleep_timeout(priv->base + SR, val,
> +					       val & SR_LOCKED, 1, 100);
> +	}
> +
> +	return ret;
> +}
> +
> +static unsigned long clk_wzrd_set_rate(struct clk *clk, ulong rate)
> +{
> +	struct clkwzd *priv = dev_get_priv(clk->dev);
> +	u64 div;
> +	u32 cfg;
> +
> +	/* Get output clock divide value */
> +	div = DIV_ROUND_DOWN_ULL(priv->vco_clk * 1000, rate);
> +	if (div < 1000 || div > 255999)
> +		return -EINVAL;
> +
> +	cfg = OUT_DIV((u32)div / 1000);
> +
> +	writel(cfg, priv->base + OUT_CFG(clk->id));
> +
> +	return 0;
> +}
> +
> +static struct clk_ops clk_wzrd_ops = {
> +	.enable = clk_wzrd_enable,
> +	.set_rate = clk_wzrd_set_rate,
> +};
> +
> +static int clk_wzrd_probe(struct udevice *dev)
> +{
> +	struct clkwzd_plat *plat = dev_get_plat(dev);
> +	struct clkwzd *priv = dev_get_priv(dev);
> +	struct clk clk_in1;
> +	u64 clock, vco_clk;
> +	u32 cfg;
> +	int ret;
> +
> +	priv->base = (void *)plat->addr;
> +
> +	ret = clk_get_by_name(dev, "clk_in1", &clk_in1);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get clock\n");
> +		return ret;
> +	}
> +
> +	clock = clk_get_rate(&clk_in1);
> +	if (IS_ERR_VALUE(clock)) {
> +		dev_err(dev, "failed to get rate\n");
> +		return clock;
> +	}
> +
> +	ret = clk_enable(&clk_in1);
> +	if (ret) {
> +		dev_err(dev, "failed to enable clock\n");
> +		clk_free(&clk_in1);
> +		return ret;
> +	}
> +
> +	/* Read clock configuration registers */
> +	cfg = readl(priv->base + FBOUT_CFG);
> +
> +	/* Recalculate VCO rate */
> +	if (cfg & FBOUT_FRAC_EN)
> +		vco_clk = DIV_ROUND_DOWN_ULL(clock *
> +					     ((FBOUT_GET_MUL(cfg) * 1000) +
> +					      FBOUT_GET_FRAC(cfg)),
> +					     1000);
> +	else
> +		vco_clk = clock * FBOUT_GET_MUL(cfg);
> +
> +	priv->vco_clk = DIV_ROUND_DOWN_ULL(vco_clk, FBOUT_GET_DIV(cfg));
> +
> +	return 0;
> +}
> +
> +static int clk_wzrd_of_to_plat(struct udevice *dev)
> +{
> +	struct clkwzd_plat *plat = dev_get_plat(dev);
> +
> +	plat->addr = dev_read_addr(dev);
> +	if (plat->addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static const struct udevice_id clk_wzrd_ids[] = {
> +	{ .compatible = "xlnx,clocking-wizard" },
> +	{ /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(clk_wzrd) = {
> +	.name = "zynq-clk-wizard",
> +	.id = UCLASS_CLK,
> +	.of_match = clk_wzrd_ids,
> +	.ops = &clk_wzrd_ops,
> +	.probe = clk_wzrd_probe,
> +	.of_to_plat = clk_wzrd_of_to_plat,
> +	.priv_auto = sizeof(struct clkwzd),
> +	.plat_auto = sizeof(struct clkwzd_plat),
> +};
> 

Applied.
M

Re: [PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

[Michal Simek]

Hi,

On 6/16/21 5:26 PM, Zhengxun Li wrote:
> Hi Sean,
> 
> Thank you for your reply.
> 
> Sean Anderson <sean.anderson@seco.com> 於 2021年6月15日 週二 下午2:52寫道：
>>
>>
>>
>> On 6/15/21 1:21 PM, Zhengxun Li wrote:
>>  > Hi Michal,
>>  >
>>  > Sean Anderson <sean.anderson@seco.com> 於 2021年6月11日 週五 下午3:07寫道：
>>  >>
>>  >>
>>  >>
>>  >> On 6/11/21 11:10 AM, Zhengxun wrote:
>>  >>   > Add the devicetree binding for the xilinx clocking wizard.
>>  >>   >
>>  >>   > Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
>>  >>   > ---
>>  >>   >   .../clock/xlnx,clocking-wizard.txt            | 43 +++++++++++++++++++
>>  >>   >   1 file changed, 43 insertions(+)
>>  >>   >   create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
>>  >>
>>  >> Should this be .yml? In general, I believe we only need to document
>>  >> bindings in U-Boot if they differ from Linux. In Linux, I see that
>>  >> Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.txt exists.
>>
>> Because the binding is documented in Linux, we do not need to document
>> it in U-Boot.
>>
>>  >> Will you be upstreaming this conversion to yaml?
>>
>> This patch documents the binding in yaml. However, the version in Linux
>> is in old-style txt. Will you be sending a patch to Linux to convert
>> this binding to yaml?
> 
> In linux, I saw Shubhrajyoti Datta do this, although it has not been
> accepted yet. The link is below.
> 
> https://patchwork.kernel.org/project/linux-clk/patch/1617886723-27117-2-git-send-email-shubhrajyoti.datta@xilinx.com/
> 
> Should I do this again?

I prefer not to do it. Linux binding will be reviewed by Rob and that
should be enough.

Thanks,
Michal

Re: [PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

[Zhengxun Li]

Hi Sean,

Thank you for your reply.

Sean Anderson <sean.anderson@seco.com> 於 2021年6月15日 週二 下午2:52寫道：
>
>
>
> On 6/15/21 1:21 PM, Zhengxun Li wrote:
>  > Hi Michal,
>  >
>  > Sean Anderson <sean.anderson@seco.com> 於 2021年6月11日 週五 下午3:07寫道：
>  >>
>  >>
>  >>
>  >> On 6/11/21 11:10 AM, Zhengxun wrote:
>  >>   > Add the devicetree binding for the xilinx clocking wizard.
>  >>   >
>  >>   > Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
>  >>   > ---
>  >>   >   .../clock/xlnx,clocking-wizard.txt            | 43 +++++++++++++++++++
>  >>   >   1 file changed, 43 insertions(+)
>  >>   >   create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
>  >>
>  >> Should this be .yml? In general, I believe we only need to document
>  >> bindings in U-Boot if they differ from Linux. In Linux, I see that
>  >> Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.txt exists.
>
> Because the binding is documented in Linux, we do not need to document
> it in U-Boot.
>
>  >> Will you be upstreaming this conversion to yaml?
>
> This patch documents the binding in yaml. However, the version in Linux
> is in old-style txt. Will you be sending a patch to Linux to convert
> this binding to yaml?

In linux, I saw Shubhrajyoti Datta do this, although it has not been
accepted yet. The link is below.

https://patchwork.kernel.org/project/linux-clk/patch/1617886723-27117-2-git-send-email-shubhrajyoti.datta@xilinx.com/

Should I do this again?

Thanks,
Zhengxun

Re: [PATCH v6 2/2] dt-bindings: add xilinx clocking wizard bindings

[Zhengxun Li]

Hi,

Michal Simek <michal.simek@xilinx.com> 於 2021年6月16日 週三 上午10:05寫道：
>
> Hi,
>
> On 6/16/21 5:26 PM, Zhengxun Li wrote:
> > Hi Sean,
> >
> > Thank you for your reply.
> >
> > Sean Anderson <sean.anderson@seco.com> 於 2021年6月15日 週二 下午2:52寫道：
> >>
> >>
> >>
> >> On 6/15/21 1:21 PM, Zhengxun Li wrote:
> >>  > Hi Michal,
> >>  >
> >>  > Sean Anderson <sean.anderson@seco.com> 於 2021年6月11日 週五 下午3:07寫道：
> >>  >>
> >>  >>
> >>  >>
> >>  >> On 6/11/21 11:10 AM, Zhengxun wrote:
> >>  >>   > Add the devicetree binding for the xilinx clocking wizard.
> >>  >>   >
> >>  >>   > Signed-off-by: Zhengxun <zhengxunli.mxic@gmail.com>
> >>  >>   > ---
> >>  >>   >   .../clock/xlnx,clocking-wizard.txt            | 43 +++++++++++++++++++
> >>  >>   >   1 file changed, 43 insertions(+)
> >>  >>   >   create mode 100644 doc/device-tree-bindings/clock/xlnx,clocking-wizard.txt
> >>  >>
> >>  >> Should this be .yml? In general, I believe we only need to document
> >>  >> bindings in U-Boot if they differ from Linux. In Linux, I see that
> >>  >> Documentation/devicetree/bindings/clock/xlnx,clocking-wizard.txt exists.
> >>
> >> Because the binding is documented in Linux, we do not need to document
> >> it in U-Boot.
> >>
> >>  >> Will you be upstreaming this conversion to yaml?
> >>
> >> This patch documents the binding in yaml. However, the version in Linux
> >> is in old-style txt. Will you be sending a patch to Linux to convert
> >> this binding to yaml?
> >
> > In linux, I saw Shubhrajyoti Datta do this, although it has not been
> > accepted yet. The link is below.
> >
> > https://patchwork.kernel.org/project/linux-clk/patch/1617886723-27117-2-git-send-email-shubhrajyoti.datta@xilinx.com/
> >
> > Should I do this again?
>
> I prefer not to do it. Linux binding will be reviewed by Rob and that
> should be enough.

Thank you for your reply.

Thanks,
Zhengxun