[PATCH v4 0/3] Add support for proxy interconnect bandwidth votes

[PATCH v4 0/3] Add support for proxy interconnect bandwidth votes

[Sibi Sankar]

Add proxy interconnect bandwidth votes during modem bootup on SC7280 SoCs.

V4:
 * Remove older bindings [Matthias/Krzysztof]
 * Convert sc7180/sc7280 to yaml and leave the rest to Sireesh's series
 * Rebased on v2 of Krzysztof's bindings cleanups
 * Misc. Fixes [Krzysztof]

V3:
 * Re-ordered clock list, fixed pdc_sync typo [Rob/Matthias]

V2:
 * Dropped patch 3 from version 1 [Sub with Bjorn's patch]
 * Add YAML support [Krzysztof]
 * Drop interconnect names [Bjorn]

 Depends on:
 https://lore.kernel.org/lkml/20220517070113.18023-1-krzysztof.kozlowski@linaro.org/

Sibi Sankar (3):
  arm64: dts: qcom: sc7280: Add proxy interconnect requirements for
    modem
  dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML
  dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |  47 +---
 .../bindings/remoteproc/qcom,sc7180-mss-pil.yaml   | 236 +++++++++++++++++++
 .../bindings/remoteproc/qcom,sc7280-mss-pil.yaml   | 250 +++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi |   1 +
 4 files changed, 489 insertions(+), 45 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml

-- 
2.7.4

[PATCH v4 1/3] arm64: dts: qcom: sc7280: Add proxy interconnect requirements for modem

[Sibi Sankar]

Add interconnects that are required to be proxy voted upon during modem
bootup on SC7280 SoCs.

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index 9f4a9c263c35..91aad86cc708 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -88,6 +88,7 @@
 	status = "okay";
 	compatible = "qcom,sc7280-mss-pil";
 	iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>;
+	interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>;
 	memory-region = <&mba_mem>, <&mpss_mem>;
 };
 
-- 
2.7.4

Re: [PATCH v4 1/3] arm64: dts: qcom: sc7280: Add proxy interconnect requirements for modem

[Bjorn Andersson]

On Thu 19 May 09:47 PDT 2022, Sibi Sankar wrote:

> Add interconnects that are required to be proxy voted upon during modem
> bootup on SC7280 SoCs.

This looks reasonable, but how come the vote is only for DDR frequency?
What about the buses between modem and ddr?

Regards,
Bjorn

> 
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> index 9f4a9c263c35..91aad86cc708 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
> @@ -88,6 +88,7 @@
>  	status = "okay";
>  	compatible = "qcom,sc7280-mss-pil";
>  	iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>;
> +	interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>;
>  	memory-region = <&mba_mem>, <&mpss_mem>;
>  };
>  
> -- 
> 2.7.4
> 

Re: [PATCH v4 1/3] arm64: dts: qcom: sc7280: Add proxy interconnect requirements for modem

[Sibi Sankar]

Hey Bjorn,
Thanks for taking time to review the series.

On 5/20/22 2:05 AM, Bjorn Andersson wrote:
> On Thu 19 May 09:47 PDT 2022, Sibi Sankar wrote:
> 
>> Add interconnects that are required to be proxy voted upon during modem
>> bootup on SC7280 SoCs.
> 
> This looks reasonable, but how come the vote is only for DDR frequency?
> What about the buses between modem and ddr?

The proxy votes that are put in aren't for perf related reasons, the
modem was getting llcc timeouts while trying to read contents from 
memory. The hw team recommended the proxy votes as the fix.

-Sibi

> 
> Regards,
> Bjorn
> 
>>
>> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
>> index 9f4a9c263c35..91aad86cc708 100644
>> --- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
>> @@ -88,6 +88,7 @@
>>   	status = "okay";
>>   	compatible = "qcom,sc7280-mss-pil";
>>   	iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>;
>> +	interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>;
>>   	memory-region = <&mba_mem>, <&mpss_mem>;
>>   };
>>   
>> -- 
>> 2.7.4
>>

Re: [PATCH v4 1/3] arm64: dts: qcom: sc7280: Add proxy interconnect requirements for modem

[Stephen Boyd]

Quoting Sibi Sankar (2022-05-20 11:08:52)
> Hey Bjorn,
> Thanks for taking time to review the series.
>
> On 5/20/22 2:05 AM, Bjorn Andersson wrote:
> > On Thu 19 May 09:47 PDT 2022, Sibi Sankar wrote:
> >
> >> Add interconnects that are required to be proxy voted upon during modem
> >> bootup on SC7280 SoCs.
> >
> > This looks reasonable, but how come the vote is only for DDR frequency?
> > What about the buses between modem and ddr?
>
> The proxy votes that are put in aren't for perf related reasons, the
> modem was getting llcc timeouts while trying to read contents from
> memory. The hw team recommended the proxy votes as the fix.

Presumably the bootloader sets up some initial modem and ddr bus
bandwidth requests? Or the modem bootloader stage (MSA?) handles that
part?

Re: [PATCH v4 1/3] arm64: dts: qcom: sc7280: Add proxy interconnect requirements for modem

[Sibi Sankar]

On 5/21/22 12:37 AM, Stephen Boyd wrote:
> Quoting Sibi Sankar (2022-05-20 11:08:52)
>> Hey Bjorn,
>> Thanks for taking time to review the series.
>>
>> On 5/20/22 2:05 AM, Bjorn Andersson wrote:
>>> On Thu 19 May 09:47 PDT 2022, Sibi Sankar wrote:
>>>
>>>> Add interconnects that are required to be proxy voted upon during modem
>>>> bootup on SC7280 SoCs.
>>>
>>> This looks reasonable, but how come the vote is only for DDR frequency?
>>> What about the buses between modem and ddr?
>>
>> The proxy votes that are put in aren't for perf related reasons, the
>> modem was getting llcc timeouts while trying to read contents from
>> memory. The hw team recommended the proxy votes as the fix.
> 
> Presumably the bootloader sets up some initial modem and ddr bus
> bandwidth requests? Or the modem bootloader stage (MSA?) handles that
> part?

Stephen/Bjorn,
Sorry for the delay, took a while to dig this up. The modem interconnect
is connected directly to gemnoc ddr. The path info from modem --> ddr is
split up into modem --> llcc and llcc --> ddr (Similar to CPUs) i.e. in
the end scaling of the path involves scaling of the two clocks, gemnoc
and ddr. There isn't any default vote for modem --> llcc as such but it
gets implicitly scaled when we vote max for llcc --> ddr path due to
dependency maintained between the two clocks by rpmh.

-Sibi

> 

Re: (subset) [PATCH v4 1/3] arm64: dts: qcom: sc7280: Add proxy interconnect requirements for modem

[Bjorn Andersson]

On Thu, 19 May 2022 22:17:03 +0530, Sibi Sankar wrote:
> Add interconnects that are required to be proxy voted upon during modem
> bootup on SC7280 SoCs.
> 
> 

Applied, thanks!

[1/3] arm64: dts: qcom: sc7280: Add proxy interconnect requirements for modem
      commit: a0cdc83fa89b3a53cf03ecd338832392be0dd4b3

Best regards,
-- 
Bjorn Andersson <bjorn.andersson@linaro.org>

[PATCH v4 2/3] dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML

[Sibi Sankar]

Convert SC7280 MSS PIL loading bindings to YAML.

Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---

v4:
 * Remove older bindings [Matthias/Krzysztof]
 * Misc. Fixes [Krzysztof]
 * Rebased on v2 of Krzysztof's bindings cleanups

v3:
 * Re-ordered clock list, fixed pdc_sync typo [Rob/Matthias]

 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |  31 +--
 .../bindings/remoteproc/qcom,sc7280-mss-pil.yaml   | 250 +++++++++++++++++++++
 2 files changed, 252 insertions(+), 29 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
index b677900b3aae..1ec9093c3a82 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
@@ -15,7 +15,6 @@ on the Qualcomm Hexagon core.
 		    "qcom,msm8996-mss-pil"
 		    "qcom,msm8998-mss-pil"
 		    "qcom,sc7180-mss-pil"
-		    "qcom,sc7280-mss-pil"
 		    "qcom,sdm845-mss-pil"
 
 - reg:
@@ -48,7 +47,6 @@ on the Qualcomm Hexagon core.
 	qcom,msm8996-mss-pil:
 	qcom,msm8998-mss-pil:
 	qcom,sc7180-mss-pil:
-	qcom,sc7280-mss-pil:
 	qcom,sdm845-mss-pil:
 		    must be "wdog", "fatal", "ready", "handover", "stop-ack",
 		    "shutdown-ack"
@@ -89,8 +87,6 @@ on the Qualcomm Hexagon core.
 	qcom,sc7180-mss-pil:
 		    must be "iface", "bus", "xo", "snoc_axi", "mnoc_axi",
 		    "nav"
