[PATCH v2 06/17] clk: starfive: Add StarFive JH7110 clock driver

Sean Anderson seanga2 at gmail.com
Sat Jan 21 19:56:26 CET 2023


On 1/18/23 03:11, Yanhong Wang wrote:
> Add a DM clock driver for StarFive JH7110 SoC.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang at starfivetech.com>
> ---
>   drivers/clk/Kconfig                   |   1 +
>   drivers/clk/Makefile                  |   1 +
>   drivers/clk/starfive/Kconfig          |  17 +
>   drivers/clk/starfive/Makefile         |   4 +
>   drivers/clk/starfive/clk-jh7110-pll.c | 293 ++++++++++++++
>   drivers/clk/starfive/clk-jh7110.c     | 559 ++++++++++++++++++++++++++
>   drivers/clk/starfive/clk.h            |  60 +++
>   7 files changed, 935 insertions(+)
>   create mode 100644 drivers/clk/starfive/Kconfig
>   create mode 100644 drivers/clk/starfive/Makefile
>   create mode 100644 drivers/clk/starfive/clk-jh7110-pll.c
>   create mode 100644 drivers/clk/starfive/clk-jh7110.c
>   create mode 100644 drivers/clk/starfive/clk.h
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 09aa97ee8c..4d60c84aad 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -235,6 +235,7 @@ source "drivers/clk/owl/Kconfig"
>   source "drivers/clk/renesas/Kconfig"
>   source "drivers/clk/sunxi/Kconfig"
>   source "drivers/clk/sifive/Kconfig"
> +source "drivers/clk/starfive/Kconfig"
>   source "drivers/clk/stm32/Kconfig"
>   source "drivers/clk/tegra/Kconfig"
>   source "drivers/clk/ti/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index c274cda77c..66f5860356 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -13,6 +13,7 @@ obj-$(CONFIG_$(SPL_TPL_)CLK_COMPOSITE_CCF) += clk-composite.o
>   
>   obj-y += analogbits/
>   obj-y += imx/
> +obj-$(CONFIG_CLK_JH7110) += starfive/
>   obj-y += tegra/
>   obj-y += ti/
>   obj-$(CONFIG_$(SPL_TPL_)CLK_INTEL) += intel/
> diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig
> new file mode 100644
> index 0000000000..9399ef6d51
> --- /dev/null
> +++ b/drivers/clk/starfive/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +config SPL_CLK_JH7110
> +	bool "SPL clock support for JH7110"
> +	depends on STARFIVE_JH7110 && SPL
> +	select SPL_CLK
> +	select SPL_CLK_CCF
> +	help
> +	  This enables SPL DM support for clock driver in JH7110.
> +
> +config CLK_JH7110
> +	bool "StarFive JH7110 clock support"
> +	depends on STARFIVE_JH7110
> +	select CLK
> +	select CLK_CCF
> +	help
> +	  This enables support clock driver for StarFive JH7110 SoC platform.
> diff --git a/drivers/clk/starfive/Makefile b/drivers/clk/starfive/Makefile
> new file mode 100644
> index 0000000000..ec0d157094
> --- /dev/null
> +++ b/drivers/clk/starfive/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +obj-y += clk-jh7110.o
> +obj-y += clk-jh7110-pll.o
> diff --git a/drivers/clk/starfive/clk-jh7110-pll.c b/drivers/clk/starfive/clk-jh7110-pll.c
> new file mode 100644
> index 0000000000..08e1755d3a
> --- /dev/null
> +++ b/drivers/clk/starfive/clk-jh7110-pll.c
> @@ -0,0 +1,293 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.

2022-23 :)

> + *
> + * Author:	Yanhong Wang <yanhong.wang at starfivetech.com>
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <malloc.h>
> +#include <clk-uclass.h>
> +#include <div64.h>
> +#include <dm/device.h>
> +#include <linux/bitops.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +
> +#include "clk.h"
> +
> +#define UBOOT_DM_CLK_JH7110_PLLX "jh7110_clk_pllx"
> +
> +#define PLL_PD_OFF		1
> +#define PLL_PD_ON		0
> +
> +#define CLK_DDR_BUS_MASK	GENMASK(29, 24)
> +#define CLK_DDR_BUS_OFFSET	0xAC
> +#define CLK_DDR_BUS_OSC_DIV2	0
> +#define CLK_DDR_BUS_PLL1_DIV2	1
> +#define CLK_DDR_BUS_PLL1_DIV4	2
> +#define CLK_DDR_BUS_PLL1_DIV8	3
> +
> +struct clk_jh7110_pllx {
> +	struct clk		clk;
> +	void __iomem	*base;
> +	void __iomem	*sysreg;
> +	enum starfive_pll_type	type;
> +	const struct starfive_pllx_offset *offset;
> +	const struct starfive_pllx_rate *rate_table;
> +	int rate_count;
> +};
> +
> +#define getbits_le32(addr, mask) ((in_le32(addr) & (mask)) >> __ffs((mask)))
> +
> +#define PLLX_SET(offset, mask, val) do {\
> +		reg = readl((ulong *)((ulong)pll->base + (offset))); \
> +		reg &= ~(mask); \
> +		reg |= (mask) & ((val) << __ffs(mask)); \
> +		writel(reg, (ulong *)((ulong)pll->base + (offset))); \
> +	} while (0)
> +
> +#define PLLX_RATE(_rate, _pd, _fd, _pd1, _da, _ds)	\
> +	{						\
> +		.rate		= (_rate),	\
> +		.prediv		= (_pd),	\
> +		.fbdiv		= (_fd),	\
> +		.postdiv1	= (_pd1),	\
> +		.dacpd		= (_da),	\
> +		.dsmpd		= (_ds),	\
> +	}
> +
> +#define to_clk_pllx(_clk) container_of(_clk, struct clk_jh7110_pllx, clk)
> +
> +static const struct starfive_pllx_rate jh7110_pll0_tbl[] = {
> +	PLLX_RATE(375000000UL, 8, 125, 1, 1, 1),
> +	PLLX_RATE(500000000UL, 6, 125, 1, 1, 1),
> +	PLLX_RATE(625000000UL, 24, 625, 1, 1, 1),
> +	PLLX_RATE(750000000UL, 4, 125, 1, 1, 1),
> +	PLLX_RATE(875000000UL, 24, 875, 1, 1, 1),
> +	PLLX_RATE(1000000000UL, 3, 125, 1, 1, 1),
> +	PLLX_RATE(1250000000UL, 12, 625, 1, 1, 1),
> +	PLLX_RATE(1375000000UL, 24, 1375, 1, 1, 1),
> +	PLLX_RATE(1500000000UL, 2, 125, 1, 1, 1),
> +	PLLX_RATE(1625000000UL, 24, 1625, 1, 1, 1),
> +	PLLX_RATE(1750000000UL, 12, 875, 1, 1, 1),
> +	PLLX_RATE(1800000000UL, 3, 225, 1, 1, 1),
> +};
> +
> +static const struct starfive_pllx_rate jh7110_pll1_tbl[] = {
> +	PLLX_RATE(1066000000UL, 12, 533, 1, 1, 1),
> +	PLLX_RATE(1200000000UL, 1, 50, 1, 1, 1),
> +	PLLX_RATE(1400000000UL, 6, 350, 1, 1, 1),
> +	PLLX_RATE(1600000000UL, 3, 200, 1, 1, 1),
> +};
> +
> +static const struct starfive_pllx_rate jh7110_pll2_tbl[] = {
> +	PLLX_RATE(1228800000UL, 15, 768, 1, 1, 1),
> +	PLLX_RATE(1188000000UL, 2, 99, 1, 1, 1),
> +};

