Re: [PATCH v8 3/7] dt-bindings: interconnect: Add Qualcomm IPQ9574 support

From: Varadarajan Narayanan
Date: Wed Apr 17 2024 - 12:38:58 EST


On Wed, Apr 17, 2024 at 04:07:59PM +0200, Krzysztof Kozlowski wrote:
> On 17/04/2024 12:56, Varadarajan Narayanan wrote:
> > Add interconnect-cells to clock provider so that it can be
> > used as icc provider.
> >
> > Add master/slave ids for Qualcomm IPQ9574 Network-On-Chip
> > interfaces. This will be used by the gcc-ipq9574 driver
> > that will for providing interconnect services using the
> > icc-clk framework.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> > ---
> > v8:
> > Remove ICC_xxx macros
> > Fix macro defines to be consistent with other bindings
> > v7:
> > Fix macro names to be consistent with other bindings
> > v6:
> > Removed Reviewed-by: Krzysztof Kozlowski
> > Redefine the bindings such that driver and DT can share them
> >
> > v3:
> > Squash Documentation/ and include/ changes into same patch
> >
> > qcom,ipq9574.h
> > Move 'first id' to clock driver
> >
> > ---
> > .../bindings/clock/qcom,ipq9574-gcc.yaml | 3 +
> > .../dt-bindings/interconnect/qcom,ipq9574.h | 59 +++++++++++++++++++
> > 2 files changed, 62 insertions(+)
> > create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > index 944a0ea79cd6..824781cbdf34 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,ipq9574-gcc.yaml
> > @@ -33,6 +33,9 @@ properties:
> > - description: PCIE30 PHY3 pipe clock source
> > - description: USB3 PHY pipe clock source
> >
> > + '#interconnect-cells':
> > + const: 1
> > +
> > required:
> > - compatible
> > - clocks
> > diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h
> > new file mode 100644
> > index 000000000000..42019335c7dd
> > --- /dev/null
> > +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +#ifndef INTERCONNECT_QCOM_IPQ9574_H
> > +#define INTERCONNECT_QCOM_IPQ9574_H
> > +
> > +#define MASTER_ANOC_PCIE0 0
> > +#define SLAVE_ANOC_PCIE0 1
>
> I still do not see any usage of it. At least symbol cannot be resolved.
> I assume you use the value, otherwise it would mean our entire feedback
> was ignored, but then why this cannot be searchable?
>
> Again, open existing drivers and look how it is there. Not being able to
> find the constant is not good.

It is used in the 6th patch in drivers/clk/qcom/gcc-ipq9574.c via the
HWS_DATA macro. Will remove the macro and use it explicitly (as suggested
by Dmitry also).

Thanks
Varada