-	qcom,sc7280-mss-pil:
-		    must be "iface", "xo", "snoc_axi", "offline", "pka"
 	qcom,sdm845-mss-pil:
 		    must be "iface", "bus", "mem", "xo", "gpll0_mss",
 		    "snoc_axi", "mnoc_axi", "prng"
@@ -102,7 +98,7 @@ on the Qualcomm Hexagon core.
 		    reference to the list of 3 reset-controllers for the
 		    wcss sub-system
 		    reference to the list of 2 reset-controllers for the modem
-		    sub-system on SC7180, SC7280, SDM845 SoCs
+		    sub-system on SC7180, SDM845 SoCs
 
 - reset-names:
 	Usage: required
@@ -111,7 +107,7 @@ on the Qualcomm Hexagon core.
 		    must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
 		    for the wcss sub-system
 		    must be "mss_restart", "pdc_reset" for the modem
-		    sub-system on SC7180, SC7280, SDM845 SoCs
+		    sub-system on SC7180, SDM845 SoCs
 
 For devices where the mba and mpss sub-nodes are not specified, mba/mpss region
 should be referenced as follows:
@@ -178,8 +174,6 @@ For the compatible string below the following supplies are required:
 		    must be "cx", "mx"
 	qcom,sc7180-mss-pil:
 		    must be "cx", "mx", "mss"
-	qcom,sc7280-mss-pil:
-		    must be "cx", "mss"
 	qcom,sdm845-mss-pil:
 		    must be "cx", "mx", "mss"
 
@@ -205,9 +199,6 @@ For the compatible string below the following supplies are required:
 	Definition: a phandle reference to a syscon representing TCSR followed
 		    by the three offsets within syscon for q6, modem and nc
 		    halt registers.
-		    a phandle reference to a syscon representing TCSR followed
-		    by the four offsets within syscon for q6, modem, nc and vq6
-		    halt registers on SC7280 SoCs.
 
 For the compatible strings below the following phandle references are required:
   "qcom,sc7180-mss-pil"
@@ -218,24 +209,6 @@ For the compatible strings below the following phandle references are required:
 		    by the offset within syscon for conn_box_spare0 register
 		    used by the modem sub-system running on SC7180 SoC.
 
-For the compatible strings below the following phandle references are required:
-  "qcom,sc7280-mss-pil"
-- qcom,ext-regs:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: two phandle references to syscons representing TCSR_REG and
-		    TCSR register space followed by the two offsets within the syscon
-		    to force_clk_en/rscc_disable and axim1_clk_off/crypto_clk_off
-		    registers respectively.
-
-- qcom,qaccept-regs:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: a phandle reference to a syscon representing TCSR followed
-		    by the three offsets within syscon for mdm, cx and axi
-		    qaccept registers used by the modem sub-system running on
-		    SC7280 SoC.
-
 The Hexagon node must contain iommus property as described in ../iommu/iommu.txt
 on platforms which do not have TrustZone.
 
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
new file mode 100644
index 000000000000..a936d84eefa6
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
@@ -0,0 +1,250 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,sc7280-mss-pil.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SC7280 MSS Peripheral Image Loader
+
+maintainers:
+  - Sibi Sankar <quic_sibis@quicinc.com>
+
+description:
+  This document describes the hardware for a component that loads and boots firmware
+  on the Qualcomm Technology Inc. SC7280 Modem Hexagon Core.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sc7280-mss-pil
+
+  reg:
+    items:
+      - description: MSS QDSP6 registers
+      - description: RMB registers
+
+  reg-names:
+    items:
+      - const: qdsp6
+      - const: rmb
+
+  iommus:
+    items:
+      - description: MSA Stream 1
+      - description: MSA Stream 2
+
+  interconnects:
+    items:
+      - description: Path leading to system memory
+
+  interrupts:
+    items:
+      - description: Watchdog interrupt
+      - description: Fatal interrupt
+      - description: Ready interrupt
+      - description: Handover interrupt
+      - description: Stop acknowledge interrupt
+      - description: Shutdown acknowledge interrupt
+
+  interrupt-names:
+    items:
+      - const: wdog
+      - const: fatal
+      - const: ready
+      - const: handover
+      - const: stop-ack
+      - const: shutdown-ack
+
+  clocks:
+    items:
+      - description: GCC MSS IFACE clock
+      - description: GCC MSS OFFLINE clock
+      - description: GCC MSS SNOC_AXI clock
+      - description: RPMH PKA clock
+      - description: RPMH XO clock
+
+  clock-names:
+    items:
+      - const: iface
+      - const: offline
+      - const: snoc_axi
+      - const: pka
+      - const: xo
+
+  power-domains:
+    items:
+      - description: CX power domain
+      - description: MSS power domain
+
+  power-domain-names:
+    items:
+      - const: cx
+      - const: mss
+
+  resets:
+    items:
+      - description: AOSS restart
+      - description: PDC reset
+
+  reset-names:
+    items:
+      - const: mss_restart
+      - const: pdc_reset
+
+  memory-region:
+    maxItems: 2
+    description: Phandle reference to the reserved-memory for the MBA region followed
+                 by the modem region.
+
+  firmware-name:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    maxItems: 2
+    description:
+      The name of the MBA and modem firmware to be loaded for this remote processor.
+
+  qcom,halt-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Phandle reference to a syscon representing TCSR followed by the
+      four offsets within syscon for q6, modem, nc and vq6 halt registers.
+
+  qcom,ext-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Two phandle references to syscons representing TCSR_REG and TCSR register
+      space followed by the two offsets within the syscon to force_clk_en/rscc_disable
+      and axim1_clk_off/crypto_clk_off registers respectively.
+
+  qcom,qaccept-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Phandle reference to a syscon representing TCSR followed by the
+      three offsets within syscon for mdm, cx and axi qaccept registers.
+
+  qcom,qmp:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Reference to the AOSS side-channel message RAM.
+
+  qcom,smem-states:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: States used by the AP to signal the Hexagon core
+    items:
+      - description: Stop the modem
+
+  qcom,smem-state-names:
+    description: The names of the state bits used for SMP2P output
+    const: stop
+
+  glink-edge:
+    $ref: qcom,glink-edge.yaml#
+    description:
+      Qualcomm G-Link subnode which represents communication edge, channels
+      and devices related to the DSP.
+
+    properties:
+      interrupts:
+        items:
+          - description: IRQ from MSS to GLINK
+
+      mboxes:
+        items:
+          - description: Mailbox for communication between APPS and MSS
+
+      label:
+        items:
+          - const: modem
+
+      apr: false
+      fastrpc: false
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - iommus
+  - interconnects
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - power-domains
+  - power-domain-names
+  - resets
+  - reset-names
+  - qcom,halt-regs
+  - qcom,ext-regs
+  - qcom,qaccept-regs
+  - memory-region
+  - qcom,qmp
+  - qcom,smem-states
+  - qcom,smem-state-names
+  - glink-edge
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sc7280.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/interconnect/qcom,sc7280.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/mailbox/qcom-ipcc.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
+    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
+
+    remoteproc_mpss: remoteproc@4080000 {
+        compatible = "qcom,sc7280-mss-pil";
+        reg = <0x04080000 0x10000>, <0x04180000 0x48>;
+        reg-names = "qdsp6", "rmb";
+
+        iommus = <&apps_smmu 0x124 0x0>, <&apps_smmu 0x488 0x7>;
+
+        interconnects = <&mc_virt MASTER_LLCC 0 &mc_virt SLAVE_EBI1 0>;
+
+        interrupts-extended = <&intc GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
+
+        interrupt-names = "wdog", "fatal", "ready", "handover",
+                          "stop-ack", "shutdown-ack";
+
+        clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
+                 <&gcc GCC_MSS_OFFLINE_AXI_CLK>,
+                 <&gcc GCC_MSS_SNOC_AXI_CLK>,
+                 <&rpmhcc RPMH_PKA_CLK>,
+                 <&rpmhcc RPMH_CXO_CLK>;
+        clock-names = "iface", "offline", "snoc_axi", "pka", "xo";
+
+        power-domains = <&rpmhpd SC7280_CX>,
+                        <&rpmhpd SC7280_MSS>;
+        power-domain-names = "cx", "mss";
+
+        memory-region = <&mba_mem>, <&mpss_mem>;
+
+        qcom,qmp = <&aoss_qmp>;
+
+        qcom,smem-states = <&modem_smp2p_out 0>;
+        qcom,smem-state-names = "stop";
+
+        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
+                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
+        reset-names = "mss_restart", "pdc_reset";
+
+        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
+        qcom,ext-regs = <&tcsr 0x10000 0x10004 &tcsr_mutex 0x26004 0x26008>;
+        qcom,qaccept-regs = <&tcsr_mutex 0x23030 0x23040 0x23020>;
+
+        glink-edge {
+            interrupts-extended = <&ipcc IPCC_CLIENT_MPSS
+                                   IPCC_MPROC_SIGNAL_GLINK_QMP
+                                   IRQ_TYPE_EDGE_RISING>;
+            mboxes = <&ipcc IPCC_CLIENT_MPSS
+                      IPCC_MPROC_SIGNAL_GLINK_QMP>;
+            label = "modem";
+            qcom,remote-pid = <1>;
+        };
+    };
-- 
2.7.4

