LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/1] Fix AST2600 hpll calculate formula
@ 2021-08-18  8:05 Ryan Chen
  2021-08-18  8:05 ` [PATCH 1/1] clk:aspeed:Fix " Ryan Chen
  2021-08-29  4:37 ` [PATCH 0/1] Fix " Stephen Boyd
  0 siblings, 2 replies; 8+ messages in thread
From: Ryan Chen @ 2021-08-18  8:05 UTC (permalink / raw)
  To: bmc-sw, ryan_chen, Michael Turquette, Stephen Boyd, Joel Stanley,
	Andrew Jeffery, linux-clk, linux-kernel

AST2600 HPLL calculate has few different with other pll calculate

Ryan Chen (1):
  clk:aspeed:Fix AST2600 hpll calculate formula

 drivers/clk/clk-ast2600.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] clk:aspeed:Fix AST2600 hpll calculate formula
  2021-08-18  8:05 [PATCH 0/1] Fix AST2600 hpll calculate formula Ryan Chen
@ 2021-08-18  8:05 ` Ryan Chen
  2021-08-29  4:37   ` Stephen Boyd
  2021-09-01  6:49   ` Joel Stanley
  2021-08-29  4:37 ` [PATCH 0/1] Fix " Stephen Boyd
  1 sibling, 2 replies; 8+ messages in thread
From: Ryan Chen @ 2021-08-18  8:05 UTC (permalink / raw)
  To: bmc-sw, ryan_chen, Michael Turquette, Stephen Boyd, Joel Stanley,
	Andrew Jeffery, linux-clk, linux-kernel

AST2600 HPLL calculate formula [SCU200]
HPLL Numerator(M): have fixed value depend on SCU strap.
M = SCU500[10] ? 0x5F : SCU500[8] ? 0xBF : SCU200[12:0]

if SCU500[10] = 1, M=0x5F.
else if SCU500[10]=0 & SCU500[8]=1, M=0xBF.
others (SCU510[10]=0 and SCU510[8]=0)
depend on SCU200[12:0] (default 0x8F) register setting.

HPLL Denumerator (N) =	SCU200[18:13] (default 0x2)
HPLL Divider (P)	 =	SCU200[22:19] (default 0x0)

Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
 drivers/clk/clk-ast2600.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index 085d0a18b2b6..5d8c46bcf237 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -169,6 +169,33 @@ static const struct clk_div_table ast2600_div_table[] = {
 };
 
 /* For hpll/dpll/epll/mpll */
+static struct clk_hw *ast2600_calc_hpll(const char *name, u32 val)
+{
+	unsigned int mult, div;
+	u32 hwstrap = readl(scu_g6_base + ASPEED_G6_STRAP1);
+
+	if (val & BIT(24)) {
+		/* Pass through mode */
+		mult = div = 1;
+	} else {
+		/* F = 25Mhz * [(M + 2) / (n + 1)] / (p + 1) */
+		u32 m = val  & 0x1fff;
+		u32 n = (val >> 13) & 0x3f;
+		u32 p = (val >> 19) & 0xf;
+
+		if (hwstrap & BIT(10))
+			m = 0x5F;
+		else {
+			if (hwstrap & BIT(8))
+				m = 0xBF;
+		}
+		mult = (m + 1) / (n + 1);
+		div = (p + 1);
+	}
+	return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
+			mult, div);
+};
+
 static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
 {
 	unsigned int mult, div;
@@ -716,7 +743,7 @@ static void __init aspeed_g6_cc(struct regmap *map)
 	 * and we assume that it is enabled
 	 */
 	regmap_read(map, ASPEED_HPLL_PARAM, &val);
-	aspeed_g6_clk_data->hws[ASPEED_CLK_HPLL] = ast2600_calc_pll("hpll", val);
+	aspeed_g6_clk_data->hws[ASPEED_CLK_HPLL] = ast2600_calc_hpll("hpll", val);
 
 	regmap_read(map, ASPEED_MPLL_PARAM, &val);
 	aspeed_g6_clk_data->hws[ASPEED_CLK_MPLL] = ast2600_calc_pll("mpll", val);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] clk:aspeed:Fix AST2600 hpll calculate formula
  2021-08-18  8:05 ` [PATCH 1/1] clk:aspeed:Fix " Ryan Chen
@ 2021-08-29  4:37   ` Stephen Boyd
  2021-09-01  5:59     ` Ryan Chen
  2021-09-01  6:49   ` Joel Stanley
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Boyd @ 2021-08-29  4:37 UTC (permalink / raw)
  To: Andrew Jeffery, Joel Stanley, Michael Turquette, bmc-sw,
	linux-clk, linux-kernel, ryan_chen

