Re: [PATCH 05/13] clk: qcom: apcs-msm8916: get parent clock names from DT

[Stephen Boyd]

Quoting Niklas Cassel (2018-12-18 06:35:03)
> 
> This is the existing DT nodes for msm8916:
> 
>                a53pll: clock@b016000 {
>                         compatible = "qcom,msm8916-a53pll";
>                         reg = <0xb016000 0x40>;
>                         #clock-cells = <0>;
>                 };
> 
>                 apcs: mailbox@b011000 {
>                         compatible = "qcom,msm8916-apcs-kpss-global", "syscon";
>                         reg = <0xb011000 0x1000>;
>                         #mbox-cells = <1>;
>                         clocks = <&a53pll>;
>                         #clock-cells = <0>;
>                 };
> 
> 
> This is the (suggested) DT nodes for qcs404:
> 
>                 apcs_hfpll: clock-controller@0b016000 {
>                         compatible = "qcom,hfpll";
>                         reg = <0x0b016000 0x30>;
>                         #clock-cells = <0>;
>                         clock-output-names = "apcs_hfpll";
>                         clocks = <&xo_board>;
>                         clock-names = "xo";
>                 };
> 
>                 apcs_glb: mailbox@b011000 {
>                         compatible = "qcom,qcs404-apcs-apps-global", "syscon";
>                         reg = <0x0b011000 0x1000>;
>                         #mbox-cells = <1>;
>                         clocks = <&gcc GCC_GPLL0_AO_OUT_MAIN>, <&apcs_hfpll>;
>                         clock-names = "aux", "pll";
>                         #clock-cells = <0>;
>                 };
> 
> qcs404 specifies two clocks, with an accompanied clock-name for each clock.
> 
> msm8916 specifies a single clock, without an accompanied clock-name.
> 
> It is possible to append clock-names = "pll" for the existing clock,
> as well as to define the aux clock in the apcs node in the msm8916 DT:
> clocks = <&gcc GPLL0_VOTE>;
> clock-names = "aux";
> 
> However, since the DT is treated as an ABI, the existing DT for msm8916 must
> still work, so I don't think that it is possible to ignore having backwards
> compability in the apcs clock driver.
> 

We typically allow one clk to match the NULL connection name, so I think
things should work if you clk_get(dev, NULL) on 8916 and then try the
specific "aux" and "pll" strings strings after that for updated DT. Not
sure if that helps you here though.

And I would push to try and make all the clk connections between clk
controller nodes specified now. It will be useful to avoid global string
lookups in the future, so the sooner the better.