[PATCH linux dev-5.4] clk: ast2600: Fix AHB clock divider for A1

[Andrew Jeffery]

On Fri, 10 Apr 2020, at 06:59, Eddie James wrote:
> 
> On 4/8/20 11:53 PM, Andrew Jeffery wrote:
> >
> > On Thu, 9 Apr 2020, at 05:57, Eddie James wrote:
> >> The latest specs for the AST2600 A1 chip include some different bit
> >> definitions for calculating the AHB clock divider. Implement these in
> >> order to get the correct AHB clock value in Linux.
> >>
> >> Signed-off-by: Eddie James <eajames at linux.ibm.com>
> >> ---
> >>   drivers/clk/clk-ast2600.c | 31 +++++++++++++++++++++++++------
> >>   1 file changed, 25 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/clk/clk-ast2600.c b/drivers/clk/clk-ast2600.c
> >> index 42bfdc16bf7a..35f53956c762 100644
> >> --- a/drivers/clk/clk-ast2600.c
> >> +++ b/drivers/clk/clk-ast2600.c
> >> @@ -642,14 +642,22 @@ static const u32 ast2600_a0_axi_ahb_div_table[] = {
> >>   	2, 2, 3, 5,
> >>   };
> >>   
> >> -static const u32 ast2600_a1_axi_ahb_div_table[] = {
> >> -	4, 6, 2, 4,
> >> +static const u32 ast2600_a1_axi_ahb_div0_tbl[] = {
> >> +	3, 2, 3, 4,
> >> +};
> >> +
> >> +static const u32 ast2600_a1_axi_ahb_div1_tbl[] = {
> >> +	3, 4, 6, 8,
> >> +};
> >> +
> >> +static const u32 ast2600_a1_axi_ahb200_tbl[] = {
> >> +	3, 4, 3, 4, 2, 2, 2, 2,
> >>   };
> >>   
> >>   static void __init aspeed_g6_cc(struct regmap *map)
> >>   {
> >>   	struct clk_hw *hw;
> >> -	u32 val, div, chip_id, axi_div, ahb_div;
> >> +	u32 val, div, divbits, chip_id, axi_div, ahb_div;
> >>   
> >>   	clk_hw_register_fixed_rate(NULL, "clkin", NULL, 0, 25000000);
> >>   
> >> @@ -679,11 +687,22 @@ static void __init aspeed_g6_cc(struct regmap *map)
> >>   	else
> >>   		axi_div = 2;
> >>   
> >> +	divbits = (val >> 11) & 0x3;
> >>   	regmap_read(map, ASPEED_G6_SILICON_REV, &chip_id);
> >> -	if (chip_id & BIT(16))
> >> -		ahb_div = ast2600_a1_axi_ahb_div_table[(val >> 11) & 0x3];
> >> -	else
> >> +	if (chip_id & BIT(16)) {
> >> +		if (!divbits) {
> >> +			ahb_div = ast2600_a1_axi_ahb200_tbl[(val >> 8) & 0x3];
> >> +			if (val & BIT(16))
> >> +				ahb_div *= 2;
> >> +		} else {
> >> +			if (val & BIT(16))
> >> +				ahb_div = ast2600_a1_axi_ahb_div1_tbl[divbits];
> >> +			else
> >> +				ahb_div = ast2600_a1_axi_ahb_div0_tbl[divbits];
> >> +		}
> >> +	} else {
> >>   		ahb_div = ast2600_a0_axi_ahb_div_table[(val >> 11) & 0x3];
> >> +	}
> > This was hard for me to read. Have you considered giving the conditions
> > names?
> 
> 
> Yea it's a bit complicated. Do you mean use some boolean variables or 
> add some comments?

Yeah, pretty much that. I had a bit of a crack but then figured I'd just ask
the question :)

Andrew