diff mbox series

[v2,1/8] dt-bindings: interconnect: qcom: Add Qualcomm MSM8976 NoC

Message ID 20240704200327.8583-2-a39.skl@gmail.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series msm8937/msm8976/qcs404 icc patches | expand

Commit Message

Adam Skladowski July 4, 2024, 8:02 p.m. UTC 
Add bindings for Qualcomm MSM8976 Network-On-Chip interconnect devices.

Signed-off-by: Adam Skladowski <a39.skl@gmail.com>
---
 .../bindings/interconnect/qcom,msm8976.yaml   | 63 ++++++++++++
 .../dt-bindings/interconnect/qcom,msm8976.h   | 97 +++++++++++++++++++
 2 files changed, 160 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
 create mode 100644 include/dt-bindings/interconnect/qcom,msm8976.h

Comments

Krzysztof Kozlowski July 5, 2024, 6:55 a.m. UTC | #1 
On 04/07/2024 22:02, Adam Skladowski wrote:
> Add bindings for Qualcomm MSM8976 Network-On-Chip interconnect devices.
> 
> Signed-off-by: Adam Skladowski <a39.skl@gmail.com>
> ---
>  .../bindings/interconnect/qcom,msm8976.yaml   | 63 ++++++++++++
>  .../dt-bindings/interconnect/qcom,msm8976.h   | 97 +++++++++++++++++++
>  2 files changed, 160 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
>  create mode 100644 include/dt-bindings/interconnect/qcom,msm8976.h
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml b/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
> new file mode 100644
> index 000000000000..fcb50f60dce3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/qcom,msm8976.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm MSM8976 Network-On-Chip interconnect
> +
> +maintainers:
> +  - Konrad Dybcio <konradybcio@kernel.org>
> +
> +description: |
> +  The Qualcomm MSM8976 interconnect providers support adjusting the
> +  bandwidth requirements between the various NoC fabrics.
> +
> +  See also:
> +  - dt-bindings/interconnect/qcom,msm8976.h

This is not a valid path. Please correct it, otherwise tools cannot
validate it.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,msm8976-bimc
> +      - qcom,msm8976-pcnoc
> +      - qcom,msm8976-snoc
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#interconnect-cells':
> +    const: 2
> +

I don't know what and why happened here. I asked for different order of
properties and properties are gone. Provide detailed changelog.

> +patternProperties:
> +  '^interconnect-[a-z0-9\-]+$':
> +    type: object
> +    $ref: qcom,rpm-common.yaml#
> +    unevaluatedProperties: false
> +    description:
> +      The interconnect providers do not have a separate QoS register space,
> +      but share parent's space.
> +
> +    properties:
> +      compatible:
> +        const: qcom,msm8976-snoc-mm
> +
> +    required:
> +      - compatible
> +      - '#interconnect-cells'
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#interconnect-cells'
> +


So no schema? Sorry, this is very confusing.

I am not going to review the rest. You implemented some odd changes, not
what was asked. At least not entirely. With no changelog explaining
this, you basically expect me to do review from scratch like there was
no previous review.

That's not how it works.

Best regards,
Krzysztof
Adam Skladowski July 6, 2024, 2:06 p.m. UTC | #2 
On 7/5/24 08:55, Krzysztof Kozlowski wrote:
> On 04/07/2024 22:02, Adam Skladowski wrote:
>> Add bindings for Qualcomm MSM8976 Network-On-Chip interconnect devices.
>>
>> Signed-off-by: Adam Skladowski <a39.skl@gmail.com>
>> ---
>>  .../bindings/interconnect/qcom,msm8976.yaml   | 63 ++++++++++++
>>  .../dt-bindings/interconnect/qcom,msm8976.h   | 97 +++++++++++++++++++
>>  2 files changed, 160 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
>>  create mode 100644 include/dt-bindings/interconnect/qcom,msm8976.h
>>
> This is not a valid path. Please correct it, otherwise tools cannot
> validate it.

Somehow got this weird idea out of qcom,msm8953.yaml

seems its wrong over there too.

Should proper line be like? :
  See also:: include/dt-bindings/interconnect/qcom,msm8976.h

