[PATCH linux dev-4.17 v2] clk: aspeed: Support HPLL strapping on ast2400

[Andrew Jeffery]

On Mon, 25 Jun 2018, at 10:43, Joel Stanley wrote:
> On 25 June 2018 at 10:41, Andrew Jeffery <andrew at aj.id.au> wrote:
> > On Thu, 21 Jun 2018, at 16:55, Joel Stanley wrote:
> >> The HPLL can be configured through a register (SCU24), however some
> >> platforms chose to configure it through the strapping settings and do
> >> not use the register. This was not noticed as the logic for bit 18 in
> >> SCU24 was confused: set means programmed, but the driver read it as set
> >> means strapped.
> >>
> >> This gives us the correct HPLL value on Palmetto systems, from which
> >> most of the peripheral clocks are generated.
> >>
> >> Signed-off-by: Joel Stanley <joel at jms.id.au>
> >> ---
> >> v2: Fix comment and commit message
> >> ---
> >>  drivers/clk/clk-aspeed.c | 22 +++++++++++++++++++---
> >>  1 file changed, 19 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
> >> index c17032bc853a..7d8cc412959d 100644
> >> --- a/drivers/clk/clk-aspeed.c
> >> +++ b/drivers/clk/clk-aspeed.c
> >> @@ -24,7 +24,7 @@
> >>  #define ASPEED_MPLL_PARAM    0x20
> >>  #define ASPEED_HPLL_PARAM    0x24
> >>  #define  AST2500_HPLL_BYPASS_EN      BIT(20)
> >> -#define  AST2400_HPLL_STRAPPED       BIT(18)
> >> +#define  AST2400_HPLL_PROGRAMMED BIT(18)
> >>  #define  AST2400_HPLL_BYPASS_EN      BIT(17)
> >>  #define ASPEED_MISC_CTRL     0x2c
> >>  #define  UART_DIV13_EN               BIT(12)
> >> @@ -586,8 +586,24 @@ static void __init aspeed_ast2400_cc(struct regmap *map)
> >>        * and we assume that it is enabled
> >>        */
> >>       regmap_read(map, ASPEED_HPLL_PARAM, &val);
> >> -     WARN(val & AST2400_HPLL_STRAPPED, "hpll is strapped not configured");
> >> -     aspeed_clk_data->hws[ASPEED_CLK_HPLL] = aspeed_ast2400_calc_pll("hpll", val);
> >> +     if (val & AST2400_HPLL_PROGRAMMED) {
> >> +             aspeed_clk_data->hws[ASPEED_CLK_HPLL] =
> >> +                     aspeed_ast2400_calc_pll("hpll", val);
> >> +     } else {
> >> +             /* hpll is configured by the strap register */
> >> +             regmap_read(map, ASPEED_STRAP, &val);
> >> +             val = (val >> 8) & 0x3;
> >> +             if (val == 0x00)
> >> +                     freq = 384000000;
> >> +             else if (val == 0x01)
> >> +                     freq = 360000000;
> >> +             else if (val == 0x02)
> >> +                     freq = 336000000;
> >> +             else if (val == 0x03)
> >> +                     freq = 408000000;
> >> +             aspeed_clk_data->hws[ASPEED_CLK_HPLL] =
> >> +                     clk_hw_register_fixed_rate(NULL, "hpll", "clkin", 0, freq);
> >> +     }
> >
> > Logic looks fine, but I feel this might be better as a lookup table. Bikeshed will be hot pink.
> 
> I anticipated your review, and sent v3 last week with a lookup table.
> I also merged it into the openbmc tree :)

Hah, right after I sent it I saw there was v3 and :facepalm:ed. I've got to get more responsive with mail :/