Messages in this thread | | | From | Chen-Yu Tsai <> | Date | Tue, 14 Feb 2017 18:26:39 +0800 | Subject | Re: [PATCH 4/5] clk: sunxi-ng: Add driver for A83T CCU |
| |
On Tue, Feb 14, 2017 at 5:58 PM, Maxime Ripard <maxime.ripard@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.
> >> +/* 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.
> >> +/* >> + * 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.
Regards ChenYu
> > Or we could introduce a new clk_ops function pointer, but I'm not sure > if Mike and Stephen are going to be happy with that. > > Thanks, > Maxime > > -- > Maxime Ripard, Free Electrons > Embedded Linux and Kernel engineering > http://free-electrons.com
| |