RE: [PATCH v2 01/11] clk: aspeed: Get HCLK frequency support

[Chin-Ting Kuo]

Hi Cédric,

Thanks for the review.

> -Original Message-
> From: Cédric Le Goater 
> Sent: Tuesday, May 17, 2022 8:19 PM
> Subject: Re: [PATCH v2 01/11] clk: aspeed: Get HCLK frequency support
> 
> On 5/9/22 09:23, Chin-Ting Kuo wrote:
> > User can get correct HCLK frequency during driver probe stage by
> > adding the following configuration in the device tree.
> > "clocks = < ASPEED_CLK_AHB>".
> >
> > Signed-off-by: Chin-Ting Kuo 
> > ---
> >   drivers/clk/aspeed/clk_ast2500.c | 24 
> >   1 file changed, 24 insertions(+)
> >
> > diff --git a/drivers/clk/aspeed/clk_ast2500.c
> > b/drivers/clk/aspeed/clk_ast2500.c
> > index a1b4496ca2..d2a58a8462 100644
> > --- a/drivers/clk/aspeed/clk_ast2500.c
> > +++ b/drivers/clk/aspeed/clk_ast2500.c
> > @@ -29,6 +29,12 @@
> >
> >   #define D2PLL_DEFAULT_RATE(250 * 1000 * 1000)
> >
> > +/*
> > + * AXI/AHB clock selection, taken from Aspeed SDK  */
> > +#define SCU_HWSTRAP_AXIAHB_DIV_SHIFT9
> > +#define SCU_HWSTRAP_AXIAHB_DIV_MASK (0x7 <<
> SCU_HWSTRAP_AXIAHB_DIV_SHIFT)
> > +
> >   DECLARE_GLOBAL_DATA_PTR;
> >
> >   /*
> > @@ -85,6 +91,21 @@ static ulong ast2500_get_clkin(struct ast2500_scu
> *scu)
> > ? 25 * 1000 * 1000 : 24 * 1000 * 1000;
> >   }
> >
> > +static u32 ast2500_get_hclk(struct ast2500_scu *scu) {
> > +   ulong clkin = ast2500_get_clkin(scu);
> 
> you could avoid reading clkin and pass it as a parameter like the other
> routines do : ast2500_get_hpll_rate() and ast2500_get_mpll_rate()
> 
> It's minor.
> 

I may need to change the other patch in this patch series, this part will also 
be included.


Chin-Ting

> Reviewed-by: Cédric Le Goater 
> 
> Thanks,
> 
> C.
> 
> 
> > +   u32 hpll_reg = readl(>h_pll_param);
> > +   ulong axi_div = 2;
> > +   u32 rate;
> > +   ulong ahb_div = 1 + ((readl(>hwstrap)
> > + & SCU_HWSTRAP_AXIAHB_DIV_MASK)
> > +>> SCU_HWSTRAP_AXIAHB_DIV_SHIFT);
> > +
> > +   rate = ast2500_get_hpll_rate(clkin, hpll_reg);
> > +
> > +   return (rate / axi_div / ahb_div);
> > +}
> > +
> >   /**
> >* Get current rate or uart clock
> >*
> > @@ -146,6 +167,9 @@ static ulong ast2500_clk_get_rate(struct clk *clk)
> > rate = rate / apb_div;
> > }
> > break;
> > +   case ASPEED_CLK_AHB:
> > +   rate = ast2500_get_hclk(priv->scu);
> > +   break;
> > case ASPEED_CLK_SDIO:
> > {
> > ulong apb_div = 4 + 4 * ((readl(>scu->clk_sel1)

Re: [PATCH v2 01/11] clk: aspeed: Get HCLK frequency support

[Cédric Le Goater]

On 5/9/22 09:23, Chin-Ting Kuo wrote:

User can get correct HCLK frequency during driver probe stage
by adding the following configuration in the device tree.
"clocks = < ASPEED_CLK_AHB>".

Signed-off-by: Chin-Ting Kuo 
---
  drivers/clk/aspeed/clk_ast2500.c | 24 
  1 file changed, 24 insertions(+)

diff --git a/drivers/clk/aspeed/clk_ast2500.c b/drivers/clk/aspeed/clk_ast2500.c
index a1b4496ca2..d2a58a8462 100644
--- a/drivers/clk/aspeed/clk_ast2500.c
+++ b/drivers/clk/aspeed/clk_ast2500.c
@@ -29,6 +29,12 @@
  
  #define D2PLL_DEFAULT_RATE	(250 * 1000 * 1000)
  
+/*

+ * AXI/AHB clock selection, taken from Aspeed SDK
+ */
+#define SCU_HWSTRAP_AXIAHB_DIV_SHIFT9
+#define SCU_HWSTRAP_AXIAHB_DIV_MASK (0x7 << SCU_HWSTRAP_AXIAHB_DIV_SHIFT)
+
  DECLARE_GLOBAL_DATA_PTR;
  
  /*

@@ -85,6 +91,21 @@ static ulong ast2500_get_clkin(struct ast2500_scu *scu)
? 25 * 1000 * 1000 : 24 * 1000 * 1000;
  }
  
+static u32 ast2500_get_hclk(struct ast2500_scu *scu)

+{
+   ulong clkin = ast2500_get_clkin(scu);


you could avoid reading clkin and pass it as a parameter like the
other routines do : ast2500_get_hpll_rate() and ast2500_get_mpll_rate()

It's minor.

Reviewed-by: Cédric Le Goater 

Thanks,

C.



+   u32 hpll_reg = readl(>h_pll_param);
+   ulong axi_div = 2;
+   u32 rate;
+   ulong ahb_div = 1 + ((readl(>hwstrap)
+ & SCU_HWSTRAP_AXIAHB_DIV_MASK)
+>> SCU_HWSTRAP_AXIAHB_DIV_SHIFT);
+
+   rate = ast2500_get_hpll_rate(clkin, hpll_reg);
+
+   return (rate / axi_div / ahb_div);
+}
+
  /**
   * Get current rate or uart clock
   *
@@ -146,6 +167,9 @@ static ulong ast2500_clk_get_rate(struct clk *clk)
rate = rate / apb_div;
}
break;
+   case ASPEED_CLK_AHB:
+   rate = ast2500_get_hclk(priv->scu);
+   break;
case ASPEED_CLK_SDIO:
{
ulong apb_div = 4 + 4 * ((readl(>scu->clk_sel1)