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

yanhong wang yanhong.wang at starfivetech.com
Mon Jan 9 10:31:25 CET 2023



On 2023/1/5 3:13, Sean Anderson wrote:
> On 12/11/22 21:50, 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          |  15 +
>>   drivers/clk/starfive/Makefile         |   4 +
>>   drivers/clk/starfive/clk-jh7110-pll.c | 362 ++++++++++++++
>>   drivers/clk/starfive/clk-jh7110.c     | 651 ++++++++++++++++++++++++++
>>   drivers/clk/starfive/clk.h            |  42 ++
>>   7 files changed, 1076 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..e4bf2a5c5e
>> --- /dev/null
>> +++ b/drivers/clk/starfive/Kconfig
>> @@ -0,0 +1,15 @@
>> +# 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 CLK && CLK_CCF && STARFIVE_JH7110
>> +    help
>> +      This enables support clock driver for StarFive JH7110 SoC platform.
> 
> Not a huge fan of CCF drivers, but whatever works for you.
> 

Thanks, i will fix.

>> 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..8be9500b62
>> --- /dev/null
>> +++ b/drivers/clk/starfive/clk-jh7110-pll.c
>> @@ -0,0 +1,362 @@
>> +// 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 <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 PLL0_DACPD_MASK    BIT(24)
>> +#define PLL0_DSMPD_MASK    BIT(25)
>> +#define PLL0_FBDIV_MASK    GENMASK(11, 0)
>> +#define PLL0_FRAC_MASK        GENMASK(23, 0)
>> +#define PLL0_PD_MASK        BIT(27)
>> +#define PLL0_POSTDIV1_MASK    GENMASK(29, 28)
>> +#define PLL0_PREDIV_MASK    GENMASK(5, 0)
>> +#define PLL1_DACPD_MASK    BIT(15)
>> +#define PLL1_DSMPD_MASK    BIT(16)
>> +#define PLL1_FBDIV_MASK    GENMASK(28, 17)
>> +#define PLL1_FRAC_MASK        GENMASK(23, 0)
>> +#define PLL1_PD_MASK        BIT(27)
>> +#define PLL1_POSTDIV1_MASK    GENMASK(29, 28)
>> +#define PLL1_PREDIV_MASK    GENMASK(5, 0)
>> +#define PLL2_DACPD_MASK    BIT(15)
>> +#define PLL2_DSMPD_MASK    BIT(16)
>> +#define PLL2_FBDIV_MASK    GENMASK(28, 17)
>> +#define PLL2_FRAC_MASK        GENMASK(23, 0)
>> +#define PLL2_PD_MASK        BIT(27)
>> +#define PLL2_POSTDIV1_MASK    GENMASK(29, 28)
>> +#define PLL2_PREDIV_MASK    GENMASK(5, 0)
>> +
>> +#define PLL0_DACPD_OFFSET    0x18
>> +#define PLL0_DSMPD_OFFSET    0x18
>> +#define PLL0_FBDIV_OFFSET    0x1C
>> +#define PLL0_FRAC_OFFSET    0x20
>> +#define PLL0_PD_OFFSET        0x20
>> +#define PLL0_POSTDIV1_OFFSET    0x20
>> +#define PLL0_PREDIV_OFFSET    0x24
>> +#define PLL1_DACPD_OFFSET    0x24
>> +#define PLL1_DSMPD_OFFSET    0x24
>> +#define PLL1_FBDIV_OFFSET    0x24
>> +#define PLL1_FRAC_OFFSET    0x28
>> +#define PLL1_PD_OFFSET        0x28
>> +#define PLL1_POSTDIV1_OFFSET    0x28
>> +#define PLL1_PREDIV_OFFSET    0x2c
>> +#define PLL2_DACPD_OFFSET    0x2c
>> +#define PLL2_DSMPD_OFFSET    0x2c
>> +#define PLL2_FBDIV_OFFSET    0x2c
>> +#define PLL2_FRAC_OFFSET    0x30
>> +#define PLL2_PD_OFFSET        0x30
>> +#define PLL2_POSTDIV1_OFFSET    0x30
>> +#define PLL2_PREDIV_OFFSET    0x34
>> +
>> +#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_rate *rate_table;
>> +    int rate_count;
>> +};
>> +
>> +#define getbits_le32(addr, mask) ((in_le32(addr) & (mask)) >> __ffs((mask)))
>> +
>> +#define GET_PLL(index, type) \
>> +        getbits_le32((ulong)pll->base + index##_##type##_OFFSET, index##_##type##_MASK)
>> +
>> +#define SET_PLL(index, type, val) \
>> +        clrsetbits_le32((ulong)pll->base + index##_##type##_OFFSET, \
>> +            index##_##type##_MASK, \
>> +            ((val) << __ffs(index##_##type##_MASK)) & index##_##type##_MASK)
>> +
>> +#define CLK_DDR_REG_SET(type, val) \
>> +        clrsetbits_le32((ulong)pll->sysreg + CLK_DDR_##type##_OFFSET, \
>> +            CLK_DDR_##type##_MASK, \
>> +            ((val) << __ffs(CLK_DDR_##type##_MASK)) & CLK_DDR_##type##_MASK)
>> +
>> +#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),
>> +};
> 
> All of these rates set postdiv1/dacpd/dsmpd to 1. Do these fields need
> to be stored?
> 
The PLL supports integer and fraction muligple, you should set dacpd and dsmpd to high while integer multiple mode,
and set both them to low while fraction multiple mode. The default configration set to integer multiple mode.

Integer Multiple Mode

Both dacpd and dsmpd should be set as 1 while integer multiple mode.

The frequency of outputs can be figured out as below.

            Fvco = Fref*Nl/M
NI is integer frequency dividing ratio of feedback divider, set by fbdiv1[11:0] , NI = 8, 9, 10, 12.13....4095
M is frequency dividing ratio of pre-divider, set by prediv[5:0],M = 1,2...63

           Fclko1 = Fvco/Q1
Q1 is frequency dividing ratio of post divider, set by postdiv1[1:0],Q1= 1,2,4,8


Fraction Multiple Mode

Both dacpd and dsmpd should be set as 0 while integer multiple mode.

           Fvco = Fref*(NI+NE)/M
NI is integer frequency dividing ratio of feedback divider, set by fbdiv[11:0] , NI = 8, 9, 10, 12.13....4095
NF is fractional frequency dividing ratio, set by frac[23:0],  NF =frac[23:0]/2^24= 0~0.99999994
M is frequency dividing ratio of pre-divider, set by prediv[5:0],M = 1,2...63

          Fclko1 = Fvco/Q1
Q1 is frequency dividing ratio of post divider, set by postdivl[1:0],Q1= 1,24,8

>> +struct starfive_pllx_clk starfive_jh7110_pll0 __initdata = {
>> +    .type = PLL0,
>> +    .rate_table = jh7110_pll0_tbl,
>> +    .rate_count = ARRAY_SIZE(jh7110_pll0_tbl),
>> +};
>> +EXPORT_SYMBOL_GPL(starfive_jh7110_pll0);
>> +
>> +struct starfive_pllx_clk starfive_jh7110_pll1 __initdata = {
>> +    .type = PLL1,
>> +    .rate_table = jh7110_pll1_tbl,
>> +    .rate_count = ARRAY_SIZE(jh7110_pll1_tbl),
>> +};
>> +EXPORT_SYMBOL_GPL(starfive_jh7110_pll1);
>> +
>> +struct starfive_pllx_clk starfive_jh7110_pll2 __initdata = {
>> +    .type = PLL2,
>> +    .rate_table = jh7110_pll2_tbl,
>> +    .rate_count = ARRAY_SIZE(jh7110_pll2_tbl),
>> +};
>> +EXPORT_SYMBOL_GPL(starfive_jh7110_pll2);
>> +
>> +static const struct starfive_pllx_rate *jh7110_get_pll_settings
>> +    (struct clk_jh7110_pllx *pll, unsigned long rate)
> 
> Please break this like
> 
> static const struct starfive_pllx_rate *
> jh7110_get_pll_settings(struct clk_jh7110_pllx *pll, unsigned long rate)
> 

I will fix.

>> +{
>> +    const struct starfive_pllx_rate *rate_table = pll->rate_table;
>> +    int i;
>> +
>> +    for (i = 0; i < pll->rate_count; i++)
>> +        if (rate == rate_table[i].rate)
>> +            return &rate_table[i];
> 
> Just do
> 
>         if (rate == pll->ate_table[i].rate)
>             return &pll->rate_table[i];
> 
> and skip the local variable.
> 

I will fix.

>> +
>> +    return NULL;
>> +}
>> +
>> +static void jh7110_pll_set_rate(struct clk_jh7110_pllx *pll,
>> +                const struct starfive_pllx_rate *rate)
>> +{
>> +    switch (pll->type) {
>> +    case PLL0:
>> +        SET_PLL(PLL0, PD, PLL_PD_OFF);
>> +        SET_PLL(PLL0, DACPD, rate->dacpd);
>> +        SET_PLL(PLL0, DSMPD, rate->dsmpd);
>> +        SET_PLL(PLL0, PREDIV, rate->prediv);
>> +        SET_PLL(PLL0, FBDIV, rate->fbdiv);
>> +        SET_PLL(PLL0, POSTDIV1, rate->postdiv1 >> 1);
>> +        SET_PLL(PLL0, PD, PLL_PD_ON);
>> +        break;
> 
> This is not idiomatic. A more conventional approach is e.g.
> 
>     reg = readl(pll->sysreg + PLL_FOO);
>     reg &= ~PLL_FOO_BAR;
>     reg |= FIELD_PREP(PLL_FOO_BAR, rate->bar);
>     writel(pll->sysreg + PLL_FOO, reg);
> 
> Especially since it is not obvious that some of these fields share the
> same register.

Thanks, i will fix.

> 
>> +    case PLL1:
>> +        CLK_DDR_REG_SET(BUS, CLK_DDR_BUS_OSC_DIV2);
>> +        SET_PLL(PLL1, PD, PLL_PD_OFF);
>> +        SET_PLL(PLL1, DACPD, rate->dacpd);
>> +        SET_PLL(PLL1, DSMPD, rate->dsmpd);
>> +        SET_PLL(PLL1, PREDIV, rate->prediv);
>> +        SET_PLL(PLL1, FBDIV, rate->fbdiv);
>> +        SET_PLL(PLL1, POSTDIV1, rate->postdiv1 >> 1);
>> +        SET_PLL(PLL1, PD, PLL_PD_ON);
>> +        udelay(100);
>> +        CLK_DDR_REG_SET(BUS, CLK_DDR_BUS_PLL1_DIV2);
>> +        break;
>> +
>> +    case PLL2:
>> +        SET_PLL(PLL2, PD, PLL_PD_OFF);
>> +        SET_PLL(PLL2, DACPD, rate->dacpd);
>> +        SET_PLL(PLL2, DSMPD, rate->dsmpd);
>> +        SET_PLL(PLL2, PREDIV, rate->prediv);
>> +        SET_PLL(PLL2, FBDIV, rate->fbdiv);
>> +        SET_PLL(PLL2, POSTDIV1, rate->postdiv1 >> 1);
>> +        SET_PLL(PLL2, PD, PLL_PD_ON);
>> +        break;
> 
> It looks like all of these PLLs have identical fields. Can you make
> sysreg the base of the PLL? E.g. in starfive_jh7110_pll have
> 
>     pll->sysreg = sysreg + pll_clk->base;
> 
> and then in this function you don't have to have a switch statement.
> 
Thanks, i will use this sysreg strategy in the next version.

>> +    default:
>> +        break;
>> +    }
>> +}
>> +
>> +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;
>> +
>> +    switch (pll->type) {
>> +    case PLL0:
>> +        dacpd = GET_PLL(PLL0, DACPD);
>> +        dsmpd = GET_PLL(PLL0, DSMPD);
>> +        prediv = GET_PLL(PLL0, PREDIV);
>> +        fbdiv = GET_PLL(PLL0, FBDIV);
>> +        postdiv1 = 1 << GET_PLL(PLL0, POSTDIV1);
> 
> Why isn't this the same calculation as above?
> 

Please refer to the previous description for details.

>> +        frac = (u64)GET_PLL(PLL0, FRAC);
>> +        break;
>> +
>> +    case PLL1:
>> +        dacpd = GET_PLL(PLL1, DACPD);
>> +        dsmpd = GET_PLL(PLL1, DSMPD);
>> +        prediv = GET_PLL(PLL1, PREDIV);
>> +        fbdiv = GET_PLL(PLL1, FBDIV);
>> +        postdiv1 = 1 << GET_PLL(PLL1, POSTDIV1);
>> +        frac = (u64)GET_PLL(PLL1, FRAC);
>> +        break;
>> +
>> +    case PLL2:
>> +        dacpd = GET_PLL(PLL2, DACPD);
>> +        dsmpd = GET_PLL(PLL2, DSMPD);
>> +        prediv = GET_PLL(PLL2, PREDIV);
>> +        fbdiv = GET_PLL(PLL2, FBDIV);
>> +        postdiv1 = 1 << GET_PLL(PLL2, POSTDIV1);
>> +        frac = (u64)GET_PLL(PLL2, FRAC);
>> +        break;
>> +
>> +    default:
>> +        debug("Unsupported pll type %d.\n", pll->type);
>> +        return 0;
>> +    }
>> +
>> +    /* Integer Mode or Fraction Mode */
>> +    if (dacpd == 1 && dsmpd == 1)
>> +        frac = 0;
>> +    else if (dacpd == 0 && dsmpd == 0)
>> +        do_div(frac, 1 << 24);
> 
> Where does this constant come from?
>
   NF =frac[23:0]/2^24= 0~0.99999994,    2^24 = 1 << 24

Please refer to the previous description for details.
 
>> +    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;
>> +
>> +    switch (pll->type) {
>> +    case PLL0:
>> +    case PLL1:
>> +    case PLL2:
>> +        break;
>> +
>> +    default:
>> +        debug("%s: Unknown pll clk type\n", __func__);
>> +        return -EINVAL;
>> +    };
> 
> And if you use this sysreg strategy you can remove checks like this.
>
Thanks, i will use this sysreg strategy in the next version.

>> +    rate = jh7110_get_pll_settings(pll, drate);
>> +    if (!rate)
>> +        return -EINVAL;
>> +
>> +    jh7110_pll_set_rate(pll, rate);
>> +
>> +    return jh7110_pllx_recalc_rate(clk);
>> +}
>> +
>> +static void jh7110_pllx_init_rate(struct clk_jh7110_pllx *pll, ulong drate)
>> +{
>> +    const struct starfive_pllx_rate *rate;
>> +
>> +    rate = jh7110_get_pll_settings(pll, drate);
>> +    if (!rate)
>> +        return;
>> +
>> +    jh7110_pll_set_rate(pll, rate);
>> +}
> 
> Can we reuse jh7110_pllx_set_rate for this purpose?
> 

I will fix.

>> +
>> +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;
>> +    char *type_name;
>> +    int ret;
>> +
>> +    switch (pll_clk->type) {
>> +    case PLL0:
>> +    case PLL1:
>> +    case PLL2:
>> +        type_name = UBOOT_DM_CLK_JH7110_PLLX;
>> +        break;
>> +
>> +    default:
>> +        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->rate_table = pll_clk->rate_table;
>> +    pll->rate_count = pll_clk->rate_count;
>> +
>> +    clk = &pll->clk;
>> +    ret = clk_register(clk, type_name, name, parent_name);
>> +    if (ret) {
>> +        kfree(pll);
>> +        return ERR_PTR(ret);
>> +    }
>> +
>> +    if (IS_ENABLED(CONFIG_SPL_BUILD) && pll->type == PLL0)
>> +        jh7110_pllx_init_rate(pll, 1250000000);
>> +
>> +    if (IS_ENABLED(CONFIG_SPL_BUILD) && pll->type == PLL2)
>> +        jh7110_pllx_init_rate(pll, 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..223adcc904
>> --- /dev/null
>> +++ b/drivers/clk/starfive/clk-jh7110.c
>> @@ -0,0 +1,651 @@
>> +// 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] */
> 
> Unused.
> 

I will fix.

>> +#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)
>> +
>> +struct jh7110_clk_config {
>> +    int (*init)(struct udevice *dev);
>> +};
> 
> Just use a typedef.
> 

I will fix.

>> +
>> +struct jh7110_clk_priv {
>> +    void __iomem *reg;
>> +    const struct jh7110_clk_config *config;
>> +};
>> +
>> +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 ulong starfive_clk_get_rate(struct clk *clk)
>> +{
>> +    struct clk *c;
>> +    int ret;
>> +
>> +    ret = clk_get_by_id(clk->id, &c);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return clk_get_rate(c);
>> +}
>> +
>> +static ulong starfive_clk_set_rate(struct clk *clk, unsigned long rate)
>> +{
>> +    struct clk *c;
>> +    int ret;
>> +
>> +    ret = clk_get_by_id(clk->id, &c);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return clk_set_rate(c, rate);
>> +}
>> +
>> +static int __starfive_clk_enable(struct clk *clk, bool enable)
>> +{
>> +    struct clk *c;
>> +    int ret;
>> +
>> +    ret = clk_get_by_id(clk->id, &c);
>> +    if (ret)
>> +        return ret;
>> +
>> +    if (enable)
>> +        ret = clk_enable(c);
>> +    else
>> +        ret = clk_disable(c);
>> +
>> +    return ret;
>> +}
>> +
>> +static int starfive_clk_disable(struct clk *clk)
>> +{
>> +    return __starfive_clk_enable(clk, 0);
>> +}
>> +
>> +static int starfive_clk_enable(struct clk *clk)
>> +{
>> +    return __starfive_clk_enable(clk, 1);
>> +}
>> +
>> +static int starfive_clk_set_parent(struct clk *clk, struct clk *parent)
>> +{
>> +    struct clk *c, *cp;
>> +    int ret;
>> +
>> +    ret = clk_get_by_id(clk->id, &c);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = clk_get_by_id(parent->id, &cp);
>> +    if (ret)
>> +        return ret;
>> +
>> +    ret = clk_set_parent(c, cp);
>> +    c->dev->parent = cp->dev;
>> +
>> +    return ret;
>> +}
>> +
>> +static const struct clk_ops starfive_clk_ops = {
>> +    .set_rate = starfive_clk_set_rate,
>> +    .get_rate = starfive_clk_get_rate,
>> +    .enable = starfive_clk_enable,
>> +    .disable = starfive_clk_disable,
>> +    .set_parent = starfive_clk_set_parent,
>> +};
> 
> Use ccf_clk_ops.
> 

I will fix.

>> +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;
>> +
>> +    addr =  ofnode_get_addr(args.node);
>> +    if (addr == FDT_ADDR_T_NONE)
>> +        return -EINVAL;
> 
> This does not match the binding submitted to Linux [1]. Use
> dev_read_addr_ptr.
> 
> [1] https://lore.kernel.org/linux-clk/20221220005054.34518-8-hal.feng@starfivetech.com/
> 

The binding submitted to Linux doesn't include pll clock, so it does not match. 

>> +
>> +    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;
>> +}
>> +
>> +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));
> 
> Does this stuff need to be probed in a particular order?
> 

Yes, some clks have dependencies, otherwise, it will return error when call clk_register.

>> +    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->config = (void *)dev_get_driver_data(dev);
>> +    priv->reg =  (void __iomem *)ofnode_get_addr(dev_ofnode(dev));
>> +
>> +    if (priv->config->init)
>> +        return priv->config->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);
>> +}
>> +
>> +struct jh7110_clk_config __maybe_unused jh7110_clk_syscrg = {
>> +    .init = jh7110_syscrg_init
>> +};
>> +
>> +struct jh7110_clk_config __maybe_unused jh7110_clk_stgcrg = {
>> +    .init = jh7110_stgcrg_init
>> +};
>> +
>> +struct jh7110_clk_config __maybe_unused jh7110_clk_aoncrg = {
>> +    .init = jh7110_aoncrg_init
>> +};
>> +
>> +static const struct udevice_id jh7110_clk_of_match[] = {
>> +    { .compatible = "starfive,jh7110-syscrg",
>> +      .data = (ulong)&jh7110_clk_syscrg
> 
> You can just do
> 
> .data = (ulong)jh7110_syscrg_init,
> 

I will fix.

> --Sean
> 
>> +    },
>> +    { .compatible = "starfive,jh7110-stgcrg",
>> +      .data = (ulong)&jh7110_clk_stgcrg
>> +    },
>> +    { .compatible = "starfive,jh7110-aoncrg",
>> +      .data = (ulong)&jh7110_clk_aoncrg
>> +    },
>> +    { }
>> +};
>> +
>> +U_BOOT_DRIVER(jh7110_clk) = {
>> +    .name = "jh7110_clk",
>> +    .id = UCLASS_CLK,
>> +    .of_match = jh7110_clk_of_match,
>> +    .probe = jh7110_clk_probe,
>> +    .ops = &starfive_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..d86280d523
>> --- /dev/null
>> +++ b/drivers/clk/starfive/clk.h
>> @@ -0,0 +1,42 @@
>> +/* 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_clk {
>> +    enum starfive_pll_type type;
>> +    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