Re: [PATCH v4 2/3] dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML

[Rob Herring]

On Thu, 19 May 2022 22:17:04 +0530, Sibi Sankar wrote:
> Convert SC7280 MSS PIL loading bindings to YAML.
> 
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
> 
> v4:
>  * Remove older bindings [Matthias/Krzysztof]
>  * Misc. Fixes [Krzysztof]
>  * Rebased on v2 of Krzysztof's bindings cleanups
> 
> v3:
>  * Re-ordered clock list, fixed pdc_sync typo [Rob/Matthias]
> 
>  .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |  31 +--
>  .../bindings/remoteproc/qcom,sc7280-mss-pil.yaml   | 250 +++++++++++++++++++++
>  2 files changed, 252 insertions(+), 29 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/remoteproc/qcom,glink-edge.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.example.dtb: remoteproc@4080000: glink-edge: False schema does not allow {'interrupts-extended': [[4294967295, 2, 0, 1]], 'mboxes': [[4294967295, 2, 0]], 'label': ['modem'], 'qcom,remote-pid': [[1]]}
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v4 2/3] dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML

[Stephen Boyd]

Quoting Sibi Sankar (2022-05-19 09:47:04)
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
> new file mode 100644
> index 000000000000..a936d84eefa6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
> @@ -0,0 +1,250 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/qcom,sc7280-mss-pil.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SC7280 MSS Peripheral Image Loader
> +
> +maintainers:
> +  - Sibi Sankar <quic_sibis@quicinc.com>
> +
> +description:
> +  This document describes the hardware for a component that loads and boots firmware
> +  on the Qualcomm Technology Inc. SC7280 Modem Hexagon Core.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sc7280-mss-pil
> +
[..]
> +
> +  resets:
> +    items:
> +      - description: AOSS restart
> +      - description: PDC reset
> +
> +  reset-names:
> +    items:
> +      - const: mss_restart
> +      - const: pdc_reset
> +
> +  memory-region:
> +    maxItems: 2
> +    description: Phandle reference to the reserved-memory for the MBA region followed
> +                 by the modem region.
> +
> +  firmware-name:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    maxItems: 2

Instead of maxItems can this be

       items:
         - description: Name of MBA firmware
	 - description: Name of modem firmware

so that we know the order? Same for 'memory-region' above.

> +    description:
> +      The name of the MBA and modem firmware to be loaded for this remote processor.
> +
> +  qcom,halt-regs:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array

Should this have maxItems: 1? Or that's implicit from description?

> +    description:
> +      Phandle reference to a syscon representing TCSR followed by the
> +      four offsets within syscon for q6, modem, nc and vq6 halt registers.
> +
> +  qcom,ext-regs:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array

Should this have min/maxItems: 2?

> +    description:
> +      Two phandle references to syscons representing TCSR_REG and TCSR register
> +      space followed by the two offsets within the syscon to force_clk_en/rscc_disable
> +      and axim1_clk_off/crypto_clk_off registers respectively.
> +
> +  qcom,qaccept-regs:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Phandle reference to a syscon representing TCSR followed by the
> +      three offsets within syscon for mdm, cx and axi qaccept registers.
> +
> +  qcom,qmp:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Reference to the AOSS side-channel message RAM.
> +
> +  qcom,smem-states:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: States used by the AP to signal the Hexagon core
> +    items:
> +      - description: Stop the modem

This one did items for a phandle array so I think we should follow the
same above.

> +
> +  qcom,smem-state-names:
> +    description: The names of the state bits used for SMP2P output
> +    const: stop
> +
> +  glink-edge:
> +    $ref: qcom,glink-edge.yaml#
> +    description:
> +      Qualcomm G-Link subnode which represents communication edge, channels
> +      and devices related to the DSP.
[..]
> +        power-domain-names = "cx", "mss";
> +
> +        memory-region = <&mba_mem>, <&mpss_mem>;
> +
> +        qcom,qmp = <&aoss_qmp>;
> +
> +        qcom,smem-states = <&modem_smp2p_out 0>;
> +        qcom,smem-state-names = "stop";
> +
> +        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
> +                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
> +        reset-names = "mss_restart", "pdc_reset";
> +
> +        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
> +        qcom,ext-regs = <&tcsr 0x10000 0x10004 &tcsr_mutex 0x26004 0x26008>;

Because it's two items I'd expect:
	
	<&tcsr 0x10000 0x10004>, <&tcsr_mutex 0x26004 0x26008>;

