On Wed, Apr 03, 2024 at 04:59:40PM +0200, Krzysztof Kozlowski wrote: > On 03/04/2024 12:42, 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> > > --- > > 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 | 87 +++++++++++++++++++ > > 2 files changed, 90 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..0b076b0cf880 > > --- /dev/null > > +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h > > @@ -0,0 +1,87 @@ > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > > +#ifndef INTERCONNECT_QCOM_IPQ9574_H > > +#define INTERCONNECT_QCOM_IPQ9574_H > > + > > +#define ICC_ANOC_PCIE0 0 > > +#define ICC_SNOC_PCIE0 1 > > +#define ICC_ANOC_PCIE1 2 > > +#define ICC_SNOC_PCIE1 3 > > +#define ICC_ANOC_PCIE2 4 > > +#define ICC_SNOC_PCIE2 5 > > +#define ICC_ANOC_PCIE3 6 > > +#define ICC_SNOC_PCIE3 7 > > +#define ICC_SNOC_USB 8 > > +#define ICC_ANOC_USB_AXI 9 > > +#define ICC_NSSNOC_NSSCC 10 > > +#define ICC_NSSNOC_SNOC_0 11 > > +#define ICC_NSSNOC_SNOC_1 12 > > +#define ICC_NSSNOC_PCNOC_1 13 > > +#define ICC_NSSNOC_QOSGEN_REF 14 > > +#define ICC_NSSNOC_TIMEOUT_REF 15 > > +#define ICC_NSSNOC_XO_DCD 16 > > +#define ICC_NSSNOC_ATB 17 > > +#define ICC_MEM_NOC_NSSNOC 18 > > +#define ICC_NSSNOC_MEMNOC 19 > > +#define ICC_NSSNOC_MEM_NOC_1 20 > > + > > +#define ICC_NSSNOC_PPE 0 > > +#define ICC_NSSNOC_PPE_CFG 1 > > +#define ICC_NSSNOC_NSS_CSR 2 > > +#define ICC_NSSNOC_IMEM_QSB 3 > > +#define ICC_NSSNOC_IMEM_AHB 4 > > + > > +#define MASTER_ANOC_PCIE0 (ICC_ANOC_PCIE0 * 2) > > +#define SLAVE_ANOC_PCIE0 ((ICC_ANOC_PCIE0 * 2) + 1) > > Which existing Qualcomm platform has such code? Existing Qualcomm platforms don't use icc-clk. They use icc-rpm or icc-rpmh. clk-cbf-msm8996.c is the only driver that uses icc-clk. The icc_clk_register automatically creates master & slave nodes for each clk entry provided as input with the node-ids 'n' and 'n+1'. Since clk-cbf-msm8996.c has only one entry, it could just define MASTER_CBF_M4M and SLAVE_CBF_M4M with 0 and 1 and avoid these calculations. However, ipq9574 gives an array of clock entries as input to icc_clk_register. To tie the order/sequence of these clock entries correctly with the node-ids, this calculation is needed. > This is the third time I am asking for consistent headers. Open > existing, recently added headers and look how it is done there. Why? > Because I am against such calculations and see no reason for them. Apologies. Regret that I have to trouble you. In this ipq9574 case, have to reconcile between the following feedbacks. 1. https://lore.kernel.org/linux-arm-msm/fe40b307-26d0-4b2a-869b-5d093415b9d1@xxxxxxxxxx/ We could probably use indexed identifiers here to avoid confusion: [ICC_BINDING_NAME] = CLK_BINDING_NAME 2. https://lore.kernel.org/linux-arm-msm/95f4e99a60cc97770fc3cee850b62faf.sboyd@xxxxxxxxxx/ Are these supposed to be in a dt-binding header? 3. https://lore.kernel.org/linux-arm-msm/031d0a35-b192-4161-beef-97b89d5d1da6@xxxxxxxxxx/ Do you use them as well in the DTS? Having the defines (with the calculations) seemed to to comply with the above three feedbacks. Please let me know if this can be handled in a different way that would be consistent with other Qualcomm platforms. Thanks Varada
