Re: [PATCH 5/8] dt-bindings: clock: qcom: Add NSS clock controller for IPQ5424 SoC

[Luo Jie]

On 6/17/2025 10:49 PM, Krzysztof Kozlowski wrote:
> On 17/06/2025 14:06, Luo Jie wrote:
> > NSS clock controller provides the clocks and resets to the
> > networking blocks such as PPE (Packet Process Engine) and
> > UNIPHY (PCS) on IPQ5424 devices.
>
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

OK.

> > Add the compatible "qcom,ipq5424-nsscc" support based on the
> > current IPQ9574 NSS clock controller DT binding file.
> > ICC clocks are always provided by the NSS clock controller
> > of IPQ9574 and IPQ5424, so add interconnect-cells as required
> > DT property.
> >
> > Also add master/slave ids for IPQ5424 networking interfaces,
> > which is used by nss-ipq5424 driver for providing interconnect
> > services using icc-clk framework.
> >
> > Signed-off-by: Luo Jie <quic_luoj@xxxxxxxxxxx>
> > ---
> >   .../bindings/clock/qcom,ipq9574-nsscc.yaml         | 66 +++++++++++++++++++---
> >   include/dt-bindings/clock/qcom,ipq5424-nsscc.h     | 65 +++++++++++++++++++++
> >   include/dt-bindings/interconnect/qcom,ipq5424.h    | 13 +++++
> >   include/dt-bindings/reset/qcom,ipq5424-nsscc.h     | 46 +++++++++++++++
> >   4 files changed, 182 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
> > index 17252b6ea3be..5bc2fe049b26 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-nsscc.yaml
> > @@ -4,7 +4,7 @@
> >   $id: http://devicetree.org/schemas/clock/qcom,ipq9574-nsscc.yaml#
> >   $schema: http://devicetree.org/meta-schemas/core.yaml#
> >   
> > -title: Qualcomm Networking Sub System Clock & Reset Controller on IPQ9574
> > +title: Qualcomm Networking Sub System Clock & Reset Controller on IPQ9574 and IPQ5424
> >   
> >   maintainers:
> >     - Bjorn Andersson <andersson@xxxxxxxxxx>
> > @@ -12,21 +12,25 @@ maintainers:
> >   
> >   description: |
> >     Qualcomm networking sub system clock control module provides the clocks,
> > -  resets on IPQ9574
> > +  resets on IPQ9574 and IPQ5424
> >   
> > -  See also::
> > +  See also:
> > +    include/dt-bindings/clock/qcom,ipq5424-nsscc.h
> >       include/dt-bindings/clock/qcom,ipq9574-nsscc.h
> > +    include/dt-bindings/reset/qcom,ipq5424-nsscc.h
> >       include/dt-bindings/reset/qcom,ipq9574-nsscc.h
> >   
> >   properties:
> >     compatible:
> > -    const: qcom,ipq9574-nsscc
> > +    enum:
> > +      - qcom,ipq5424-nsscc
> > +      - qcom,ipq9574-nsscc
> >   
> >     clocks:
> >       items:
> >         - description: Board XO source
> > -      - description: CMN_PLL NSS 1200MHz (Bias PLL cc) clock source
> > -      - description: CMN_PLL PPE 353MHz (Bias PLL ubi nc) clock source
> > +      - description: CMN_PLL NSS 1200 MHz or 300 MHZ (Bias PLL cc) clock source
> > +      - description: CMN_PLL PPE 353 MHz  or 375 MHZ (Bias PLL ubi nc) clock source
>
> This change means devices are different. Just ocme with your own schema.

The NSS clock controller hardware block on the IPQ5424 SoC is identical
in design to that of the IPQ9574 SoC. The main difference is in the
clock rates for its two parent clocks sourced from the CMN PLL block.

Given this, would it be acceptable to update the clock name and its
description to use a more generic clock name, such as "nss" and "ppe"
instead of the current "nss_1200" and "ppe_353"?

> Best regards,
> Krzysztof