> +        qcom,qaccept-regs = <&tcsr_mutex 0x23030 0x23040 0x23020>;
> +
> +        glink-edge {
> +            interrupts-extended = <&ipcc IPCC_CLIENT_MPSS
> +                                   IPCC_MPROC_SIGNAL_GLINK_QMP
> +                                   IRQ_TYPE_EDGE_RISING>;

Re: [PATCH v4 2/3] dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML

[Sibi Sankar]

Hey Stephen,

On 5/20/22 4:05 AM, Stephen Boyd wrote:
> Quoting Sibi Sankar (2022-05-19 09:47:04)
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
>> new file mode 100644
>> index 000000000000..a936d84eefa6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
>> @@ -0,0 +1,250 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/remoteproc/qcom,sc7280-mss-pil.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm SC7280 MSS Peripheral Image Loader
>> +
>> +maintainers:
>> +  - Sibi Sankar <quic_sibis@quicinc.com>
>> +
>> +description:
>> +  This document describes the hardware for a component that loads and boots firmware
>> +  on the Qualcomm Technology Inc. SC7280 Modem Hexagon Core.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,sc7280-mss-pil
>> +
> [..]
>> +
>> +  resets:
>> +    items:
>> +      - description: AOSS restart
>> +      - description: PDC reset
>> +
>> +  reset-names:
>> +    items:
>> +      - const: mss_restart
>> +      - const: pdc_reset
>> +
>> +  memory-region:
>> +    maxItems: 2
>> +    description: Phandle reference to the reserved-memory for the MBA region followed
>> +                 by the modem region.
>> +
>> +  firmware-name:
>> +    $ref: /schemas/types.yaml#/definitions/string-array
>> +    maxItems: 2
> 
> Instead of maxItems can this be
> 
>         items:
>           - description: Name of MBA firmware
> 	 - description: Name of modem firmware
> 
> so that we know the order? Same for 'memory-region' above.

ack

> 
>> +    description:
>> +      The name of the MBA and modem firmware to be loaded for this remote processor.
>> +
>> +  qcom,halt-regs:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> Should this have maxItems: 1? Or that's implicit from description?

It's implicit!

> 
>> +    description:
>> +      Phandle reference to a syscon representing TCSR followed by the
>> +      four offsets within syscon for q6, modem, nc and vq6 halt registers.
>> +
>> +  qcom,ext-regs:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> Should this have min/maxItems: 2?

ack

> 
>> +    description:
>> +      Two phandle references to syscons representing TCSR_REG and TCSR register
>> +      space followed by the two offsets within the syscon to force_clk_en/rscc_disable
>> +      and axim1_clk_off/crypto_clk_off registers respectively.
>> +
>> +  qcom,qaccept-regs:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description:
>> +      Phandle reference to a syscon representing TCSR followed by the
>> +      three offsets within syscon for mdm, cx and axi qaccept registers.
>> +
>> +  qcom,qmp:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description: Reference to the AOSS side-channel message RAM.
>> +
>> +  qcom,smem-states:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description: States used by the AP to signal the Hexagon core
>> +    items:
>> +      - description: Stop the modem
> 
> This one did items for a phandle array so I think we should follow the
> same above.

ack

> 
>> +
>> +  qcom,smem-state-names:
>> +    description: The names of the state bits used for SMP2P output
>> +    const: stop
>> +
>> +  glink-edge:
>> +    $ref: qcom,glink-edge.yaml#
>> +    description:
>> +      Qualcomm G-Link subnode which represents communication edge, channels
>> +      and devices related to the DSP.
> [..]
>> +        power-domain-names = "cx", "mss";
>> +
>> +        memory-region = <&mba_mem>, <&mpss_mem>;
>> +
>> +        qcom,qmp = <&aoss_qmp>;
>> +
>> +        qcom,smem-states = <&modem_smp2p_out 0>;
>> +        qcom,smem-state-names = "stop";
>> +
>> +        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
>> +                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
>> +        reset-names = "mss_restart", "pdc_reset";
>> +
>> +        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
>> +        qcom,ext-regs = <&tcsr 0x10000 0x10004 &tcsr_mutex 0x26004 0x26008>;
> 
> Because it's two items I'd expect:
> 	
> 	<&tcsr 0x10000 0x10004>, <&tcsr_mutex 0x26004 0x26008>;

I guess both the ways work since the driver uses
of_parse_phandle_with_fixed_args.

> 
>> +        qcom,qaccept-regs = <&tcsr_mutex 0x23030 0x23040 0x23020>;
>> +
>> +        glink-edge {
>> +            interrupts-extended = <&ipcc IPCC_CLIENT_MPSS
>> +                                   IPCC_MPROC_SIGNAL_GLINK_QMP
>> +                                   IRQ_TYPE_EDGE_RISING>;

Re: [PATCH v4 2/3] dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML

[Stephen Boyd]

Quoting Sibi Sankar (2022-05-20 11:46:58)
> On 5/20/22 4:05 AM, Stephen Boyd wrote:
> > Quoting Sibi Sankar (2022-05-19 09:47:04)
> >> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
> >> new file mode 100644
> >> index 000000000000..a936d84eefa6
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
> >> +        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
> >> +                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
> >> +        reset-names = "mss_restart", "pdc_reset";
> >> +
> >> +        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
> >> +        qcom,ext-regs = <&tcsr 0x10000 0x10004 &tcsr_mutex 0x26004 0x26008>;
> >
> > Because it's two items I'd expect:
> >
> >       <&tcsr 0x10000 0x10004>, <&tcsr_mutex 0x26004 0x26008>;
>
> I guess both the ways work since the driver uses
> of_parse_phandle_with_fixed_args.

See commit 39bd2b6a3783 ("dt-bindings: Improve phandle-array schemas")
for why the way you have it is not preferred.

Re: [PATCH v4 2/3] dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML

[Sibi Sankar]

On 5/21/22 1:40 AM, Stephen Boyd wrote:
> Quoting Sibi Sankar (2022-05-20 11:46:58)
>> On 5/20/22 4:05 AM, Stephen Boyd wrote:
>>> Quoting Sibi Sankar (2022-05-19 09:47:04)
>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
>>>> new file mode 100644
>>>> index 000000000000..a936d84eefa6
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-mss-pil.yaml
>>>> +        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
>>>> +                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
>>>> +        reset-names = "mss_restart", "pdc_reset";
>>>> +
>>>> +        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
>>>> +        qcom,ext-regs = <&tcsr 0x10000 0x10004 &tcsr_mutex 0x26004 0x26008>;
>>>
>>> Because it's two items I'd expect:
>>>
>>>        <&tcsr 0x10000 0x10004>, <&tcsr_mutex 0x26004 0x26008>;
>>
>> I guess both the ways work since the driver uses
>> of_parse_phandle_with_fixed_args.
> 
> See commit 39bd2b6a3783 ("dt-bindings: Improve phandle-array schemas")
> for why the way you have it is not preferred.

Sure, I'll fix the dt up and update the example.

-Sibi

> 

Re: [PATCH v4 2/3] dt-bindings: remoteproc: qcom: Convert SC7280 MSS bindings to YAML

[Krzysztof Kozlowski]

On 20/05/2022 20:46, Sibi Sankar wrote:
>>> +  memory-region:
>>> +    maxItems: 2
>>> +    description: Phandle reference to the reserved-memory for the MBA region followed
>>> +                 by the modem region.
>>> +
>>> +  firmware-name:
>>> +    $ref: /schemas/types.yaml#/definitions/string-array
>>> +    maxItems: 2
>>
>> Instead of maxItems can this be
>>
>>         items:
>>           - description: Name of MBA firmware
>> 	 - description: Name of modem firmware
>>
>> so that we know the order? Same for 'memory-region' above.
> 
> ack
> 
>>
>>> +    description:
>>> +      The name of the MBA and modem firmware to be loaded for this remote processor.
>>> +
>>> +  qcom,halt-regs:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>
>> Should this have maxItems: 1? Or that's implicit from description?
> 
> It's implicit!

I am not aware of such implicit rule in schema. maxItems are always
required. If this is maxItems:1 it is not even an array.

> 
>>
>>> +    description:
>>> +      Phandle reference to a syscon representing TCSR followed by the
>>> +      four offsets within syscon for q6, modem, nc and vq6 halt registers.
>>> +
>>> +  qcom,ext-regs:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>
>> Should this have min/maxItems: 2?
> 
> ack

You should also define the items. This applies to all such fields. Check
the examples of syscon consumers.

> 
>>
>>> +    description:
>>> +      Two phandle references to syscons representing TCSR_REG and TCSR register
>>> +      space followed by the two offsets within the syscon to force_clk_en/rscc_disable
>>> +      and axim1_clk_off/crypto_clk_off registers respectively.
>>> +
>>> +  qcom,qaccept-regs:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    description:
>>> +      Phandle reference to a syscon representing TCSR followed by the
>>> +      three offsets within syscon for mdm, cx and axi qaccept registers.
>>> +
>>> +  qcom,qmp:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description: Reference to the AOSS side-channel message RAM.
>>> +
>>> +  qcom,smem-states:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    description: States used by the AP to signal the Hexagon core
>>> +    items:
>>> +      - description: Stop the modem
>>
>> This one did items for a phandle array so I think we should follow the
>> same above.
> 
> ack
> 
>>
>>> +
>>> +  qcom,smem-state-names:
>>> +    description: The names of the state bits used for SMP2P output
>>> +    const: stop
>>> +
>>> +  glink-edge:
>>> +    $ref: qcom,glink-edge.yaml#
>>> +    description:
>>> +      Qualcomm G-Link subnode which represents communication edge, channels
>>> +      and devices related to the DSP.
>> [..]
>>> +        power-domain-names = "cx", "mss";
>>> +
>>> +        memory-region = <&mba_mem>, <&mpss_mem>;
>>> +
>>> +        qcom,qmp = <&aoss_qmp>;
>>> +
>>> +        qcom,smem-states = <&modem_smp2p_out 0>;
>>> +        qcom,smem-state-names = "stop";
>>> +
>>> +        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
>>> +                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
>>> +        reset-names = "mss_restart", "pdc_reset";
>>> +
>>> +        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
>>> +        qcom,ext-regs = <&tcsr 0x10000 0x10004 &tcsr_mutex 0x26004 0x26008>;
>>
>> Because it's two items I'd expect:
>> 	
>> 	<&tcsr 0x10000 0x10004>, <&tcsr_mutex 0x26004 0x26008>;
> 
> I guess both the ways work since the driver uses
> of_parse_phandle_with_fixed_args.

But only one is correct...


Best regards,
Krzysztof

[PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Sibi Sankar]

Convert SC7180 MSS PIL loading bindings to YAML.

Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
---
 .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |  20 +-
 .../bindings/remoteproc/qcom,sc7180-mss-pil.yaml   | 236 +++++++++++++++++++++
 2 files changed, 238 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
index 1ec9093c3a82..1a691c1435f3 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
@@ -14,7 +14,6 @@ on the Qualcomm Hexagon core.
 		    "qcom,msm8974-mss-pil"
 		    "qcom,msm8996-mss-pil"
 		    "qcom,msm8998-mss-pil"
-		    "qcom,sc7180-mss-pil"
 		    "qcom,sdm845-mss-pil"
 
 - reg:
@@ -46,7 +45,6 @@ on the Qualcomm Hexagon core.
 		    must be "wdog", "fatal", "ready", "handover", "stop-ack"
 	qcom,msm8996-mss-pil:
 	qcom,msm8998-mss-pil:
-	qcom,sc7180-mss-pil:
 	qcom,sdm845-mss-pil:
 		    must be "wdog", "fatal", "ready", "handover", "stop-ack",
 		    "shutdown-ack"
@@ -84,9 +82,6 @@ on the Qualcomm Hexagon core.
 	qcom,msm8998-mss-pil:
 		    must be "iface", "bus", "mem", "xo", "gpll0_mss",
 		    "snoc_axi", "mnoc_axi", "qdss"
-	qcom,sc7180-mss-pil:
-		    must be "iface", "bus", "xo", "snoc_axi", "mnoc_axi",
-		    "nav"
 	qcom,sdm845-mss-pil:
 		    must be "iface", "bus", "mem", "xo", "gpll0_mss",
 		    "snoc_axi", "mnoc_axi", "prng"
@@ -98,7 +93,7 @@ on the Qualcomm Hexagon core.
 		    reference to the list of 3 reset-controllers for the
 		    wcss sub-system
 		    reference to the list of 2 reset-controllers for the modem
-		    sub-system on SC7180, SDM845 SoCs
+		    sub-system on SDM845 SoCs
 
 - reset-names:
 	Usage: required
@@ -107,7 +102,7 @@ on the Qualcomm Hexagon core.
 		    must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
 		    for the wcss sub-system
 		    must be "mss_restart", "pdc_reset" for the modem
-		    sub-system on SC7180, SDM845 SoCs
+		    sub-system on SDM845 SoCs
 
 For devices where the mba and mpss sub-nodes are not specified, mba/mpss region
 should be referenced as follows:
@@ -172,8 +167,6 @@ For the compatible string below the following supplies are required:
 	qcom,msm8996-mss-pil:
 	qcom,msm8998-mss-pil:
 		    must be "cx", "mx"
-	qcom,sc7180-mss-pil:
-		    must be "cx", "mx", "mss"
 	qcom,sdm845-mss-pil:
 		    must be "cx", "mx", "mss"
 
@@ -200,15 +193,6 @@ For the compatible string below the following supplies are required:
 		    by the three offsets within syscon for q6, modem and nc
 		    halt registers.
 
-For the compatible strings below the following phandle references are required:
-  "qcom,sc7180-mss-pil"
-- qcom,spare-regs:
-	Usage: required
-	Value type: <prop-encoded-array>
-	Definition: a phandle reference to a syscon representing TCSR followed
-		    by the offset within syscon for conn_box_spare0 register
-		    used by the modem sub-system running on SC7180 SoC.
-
 The Hexagon node must contain iommus property as described in ../iommu/iommu.txt
 on platforms which do not have TrustZone.
 
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
new file mode 100644
index 000000000000..b00fdf9e3eeb
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
@@ -0,0 +1,236 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,sc7180-mss-pil.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SC7180 MSS Peripheral Image Loader
+
+maintainers:
+  - Sibi Sankar <quic_sibis@quicinc.com>
+
+description:
+  This document describes the hardware for a component that loads and boots firmware
+  on the Qualcomm Technology Inc. SC7180 Modem Hexagon Core.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sc7180-mss-pil
+
+  reg:
+    items:
+      - description: MSS QDSP6 registers
+      - description: RMB registers
+
+  reg-names:
+    items:
+      - const: qdsp6
+      - const: rmb
+
+  iommus:
+    items:
+      - description: MSA Stream 1
+      - description: MSA Stream 2
+
+  interrupts:
+    items:
+      - description: Watchdog interrupt
+      - description: Fatal interrupt
+      - description: Ready interrupt
+      - description: Handover interrupt
+      - description: Stop acknowledge interrupt
+      - description: Shutdown acknowledge interrupt
+
+  interrupt-names:
+    items:
+      - const: wdog
+      - const: fatal
+      - const: ready
+      - const: handover
+      - const: stop-ack
+      - const: shutdown-ack
+
+  clocks:
+    items:
+      - description: GCC MSS IFACE clock
+      - description: GCC MSS BUS clock
+      - description: GCC MSS NAV clock
+      - description: GCC MSS SNOC_AXI clock
+      - description: GCC MSS MFAB_AXIS clock
+      - description: RPMH XO clock
+
+  clock-names:
+    items:
+      - const: iface
+      - const: bus
+      - const: nav
+      - const: snoc_axi
+      - const: mnoc_axi
+      - const: xo
+
+  power-domains:
+    items:
+      - description: CX power domain
+      - description: MX power domain
+      - description: MSS power domain
+
+  power-domain-names:
+    items:
+      - const: cx
+      - const: mx
+      - const: mss
+
+  resets:
+    items:
+      - description: AOSS restart
+      - description: PDC reset
+
+  reset-names:
+    items:
+      - const: mss_restart
+      - const: pdc_reset
+
+  memory-region:
+    maxItems: 2
+    description: Phandle reference to the reserved-memory for the MBA region followed
+                 by the modem region.
+
+  firmware-name:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    maxItems: 2
+    description:
+      The name of the MBA and modem firmware to be loaded for this remote processor.
+
+  qcom,halt-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Phandle reference to a syscon representing TCSR followed by the
+      three offsets within syscon for q6, modem and nc halt registers.
+
+  qcom,spare-regs:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Phandle reference to a syscon representing TCSR followed by the
+      offset within syscon for conn_box_spare0 register.
+
+  qcom,qmp:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Reference to the AOSS side-channel message RAM.
+
+  qcom,smem-states:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: States used by the AP to signal the Hexagon core
+    items:
+      - description: Stop the modem
+
+  qcom,smem-state-names:
+    description: The names of the state bits used for SMP2P output
+    const: stop
+
+  glink-edge:
+    $ref: qcom,glink-edge.yaml#
+    description:
+      Qualcomm G-Link subnode which represents communication edge, channels
+      and devices related to the DSP.
+
+    properties:
+      interrupts:
+        items:
+          - description: IRQ from MSS to GLINK
+
+      mboxes:
+        items:
+          - description: Mailbox for communication between APPS and MSS
+
+      label:
+        items:
+          - const: modem
+
+      apr: false
+      fastrpc: false
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - iommus
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - power-domains
+  - power-domain-names
+  - resets
+  - reset-names
+  - qcom,halt-regs
+  - qcom,spare-regs
+  - memory-region
+  - qcom,qmp
+  - qcom,smem-states
+  - qcom,smem-state-names
+  - glink-edge
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
+    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
+
+    remoteproc_mpss: remoteproc@4080000 {
+        compatible = "qcom,sc7180-mss-pil";
+        reg = <0x04080000 0x10000>, <0x04180000 0x48>;
+        reg-names = "qdsp6", "rmb";
+
+        iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>;
+
+        interrupts-extended = <&intc GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
+                              <&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
+
+        interrupt-names = "wdog", "fatal", "ready", "handover",
+                          "stop-ack", "shutdown-ack";
+
+        clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
+                 <&gcc GCC_MSS_Q6_MEMNOC_AXI_CLK>,
+                 <&gcc GCC_MSS_NAV_AXI_CLK>,
+                 <&gcc GCC_MSS_SNOC_AXI_CLK>,
+                 <&gcc GCC_MSS_MFAB_AXIS_CLK>,
+                 <&rpmhcc RPMH_CXO_CLK>;
+        clock-names = "iface", "bus", "nav", "snoc_axi",
+                      "mnoc_axi", "xo";
+
+        power-domains = <&rpmhpd SC7180_CX>,
+                        <&rpmhpd SC7180_MX>,
+                        <&rpmhpd SC7180_MSS>;
+        power-domain-names = "cx", "mx", "mss";
+
+        memory-region = <&mba_mem>, <&mpss_mem>;
+
+        qcom,qmp = <&aoss_qmp>;
+
+        qcom,smem-states = <&modem_smp2p_out 0>;
+        qcom,smem-state-names = "stop";
+
+        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
+                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
+        reset-names = "mss_restart", "pdc_reset";
+
+        qcom,halt-regs = <&tcsr_mutex_regs 0x23000 0x25000 0x24000>;
+        qcom,spare-regs = <&tcsr_regs 0xb3e4>;
+
+        glink-edge {
+            interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>;
+            mboxes = <&apss_shared 12>;
+            qcom,remote-pid = <1>;
+            label = "modem";
+        };
+    };
-- 
2.7.4

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Rob Herring]

On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
> Convert SC7180 MSS PIL loading bindings to YAML.

I suppose there is a reason the sc7180 is being split out and the only 
one converted, but this doesn't tell me.
> 
> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
> ---
>  .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |  20 +-
>  .../bindings/remoteproc/qcom,sc7180-mss-pil.yaml   | 236 +++++++++++++++++++++
>  2 files changed, 238 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> index 1ec9093c3a82..1a691c1435f3 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
> @@ -14,7 +14,6 @@ on the Qualcomm Hexagon core.
>  		    "qcom,msm8974-mss-pil"
>  		    "qcom,msm8996-mss-pil"
>  		    "qcom,msm8998-mss-pil"
> -		    "qcom,sc7180-mss-pil"
>  		    "qcom,sdm845-mss-pil"
>  
>  - reg:
> @@ -46,7 +45,6 @@ on the Qualcomm Hexagon core.
>  		    must be "wdog", "fatal", "ready", "handover", "stop-ack"
>  	qcom,msm8996-mss-pil:
>  	qcom,msm8998-mss-pil:
> -	qcom,sc7180-mss-pil:
>  	qcom,sdm845-mss-pil:
>  		    must be "wdog", "fatal", "ready", "handover", "stop-ack",
>  		    "shutdown-ack"
> @@ -84,9 +82,6 @@ on the Qualcomm Hexagon core.
>  	qcom,msm8998-mss-pil:
>  		    must be "iface", "bus", "mem", "xo", "gpll0_mss",
>  		    "snoc_axi", "mnoc_axi", "qdss"
> -	qcom,sc7180-mss-pil:
> -		    must be "iface", "bus", "xo", "snoc_axi", "mnoc_axi",
> -		    "nav"
>  	qcom,sdm845-mss-pil:
>  		    must be "iface", "bus", "mem", "xo", "gpll0_mss",
>  		    "snoc_axi", "mnoc_axi", "prng"
> @@ -98,7 +93,7 @@ on the Qualcomm Hexagon core.
>  		    reference to the list of 3 reset-controllers for the
>  		    wcss sub-system
>  		    reference to the list of 2 reset-controllers for the modem
> -		    sub-system on SC7180, SDM845 SoCs
> +		    sub-system on SDM845 SoCs
>  
>  - reset-names:
>  	Usage: required
> @@ -107,7 +102,7 @@ on the Qualcomm Hexagon core.
>  		    must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
>  		    for the wcss sub-system
>  		    must be "mss_restart", "pdc_reset" for the modem
> -		    sub-system on SC7180, SDM845 SoCs
> +		    sub-system on SDM845 SoCs
>  
>  For devices where the mba and mpss sub-nodes are not specified, mba/mpss region
>  should be referenced as follows:
> @@ -172,8 +167,6 @@ For the compatible string below the following supplies are required:
>  	qcom,msm8996-mss-pil:
>  	qcom,msm8998-mss-pil:
>  		    must be "cx", "mx"
> -	qcom,sc7180-mss-pil:
> -		    must be "cx", "mx", "mss"
>  	qcom,sdm845-mss-pil:
>  		    must be "cx", "mx", "mss"
>  
> @@ -200,15 +193,6 @@ For the compatible string below the following supplies are required:
>  		    by the three offsets within syscon for q6, modem and nc
>  		    halt registers.
>  
> -For the compatible strings below the following phandle references are required:
> -  "qcom,sc7180-mss-pil"
> -- qcom,spare-regs:
> -	Usage: required
> -	Value type: <prop-encoded-array>
> -	Definition: a phandle reference to a syscon representing TCSR followed
> -		    by the offset within syscon for conn_box_spare0 register
> -		    used by the modem sub-system running on SC7180 SoC.
> -
>  The Hexagon node must contain iommus property as described in ../iommu/iommu.txt
>  on platforms which do not have TrustZone.
>  
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
> new file mode 100644
> index 000000000000..b00fdf9e3eeb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
> @@ -0,0 +1,236 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/qcom,sc7180-mss-pil.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SC7180 MSS Peripheral Image Loader
> +
> +maintainers:
> +  - Sibi Sankar <quic_sibis@quicinc.com>
> +
> +description:
> +  This document describes the hardware for a component that loads and boots firmware
> +  on the Qualcomm Technology Inc. SC7180 Modem Hexagon Core.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sc7180-mss-pil
> +
> +  reg:
> +    items:
> +      - description: MSS QDSP6 registers
> +      - description: RMB registers
> +
> +  reg-names:
> +    items:
> +      - const: qdsp6
> +      - const: rmb
> +
> +  iommus:
> +    items:
> +      - description: MSA Stream 1
> +      - description: MSA Stream 2
> +
> +  interrupts:
> +    items:
> +      - description: Watchdog interrupt
> +      - description: Fatal interrupt
> +      - description: Ready interrupt
> +      - description: Handover interrupt
> +      - description: Stop acknowledge interrupt
> +      - description: Shutdown acknowledge interrupt
> +
> +  interrupt-names:
> +    items:
> +      - const: wdog
> +      - const: fatal
> +      - const: ready
> +      - const: handover
> +      - const: stop-ack
> +      - const: shutdown-ack
> +
> +  clocks:
> +    items:
> +      - description: GCC MSS IFACE clock
> +      - description: GCC MSS BUS clock
> +      - description: GCC MSS NAV clock
> +      - description: GCC MSS SNOC_AXI clock
> +      - description: GCC MSS MFAB_AXIS clock
> +      - description: RPMH XO clock
> +
> +  clock-names:
> +    items:
> +      - const: iface
> +      - const: bus
> +      - const: nav
> +      - const: snoc_axi
> +      - const: mnoc_axi
> +      - const: xo
> +
> +  power-domains:
> +    items:
> +      - description: CX power domain
> +      - description: MX power domain
> +      - description: MSS power domain
> +
> +  power-domain-names:
> +    items:
> +      - const: cx
> +      - const: mx
> +      - const: mss
> +
> +  resets:
> +    items:
> +      - description: AOSS restart
> +      - description: PDC reset
> +
> +  reset-names:
> +    items:
> +      - const: mss_restart
> +      - const: pdc_reset
> +
> +  memory-region:
> +    maxItems: 2
> +    description: Phandle reference to the reserved-memory for the MBA region followed
> +                 by the modem region.

Use 'items' to describe each entry.

> +
> +  firmware-name:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    maxItems: 2
> +    description:
> +      The name of the MBA and modem firmware to be loaded for this remote processor.
> +
> +  qcom,halt-regs:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Phandle reference to a syscon representing TCSR followed by the
> +      three offsets within syscon for q6, modem and nc halt registers.

You need to define this as a schema:

items:
  - items:
      - description: ...
      - description: ...
      - description: ...
      - description: ...

> +
> +  qcom,spare-regs:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Phandle reference to a syscon representing TCSR followed by the
> +      offset within syscon for conn_box_spare0 register.

Same here.

> +
> +  qcom,qmp:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: Reference to the AOSS side-channel message RAM.
> +
> +  qcom,smem-states:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array

I believe there's a common schema for this now or one pending so you 
should drop the type here.

> +    description: States used by the AP to signal the Hexagon core
> +    items:
> +      - description: Stop the modem

That's not sufficient for a phandle-array which is really a matrix.

> +
> +  qcom,smem-state-names:
> +    description: The names of the state bits used for SMP2P output
> +    const: stop
> +
> +  glink-edge:
> +    $ref: qcom,glink-edge.yaml#
> +    description:
> +      Qualcomm G-Link subnode which represents communication edge, channels
> +      and devices related to the DSP.
> +
> +    properties:
> +      interrupts:
> +        items:
> +          - description: IRQ from MSS to GLINK
> +
> +      mboxes:
> +        items:
> +          - description: Mailbox for communication between APPS and MSS
> +
> +      label:
> +        items:

Only 1, you can drop 'items'. And 'label' is always 1 entry.

> +          - const: modem
> +
> +      apr: false
> +      fastrpc: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - iommus
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - power-domain-names
> +  - resets
> +  - reset-names
> +  - qcom,halt-regs
> +  - qcom,spare-regs
> +  - memory-region
> +  - qcom,qmp
> +  - qcom,smem-states
> +  - qcom,smem-state-names
> +  - glink-edge
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
> +    #include <dt-bindings/clock/qcom,rpmh.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/qcom-rpmpd.h>
> +    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
> +    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
> +
> +    remoteproc_mpss: remoteproc@4080000 {
> +        compatible = "qcom,sc7180-mss-pil";
> +        reg = <0x04080000 0x10000>, <0x04180000 0x48>;
> +        reg-names = "qdsp6", "rmb";
> +
> +        iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>;
> +
> +        interrupts-extended = <&intc GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
> +                              <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> +                              <&modem_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> +                              <&modem_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> +                              <&modem_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
> +                              <&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
> +
> +        interrupt-names = "wdog", "fatal", "ready", "handover",
> +                          "stop-ack", "shutdown-ack";
> +
> +        clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
> +                 <&gcc GCC_MSS_Q6_MEMNOC_AXI_CLK>,
> +                 <&gcc GCC_MSS_NAV_AXI_CLK>,
> +                 <&gcc GCC_MSS_SNOC_AXI_CLK>,
> +                 <&gcc GCC_MSS_MFAB_AXIS_CLK>,
> +                 <&rpmhcc RPMH_CXO_CLK>;
> +        clock-names = "iface", "bus", "nav", "snoc_axi",
> +                      "mnoc_axi", "xo";
> +
> +        power-domains = <&rpmhpd SC7180_CX>,
> +                        <&rpmhpd SC7180_MX>,
> +                        <&rpmhpd SC7180_MSS>;
> +        power-domain-names = "cx", "mx", "mss";
> +
> +        memory-region = <&mba_mem>, <&mpss_mem>;
> +
> +        qcom,qmp = <&aoss_qmp>;
> +
> +        qcom,smem-states = <&modem_smp2p_out 0>;
> +        qcom,smem-state-names = "stop";
> +
> +        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
> +                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
> +        reset-names = "mss_restart", "pdc_reset";
> +
> +        qcom,halt-regs = <&tcsr_mutex_regs 0x23000 0x25000 0x24000>;
> +        qcom,spare-regs = <&tcsr_regs 0xb3e4>;
> +
> +        glink-edge {
> +            interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>;
> +            mboxes = <&apss_shared 12>;
> +            qcom,remote-pid = <1>;
> +            label = "modem";
> +        };
> +    };
> -- 
> 2.7.4
> 
> 

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Krzysztof Kozlowski]

