Re: [PATCH v2] clk: nuvoton: add read only feature for clk driver

On 11/7/23 04:01, Jim Liu wrote:

Add a flag to set ahb/apb/fiu clock divider as read-only It just protects the clock source and can't modify it in uboot.

Thanks for adding this

Signed-off-by: Jim Liu JJLIU0@nuvoton.com

Changes for v2: - add commit message

---

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},

And this makes sense for bus clocks

• {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},

But it's a little unclear to me why this is enabled for SPI. Are you e.g. doing XIP from SPI?

--Sean

{NPCM8XX_CLK_SPIX, CLKDIV3, SPIXCKDIV, DIV_TYPE1}, {NPCM8XX_CLK_UART, CLKDIV1, UARTDIV1, DIV_TYPE1}, {NPCM8XX_CLK_UART2, CLKDIV3, UARTDIV2, DIV_TYPE1},