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