2022-09-08 13:32:00

by Srinivasa Rao Mandadapu

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

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

2022-09-08 13:32:08

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
---
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

2022-09-08 13:32:22

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
---
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

2022-09-08 13:32:39

by Srinivasa Rao Mandadapu

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

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

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
---
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

2022-09-08 13:32:40

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
---
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

2022-09-08 13:47:40

by Srinivasa Rao Mandadapu

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

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

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
---
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

2022-09-08 13:50:49

by Srinivasa Rao Mandadapu

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

Add ADSP PIL loading support for SC7280 SoCs.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
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 <[email protected]>
+
+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

2022-09-08 13:51:28

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
---
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

2022-09-08 14:01:24

by Srinivasa Rao Mandadapu

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

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

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
---
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

2022-09-11 22:55:53

by Stephen Boyd

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

Quoting Srinivasa Rao Mandadapu (2022-09-08 06:23:35)
> Add ADSP PIL loading support for SC7280 SoCs.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2022-09-11 23:02:55

by Stephen Boyd

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

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 <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2022-09-11 23:21:00

by Stephen Boyd

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

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,

2022-09-11 23:21:08

by Stephen Boyd

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

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
>

2022-09-11 23:25:44

by Stephen Boyd

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

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 <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2022-09-14 09:17:22

by Sibi Sankar

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

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 <[email protected]>
> Reviewed-by: Rob Herring <[email protected]>
> ---
> 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 <[email protected]>
> +
> +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";
> +
> + };
>

2022-09-14 09:41:12

by Sibi Sankar

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

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 <[email protected]>

>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> ---
> 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)
>

2022-09-14 09:45:02

by Sibi Sankar

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

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 <[email protected]>
> ---
> 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 },
> { },
>

2022-09-14 09:45:40

by Sibi Sankar

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

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 <[email protected]>
> ---
> 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) {
>

2022-09-14 09:49:44

by Sibi Sankar

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

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 <[email protected]>

Reviewed-by: Sibi Sankar <[email protected]>

> ---
> 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");
>

2022-09-14 10:10:13

by Sibi Sankar

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

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 <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> ---
> 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;
>

2022-09-14 10:14:30

by Sibi Sankar

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

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 <[email protected]>
> ---
> 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);
>

2022-09-14 10:21:45

by Sibi Sankar

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

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 <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>

Reviewed-by: Sibi Sankar <[email protected]>

> ---
> 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,
>

2022-09-19 14:15:47

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
>> Reviewed-by: Rob Herring <[email protected]>
>> ---
>> 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 <[email protected]>
>> +
>> +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";
>> +
>> +    };
>>

2022-09-19 14:18:23

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
>> ---
>>   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) {
>>

2022-09-19 14:22:38

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
>
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> Reviewed-by: Stephen Boyd <[email protected]>
>> ---
>> 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)
>>

2022-09-19 14:22:49

by Srinivasa Rao Mandadapu

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

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,

2022-09-19 14:26:08

by Srinivasa Rao Mandadapu

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

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

2022-09-19 14:27:21

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
>> ---
>> 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 },
>>       { },
>>

2022-09-19 14:28:30

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
>> ---
>> 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);
>>

2022-09-19 14:51:37

by Srinivasa Rao Mandadapu

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

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 <[email protected]>
>> Reviewed-by: Stephen Boyd <[email protected]>
>> ---
>>   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;
>>