My comment from last time stands about pd1/da/ds

(sorry for not getting back to you faster)

> +static const struct starfive_pllx_offset jh7110_pll0_offset = {
> +	.prediv = 0x24,
> +	.fbdiv = 0x1c,
> +	.frac = 0x20,
> +	.postdiv1 = 0x20,
> +	.dacpd = 0x18,
> +	.dsmpd = 0x18,
> +	.prediv_mask = GENMASK(5, 0),
> +	.fbdiv_mask = GENMASK(11, 0),
> +	.frac_mask = GENMASK(23, 0),
> +	.postdiv1_mask = GENMASK(29, 28),
> +	.dacpd_mask = BIT(24),
> +	.dsmpd_mask = BIT(25)
> +};
> +
> +static const struct starfive_pllx_offset jh7110_pll1_offset = {
> +	.prediv = 0x2c,
> +	.fbdiv = 0x24,
> +	.frac = 0x28,
> +	.postdiv1 = 0x28,
> +	.dacpd = 0x24,
> +	.dsmpd = 0x24,
> +	.prediv_mask = GENMASK(5, 0),
> +	.fbdiv_mask = GENMASK(28, 17),
> +	.frac_mask = GENMASK(23, 0),
> +	.postdiv1_mask = GENMASK(29, 28),
> +	.dacpd_mask = BIT(15),
> +	.dsmpd_mask = BIT(16)
> +};
> +
> +static const struct starfive_pllx_offset jh7110_pll2_offset = {
> +	.prediv = 0x34,
> +	.fbdiv = 0x2c,
> +	.frac = 0x30,
> +	.postdiv1 = 0x30,
> +	.dacpd = 0x2c,
> +	.dsmpd = 0x2c,
> +	.prediv_mask = GENMASK(5, 0),
> +	.fbdiv_mask = GENMASK(28, 17),
> +	.frac_mask = GENMASK(23, 0),
> +	.postdiv1_mask = GENMASK(29, 28),
> +	.dacpd_mask = BIT(15),
> +	.dsmpd_mask = BIT(16)
> +};

OK, so it looks like these PLLs don't have the same register offsets
like I thought. Since this is the case, you can keep your existing style
or use the style from v1.

> +struct starfive_pllx_clk starfive_jh7110_pll0 __initdata = {
> +	.type = PLL0,
> +	.offset = &jh7110_pll0_offset,
> +	.rate_table = jh7110_pll0_tbl,
> +	.rate_count = ARRAY_SIZE(jh7110_pll0_tbl),
> +};
> +
> +struct starfive_pllx_clk starfive_jh7110_pll1 __initdata = {
> +	.type = PLL1,
> +	.offset = &jh7110_pll1_offset,
> +	.rate_table = jh7110_pll1_tbl,
> +	.rate_count = ARRAY_SIZE(jh7110_pll1_tbl),
> +};
> +
> +struct starfive_pllx_clk starfive_jh7110_pll2 __initdata = {
> +	.type = PLL2,
> +	.offset = &jh7110_pll2_offset,
> +	.rate_table = jh7110_pll2_tbl,
> +	.rate_count = ARRAY_SIZE(jh7110_pll2_tbl),
> +};
> +
> +static const struct starfive_pllx_rate *
> +	jh7110_get_pll_settings(struct clk_jh7110_pllx *pll, unsigned long rate)

No indent necessary.