On 21/05/2022 00:40, Rob Herring wrote:
> On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
>> Convert SC7180 MSS PIL loading bindings to YAML.
> 
> I suppose there is a reason the sc7180 is being split out and the only 
> one converted, but this doesn't tell me.

I am also confused, especially that last time I pointed out that there
is work already:
https://lore.kernel.org/all/20220511161602.117772-7-sireeshkodali1@gmail.com/


Best regards,
Krzysztof

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Sibi Sankar]

Hey Rob/Krzysztof,

On 5/21/22 8:04 PM, Krzysztof Kozlowski wrote:
> On 21/05/2022 00:40, Rob Herring wrote:
>> On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
>>> Convert SC7180 MSS PIL loading bindings to YAML.
>>
>> I suppose there is a reason the sc7180 is being split out and the only
>> one converted, but this doesn't tell me.
> 
> I am also confused, especially that last time I pointed out that there
> is work already:
> https://lore.kernel.org/all/20220511161602.117772-7-sireeshkodali1@gmail.com/

https://lore.kernel.org/all/e3543961-1645-b02a-c869-f8fa1ad2d41c@quicinc.com/#t

The reason for the split was discussed on the list ^^, thought it
wouldn't make much sense adding any of it to the commit message.
Also since Krzysztof said he wanted a alignment between Sireesh/me
we did exchange mails saying I'll take care of SC7180/SC7280 (since
they had pas compatible which is overridden by mss compatible) and
he could continue with the rest.