>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,msm8976-bimc
>> +      - qcom,msm8976-pcnoc
>> +      - qcom,msm8976-snoc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  '#interconnect-cells':
>> +    const: 2
>> +
> I don't know what and why happened here. I asked for different order of
> properties and properties are gone. Provide detailed changelog.

For disappearing props it turns out clocks which i had defined for it
aren't required so these were wiped from yaml+driver.

>> +patternProperties:
>> +  '^interconnect-[a-z0-9\-]+$':
>> +    type: object
>> +    $ref: qcom,rpm-common.yaml#
>> +    unevaluatedProperties: false
>> +    description:
>> +      The interconnect providers do not have a separate QoS register space,
>> +      but share parent's space.
>> +
>> +    properties:
>> +      compatible:
>> +        const: qcom,msm8976-snoc-mm
>> +
>> +    required:
>> +      - compatible
>> +      - '#interconnect-cells'
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - '#interconnect-cells'
>> +
>
> So no schema? Sorry, this is very confusing.
>
> I am not going to review the rest. You implemented some odd changes, not
> what was asked. At least not entirely. With no changelog explaining
> this, you basically expect me to do review from scratch like there was
> no previous review.
>
> That's not how it works.
>
> Best regards,
> Krzysztof
>
As both yamls will not differ much from qcom,msm8939.yaml 
i moved compatibles in it for v3.

Would be great to discuss it before resending on #linux-msm
Krzysztof Kozlowski July 7, 2024, 11:43 a.m. UTC | #3 
On 06/07/2024 16:06, Adam Skladowski wrote:
> 
> On 7/5/24 08:55, Krzysztof Kozlowski wrote:
>> On 04/07/2024 22:02, Adam Skladowski wrote:
>>> Add bindings for Qualcomm MSM8976 Network-On-Chip interconnect devices.
>>>
>>> Signed-off-by: Adam Skladowski <a39.skl@gmail.com>
>>> ---
>>>  .../bindings/interconnect/qcom,msm8976.yaml   | 63 ++++++++++++
>>>  .../dt-bindings/interconnect/qcom,msm8976.h   | 97 +++++++++++++++++++
>>>  2 files changed, 160 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
>>>  create mode 100644 include/dt-bindings/interconnect/qcom,msm8976.h
>>>
>> This is not a valid path. Please correct it, otherwise tools cannot
>> validate it.
> 
> Somehow got this weird idea out of qcom,msm8953.yaml
> 
> seems its wrong over there too.
> 
> Should proper line be like? :
>   See also:: include/dt-bindings/interconnect/qcom,msm8976.h

