AST2600A1/A2 have different pll calculate formula.
Signed-off-by: Ryan Chen <[email protected]>
---
drivers/clk/clk-ast2600.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index bbacaccad554..8933bd1506b3 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -17,7 +17,8 @@
#define ASPEED_G6_NUM_CLKS 71
-#define ASPEED_G6_SILICON_REV 0x004
+#define ASPEED_G6_SILICON_REV 0x014
+#define CHIP_REVISION_ID GENMASK(23, 16)
#define ASPEED_G6_RESET_CTRL 0x040
#define ASPEED_G6_RESET_CTRL2 0x050
@@ -190,18 +191,34 @@ static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
{
unsigned int mult, div;
+ u32 chip_id = readl(scu_g6_base + ASPEED_G6_SILICON_REV);
- if (val & BIT(20)) {
- /* Pass through mode */
- mult = div = 1;
+ if (((chip_id & CHIP_REVISION_ID) >> 16) >= 2) {
+ if (val & BIT(24)) {
+ /* Pass through mode */
+ mult = div = 1;
+ } else {
+ /* F = 25Mhz * [(m + 1) / (n + 1)] / (p + 1) */
+ u32 m = val & 0x1fff;
+ u32 n = (val >> 13) & 0x3f;
+ u32 p = (val >> 19) & 0xf;
+
+ mult = (m + 1);
+ div = (n + 1) * (p + 1);
+ }
} else {
- /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
- u32 m = (val >> 5) & 0x3f;
- u32 od = (val >> 4) & 0x1;
- u32 n = val & 0xf;
+ if (val & BIT(20)) {
+ /* Pass through mode */
+ mult = div = 1;
+ } else {
+ /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
+ u32 m = (val >> 5) & 0x3f;
+ u32 od = (val >> 4) & 0x1;
+ u32 n = val & 0xf;
- mult = (2 - od) * (m + 2);
- div = n + 1;
+ mult = (2 - od) * (m + 2);
+ div = n + 1;
+ }
}
return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
mult, div);
--
2.17.1
On Mon, 2021-01-18 at 18:08 +0800, Ryan Chen wrote:
> AST2600A1/A2 have different pll calculate formula.
To clarify, only the A0 has the old calculation, and all subsequent
revisions use the new calculation?
If this is the case, do we need to support A0 in mainline linux, or
should we drop support for A0 and only support A1, A2 and onwards?
You should add a line to indicate this is a fix:
Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
Also, when sending single patches you do not need to include the cover
letter. You should include all of the relevant information in the
patch's commit message.
>
> Signed-off-by: Ryan Chen <[email protected]>
> ---
> drivers/clk/clk-ast2600.c | 37 +++++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> index bbacaccad554..8933bd1506b3 100644
> --- a/drivers/clk/clk-ast2600.c
> +++ b/drivers/clk/clk-ast2600.c
> @@ -17,7 +17,8 @@
>
> #define ASPEED_G6_NUM_CLKS 71
>
> -#define ASPEED_G6_SILICON_REV 0x004
> +#define ASPEED_G6_SILICON_REV 0x014
> +#define CHIP_REVISION_ID GENMASK(23, 16)
>
> #define ASPEED_G6_RESET_CTRL 0x040
> #define ASPEED_G6_RESET_CTRL2 0x050
> @@ -190,18 +191,34 @@ static struct clk_hw *ast2600_calc_pll(const
> char *name, u32 val)
> static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
> {
> unsigned int mult, div;
> + u32 chip_id = readl(scu_g6_base + ASPEED_G6_SILICON_REV);
>
> - if (val & BIT(20)) {
> - /* Pass through mode */
> - mult = div = 1;
> + if (((chip_id & CHIP_REVISION_ID) >> 16) >= 2) {
> + if (val & BIT(24)) {
> + /* Pass through mode */
> + mult = div = 1;
> + } else {
> + /* F = 25Mhz * [(m + 1) / (n + 1)] / (p + 1)
> */
> + u32 m = val & 0x1fff;
> + u32 n = (val >> 13) & 0x3f;
> + u32 p = (val >> 19) & 0xf;
> +
> + mult = (m + 1);
> + div = (n + 1) * (p + 1);
> + }
> } else {
> - /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
> - u32 m = (val >> 5) & 0x3f;
> - u32 od = (val >> 4) & 0x1;
> - u32 n = val & 0xf;
> + if (val & BIT(20)) {
> + /* Pass through mode */
> + mult = div = 1;
> + } else {
> + /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)]
> */
> + u32 m = (val >> 5) & 0x3f;
> + u32 od = (val >> 4) & 0x1;
> + u32 n = val & 0xf;
>
> - mult = (2 - od) * (m + 2);
> - div = n + 1;
> + mult = (2 - od) * (m + 2);
> + div = n + 1;
> + }
> }
> return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
> mult, div);
On Tue, 19 Jan 2021 at 03:04, Ryan Chen <[email protected]> wrote:
>
> > -----Original Message-----
> > From: Joel Stanley <[email protected]>
> > Sent: Tuesday, January 19, 2021 10:20 AM
> > To: Ryan Chen <[email protected]>; Michael Turquette
> > <[email protected]>; Stephen Boyd <[email protected]>;
> > [email protected]; [email protected];
> > [email protected]; BMC-SW <[email protected]>
> > Cc: [email protected]; Andrew Jeffery <[email protected]>
> > Subject: Re: [PATCH 1/1] clk: aspeed: Fix APLL calculate formula for ast2600-A2
> >
> > On Mon, 2021-01-18 at 18:08 +0800, Ryan Chen wrote:
> > > AST2600A1/A2 have different pll calculate formula.
> >
> > To clarify, only the A0 has the old calculation, and all subsequent revisions use
> > the new calculation?
> >
> > If this is the case, do we need to support A0 in mainline linux, or should we
> > drop support for A0 and only support A1, A2 and onwards?
> >
> A0/A1 is use older calculate formula
> After A2 is new calculate formula as the patch.
Thanks for clarifying. I suggest you change the commit log to say
something like this:
Starting from A2, the A-PLL calculation has changed. Use the existing
formula for A0/A1 and the new formula for A2 onwards.
>
> > You should add a line to indicate this is a fix:
> >
> > Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
> >
> Got it. so should I send new patch?
Yes, please consider adjusting the commit message as I suggested
above, and add the fixes line.
> > > + u32 chip_id = readl(scu_g6_base + ASPEED_G6_SILICON_REV);
> > >
> > > - if (val & BIT(20)) {
> > > - /* Pass through mode */
> > > - mult = div = 1;
> > > + if (((chip_id & CHIP_REVISION_ID) >> 16) >= 2) {
Will this test be true if there are future versions of the chip (A3, etc)?
Cheers,
Joel
> -----Original Message-----
> From: Joel Stanley <[email protected]>
> Sent: Tuesday, January 19, 2021 10:20 AM
> To: Ryan Chen <[email protected]>; Michael Turquette
> <[email protected]>; Stephen Boyd <[email protected]>;
> [email protected]; [email protected];
> [email protected]; BMC-SW <[email protected]>
> Cc: [email protected]; Andrew Jeffery <[email protected]>
> Subject: Re: [PATCH 1/1] clk: aspeed: Fix APLL calculate formula for ast2600-A2
>
> On Mon, 2021-01-18 at 18:08 +0800, Ryan Chen wrote:
> > AST2600A1/A2 have different pll calculate formula.
>
> To clarify, only the A0 has the old calculation, and all subsequent revisions use
> the new calculation?
>
> If this is the case, do we need to support A0 in mainline linux, or should we
> drop support for A0 and only support A1, A2 and onwards?
>
A0/A1 is use older calculate formula
After A2 is new calculate formula as the patch.
> You should add a line to indicate this is a fix:
>
> Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
>
Got it. so should I send new patch?
> Also, when sending single patches you do not need to include the cover letter.
> You should include all of the relevant information in the patch's commit
> message.
>
> >
> > Signed-off-by: Ryan Chen <[email protected]>
> > ---
> > drivers/clk/clk-ast2600.c | 37 +++++++++++++++++++++++++++----------
> > 1 file changed, 27 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> > index bbacaccad554..8933bd1506b3 100644
> > --- a/drivers/clk/clk-ast2600.c
> > +++ b/drivers/clk/clk-ast2600.c
> > @@ -17,7 +17,8 @@
> >
> > #define ASPEED_G6_NUM_CLKS 71
> >
> > -#define ASPEED_G6_SILICON_REV 0x004
> > +#define ASPEED_G6_SILICON_REV 0x014 #define
> CHIP_REVISION_ID
> > +GENMASK(23, 16)
> >
> > #define ASPEED_G6_RESET_CTRL 0x040
> > #define ASPEED_G6_RESET_CTRL2 0x050 @@ -190,18
> +191,34 @@
> > static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
> > static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
> > {
> > unsigned int mult, div;
> > + u32 chip_id = readl(scu_g6_base + ASPEED_G6_SILICON_REV);
> >
> > - if (val & BIT(20)) {
> > - /* Pass through mode */
> > - mult = div = 1;
> > + if (((chip_id & CHIP_REVISION_ID) >> 16) >= 2) {
> > + if (val & BIT(24)) {
> > + /* Pass through mode */
> > + mult = div = 1;
> > + } else {
> > + /* F = 25Mhz * [(m + 1) / (n + 1)] / (p +
> 1)
> > */
> > + u32 m = val & 0x1fff;
> > + u32 n = (val >> 13) & 0x3f;
> > + u32 p = (val >> 19) & 0xf;
> > +
> > + mult = (m + 1);
> > + div = (n + 1) * (p + 1);
> > + }
> > } else {
> > - /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
> > - u32 m = (val >> 5) & 0x3f;
> > - u32 od = (val >> 4) & 0x1;
> > - u32 n = val & 0xf;
> > + if (val & BIT(20)) {
> > + /* Pass through mode */
> > + mult = div = 1;
> > + } else {
> > + /* F = 25Mhz * (2-od) * [(m + 2) / (n +
> 1)]
> > */
> > + u32 m = (val >> 5) & 0x3f;
> > + u32 od = (val >> 4) & 0x1;
> > + u32 n = val & 0xf;
> >
> > - mult = (2 - od) * (m + 2);
> > - div = n + 1;
> > + mult = (2 - od) * (m + 2);
> > + div = n + 1;
> > + }
> > }
> > return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
> > mult, div);
>
> -----Original Message-----
> From: Joel Stanley <[email protected]>
> Sent: Tuesday, January 19, 2021 11:10 AM
> To: Ryan Chen <[email protected]>
> Cc: Joel Stanley <[email protected]>; Michael Turquette
> <[email protected]>; Stephen Boyd <[email protected]>;
> [email protected]; [email protected];
> [email protected]; BMC-SW <[email protected]>; Andrew
> Jeffery <[email protected]>
> Subject: Re: [PATCH 1/1] clk: aspeed: Fix APLL calculate formula for ast2600-A2
>
> On Tue, 19 Jan 2021 at 03:04, Ryan Chen <[email protected]>
> wrote:
> >
> > > -----Original Message-----
> > > From: Joel Stanley <[email protected]>
> > > Sent: Tuesday, January 19, 2021 10:20 AM
> > > To: Ryan Chen <[email protected]>; Michael Turquette
> > > <[email protected]>; Stephen Boyd <[email protected]>;
> > > [email protected]; [email protected];
> > > [email protected]; BMC-SW <[email protected]>
> > > Cc: [email protected]; Andrew Jeffery <[email protected]>
> > > Subject: Re: [PATCH 1/1] clk: aspeed: Fix APLL calculate formula for
> > > ast2600-A2
> > >
> > > On Mon, 2021-01-18 at 18:08 +0800, Ryan Chen wrote:
> > > > AST2600A1/A2 have different pll calculate formula.
> > >
> > > To clarify, only the A0 has the old calculation, and all subsequent
> > > revisions use the new calculation?
> > >
> > > If this is the case, do we need to support A0 in mainline linux, or
> > > should we drop support for A0 and only support A1, A2 and onwards?
> > >
> > A0/A1 is use older calculate formula
> > After A2 is new calculate formula as the patch.
>
> Thanks for clarifying. I suggest you change the commit log to say something
> like this:
>
> Starting from A2, the A-PLL calculation has changed. Use the existing formula
> for A0/A1 and the new formula for A2 onwards.
>
> >
> > > You should add a line to indicate this is a fix:
> > >
> > > Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
> > >
> > Got it. so should I send new patch?
>
> Yes, please consider adjusting the commit message as I suggested above, and
> add the fixes line.
>
> > > > + u32 chip_id = readl(scu_g6_base + ASPEED_G6_SILICON_REV);
> > > >
> > > > - if (val & BIT(20)) {
> > > > - /* Pass through mode */
> > > > - mult = div = 1;
> > > > + if (((chip_id & CHIP_REVISION_ID) >> 16) >= 2) {
>
> Will this test be true if there are future versions of the chip (A3, etc)?
Yes, is also support for A3.
Starting from A2, the A-PLL calculation has changed. Use the
existing formula for A0/A1 and the new formula for A2 onwards.
Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
Signed-off-by: Ryan Chen <[email protected]>
---
drivers/clk/clk-ast2600.c | 37 +++++++++++++++++++++++++++----------
1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
index bbacaccad554..8933bd1506b3 100644
--- a/drivers/clk/clk-ast2600.c
+++ b/drivers/clk/clk-ast2600.c
@@ -17,7 +17,8 @@
#define ASPEED_G6_NUM_CLKS 71
-#define ASPEED_G6_SILICON_REV 0x004
+#define ASPEED_G6_SILICON_REV 0x014
+#define CHIP_REVISION_ID GENMASK(23, 16)
#define ASPEED_G6_RESET_CTRL 0x040
#define ASPEED_G6_RESET_CTRL2 0x050
@@ -190,18 +191,34 @@ static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
{
unsigned int mult, div;
+ u32 chip_id = readl(scu_g6_base + ASPEED_G6_SILICON_REV);
- if (val & BIT(20)) {
- /* Pass through mode */
- mult = div = 1;
+ if (((chip_id & CHIP_REVISION_ID) >> 16) >= 2) {
+ if (val & BIT(24)) {
+ /* Pass through mode */
+ mult = div = 1;
+ } else {
+ /* F = 25Mhz * [(m + 1) / (n + 1)] / (p + 1) */
+ u32 m = val & 0x1fff;
+ u32 n = (val >> 13) & 0x3f;
+ u32 p = (val >> 19) & 0xf;
+
+ mult = (m + 1);
+ div = (n + 1) * (p + 1);
+ }
} else {
- /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
- u32 m = (val >> 5) & 0x3f;
- u32 od = (val >> 4) & 0x1;
- u32 n = val & 0xf;
+ if (val & BIT(20)) {
+ /* Pass through mode */
+ mult = div = 1;
+ } else {
+ /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
+ u32 m = (val >> 5) & 0x3f;
+ u32 od = (val >> 4) & 0x1;
+ u32 n = val & 0xf;
- mult = (2 - od) * (m + 2);
- div = n + 1;
+ mult = (2 - od) * (m + 2);
+ div = n + 1;
+ }
}
return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
mult, div);
--
2.17.1
On Tue, 19 Jan 2021 at 06:31, Ryan Chen <[email protected]> wrote:
>
> Starting from A2, the A-PLL calculation has changed. Use the
> existing formula for A0/A1 and the new formula for A2 onwards.
>
> Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
> Signed-off-by: Ryan Chen <[email protected]>
Reviewed-by: Joel Stanley <[email protected]>
> ---
> drivers/clk/clk-ast2600.c | 37 +++++++++++++++++++++++++++----------
> 1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> index bbacaccad554..8933bd1506b3 100644
> --- a/drivers/clk/clk-ast2600.c
> +++ b/drivers/clk/clk-ast2600.c
> @@ -17,7 +17,8 @@
>
> #define ASPEED_G6_NUM_CLKS 71
>
> -#define ASPEED_G6_SILICON_REV 0x004
> +#define ASPEED_G6_SILICON_REV 0x014
> +#define CHIP_REVISION_ID GENMASK(23, 16)
>
> #define ASPEED_G6_RESET_CTRL 0x040
> #define ASPEED_G6_RESET_CTRL2 0x050
> @@ -190,18 +191,34 @@ static struct clk_hw *ast2600_calc_pll(const char *name, u32 val)
> static struct clk_hw *ast2600_calc_apll(const char *name, u32 val)
> {
> unsigned int mult, div;
> + u32 chip_id = readl(scu_g6_base + ASPEED_G6_SILICON_REV);
>
> - if (val & BIT(20)) {
> - /* Pass through mode */
> - mult = div = 1;
> + if (((chip_id & CHIP_REVISION_ID) >> 16) >= 2) {
> + if (val & BIT(24)) {
> + /* Pass through mode */
> + mult = div = 1;
> + } else {
> + /* F = 25Mhz * [(m + 1) / (n + 1)] / (p + 1) */
> + u32 m = val & 0x1fff;
> + u32 n = (val >> 13) & 0x3f;
> + u32 p = (val >> 19) & 0xf;
> +
> + mult = (m + 1);
> + div = (n + 1) * (p + 1);
> + }
> } else {
> - /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
> - u32 m = (val >> 5) & 0x3f;
> - u32 od = (val >> 4) & 0x1;
> - u32 n = val & 0xf;
> + if (val & BIT(20)) {
> + /* Pass through mode */
> + mult = div = 1;
> + } else {
> + /* F = 25Mhz * (2-od) * [(m + 2) / (n + 1)] */
> + u32 m = (val >> 5) & 0x3f;
> + u32 od = (val >> 4) & 0x1;
> + u32 n = val & 0xf;
>
> - mult = (2 - od) * (m + 2);
> - div = n + 1;
> + mult = (2 - od) * (m + 2);
> + div = n + 1;
> + }
> }
> return clk_hw_register_fixed_factor(NULL, name, "clkin", 0,
> mult, div);
> --
> 2.17.1
>
Quoting Ryan Chen (2021-01-18 22:17:15)
> Starting from A2, the A-PLL calculation has changed. Use the
> existing formula for A0/A1 and the new formula for A2 onwards.
>
> Fixes: d3d04f6c330a ("clk: Add support for AST2600 SoC")
> Signed-off-by: Ryan Chen <[email protected]>
> ---
Applied to clk-next