diff mbox series

[v2,1/2] clk: zynq: Add clock wizard driver

Message ID 1619688674-2302-2-git-send-email-zhengxunli@mxic.com.tw
State Superseded
Delegated to: Michal Simek
Headers show
Series Add Xilinx clock wizard driver support | expand

Commit Message

Zhengxun Li April 29, 2021, 9:31 a.m. UTC 
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 uboot clk uclass without set_phase API, this
patch only provides set_rate to modify the frequency.

Signed-off-by: Zhengxun Li <zhengxunli@mxic.com.tw>
---
 drivers/clk/Kconfig                 |   9 +++
 drivers/clk/Makefile                |   1 +
 drivers/clk/clk-xlnx-clock-wizard.c | 152 ++++++++++++++++++++++++++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c

Comments

Zhengxun Li May 7, 2021, 8:07 a.m. UTC | #1 
Hi Michal,

Thank you for your review.
 
> On 4/29/21 11:31 AM, Zhengxun Li 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 uboot clk uclass without set_phase API, this
> > patch only provides set_rate to modify the frequency.
> > 
> > Signed-off-by: Zhengxun Li <zhengxunli@mxic.com.tw>
> > ---
> >  drivers/clk/Kconfig                 |   9 +++
> >  drivers/clk/Makefile                |   1 +
> >  drivers/clk/clk-xlnx-clock-wizard.c | 152 +++++++++++++++++++++++
> +++++++++++++
> >  3 files changed, 162 insertions(+)
> >  create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
> > 
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 4aeaa0c..f0d4fe0 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -136,6 +136,15 @@ config CLK_ZYNQMP
> >       This clock driver adds support for clock realted settings for
> >       ZynqMP platform.
> > 
> > +config COMMON_CLK_XLNX_CLKWZRD
> > +   bool "Xilinx Clocking Wizard"
> > +   depends on CLK
> > +   help
> > +     Support for the Xilinx Clocking Wizard IP core clock generator.
> > +     Adds support for clocking wizard and compatible.
> > +     This driver supports the Xilinx clocking wizard programmable 
clock
> > +     synthesizer.
> > +
> >  config CLK_STM32MP1
> >     bool "Enable RCC clock driver for STM32MP1"
> >     depends on ARCH_STM32MP && CLK
> > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> > index 645709b..f4ddbe8 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_COMMON_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 0000000..55adb16
> > --- /dev/null
> > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
> > @@ -0,0 +1,152 @@
> > +// 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 <linux/iopoll.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_GET_DIV(x)   ((x) & GENMASK(7, 0))
> > +#define FBOUT_MUL(x)      ((x) << 8)
> > +#define FBOUT_GET_MUL(x)   (((x) & GENMASK(15, 8)) >> 8)
> > +#define FBOUT_FRAC(x)      ((x) << 16)
> > +#define FBOUT_GET_FRAC(x)   (((x) & GENMASK(25, 16)) >> 16)
> > +#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_GET_DIV(x)      ((x) & GENMASK(7, 0))
> > +#define OUT_FRAC(x)      ((x) << 8)
> > +#define OUT_GET_FRAC(x)      (((x) & GENMASK(17, 8)) >> 8)
> > +#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
> > + *
> > + * @lock      Lock pointer
> 
> not listed below
>
> > + * @base      Memory base
>
>
> > + * @vco_clk      voltage-controlled oscillator frequency

Okay.
 