Only one ':'


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml b/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
new file mode 100644
index 000000000000..fcb50f60dce3
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/qcom,msm8976.yaml
@@ -0,0 +1,63 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/qcom,msm8976.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm MSM8976 Network-On-Chip interconnect
+
+maintainers:
+  - Konrad Dybcio <konradybcio@kernel.org>
+
+description: |
+  The Qualcomm MSM8976 interconnect providers support adjusting the
+  bandwidth requirements between the various NoC fabrics.
+
+  See also:
+  - dt-bindings/interconnect/qcom,msm8976.h
+
+properties:
+  compatible:
+    enum:
+      - qcom,msm8976-bimc
+      - qcom,msm8976-pcnoc
+      - qcom,msm8976-snoc
+
+  reg:
+    maxItems: 1
+
+  '#interconnect-cells':
+    const: 2
+
+patternProperties:
+  '^interconnect-[a-z0-9\-]+$':
+    type: object
+    $ref: qcom,rpm-common.yaml#
+    unevaluatedProperties: false
+    description:
+      The interconnect providers do not have a separate QoS register space,
+      but share parent's space.
+
+    properties:
+      compatible:
+        const: qcom,msm8976-snoc-mm
+
+    required:
+      - compatible
+      - '#interconnect-cells'
+
+required:
+  - compatible
+  - reg
+  - '#interconnect-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+      bimc: interconnect@400000 {
+          compatible = "qcom,msm8976-bimc";
+          reg = <0x00400000 0x62000>;
+
+          #interconnect-cells = <2>;
+      };
diff --git a/include/dt-bindings/interconnect/qcom,msm8976.h b/include/dt-bindings/interconnect/qcom,msm8976.h
new file mode 100644
index 000000000000..4ea90f22320e
--- /dev/null
+++ b/include/dt-bindings/interconnect/qcom,msm8976.h
@@ -0,0 +1,97 @@ 
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Qualcomm MSM8976 interconnect IDs
+ */
+
+#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_MSM8976_H
+#define __DT_BINDINGS_INTERCONNECT_QCOM_MSM8976_H
+
+/* BIMC fabric */
+#define MAS_APPS_PROC		0
+#define MAS_SMMNOC_BIMC	        1
+#define MAS_SNOC_BIMC		2
+#define MAS_TCU_0		3
+#define SLV_EBI		        4
+#define SLV_BIMC_SNOC		5
+
+/* PCNOC fabric */
+#define MAS_USB_HS2		0
+#define MAS_BLSP_1		1
+#define MAS_USB_HS1		2
+#define MAS_BLSP_2		3
+#define MAS_CRYPTO		4
+#define MAS_SDCC_1		5
+#define MAS_SDCC_2		6
+#define MAS_SDCC_3		7
+#define MAS_SNOC_PCNOC		8
+#define MAS_LPASS_AHB		9
+#define MAS_SPDM		10
+#define MAS_DEHR		11
+#define MAS_XM_USB_HS1		12
+#define PCNOC_M_0		13
+#define PCNOC_M_1		14
+#define PCNOC_INT_0		15
+#define PCNOC_INT_1		16
+#define PCNOC_INT_2		17
+#define PCNOC_S_1		18
+#define PCNOC_S_2		19
+#define PCNOC_S_3		20
+#define PCNOC_S_4		21
+#define PCNOC_S_8		22
+#define PCNOC_S_9		23
+#define SLV_TCSR		24
+#define SLV_TLMM		25
+#define SLV_CRYPTO_0_CFG	26
+#define SLV_MESSAGE_RAM	        27
+#define SLV_PDM		        28
+#define SLV_PRNG		29
+#define SLV_PMIC_ARB		30
+#define SLV_SNOC_CFG		31
+#define SLV_DCC_CFG		32
+#define SLV_CAMERA_SS_CFG	33
+#define SLV_DISP_SS_CFG	        34
+#define SLV_VENUS_CFG		35
+#define SLV_SDCC_1		36
+#define SLV_BLSP_1		37
+#define SLV_USB_HS		38
+#define SLV_SDCC_3		39
+#define SLV_SDCC_2		40
+#define SLV_GPU_CFG		41
+#define SLV_USB_HS2		42
+#define SLV_BLSP_2		43
+#define SLV_PCNOC_SNOC		44
+
+/* SNOC fabric */
+#define MAS_QDSS_BAM		0
+#define MAS_BIMC_SNOC		1
+#define MAS_PCNOC_SNOC		2
+#define MAS_QDSS_ETR		3
+#define MAS_LPASS_PROC		4
+#define MAS_IPA		        5
+#define QDSS_INT		6
+#define SNOC_INT_0		7
+#define SNOC_INT_1		8
+#define SNOC_INT_2		9
+#define SLV_KPSS_AHB		10
+#define SLV_SNOC_BIMC		11
+#define SLV_IMEM		12
+#define SLV_SNOC_PCNOC		13
+#define SLV_QDSS_STM		14
+#define SLV_CATS_0		15
+#define SLV_CATS_1		16
+#define SLV_LPASS		17
+
+/* SNOC-MM fabric */
+#define MAS_JPEG		0
+#define MAS_OXILI		1
+#define MAS_MDP0		2
+#define MAS_MDP1		3
+#define MAS_VENUS_0		4
+#define MAS_VENUS_1		5
+#define MAS_VFE_0		6
+#define MAS_VFE_1		7
+#define MAS_CPP		        8
+#define MM_INT_0		9
+#define SLV_SMMNOC_BIMC		10
+
+#endif /* __DT_BINDINGS_INTERCONNECT_QCOM_MSM8976_H */