Re: [PATCH v7 1/7] dt-bindings: clock: Convert qcom,krait-cc to yaml

[Christian Marangi <ansuelsmth@gmail.com>]

On Mon, Jan 16, 2023 at 11:29:04PM +0200, Dmitry Baryshkov wrote:
> 16 января 2023 г. 23:20:17 GMT+02:00, Christian Marangi <ansuelsmth@gmail.com> пишет:
> >On Mon, Jan 16, 2023 at 11:14:10PM +0200, Dmitry Baryshkov wrote:
> >> On Mon, 16 Jan 2023 at 22:48, Christian Marangi <ansuelsmth@gmail.com> wrote:
> >> >
> >> > Convert qcom,krait-cc to yaml Documentation.
> >> >
> >> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> >> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> >> > Acked-by: Rob Herring <robh@kernel.org>
> >> 
> >> I know this has been reviewed already. I checked again my apq8064
> >> branch and noticed that the bindings are not compatible with the
> >> apq8064. The SoC in question is a 4-core device, so this is what I had
> >> in mind:
> >> 
> >>        kraitcc: clock-controller {
> >>                compatible = "qcom,krait-cc-v1";
> >>                clocks = <&gcc PLL9>, /* hfpll0 */
> >>                         <&gcc PLL10>, /* hfpll1 */
> >>                         <&gcc PLL16>, /* hfpll2 */
> >>                         <&gcc PLL17>, /* hfpll3 */
> >>                         <&gcc PLL12>, /* hfpll_l2 */
> >>                         <&acc0>,
> >>                         <&acc1>,
> >>                         <&acc2>,
> >>                         <&acc3>,
> >>                         <&l2cc>;
> >>                clock-names = "hfpll0",
> >>                              "hfpll1",
> >>                              "hfpll2",
> >>                              "hfpll3",
> >>                              "hfpll_l2",
> >>                              "acpu0_aux",
> >>                              "acpu1_aux",
> >>                              "acpu2_aux",
> >>                              "acpu3_aux",
> >>                              "acpu_l2_aux";
> >>                #clock-cells = <1>;
> >>        };
> >> 
> >
> >Oh wow... Wasn't aware, but I wonder if a good idea would be to patch
> >the yaml if and when this series is merged? This is a direct conversion
> >of the krait-cc txt...
> >
> >If we really want I can send a v8 with adding these extra stuff to the
> >just converted yaml in a different commit? (afaik conversion should not
> >add more info if the documentation is not broken enough and can be
> >converted)
> 
> I think we can ask Bjorn to pick the rest of the patches (if they are acked/reviewed, they look good to me) and work on this one. It might make sense to add per-SoC compatibles. Also note, the qsb clock. Which block generates it on ipq8064?
>

The krait-cc driver is really very generic and modular so IMHO it
doesn't require different compatibiles... But now that I think about
it... It may be problematic to differentiate them for the clock
configuration...