> > + */
> > +struct clkwzd {
> > +   void __iomem *base;
> > +   u64 vco_clk;
> > +};
> > +
> > +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 int clk_wzrd_of_to_plat(struct udevice *dev)
> > +{
> > +   struct clkwzd *priv = dev_get_priv(dev);
> 
> It is not like this.
> 
> This is of_to_plat. It means you are taking memory from
> dev_get_plat(dev); here.

Okay, got it.

> > +   fdt_addr_t addr;
> > +   u64 clk_in1, vco_clk;
> > +   u32 cfg;
> > +
> > +   addr = dev_read_addr(dev);
> > +   if (addr == FDT_ADDR_T_NONE)
> > +      return -EINVAL;
> 
> This is fine.
> 
> > +
> > +   priv->base = (void __iomem *)addr;
> 
> But this assignment should be done in probe where you copy data from
> plat structures to priv structures.

Do you mean priv->base = (void __iomem *)plat->addr?
 
> > +
> > +   clk_in1 = dev_read_u32_default(dev, "clock-frequency", 0);
> 
> This is not the part of DT binding. You should be able to get that
> frequencies via clock framework.

Can you provide some hints about this? I am new to clock driver.

> > +
> > +   /* Read clock configuration registers */
> > +   cfg = readl(priv->base + FBOUT_CFG);
> > +
> > +   /* Recalculate VCO rate */
> > +   if (cfg & FBOUT_FRAC_EN)
> > +      vco_clk = DIV_ROUND_DOWN_ULL(clk_in1 *
> > +                    ((FBOUT_GET_MUL(cfg) * 1000) +
> > +                     FBOUT_GET_FRAC(cfg)),
> > +                    1000);
> > +   else
> > +      vco_clk = clk_in1 * FBOUT_GET_MUL(cfg);
> > +
> > +   priv->vco_clk = DIV_ROUND_DOWN_ULL(vco_clk, FBOUT_GET_DIV(cfg));
> 
> And all of this should be in probe.
> 
> of_to_plat function is here just to read data from DT and save it to
> platdata structure. The reason is that you can create that platdata
> structures yourself and you can support also !DT case.

Okay, got it.
 
> > +
> > +   return 0;
> > +}
> > +
> > +static int clk_wzrd_probe(struct udevice *dev)
> > +{
> > +   return 0;
> > +}
> > +
> > +static struct clk_ops clk_wzrd_ops = {
> > +   .enable = clk_wzrd_enable,
> > +   .set_rate = clk_wzrd_set_rate,
> > +};
> > +
> > +static const struct udevice_id clk_wzrd_ids[] = {
> > +   { .compatible = "xlnx,clk-wizard-5.1" },
> 
> xlnx,clocking-wizard
> please

Okay.

> > +   { /* 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),
> 
> you need to get memory for .plat_auto here.

Okay.

On the other hand, if we want to add set_phase feature to clock wizard, 
can you make some suggestions? I checked the clk-uclass and it does not 
seem to be supported.

Thanks,
Zhengxun


CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================
Michal Simek May 7, 2021, 8:18 a.m. UTC | #2 
Hi,

snip.

>>> +
>>> +   priv->base = (void __iomem *)addr;
>>
>> But this assignment should be done in probe where you copy data from
>> plat structures to priv structures.
> 
> Do you mean priv->base = (void __iomem *)plat->addr?

yes.


>>> +
>>> +   clk_in1 = dev_read_u32_default(dev, "clock-frequency", 0);
>>
>> This is not the part of DT binding. You should be able to get that
>> frequencies via clock framework.
> 
> Can you provide some hints about this? I am new to clock driver.

You should look at drivers/serial/serial_zynq.c where you can find out
clk_get_by_index(), clk_get_rate(), clk_enable()

That clk_get_rate() is the function you should use to get that frequency.

snip.

> 
>>> +   { /* 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),
>>
>> you need to get memory for .plat_auto here.
> 
> Okay.
> 
> On the other hand, if we want to add set_phase feature to clock wizard, 
> can you make some suggestions? I checked the clk-uclass and it does not 
> seem to be supported.

Then you have to add it to uclass first but don't know content here.

> 
> Thanks,
> Zhengxun
> 
> 
> CONFIDENTIALITY NOTE:
> 
> This e-mail and any attachments may contain confidential information 
> and/or personal data, which is protected by applicable laws. Please be 
> reminded that duplication, disclosure, distribution, or use of this e-mail 
> (and/or its attachments) or any part thereof is prohibited. If you receive 
> this e-mail in error, please notify us immediately and delete this mail as 
> well as its attachment(s) from your system. In addition, please be 
> informed that collection, processing, and/or use of personal data is 
> prohibited unless expressly permitted by personal data protection laws. 
> Thank you for your attention and cooperation.
> 
> Macronix International Co., Ltd.
> 
> =====================================================================
> 
> 
> 
> ============================================================================
> 
> CONFIDENTIALITY NOTE:
> 
> This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.
> 
> Macronix International Co., Ltd.
> 
> =====================================================================

And get rid of these. Next time I won't reply to this email. Sorry.

Thanks,
Michal
diff mbox series

Patch

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 4aeaa0c..f0d4fe0 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -136,6 +136,15 @@  config CLK_ZYNQMP
 	  This clock driver adds support for clock realted settings for
 	  ZynqMP platform.
 
+config COMMON_CLK_XLNX_CLKWZRD
+	bool "Xilinx Clocking Wizard"
+	depends on CLK
+	help
+	  Support for the Xilinx Clocking Wizard IP core clock generator.
+	  Adds support for clocking wizard and compatible.
+	  This driver supports the Xilinx clocking wizard programmable clock
+	  synthesizer.
+
 config CLK_STM32MP1
 	bool "Enable RCC clock driver for STM32MP1"
 	depends on ARCH_STM32MP && CLK
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 645709b..f4ddbe8 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_COMMON_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 0000000..55adb16
--- /dev/null
+++ b/drivers/clk/clk-xlnx-clock-wizard.c
@@ -0,0 +1,152 @@ 
+// 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 <linux/iopoll.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_GET_DIV(x)	((x) & GENMASK(7, 0))
+#define FBOUT_MUL(x)		((x) << 8)
+#define FBOUT_GET_MUL(x)	(((x) & GENMASK(15, 8)) >> 8)
+#define FBOUT_FRAC(x)		((x) << 16)
+#define FBOUT_GET_FRAC(x)	(((x) & GENMASK(25, 16)) >> 16)
+#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_GET_DIV(x)		((x) & GENMASK(7, 0))
+#define OUT_FRAC(x)		((x) << 8)
+#define OUT_GET_FRAC(x)		(((x) & GENMASK(17, 8)) >> 8)
+#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
+ *
+ * @lock		Lock pointer
+ * @base		Memory base
+ * @vco_clk		voltage-controlled oscillator frequency
+ */
+struct clkwzd {
+	void __iomem *base;
+	u64 vco_clk;
+};
+
+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 int clk_wzrd_of_to_plat(struct udevice *dev)
+{
+	struct clkwzd *priv = dev_get_priv(dev);
+	fdt_addr_t addr;
+	u64 clk_in1, vco_clk;
+	u32 cfg;
+
+	addr = dev_read_addr(dev);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->base = (void __iomem *)addr;
+
+	clk_in1 = dev_read_u32_default(dev, "clock-frequency", 0);
+
+	/* Read clock configuration registers */
+	cfg = readl(priv->base + FBOUT_CFG);
+
+	/* Recalculate VCO rate */
+	if (cfg & FBOUT_FRAC_EN)
+		vco_clk = DIV_ROUND_DOWN_ULL(clk_in1 *
+					     ((FBOUT_GET_MUL(cfg) * 1000) +
+					      FBOUT_GET_FRAC(cfg)),
+					     1000);
+	else
+		vco_clk = clk_in1 * FBOUT_GET_MUL(cfg);
+
+	priv->vco_clk = DIV_ROUND_DOWN_ULL(vco_clk, FBOUT_GET_DIV(cfg));
+
+	return 0;
+}
+
+static int clk_wzrd_probe(struct udevice *dev)
+{
+	return 0;
+}
+
+static struct clk_ops clk_wzrd_ops = {
+	.enable = clk_wzrd_enable,
+	.set_rate = clk_wzrd_set_rate,
+};
+
+static const struct udevice_id clk_wzrd_ids[] = {
+	{ .compatible = "xlnx,clk-wizard-5.1" },
+	{ /* 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),
+};