Re: [PATCH 02/12] drm/ast: Handle configuration without P2A bridge

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

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.

> > +       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"

> > +               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".

> > +       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;
> >  }