| Message ID | 20230307081326.13359-1-JJLIU0@nuvoton.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Sean Anderson |
| Headers | show |
| Series | [v1] clk: nuvoton: add read only feature for clk driver | expand |
On 3/7/23 03:13, Jim Liu wrote: > Set ahb/apb/fiu clock divider as read-only Are these read-only in hardware, or is there some other reason? --Sean > Signed-off-by: Jim Liu <JJLIU0@nuvoton.com> > --- > drivers/clk/nuvoton/clk_npcm.c | 15 ++++++++++++--- > drivers/clk/nuvoton/clk_npcm.h | 1 + > drivers/clk/nuvoton/clk_npcm8xx.c | 12 ++++++------ > 3 files changed, 19 insertions(+), 9 deletions(-) > > diff --git a/drivers/clk/nuvoton/clk_npcm.c b/drivers/clk/nuvoton/clk_npcm.c > index 8d71f2a24b..18cb9cddbf 100644 > --- a/drivers/clk/nuvoton/clk_npcm.c > +++ b/drivers/clk/nuvoton/clk_npcm.c > @@ -135,7 +135,7 @@ static u32 npcm_clk_get_div(struct clk *clk) > return div; > } > > -static u32 npcm_clk_set_div(struct clk *clk, u32 div) > +static int npcm_clk_set_div(struct clk *clk, u32 div) > { > struct npcm_clk_priv *priv = dev_get_priv(clk->dev); > struct npcm_clk_div *divider; > @@ -145,6 +145,9 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) > if (!divider) > return -EINVAL; > > + if (divider->flags & DIV_RO) > + return 0; > + > if (divider->flags & PRE_DIV2) > div = div >> 1; > > @@ -153,6 +156,12 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) > else > clkdiv = ilog2(div); > > + if (clkdiv > (divider->mask >> (ffs(divider->mask) - 1))) { > + printf("clkdiv(%d) for clk(%ld) is over limit\n", > + clkdiv, clk->id); > + return -EINVAL; > + } > + > val = readl(priv->base + divider->reg); > val &= ~divider->mask; > val |= (clkdiv << (ffs(divider->mask) - 1)) & divider->mask; > @@ -253,8 +262,8 @@ static ulong npcm_clk_set_rate(struct clk *clk, ulong rate) > if (ret) > return ret; > > - debug("%s: rate %lu, new rate (%lu / %u)\n", __func__, rate, parent_rate, div); > - return (parent_rate / div); > + debug("%s: rate %lu, new rate %lu\n", __func__, rate, npcm_clk_get_rate(clk)); > + return npcm_clk_get_rate(clk); > } > > static int npcm_clk_set_parent(struct clk *clk, struct clk *parent) > diff --git a/drivers/clk/nuvoton/clk_npcm.h b/drivers/clk/nuvoton/clk_npcm.h > index 06b60dc8b8..b4726d8381 100644 > --- a/drivers/clk/nuvoton/clk_npcm.h > +++ b/drivers/clk/nuvoton/clk_npcm.h > @@ -50,6 +50,7 @@ > #define PRE_DIV2 BIT(2) /* Pre divisor = 2 */ > #define POST_DIV2 BIT(3) /* Post divisor = 2 */ > #define FIXED_PARENT BIT(4) /* clock source is fixed */ > +#define DIV_RO BIT(5) /* divider is read-only */ > > /* Parameters of PLL configuration */ > struct npcm_clk_pll { > diff --git a/drivers/clk/nuvoton/clk_npcm8xx.c b/drivers/clk/nuvoton/clk_npcm8xx.c > index 27e3cfcf55..d1b32e3237 100644 > --- a/drivers/clk/nuvoton/clk_npcm8xx.c > +++ b/drivers/clk/nuvoton/clk_npcm8xx.c > @@ -45,12 +45,12 @@ static struct npcm_clk_select npcm8xx_clk_selectors[] = { > }; > > static struct npcm_clk_div npcm8xx_clk_dividers[] = { > - {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2}, > - {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2}, > - {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2}, > - {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1}, > - {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1}, > - {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1}, > + {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2 | DIV_RO}, > + {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2 | DIV_RO}, > + {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2 | DIV_RO}, > + {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1 | DIV_RO}, > + {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1 | DIV_RO}, > + {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1 | DIV_RO}, > {NPCM8XX_CLK_SPIX, CLKDIV3, SPIXCKDIV, DIV_TYPE1}, > {NPCM8XX_CLK_UART, CLKDIV1, UARTDIV1, DIV_TYPE1}, > {NPCM8XX_CLK_UART2, CLKDIV3, UARTDIV2, DIV_TYPE1},
Hi Sean Thanks for your review. This is a software flag not hardware reg. It just protects the clock source and can't modify it in uboot. Best regards, Jim On Tue, Mar 7, 2023 at 9:57 PM Sean Anderson <seanga2@gmail.com> wrote: > > On 3/7/23 03:13, Jim Liu wrote: > > Set ahb/apb/fiu clock divider as read-only > > Are these read-only in hardware, or is there some other reason? > > --Sean > > > Signed-off-by: Jim Liu <JJLIU0@nuvoton.com> > > --- > > drivers/clk/nuvoton/clk_npcm.c | 15 ++++++++++++--- > > drivers/clk/nuvoton/clk_npcm.h | 1 + > > drivers/clk/nuvoton/clk_npcm8xx.c | 12 ++++++------ > > 3 files changed, 19 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/clk/nuvoton/clk_npcm.c b/drivers/clk/nuvoton/clk_npcm.c > > index 8d71f2a24b..18cb9cddbf 100644 > > --- a/drivers/clk/nuvoton/clk_npcm.c > > +++ b/drivers/clk/nuvoton/clk_npcm.c > > @@ -135,7 +135,7 @@ static u32 npcm_clk_get_div(struct clk *clk) > > return div; > > } > > > > -static u32 npcm_clk_set_div(struct clk *clk, u32 div) > > +static int npcm_clk_set_div(struct clk *clk, u32 div) > > { > > struct npcm_clk_priv *priv = dev_get_priv(clk->dev); > > struct npcm_clk_div *divider; > > @@ -145,6 +145,9 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) > > if (!divider) > > return -EINVAL; > > > > + if (divider->flags & DIV_RO) > > + return 0; > > + > > if (divider->flags & PRE_DIV2) > > div = div >> 1; > > > > @@ -153,6 +156,12 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) > > else > > clkdiv = ilog2(div); > > > > + if (clkdiv > (divider->mask >> (ffs(divider->mask) - 1))) { > > + printf("clkdiv(%d) for clk(%ld) is over limit\n", > > + clkdiv, clk->id); > > + return -EINVAL; > > + } > > + > > val = readl(priv->base + divider->reg); > > val &= ~divider->mask; > > val |= (clkdiv << (ffs(divider->mask) - 1)) & divider->mask; > > @@ -253,8 +262,8 @@ static ulong npcm_clk_set_rate(struct clk *clk, ulong rate) > > if (ret) > > return ret; > > > > - debug("%s: rate %lu, new rate (%lu / %u)\n", __func__, rate, parent_rate, div); > > - return (parent_rate / div); > > + debug("%s: rate %lu, new rate %lu\n", __func__, rate, npcm_clk_get_rate(clk)); > > + return npcm_clk_get_rate(clk); > > } > > > > static int npcm_clk_set_parent(struct clk *clk, struct clk *parent) > > diff --git a/drivers/clk/nuvoton/clk_npcm.h b/drivers/clk/nuvoton/clk_npcm.h > > index 06b60dc8b8..b4726d8381 100644 > > --- a/drivers/clk/nuvoton/clk_npcm.h > > +++ b/drivers/clk/nuvoton/clk_npcm.h > > @@ -50,6 +50,7 @@ > > #define PRE_DIV2 BIT(2) /* Pre divisor = 2 */ > > #define POST_DIV2 BIT(3) /* Post divisor = 2 */ > > #define FIXED_PARENT BIT(4) /* clock source is fixed */ > > +#define DIV_RO BIT(5) /* divider is read-only */ > > > > /* Parameters of PLL configuration */ > > struct npcm_clk_pll { > > diff --git a/drivers/clk/nuvoton/clk_npcm8xx.c b/drivers/clk/nuvoton/clk_npcm8xx.c > > index 27e3cfcf55..d1b32e3237 100644 > > --- a/drivers/clk/nuvoton/clk_npcm8xx.c > > +++ b/drivers/clk/nuvoton/clk_npcm8xx.c > > @@ -45,12 +45,12 @@ static struct npcm_clk_select npcm8xx_clk_selectors[] = { > > }; > > > > static struct npcm_clk_div npcm8xx_clk_dividers[] = { > > - {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2}, > > - {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2}, > > - {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2}, > > - {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1}, > > - {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1}, > > - {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1}, > > + {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2 | DIV_RO}, > > + {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2 | DIV_RO}, > > + {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2 | DIV_RO}, > > + {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1 | DIV_RO}, > > + {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1 | DIV_RO}, > > + {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1 | DIV_RO}, > > {NPCM8XX_CLK_SPIX, CLKDIV3, SPIXCKDIV, DIV_TYPE1}, > > {NPCM8XX_CLK_UART, CLKDIV1, UARTDIV1, DIV_TYPE1}, > > {NPCM8XX_CLK_UART2, CLKDIV3, UARTDIV2, DIV_TYPE1}, >
On 3/7/23 22:23, Jim Liu wrote: > Hi Sean > > Thanks for your review. > This is a software flag not hardware reg. > It just protects the clock source and can't modify it in uboot. OK, can you add that to the commit message and resubmit. --Sean > Best regards, > Jim > > On Tue, Mar 7, 2023 at 9:57 PM Sean Anderson <seanga2@gmail.com> wrote: >> >> On 3/7/23 03:13, Jim Liu wrote: >>> Set ahb/apb/fiu clock divider as read-only >> >> Are these read-only in hardware, or is there some other reason? >> >> --Sean >> >>> Signed-off-by: Jim Liu <JJLIU0@nuvoton.com> >>> --- >>> drivers/clk/nuvoton/clk_npcm.c | 15 ++++++++++++--- >>> drivers/clk/nuvoton/clk_npcm.h | 1 + >>> drivers/clk/nuvoton/clk_npcm8xx.c | 12 ++++++------ >>> 3 files changed, 19 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/clk/nuvoton/clk_npcm.c b/drivers/clk/nuvoton/clk_npcm.c >>> index 8d71f2a24b..18cb9cddbf 100644 >>> --- a/drivers/clk/nuvoton/clk_npcm.c >>> +++ b/drivers/clk/nuvoton/clk_npcm.c >>> @@ -135,7 +135,7 @@ static u32 npcm_clk_get_div(struct clk *clk) >>> return div; >>> } >>> >>> -static u32 npcm_clk_set_div(struct clk *clk, u32 div) >>> +static int npcm_clk_set_div(struct clk *clk, u32 div) >>> { >>> struct npcm_clk_priv *priv = dev_get_priv(clk->dev); >>> struct npcm_clk_div *divider; >>> @@ -145,6 +145,9 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) >>> if (!divider) >>> return -EINVAL; >>> >>> + if (divider->flags & DIV_RO) >>> + return 0; >>> + >>> if (divider->flags & PRE_DIV2) >>> div = div >> 1; >>> >>> @@ -153,6 +156,12 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) >>> else >>> clkdiv = ilog2(div); >>> >>> + if (clkdiv > (divider->mask >> (ffs(divider->mask) - 1))) { >>> + printf("clkdiv(%d) for clk(%ld) is over limit\n", >>> + clkdiv, clk->id); >>> + return -EINVAL; >>> + } >>> + >>> val = readl(priv->base + divider->reg); >>> val &= ~divider->mask; >>> val |= (clkdiv << (ffs(divider->mask) - 1)) & divider->mask; >>> @@ -253,8 +262,8 @@ static ulong npcm_clk_set_rate(struct clk *clk, ulong rate) >>> if (ret) >>> return ret; >>> >>> - debug("%s: rate %lu, new rate (%lu / %u)\n", __func__, rate, parent_rate, div); >>> - return (parent_rate / div); >>> + debug("%s: rate %lu, new rate %lu\n", __func__, rate, npcm_clk_get_rate(clk)); >>> + return npcm_clk_get_rate(clk); >>> } >>> >>> static int npcm_clk_set_parent(struct clk *clk, struct clk *parent) >>> diff --git a/drivers/clk/nuvoton/clk_npcm.h b/drivers/clk/nuvoton/clk_npcm.h >>> index 06b60dc8b8..b4726d8381 100644 >>> --- a/drivers/clk/nuvoton/clk_npcm.h >>> +++ b/drivers/clk/nuvoton/clk_npcm.h >>> @@ -50,6 +50,7 @@ >>> #define PRE_DIV2 BIT(2) /* Pre divisor = 2 */ >>> #define POST_DIV2 BIT(3) /* Post divisor = 2 */ >>> #define FIXED_PARENT BIT(4) /* clock source is fixed */ >>> +#define DIV_RO BIT(5) /* divider is read-only */ >>> >>> /* Parameters of PLL configuration */ >>> struct npcm_clk_pll { >>> diff --git a/drivers/clk/nuvoton/clk_npcm8xx.c b/drivers/clk/nuvoton/clk_npcm8xx.c >>> index 27e3cfcf55..d1b32e3237 100644 >>> --- a/drivers/clk/nuvoton/clk_npcm8xx.c >>> +++ b/drivers/clk/nuvoton/clk_npcm8xx.c >>> @@ -45,12 +45,12 @@ static struct npcm_clk_select npcm8xx_clk_selectors[] = { >>> }; >>> >>> static struct npcm_clk_div npcm8xx_clk_dividers[] = { >>> - {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2}, >>> - {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2}, >>> - {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2}, >>> - {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1}, >>> - {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1}, >>> - {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1}, >>> + {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2 | DIV_RO}, >>> + {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2 | DIV_RO}, >>> + {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2 | DIV_RO}, >>> + {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1 | DIV_RO}, >>> + {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1 | DIV_RO}, >>> + {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1 | DIV_RO}, >>> {NPCM8XX_CLK_SPIX, CLKDIV3, SPIXCKDIV, DIV_TYPE1}, >>> {NPCM8XX_CLK_UART, CLKDIV1, UARTDIV1, DIV_TYPE1}, >>> {NPCM8XX_CLK_UART2, CLKDIV3, UARTDIV2, DIV_TYPE1}, >>
diff --git a/drivers/clk/nuvoton/clk_npcm.c b/drivers/clk/nuvoton/clk_npcm.c index 8d71f2a24b..18cb9cddbf 100644 --- a/drivers/clk/nuvoton/clk_npcm.c +++ b/drivers/clk/nuvoton/clk_npcm.c @@ -135,7 +135,7 @@ static u32 npcm_clk_get_div(struct clk *clk) return div; } -static u32 npcm_clk_set_div(struct clk *clk, u32 div) +static int npcm_clk_set_div(struct clk *clk, u32 div) { struct npcm_clk_priv *priv = dev_get_priv(clk->dev); struct npcm_clk_div *divider; @@ -145,6 +145,9 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) if (!divider) return -EINVAL; + if (divider->flags & DIV_RO) + return 0; + if (divider->flags & PRE_DIV2) div = div >> 1; @@ -153,6 +156,12 @@ static u32 npcm_clk_set_div(struct clk *clk, u32 div) else clkdiv = ilog2(div); + if (clkdiv > (divider->mask >> (ffs(divider->mask) - 1))) { + printf("clkdiv(%d) for clk(%ld) is over limit\n", + clkdiv, clk->id); + return -EINVAL; + } + val = readl(priv->base + divider->reg); val &= ~divider->mask; val |= (clkdiv << (ffs(divider->mask) - 1)) & divider->mask; @@ -253,8 +262,8 @@ static ulong npcm_clk_set_rate(struct clk *clk, ulong rate) if (ret) return ret; - debug("%s: rate %lu, new rate (%lu / %u)\n", __func__, rate, parent_rate, div); - return (parent_rate / div); + debug("%s: rate %lu, new rate %lu\n", __func__, rate, npcm_clk_get_rate(clk)); + return npcm_clk_get_rate(clk); } static int npcm_clk_set_parent(struct clk *clk, struct clk *parent) diff --git a/drivers/clk/nuvoton/clk_npcm.h b/drivers/clk/nuvoton/clk_npcm.h index 06b60dc8b8..b4726d8381 100644 --- a/drivers/clk/nuvoton/clk_npcm.h +++ b/drivers/clk/nuvoton/clk_npcm.h @@ -50,6 +50,7 @@ #define PRE_DIV2 BIT(2) /* Pre divisor = 2 */ #define POST_DIV2 BIT(3) /* Post divisor = 2 */ #define FIXED_PARENT BIT(4) /* clock source is fixed */ +#define DIV_RO BIT(5) /* divider is read-only */ /* Parameters of PLL configuration */ struct npcm_clk_pll { diff --git a/drivers/clk/nuvoton/clk_npcm8xx.c b/drivers/clk/nuvoton/clk_npcm8xx.c index 27e3cfcf55..d1b32e3237 100644 --- a/drivers/clk/nuvoton/clk_npcm8xx.c +++ b/drivers/clk/nuvoton/clk_npcm8xx.c @@ -45,12 +45,12 @@ static struct npcm_clk_select npcm8xx_clk_selectors[] = { }; static struct npcm_clk_div npcm8xx_clk_dividers[] = { - {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2}, - {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2}, - {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2}, - {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1}, - {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1}, - {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1}, + {NPCM8XX_CLK_AHB, CLKDIV1, CLK4DIV, DIV_TYPE1 | PRE_DIV2 | DIV_RO}, + {NPCM8XX_CLK_APB2, CLKDIV2, APB2CKDIV, DIV_TYPE2 | DIV_RO}, + {NPCM8XX_CLK_APB5, CLKDIV2, APB5CKDIV, DIV_TYPE2 | DIV_RO}, + {NPCM8XX_CLK_SPI0, CLKDIV3, SPI0CKDIV, DIV_TYPE1 | DIV_RO}, + {NPCM8XX_CLK_SPI1, CLKDIV3, SPI1CKDIV, DIV_TYPE1 | DIV_RO}, + {NPCM8XX_CLK_SPI3, CLKDIV1, SPI3CKDIV, DIV_TYPE1 | DIV_RO}, {NPCM8XX_CLK_SPIX, CLKDIV3, SPIXCKDIV, DIV_TYPE1}, {NPCM8XX_CLK_UART, CLKDIV1, UARTDIV1, DIV_TYPE1}, {NPCM8XX_CLK_UART2, CLKDIV3, UARTDIV2, DIV_TYPE1},
Set ahb/apb/fiu clock divider as read-only Signed-off-by: Jim Liu <JJLIU0@nuvoton.com> --- drivers/clk/nuvoton/clk_npcm.c | 15 ++++++++++++--- drivers/clk/nuvoton/clk_npcm.h | 1 + drivers/clk/nuvoton/clk_npcm8xx.c | 12 ++++++------ 3 files changed, 19 insertions(+), 9 deletions(-)