On Tue, Feb 14, 2017 at 06:26:39PM +0800, Chen-Yu Tsai wrote: > On Tue, Feb 14, 2017 at 5:58 PM, Maxime Ripard > <maxime.rip...@free-electrons.com> wrote: > > On Tue, Feb 14, 2017 at 11:35:25AM +0800, Chen-Yu Tsai wrote: > >> +/* Some PLLs are input * N / div1 / P. Model them as NKMP with no K */ > > > > Is that even working? > > Looking at the nkmp clock code, only .recalc_rate will work properly though. > Maybe I could fix up the code so it handles zero width factors. > > > I'm not quite sure we want to do that. We might model it as a NP clock > > with a variable prediv? > > There's no NP clock type yet. And a problem with a variable prediv is that > it doesn't participate in factor calculation. It's effectively fixed. > > I did this for the A80 as well though. Fixing up the NKMP clock might be > easier.
Then maybe we just need a NMP clock type then. What I'm really afraid
of is that we'll just end up in a clk-factors situation that was
simply impossible to maintain without breaking anything, hence why we
had different clock types then.
> >
> >> +/* Use a separate clock for the pre-divider on the AHB1 PLL-PERIPH input
> >> */
> >> +static SUNXI_CCU_M(pll_periph_ahb1_clk, "pll-periph-ahb1", "pll-periph",
> >> + 0x054, 6, 2, 0);
> >> +
> >> +static const char * const ahb1_parents[] = { "osc16M-d512", "osc24M",
> >> + "pll-periph-ahb1",
> >> + "pll-periph-ahb1" };
> >> +static struct ccu_div ahb1_clk = {
> >> + .div = _SUNXI_CCU_DIV_FLAGS(4, 2,
> >> CLK_DIVIDER_POWER_OF_TWO),
> >> + .mux = _SUNXI_CCU_MUX(12, 2),
> >> + .common = {
> >> + .reg = 0x054,
> >> + .hw.init = CLK_HW_INIT_PARENTS("ahb1",
> >> + ahb1_parents,
> >> + &ccu_div_ops,
> >> + 0),
> >> + },
> >> +};
> >
> > What's different from a pre divider only for a given index here?
>
> The variable pre-divider is shared for both pll-periph mux inputs.
> This is one way to handle it. The other would be to extend ccu_mux
> to handle multiple variable pre-dividers. I don't really want to do
> that if this is the only instance that needs it though.
Every addition we made was only needed by one instance at first :)
We are working that way for fixed pre-dividers already, I don't see
why we can't have it for variable ones too.
> >> +/*
> >> + * MMC2 supports what's called the "new timing mode". The CCU and the MMC
> >> + * controller must be in sync about which mode is used. The new mode moves
> >> + * the clock delay controls (and possibly the delay lines) into the MMC
> >> + * block. Also, the output of the clock is divided by 2. The output and
> >> + * sample phase clocks are unused under this mode.
> >> + *
> >> + * This new mode seems to be preferred. Hence we force this clock to the
> >> + * new mode. And we don't add the phase clocks.
> >> + */
> >
> > I'm sorry, but I said this several times, this isn't working. We
> > should model it properly, and not hack this around in the clock
> > driver.
> >
> > As you say in your comment, the MMC driver needs to be aware about
> > which mode is used, in order to also set a bit in one of its registers
> > accordingly, and modify its sampling behaviour.
> >
> > The new timing is preferred, but our previous clock implementations
> > didn't hardcode it, so we can't even rely on that behaviour to always
> > write it in our driver.
>
> Correct. With the A83T there has never been a merged clock driver though.
> I realize this is a one off thing.
>
> > This is not something specific to the A83T, but is found in all the
> > SoCs since the A23, so we need to come up with a good solution to
> > address that.
> >
> > I'm not sure what a good solution would be though. One would be to
> > just have a private function of our own to switch in the new mode (if
> > relevant, because only the MMC2 controllers have it), but that would
> > lead to troubles with !sunxi-ng. Not something we can't deal with, but
> > some extra precautions should be taken (make sure to protect the call
> > through an ifdef / IS_DEFINED, check that the sunxi-ng driver has been
> > probed, etc.)
>
> If the custom function route is acceptable, I'll come up with something.
I think it would be a great start yes. I'll try to discuss it with
Mike and Stephen at ELC and see what they think about that.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
signature.asc
Description: PGP signature
