| Message ID | 20240325102036.95484-3-quic_varada@quicinc.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add interconnect driver for IPQ9574 SoC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success |
On 25/03/2024 11:20, Varadarajan Narayanan wrote: > 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@quicinc.com> > --- > v2: Rename master slave macros > Fix license identifier Both patches should be squashed. Header is parts of bindings and your previous patch adds the interconnects, doesn't it? > --- > .../dt-bindings/interconnect/qcom,ipq9574.h | 62 +++++++++++++++++++ > 1 file changed, 62 insertions(+) > create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h > > diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h > new file mode 100644 > index 000000000000..b7b32aa6bbb1 > --- /dev/null > +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h > @@ -0,0 +1,62 @@ > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > +#ifndef INTERCONNECT_QCOM_IPQ9574_H > +#define INTERCONNECT_QCOM_IPQ9574_H > + > +#define IPQ_APPS_ID 9574 /* some unique value */ Why random unique values are bindings? Why this cannot be 0? Please explain how this is used by DTS and driver. > +#define IPQ_NSS_ID (IPQ_APPS_ID * 2) This does not seem right. Best regards, Krzysztof
On Tue, Mar 26, 2024 at 07:49:00AM +0100, Krzysztof Kozlowski wrote: > On 25/03/2024 11:20, Varadarajan Narayanan wrote: > > 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@quicinc.com> > > --- > > v2: Rename master slave macros > > Fix license identifier > > Both patches should be squashed. Header is parts of bindings and your > previous patch adds the interconnects, doesn't it? > > > > --- > > .../dt-bindings/interconnect/qcom,ipq9574.h | 62 +++++++++++++++++++ > > 1 file changed, 62 insertions(+) > > create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h > > > > diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h > > new file mode 100644 > > index 000000000000..b7b32aa6bbb1 > > --- /dev/null > > +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h > > @@ -0,0 +1,62 @@ > > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ > > +#ifndef INTERCONNECT_QCOM_IPQ9574_H > > +#define INTERCONNECT_QCOM_IPQ9574_H > > + > > +#define IPQ_APPS_ID 9574 /* some unique value */ > > Why random unique values are bindings? Why this cannot be 0? Please > explain how this is used by DTS and driver. This 'id' is not used by the driver or DTS. It is a unique id that is initialized for the node by the interconnect driver framework. A random value was chosen such that it does not conflict with an already existing node id. Chose 9574 based on this comment from clk-cbf-msm8996.c /* Random ID that doesn't clash with main qnoc and OSM */ #define CBF_MASTER_NODE 2000 > > +#define IPQ_NSS_ID (IPQ_APPS_ID * 2) > > This does not seem right. Doubled the NSS id so that APPS node ids dont clash. Thanks Varada
On 26/03/2024 09:59, Varadarajan Narayanan wrote: >>> --- >>> .../dt-bindings/interconnect/qcom,ipq9574.h | 62 +++++++++++++++++++ >>> 1 file changed, 62 insertions(+) >>> create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h >>> >>> diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h >>> new file mode 100644 >>> index 000000000000..b7b32aa6bbb1 >>> --- /dev/null >>> +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h >>> @@ -0,0 +1,62 @@ >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ >>> +#ifndef INTERCONNECT_QCOM_IPQ9574_H >>> +#define INTERCONNECT_QCOM_IPQ9574_H >>> + >>> +#define IPQ_APPS_ID 9574 /* some unique value */ >> >> Why random unique values are bindings? Why this cannot be 0? Please >> explain how this is used by DTS and driver. > > This 'id' is not used by the driver or DTS. It is a unique id that Then it is not a binding really. Don't put driver stuff to bindings. Best regards, Krzysztof
diff --git a/include/dt-bindings/interconnect/qcom,ipq9574.h b/include/dt-bindings/interconnect/qcom,ipq9574.h new file mode 100644 index 000000000000..b7b32aa6bbb1 --- /dev/null +++ b/include/dt-bindings/interconnect/qcom,ipq9574.h @@ -0,0 +1,62 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ +#ifndef INTERCONNECT_QCOM_IPQ9574_H +#define INTERCONNECT_QCOM_IPQ9574_H + +#define IPQ_APPS_ID 9574 /* some unique value */ +#define IPQ_NSS_ID (IPQ_APPS_ID * 2) + +#define MASTER_ANOC_PCIE0_1 0 +#define SLAVE_ANOC_PCIE0_1 1 +#define MASTER_SNOC_PCIE0_1 2 +#define SLAVE_SNOC_PCIE0_1 3 +#define MASTER_ANOC_PCIE1_1 4 +#define SLAVE_ANOC_PCIE1_1 5 +#define MASTER_SNOC_PCIE1_1 6 +#define SLAVE_SNOC_PCIE1_1 7 +#define MASTER_ANOC_PCIE2_2 8 +#define SLAVE_ANOC_PCIE2_2 9 +#define MASTER_SNOC_PCIE2_2 10 +#define SLAVE_SNOC_PCIE2_2 11 +#define MASTER_ANOC_PCIE3_2 12 +#define SLAVE_ANOC_PCIE3_2 13 +#define MASTER_SNOC_PCIE3_2 14 +#define SLAVE_SNOC_PCIE3_2 15 +#define MASTER_USB 16 +#define SLAVE_USB 17 +#define MASTER_USB_AXI 18 +#define SLAVE_USB_AXI 19 +#define MASTER_NSSNOC_NSSCC 20 +#define SLAVE_NSSNOC_NSSCC 21 +#define MASTER_NSSNOC_SNOC 22 +#define SLAVE_NSSNOC_SNOC 23 +#define MASTER_NSSNOC_SNOC_1 24 +#define SLAVE_NSSNOC_SNOC_1 25 +#define MASTER_NSSNOC_PCNOC_1 26 +#define SLAVE_NSSNOC_PCNOC_1 27 +#define MASTER_NSSNOC_QOSGEN_REF 28 +#define SLAVE_NSSNOC_QOSGEN_REF 29 +#define MASTER_NSSNOC_TIMEOUT_REF 30 +#define SLAVE_NSSNOC_TIMEOUT_REF 31 +#define MASTER_NSSNOC_XO_DCD 32 +#define SLAVE_NSSNOC_XO_DCD 33 +#define MASTER_NSSNOC_ATB 34 +#define SLAVE_NSSNOC_ATB 35 +#define MASTER_MEM_NOC_NSSNOC 36 +#define SLAVE_MEM_NOC_NSSNOC 37 +#define MASTER_NSSNOC_MEMNOC 38 +#define SLAVE_NSSNOC_MEMNOC 39 +#define MASTER_NSSNOC_MEM_NOC_1 40 +#define SLAVE_NSSNOC_MEM_NOC_1 41 + +#define MASTER_NSS_CC_NSSNOC_PPE 0 +#define SLAVE_NSS_CC_NSSNOC_PPE 1 +#define MASTER_NSS_CC_NSSNOC_PPE_CFG 2 +#define SLAVE_NSS_CC_NSSNOC_PPE_CFG 3 +#define MASTER_NSS_CC_NSSNOC_NSS_CSR 4 +#define SLAVE_NSS_CC_NSSNOC_NSS_CSR 5 +#define MASTER_NSS_CC_NSSNOC_IMEM_QSB 6 +#define SLAVE_NSS_CC_NSSNOC_IMEM_QSB 7 +#define MASTER_NSS_CC_NSSNOC_IMEM_AHB 8 +#define SLAVE_NSS_CC_NSSNOC_IMEM_AHB 9 + +#endif /* INTERCONNECT_QCOM_IPQ9574_H */
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@quicinc.com> --- v2: Rename master slave macros Fix license identifier --- .../dt-bindings/interconnect/qcom,ipq9574.h | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 include/dt-bindings/interconnect/qcom,ipq9574.h