[PATCH v6 0/8] Update ADSP pil loader for SC7280 platform

[PATCH v6 0/8] Update ADSP pil loader for SC7280 platform

[Srinivasa Rao Mandadapu]

Update ADSP pil loader driver for SC7280 platforms.

Changes since V5:
	-- Remove adsp_rproc_unmap_smmu, adsp_of_unmap_smmu, adsp_of_map_smmu and 
	   adsp_rproc_map_smmu functions.
	-- Remove find_loaded_rsc_table call back initialization.
	-- Rename adsp_sandbox_needed to has_iommu.
	-- Update parse_fw callback in rproc ops.
	-- Remove qcom,adsp-memory-regions property in dt-bindings.
	-- Change adsp binary extention name.
Changes since V4:
	-- Update halt registers description in dt bindings.
	-- Update Memory sandboxing with proper APIs for resource
	   allocation and free.
Changes since V3:
	-- Rename is_adsp_sb_needed to adsp_sandbox_needed.
	-- Update sc7280 compatible name entry in sorted order.
	-- Add smmu unmapping in error case and in adsp stop.
	-- Revert converting sdm845 dt bindings to generic and 
	   create new dt bindings for sc7280.
Changes since V2:
	-- Generated patch with -M flag.
	-- Add Clock property in dt bindings.
	-- Add qcom,adsp-memory-regions property.
	-- Add is_adsp_sb_needed flag instead of is_wpss.
	-- Initialize is_adsp_sb_needed flag.
	-- Remove empty proxy pds array.
	-- Replace platform_bus_type with adsp->dev->bus.
	-- Use API of_parse_phandle_with_args() instead of 
	    of_parse_phandle_with_fixed_args().
	-- Replace adsp->is_wpss with adsp->is_adsp.
	-- Update error handling in adsp_start().
Changes since V1:
	-- Change reg property maxItems to minItems and update description.
	-- Fix typo errors.

Srinivasa Rao Mandadapu (8):
  dt-bindings: remoteproc: qcom: Add SC7280 ADSP support
  remoteproc: qcom: Add flag in adsp private data structure
  remoteproc: qcom: Add compatible name for SC7280 ADSP
  remoteproc: qcom: Update rproc parse firmware callback
  remoteproc: qcom: Replace hard coded values with macros
  remoteproc: qcom: Add efuse evb selection control
  remoteproc: qcom: Add support for memory sandbox
  remoteproc: qcom: Update QDSP6 out-of-reset timeout value

 .../bindings/remoteproc/qcom,sc7280-adsp-pil.yaml  | 175 +++++++++++++++++++++
 drivers/remoteproc/qcom_q6v5_adsp.c                | 110 ++++++++++++-
 2 files changed, 279 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml

-- 
2.7.4

[PATCH v6 1/8] dt-bindings: remoteproc: qcom: Add SC7280 ADSP support

[Srinivasa Rao Mandadapu]

Add ADSP PIL loading support for SC7280 SoCs.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes since V5:
	-- Remove qcom,adsp-memory-regions property.
Changes since V4:
	-- Update halt registers description in dt bindings.

 .../bindings/remoteproc/qcom,sc7280-adsp-pil.yaml  | 175 +++++++++++++++++++++
 1 file changed, 175 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
new file mode 100644
index 0000000..1428522
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
@@ -0,0 +1,175 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,sc7280-adsp-pil.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SC7280 ADSP Peripheral Image Loader
+
+maintainers:
+  - Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
+
+description:
+  This document defines the binding for a component that loads and boots firmware
+  on the Qualcomm Technology Inc. ADSP.
+
+properties:
+  compatible:
+    enum:
+      - qcom,sc7280-adsp-pil
+
+  reg:
+    minItems: 1
+    items:
+      - description: qdsp6ss register
+      - description: efuse q6ss register
+
+  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: XO clock
+      - description: GCC CFG NOC LPASS clock
+      - description: LPASS AHBS AON clock
+      - description: LPASS AHBM AON clock
+      - description: QDSP XO clock
+      - description: Q6SP6SS SLEEP clock
+      - description: Q6SP6SS CORE clock
+
+  clock-names:
+    items:
+      - const: xo
+      - const: gcc_cfg_noc_lpass
+      - const: lpass_ahbs_aon_cbcr
+      - const: lpass_ahbm_aon_cbcr
+      - const: qdsp6ss_xo
+      - const: qdsp6ss_sleep
+      - const: qdsp6ss_core
+
+  power-domains:
+    items:
+      - description: LCX power domain
+
+  resets:
+    items:
+      - description: PDC AUDIO SYNC RESET
+      - description: CC LPASS restart
+
+  reset-names:
+    items:
+      - const: pdc_sync
+      - const: cc_lpass
+
+  memory-region:
+    maxItems: 1
+    description: Reference to the reserved-memory for the Hexagon core
+
+  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, CE, AXI and qv6 halt registers.
+    items:
+      items:
+        - description: phandle to TCSR MUTEX
+        - description: offset to q6 halt registers
+        - description: offset to CE halt registers
+        - description: offset to AXI halt registers
+        - description: offset to qv6 halt registers
+
+  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:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: The names of the state bits used for SMP2P output
+    items:
+      - const: stop
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+  - reset-names
+  - qcom,halt-regs
+  - memory-region
+  - qcom,smem-states
+  - qcom,smem-state-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,rpmh.h>
+    #include <dt-bindings/clock/qcom,gcc-sc7280.h>
+    #include <dt-bindings/clock/qcom,lpass-sc7280.h>
+    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
+    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
+    #include <dt-bindings/power/qcom-rpmpd.h>
+
+    remoteproc@3000000 {
+        compatible = "qcom,sc7280-adsp-pil";
+        reg = <0x03000000 0x5000>,
+              <0x0355b000 0x10>;
+
+        interrupts-extended = <&pdc 162 IRQ_TYPE_EDGE_RISING>,
+                <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+                <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
+                <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
+                <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
+                <&adsp_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
+
+        interrupt-names = "wdog", "fatal", "ready",
+                "handover", "stop-ack", "shutdown-ack";
+
+        clocks = <&rpmhcc RPMH_CXO_CLK>,
+                 <&gcc GCC_CFG_NOC_LPASS_CLK>,
+                 <&lpasscc LPASS_Q6SS_AHBM_CLK>,
+                 <&lpasscc LPASS_Q6SS_AHBS_CLK>,
+                 <&lpasscc LPASS_QDSP6SS_XO_CLK>,
+                 <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
+                 <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
+        clock-names = "xo", "gcc_cfg_noc_lpass",
+                "lpass_ahbs_aon_cbcr",
+                "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
+                "qdsp6ss_sleep", "qdsp6ss_core";
+
+        power-domains = <&rpmhpd SC7280_LCX>;
+
+        resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
+                 <&aoss_reset AOSS_CC_LPASS_RESTART>;
+        reset-names = "pdc_sync", "cc_lpass";
+
+        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
+
+        memory-region = <&adsp_mem>;
+
+        qcom,smem-states = <&adsp_smp2p_out 0>;
+        qcom,smem-state-names = "stop";
+
+    };
-- 
2.7.4

Re: [PATCH v6 1/8] dt-bindings: remoteproc: qcom: Add SC7280 ADSP support

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:35)
> Add ADSP PIL loading support for SC7280 SoCs.
>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v6 1/8] dt-bindings: remoteproc: qcom: Add SC7280 ADSP support

[Sibi Sankar]

Hey Srinivas,
Thanks for the patch series.

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Add ADSP PIL loading support for SC7280 SoCs.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> Changes since V5:
> 	-- Remove qcom,adsp-memory-regions property.
> Changes since V4:
> 	-- Update halt registers description in dt bindings.
> 
>   .../bindings/remoteproc/qcom,sc7280-adsp-pil.yaml  | 175 +++++++++++++++++++++
>   1 file changed, 175 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
> 
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
> new file mode 100644
> index 0000000..1428522
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/qcom,sc7280-adsp-pil.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm SC7280 ADSP Peripheral Image Loader
> +
> +maintainers:
> +  - Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> +
> +description:
> +  This document defines the binding for a component that loads and boots firmware

s/defines the binding/describes the hardware.

> +  on the Qualcomm Technology Inc. ADSP.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - qcom,sc7280-adsp-pil
> +
> +  reg:
> +    minItems: 1
> +    items:
> +      - description: qdsp6ss register
> +      - description: efuse q6ss register
> +
> +  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: XO clock
> +      - description: GCC CFG NOC LPASS clock
> +      - description: LPASS AHBS AON clock
> +      - description: LPASS AHBM AON clock
> +      - description: QDSP XO clock
> +      - description: Q6SP6SS SLEEP clock
> +      - description: Q6SP6SS CORE clock
> +
> +  clock-names:
> +    items:
> +      - const: xo
> +      - const: gcc_cfg_noc_lpass
> +      - const: lpass_ahbs_aon_cbcr
> +      - const: lpass_ahbm_aon_cbcr
> +      - const: qdsp6ss_xo
> +      - const: qdsp6ss_sleep
> +      - const: qdsp6ss_core
> +
> +  power-domains:
> +    items:
> +      - description: LCX power domain

Doesn't it need the LMX pd as well?


