linux-kernel - Re: [PATCH 1/1] clk: aspeed: Fix APLL calculate formula for ast2600-A2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Jan 2021 12:50:05 +1030
From:   Joel Stanley <joel@...ux.ibm.com>
To:     Ryan Chen <ryan_chen@...eedtech.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, andrewrj@....ibm.com,
        BMC-SW@...eedtech.com
Cc:     joel@....id.au, Andrew Jeffery <andrew@...id.au>
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?

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 <ryan_chen@...eedtech.com>
> ---
>  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);