> +{
> +	for (int i = 0; i < pll->rate_count; i++)
> +		if (rate == pll->rate_table[i].rate)
> +			return &pll->rate_table[i];
> +
> +	return NULL;
> +}
> +
> +static void jh7110_pll_set_rate(struct clk_jh7110_pllx *pll,
> +				const struct starfive_pllx_rate *rate)
> +{
> +	u32 reg;
> +	bool set = (pll->type == PLL1) ? true : false;
> +
> +	if (set) {
> +		reg = readl((ulong *)((ulong)pll->sysreg + CLK_DDR_BUS_OFFSET));
> +		reg &= ~CLK_DDR_BUS_MASK;
> +		reg |= CLK_DDR_BUS_OSC_DIV2 << __ffs(CLK_DDR_BUS_MASK);
> +		writel(reg, (ulong *)((ulong)pll->sysreg + CLK_DDR_BUS_OFFSET));
> +	}
> +
> +	PLLX_SET(pll->offset->pd, pll->offset->pd_mask, PLL_PD_OFF);
> +	PLLX_SET(pll->offset->dacpd, pll->offset->dacpd_mask, rate->dacpd);
> +	PLLX_SET(pll->offset->dsmpd, pll->offset->dsmpd_mask, rate->dsmpd);

E.g. this can be PLLX_SET(pll->offset->dsmpd, pll->offset->dsmpd_mask, 1);

> +	PLLX_SET(pll->offset->prediv, pll->offset->prediv_mask, rate->prediv);
> +	PLLX_SET(pll->offset->fbdiv, pll->offset->fbdiv_mask, rate->fbdiv);
> +	PLLX_SET(pll->offset->postdiv1, pll->offset->postdiv1, rate->postdiv1 >> 1);

Same question from last time. As you explained, we have something like

postdiv divider
0	/1
1	/2
2	/4
3	/8

But right shifting is not the correct way to do this. You need to use
ffs (or ilog2). Of course, you missed this bug because you only ever use
a /1 postdiv. So I suggest you just always write 0 to postdiv and skip
the calculation.

> +	PLLX_SET(pll->offset->pd, pll->offset->pd_mask, PLL_PD_ON);

This still obscures the actual registers...

I would prefer for you to combine writes to the same register so it is
clear what is going on.

> +
> +	if (set) {
> +		udelay(100);
> +		reg = readl((ulong *)((ulong)pll->sysreg + CLK_DDR_BUS_OFFSET));
> +		reg &= ~CLK_DDR_BUS_MASK;
> +		reg |= CLK_DDR_BUS_PLL1_DIV2 << __ffs(CLK_DDR_BUS_MASK);
> +		writel(reg, (ulong *)((ulong)pll->sysreg + CLK_DDR_BUS_OFFSET));
> +	}
> +}
> +
> +static ulong jh7110_pllx_recalc_rate(struct clk *clk)
> +{
> +	struct clk_jh7110_pllx *pll = to_clk_pllx(dev_get_clk_ptr(clk->dev));
> +	u64 refclk = clk_get_parent_rate(clk);
> +	u32 dacpd, dsmpd;
> +	u32 prediv, fbdiv, postdiv1;
> +	u64 frac;
> +
> +	dacpd = getbits_le32((ulong)pll->base + pll->offset->dacpd,
> +			     pll->offset->dacpd_mask);
> +	dsmpd = getbits_le32((ulong)pll->base + pll->offset->dsmpd,
> +			     pll->offset->dsmpd_mask);
> +	prediv = getbits_le32((ulong)pll->base + pll->offset->prediv,
> +			      pll->offset->prediv_mask);
> +	fbdiv = getbits_le32((ulong)pll->base + pll->offset->fbdiv,
> +			     pll->offset->fbdiv_mask);
> +	postdiv1 = 1 << getbits_le32((ulong)pll->base + pll->offset->postdiv1,
> +			pll->offset->postdiv1_mask);
> +	frac = (u64)getbits_le32((ulong)pll->base + pll->offset->frac,
> +			pll->offset->frac_mask);
> +
> +	/* Integer Mode or Fraction Mode */
> +	if (dacpd == 1 && dsmpd == 1)
> +		frac = 0;
> +	else if (dacpd == 0 && dsmpd == 0)
> +		do_div(frac, 1 << 24);

Last time you provided an explanation for where this came from. Please
keep that explanation as a comment here.

> +	else
> +		return -EINVAL;
> +
> +	refclk *= (fbdiv + frac);
> +	do_div(refclk, prediv * postdiv1);
> +
> +	return refclk;
> +}
> +
> +static ulong jh7110_pllx_set_rate(struct clk *clk, ulong drate)
> +{
> +	struct clk_jh7110_pllx *pll = to_clk_pllx(dev_get_clk_ptr(clk->dev));
> +	const struct starfive_pllx_rate *rate;
> +
> +	rate = jh7110_get_pll_settings(pll, drate);
> +	if (!rate)
> +		return -EINVAL;
> +
> +	jh7110_pll_set_rate(pll, rate);
> +
> +	return jh7110_pllx_recalc_rate(clk);
> +}
> +
> +static const struct clk_ops clk_jh7110_ops = {
> +	.set_rate	= jh7110_pllx_set_rate,
> +	.get_rate	= jh7110_pllx_recalc_rate,
> +};
> +
> +struct clk *starfive_jh7110_pll(const char *name, const char *parent_name,
> +				void __iomem *base, void __iomem *sysreg,
> +				const struct starfive_pllx_clk *pll_clk)
> +{
> +	struct clk_jh7110_pllx *pll;
> +	struct clk *clk;
> +	int ret;
> +
> +	if (!pll_clk || !base || !sysreg)
> +		return ERR_PTR(-EINVAL);
> +
> +	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
> +	if (!pll)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pll->base = base;
> +	pll->sysreg = sysreg;
> +	pll->type = pll_clk->type;
> +	pll->offset = pll_clk->offset;
> +	pll->rate_table = pll_clk->rate_table;
> +	pll->rate_count = pll_clk->rate_count;
> +
> +	clk = &pll->clk;
> +	ret = clk_register(clk, UBOOT_DM_CLK_JH7110_PLLX, name, parent_name);
> +	if (ret) {
> +		kfree(pll);
> +		return ERR_PTR(ret);
> +	}
> +
> +	if (IS_ENABLED(CONFIG_SPL_BUILD) && pll->type == PLL0)
> +		jh7110_pllx_set_rate(clk, 1250000000);
> +
> +	if (IS_ENABLED(CONFIG_SPL_BUILD) && pll->type == PLL2)
> +		jh7110_pllx_set_rate(clk, 1188000000);
> +
> +	return clk;
> +}
> +
> +U_BOOT_DRIVER(jh7110_clk_pllx) = {
> +	.name	= UBOOT_DM_CLK_JH7110_PLLX,
> +	.id	= UCLASS_CLK,
> +	.ops	= &clk_jh7110_ops,
> +};
> diff --git a/drivers/clk/starfive/clk-jh7110.c b/drivers/clk/starfive/clk-jh7110.c
> new file mode 100644
> index 0000000000..a904852cab
> --- /dev/null
> +++ b/drivers/clk/starfive/clk-jh7110.c
> @@ -0,0 +1,559 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + *
> + * Author:	Yanhong Wang <yanhong.wang at starfivetech.com>
> + */
> +
> +#include <common.h>
> +#include <clk.h>
> +#include <clk-uclass.h>
> +#include <dm.h>
> +#include <dm/device.h>
> +#include <dm/devres.h>
> +#include <dm/lists.h>
> +#include <dt-bindings/clock/starfive-jh7110.h>
> +#include <log.h>
> +#include <linux/clk-provider.h>
> +
> +#include "clk.h"
> +
> +#define STARFIVE_CLK_ENABLE_SHIFT	31 /* [31] */
> +#define STARFIVE_CLK_INVERT_SHIFT	30 /* [30] */
> +#define STARFIVE_CLK_MUX_SHIFT		24 /* [29:24] */
> +#define STARFIVE_CLK_DIV_SHIFT		0  /* [23:0] */
> +
> +#define OFFSET(id) ((id) * 4)
> +#define AONOFFSET(id) (((id) - JH7110_SYSCLK_END) * 4)
> +#define STGOFFSET(id) (((id) - JH7110_AONCLK_END) * 4)
> +
> +typedef int (*jh1710_init_fn)(struct udevice *dev);
> +
> +struct jh7110_clk_priv {
> +	void __iomem *reg;
> +	jh1710_init_fn init;
> +};
> +
> +static const char *cpu_root_sels[2] = {
> +	[0] = "osc",
> +	[1] = "pll0_out",
> +};
> +
> +static const char *perh_root_sels[2] = {
> +	[0] = "pll0_out",
> +	[1] = "pll2_out",
> +};
> +
> +static const char *bus_root_sels[2] = {
> +	[0] = "osc",
> +	[1] = "pll2_out",
> +};
> +
> +static const char *qspi_ref_sels[2] = {
> +	[0] = "osc",
> +	[1] = "qspi_ref_src",
> +};
> +
> +static const char *gmac1_tx_sels[2] = {
> +	[0] = "gmac1_gtxclk",
> +	[1] = "gmac1_rmii_rtx",
> +};
> +
> +static const char *gmac0_tx_sels[2] = {
> +	[0] = "gmac0_gtxclk",
> +	[1] = "gmac0_rmii_rtx",
> +};
> +
> +static struct clk *starfive_clk_mux(void __iomem *reg,
> +				    const char *name,
> +				    unsigned int offset,
> +				    u8 width,
> +				    const char * const *parent_names,
> +				    u8 num_parents)
> +{
> +	return  clk_register_mux(NULL, name, parent_names, num_parents, 0,
> +				reg + offset, STARFIVE_CLK_MUX_SHIFT,
> +				width, 0);
> +}
> +
> +static struct clk *starfive_clk_gate(void __iomem *reg,
> +				     const char *name,
> +				     const char *parent_name,
> +				     unsigned int offset)
> +{
> +	return clk_register_gate(NULL, name, parent_name, 0, reg + offset,
> +				STARFIVE_CLK_ENABLE_SHIFT, 0, NULL);
> +}
> +
> +static struct clk *starfive_clk_fix_factor(void __iomem *reg,
> +					   const char *name,
> +					   const char *parent_name,
> +					   unsigned int mult,
> +					   unsigned int div)
> +{
> +	return clk_register_fixed_factor(NULL, name, parent_name,
> +		0, mult, div);
> +}
> +
> +static struct clk *starfive_clk_divider(void __iomem *reg,
> +					const char *name,
> +					const char *parent_name,
> +					unsigned int offset,
> +					u8 width)
> +{
> +	return clk_register_divider(NULL, name, parent_name, 0, reg + offset,
> +				0, width, CLK_DIVIDER_ONE_BASED);
> +}
> +
> +static struct clk *starfive_clk_composite(void __iomem *reg,
> +					  const char *name,
> +					  const char * const *parent_names,
> +					  unsigned int num_parents,
> +					  unsigned int offset,
> +					  unsigned int mux_width,
> +					  unsigned int gate_width,
> +					  unsigned int div_width)
> +{
> +	struct clk *clk = ERR_PTR(-ENOMEM);
> +	struct clk_divider *div = NULL;
> +	struct clk_gate *gate = NULL;
> +	struct clk_mux *mux = NULL;
> +	int mask_arry[4] = {0x1, 0x3, 0x7, 0xF};
> +	int mask;
> +
> +	if (mux_width) {
> +		if (mux_width > 4)
> +			goto fail;
> +		else
> +			mask = mask_arry[mux_width - 1];
> +
> +		mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +		if (!mux)
> +			goto fail;
> +
> +		mux->reg = reg + offset;
> +		mux->mask = mask;
> +		mux->shift = STARFIVE_CLK_MUX_SHIFT;
> +		mux->num_parents = num_parents;
> +		mux->flags = 0;
> +		mux->parent_names = parent_names;
> +	}
> +
> +	if (gate_width) {
> +		gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +
> +		if (!gate)
> +			goto fail;
> +
> +		gate->reg = reg + offset;
> +		gate->bit_idx = STARFIVE_CLK_ENABLE_SHIFT;
> +		gate->flags = 0;
> +	}
> +
> +	if (div_width) {
> +		div = kzalloc(sizeof(*div), GFP_KERNEL);
> +		if (!div)
> +			goto fail;
> +
> +		div->reg = reg + offset;
> +
> +		if (offset == OFFSET(JH7110_SYSCLK_UART3_CORE) ||
> +		    offset == OFFSET(JH7110_SYSCLK_UART4_CORE) ||
> +		    offset == OFFSET(JH7110_SYSCLK_UART5_CORE)) {
> +			div->shift = 8;
> +			div->width = 8;
> +		} else {
> +			div->shift = STARFIVE_CLK_DIV_SHIFT;
> +			div->width = div_width;
> +		}
> +		div->flags = CLK_DIVIDER_ONE_BASED;
> +		div->table = NULL;
> +	}
> +
> +	clk = clk_register_composite(NULL, name,
> +				     parent_names, num_parents,
> +				     &mux->clk, &clk_mux_ops,
> +				     &div->clk, &clk_divider_ops,
> +				     &gate->clk, &clk_gate_ops, 0);
> +
> +	if (IS_ERR(clk))
> +		goto fail;
> +
> +	return clk;
> +
> +fail:
> +	kfree(gate);
> +	kfree(div);
> +	kfree(mux);
> +	return ERR_CAST(clk);
> +}
> +
> +static struct clk *starfive_clk_fix_parent_composite(void __iomem *reg,
> +						     const char *name,
> +						     const char *parent_names,
> +						     unsigned int offset,
> +						     unsigned int mux_width,
> +						     unsigned int gate_width,
> +						     unsigned int div_width)
> +{
> +	const char * const *parents;
> +
> +	parents  = &parent_names;
> +
> +	return starfive_clk_composite(reg, name, parents, 1, offset,
> +			mux_width, gate_width, div_width);
> +}
> +
> +static struct clk *starfive_clk_gate_divider(void __iomem *reg,
> +					     const char *name,
> +					     const char *parent,
> +					     unsigned int offset,
> +					     unsigned int width)
> +{
> +	const char * const *parent_names;
> +
> +	parent_names  = &parent;
> +
> +	return starfive_clk_composite(reg, name, parent_names, 1,
> +				offset, 0, 1, width);
> +}
> +
> +static int jh7110_syscrg_init(struct udevice *dev)
> +{
> +	struct jh7110_clk_priv *priv = dev_get_priv(dev);
> +	struct ofnode_phandle_args args;
> +	fdt_addr_t addr;
> +	int ret;
> +
> +	ret = ofnode_parse_phandle_with_args(dev->node_, "starfive,sys-syscon", NULL, 0, 0, &args);
> +	if (ret)
> +		return ret;

Last time you said the bindings in linux had not been updated. Have you
resubmitted with this property added?

Maybe you should have a separate binding for the PLLs? See below for
ordering advice.

> +	addr =  ofnode_get_addr(args.node);
> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	clk_dm(JH7110_SYSCLK_PLL0_OUT,
> +	       starfive_jh7110_pll("pll0_out", "osc", (void __iomem *)addr,
> +				   priv->reg, &starfive_jh7110_pll0));
> +	clk_dm(JH7110_SYSCLK_PLL1_OUT,
> +	       starfive_jh7110_pll("pll1_out", "osc", (void __iomem *)addr,
> +				   priv->reg, &starfive_jh7110_pll1));
> +	clk_dm(JH7110_SYSCLK_PLL2_OUT,
> +	       starfive_jh7110_pll("pll2_out", "osc", (void __iomem *)addr,
> +				   priv->reg, &starfive_jh7110_pll2));
> +	clk_dm(JH7110_SYSCLK_CPU_ROOT,
> +	       starfive_clk_mux(priv->reg, "cpu_root",
> +				OFFSET(JH7110_SYSCLK_CPU_ROOT), 1,
> +				cpu_root_sels, ARRAY_SIZE(cpu_root_sels)));
> +	clk_dm(JH7110_SYSCLK_CPU_CORE,
> +	       starfive_clk_divider(priv->reg,
> +				    "cpu_core", "cpu_root",
> +				    OFFSET(JH7110_SYSCLK_CPU_CORE), 3));
> +	clk_dm(JH7110_SYSCLK_CPU_BUS,
> +	       starfive_clk_divider(priv->reg,
> +				    "cpu_bus", "cpu_core",
> +				    OFFSET(JH7110_SYSCLK_CPU_BUS), 2));
> +	clk_dm(JH7110_SYSCLK_GMACUSB_ROOT,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "gmacusb_root", "pll0_out", 1, 1));
> +	clk_dm(JH7110_SYSCLK_PERH_ROOT,
> +	       starfive_clk_composite(priv->reg,
> +				      "perh_root",
> +				      perh_root_sels, ARRAY_SIZE(perh_root_sels),
> +				      OFFSET(JH7110_SYSCLK_PERH_ROOT), 1, 0, 2));
> +	clk_dm(JH7110_SYSCLK_BUS_ROOT,
> +	       starfive_clk_mux(priv->reg, "bus_root",
> +				OFFSET(JH7110_SYSCLK_BUS_ROOT), 1,
> +				bus_root_sels,	ARRAY_SIZE(bus_root_sels)));
> +	clk_dm(JH7110_SYSCLK_AXI_CFG0,
> +	       starfive_clk_divider(priv->reg,
> +				    "axi_cfg0", "bus_root",
> +				    OFFSET(JH7110_SYSCLK_AXI_CFG0), 2));
> +	clk_dm(JH7110_SYSCLK_STG_AXIAHB,
> +	       starfive_clk_divider(priv->reg,
> +				    "stg_axiahb", "axi_cfg0",
> +				    OFFSET(JH7110_SYSCLK_STG_AXIAHB), 2));
> +	clk_dm(JH7110_SYSCLK_AHB0,
> +	       starfive_clk_gate(priv->reg,
> +				 "ahb0", "stg_axiahb",
> +				 OFFSET(JH7110_SYSCLK_AHB0)));
> +	clk_dm(JH7110_SYSCLK_AHB1,
> +	       starfive_clk_gate(priv->reg,
> +				 "ahb1", "stg_axiahb",
> +				 OFFSET(JH7110_SYSCLK_AHB1)));
> +	clk_dm(JH7110_SYSCLK_APB_BUS_FUNC,
> +	       starfive_clk_divider(priv->reg,
> +				    "apb_bus_func", "stg_axiahb",
> +				    OFFSET(JH7110_SYSCLK_APB_BUS_FUNC), 4));
> +	clk_dm(JH7110_SYSCLK_PCLK2_MUX_FUNC,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "pclk2_mux_func", "apb_bus_func", 1, 1));
> +	clk_dm(JH7110_SYSCLK_PCLK2_MUX,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "pclk2_mux", "pclk2_mux_func", 1, 1));
> +	clk_dm(JH7110_SYSCLK_APB_BUS,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "apb_bus", "pclk2_mux", 1, 1));
> +	clk_dm(JH7110_SYSCLK_APB0,
> +	       starfive_clk_gate(priv->reg,
> +				 "apb0", "apb_bus",
> +				 OFFSET(JH7110_SYSCLK_APB0)));
> +	clk_dm(JH7110_SYSCLK_APB12,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "apb12", "apb_bus", 1, 1));
> +	clk_dm(JH7110_SYSCLK_AON_APB,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "aon_apb", "apb_bus_func", 1, 1));
> +	clk_dm(JH7110_SYSCLK_QSPI_AHB,
> +	       starfive_clk_gate(priv->reg,
> +				 "qspi_ahb", "ahb1",
> +				 OFFSET(JH7110_SYSCLK_QSPI_AHB)));
> +	clk_dm(JH7110_SYSCLK_QSPI_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "qspi_apb", "apb12",
> +				 OFFSET(JH7110_SYSCLK_QSPI_APB)));
> +	clk_dm(JH7110_SYSCLK_QSPI_REF_SRC,
> +	       starfive_clk_divider(priv->reg,
> +				    "qspi_ref_src", "gmacusb_root",
> +				    OFFSET(JH7110_SYSCLK_QSPI_REF_SRC), 5));
> +	clk_dm(JH7110_SYSCLK_QSPI_REF,
> +	       starfive_clk_composite(priv->reg,
> +				      "qspi_ref",
> +				      qspi_ref_sels, ARRAY_SIZE(qspi_ref_sels),
> +				      OFFSET(JH7110_SYSCLK_QSPI_REF), 1, 1, 0));
> +	clk_dm(JH7110_SYSCLK_SDIO0_AHB,
> +	       starfive_clk_gate(priv->reg,
> +				 "sdio0_ahb", "ahb0",
> +				 OFFSET(JH7110_SYSCLK_SDIO0_AHB)));
> +	clk_dm(JH7110_SYSCLK_SDIO1_AHB,
> +	       starfive_clk_gate(priv->reg,
> +				 "sdio1_ahb", "ahb0",
> +				 OFFSET(JH7110_SYSCLK_SDIO1_AHB)));
> +	clk_dm(JH7110_SYSCLK_SDIO0_SDCARD,
> +	       starfive_clk_fix_parent_composite(priv->reg,
> +						 "sdio0_sdcard", "axi_cfg0",
> +						 OFFSET(JH7110_SYSCLK_SDIO0_SDCARD), 0, 1, 4));
> +	clk_dm(JH7110_SYSCLK_SDIO1_SDCARD,
> +	       starfive_clk_fix_parent_composite(priv->reg,
> +						 "sdio1_sdcard", "axi_cfg0",
> +						 OFFSET(JH7110_SYSCLK_SDIO1_SDCARD), 0, 1, 4));
> +	clk_dm(JH7110_SYSCLK_USB_125M,
> +	       starfive_clk_divider(priv->reg,
> +				    "usb_125m", "gmacusb_root",
> +				    OFFSET(JH7110_SYSCLK_USB_125M), 4));
> +	clk_dm(JH7110_SYSCLK_GMAC1_AHB,
> +	       starfive_clk_gate(priv->reg,
> +				 "gmac1_ahb", "ahb0",
> +				 OFFSET(JH7110_SYSCLK_GMAC1_AHB)));
> +	clk_dm(JH7110_SYSCLK_GMAC1_AXI,
> +	       starfive_clk_gate(priv->reg,
> +				 "gmac1_axi", "stg_axiahb",
> +				 OFFSET(JH7110_SYSCLK_GMAC1_AXI)));
> +	clk_dm(JH7110_SYSCLK_GMAC_SRC,
> +	       starfive_clk_divider(priv->reg,
> +				    "gmac_src", "gmacusb_root",
> +				    OFFSET(JH7110_SYSCLK_GMAC_SRC), 3));
> +	clk_dm(JH7110_SYSCLK_GMAC1_GTXCLK,
> +	       starfive_clk_divider(priv->reg,
> +				    "gmac1_gtxclk", "gmacusb_root",
> +				    OFFSET(JH7110_SYSCLK_GMAC1_GTXCLK), 4));
> +	clk_dm(JH7110_SYSCLK_GMAC1_GTXC,
> +	       starfive_clk_gate(priv->reg,
> +				 "gmac1_gtxc", "gmac1_gtxclk",
> +				 OFFSET(JH7110_SYSCLK_GMAC1_GTXC)));
> +	clk_dm(JH7110_SYSCLK_GMAC1_RMII_RTX,
> +	       starfive_clk_divider(priv->reg,
> +				    "gmac1_rmii_rtx", "gmac1_rmii_refin",
> +				    OFFSET(JH7110_SYSCLK_GMAC1_RMII_RTX), 5));
> +	clk_dm(JH7110_SYSCLK_GMAC1_PTP,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "gmac1_ptp", "gmac_src",
> +					 OFFSET(JH7110_SYSCLK_GMAC1_PTP), 5));
> +	clk_dm(JH7110_SYSCLK_GMAC1_TX,
> +	       starfive_clk_composite(priv->reg,
> +				      "gmac1_tx",
> +				      gmac1_tx_sels, ARRAY_SIZE(gmac1_tx_sels),
> +				      OFFSET(JH7110_SYSCLK_GMAC1_TX), 1, 1, 0));
> +	clk_dm(JH7110_SYSCLK_AON_AHB,
> +	       starfive_clk_fix_factor(priv->reg, "aon_ahb",
> +				       "stg_axiahb", 1, 1));
> +	clk_dm(JH7110_SYSCLK_GMAC0_GTXCLK,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "gmac0_gtxclk", "gmacusb_root",
> +					 OFFSET(JH7110_SYSCLK_GMAC0_GTXCLK), 4));
> +	clk_dm(JH7110_SYSCLK_GMAC0_PTP,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "gmac0_ptp", "gmac_src",
> +					 OFFSET(JH7110_SYSCLK_GMAC0_PTP), 5));
> +	clk_dm(JH7110_SYSCLK_GMAC0_GTXC,
> +	       starfive_clk_gate(priv->reg,
> +				 "gmac0_gtxc", "gmac0_gtxclk",
> +				 OFFSET(JH7110_SYSCLK_GMAC0_GTXC)));
> +	clk_dm(JH7110_SYSCLK_UART0_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart0_apb", "apb0",
> +				 OFFSET(JH7110_SYSCLK_UART0_APB)));
> +	clk_dm(JH7110_SYSCLK_UART0_CORE,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart0_core", "osc",
> +				 OFFSET(JH7110_SYSCLK_UART0_CORE)));
> +	clk_dm(JH7110_SYSCLK_UART1_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart1_apb", "apb0",
> +				 OFFSET(JH7110_SYSCLK_UART1_APB)));
> +	clk_dm(JH7110_SYSCLK_UART1_CORE,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart1_core", "osc",
> +				 OFFSET(JH7110_SYSCLK_UART1_CORE)));
> +	clk_dm(JH7110_SYSCLK_UART2_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart2_apb", "apb0",
> +				 OFFSET(JH7110_SYSCLK_UART2_APB)));
> +	clk_dm(JH7110_SYSCLK_UART2_CORE,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart2_core", "osc",
> +				 OFFSET(JH7110_SYSCLK_UART2_CORE)));
> +	clk_dm(JH7110_SYSCLK_UART3_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart3_apb", "apb0",
> +				 OFFSET(JH7110_SYSCLK_UART3_APB)));
> +	clk_dm(JH7110_SYSCLK_UART3_CORE,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "uart3_core", "perh_root",
> +					 OFFSET(JH7110_SYSCLK_UART3_CORE), 8));
> +	clk_dm(JH7110_SYSCLK_UART4_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart4_apb", "apb0",
> +				 OFFSET(JH7110_SYSCLK_UART4_APB)));
> +	clk_dm(JH7110_SYSCLK_UART4_CORE,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "uart4_core", "perh_root",
> +					 OFFSET(JH7110_SYSCLK_UART4_CORE), 8));
> +	clk_dm(JH7110_SYSCLK_UART5_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "uart5_apb", "apb0",
> +				 OFFSET(JH7110_SYSCLK_UART5_APB)));
> +	clk_dm(JH7110_SYSCLK_UART5_CORE,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "uart5_core", "perh_root",
> +					 OFFSET(JH7110_SYSCLK_UART5_CORE), 8));
> +	clk_dm(JH7110_SYSCLK_I2C5_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "i2c5_apb", "apb12",
> +				 OFFSET(JH7110_SYSCLK_I2C5_APB)));
> +	clk_dm(JH7110_SYSCLK_I2C5_CORE,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "i2c5_core", "i2c5_apb", 1, 1));
> +
> +	return 0;
> +}

