Re: [PATCH V3 4/9] dt-bindings: clock: Add Qualcomm IPQ5332 GCC

[Kathiravan T]

Thanks Dmirty for taking time to review the patch.


On 2/6/2023 3:22 PM, Dmitry Baryshkov wrote:
> On 06/02/2023 09:12, Kathiravan T wrote:
> > Add binding for the Qualcomm IPQ5332 Global Clock Controller.
> >
> > Signed-off-by: Kathiravan T <quic_kathirav@xxxxxxxxxxx>
> > ---
> > Changes in V3:
> >     - Actually I missed to remove the clocks in V2 which are supposed to
> >       be removed. In V3 I have removed those and they are
> >       GCC_APSS_AHB_CLK, GCC_APSS_AHB_CLK_SRC, GCC_APSS_AXI_CLK
> >     - For the same, didn't add the Reviewed-By tags from Stephen and
> >       Krzysztof
> >
> > Changes in V2:
> >     - property 'clocks' is marked required
> >     - Renamed the include file name to match with compatible
> >
> >   .../bindings/clock/qcom,ipq5332-gcc.yaml      |  61 +++
> >   include/dt-bindings/clock/qcom,ipq5332-gcc.h  | 356 ++++++++++++++++++
> >   2 files changed, 417 insertions(+)
> >   create mode 100644 
> > Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml
> >   create mode 100644 include/dt-bindings/clock/qcom,ipq5332-gcc.h
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml 
> > b/Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml
> > new file mode 100644
> > index 000000000000..961311af400c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq5332-gcc.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/qcom,ipq5332-gcc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Global Clock & Reset Controller on IPQ5332
> > +
> > +maintainers:
> > +  - Stephen Boyd <sboyd@xxxxxxxxxx>
> > +
> > +description: |
> > +  Qualcomm global clock control module provides the clocks, resets 
> > and power
> > +  domains on IPQ5332.
> > +
> > +  See also:: include/dt-bindings/clock/qcom,gcc-ipq5332.h
> > +
> > +allOf:
> > +  - $ref: qcom,gcc.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: qcom,ipq5332-gcc
> > +
> > +  clocks:
> > +    items:
> > +      - description: Board XO clock source
> > +      - description: Sleep clock source
> > +      - description: PCIE 2lane PHY pipe clock source
> > +      - description: PCIE 2lane x1 PHY pipe clock source (For second 
> > lane)
> > +      - description: USB PCIE wrapper pipe clock source
> > +
> > +  clock-names:
> > +    items:
> > +      - const: xo
> > +      - const: sleep_clk
> > +      - const: pcie_2lane_phy_pipe_clk
> > +      - const: pcie_2lane_phy_pipe_clk_x1
> > +      - const: usb_pcie_wrapper_pipe_clk
>
> pcie3x1_0_pipe_clk_src, pcie3x1_1_pipe_clk_src, pcie3x2_pipe_clk_src 
> usb0_pipe_clk_src are missing.


Here is the mapping,

pcie_2lane_phy_pipe_clk  is the source for pcie3x2,

pcie_2lane_phy_pipe_clk_x1 is the source for pcie3x1_1,

usb_pcie_wrapper_pipe_clk is the source for pcie3x1_0_pipe_clk_src and 
usb0_pipe_clk_src.

Is this what you are expecting or am I missing something here?

Thanks,

Kathiravan T.


> > +
> > +required:
> > +  - compatible
> > +  - clocks
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    clock-controller@1800000 {
> > +      compatible = "qcom,ipq5332-gcc";
> > +      reg = <0x01800000 0x80000>;
> > +      clocks = <&xo_board>,
> > +               <&sleep_clk>,
> > +               <&pcie_2lane_phy_pipe_clk>,
> > +               <&pcie_2lane_phy_pipe_clk_x1>,
> > +               <&usb_pcie_wrapper_pipe_clk>;
> > +      #clock-cells = <1>;
> > +      #power-domain-cells = <1>;
> > +      #reset-cells = <1>;
> > +    };
> > +...