Quoting Ryan Chen (2021-08-18 01:05:18)
> AST2600 HPLL calculate formula [SCU200]
> HPLL Numerator(M): have fixed value depend on SCU strap.
> M = SCU500[10] ? 0x5F : SCU500[8] ? 0xBF : SCU200[12:0]
> 
> if SCU500[10] = 1, M=0x5F.
> else if SCU500[10]=0 & SCU500[8]=1, M=0xBF.
> others (SCU510[10]=0 and SCU510[8]=0)
> depend on SCU200[12:0] (default 0x8F) register setting.
> 
> HPLL Denumerator (N) =  SCU200[18:13] (default 0x2)
> HPLL Divider (P)         =      SCU200[22:19] (default 0x0)
> 
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>

Any Fixes tag? Joel, can you review?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/1] Fix AST2600 hpll calculate formula
  2021-08-18  8:05 [PATCH 0/1] Fix AST2600 hpll calculate formula Ryan Chen
  2021-08-18  8:05 ` [PATCH 1/1] clk:aspeed:Fix " Ryan Chen
@ 2021-08-29  4:37 ` Stephen Boyd
  2021-09-01  5:57   ` Ryan Chen
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Boyd @ 2021-08-29  4:37 UTC (permalink / raw)
  To: Andrew Jeffery, Joel Stanley, Michael Turquette, bmc-sw,
	linux-clk, linux-kernel, ryan_chen

Quoting Ryan Chen (2021-08-18 01:05:17)
> AST2600 HPLL calculate has few different with other pll calculate
> 
> Ryan Chen (1):

Please don't send cover letters for a single patch.

>   clk:aspeed:Fix AST2600 hpll calculate formula
> 
>  drivers/clk/clk-ast2600.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> -- 
> 2.17.1
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 0/1] Fix AST2600 hpll calculate formula
  2021-08-29  4:37 ` [PATCH 0/1] Fix " Stephen Boyd
@ 2021-09-01  5:57   ` Ryan Chen
  0 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2021-09-01  5:57 UTC (permalink / raw)
  To: Stephen Boyd, Andrew Jeffery, Joel Stanley, Michael Turquette,
	BMC-SW, linux-clk, linux-kernel

> -----Original Message-----
> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Sunday, August 29, 2021 12:37 PM
> To: Andrew Jeffery <andrew@aj.id.au>; Joel Stanley <joel@jms.id.au>; Michael
> Turquette <mturquette@baylibre.com>; BMC-SW
> <BMC-SW@aspeedtech.com>; linux-clk@vger.kernel.org;
> linux-kernel@vger.kernel.org; Ryan Chen <ryan_chen@aspeedtech.com>
> Subject: Re: [PATCH 0/1] Fix AST2600 hpll calculate formula
> 
> Quoting Ryan Chen (2021-08-18 01:05:17)
> > AST2600 HPLL calculate has few different with other pll calculate
> >
> > Ryan Chen (1):
> 
> Please don't send cover letters for a single patch.

Got it, thanks. 
> 
> >   clk:aspeed:Fix AST2600 hpll calculate formula
> >
> >  drivers/clk/clk-ast2600.c | 29 ++++++++++++++++++++++++++++-
> >  1 file changed, 28 insertions(+), 1 deletion(-)
> >
> > --
> > 2.17.1
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 1/1] clk:aspeed:Fix AST2600 hpll calculate formula
  2021-08-29  4:37   ` Stephen Boyd
