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

zhengxunli at mxic.com.tw zhengxunli at mxic.com.tw
Fri Apr 9 07:42:31 CEST 2021


Hi,

> On 4/7/21 11:05 AM, zhengxunli 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 and
> > set 50% duty cycle by default.
> > 
> > Signed-off-by: zhengxunli <zhengxunli at mxic.com.tw>
> 
> Please use full name.

Okay, got it.

> 
> > ---
> >  drivers/clk/Kconfig      |   7 ++
> >  drivers/clk/Makefile     |   1 +
> >  drivers/clk/clk_wizard.c | 180 ++++++++++++++++++++++++++++++++++
> +++++++++++++
> >  3 files changed, 188 insertions(+)
> >  create mode 100644 drivers/clk/clk_wizard.c
> > 
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 4aeaa0c..4ebeccc 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -136,6 +136,13 @@ config CLK_ZYNQMP
> >       This clock driver adds support for clock realted settings for
> >       ZynqMP platform.
> > 
> > +config CLK_WIZARD
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
> tree/drivers/staging/clocking-wizard/Kconfig?h=v5.12-rc6
> 
> Small alignment with kernel would be useful.
> At least CLK_XLNX_CLKWZRD.
>
> > +   bool "Enable clock wizard driver support for zynq"
> > +   depends on CLK && ARCH_ZYNQ
> 
> Clocking wizard is standard PL based IP not just related to Zynq. It can
> be used by Microblaze, ARM cores, etc. It means no need to have
> dependency on ZYNQ here.
> 
> > +   help
> > +     This clock driver adds support for clock wizard setting for
> > +     Zynq platform.
> 
> ditto
>

Okay, I will fix in the next version.
 
> > +
> >  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..d8b878c 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_WIZARD) += clk_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_wizard.c b/drivers/clk/clk_wizard.c
> > new file mode 100644
> > index 0000000..f5c2387
> > --- /dev/null
> > +++ b/drivers/clk/clk_wizard.c
> 
> name could be also aligned with kernel to have easier match with the 
kernel.

Okay, got it.

> 
> > @@ -0,0 +1,180 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Xilinx 'Clocking Wizard' driver
> > + *
> > + * Copyright (c) 2021 Macronix Inc.
> > + *
> > + * Author: Zhengxun Li <zhengxunli at 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)
> > +
> > +/*
> 
> /** for kernel-doc as noted below.
> 
> > + *      MMCM Block Diagram
> > + *
> > + *      +----------------+  +-----------------+
> > + * input ->| vco_clk_div_hw |->| vco_clk_mul_hw  |--+
> > + * rate    | (int divide)   |  | (frac multiply) |  |
> > + *      +----------------+  +-----------------+  |
> > + *                      |
> > + *   +--------------------------------VCO-rate---+
> > + *   |
> > + *   |  +----------------+
> > + *   +->| clkout[0]       |-> output0 rate
> > + *   |  | (frac divide)  |
> > + *   |  +----------------+
> > + *   |
> > + *   |  +----------------+
> > + *   +->| clkout[1]       |-> output1 rate
> > + *   |  | (int divide)   |
> > + *   |  +----------------+
> > + *   |
> > + *     ...
> > + *   |
> > + *   |  +----------------+
> > + *   +->| clkout[1]       |-> output6 rate
> > + *      | (int divide)   |
> > + *      +----------------+
> > + *
> > + * struct clkwzrd - Clock wizard private data structure
> > + *
> > + * @lock      Lock pointer
> > + * @base      Memory base
> > + * @vco_clk      voltage-controlled oscillator frequency
> 
> This is not kernel-doc format but it looks like you are sort of using 
it.
> Fix it and then run:
> ./scripts/kernel-doc -man -v drivers/clk/clk_wizard.c 1>/dev/null

Okay, got it.

> 
> > + */
> > +struct clkwzd {
> > +   struct mutex lock;
> 
> 
> CHECK: struct mutex definition without comment
> #144: FILE: drivers/clk/clk_wizard.c:83:
> +   struct mutex lock;

Okay, got it.

> 
> > +   void __iomem *base;
> > +   u64 vco_clk;
> > +};
> > +
> > +static int zynq_clk_wizard_enable(struct clk *clk)
> > +{
> > +   struct clkwzd *priv = dev_get_priv(clk->dev);
> > +   int ret;
> > +   u32 val;
> > +
> > +   mutex_lock(&priv->lock);
> > +   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);
> > +   }
> > +   mutex_unlock(&priv->lock);
> > +
> > +   return 0;
> 
> return ret?

Okay, I will fix it.

> 
> > +}
> > +
> > +static unsigned long zynq_clk_wizard_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));
> > +
> > +   /* Set duty cycle to 50%. */
> 
> Based on your comment style remove this . here.

Okay, I will remove this in next version.

> 
> > +   writel(50000, priv->base + OUT_DUTY(clk->id));
> > +
> > +   return 0;
> > +}
> > +
> > +static struct clk_ops zynq_clk_wizard_ops = {
> > +   .enable = zynq_clk_wizard_enable,
> > +   .set_rate = zynq_clk_wizard_set_rate,
> > +};
> > +
> > +static const struct udevice_id zynq_clk_wizard_ids[] = {
> > +   { .compatible = "xlnx,clk-wizard-5.1" },
> > +   { /* sentinel */ }
> > +};
> 
> Please move these two structures below probe which is standard location.

Okay, got it.

> > +
> > +static int zynq_clk_wizard_probe(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);
> 
> All these dt stuff should be done in of_to_plat function.

Okay, got it.

> > +
> > +   /* Read clock confguration registers */
> 
> typo in comment.
 
Okay, will fix it.

> > +   cfg = readl(priv->base + FBOUT_CFG);
> > +
> > +   /* Recaculate VCO rate */
> 
> typo

Okay, will fix it.
 
> > +   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);
> > +
> > +   vco_clk = DIV_ROUND_DOWN_ULL(vco_clk, FBOUT_GET_DIV(cfg));
> > +
> > +   priv->vco_clk = vco_clk;
> 
> 
> just
> priv->vco_clk = DIV_ROUND_DOWN_ULL(vco_clk, FBOUT_GET_DIV(cfg));

Okay.

> 
> > +
> > +   return 0;
> > +}
> > +
> > +U_BOOT_DRIVER(zynq_clk_wizard) = {
> > +   .name = "zynq-clk-wizard",
> > +   .id = UCLASS_CLK,
> > +   .of_match = zynq_clk_wizard_ids,
> > +   .ops = &zynq_clk_wizard_ops,
> > +   .probe = zynq_clk_wizard_probe,
> > +   .priv_auto = sizeof(struct clkwzd),
> > +};
> > 

Hope you’re having a great day! 

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.

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


More information about the U-Boot mailing list