[PATCHv4 00/15] clk: ti: add support for hwmod clocks

[Michael Turquette]

Quoting Tony Lindgren (2016-12-13 07:37:24)
> * Tero Kristo <t-kristo at ti.com> [161213 00:31]:
> > On 13/12/16 06:40, Michael Turquette wrote:
> > > Quoting Tony Lindgren (2016-12-12 17:31:34)
> > > > * Stephen Boyd <sboyd at codeaurora.org> [161212 16:49]:
> > > > > I spent a bunch of time looking at this again today. From a DT
> > > > > perspective we don't want to have clocks or clockdomains nodes
> > > > > below the cm1/cm2/prm dt nodes. That's getting to the point of
> > > > > describing individual elements of a device that should be
> > > > > described in the driver instead of DT.
> > > > 
> > > > I agree we don't need separate clocks and clockdomain nodes.. But
> > > > I think you're missing something here though. The clockdomains in
> > > > this case are separate devices on the interconnect, not individual
> > > > elements within a device. The outputs of a clockdomain are individual
> > > > elements of a clockdomain and can be just described as indexed
> > > > outputs of the clockdomain.
> > > 
> > > Is the goal to describe this hardware topology in DT? Is that right
> > > thing to do? I think it's cool to have this modeled *somehow* in Linux,
> > > but I'm not sure DT is the right place to model the interconnect and
> > > every device hanging off of it.
> 
> Well struct device is what we should use, the DT nodes pretty much
> map with that :)
> 
> > > I don't want to put words in Stephen's mouth, but I think the issue over
> > > whether clockdomains are CCF clock providers or some genpd thing is
> > > probably less important to him than the fact that the DT bindings are
> > > super detailed to inner workings of the SoC.
> > 
> > Ok, so your preference would be to reduce the data under DT, and the ideal
> > approach would be a single prcm node. I think we still need to keep the prm
> > / cm1 / cm2 as separate nodes, as they are pretty individual from hardware
> > point of view, provide quite different features, and they reside in some
> > cases in quite different address spaces also. Anyway, here's what I gather
> > we should probably have in DT:
> > 
> > - reset provider
> >   * example: resets = <&prm OMAP4_IVA2_RESET>;
> >   * only from 'prm' node
> > 
> > - genpd provider (for the hwmods, clockdomains, powerdomains, voltage
> > domains)
> >   * examples: power-domains = <&cm2 OMAP4_DSS_CORE_MOD>;
> >               power-domains = <&cm2 OMAP4_DSS_CLKDM>;
> >               power-domains = <&prm OMAP4_DSS_PWRDM>;
> >               power-domains = <&prm OMAP4_CORE_VOLTDM>;
> >   * from all 'prm', 'cm1' and 'cm2' nodes, though 'prm' would be the only
> > one providing _CLKDM, _PWRDM, _VOLTDM genpds.
> > 
> > - clock provider (for anything that requires clocks)
> >   * example: clocks = <&cm1 OMAP4_DPLL_MPU_CK>;
> >   * from all 'prm', 'cm1' and 'cm2' nodes
> 
> Makes sense to me in general.
> 
> For the clkctrl clocks, here's what I'd like to see. The driver should be
> just a regular device driver along the lines we did with the ADPLL as in
> Documentation/devicetree/bindings/clock/ti/adpll.txt.
> 
> For the binding, something like the following should work as a minimal
> example, this follows what we have in the hardware:
> 
> &prm {
>         ...
> 
>         /* See "CD_WKUP Clock Domain" in 4430 TRM page 393 */
>         wkup_cm: clock at 1800 {
>                 compatible = "ti,clkctrl";
>                 reg = <0x1800 0x100>;
>                 #clock-cells = <1>;
>                 clocks = <&wkup_l4_iclk2 &wkup_32k_fclk
>                           &32k_fclk &gp1_fclk>;
>                 clock-output-names =
>                         "sysctrl_padconf_wkup",
>                         "badgap",
>                         "sysctrl_general_wkup",
>                         "gpio1",
>                         "keyboard",
>                         "sar_ram",
>                         "32ktimer",
>                         "gptimer1";

Is there a technical reason to use clock-output-names? If you share a
header between the clock provider driver and DT with the phandle offsets
then we should be able to avoid this property altogether. Stephen and I
are trying to phase this one out as much as possible.

Regards,
Mike

>         };
>         ...
> 
>         /* See "CD_EMU Clock Domain" in 4430 TRM page 424 */
>         emu_cm: clock at 1a00 {
>                 compatible = "ti,clkctrl";
>                 reg = <0x1a00 0x100>;
>                 #clock-cells = <1>;
>                 clocks = <&emu_sys_clk &core_dpll_emu_clk>;
>                 clock-output-names = "debug";
>         };
>         ...
> };
> 
> So the device tree nodes could be minimal as above and the rest can
> be taken care of by the driver. We may need separate compatible strings
> for the various instances, not sure about that.
> 
> Note that the clkctrl hardware manages multiple clocks for each
> interconnect target module. AFAIK we don't need to care about that from
> the consumer device driver point of view as it can't separately manage
> functional and interface clock.
> 
> We need few clockctrl clocks early for system timers, but those can
> be registered earlier in the driver.
> 
> Then some clkctrl clocks have optional aux clocks. I think those can
> be just be regular child clocks of the module clocks.
> 
> > This would eventually cause an ABI breakage for the clock handles, if we
> > transfer the existing clocks to this format, and remove the existing clock
> > handles from DT. Otherwise, I think we could just transition the existing
> > hwmod data to this new format only, and add the clockdomain / powerdomain /
> > voltagedomain support a bit later.
> 
> Let's not break anything while doing this.. And let's not mess with the
> hwmod except where it helps moving that into regular device drivers.
> If necessary we can maybe first register the new clock instances, then
> register the old clocks if new clock is not found?
> 
> Regards,
> 
> Tony