> +
> +  resets:
> +    items:
> +      - description: PDC AUDIO SYNC RESET
> +      - description: CC LPASS restart
> +
> +  reset-names:
> +    items:
> +      - const: pdc_sync
> +      - const: cc_lpass
> +
> +  memory-region:
> +    maxItems: 1
> +    description: Reference to the reserved-memory for the Hexagon core
> +
> +  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, CE, AXI and qv6 halt registers.
> +    items:
> +      items:
> +        - description: phandle to TCSR MUTEX
> +        - description: offset to q6 halt registers
> +        - description: offset to CE halt registers
> +        - description: offset to AXI halt registers
> +        - description: offset to qv6 halt registers
> +
> +  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:
> +    $ref: /schemas/types.yaml#/definitions/string

You can skip ref and items.

> +    description: The names of the state bits used for SMP2P output
> +    items:
> +      - const: stop
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - resets
> +  - reset-names
> +  - qcom,halt-regs
> +  - memory-region
> +  - qcom,smem-states
> +  - qcom,smem-state-names

You probably also need to mention qcom,qmp as a required property.
Not sure why you choose to skip glink-edge as well.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,rpmh.h>
> +    #include <dt-bindings/clock/qcom,gcc-sc7280.h>
> +    #include <dt-bindings/clock/qcom,lpass-sc7280.h>
> +    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
> +    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
> +    #include <dt-bindings/power/qcom-rpmpd.h>
> +
> +    remoteproc@3000000 {
> +        compatible = "qcom,sc7280-adsp-pil";
> +        reg = <0x03000000 0x5000>,
> +              <0x0355b000 0x10>;
> +
> +        interrupts-extended = <&pdc 162 IRQ_TYPE_EDGE_RISING>,
> +                <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
> +                <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
> +                <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
> +                <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
> +                <&adsp_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
> +
> +        interrupt-names = "wdog", "fatal", "ready",
> +                "handover", "stop-ack", "shutdown-ack";
> +
> +        clocks = <&rpmhcc RPMH_CXO_CLK>,
> +                 <&gcc GCC_CFG_NOC_LPASS_CLK>,
> +                 <&lpasscc LPASS_Q6SS_AHBM_CLK>,
> +                 <&lpasscc LPASS_Q6SS_AHBS_CLK>,
> +                 <&lpasscc LPASS_QDSP6SS_XO_CLK>,
> +                 <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
> +                 <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
> +        clock-names = "xo", "gcc_cfg_noc_lpass",
> +                "lpass_ahbs_aon_cbcr",
> +                "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
> +                "qdsp6ss_sleep", "qdsp6ss_core";
> +
> +        power-domains = <&rpmhpd SC7280_LCX>;
> +
> +        resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
> +                 <&aoss_reset AOSS_CC_LPASS_RESTART>;
> +        reset-names = "pdc_sync", "cc_lpass";
> +
> +        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
> +
> +        memory-region = <&adsp_mem>;
> +
> +        qcom,smem-states = <&adsp_smp2p_out 0>;
> +        qcom,smem-state-names = "stop";
> +
> +    };
> 

Re: [PATCH v6 1/8] dt-bindings: remoteproc: qcom: Add SC7280 ADSP support

[Srinivasa Rao Mandadapu]

On 9/14/2022 2:28 PM, Sibi Sankar wrote:
Thanks for Your time Sibi Sankar!!!
> Hey Srinivas,
> Thanks for the patch series.
>
> On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
>> Add ADSP PIL loading support for SC7280 SoCs.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Reviewed-by: Rob Herring <robh@kernel.org>
>> ---
>> Changes since V5:
>>     -- Remove qcom,adsp-memory-regions property.
>> Changes since V4:
>>     -- Update halt registers description in dt bindings.
>>
>>   .../bindings/remoteproc/qcom,sc7280-adsp-pil.yaml  | 175 
>> +++++++++++++++++++++
>>   1 file changed, 175 insertions(+)
>>   create mode 100644 
>> Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml 
>> b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
>> new file mode 100644
>> index 0000000..1428522
>> --- /dev/null
>> +++ 
>> b/Documentation/devicetree/bindings/remoteproc/qcom,sc7280-adsp-pil.yaml
>> @@ -0,0 +1,175 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: 
>> http://devicetree.org/schemas/remoteproc/qcom,sc7280-adsp-pil.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm SC7280 ADSP Peripheral Image Loader
>> +
>> +maintainers:
>> +  - Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> +
>> +description:
>> +  This document defines the binding for a component that loads and 
>> boots firmware
>
> s/defines the binding/describes the hardware.
Okay. Will update accordingly.
>
>> +  on the Qualcomm Technology Inc. ADSP.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - qcom,sc7280-adsp-pil
>> +
>> +  reg:
>> +    minItems: 1
>> +    items:
>> +      - description: qdsp6ss register
>> +      - description: efuse q6ss register
>> +
>> +  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: XO clock
>> +      - description: GCC CFG NOC LPASS clock
>> +      - description: LPASS AHBS AON clock
>> +      - description: LPASS AHBM AON clock
>> +      - description: QDSP XO clock
>> +      - description: Q6SP6SS SLEEP clock
>> +      - description: Q6SP6SS CORE clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: xo
>> +      - const: gcc_cfg_noc_lpass
>> +      - const: lpass_ahbs_aon_cbcr
>> +      - const: lpass_ahbm_aon_cbcr
>> +      - const: qdsp6ss_xo
>> +      - const: qdsp6ss_sleep
>> +      - const: qdsp6ss_core
>> +
>> +  power-domains:
>> +    items:
>> +      - description: LCX power domain
>
> Doesn't it need the LMX pd as well?
So far we are able to do Sanity  test without LMX. I am not sure if it 
is required for any specific use case.
>
>
>> +
>> +  resets:
>> +    items:
>> +      - description: PDC AUDIO SYNC RESET
>> +      - description: CC LPASS restart
>> +
>> +  reset-names:
>> +    items:
>> +      - const: pdc_sync
>> +      - const: cc_lpass
>> +
>> +  memory-region:
>> +    maxItems: 1
>> +    description: Reference to the reserved-memory for the Hexagon core
>> +
>> +  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, CE, AXI and qv6 halt 
>> registers.
>> +    items:
>> +      items:
>> +        - description: phandle to TCSR MUTEX
>> +        - description: offset to q6 halt registers
>> +        - description: offset to CE halt registers
>> +        - description: offset to AXI halt registers
>> +        - description: offset to qv6 halt registers
>> +
>> +  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:
>> +    $ref: /schemas/types.yaml#/definitions/string
>
> You can skip ref and items.
Okay.
>
>> +    description: The names of the state bits used for SMP2P output
>> +    items:
>> +      - const: stop
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - interrupt-names
>> +  - clocks
>> +  - clock-names
>> +  - power-domains
>> +  - resets
>> +  - reset-names
>> +  - qcom,halt-regs
>> +  - memory-region
>> +  - qcom,smem-states
>> +  - qcom,smem-state-names
>
> You probably also need to mention qcom,qmp as a required property.
Okay. Will add it.
> Not sure why you choose to skip glink-edge as well.
Actually I followed previous version of ADSP PIL loader bindings. Will 
add it and re spin the patches.
>
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    #include <dt-bindings/clock/qcom,rpmh.h>
>> +    #include <dt-bindings/clock/qcom,gcc-sc7280.h>
>> +    #include <dt-bindings/clock/qcom,lpass-sc7280.h>
>> +    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>> +    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
>> +    #include <dt-bindings/power/qcom-rpmpd.h>
>> +
>> +    remoteproc@3000000 {
>> +        compatible = "qcom,sc7280-adsp-pil";
>> +        reg = <0x03000000 0x5000>,
>> +              <0x0355b000 0x10>;
>> +
>> +        interrupts-extended = <&pdc 162 IRQ_TYPE_EDGE_RISING>,
>> +                <&adsp_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
>> +                <&adsp_smp2p_in 1 IRQ_TYPE_EDGE_RISING>,
>> +                <&adsp_smp2p_in 2 IRQ_TYPE_EDGE_RISING>,
>> +                <&adsp_smp2p_in 3 IRQ_TYPE_EDGE_RISING>,
>> +                <&adsp_smp2p_in 7 IRQ_TYPE_EDGE_RISING>;
>> +
>> +        interrupt-names = "wdog", "fatal", "ready",
>> +                "handover", "stop-ack", "shutdown-ack";
>> +
>> +        clocks = <&rpmhcc RPMH_CXO_CLK>,
>> +                 <&gcc GCC_CFG_NOC_LPASS_CLK>,
>> +                 <&lpasscc LPASS_Q6SS_AHBM_CLK>,
>> +                 <&lpasscc LPASS_Q6SS_AHBS_CLK>,
>> +                 <&lpasscc LPASS_QDSP6SS_XO_CLK>,
>> +                 <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
>> +                 <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
>> +        clock-names = "xo", "gcc_cfg_noc_lpass",
>> +                "lpass_ahbs_aon_cbcr",
>> +                "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
>> +                "qdsp6ss_sleep", "qdsp6ss_core";
>> +
>> +        power-domains = <&rpmhpd SC7280_LCX>;
>> +
>> +        resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
>> +                 <&aoss_reset AOSS_CC_LPASS_RESTART>;
>> +        reset-names = "pdc_sync", "cc_lpass";
>> +
>> +        qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>;
>> +
>> +        memory-region = <&adsp_mem>;
>> +
>> +        qcom,smem-states = <&adsp_smp2p_out 0>;
>> +        qcom,smem-state-names = "stop";
>> +
>> +    };
>>

