[U-Boot] [PATCH v5 15/26] clk: sunxi: Add ccu clock tree support

[Maxime Ripard]

On Mon, Jan 07, 2019 at 02:09:12PM +0000, Andre Przywara wrote:
> > > What is MISC, exactly? Seems like an artefact clock to me, some
> > > placeholder you need because gate clocks are handled separately in
> > > the gates struct. Should this be called something with SIMPLE
> > > instead, or GATE? 
> > > > + * @CCU_CLK_TYPE_FIXED:			fixed clock type
> > > > + * @CCU_CLK_TYPE_MP:			mp clock type
> > > > + * @CCU_CLK_TYPE_NK:			nk clock type  
> > > 
> > > What is the point of those comments, as you are basically repeating
> > > the enum name? What about:
> > >  * @CCU_CLK_TYPE_PLL:		PLL clock with two multiplier
> > > fields  
> > 
> > We have PLL with 2 multipliers, but also others with other factors
> > sets, so that will end up being confusing. If the MP, NK and so on
> > stuff is confusing, maybe we should just add a comment on top of that
> > structure to explain what those factors are and what it actually
> > means?
> 
> Fair enough, or we name it CCU_CLK_TYPE_PLL_NK, because this is what
> this type deals with. Point is that by chance I happened to know about
> those naming of factors in the manual, but other might be lost by just
> seeing "mp" or "nk", without any explanation - and the comment doesn't
> help here at all.

Either way, we should really document this properly.

> The other part is that the "TYPE_MP" is twice as confusing, as it can
> perfectly describe the MMC clocks, which use "N" and "M" in the A64
> manual, for instance. That's why my suggestion for calling a spade a
> spade and saying it's a divider clock with a multiplexer. Happy to have
> the Linux naming in the comments.

NM and MP aren't really the same though. NM is one multiplier and one
divider, while MP is one divider and one right shift.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190107/6e86550c/attachment.sig>