On Fri, 2017-02-24 at 12:51 +1030, Joel Stanley wrote:

Are these properties supposed to repeat the prefix "ast,ast"?

We've chosen aspeed as the vendor prefix for Aspeed stuff.

Sent my reply too early ... so yes, I can change that, our FW hasn't
merge the FW side yet. I'll respin now.

quoted

+       if (mcr_scu_strap & 0x2000)

This bit confused me. Bit 13 of the strap (SCU70) is the SPI mode.

The register is actually "MCR170: AST2000 Backward Compatible SCU
Hardware Strapping Value"

quoted

+               ref_pll = 14318;
+       else
+               ref_pll = 12000;
+
+       denum = mcr_scu_mpll & 0x1f;
+       num = (mcr_scu_mpll & 0x3fe0) >> 5;
+       dsel = (mcr_scu_mpll & 0xc000) >> 14;

These calculations don't make sense for the ast2400 or ast2500.

They do if you look at this:

MCR120: AST2000 Backward Compatible SCU MPLL Parameter

It's not the SCU version of the register it's the MCU "copy" of it
that maintains some kind of legacy layout. Hence "mcr_scu" prefix
not "scu".

quoted

+       switch (dsel) {
+       case 3:
+               div = 0x4;
+               break;
+       case 2:
+       case 1:
+               div = 0x2;
+               break;
+       default:
+               div = 0x1;
+               break;
        }
+       ast->mclk = ref_pll * (num + 2) / (denum + 2) * (div *
1000);
        return 0;
 }