[PATCH v6 2/8] remoteproc: qcom: Add flag in adsp private data structure

[Srinivasa Rao Mandadapu]

Add flag in qcom_adsp private data structure and initialize
it to distinguish ADSP and WPSS modules for using iommu selectively.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
Changes since V5:
	-- Rename adsp_sandbox_needed to has_iommu.
Changes since V3:
	-- Rename is_adsp_sb_needed to adsp_sandbox_needed.
Changes since V2:
	-- Add is_adsp_sb_needed flag instead of is_wpss.

 drivers/remoteproc/qcom_q6v5_adsp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 2f3b9f5..fa2ccac 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -62,6 +62,7 @@ struct adsp_pil_data {
 	const char *sysmon_name;
 	int ssctl_id;
 	bool is_wpss;
+	bool has_iommu;
 	bool auto_boot;
 
 	const char **clk_ids;
@@ -99,6 +100,7 @@ struct qcom_adsp {
 	phys_addr_t mem_reloc;
 	void *mem_region;
 	size_t mem_size;
+	bool has_iommu;
 
 	struct device *proxy_pds[QCOM_Q6V5_RPROC_PROXY_PD_MAX];
 	size_t proxy_pd_count;
@@ -596,12 +598,15 @@ static int adsp_probe(struct platform_device *pdev)
 	}
 
 	rproc->auto_boot = desc->auto_boot;
+	rproc->has_iommu = desc->has_iommu;
 	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
 
 	adsp = (struct qcom_adsp *)rproc->priv;
 	adsp->dev = &pdev->dev;
 	adsp->rproc = rproc;
 	adsp->info_name = desc->sysmon_name;
+	adsp->has_iommu = desc->has_iommu;
+
 	platform_set_drvdata(pdev, adsp);
 
 	if (desc->is_wpss)
-- 
2.7.4

Re: [PATCH v6 2/8] remoteproc: qcom: Add flag in adsp private data structure

[Sibi Sankar]

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Add flag in qcom_adsp private data structure and initialize
> it to distinguish ADSP and WPSS modules for using iommu selectively.

There are other flags available to distinguish between ADSP and WPSS
like 'is_wpss'. So you probably want to tweak your commit message to
just say if it has a iommu in front of it or not and skip referencing
WPSS.

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

> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
> Changes since V5:
> 	-- Rename adsp_sandbox_needed to has_iommu.
> Changes since V3:
> 	-- Rename is_adsp_sb_needed to adsp_sandbox_needed.
> Changes since V2:
> 	-- Add is_adsp_sb_needed flag instead of is_wpss.
> 
>   drivers/remoteproc/qcom_q6v5_adsp.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 2f3b9f5..fa2ccac 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -62,6 +62,7 @@ struct adsp_pil_data {
>   	const char *sysmon_name;
>   	int ssctl_id;
>   	bool is_wpss;
> +	bool has_iommu;
>   	bool auto_boot;
>   
>   	const char **clk_ids;
> @@ -99,6 +100,7 @@ struct qcom_adsp {
>   	phys_addr_t mem_reloc;
>   	void *mem_region;
>   	size_t mem_size;
> +	bool has_iommu;
>   
>   	struct device *proxy_pds[QCOM_Q6V5_RPROC_PROXY_PD_MAX];
>   	size_t proxy_pd_count;
> @@ -596,12 +598,15 @@ static int adsp_probe(struct platform_device *pdev)
>   	}
>   
>   	rproc->auto_boot = desc->auto_boot;
> +	rproc->has_iommu = desc->has_iommu;
>   	rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
>   
>   	adsp = (struct qcom_adsp *)rproc->priv;
>   	adsp->dev = &pdev->dev;
>   	adsp->rproc = rproc;
>   	adsp->info_name = desc->sysmon_name;
> +	adsp->has_iommu = desc->has_iommu;
> +
>   	platform_set_drvdata(pdev, adsp);
>   
>   	if (desc->is_wpss)
> 

Re: [PATCH v6 2/8] remoteproc: qcom: Add flag in adsp private data structure

[Srinivasa Rao Mandadapu]

On 9/14/2022 2:36 PM, Sibi Sankar wrote:
Thanks for Your time Sibi Sankar!!!
> On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
>> Add flag in qcom_adsp private data structure and initialize
>> it to distinguish ADSP and WPSS modules for using iommu selectively.
>
> There are other flags available to distinguish between ADSP and WPSS
> like 'is_wpss'. So you probably want to tweak your commit message to
> just say if it has a iommu in front of it or not and skip referencing
> WPSS.
Okay. Will update commit message.
>
> Reviewed-by: Sibi Sankar <quic_sibis@quicinc.com>
>
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>> Changes since V5:
>>     -- Rename adsp_sandbox_needed to has_iommu.
>> Changes since V3:
>>     -- Rename is_adsp_sb_needed to adsp_sandbox_needed.
>> Changes since V2:
>>     -- Add is_adsp_sb_needed flag instead of is_wpss.
>>
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c 
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index 2f3b9f5..fa2ccac 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -62,6 +62,7 @@ struct adsp_pil_data {
>>       const char *sysmon_name;
>>       int ssctl_id;
>>       bool is_wpss;
>> +    bool has_iommu;
>>       bool auto_boot;
>>         const char **clk_ids;
>> @@ -99,6 +100,7 @@ struct qcom_adsp {
>>       phys_addr_t mem_reloc;
>>       void *mem_region;
>>       size_t mem_size;
>> +    bool has_iommu;
>>         struct device *proxy_pds[QCOM_Q6V5_RPROC_PROXY_PD_MAX];
>>       size_t proxy_pd_count;
>> @@ -596,12 +598,15 @@ static int adsp_probe(struct platform_device 
>> *pdev)
>>       }
>>         rproc->auto_boot = desc->auto_boot;
>> +    rproc->has_iommu = desc->has_iommu;
>>       rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
>>         adsp = (struct qcom_adsp *)rproc->priv;
>>       adsp->dev = &pdev->dev;
>>       adsp->rproc = rproc;
>>       adsp->info_name = desc->sysmon_name;
>> +    adsp->has_iommu = desc->has_iommu;
>> +
>>       platform_set_drvdata(pdev, adsp);
>>         if (desc->is_wpss)
>>

[PATCH v6 3/8] remoteproc: qcom: Add compatible name for SC7280 ADSP

[Srinivasa Rao Mandadapu]

Update adsp pil data and compatible name for loading ADSP
binary on SC7280 based platforms.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
Changes since V5:
	-- Rename adsp_sandbox_needed to has_iommu.
	-- Change adsp binary extention name.
Changes since V3:
	-- Rename is_adsp_sb_needed to adsp_sandbox_needed.
	-- Update sc7280 compatible name entry in sorted order.
Changes since V2:
	-- Initialize is_adsp_sb_needed flag.
	-- Remove empty proxy pds array.

 drivers/remoteproc/qcom_q6v5_adsp.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index fa2ccac..02d17b4 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -702,6 +702,21 @@ static const struct adsp_pil_data adsp_resource_init = {
 	},
 };
 
+static const struct adsp_pil_data adsp_sc7280_resource_init = {
+	.crash_reason_smem = 423,
+	.firmware_name = "adsp.pbn",
+	.load_state = "adsp",
+	.ssr_name = "lpass",
+	.sysmon_name = "adsp",
+	.ssctl_id = 0x14,
+	.has_iommu = true,
+	.auto_boot = true,
+	.clk_ids = (const char*[]) {
+		"gcc_cfg_noc_lpass", NULL
+	},
+	.num_clks = 1,
+};
+
 static const struct adsp_pil_data cdsp_resource_init = {
 	.crash_reason_smem = 601,
 	.firmware_name = "cdsp.mdt",
@@ -740,6 +755,7 @@ static const struct adsp_pil_data wpss_resource_init = {
 
 static const struct of_device_id adsp_of_match[] = {
 	{ .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init },
+	{ .compatible = "qcom,sc7280-adsp-pil", .data = &adsp_sc7280_resource_init },
 	{ .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init },
 	{ .compatible = "qcom,sdm845-adsp-pil", .data = &adsp_resource_init },
 	{ },
-- 
2.7.4

Re: [PATCH v6 3/8] remoteproc: qcom: Add compatible name for SC7280 ADSP

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:37)
> Update adsp pil data and compatible name for loading ADSP
> binary on SC7280 based platforms.
>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v6 3/8] remoteproc: qcom: Add compatible name for SC7280 ADSP

[Sibi Sankar]

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Update adsp pil data and compatible name for loading ADSP
> binary on SC7280 based platforms.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
> Changes since V5:
> 	-- Rename adsp_sandbox_needed to has_iommu.
> 	-- Change adsp binary extention name.
> Changes since V3:
> 	-- Rename is_adsp_sb_needed to adsp_sandbox_needed.
> 	-- Update sc7280 compatible name entry in sorted order.
> Changes since V2:
> 	-- Initialize is_adsp_sb_needed flag.
> 	-- Remove empty proxy pds array.
> 
>   drivers/remoteproc/qcom_q6v5_adsp.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index fa2ccac..02d17b4 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -702,6 +702,21 @@ static const struct adsp_pil_data adsp_resource_init = {
>   	},
>   };
>   
> +static const struct adsp_pil_data adsp_sc7280_resource_init = {
> +	.crash_reason_smem = 423,
> +	.firmware_name = "adsp.pbn",
> +	.load_state = "adsp",

given that you mention load_state info please make sure you
mention qcom,qmp as a required property in the bindings.

> +	.ssr_name = "lpass",
> +	.sysmon_name = "adsp",
> +	.ssctl_id = 0x14,
> +	.has_iommu = true,
> +	.auto_boot = true,
> +	.clk_ids = (const char*[]) {
> +		"gcc_cfg_noc_lpass", NULL
> +	},
> +	.num_clks = 1,

bindings seem to mention 6 other required clocks any reason why
they were skipped?

AFAIK you'll also need lmx so you'll have to mention proxy_pd_names
as well.

> +};
> +
>   static const struct adsp_pil_data cdsp_resource_init = {
>   	.crash_reason_smem = 601,
>   	.firmware_name = "cdsp.mdt",
> @@ -740,6 +755,7 @@ static const struct adsp_pil_data wpss_resource_init = {
>   
>   static const struct of_device_id adsp_of_match[] = {
>   	{ .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init },
> +	{ .compatible = "qcom,sc7280-adsp-pil", .data = &adsp_sc7280_resource_init },
>   	{ .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init },
>   	{ .compatible = "qcom,sdm845-adsp-pil", .data = &adsp_resource_init },
>   	{ },
> 

Re: [PATCH v6 3/8] remoteproc: qcom: Add compatible name for SC7280 ADSP

[Srinivasa Rao Mandadapu]

On 9/14/2022 2:52 PM, Sibi Sankar wrote:
Thanks for Your time Sibi Sankar!!!
>
> On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
>> Update adsp pil data and compatible name for loading ADSP
>> binary on SC7280 based platforms.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>> Changes since V5:
>>     -- Rename adsp_sandbox_needed to has_iommu.
>>     -- Change adsp binary extention name.
>> Changes since V3:
>>     -- Rename is_adsp_sb_needed to adsp_sandbox_needed.
>>     -- Update sc7280 compatible name entry in sorted order.
>> Changes since V2:
>>     -- Initialize is_adsp_sb_needed flag.
>>     -- Remove empty proxy pds array.
>>
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c 
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index fa2ccac..02d17b4 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -702,6 +702,21 @@ static const struct adsp_pil_data 
>> adsp_resource_init = {
>>       },
>>   };
>>   +static const struct adsp_pil_data adsp_sc7280_resource_init = {
>> +    .crash_reason_smem = 423,
>> +    .firmware_name = "adsp.pbn",
>> +    .load_state = "adsp",
>
> given that you mention load_state info please make sure you
> mention qcom,qmp as a required property in the bindings.
Okay. Will add it in the bindings.
>
>> +    .ssr_name = "lpass",
>> +    .sysmon_name = "adsp",
>> +    .ssctl_id = 0x14,
>> +    .has_iommu = true,
>> +    .auto_boot = true,
>> +    .clk_ids = (const char*[]) {
>> +        "gcc_cfg_noc_lpass", NULL
>> +    },
>> +    .num_clks = 1,
>
> bindings seem to mention 6 other required clocks any reason why
> they were skipped?
Actually all other clocks are being enabled locally, without using clock 
framework, as the memory space conflict with other module occurred.
>
> AFAIK you'll also need lmx so you'll have to mention proxy_pd_names
> as well.
So far we didn't see any issue without LMX PD.
>
>> +};
>> +
>>   static const struct adsp_pil_data cdsp_resource_init = {
>>       .crash_reason_smem = 601,
>>       .firmware_name = "cdsp.mdt",
>> @@ -740,6 +755,7 @@ static const struct adsp_pil_data 
>> wpss_resource_init = {
>>     static const struct of_device_id adsp_of_match[] = {
>>       { .compatible = "qcom,qcs404-cdsp-pil", .data = 
>> &cdsp_resource_init },
>> +    { .compatible = "qcom,sc7280-adsp-pil", .data = 
>> &adsp_sc7280_resource_init },
>>       { .compatible = "qcom,sc7280-wpss-pil", .data = 
>> &wpss_resource_init },
>>       { .compatible = "qcom,sdm845-adsp-pil", .data = 
>> &adsp_resource_init },
>>       { },
>>

[PATCH v6 4/8] remoteproc: qcom: Update rproc parse firmware callback

[Srinivasa Rao Mandadapu]

Change parse_fw callback in rproc ops from qcom_register_dump_segments
to rproc_elf_load_rsc_table, as section header to be parsed for memory
sandboxing required platforms.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
 drivers/remoteproc/qcom_q6v5_adsp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 02d17b4..207270d4 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -447,7 +447,7 @@ static unsigned long adsp_panic(struct rproc *rproc)
 	return qcom_q6v5_panic(&adsp->q6v5);
 }
 
-static const struct rproc_ops adsp_ops = {
+static struct rproc_ops adsp_ops = {
 	.start = adsp_start,
 	.stop = adsp_stop,
 	.da_to_va = adsp_da_to_va,
@@ -590,6 +590,9 @@ static int adsp_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	if (desc->has_iommu)
+		adsp_ops.parse_fw = rproc_elf_load_rsc_table;
+
 	rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,
 			    firmware_name, sizeof(*adsp));
 	if (!rproc) {
-- 
2.7.4

Re: [PATCH v6 4/8] remoteproc: qcom: Update rproc parse firmware callback

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:38)
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 02d17b4..207270d4 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -447,7 +447,7 @@ static unsigned long adsp_panic(struct rproc *rproc)
>         return qcom_q6v5_panic(&adsp->q6v5);
>  }
>
> -static const struct rproc_ops adsp_ops = {
> +static struct rproc_ops adsp_ops = {

This is sad.

>         .start = adsp_start,
>         .stop = adsp_stop,
>         .da_to_va = adsp_da_to_va,
> @@ -590,6 +590,9 @@ static int adsp_probe(struct platform_device *pdev)
>                 return ret;
>         }
>
> +       if (desc->has_iommu)
> +               adsp_ops.parse_fw = rproc_elf_load_rsc_table;
> +

Why not have two different set of ops so that the function pointer table
can't be hijacked? One for the parse_fw callback? Or simply return from
rproc_elf_load_rsc_table() when has_iommu is false?

>         rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,

Re: [PATCH v6 4/8] remoteproc: qcom: Update rproc parse firmware callback

[Srinivasa Rao Mandadapu]

On 9/12/2022 4:25 AM, Stephen Boyd wrote:
Thanks for your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:38)
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index 02d17b4..207270d4 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -447,7 +447,7 @@ static unsigned long adsp_panic(struct rproc *rproc)
>>          return qcom_q6v5_panic(&adsp->q6v5);
>>   }
>>
>> -static const struct rproc_ops adsp_ops = {
>> +static struct rproc_ops adsp_ops = {
> This is sad.
>
>>          .start = adsp_start,
>>          .stop = adsp_stop,
>>          .da_to_va = adsp_da_to_va,
>> @@ -590,6 +590,9 @@ static int adsp_probe(struct platform_device *pdev)
>>                  return ret;
>>          }
>>
>> +       if (desc->has_iommu)
>> +               adsp_ops.parse_fw = rproc_elf_load_rsc_table;
>> +
> Why not have two different set of ops so that the function pointer table
> can't be hijacked? One for the parse_fw callback? Or simply return from
> rproc_elf_load_rsc_table() when has_iommu is false?
Okay. Will change accordingly.
>
>>          rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,

Re: [PATCH v6 4/8] remoteproc: qcom: Update rproc parse firmware callback

[Sibi Sankar]

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Change parse_fw callback in rproc ops from qcom_register_dump_segments
> to rproc_elf_load_rsc_table, as section header to be parsed for memory
> sandboxing required platforms.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
>   drivers/remoteproc/qcom_q6v5_adsp.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 02d17b4..207270d4 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -447,7 +447,7 @@ static unsigned long adsp_panic(struct rproc *rproc)
>   	return qcom_q6v5_panic(&adsp->q6v5);
>   }
>   
> -static const struct rproc_ops adsp_ops = {
> +static struct rproc_ops adsp_ops = {
>   	.start = adsp_start,
>   	.stop = adsp_stop,
>   	.da_to_va = adsp_da_to_va,
> @@ -590,6 +590,9 @@ static int adsp_probe(struct platform_device *pdev)
>   		return ret;
>   	}
>   
> +	if (desc->has_iommu)
> +		adsp_ops.parse_fw = rproc_elf_load_rsc_table;
> +

The parse_fw would still need to perform the register_dump_segments
in addition to elf_load_rsc_table, otherwise you'll lose coredump
functionality for ADSP on SC7280. You could perhaps just follow
qcom_q6v5_mss parse_fw i.e. have a static func internal to adsp
doing both and have it assigned to both wpss/adsp with the
pre-existing has_iommu flag to differentiate between the two. With
this you wouldn't need to remove the const in adsp_ops as well.

>   	rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,
>   			    firmware_name, sizeof(*adsp));
>   	if (!rproc) {
> 

Re: [PATCH v6 4/8] remoteproc: qcom: Update rproc parse firmware callback

[Srinivasa Rao Mandadapu]

On 9/14/2022 3:05 PM, Sibi Sankar wrote:
Thanks for Your time and valuable inputs Sibi Sankar!!!
>
> On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
>> Change parse_fw callback in rproc ops from qcom_register_dump_segments
>> to rproc_elf_load_rsc_table, as section header to be parsed for memory
>> sandboxing required platforms.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c 
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index 02d17b4..207270d4 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -447,7 +447,7 @@ static unsigned long adsp_panic(struct rproc *rproc)
>>       return qcom_q6v5_panic(&adsp->q6v5);
>>   }
>>   -static const struct rproc_ops adsp_ops = {
>> +static struct rproc_ops adsp_ops = {
>>       .start = adsp_start,
>>       .stop = adsp_stop,
>>       .da_to_va = adsp_da_to_va,
>> @@ -590,6 +590,9 @@ static int adsp_probe(struct platform_device *pdev)
>>           return ret;
>>       }
>>   +    if (desc->has_iommu)
>> +        adsp_ops.parse_fw = rproc_elf_load_rsc_table;
>> +
>
> The parse_fw would still need to perform the register_dump_segments
> in addition to elf_load_rsc_table, otherwise you'll lose coredump
> functionality for ADSP on SC7280. You could perhaps just follow
> qcom_q6v5_mss parse_fw i.e. have a static func internal to adsp
> doing both and have it assigned to both wpss/adsp with the
> pre-existing has_iommu flag to differentiate between the two. With
> this you wouldn't need to remove the const in adsp_ops as well.
Okay. Will update accordingly and re spin the patches.
>
>>       rproc = rproc_alloc(&pdev->dev, pdev->name, &adsp_ops,
>>                   firmware_name, sizeof(*adsp));
>>       if (!rproc) {
>>

[PATCH v6 5/8] remoteproc: qcom: Replace hard coded values with macros

[Srinivasa Rao Mandadapu]

Replace hard coded values of QDSP6 boot control reg params
with appropriate macro names.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/remoteproc/qcom_q6v5_adsp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 207270d4..389b2c0 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -54,6 +54,9 @@
 
 #define QCOM_Q6V5_RPROC_PROXY_PD_MAX	3
 
+#define LPASS_BOOT_CORE_START	BIT(0)
+#define LPASS_BOOT_CMD_START	BIT(0)
+
 struct adsp_pil_data {
 	int crash_reason_smem;
 	const char *firmware_name;
@@ -366,10 +369,10 @@ static int adsp_start(struct rproc *rproc)
 	writel(adsp->mem_phys >> 4, adsp->qdsp6ss_base + RST_EVB_REG);
 
 	/* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */
-	writel(0x1, adsp->qdsp6ss_base + CORE_START_REG);
+	writel(LPASS_BOOT_CORE_START, adsp->qdsp6ss_base + CORE_START_REG);
 
 	/* Trigger boot FSM to start QDSP6 */
-	writel(0x1, adsp->qdsp6ss_base + BOOT_CMD_REG);
+	writel(LPASS_BOOT_CMD_START, adsp->qdsp6ss_base + BOOT_CMD_REG);
 
 	/* Wait for core to come out of reset */
 	ret = readl_poll_timeout(adsp->qdsp6ss_base + BOOT_STATUS_REG,
-- 
2.7.4

Re: [PATCH v6 5/8] remoteproc: qcom: Replace hard coded values with macros

[Sibi Sankar]

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Replace hard coded values of QDSP6 boot control reg params
> with appropriate macro names.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>

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

> ---
>   drivers/remoteproc/qcom_q6v5_adsp.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 207270d4..389b2c0 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -54,6 +54,9 @@
>   
>   #define QCOM_Q6V5_RPROC_PROXY_PD_MAX	3
>   
> +#define LPASS_BOOT_CORE_START	BIT(0)
> +#define LPASS_BOOT_CMD_START	BIT(0)
> +
>   struct adsp_pil_data {
>   	int crash_reason_smem;
>   	const char *firmware_name;
> @@ -366,10 +369,10 @@ static int adsp_start(struct rproc *rproc)
>   	writel(adsp->mem_phys >> 4, adsp->qdsp6ss_base + RST_EVB_REG);
>   
>   	/* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */
> -	writel(0x1, adsp->qdsp6ss_base + CORE_START_REG);
> +	writel(LPASS_BOOT_CORE_START, adsp->qdsp6ss_base + CORE_START_REG);
>   
>   	/* Trigger boot FSM to start QDSP6 */
> -	writel(0x1, adsp->qdsp6ss_base + BOOT_CMD_REG);
> +	writel(LPASS_BOOT_CMD_START, adsp->qdsp6ss_base + BOOT_CMD_REG);
>   
>   	/* Wait for core to come out of reset */
>   	ret = readl_poll_timeout(adsp->qdsp6ss_base + BOOT_STATUS_REG,
> 

[PATCH v6 6/8] remoteproc: qcom: Add efuse evb selection control

[Srinivasa Rao Mandadapu]

Add efuse evb selection control and enable it for starting ADSP.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
Changes since V5:
	-- Split devm_platform_ioremap_resource_byname function.

 drivers/remoteproc/qcom_q6v5_adsp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 389b2c0..ccb5592 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -56,6 +56,7 @@
 
 #define LPASS_BOOT_CORE_START	BIT(0)
 #define LPASS_BOOT_CMD_START	BIT(0)
+#define LPASS_EFUSE_Q6SS_EVB_SEL 0x0
 
 struct adsp_pil_data {
 	int crash_reason_smem;
@@ -86,6 +87,7 @@ struct qcom_adsp {
 	struct clk_bulk_data *clks;
 
 	void __iomem *qdsp6ss_base;
+	void __iomem *lpass_efuse;
 
 	struct reset_control *pdc_sync_reset;
 	struct reset_control *restart;
@@ -368,6 +370,9 @@ static int adsp_start(struct rproc *rproc)
 	/* Program boot address */
 	writel(adsp->mem_phys >> 4, adsp->qdsp6ss_base + RST_EVB_REG);
 
+	if (adsp->lpass_efuse)
+		writel(LPASS_EFUSE_Q6SS_EVB_SEL, adsp->lpass_efuse);
+
 	/* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */
 	writel(LPASS_BOOT_CORE_START, adsp->qdsp6ss_base + CORE_START_REG);
 
@@ -513,6 +518,7 @@ static int adsp_init_reset(struct qcom_adsp *adsp)
 static int adsp_init_mmio(struct qcom_adsp *adsp,
 				struct platform_device *pdev)
 {
+	struct resource *efuse_region;
 	struct device_node *syscon;
 	int ret;
 
@@ -522,6 +528,17 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
 		return PTR_ERR(adsp->qdsp6ss_base);
 	}
 
+	efuse_region = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!efuse_region) {
+		adsp->lpass_efuse = NULL;
+		dev_dbg(adsp->dev, "failed to get efuse memory region\n");
+	} else {
+		adsp->lpass_efuse = devm_ioremap_resource(&pdev->dev, efuse_region);
+		if (IS_ERR(adsp->lpass_efuse)) {
+			dev_err(adsp->dev, "failed to map efuse registers\n");
+			return PTR_ERR(adsp->lpass_efuse);
+		}
+	}
 	syscon = of_parse_phandle(pdev->dev.of_node, "qcom,halt-regs", 0);
 	if (!syscon) {
 		dev_err(&pdev->dev, "failed to parse qcom,halt-regs\n");
-- 
2.7.4

Re: [PATCH v6 6/8] remoteproc: qcom: Add efuse evb selection control

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:40)
> Add efuse evb selection control and enable it for starting ADSP.
>
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---

Reviewed-by: Stephen Boyd <swboyd@chromium.org>

Re: [PATCH v6 6/8] remoteproc: qcom: Add efuse evb selection control

[Sibi Sankar]

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Add efuse evb selection control and enable it for starting ADSP.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>

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

> ---
> Changes since V5:
> 	-- Split devm_platform_ioremap_resource_byname function.
> 
>   drivers/remoteproc/qcom_q6v5_adsp.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 389b2c0..ccb5592 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -56,6 +56,7 @@
>   
>   #define LPASS_BOOT_CORE_START	BIT(0)
>   #define LPASS_BOOT_CMD_START	BIT(0)
> +#define LPASS_EFUSE_Q6SS_EVB_SEL 0x0
>   
>   struct adsp_pil_data {
>   	int crash_reason_smem;
> @@ -86,6 +87,7 @@ struct qcom_adsp {
>   	struct clk_bulk_data *clks;
>   
>   	void __iomem *qdsp6ss_base;
> +	void __iomem *lpass_efuse;
>   
>   	struct reset_control *pdc_sync_reset;
>   	struct reset_control *restart;
> @@ -368,6 +370,9 @@ static int adsp_start(struct rproc *rproc)
>   	/* Program boot address */
>   	writel(adsp->mem_phys >> 4, adsp->qdsp6ss_base + RST_EVB_REG);
>   
> +	if (adsp->lpass_efuse)
> +		writel(LPASS_EFUSE_Q6SS_EVB_SEL, adsp->lpass_efuse);
> +
>   	/* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */
>   	writel(LPASS_BOOT_CORE_START, adsp->qdsp6ss_base + CORE_START_REG);
>   
> @@ -513,6 +518,7 @@ static int adsp_init_reset(struct qcom_adsp *adsp)
>   static int adsp_init_mmio(struct qcom_adsp *adsp,
>   				struct platform_device *pdev)
>   {
> +	struct resource *efuse_region;
>   	struct device_node *syscon;
>   	int ret;
>   
> @@ -522,6 +528,17 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
>   		return PTR_ERR(adsp->qdsp6ss_base);
>   	}
>   
> +	efuse_region = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!efuse_region) {
> +		adsp->lpass_efuse = NULL;
> +		dev_dbg(adsp->dev, "failed to get efuse memory region\n");
> +	} else {
> +		adsp->lpass_efuse = devm_ioremap_resource(&pdev->dev, efuse_region);
> +		if (IS_ERR(adsp->lpass_efuse)) {
> +			dev_err(adsp->dev, "failed to map efuse registers\n");
> +			return PTR_ERR(adsp->lpass_efuse);
> +		}
> +	}
>   	syscon = of_parse_phandle(pdev->dev.of_node, "qcom,halt-regs", 0);
>   	if (!syscon) {
>   		dev_err(&pdev->dev, "failed to parse qcom,halt-regs\n");
> 

[PATCH v6 7/8] remoteproc: qcom: Add support for memory sandbox

[Srinivasa Rao Mandadapu]

Update pil driver with SMMU mapping for allowing authorised
memory access to ADSP firmware, by carveout reserved adsp memory
region from device tree file.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
---
Changes since V5:
	-- Remove adsp_rproc_unmap_smmu, adsp_of_unmap_smmu, adsp_of_map_smmu and 
	   adsp_rproc_map_smmu functions.
	-- Remove find_loaded_rsc_table call back initialization.
	-- Rename adsp_sandbox_needed to has_iommu.
Changes since V4:
	-- Split the code and add appropriate APIs for resource allocation and free.
	-- Update adsp_unmap_smmu with missing free ops call.
	-- Update normalizing length value in adsp_of_unmap_smmu.
Changes since V3:
	-- Rename is_adsp_sb_needed to adsp_sandbox_needed.
	-- Add smmu unmapping in error case and in adsp stop.
Changes since V2:
	-- Replace platform_bus_type with adsp->dev->bus.
	-- Use API of_parse_phandle_with_args() instead of of_parse_phandle_with_fixed_args().
	-- Replace adsp->is_wpss with adsp->is_adsp.
	-- Update error handling in adsp_start().

 drivers/remoteproc/qcom_q6v5_adsp.c | 55 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index ccb5592..e55d593 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -9,6 +9,7 @@
 #include <linux/firmware.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/iommu.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
@@ -48,6 +49,8 @@
 #define LPASS_PWR_ON_REG		0x10
 #define LPASS_HALTREQ_REG		0x0
 
+#define SID_MASK_DEFAULT        0xF
+
 #define QDSP6SS_XO_CBCR		0x38
 #define QDSP6SS_CORE_CBCR	0x20
 #define QDSP6SS_SLEEP_CBCR	0x3c
@@ -333,6 +336,42 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
 	return 0;
 }
 
+static void adsp_unmap_smmu(struct rproc *rproc)
+{
+	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
+
+	iommu_unmap(rproc->domain, adsp->mem_phys, adsp->mem_size);
+}
+
+static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc)
+{
+	struct of_phandle_args args;
+	long long sid;
+	unsigned long iova;
+	int ret;
+
+	if (!rproc->domain)
+		return -EINVAL;
+
+	ret = of_parse_phandle_with_args(adsp->dev->of_node, "iommus", "#iommu-cells", 0, &args);
+	if (ret < 0)
+		return ret;
+
+	sid = args.args[0] & SID_MASK_DEFAULT;
+
+	/* Add SID configuration for ADSP Firmware to SMMU */
+	iova =  adsp->mem_phys | (sid << 32);
+
+	ret = iommu_map(rproc->domain, iova, adsp->mem_phys,
+			adsp->mem_size,	IOMMU_READ | IOMMU_WRITE);
+	if (ret) {
+		dev_err(adsp->dev, "Unable to map ADSP Physical Memory\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 static int adsp_start(struct rproc *rproc)
 {
 	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
@@ -343,9 +382,17 @@ static int adsp_start(struct rproc *rproc)
 	if (ret)
 		return ret;
 
+	if (adsp->has_iommu) {
+		ret = adsp_map_smmu(adsp, rproc);
+		if (ret) {
+			dev_err(adsp->dev, "ADSP smmu mapping failed\n");
+			goto disable_irqs;
+		}
+	}
+
 	ret = clk_prepare_enable(adsp->xo);
 	if (ret)
-		goto disable_irqs;
+		goto adsp_smmu_unmap;
 
 	ret = qcom_rproc_pds_enable(adsp, adsp->proxy_pds,
 				    adsp->proxy_pd_count);
@@ -401,6 +448,9 @@ static int adsp_start(struct rproc *rproc)
 	qcom_rproc_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
 disable_xo_clk:
 	clk_disable_unprepare(adsp->xo);
+adsp_smmu_unmap:
+	if (adsp->has_iommu)
+		adsp_unmap_smmu(rproc);
 disable_irqs:
 	qcom_q6v5_unprepare(&adsp->q6v5);
 
@@ -429,6 +479,9 @@ static int adsp_stop(struct rproc *rproc)
 	if (ret)
 		dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
 
+	if (adsp->has_iommu)
+		adsp_unmap_smmu(rproc);
+
 	handover = qcom_q6v5_unprepare(&adsp->q6v5);
 	if (handover)
 		qcom_adsp_pil_handover(&adsp->q6v5);
-- 
2.7.4

Re: [PATCH v6 7/8] remoteproc: qcom: Add support for memory sandbox

[Stephen Boyd]

Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:41)
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index ccb5592..e55d593 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -9,6 +9,7 @@
>  #include <linux/firmware.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> +#include <linux/iommu.h>
>  #include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
> @@ -48,6 +49,8 @@
>  #define LPASS_PWR_ON_REG               0x10
>  #define LPASS_HALTREQ_REG              0x0
>
> +#define SID_MASK_DEFAULT        0xF
> +
>  #define QDSP6SS_XO_CBCR                0x38
>  #define QDSP6SS_CORE_CBCR      0x20
>  #define QDSP6SS_SLEEP_CBCR     0x3c
> @@ -333,6 +336,42 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
>         return 0;
>  }
>
> +static void adsp_unmap_smmu(struct rproc *rproc)
> +{
> +       struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;

Drop the cast, it's unnecessary.

> +
> +       iommu_unmap(rproc->domain, adsp->mem_phys, adsp->mem_size);
> +}
> +
[..]
> @@ -343,9 +382,17 @@ static int adsp_start(struct rproc *rproc)
>         if (ret)
>                 return ret;
>
> +       if (adsp->has_iommu) {
> +               ret = adsp_map_smmu(adsp, rproc);
> +               if (ret) {
> +                       dev_err(adsp->dev, "ADSP smmu mapping failed\n");
> +                       goto disable_irqs;
> +               }
> +       }
> +
>         ret = clk_prepare_enable(adsp->xo);
>         if (ret)
> -               goto disable_irqs;
> +               goto adsp_smmu_unmap;
>
>         ret = qcom_rproc_pds_enable(adsp, adsp->proxy_pds,
>                                     adsp->proxy_pd_count);
> @@ -401,6 +448,9 @@ static int adsp_start(struct rproc *rproc)
>         qcom_rproc_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
>  disable_xo_clk:
>         clk_disable_unprepare(adsp->xo);
> +adsp_smmu_unmap:
> +       if (adsp->has_iommu)
> +               adsp_unmap_smmu(rproc);

Why not pass adsp directly to adsp_unmap_smmu()? And even better would
be to make it a no-op when adsp->has_iommu is false, so that the code
reads straight-line otherwise.

>  disable_irqs:
>         qcom_q6v5_unprepare(&adsp->q6v5);
>
> @@ -429,6 +479,9 @@ static int adsp_stop(struct rproc *rproc)
>         if (ret)
>                 dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
>
> +       if (adsp->has_iommu)
> +               adsp_unmap_smmu(rproc);
> +
>         handover = qcom_q6v5_unprepare(&adsp->q6v5);
>         if (handover)
>                 qcom_adsp_pil_handover(&adsp->q6v5);
> --
> 2.7.4
>

Re: [PATCH v6 7/8] remoteproc: qcom: Add support for memory sandbox

[Srinivasa Rao Mandadapu]

On 9/12/2022 4:31 AM, Stephen Boyd wrote:
Thanks for Your time Stephen!!!
> Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:41)
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index ccb5592..e55d593 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/firmware.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/io.h>
>> +#include <linux/iommu.h>
>>   #include <linux/iopoll.h>
>>   #include <linux/kernel.h>
>>   #include <linux/mfd/syscon.h>
>> @@ -48,6 +49,8 @@
>>   #define LPASS_PWR_ON_REG               0x10
>>   #define LPASS_HALTREQ_REG              0x0
>>
>> +#define SID_MASK_DEFAULT        0xF
>> +
>>   #define QDSP6SS_XO_CBCR                0x38
>>   #define QDSP6SS_CORE_CBCR      0x20
>>   #define QDSP6SS_SLEEP_CBCR     0x3c
>> @@ -333,6 +336,42 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
>>          return 0;
>>   }
>>
>> +static void adsp_unmap_smmu(struct rproc *rproc)
>> +{
>> +       struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> Drop the cast, it's unnecessary.
Okay. Will drop casting.
>
>> +
>> +       iommu_unmap(rproc->domain, adsp->mem_phys, adsp->mem_size);
>> +}
>> +
> [..]
>> @@ -343,9 +382,17 @@ static int adsp_start(struct rproc *rproc)
>>          if (ret)
>>                  return ret;
>>
>> +       if (adsp->has_iommu) {
>> +               ret = adsp_map_smmu(adsp, rproc);
>> +               if (ret) {
>> +                       dev_err(adsp->dev, "ADSP smmu mapping failed\n");
>> +                       goto disable_irqs;
>> +               }
>> +       }
>> +
>>          ret = clk_prepare_enable(adsp->xo);
>>          if (ret)
>> -               goto disable_irqs;
>> +               goto adsp_smmu_unmap;
>>
>>          ret = qcom_rproc_pds_enable(adsp, adsp->proxy_pds,
>>                                      adsp->proxy_pd_count);
>> @@ -401,6 +448,9 @@ static int adsp_start(struct rproc *rproc)
>>          qcom_rproc_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
>>   disable_xo_clk:
>>          clk_disable_unprepare(adsp->xo);
>> +adsp_smmu_unmap:
>> +       if (adsp->has_iommu)
>> +               adsp_unmap_smmu(rproc);
> Why not pass adsp directly to adsp_unmap_smmu()? And even better would
> be to make it a no-op when adsp->has_iommu is false, so that the code
> reads straight-line otherwise.
Okay. Will update accordingly.
>
>>   disable_irqs:
>>          qcom_q6v5_unprepare(&adsp->q6v5);
>>
>> @@ -429,6 +479,9 @@ static int adsp_stop(struct rproc *rproc)
>>          if (ret)
>>                  dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
>>
>> +       if (adsp->has_iommu)
>> +               adsp_unmap_smmu(rproc);
>> +
>>          handover = qcom_q6v5_unprepare(&adsp->q6v5);
>>          if (handover)
>>                  qcom_adsp_pil_handover(&adsp->q6v5);
>> --
>> 2.7.4
>>

Re: [PATCH v6 7/8] remoteproc: qcom: Add support for memory sandbox

[Sibi Sankar]

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Update pil driver with SMMU mapping for allowing authorised
> memory access to ADSP firmware, by carveout reserved adsp memory
> region from device tree file.
> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> ---
> Changes since V5:
> 	-- Remove adsp_rproc_unmap_smmu, adsp_of_unmap_smmu, adsp_of_map_smmu and
> 	   adsp_rproc_map_smmu functions.
> 	-- Remove find_loaded_rsc_table call back initialization.
> 	-- Rename adsp_sandbox_needed to has_iommu.
> Changes since V4:
> 	-- Split the code and add appropriate APIs for resource allocation and free.
> 	-- Update adsp_unmap_smmu with missing free ops call.
> 	-- Update normalizing length value in adsp_of_unmap_smmu.
> Changes since V3:
> 	-- Rename is_adsp_sb_needed to adsp_sandbox_needed.
> 	-- Add smmu unmapping in error case and in adsp stop.
> Changes since V2:
> 	-- Replace platform_bus_type with adsp->dev->bus.
> 	-- Use API of_parse_phandle_with_args() instead of of_parse_phandle_with_fixed_args().
> 	-- Replace adsp->is_wpss with adsp->is_adsp.
> 	-- Update error handling in adsp_start().
> 
>   drivers/remoteproc/qcom_q6v5_adsp.c | 55 ++++++++++++++++++++++++++++++++++++-
>   1 file changed, 54 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index ccb5592..e55d593 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -9,6 +9,7 @@
>   #include <linux/firmware.h>
>   #include <linux/interrupt.h>
>   #include <linux/io.h>
> +#include <linux/iommu.h>
>   #include <linux/iopoll.h>
>   #include <linux/kernel.h>
>   #include <linux/mfd/syscon.h>
> @@ -48,6 +49,8 @@
>   #define LPASS_PWR_ON_REG		0x10
>   #define LPASS_HALTREQ_REG		0x0
>   
> +#define SID_MASK_DEFAULT        0xF
> +
>   #define QDSP6SS_XO_CBCR		0x38
>   #define QDSP6SS_CORE_CBCR	0x20
>   #define QDSP6SS_SLEEP_CBCR	0x3c
> @@ -333,6 +336,42 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
>   	return 0;
>   }
>   
> +static void adsp_unmap_smmu(struct rproc *rproc)
> +{
> +	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> +
> +	iommu_unmap(rproc->domain, adsp->mem_phys, adsp->mem_size);
> +}
> +
> +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc)

you could perhaps name the func to adsp_map_carveout/adsp_unmap_carveout


> +{
> +	struct of_phandle_args args;
> +	long long sid;
> +	unsigned long iova;
> +	int ret;
> +
> +	if (!rproc->domain)
> +		return -EINVAL;
> +
> +	ret = of_parse_phandle_with_args(adsp->dev->of_node, "iommus", "#iommu-cells", 0, &args);
> +	if (ret < 0)
> +		return ret;
> +
> +	sid = args.args[0] & SID_MASK_DEFAULT;
> +
> +	/* Add SID configuration for ADSP Firmware to SMMU */
> +	iova =  adsp->mem_phys | (sid << 32);
> +
> +	ret = iommu_map(rproc->domain, iova, adsp->mem_phys,
> +			adsp->mem_size,	IOMMU_READ | IOMMU_WRITE);
> +	if (ret) {
> +		dev_err(adsp->dev, "Unable to map ADSP Physical Memory\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>   static int adsp_start(struct rproc *rproc)
>   {
>   	struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> @@ -343,9 +382,17 @@ static int adsp_start(struct rproc *rproc)
>   	if (ret)
>   		return ret;
>   
> +	if (adsp->has_iommu) {
> +		ret = adsp_map_smmu(adsp, rproc);
> +		if (ret) {
> +			dev_err(adsp->dev, "ADSP smmu mapping failed\n");
> +			goto disable_irqs;
> +		}
> +	}
> +
>   	ret = clk_prepare_enable(adsp->xo);
>   	if (ret)
> -		goto disable_irqs;
> +		goto adsp_smmu_unmap;
>   
>   	ret = qcom_rproc_pds_enable(adsp, adsp->proxy_pds,
>   				    adsp->proxy_pd_count);
> @@ -401,6 +448,9 @@ static int adsp_start(struct rproc *rproc)
>   	qcom_rproc_pds_disable(adsp, adsp->proxy_pds, adsp->proxy_pd_count);
>   disable_xo_clk:
>   	clk_disable_unprepare(adsp->xo);
> +adsp_smmu_unmap:
> +	if (adsp->has_iommu)
> +		adsp_unmap_smmu(rproc);
>   disable_irqs:
>   	qcom_q6v5_unprepare(&adsp->q6v5);
>   
> @@ -429,6 +479,9 @@ static int adsp_stop(struct rproc *rproc)
>   	if (ret)
>   		dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
>   
> +	if (adsp->has_iommu)
> +		adsp_unmap_smmu(rproc);
> +
>   	handover = qcom_q6v5_unprepare(&adsp->q6v5);
>   	if (handover)
>   		qcom_adsp_pil_handover(&adsp->q6v5);
> 

Re: [PATCH v6 7/8] remoteproc: qcom: Add support for memory sandbox

[Srinivasa Rao Mandadapu]

On 9/14/2022 3:40 PM, Sibi Sankar wrote:
Thanks for Your Time Sibi Sankar!!!
>
> On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
>> Update pil driver with SMMU mapping for allowing authorised
>> memory access to ADSP firmware, by carveout reserved adsp memory
>> region from device tree file.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> ---
>> Changes since V5:
>>     -- Remove adsp_rproc_unmap_smmu, adsp_of_unmap_smmu, 
>> adsp_of_map_smmu and
>>        adsp_rproc_map_smmu functions.
>>     -- Remove find_loaded_rsc_table call back initialization.
>>     -- Rename adsp_sandbox_needed to has_iommu.
>> Changes since V4:
>>     -- Split the code and add appropriate APIs for resource 
>> allocation and free.
>>     -- Update adsp_unmap_smmu with missing free ops call.
>>     -- Update normalizing length value in adsp_of_unmap_smmu.
>> Changes since V3:
>>     -- Rename is_adsp_sb_needed to adsp_sandbox_needed.
>>     -- Add smmu unmapping in error case and in adsp stop.
>> Changes since V2:
>>     -- Replace platform_bus_type with adsp->dev->bus.
>>     -- Use API of_parse_phandle_with_args() instead of 
>> of_parse_phandle_with_fixed_args().
>>     -- Replace adsp->is_wpss with adsp->is_adsp.
>>     -- Update error handling in adsp_start().
>>
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 55 
>> ++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 54 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c 
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index ccb5592..e55d593 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -9,6 +9,7 @@
>>   #include <linux/firmware.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/io.h>
>> +#include <linux/iommu.h>
>>   #include <linux/iopoll.h>
>>   #include <linux/kernel.h>
>>   #include <linux/mfd/syscon.h>
>> @@ -48,6 +49,8 @@
>>   #define LPASS_PWR_ON_REG        0x10
>>   #define LPASS_HALTREQ_REG        0x0
>>   +#define SID_MASK_DEFAULT        0xF
>> +
>>   #define QDSP6SS_XO_CBCR        0x38
>>   #define QDSP6SS_CORE_CBCR    0x20
>>   #define QDSP6SS_SLEEP_CBCR    0x3c
>> @@ -333,6 +336,42 @@ static int adsp_load(struct rproc *rproc, const 
>> struct firmware *fw)
>>       return 0;
>>   }
>>   +static void adsp_unmap_smmu(struct rproc *rproc)
>> +{
>> +    struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
>> +
>> +    iommu_unmap(rproc->domain, adsp->mem_phys, adsp->mem_size);
>> +}
>> +
>> +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc)
>
> you could perhaps name the func to adsp_map_carveout/adsp_unmap_carveout
Okay. Will modify accordingly.
>
>
>> +{
>> +    struct of_phandle_args args;
>> +    long long sid;
>> +    unsigned long iova;
>> +    int ret;
>> +
>> +    if (!rproc->domain)
>> +        return -EINVAL;
>> +
>> +    ret = of_parse_phandle_with_args(adsp->dev->of_node, "iommus", 
>> "#iommu-cells", 0, &args);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    sid = args.args[0] & SID_MASK_DEFAULT;
>> +
>> +    /* Add SID configuration for ADSP Firmware to SMMU */
>> +    iova =  adsp->mem_phys | (sid << 32);
>> +
>> +    ret = iommu_map(rproc->domain, iova, adsp->mem_phys,
>> +            adsp->mem_size,    IOMMU_READ | IOMMU_WRITE);
>> +    if (ret) {
>> +        dev_err(adsp->dev, "Unable to map ADSP Physical Memory\n");
>> +        return ret;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>>   static int adsp_start(struct rproc *rproc)
>>   {
>>       struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
>> @@ -343,9 +382,17 @@ static int adsp_start(struct rproc *rproc)
>>       if (ret)
>>           return ret;
>>   +    if (adsp->has_iommu) {
>> +        ret = adsp_map_smmu(adsp, rproc);
>> +        if (ret) {
>> +            dev_err(adsp->dev, "ADSP smmu mapping failed\n");
>> +            goto disable_irqs;
>> +        }
>> +    }
>> +
>>       ret = clk_prepare_enable(adsp->xo);
>>       if (ret)
>> -        goto disable_irqs;
>> +        goto adsp_smmu_unmap;
>>         ret = qcom_rproc_pds_enable(adsp, adsp->proxy_pds,
>>                       adsp->proxy_pd_count);
>> @@ -401,6 +448,9 @@ static int adsp_start(struct rproc *rproc)
>>       qcom_rproc_pds_disable(adsp, adsp->proxy_pds, 
>> adsp->proxy_pd_count);
>>   disable_xo_clk:
>>       clk_disable_unprepare(adsp->xo);
>> +adsp_smmu_unmap:
>> +    if (adsp->has_iommu)
>> +        adsp_unmap_smmu(rproc);
>>   disable_irqs:
>>       qcom_q6v5_unprepare(&adsp->q6v5);
>>   @@ -429,6 +479,9 @@ static int adsp_stop(struct rproc *rproc)
>>       if (ret)
>>           dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
>>   +    if (adsp->has_iommu)
>> +        adsp_unmap_smmu(rproc);
>> +
>>       handover = qcom_q6v5_unprepare(&adsp->q6v5);
>>       if (handover)
>>           qcom_adsp_pil_handover(&adsp->q6v5);
>>

[PATCH v6 8/8] remoteproc: qcom: Update QDSP6 out-of-reset timeout value

[Srinivasa Rao Mandadapu]

Update QDSP6 out-of-reset timeout value to 1 second, as sometimes
ADSP boot failing on SC7280 based platforms with existing value.
Also add few micro seconds sleep after enabling boot core
start register.

Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
---
 drivers/remoteproc/qcom_q6v5_adsp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index e55d593..4414e23 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -34,7 +34,7 @@
 /* time out value */
 #define ACK_TIMEOUT			1000
 #define ACK_TIMEOUT_US			1000000
-#define BOOT_FSM_TIMEOUT		10000
+#define BOOT_FSM_TIMEOUT		1000000
 /* mask values */
 #define EVB_MASK			GENMASK(27, 4)
 /*QDSP6SS register offsets*/
@@ -422,13 +422,14 @@ static int adsp_start(struct rproc *rproc)
 
 	/* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */
 	writel(LPASS_BOOT_CORE_START, adsp->qdsp6ss_base + CORE_START_REG);
+	usleep_range(100, 110);
 
 	/* Trigger boot FSM to start QDSP6 */
 	writel(LPASS_BOOT_CMD_START, adsp->qdsp6ss_base + BOOT_CMD_REG);
 
 	/* Wait for core to come out of reset */
 	ret = readl_poll_timeout(adsp->qdsp6ss_base + BOOT_STATUS_REG,
-			val, (val & BIT(0)) != 0, 10, BOOT_FSM_TIMEOUT);
+			val, (val & BIT(0)) != 0, 100, BOOT_FSM_TIMEOUT);
 	if (ret) {
 		dev_err(adsp->dev, "failed to bootup adsp\n");
 		goto disable_adsp_clks;
-- 
2.7.4

Re: [PATCH v6 8/8] remoteproc: qcom: Update QDSP6 out-of-reset timeout value

[Sibi Sankar]

On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
> Update QDSP6 out-of-reset timeout value to 1 second, as sometimes
> ADSP boot failing on SC7280 based platforms with existing value.
> Also add few micro seconds sleep after enabling boot core
> start register.

Please do check if the timeout that you hit is due to lack of
required clock/bus scaling. If so increasing the timeout would
be just an interim hack and might stop working in the future.

> 
> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> ---
>   drivers/remoteproc/qcom_q6v5_adsp.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index e55d593..4414e23 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -34,7 +34,7 @@
>   /* time out value */
>   #define ACK_TIMEOUT			1000
>   #define ACK_TIMEOUT_US			1000000
> -#define BOOT_FSM_TIMEOUT		10000
> +#define BOOT_FSM_TIMEOUT		1000000
>   /* mask values */
>   #define EVB_MASK			GENMASK(27, 4)
>   /*QDSP6SS register offsets*/
> @@ -422,13 +422,14 @@ static int adsp_start(struct rproc *rproc)
>   
>   	/* De-assert QDSP6 stop core. QDSP6 will execute after out of reset */
>   	writel(LPASS_BOOT_CORE_START, adsp->qdsp6ss_base + CORE_START_REG);
> +	usleep_range(100, 110);
>   
>   	/* Trigger boot FSM to start QDSP6 */
>   	writel(LPASS_BOOT_CMD_START, adsp->qdsp6ss_base + BOOT_CMD_REG);
>   
>   	/* Wait for core to come out of reset */
>   	ret = readl_poll_timeout(adsp->qdsp6ss_base + BOOT_STATUS_REG,
> -			val, (val & BIT(0)) != 0, 10, BOOT_FSM_TIMEOUT);
> +			val, (val & BIT(0)) != 0, 100, BOOT_FSM_TIMEOUT);
>   	if (ret) {
>   		dev_err(adsp->dev, "failed to bootup adsp\n");
>   		goto disable_adsp_clks;
> 

Re: [PATCH v6 8/8] remoteproc: qcom: Update QDSP6 out-of-reset timeout value

[Srinivasa Rao Mandadapu]

On 9/14/2022 3:15 PM, Sibi Sankar wrote:
Thanks for Your time Sibi Sankar!!!
>
> On 9/8/22 6:53 PM, Srinivasa Rao Mandadapu wrote:
>> Update QDSP6 out-of-reset timeout value to 1 second, as sometimes
>> ADSP boot failing on SC7280 based platforms with existing value.
>> Also add few micro seconds sleep after enabling boot core
>> start register.
>
> Please do check if the timeout that you hit is due to lack of
> required clock/bus scaling. If so increasing the timeout would
> be just an interim hack and might stop working in the future.
Okay. Will check and update it as per new findings.
>
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <quic_srivasam@quicinc.com>
>> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c 
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index e55d593..4414e23 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -34,7 +34,7 @@
>>   /* time out value */
>>   #define ACK_TIMEOUT            1000
>>   #define ACK_TIMEOUT_US            1000000
>> -#define BOOT_FSM_TIMEOUT        10000
>> +#define BOOT_FSM_TIMEOUT        1000000
>>   /* mask values */
>>   #define EVB_MASK            GENMASK(27, 4)
>>   /*QDSP6SS register offsets*/
>> @@ -422,13 +422,14 @@ static int adsp_start(struct rproc *rproc)
>>         /* De-assert QDSP6 stop core. QDSP6 will execute after out of 
>> reset */
>>       writel(LPASS_BOOT_CORE_START, adsp->qdsp6ss_base + 
>> CORE_START_REG);
>> +    usleep_range(100, 110);
>>         /* Trigger boot FSM to start QDSP6 */
>>       writel(LPASS_BOOT_CMD_START, adsp->qdsp6ss_base + BOOT_CMD_REG);
>>         /* Wait for core to come out of reset */
>>       ret = readl_poll_timeout(adsp->qdsp6ss_base + BOOT_STATUS_REG,
>> -            val, (val & BIT(0)) != 0, 10, BOOT_FSM_TIMEOUT);
>> +            val, (val & BIT(0)) != 0, 100, BOOT_FSM_TIMEOUT);
>>       if (ret) {
>>           dev_err(adsp->dev, "failed to bootup adsp\n");
>>           goto disable_adsp_clks;
>>