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

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                 |   9 ++
 drivers/clk/Makefile                |   1 +
 drivers/clk/clk-xlnx-clock-wizard.c | 177 ++++++++++++++++++++++++++++
 3 files changed, 187 insertions(+)
 create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c

On 5/21/21 12:18 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                 |   9 ++
 >   drivers/clk/Makefile                |   1 +
 >   drivers/clk/clk-xlnx-clock-wizard.c | 177 ++++++++++++++++++++++++++++
 >   3 files changed, 187 insertions(+)
 >   create mode 100644 drivers/clk/clk-xlnx-clock-wizard.c
 >
 > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
 > index 40a5a5dd88..dfaeac0204 100644
 > --- a/drivers/clk/Kconfig
 > +++ b/drivers/clk/Kconfig
 > @@ -128,6 +128,15 @@ config CLK_ZYNQ
 >   	  This clock driver adds support for clock related settings for
 >   	  Zynq platform.
 >
 > +config COMMON_CLK_XLNX_CLKWZRD

Why not just CLK_XILNIX_WIZARD? Do we need "COMMON" in here?

 > +	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.

This can all be one sentence. Please also add some basic information
about the clock wizard. What sorts of systems might one find this device
on? It looks like configuration is determined from registers within the
clock itself. Perhaps add a note about that.

 > +
 >   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..f4ddbe831a 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 0000000000..76c0eb27e6
 > --- /dev/null
 > +++ b/drivers/clk/clk-xlnx-clock-wizard.c
 > @@ -0,0 +1,177 @@
 > +// 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>
 > +
 > +#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))

Please use FIELD_PREP and FIELD_GET. And please define the mask
separately.

 > +#define OUT_FRAC(x)		((x) << 8)
 > +#define OUT_GET_FRAC(x)		(((x) & GENMASK(17, 8)) >> 8)

ditto

 > +#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;
 > +	u64 clock, vco_clk;
 > +	u32 cfg;
 > +	int ret;
 > +
 > +	priv->base = (void *)plat->addr;
 > +
 > +	ret = clk_get_by_index(dev, 0, &clk);

With reference to Linux's drivers/staging/clocking-wizard/dt-binding.txt,
please use clk_get_by_name(dev, "clk_in1", &clk)

 > +	if (ret < 0) {
 > +		dev_err(dev, "failed to get clock\n");
 > +		return ret;
 > +	}
 > +
 > +	clock = clk_get_rate(&clk);

Please name this rate or something to that effect.

 > +	if (IS_ERR_VALUE(clock)) {
 > +		dev_err(dev, "failed to get rate\n");
 > +		return clock;
 > +	}
 > +
 > +	ret = clk_enable(&clk);
 > +	if (ret) {
 > +		dev_err(dev, "failed to enable clock\n");

Missing clk_free()

--Sean

 > +		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),
 > +};
 >