@ 2021-09-01  5:59     ` Ryan Chen
  0 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2021-09-01  5:59 UTC (permalink / raw)
  To: Stephen Boyd, Andrew Jeffery, Joel Stanley, Michael Turquette,
	BMC-SW, linux-clk, linux-kernel

> -----Original Message-----
> From: Stephen Boyd <sboyd@kernel.org>
> Sent: Sunday, August 29, 2021 12:37 PM
> To: Andrew Jeffery <andrew@aj.id.au>; Joel Stanley <joel@jms.id.au>; Michael
> Turquette <mturquette@baylibre.com>; BMC-SW
> <BMC-SW@aspeedtech.com>; linux-clk@vger.kernel.org;
> linux-kernel@vger.kernel.org; Ryan Chen <ryan_chen@aspeedtech.com>
> Subject: Re: [PATCH 1/1] clk:aspeed:Fix AST2600 hpll calculate formula
> 
> Quoting Ryan Chen (2021-08-18 01:05:18)
> > AST2600 HPLL calculate formula [SCU200] HPLL Numerator(M): have fixed
> > value depend on SCU strap.
> > M = SCU500[10] ? 0x5F : SCU500[8] ? 0xBF : SCU200[12:0]
> >
> > if SCU500[10] = 1, M=0x5F.
> > else if SCU500[10]=0 & SCU500[8]=1, M=0xBF.
> > others (SCU510[10]=0 and SCU510[8]=0)
> > depend on SCU200[12:0] (default 0x8F) register setting.
> >
> > HPLL Denumerator (N) =  SCU200[18:13] (default 0x2)
> > HPLL Divider (P)         =      SCU200[22:19] (default 0x0)
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> 
> Any Fixes tag? Joel, can you review?

Hello Joel,

Should I apply to this tag? d3d04f6c330a

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] clk:aspeed:Fix AST2600 hpll calculate formula
  2021-08-18  8:05 ` [PATCH 1/1] clk:aspeed:Fix " Ryan Chen
  2021-08-29  4:37   ` Stephen Boyd
@ 2021-09-01  6:49   ` Joel Stanley
  2021-09-01  9:03     ` Ryan Chen
  1 sibling, 1 reply; 8+ messages in thread
From: Joel Stanley @ 2021-09-01  6:49 UTC (permalink / raw)
  To: Ryan Chen
  Cc: BMC-SW, Michael Turquette, Stephen Boyd, Andrew Jeffery,
	linux-clk, Linux Kernel Mailing List

Hello Ryan,

Thanks for the patch. I have some questions about it below.

On Wed, 18 Aug 2021 at 08:05, Ryan Chen <ryan_chen@aspeedtech.com> wrote:
>
> AST2600 HPLL calculate formula [SCU200]
> HPLL Numerator(M): have fixed value depend on SCU strap.
> M = SCU500[10] ? 0x5F : SCU500[8] ? 0xBF : SCU200[12:0]

I see from the datasheet:

CPU frequency selection
000 1.2GHz
001 1.6GHz
010 1.2GHz
011 1.6GHz
100 800MHz
101 800MHz
110 800MHz
111 800MHz

So when the system is running at 800MHz or 1.6GHz, the value for the
numerator (m) in SCU204 is incorrect, and must be overridden?

> if SCU500[10] = 1, M=0x5F.
> else if SCU500[10]=0 & SCU500[8]=1, M=0xBF.
> others (SCU510[10]=0 and SCU510[8]=0)
> depend on SCU200[12:0] (default 0x8F) register setting.
>
> HPLL Denumerator (N) =  SCU200[18:13] (default 0x2)
> HPLL Divider (P)         =      SCU200[22:19] (default 0x0)

Is this the case for all revisions of the soc, from A0 through to A3?

Do you have a datasheet update that captures this information?

When you resend, please add a fixes line as follows, as this code has
been present since we introduced the driver:

Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")

>
> Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> ---
>  drivers/clk/clk-ast2600.c | 29 ++++++++++++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> index 085d0a18b2b6..5d8c46bcf237 100644
> --- a/drivers/clk/clk-ast2600.c
> +++ b/drivers/clk/clk-ast2600.c
> @@ -169,6 +169,33 @@ static const struct clk_div_table ast2600_div_table[] = {
>  };
>
>  /* For hpll/dpll/epll/mpll */

This comment needs to stay with ast2600_calc_pll, and just drop hpll
from the list.

> +static struct clk_hw *ast2600_calc_hpll(const char *name, u32 val)
> +{
> +       unsigned int mult, div;
> +       u32 hwstrap = readl(scu_g6_base + ASPEED_G6_STRAP1);
> +
> +       if (val & BIT(24)) {
> +               /* Pass through mode */
> +               mult = div = 1;
> +       } else {
> +               /* F = 25Mhz * [(M + 2) / (n + 1)] / (p + 1) */
> +               u32 m = val  & 0x1fff;
> +               u32 n = (val >> 13) & 0x3f;
> +               u32 p = (val >> 19) & 0xf;
> +
> +               if (hwstrap & BIT(10))

So this is testing if the CPU is running at 800Mhz.

> +                       m = 0x5F;
> +               else {
> +                       if (hwstrap & BIT(8))

And this is testing if the CPU is running at 1.6GHz.

I would write it like this:

u32 m;

if (hwstrap & BIT(10)) {
    /* CPU running at 800MHz */
   m = 95;
} else if (hwstrap & BIT(10)) {
    /* CPU running at 1.6GHz */
  m  = 191;
} else {
   /* CPU running at 1.2Ghz */
  m = val  & 0x1fff;
}

Cheers,

Joel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH 1/1] clk:aspeed:Fix AST2600 hpll calculate formula
  2021-09-01  6:49   ` Joel Stanley