Last time you said that these had to be probed in a particular order.
If that is the case, you need to enforce it. An easy way is to get the
clock parents (which are in your device tree but not used by this
driver), which will ensure that the parent gets probed.

> +static int jh7110_aoncrg_init(struct udevice *dev)
> +{
> +	struct jh7110_clk_priv *priv = dev_get_priv(dev);
> +
> +	clk_dm(JH7110_AONCLK_GMAC0_AHB,
> +	       starfive_clk_gate(priv->reg,
> +				 "gmac0_ahb", "aon_ahb",
> +				 AONOFFSET(JH7110_AONCLK_GMAC0_AHB)));
> +	clk_dm(JH7110_AONCLK_GMAC0_AXI,
> +	       starfive_clk_gate(priv->reg,
> +				 "gmac0_axi", "aon_ahb",
> +				 AONOFFSET(JH7110_AONCLK_GMAC0_AXI)));
> +	clk_dm(JH7110_AONCLK_GMAC0_RMII_RTX,
> +	       starfive_clk_divider(priv->reg,
> +				    "gmac0_rmii_rtx", "gmac0_rmii_refin",
> +				    AONOFFSET(JH7110_AONCLK_GMAC0_RMII_RTX), 5));
> +	clk_dm(JH7110_AONCLK_GMAC0_TX,
> +	       starfive_clk_composite(priv->reg,
> +				      "gmac0_tx", gmac0_tx_sels,
> +				      ARRAY_SIZE(gmac0_tx_sels),
> +				      AONOFFSET(JH7110_AONCLK_GMAC0_TX), 1, 1, 0));
> +	clk_dm(JH7110_AONCLK_OTPC_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "otpc_apb", "aon_apb",
> +				 AONOFFSET(JH7110_AONCLK_OTPC_APB)));
> +
> +	return 0;
> +}
> +
> +static int jh7110_stgcrg_init(struct udevice *dev)
> +{
> +	struct jh7110_clk_priv *priv = dev_get_priv(dev);
> +
> +	clk_dm(JH7110_STGCLK_STG_APB,
> +	       starfive_clk_fix_factor(priv->reg,
> +				       "stg_apb", "apb_bus", 1, 1));
> +	clk_dm(JH7110_STGCLK_USB_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "usb_apb", "stg_apb",
> +				 STGOFFSET(JH7110_STGCLK_USB_APB)));
> +	clk_dm(JH7110_STGCLK_USB_UTMI_APB,
> +	       starfive_clk_gate(priv->reg,
> +				 "usb_utmi_apb", "stg_apb",
> +				 STGOFFSET(JH7110_STGCLK_USB_UTMI_APB)));
> +	clk_dm(JH7110_STGCLK_USB_AXI,
> +	       starfive_clk_gate(priv->reg,
> +				 "usb_axi", "stg_axiahb",
> +				 STGOFFSET(JH7110_STGCLK_USB_AXI)));
> +	clk_dm(JH7110_STGCLK_USB_LPM,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "usb_lpm", "osc",
> +					 STGOFFSET(JH7110_STGCLK_USB_LPM), 2));
> +	clk_dm(JH7110_STGCLK_USB_STB,
> +	       starfive_clk_gate_divider(priv->reg,
> +					 "usb_stb", "osc",
> +					 STGOFFSET(JH7110_STGCLK_USB_STB), 3));
> +	clk_dm(JH7110_STGCLK_USB_APP_125,
> +	       starfive_clk_gate(priv->reg,
> +				 "usb_app_125", "usb_125m",
> +				 STGOFFSET(JH7110_STGCLK_USB_APP_125)));
> +	clk_dm(JH7110_STGCLK_USB_REFCLK,
> +	       starfive_clk_divider(priv->reg, "usb_refclk", "osc",
> +				    STGOFFSET(JH7110_STGCLK_USB_REFCLK), 2));
> +	return 0;
> +}
> +
> +static int jh7110_clk_probe(struct udevice *dev)
> +{
> +	struct jh7110_clk_priv *priv = dev_get_priv(dev);
> +
> +	priv->init = (jh1710_init_fn)dev_get_driver_data(dev);
> +	priv->reg =  (void __iomem *)dev_read_addr_ptr(dev);
> +
> +	if (priv->init)
> +		return priv->init(dev);
> +
> +	return 0;
> +}
> +
> +static int jh7110_clk_bind(struct udevice *dev)
> +{
> +	/* The reset driver does not have a device node, so bind it here */
> +	return device_bind_driver_to_node(dev, "jh7110_reset", dev->name,
> +							dev_ofnode(dev), NULL);

This will get called for syscrg, stgcrg, and aoncrg. Is that
intentional?

> +}
> +
> +static const struct udevice_id jh7110_clk_of_match[] = {
> +	{ .compatible = "starfive,jh7110-syscrg",
> +	  .data = (ulong)&jh7110_syscrg_init
> +	},
> +	{ .compatible = "starfive,jh7110-stgcrg",
> +	  .data = (ulong)&jh7110_stgcrg_init
> +	},
> +	{ .compatible = "starfive,jh7110-aoncrg",
> +	  .data = (ulong)&jh7110_aoncrg_init
> +	},
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(jh7110_clk) = {
> +	.name = "jh7110_clk",
> +	.id = UCLASS_CLK,
> +	.of_match = jh7110_clk_of_match,
> +	.probe = jh7110_clk_probe,
> +	.ops = &ccf_clk_ops,
> +	.priv_auto = sizeof(struct jh7110_clk_priv),
> +	.bind		= jh7110_clk_bind,
> +};
> diff --git a/drivers/clk/starfive/clk.h b/drivers/clk/starfive/clk.h
> new file mode 100644
> index 0000000000..eb158e6517
> --- /dev/null
> +++ b/drivers/clk/starfive/clk.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2022 Starfive, Inc.
> + * Author:	Yanhong Wang <yanhong.wang at starfivetech.com>
> + *
> + */
> +
> +#ifndef __CLK_STARFIVE_H
> +#define __CLK_STARFIVE_H
> +
> +enum starfive_pll_type {
> +	PLL0 = 0,
> +	PLL1,
> +	PLL2,
> +	PLL_MAX = PLL2
> +};
> +
> +struct starfive_pllx_rate {
> +	u64 rate;
> +	u32 prediv;
> +	u32 fbdiv;
> +	u32 frac;
> +	u32 postdiv1;
> +	u32 dacpd;
> +	u32 dsmpd;
> +};
> +
> +struct starfive_pllx_offset {
> +	u32 pd;
> +	u32 prediv;
> +	u32 fbdiv;
> +	u32 frac;
> +	u32 postdiv1;
> +	u32 dacpd;
> +	u32 dsmpd;
> +	u32 pd_mask;
> +	u32 prediv_mask;
> +	u32 fbdiv_mask;
> +	u32 frac_mask;
> +	u32 postdiv1_mask;
> +	u32 dacpd_mask;
> +	u32 dsmpd_mask;
> +};
> +
> +struct starfive_pllx_clk {
> +	enum starfive_pll_type type;
> +	const struct starfive_pllx_offset *offset;
> +	const struct starfive_pllx_rate *rate_table;
> +	int rate_count;
> +	int flags;
> +};
> +
> +extern struct starfive_pllx_clk starfive_jh7110_pll0;
> +extern struct starfive_pllx_clk starfive_jh7110_pll1;
> +extern struct starfive_pllx_clk starfive_jh7110_pll2;
> +
> +struct clk *starfive_jh7110_pll(const char *name, const char *parent_name,
> +				void __iomem *base, void __iomem *sysreg,
> +				const struct starfive_pllx_clk *pll_clk);
> +#endif



More information about the U-Boot mailing list