diff mbox series

[v2,2/4] dt-bindings: interconnect: Add Qualcomm IPQ9574 support

Message ID 20240325102036.95484-3-quic_varada@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add interconnect driver for IPQ9574 SoC | expand

Commit Message

Varadarajan Narayanan March 25, 2024, 10:20 a.m. UTC 
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

Comments

Krzysztof Kozlowski March 26, 2024, 6:49 a.m. UTC | #1 
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
Varadarajan Narayanan March 26, 2024, 8:59 a.m. UTC | #2 
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
Krzysztof Kozlowski March 26, 2024, 9:12 a.m. UTC | #3 
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 mbox series

Patch

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 */