-Sibi

> 
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Krzysztof Kozlowski]

On 24/05/2022 04:00, Sibi Sankar wrote:
> Hey Rob/Krzysztof,
> 
> On 5/21/22 8:04 PM, Krzysztof Kozlowski wrote:
>> On 21/05/2022 00:40, Rob Herring wrote:
>>> On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
>>>> Convert SC7180 MSS PIL loading bindings to YAML.
>>>
>>> I suppose there is a reason the sc7180 is being split out and the only
>>> one converted, but this doesn't tell me.
>>
>> I am also confused, especially that last time I pointed out that there
>> is work already:
>> https://lore.kernel.org/all/20220511161602.117772-7-sireeshkodali1@gmail.com/
> 
> https://lore.kernel.org/all/e3543961-1645-b02a-c869-f8fa1ad2d41c@quicinc.com/#t
> 
> The reason for the split was discussed on the list ^^, thought it
> wouldn't make much sense adding any of it to the commit message.
> Also since Krzysztof said he wanted a alignment between Sireesh/me
> we did exchange mails saying I'll take care of SC7180/SC7280 (since
> they had pas compatible which is overridden by mss compatible) and
> he could continue with the rest.


Sounds good to me, but Rob's got a point - this background should be
better explained.


Best regards,
Krzysztof

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Sibi Sankar]

