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

[Michal Simek]

On 5/14/21 1:23 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 uboot clk uclass without set_phase API, this

U-Boot please

> patch only provides set_rate to modify the frequency.
> 
> Signed-off-by: zhengxun <zhengxunli.mxic at gmail.com>
> ---
>  drivers/clk/Kconfig                 |   9 ++
>  drivers/clk/Makefile                |   1 +
>  drivers/clk/clk-xlnx-clock-wizard.c | 177 ++++++++++++++++++++++++++++

Also add yourself to MAINTAINERS file.

>  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
> +	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_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..9ca4144700
> --- /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 at 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))
> +#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
> + *
> + * @base :		memory base

@base:

> + * @vco_clk :		voltage-controlled oscillator frequency

@vco_clk:


this is kernel-doc format that's why check it by:
./scripts/kernel-doc -man -v drivers/clk/clk-xlnx-clock-wizard.c 1>/dev/null


> + */
> +struct clkwzd {
> +	void __iomem *base;

nit: I think you can remove this __iomem here. writel below are requred
only void*.

> +	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 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 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 __iomem *)plat->addr;

nit: As above remove __iomem from here.

> +
> +	ret = clk_get_by_index(dev, 0, &clk);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get clock\n");
> +		return ret;
> +	}
> +
> +	clock = clk_get_rate(&clk);
> +	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");
> +		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 struct clk_ops clk_wzrd_ops = {
> +	.enable = clk_wzrd_enable,
> +	.set_rate = clk_wzrd_set_rate,
> +};

Can you please rearrange these functions and structures?
clk_wzrd_enable()
clk_wzrd_set_rate()
struct clk_ops clk_wzrd_ops

clk_wzrd_probe()
clk_wzrd_of_to_plat()


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

Thanks,
Michal