@ 2021-09-01  9:03     ` Ryan Chen
  0 siblings, 0 replies; 8+ messages in thread
From: Ryan Chen @ 2021-09-01  9:03 UTC (permalink / raw)
  To: Joel Stanley
  Cc: BMC-SW, Michael Turquette, Stephen Boyd, Andrew Jeffery,
	linux-clk, Linux Kernel Mailing List

> -----Original Message-----
> From: Joel Stanley <joel@jms.id.au>
> Sent: Wednesday, September 1, 2021 2:49 PM
> To: Ryan Chen <ryan_chen@aspeedtech.com>
> Cc: BMC-SW <BMC-SW@aspeedtech.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Andrew
> Jeffery <andrew@aj.id.au>; linux-clk@vger.kernel.org; Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 1/1] clk:aspeed:Fix AST2600 hpll calculate formula
> 
> Hello Ryan,
> 
> Thanks for the patch. I have some questions about it below.
> 
> On Wed, 18 Aug 2021 at 08:05, Ryan Chen <ryan_chen@aspeedtech.com>
> wrote:
> >
> > AST2600 HPLL calculate formula [SCU200] HPLL Numerator(M): have fixed
> > value depend on SCU strap.
> > M = SCU500[10] ? 0x5F : SCU500[8] ? 0xBF : SCU200[12:0]
> 
> I see from the datasheet:
> 
> CPU frequency selection
> 000 1.2GHz
> 001 1.6GHz
> 010 1.2GHz
> 011 1.6GHz
> 100 800MHz
> 101 800MHz
> 110 800MHz
> 111 800MHz
> 
> So when the system is running at 800MHz or 1.6GHz, the value for the
> numerator (m) in SCU204 is incorrect, and must be overridden?
> 
M is in SCU200 not SCU204.
The patch code is point out the issue, that not only check the SCU200 to calculate the hpll, 
but also need check the SCU500[10] and SCU510[8], it will effect the M value.

> > if SCU500[10] = 1, M=0x5F.
> > else if SCU500[10]=0 & SCU500[8]=1, M=0xBF.
> > others (SCU510[10]=0 and SCU510[8]=0)
> > depend on SCU200[12:0] (default 0x8F) register setting.
> >
> > HPLL Denumerator (N) =  SCU200[18:13] (default 0x2)
> > HPLL Divider (P)         =      SCU200[22:19] (default 0x0)
> 
> Is this the case for all revisions of the soc, from A0 through to A3?
Yes. it is.
> 
> Do you have a datasheet update that captures this information?
No.
> 
> When you resend, please add a fixes line as follows, as this code has been
> present since we introduced the driver:
Thanks a lot.
> 
> Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
> 
> >
> > Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
> > ---
> >  drivers/clk/clk-ast2600.c | 29 ++++++++++++++++++++++++++++-
> >  1 file changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> > index 085d0a18b2b6..5d8c46bcf237 100644
> > --- a/drivers/clk/clk-ast2600.c
> > +++ b/drivers/clk/clk-ast2600.c
> > @@ -169,6 +169,33 @@ static const struct clk_div_table
> > ast2600_div_table[] = {  };
> >
> >  /* For hpll/dpll/epll/mpll */
> 
> This comment needs to stay with ast2600_calc_pll, and just drop hpll from the
> list.
> 
> > +static struct clk_hw *ast2600_calc_hpll(const char *name, u32 val) {
> > +       unsigned int mult, div;
> > +       u32 hwstrap = readl(scu_g6_base + ASPEED_G6_STRAP1);
> > +
> > +       if (val & BIT(24)) {
> > +               /* Pass through mode */
> > +               mult = div = 1;
> > +       } else {
> > +               /* F = 25Mhz * [(M + 2) / (n + 1)] / (p + 1) */
> > +               u32 m = val  & 0x1fff;
> > +               u32 n = (val >> 13) & 0x3f;
> > +               u32 p = (val >> 19) & 0xf;
> > +
> > +               if (hwstrap & BIT(10))
> 
> So this is testing if the CPU is running at 800Mhz.
> 
> > +                       m = 0x5F;
> > +               else {
> > +                       if (hwstrap & BIT(8))
> 
> And this is testing if the CPU is running at 1.6GHz.
> 
> I would write it like this:
> 
> u32 m;
> 
> if (hwstrap & BIT(10)) {
>     /* CPU running at 800MHz */
>    m = 95;
> } else if (hwstrap & BIT(10)) {
>     /* CPU running at 1.6GHz */
>   m  = 191;
> } else {
>    /* CPU running at 1.2Ghz */
>   m = val  & 0x1fff;
> }
> 
> Cheers,
> 
> Joel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-01  9:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18  8:05 [PATCH 0/1] Fix AST2600 hpll calculate formula Ryan Chen
2021-08-18  8:05 ` [PATCH 1/1] clk:aspeed:Fix " Ryan Chen
2021-08-29  4:37   ` Stephen Boyd
2021-09-01  5:59     ` Ryan Chen
2021-09-01  6:49   ` Joel Stanley
2021-09-01  9:03     ` Ryan Chen
2021-08-29  4:37 ` [PATCH 0/1] Fix " Stephen Boyd
2021-09-01  5:57   ` Ryan Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).