On 5/24/22 11:03 PM, Krzysztof Kozlowski wrote:
> On 24/05/2022 04:00, Sibi Sankar wrote:
>> Hey Rob/Krzysztof,
>>
>> On 5/21/22 8:04 PM, Krzysztof Kozlowski wrote:
>>> On 21/05/2022 00:40, Rob Herring wrote:
>>>> On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
>>>>> Convert SC7180 MSS PIL loading bindings to YAML.
>>>>
>>>> I suppose there is a reason the sc7180 is being split out and the only
>>>> one converted, but this doesn't tell me.
>>>
>>> I am also confused, especially that last time I pointed out that there
>>> is work already:
>>> https://lore.kernel.org/all/20220511161602.117772-7-sireeshkodali1@gmail.com/
>>
>> https://lore.kernel.org/all/e3543961-1645-b02a-c869-f8fa1ad2d41c@quicinc.com/#t
>>
>> The reason for the split was discussed on the list ^^, thought it
>> wouldn't make much sense adding any of it to the commit message.
>> Also since Krzysztof said he wanted a alignment between Sireesh/me
>> we did exchange mails saying I'll take care of SC7180/SC7280 (since
>> they had pas compatible which is overridden by mss compatible) and
>> he could continue with the rest.
> 
> 
> Sounds good to me, but Rob's got a point - this background should be
> better explained.
> 

ack

-Sibi

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Sibi Sankar]

Hey Rob,
Thanks for taking time to review the series.

On 5/21/22 4:10 AM, Rob Herring wrote:
> On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
>> Convert SC7180 MSS PIL loading bindings to YAML.
> 
> I suppose there is a reason the sc7180 is being split out and the only
> one converted, but this doesn't tell me.

https://lore.kernel.org/all/e3543961-1645-b02a-c869-f8fa1ad2d41c@quicinc.com/#t

The reason for the split was discussed on the list ^^, thought it
wouldn't make much sense adding any of it to the commit message.

>>
>> Signed-off-by: Sibi Sankar <quic_sibis@quicinc.com>
>> ---
>>   .../devicetree/bindings/remoteproc/qcom,q6v5.txt   |  20 +-
>>   .../bindings/remoteproc/qcom,sc7180-mss-pil.yaml   | 236 +++++++++++++++++++++
>>   2 files changed, 238 insertions(+), 18 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> index 1ec9093c3a82..1a691c1435f3 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt
>> @@ -14,7 +14,6 @@ on the Qualcomm Hexagon core.
>>   		    "qcom,msm8974-mss-pil"
>>   		    "qcom,msm8996-mss-pil"
>>   		    "qcom,msm8998-mss-pil"
>> -		    "qcom,sc7180-mss-pil"
>>   		    "qcom,sdm845-mss-pil"
>>   
>>   - reg:
>> @@ -46,7 +45,6 @@ on the Qualcomm Hexagon core.
>>   		    must be "wdog", "fatal", "ready", "handover", "stop-ack"
>>   	qcom,msm8996-mss-pil:
>>   	qcom,msm8998-mss-pil:
>> -	qcom,sc7180-mss-pil:
>>   	qcom,sdm845-mss-pil:
>>   		    must be "wdog", "fatal", "ready", "handover", "stop-ack",
>>   		    "shutdown-ack"
>> @@ -84,9 +82,6 @@ on the Qualcomm Hexagon core.
>>   	qcom,msm8998-mss-pil:
>>   		    must be "iface", "bus", "mem", "xo", "gpll0_mss",
>>   		    "snoc_axi", "mnoc_axi", "qdss"
>> -	qcom,sc7180-mss-pil:
>> -		    must be "iface", "bus", "xo", "snoc_axi", "mnoc_axi",
>> -		    "nav"
>>   	qcom,sdm845-mss-pil:
>>   		    must be "iface", "bus", "mem", "xo", "gpll0_mss",
>>   		    "snoc_axi", "mnoc_axi", "prng"
>> @@ -98,7 +93,7 @@ on the Qualcomm Hexagon core.
>>   		    reference to the list of 3 reset-controllers for the
>>   		    wcss sub-system
>>   		    reference to the list of 2 reset-controllers for the modem
>> -		    sub-system on SC7180, SDM845 SoCs
>> +		    sub-system on SDM845 SoCs
>>   
>>   - reset-names:
>>   	Usage: required
>> @@ -107,7 +102,7 @@ on the Qualcomm Hexagon core.
>>   		    must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset"
>>   		    for the wcss sub-system
>>   		    must be "mss_restart", "pdc_reset" for the modem
>> -		    sub-system on SC7180, SDM845 SoCs
>> +		    sub-system on SDM845 SoCs
>>   
>>   For devices where the mba and mpss sub-nodes are not specified, mba/mpss region
>>   should be referenced as follows:
>> @@ -172,8 +167,6 @@ For the compatible string below the following supplies are required:
>>   	qcom,msm8996-mss-pil:
>>   	qcom,msm8998-mss-pil:
>>   		    must be "cx", "mx"
>> -	qcom,sc7180-mss-pil:
>> -		    must be "cx", "mx", "mss"
>>   	qcom,sdm845-mss-pil:
>>   		    must be "cx", "mx", "mss"
>>   
>> @@ -200,15 +193,6 @@ For the compatible string below the following supplies are required:
>>   		    by the three offsets within syscon for q6, modem and nc
>>   		    halt registers.
>>   
>> -For the compatible strings below the following phandle references are required:
>> -  "qcom,sc7180-mss-pil"
>> -- qcom,spare-regs:
>> -	Usage: required
>> -	Value type: <prop-encoded-array>
>> -	Definition: a phandle reference to a syscon representing TCSR followed
>> -		    by the offset within syscon for conn_box_spare0 register
>> -		    used by the modem sub-system running on SC7180 SoC.
>> -
>>   The Hexagon node must contain iommus property as described in ../iommu/iommu.txt
>>   on platforms which do not have TrustZone.
>>   
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
>> new file mode 100644
>> index 000000000000..b00fdf9e3eeb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7180-mss-pil.yaml
>> @@ -0,0 +1,236 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/remoteproc/qcom,sc7180-mss-pil.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm SC7180 MSS Peripheral Image Loader
>> +
>> +maintainers:
>> +  - Sibi Sankar <quic_sibis@quicinc.com>
>> +
>> +description:
>> +  This document describes the hardware for a component that loads and boots firmware
>> +  on the Qualcomm Technology Inc. SC7180 Modem Hexagon Core.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,sc7180-mss-pil
>> +
>> +  reg:
>> +    items:
>> +      - description: MSS QDSP6 registers
>> +      - description: RMB registers
>> +
>> +  reg-names:
>> +    items:
>> +      - const: qdsp6
>> +      - const: rmb
>> +
>> +  iommus:
>> +    items:
>> +      - description: MSA Stream 1
>> +      - description: MSA Stream 2
>> +
>> +  interrupts:
>> +    items:
>> +      - description: Watchdog interrupt
>> +      - description: Fatal interrupt
>> +      - description: Ready interrupt
>> +      - description: Handover interrupt
>> +      - description: Stop acknowledge interrupt
>> +      - description: Shutdown acknowledge interrupt
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: wdog
>> +      - const: fatal
>> +      - const: ready
>> +      - const: handover
>> +      - const: stop-ack
>> +      - const: shutdown-ack
>> +
>> +  clocks:
>> +    items:
>> +      - description: GCC MSS IFACE clock
>> +      - description: GCC MSS BUS clock
>> +      - description: GCC MSS NAV clock
>> +      - description: GCC MSS SNOC_AXI clock
>> +      - description: GCC MSS MFAB_AXIS clock
>> +      - description: RPMH XO clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: iface
>> +      - const: bus
>> +      - const: nav
>> +      - const: snoc_axi
>> +      - const: mnoc_axi
>> +      - const: xo
>> +
>> +  power-domains:
>> +    items:
>> +      - description: CX power domain
>> +      - description: MX power domain
>> +      - description: MSS power domain
>> +
>> +  power-domain-names:
>> +    items:
>> +      - const: cx
>> +      - const: mx
>> +      - const: mss
>> +
>> +  resets:
>> +    items:
>> +      - description: AOSS restart
>> +      - description: PDC reset
>> +
>> +  reset-names:
>> +    items:
>> +      - const: mss_restart
>> +      - const: pdc_reset
>> +
>> +  memory-region:
>> +    maxItems: 2
>> +    description: Phandle reference to the reserved-memory for the MBA region followed
>> +                 by the modem region.
> 
> Use 'items' to describe each entry.