Btw the qsb clk from krait-cc driver is still a fixed clock set to 1.
In hw side it's the bus clk and it's driven by rpm... (this link is
totally lacking from krait-cc and I think nobody was aware of this
(except old qcom)... To find out this I had to resurrect a pre-DT
migration driver that use the cxo clk to perform hw measurement of the
clk and I discovered that. (cpu and l2 clk can be measured and by
setting the mux to sourcing out of qsb you can notice that the freq will
always match the bus frequency)

But I got sidetracked by too much discoveries... ipq806x simply don't
have a krait-cc driver currently so the qsb doesn't apply...

My idea is to push kriat-cc node + opp node.

> >
> >> > ---
> >> >  .../bindings/clock/qcom,krait-cc.txt          | 34 -----------
> >> >  .../bindings/clock/qcom,krait-cc.yaml         | 59 +++++++++++++++++++
> >> >  2 files changed, 59 insertions(+), 34 deletions(-)
> >> >  delete mode 100644 Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
> >> >  create mode 100644 Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/clock/qcom,krait-cc.txt b/Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
> >> > deleted file mode 100644
> >> > index 030ba60dab08..000000000000
> >> > --- a/Documentation/devicetree/bindings/clock/qcom,krait-cc.txt
> >> > +++ /dev/null
> >> > @@ -1,34 +0,0 @@
> >> > -Krait Clock Controller
> >> > -
> >> > -PROPERTIES
> >> > -
> >> > -- compatible:
> >> > -       Usage: required
> >> > -       Value type: <string>
> >> > -       Definition: must be one of:
> >> > -                       "qcom,krait-cc-v1"
> >> > -                       "qcom,krait-cc-v2"
> >> > -
> >> > -- #clock-cells:
> >> > -       Usage: required
> >> > -       Value type: <u32>
> >> > -       Definition: must be 1
> >> > -
> >> > -- clocks:
> >> > -       Usage: required
> >> > -       Value type: <prop-encoded-array>
> >> > -       Definition: reference to the clock parents of hfpll, secondary muxes.
> >> > -
> >> > -- clock-names:
> >> > -       Usage: required
> >> > -       Value type: <stringlist>
> >> > -       Definition: must be "hfpll0", "hfpll1", "acpu0_aux", "acpu1_aux", "qsb".
> >> > -
> >> > -Example:
> >> > -
> >> > -       kraitcc: clock-controller {
> >> > -               compatible = "qcom,krait-cc-v1";
> >> > -               clocks = <&hfpll0>, <&hfpll1>, <&acpu0_aux>, <&acpu1_aux>, <qsb>;
> >> > -               clock-names = "hfpll0", "hfpll1", "acpu0_aux", "acpu1_aux", "qsb";
> >> > -               #clock-cells = <1>;
> >> > -       };
> >> > diff --git a/Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml b/Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml
> >> > new file mode 100644
> >> > index 000000000000..8caa5a677394
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/clock/qcom,krait-cc.yaml
> >> > @@ -0,0 +1,59 @@
> >> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >> > +%YAML 1.2
> >> > +---
> >> > +$id: http://devicetree.org/schemas/clock/qcom,krait-cc.yaml#
> >> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> > +
> >> > +title: Qualcomm Krait Clock Controller
> >> > +
> >> > +maintainers:
> >> > +  - Christian Marangi <ansuelsmth@gmail.com>
> >> > +
> >> > +description: |
> >> > +  Qualcomm Krait Clock Controller used to correctly scale the CPU and the L2
> >> > +  rates.
> >> > +
> >> > +properties:
> >> > +  compatible:
> >> > +    enum:
> >> > +      - qcom,krait-cc-v1
> >> > +      - qcom,krait-cc-v2
> >> > +
> >> > +  clocks:
> >> > +    items:
> >> > +      - description: phandle to hfpll for CPU0 mux
> >> > +      - description: phandle to hfpll for CPU1 mux
> >> > +      - description: phandle to CPU0 aux clock
> >> > +      - description: phandle to CPU1 aux clock
> >> > +      - description: phandle to QSB fixed clk
> >> > +
> >> > +  clock-names:
> >> > +    items:
> >> > +      - const: hfpll0
> >> > +      - const: hfpll1
> >> > +      - const: acpu0_aux
> >> > +      - const: acpu1_aux
> >> > +      - const: qsb
> >> > +
> >> > +  '#clock-cells':
> >> > +    const: 1
> >> > +
> >> > +required:
> >> > +  - compatible
> >> > +  - clocks
> >> > +  - clock-names
> >> > +  - '#clock-cells'
> >> > +
> >> > +additionalProperties: false
> >> > +
> >> > +examples:
> >> > +  - |
> >> > +    clock-controller {
> >> > +      compatible = "qcom,krait-cc-v1";
> >> > +      clocks = <&hfpll0>, <&hfpll1>,
> >> > +               <&acpu0_aux>, <&acpu1_aux>, <&qsb>;
> >> > +      clock-names = "hfpll0", "hfpll1",
> >> > +                    "acpu0_aux", "acpu1_aux", "qsb";
> >> > +      #clock-cells = <1>;
> >> > +    };
> >> > +...
> >> > --
> >> > 2.37.2
> >> >
> >> 
> >> 
> >> -- 
> >> With best wishes
> >> Dmitry
> >
> 

-- 
	Ansuel