ack

> 
>> +
>> +  firmware-name:
>> +    $ref: /schemas/types.yaml#/definitions/string-array
>> +    maxItems: 2
>> +    description:
>> +      The name of the MBA and modem firmware to be loaded for this remote processor.
>> +
>> +  qcom,halt-regs:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description:
>> +      Phandle reference to a syscon representing TCSR followed by the
>> +      three offsets within syscon for q6, modem and nc halt registers.
> 
> You need to define this as a schema:
> 
> items:
>    - items:
>        - description: ...
>        - description: ...
>        - description: ...
>        - description: ...
> 

ack

>> +
>> +  qcom,spare-regs:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description:
>> +      Phandle reference to a syscon representing TCSR followed by the
>> +      offset within syscon for conn_box_spare0 register.
> 
> Same here.

ack

> 
>> +
>> +  qcom,qmp:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description: Reference to the AOSS side-channel message RAM.
>> +
>> +  qcom,smem-states:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> I believe there's a common schema for this now or one pending so you
> should drop the type here.

Last time I checked all the other remoteprocs handle
them this way and there wasn't a common schema for it.

> 
>> +    description: States used by the AP to signal the Hexagon core
>> +    items:
>> +      - description: Stop the modem
> 
> That's not sufficient for a phandle-array which is really a matrix.

will update the description.

> 
>> +
>> +  qcom,smem-state-names:
>> +    description: The names of the state bits used for SMP2P output
>> +    const: stop
>> +
>> +  glink-edge:
>> +    $ref: qcom,glink-edge.yaml#
>> +    description:
>> +      Qualcomm G-Link subnode which represents communication edge, channels
>> +      and devices related to the DSP.
>> +
>> +    properties:
>> +      interrupts:
>> +        items:
>> +          - description: IRQ from MSS to GLINK
>> +
>> +      mboxes:
>> +        items:
>> +          - description: Mailbox for communication between APPS and MSS
>> +
>> +      label:
>> +        items:
> 
> Only 1, you can drop 'items'. And 'label' is always 1 entry.

ack

> 
>> +          - const: modem
>> +
>> +      apr: false
>> +      fastrpc: false
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - iommus
>> +  - interrupts
>> +  - interrupt-names
>> +  - clocks
>> +  - clock-names
>> +  - power-domains
>> +  - power-domain-names
>> +  - resets
>> +  - reset-names
>> +  - qcom,halt-regs
>> +  - qcom,spare-regs
>> +  - memory-region
>> +  - qcom,qmp
>> +  - qcom,smem-states
>> +  - qcom,smem-state-names
>> +  - glink-edge
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/qcom,gcc-sc7180.h>
>> +    #include <dt-bindings/clock/qcom,rpmh.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/power/qcom-rpmpd.h>
>> +    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>> +    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
>> +
>> +    remoteproc_mpss: remoteproc@4080000 {
>> +        compatible = "qcom,sc7180-mss-pil";
>> +        reg = <0x04080000 0x10000>, <0x04180000 0x48>;
>> +        reg-names = "qdsp6", "rmb";
>> +
>> +        iommus = <&apps_smmu 0x461 0x0>, <&apps_smmu 0x444 0x3>;
>> +
>> +        interrupts-extended = <&intc GIC_SPI 264 IRQ_TYPE_EDGE_RISING>,
>> +                              <&modem_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
>> +                              <&modem_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
>> +                              <&modem_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
>> +                              <&modem_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
>> +                              <&modem_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
>> +
>> +        interrupt-names = "wdog", "fatal", "ready", "handover",
>> +                          "stop-ack", "shutdown-ack";
>> +
>> +        clocks = <&gcc GCC_MSS_CFG_AHB_CLK>,
>> +                 <&gcc GCC_MSS_Q6_MEMNOC_AXI_CLK>,
>> +                 <&gcc GCC_MSS_NAV_AXI_CLK>,
>> +                 <&gcc GCC_MSS_SNOC_AXI_CLK>,
>> +                 <&gcc GCC_MSS_MFAB_AXIS_CLK>,
>> +                 <&rpmhcc RPMH_CXO_CLK>;
>> +        clock-names = "iface", "bus", "nav", "snoc_axi",
>> +                      "mnoc_axi", "xo";
>> +
>> +        power-domains = <&rpmhpd SC7180_CX>,
>> +                        <&rpmhpd SC7180_MX>,
>> +                        <&rpmhpd SC7180_MSS>;
>> +        power-domain-names = "cx", "mx", "mss";
>> +
>> +        memory-region = <&mba_mem>, <&mpss_mem>;
>> +
>> +        qcom,qmp = <&aoss_qmp>;
>> +
>> +        qcom,smem-states = <&modem_smp2p_out 0>;
>> +        qcom,smem-state-names = "stop";
>> +
>> +        resets = <&aoss_reset AOSS_CC_MSS_RESTART>,
>> +                 <&pdc_reset PDC_MODEM_SYNC_RESET>;
>> +        reset-names = "mss_restart", "pdc_reset";
>> +
>> +        qcom,halt-regs = <&tcsr_mutex_regs 0x23000 0x25000 0x24000>;
>> +        qcom,spare-regs = <&tcsr_regs 0xb3e4>;
>> +
>> +        glink-edge {
>> +            interrupts = <GIC_SPI 449 IRQ_TYPE_EDGE_RISING>;
>> +            mboxes = <&apss_shared 12>;
>> +            qcom,remote-pid = <1>;
>> +            label = "modem";
>> +        };
>> +    };
>> -- 
>> 2.7.4
>>
>>

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Rob Herring]

On Tue, May 24, 2022 at 07:40:51AM +0530, Sibi Sankar wrote:
> Hey Rob,
> Thanks for taking time to review the series.
> 
> On 5/21/22 4:10 AM, Rob Herring wrote:
> > On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
> > > Convert SC7180 MSS PIL loading bindings to YAML.
> > 
> > I suppose there is a reason the sc7180 is being split out and the only
> > one converted, but this doesn't tell me.
> 
> https://lore.kernel.org/all/e3543961-1645-b02a-c869-f8fa1ad2d41c@quicinc.com/#t
> 
> The reason for the split was discussed on the list ^^, thought it
> wouldn't make much sense adding any of it to the commit message.

Why not? If you did, then we wouldn't be having this conversation.

Commit messages, at a minimum, should answer why are you making the 
change. They don't really need to explain what the change is. We can all 
read the diff to understand that.

Rob

Re: [PATCH v4 3/3] dt-bindings: remoteproc: qcom: Convert SC7180 MSS bindings to YAML

[Sibi Sankar]

Hey Rob,

On 5/24/22 7:39 PM, Rob Herring wrote:
> On Tue, May 24, 2022 at 07:40:51AM +0530, Sibi Sankar wrote:
>> Hey Rob,
>> Thanks for taking time to review the series.
>>
>> On 5/21/22 4:10 AM, Rob Herring wrote:
>>> On Thu, May 19, 2022 at 10:17:05PM +0530, Sibi Sankar wrote:
>>>> Convert SC7180 MSS PIL loading bindings to YAML.
>>>
>>> I suppose there is a reason the sc7180 is being split out and the only
>>> one converted, but this doesn't tell me.
>>
>> https://lore.kernel.org/all/e3543961-1645-b02a-c869-f8fa1ad2d41c@quicinc.com/#t
>>
>> The reason for the split was discussed on the list ^^, thought it
>> wouldn't make much sense adding any of it to the commit message.
> 
> Why not? If you did, then we wouldn't be having this conversation.
> 
> Commit messages, at a minimum, should answer why are you making the
> change. They don't really need to explain what the change is. We can all
> read the diff to understand that.
> 

Sure will add the details in the next re-spin.

-Sibi

> Rob
>