2022-08-03 14:25:12

by Srinivasa Rao Mandadapu

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

Update ADSP pil loader driver for SC7280 platforms.

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

.../bindings/remoteproc/qcom,lpass-adsp-pil.yaml | 161 +++++++++++++++++++++
.../bindings/remoteproc/qcom,sdm845-adsp-pil.yaml | 160 --------------------
drivers/remoteproc/qcom_q6v5_adsp.c | 145 ++++++++++++++++++-
3 files changed, 300 insertions(+), 166 deletions(-)
create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
delete mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml

--
2.7.4



2022-08-03 14:25:27

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 2/8] dt-bindings: remoteproc: qcom: adsp: Add compatible name for SC7280

Add compatible name and update max reg items for SC7280 base platforms.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
---
.../devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
index 9f11332..147996f 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
@@ -17,11 +17,12 @@ properties:
compatible:
enum:
- qcom,sdm845-adsp-pil
+ - qcom,sc7280-adsp-pil

reg:
- maxItems: 1
+ maxItems: 2
description:
- The base address and size of the qdsp6ss register
+ The base address and size of the qdsp6ss register and mcc register

interrupts:
items:
--
2.7.4


2022-08-03 14:25:46

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 6/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.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
---
drivers/remoteproc/qcom_q6v5_adsp.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 201cc21..3dbd035 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -104,6 +104,7 @@ struct qcom_adsp {
phys_addr_t mem_reloc;
void *mem_region;
size_t mem_size;
+ bool is_wpss;

struct device *proxy_pds[QCOM_Q6V5_RPROC_PROXY_PD_MAX];
size_t proxy_pd_count;
@@ -615,6 +616,8 @@ static int adsp_probe(struct platform_device *pdev)
adsp->dev = &pdev->dev;
adsp->rproc = rproc;
adsp->info_name = desc->sysmon_name;
+ adsp->is_wpss = desc->is_wpss;
+
platform_set_drvdata(pdev, adsp);

if (desc->is_wpss)
--
2.7.4


2022-08-03 14:26:03

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 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]>
---
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 f81da47..1c0adc9 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*/
@@ -468,13 +468,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_ranage(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-08-03 14:36:41

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 4/8] remoteproc: qcom: Update hard coded values with macros

Update hard coded values with appropriate macro names.

Signed-off-by: Srinivasa Rao Mandadapu <[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 bb4494c..a9fcb5c 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;
@@ -364,10 +367,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-08-03 14:38:19

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 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]>
---
drivers/remoteproc/qcom_q6v5_adsp.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 2f3b9f5..bb4494c 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -697,6 +697,24 @@ 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.mbn",
+ .load_state = "adsp",
+ .ssr_name = "lpass",
+ .sysmon_name = "adsp",
+ .ssctl_id = 0x14,
+ .is_wpss = false,
+ .auto_boot = true,
+ .clk_ids = (const char*[]) {
+ "gcc_cfg_noc_lpass", NULL
+ },
+ .num_clks = 1,
+ .proxy_pd_names = (const char*[]) {
+ NULL
+ },
+};
+
static const struct adsp_pil_data cdsp_resource_init = {
.crash_reason_smem = 601,
.firmware_name = "cdsp.mdt",
@@ -737,6 +755,7 @@ static const struct of_device_id adsp_of_match[] = {
{ .compatible = "qcom,qcs404-cdsp-pil", .data = &cdsp_resource_init },
{ .compatible = "qcom,sc7280-wpss-pil", .data = &wpss_resource_init },
{ .compatible = "qcom,sdm845-adsp-pil", .data = &adsp_resource_init },
+ { .compatible = "qcom,sc7280-adsp-pil", .data = &adsp_sc7280_resource_init },
{ },
};
MODULE_DEVICE_TABLE(of, adsp_of_match);
--
2.7.4


2022-08-03 14:39:37

by Srinivasa Rao Mandadapu

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

Add memory sandbox support for ADSP based platforms secure booting.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
---
drivers/remoteproc/qcom_q6v5_adsp.c | 101 +++++++++++++++++++++++++++++++++++-
1 file changed, 99 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index 3dbd035..f81da47 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
@@ -77,7 +80,7 @@ struct adsp_pil_data {
struct qcom_adsp {
struct device *dev;
struct rproc *rproc;
-
+ struct iommu_domain *iommu_dom;
struct qcom_q6v5 q6v5;

struct clk *xo;
@@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
return 0;
}

+static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc)
+{
+ struct of_phandle_args args;
+ int ret, rc, i;
+ long long sid;
+
+ unsigned long mem_phys;
+ unsigned long iova;
+ const __be32 *prop;
+ int access_level;
+ uint32_t len, flag, mem_size;
+ int offset;
+ struct fw_rsc_hdr *hdr;
+ struct fw_rsc_devmem *rsc_fw;
+
+ rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node, "iommus", 1, 0, &args);
+ if (rc < 0)
+ sid = -1;
+ else
+ sid = args.args[0] & SID_MASK_DEFAULT;
+
+ adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type);
+ if (!adsp->iommu_dom) {
+ dev_err(adsp->dev, "failed to allocate iommu domain\n");
+ return -ENOMEM;
+ }
+
+ ret = iommu_attach_device(adsp->iommu_dom, adsp->dev);
+ if (ret) {
+ dev_err(adsp->dev, "could not attach device ret = %d\n", ret);
+ return -EBUSY;
+ }
+
+ /* Add SID configuration for ADSP Firmware to SMMU */
+ adsp->mem_phys = adsp->mem_phys | (sid << 32);
+
+ ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, 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;
+ }
+
+ prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory", &len);
+ if (prop) {
+ len /= sizeof(__be32);
+ for (i = 0; i < len; i++) {
+ iova = be32_to_cpu(prop[i++]);
+ mem_phys = be32_to_cpu(prop[i++]);
+ mem_size = be32_to_cpu(prop[i++]);
+ access_level = be32_to_cpu(prop[i]);
+
+ if (access_level)
+ flag = IOMMU_READ | IOMMU_WRITE;
+ else
+ flag = IOMMU_READ;
+
+ ret = iommu_map(adsp->iommu_dom, iova, mem_phys, mem_size, flag);
+ if (ret) {
+ dev_err(adsp->dev, "failed to map addr = %p mem_size = %x\n",
+ &(mem_phys), mem_size);
+ return ret;
+ }
+ }
+ } else {
+ if (!rproc->table_ptr)
+ return 0;
+
+ for (i = 0; i < rproc->table_ptr->num; i++) {
+ offset = rproc->table_ptr->offset[i];
+ hdr = (void *)rproc->table_ptr + offset;
+ rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr);
+
+ ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa,
+ rsc_fw->len, rsc_fw->flags);
+ if (ret) {
+ pr_err("%s; unable to map adsp memory address\n", __func__);
+ return ret;
+ }
+ }
+ }
+ return 0;
+}
+
+
static int adsp_start(struct rproc *rproc)
{
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
@@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc)
ret = qcom_q6v5_prepare(&adsp->q6v5);
if (ret)
return ret;
-
+ if (!adsp->is_wpss) {
+ ret = adsp_map_smmu(adsp, rproc);
+ if (ret) {
+ dev_err(adsp->dev, "ADSP smmu mapping failed\n");
+ goto adsp_smmu_unmap;
+ }
+ }
ret = clk_prepare_enable(adsp->xo);
if (ret)
goto disable_irqs;
@@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc)
clk_disable_unprepare(adsp->xo);
disable_irqs:
qcom_q6v5_unprepare(&adsp->q6v5);
+adsp_smmu_unmap:
+ iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size);
+ iommu_domain_free(adsp->iommu_dom);

return ret;
}
--
2.7.4


2022-08-03 14:50:12

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 1/8] dt-bindings: remoteproc: qcom: adsp: Make ADSP pil loader as generic

Rename sdm845 adsp pil bindings to generic name, for using same binings
file for subsequent SoCs.

Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
---
.../bindings/remoteproc/qcom,lpass-adsp-pil.yaml | 160 +++++++++++++++++++++
.../bindings/remoteproc/qcom,sdm845-adsp-pil.yaml | 160 ---------------------
2 files changed, 160 insertions(+), 160 deletions(-)
create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
delete mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
new file mode 100644
index 0000000..9f11332
--- /dev/null
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
@@ -0,0 +1,160 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/qcom,lpass-adsp-pil.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm LPASS ADSP Peripheral Image Loader
+
+maintainers:
+ - Bjorn Andersson <[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,sdm845-adsp-pil
+
+ reg:
+ maxItems: 1
+ description:
+ The base address and size of the qdsp6ss register
+
+ interrupts:
+ items:
+ - description: Watchdog interrupt
+ - description: Fatal interrupt
+ - description: Ready interrupt
+ - description: Handover interrupt
+ - description: Stop acknowledge interrupt
+
+ interrupt-names:
+ items:
+ - const: wdog
+ - const: fatal
+ - const: ready
+ - const: handover
+ - const: stop-ack
+
+ clocks:
+ items:
+ - description: XO clock
+ - description: SWAY 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: sway_cbcr
+ - const: lpass_ahbs_aon_cbcr
+ - const: lpass_ahbm_aon_cbcr
+ - const: qdsp6ss_xo
+ - const: qdsp6ss_sleep
+ - const: qdsp6ss_core
+
+ power-domains:
+ items:
+ - description: CX 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
+ three offsets within syscon for q6, modem and nc 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-sdm845.h>
+ #include <dt-bindings/clock/qcom,lpass-sdm845.h>
+ #include <dt-bindings/power/qcom-rpmpd.h>
+ #include <dt-bindings/reset/qcom,sdm845-pdc.h>
+ #include <dt-bindings/reset/qcom,sdm845-aoss.h>
+ remoteproc@17300000 {
+ compatible = "qcom,sdm845-adsp-pil";
+ reg = <0x17300000 0x40c>;
+
+ interrupts-extended = <&intc GIC_SPI 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>;
+ interrupt-names = "wdog", "fatal", "ready",
+ "handover", "stop-ack";
+
+ clocks = <&rpmhcc RPMH_CXO_CLK>,
+ <&gcc GCC_LPASS_SWAY_CLK>,
+ <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>,
+ <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>,
+ <&lpasscc LPASS_QDSP6SS_XO_CLK>,
+ <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
+ <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
+ clock-names = "xo", "sway_cbcr",
+ "lpass_ahbs_aon_cbcr",
+ "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
+ "qdsp6ss_sleep", "qdsp6ss_core";
+
+ power-domains = <&rpmhpd SDM845_CX>;
+
+ resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
+ <&aoss_reset AOSS_CC_LPASS_RESTART>;
+ reset-names = "pdc_sync", "cc_lpass";
+
+ qcom,halt-regs = <&tcsr_mutex_regs 0x22000>;
+
+ memory-region = <&pil_adsp_mem>;
+
+ qcom,smem-states = <&adsp_smp2p_out 0>;
+ qcom,smem-state-names = "stop";
+ };
diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
deleted file mode 100644
index 1535bbb..0000000
--- a/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
+++ /dev/null
@@ -1,160 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/remoteproc/qcom,sdm845-adsp-pil.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm SDM845 ADSP Peripheral Image Loader
-
-maintainers:
- - Bjorn Andersson <[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,sdm845-adsp-pil
-
- reg:
- maxItems: 1
- description:
- The base address and size of the qdsp6ss register
-
- interrupts:
- items:
- - description: Watchdog interrupt
- - description: Fatal interrupt
- - description: Ready interrupt
- - description: Handover interrupt
- - description: Stop acknowledge interrupt
-
- interrupt-names:
- items:
- - const: wdog
- - const: fatal
- - const: ready
- - const: handover
- - const: stop-ack
-
- clocks:
- items:
- - description: XO clock
- - description: SWAY 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: sway_cbcr
- - const: lpass_ahbs_aon_cbcr
- - const: lpass_ahbm_aon_cbcr
- - const: qdsp6ss_xo
- - const: qdsp6ss_sleep
- - const: qdsp6ss_core
-
- power-domains:
- items:
- - description: CX 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
- three offsets within syscon for q6, modem and nc 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-sdm845.h>
- #include <dt-bindings/clock/qcom,lpass-sdm845.h>
- #include <dt-bindings/power/qcom-rpmpd.h>
- #include <dt-bindings/reset/qcom,sdm845-pdc.h>
- #include <dt-bindings/reset/qcom,sdm845-aoss.h>
- remoteproc@17300000 {
- compatible = "qcom,sdm845-adsp-pil";
- reg = <0x17300000 0x40c>;
-
- interrupts-extended = <&intc GIC_SPI 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>;
- interrupt-names = "wdog", "fatal", "ready",
- "handover", "stop-ack";
-
- clocks = <&rpmhcc RPMH_CXO_CLK>,
- <&gcc GCC_LPASS_SWAY_CLK>,
- <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>,
- <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>,
- <&lpasscc LPASS_QDSP6SS_XO_CLK>,
- <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
- <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
- clock-names = "xo", "sway_cbcr",
- "lpass_ahbs_aon_cbcr",
- "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
- "qdsp6ss_sleep", "qdsp6ss_core";
-
- power-domains = <&rpmhpd SDM845_CX>;
-
- resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
- <&aoss_reset AOSS_CC_LPASS_RESTART>;
- reset-names = "pdc_sync", "cc_lpass";
-
- qcom,halt-regs = <&tcsr_mutex_regs 0x22000>;
-
- memory-region = <&pil_adsp_mem>;
-
- qcom,smem-states = <&adsp_smp2p_out 0>;
- qcom,smem-state-names = "stop";
- };
--
2.7.4


2022-08-03 14:50:30

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: [PATCH 5/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]>
---
drivers/remoteproc/qcom_q6v5_adsp.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index a9fcb5c..201cc21 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;
@@ -85,6 +86,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;
@@ -366,6 +368,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);

@@ -520,6 +525,11 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
return PTR_ERR(adsp->qdsp6ss_base);
}

+ adsp->lpass_efuse = devm_platform_ioremap_resource_byname(pdev, "lpass_efuse");
+ if (IS_ERR(adsp->lpass_efuse)) {
+ adsp->lpass_efuse = NULL;
+ dev_dbg(adsp->dev, "failed to map LPASS efuse registers\n");
+ }
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-08-03 20:47:23

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/8] dt-bindings: remoteproc: qcom: adsp: Add compatible name for SC7280

On Wed, 03 Aug 2022 19:51:14 +0530, Srinivasa Rao Mandadapu wrote:
> Add compatible name and update max reg items for SC7280 base platforms.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> ---
> .../devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>

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

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.example.dtb: remoteproc@17300000: reg: [[389021696, 1036]] is too short
From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml

doc reference errors (make refcheckdocs):

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

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

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

pip3 install dtschema --upgrade

Please check and re-submit.


2022-08-03 20:50:55

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/8] dt-bindings: remoteproc: qcom: adsp: Add compatible name for SC7280

On Wed, Aug 03, 2022 at 07:51:14PM +0530, Srinivasa Rao Mandadapu wrote:
> Add compatible name and update max reg items for SC7280 base platforms.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> ---
> .../devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
> index 9f11332..147996f 100644
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
> @@ -17,11 +17,12 @@ properties:
> compatible:
> enum:
> - qcom,sdm845-adsp-pil
> + - qcom,sc7280-adsp-pil
>
> reg:
> - maxItems: 1
> + maxItems: 2

sdm845 has 2 entries too?

> description:
> - The base address and size of the qdsp6ss register
> + The base address and size of the qdsp6ss register and mcc register

Better expressed as:

minItems: 1
items:
- description: qdsp6ss register
- description: mcc register

Though the descriptions could expand on what those registers are.

Rob

2022-08-04 10:43:58

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: Re: [PATCH 2/8] dt-bindings: remoteproc: qcom: adsp: Add compatible name for SC7280


On 8/4/2022 2:13 AM, Rob Herring wrote:
Thanks for your time and valuable inputs Rob!!!
> On Wed, Aug 03, 2022 at 07:51:14PM +0530, Srinivasa Rao Mandadapu wrote:
>> Add compatible name and update max reg items for SC7280 base platforms.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> ---
>> .../devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
>> index 9f11332..147996f 100644
>> --- a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
>> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
>> @@ -17,11 +17,12 @@ properties:
>> compatible:
>> enum:
>> - qcom,sdm845-adsp-pil
>> + - qcom,sc7280-adsp-pil
>>
>> reg:
>> - maxItems: 1
>> + maxItems: 2
> sdm845 has 2 entries too?
No. There max items not changed.
>
>> description:
>> - The base address and size of the qdsp6ss register
>> + The base address and size of the qdsp6ss register and mcc register
> Better expressed as:
>
> minItems: 1
> items:
> - description: qdsp6ss register
> - description: mcc register
>
> Though the descriptions could expand on what those registers are.
>
> Rob
Okay. Will change accordingly and re spin the patches.

2022-08-04 20:12:27

by Caleb Connolly

[permalink] [raw]
Subject: Re: [PATCH 1/8] dt-bindings: remoteproc: qcom: adsp: Make ADSP pil loader as generic


Hi,
On 03/08/2022 15:21, Srinivasa Rao Mandadapu wrote:
> Rename sdm845 adsp pil bindings to generic name, for using same binings
> file for subsequent SoCs.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> ---
> .../bindings/remoteproc/qcom,lpass-adsp-pil.yaml | 160 +++++++++++++++++++++
> .../bindings/remoteproc/qcom,sdm845-adsp-pil.yaml | 160 ---------------------
> 2 files changed, 160 insertions(+), 160 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
> delete mode 100644 Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
This should be a rename, I think passing the -M flag to "git format-patch"
detects it.
>
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
> new file mode 100644
> index 0000000..9f11332
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
> @@ -0,0 +1,160 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/remoteproc/qcom,lpass-adsp-pil.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm LPASS ADSP Peripheral Image Loader
> +
> +maintainers:
> + - Bjorn Andersson <[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,sdm845-adsp-pil
> +
> + reg:
> + maxItems: 1
> + description:
> + The base address and size of the qdsp6ss register
> +
> + interrupts:
> + items:
> + - description: Watchdog interrupt
> + - description: Fatal interrupt
> + - description: Ready interrupt
> + - description: Handover interrupt
> + - description: Stop acknowledge interrupt
> +
> + interrupt-names:
> + items:
> + - const: wdog
> + - const: fatal
> + - const: ready
> + - const: handover
> + - const: stop-ack
> +
> + clocks:
> + items:
> + - description: XO clock
> + - description: SWAY 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: sway_cbcr
> + - const: lpass_ahbs_aon_cbcr
> + - const: lpass_ahbm_aon_cbcr
> + - const: qdsp6ss_xo
> + - const: qdsp6ss_sleep
> + - const: qdsp6ss_core
> +
> + power-domains:
> + items:
> + - description: CX 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
> + three offsets within syscon for q6, modem and nc 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-sdm845.h>
> + #include <dt-bindings/clock/qcom,lpass-sdm845.h>
> + #include <dt-bindings/power/qcom-rpmpd.h>
> + #include <dt-bindings/reset/qcom,sdm845-pdc.h>
> + #include <dt-bindings/reset/qcom,sdm845-aoss.h>
> + remoteproc@17300000 {
> + compatible = "qcom,sdm845-adsp-pil";
> + reg = <0x17300000 0x40c>;
> +
> + interrupts-extended = <&intc GIC_SPI 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>;
> + interrupt-names = "wdog", "fatal", "ready",
> + "handover", "stop-ack";
> +
> + clocks = <&rpmhcc RPMH_CXO_CLK>,
> + <&gcc GCC_LPASS_SWAY_CLK>,
> + <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>,
> + <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>,
> + <&lpasscc LPASS_QDSP6SS_XO_CLK>,
> + <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
> + <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
> + clock-names = "xo", "sway_cbcr",
> + "lpass_ahbs_aon_cbcr",
> + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
> + "qdsp6ss_sleep", "qdsp6ss_core";
> +
> + power-domains = <&rpmhpd SDM845_CX>;
> +
> + resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
> + <&aoss_reset AOSS_CC_LPASS_RESTART>;
> + reset-names = "pdc_sync", "cc_lpass";
> +
> + qcom,halt-regs = <&tcsr_mutex_regs 0x22000>;
> +
> + memory-region = <&pil_adsp_mem>;
> +
> + qcom,smem-states = <&adsp_smp2p_out 0>;
> + qcom,smem-state-names = "stop";
> + };
> diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
> deleted file mode 100644
> index 1535bbb..0000000
> --- a/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
> +++ /dev/null
> @@ -1,160 +0,0 @@
> -# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> -%YAML 1.2
> ----
> -$id: http://devicetree.org/schemas/remoteproc/qcom,sdm845-adsp-pil.yaml#
> -$schema: http://devicetree.org/meta-schemas/core.yaml#
> -
> -title: Qualcomm SDM845 ADSP Peripheral Image Loader
> -
> -maintainers:
> - - Bjorn Andersson <[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,sdm845-adsp-pil
> -
> - reg:
> - maxItems: 1
> - description:
> - The base address and size of the qdsp6ss register
> -
> - interrupts:
> - items:
> - - description: Watchdog interrupt
> - - description: Fatal interrupt
> - - description: Ready interrupt
> - - description: Handover interrupt
> - - description: Stop acknowledge interrupt
> -
> - interrupt-names:
> - items:
> - - const: wdog
> - - const: fatal
> - - const: ready
> - - const: handover
> - - const: stop-ack
> -
> - clocks:
> - items:
> - - description: XO clock
> - - description: SWAY 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: sway_cbcr
> - - const: lpass_ahbs_aon_cbcr
> - - const: lpass_ahbm_aon_cbcr
> - - const: qdsp6ss_xo
> - - const: qdsp6ss_sleep
> - - const: qdsp6ss_core
> -
> - power-domains:
> - items:
> - - description: CX 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
> - three offsets within syscon for q6, modem and nc 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-sdm845.h>
> - #include <dt-bindings/clock/qcom,lpass-sdm845.h>
> - #include <dt-bindings/power/qcom-rpmpd.h>
> - #include <dt-bindings/reset/qcom,sdm845-pdc.h>
> - #include <dt-bindings/reset/qcom,sdm845-aoss.h>
> - remoteproc@17300000 {
> - compatible = "qcom,sdm845-adsp-pil";
> - reg = <0x17300000 0x40c>;
> -
> - interrupts-extended = <&intc GIC_SPI 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>;
> - interrupt-names = "wdog", "fatal", "ready",
> - "handover", "stop-ack";
> -
> - clocks = <&rpmhcc RPMH_CXO_CLK>,
> - <&gcc GCC_LPASS_SWAY_CLK>,
> - <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>,
> - <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>,
> - <&lpasscc LPASS_QDSP6SS_XO_CLK>,
> - <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
> - <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
> - clock-names = "xo", "sway_cbcr",
> - "lpass_ahbs_aon_cbcr",
> - "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
> - "qdsp6ss_sleep", "qdsp6ss_core";
> -
> - power-domains = <&rpmhpd SDM845_CX>;
> -
> - resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
> - <&aoss_reset AOSS_CC_LPASS_RESTART>;
> - reset-names = "pdc_sync", "cc_lpass";
> -
> - qcom,halt-regs = <&tcsr_mutex_regs 0x22000>;
> -
> - memory-region = <&pil_adsp_mem>;
> -
> - qcom,smem-states = <&adsp_smp2p_out 0>;
> - qcom,smem-state-names = "stop";
> - };
> --
> 2.7.4
>

--
Kind Regards,
Caleb (they/he)

2022-08-06 20:31:41

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 4/8] remoteproc: qcom: Update hard coded values with macros

On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote:
> Update hard coded values with appropriate macro names.

'Replace'

Other than that,

Reviewed-by: Dmitry Baryshkov <[email protected]>

>
> Signed-off-by: Srinivasa Rao Mandadapu <[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 bb4494c..a9fcb5c 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;
> @@ -364,10 +367,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,


--
With best wishes
Dmitry

2022-08-06 20:35:18

by Dmitry Baryshkov

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

On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote:
> Add memory sandbox support for ADSP based platforms secure booting.

This repeats commit subject. Please replace it with proper commit
message text describing what is done and why.

>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> ---
> drivers/remoteproc/qcom_q6v5_adsp.c | 101 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 3dbd035..f81da47 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
> @@ -77,7 +80,7 @@ struct adsp_pil_data {
> struct qcom_adsp {
> struct device *dev;
> struct rproc *rproc;
> -
> + struct iommu_domain *iommu_dom;
> struct qcom_q6v5 q6v5;
>
> struct clk *xo;
> @@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
> return 0;
> }
>
> +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc)
> +{
> + struct of_phandle_args args;
> + int ret, rc, i;
> + long long sid;
> +
> + unsigned long mem_phys;
> + unsigned long iova;
> + const __be32 *prop;
> + int access_level;
> + uint32_t len, flag, mem_size;
> + int offset;
> + struct fw_rsc_hdr *hdr;
> + struct fw_rsc_devmem *rsc_fw;
> +
> + rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node, "iommus", 1, 0, &args);

Please do not add implicit dependency on #iommu-cells value.

> + if (rc < 0)
> + sid = -1;
> + else
> + sid = args.args[0] & SID_MASK_DEFAULT;
> +
> + adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type);

please use adsp->dev->bus instead of platform_bus_type here.

> + if (!adsp->iommu_dom) {
> + dev_err(adsp->dev, "failed to allocate iommu domain\n");
> + return -ENOMEM;
> + }
> +
> + ret = iommu_attach_device(adsp->iommu_dom, adsp->dev);
> + if (ret) {
> + dev_err(adsp->dev, "could not attach device ret = %d\n", ret);
> + return -EBUSY;
> + }
> +
> + /* Add SID configuration for ADSP Firmware to SMMU */
> + adsp->mem_phys = adsp->mem_phys | (sid << 32);
> +
> + ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, 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;
> + }
> +
> + prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory", &len);

Non-documented property. So, this chunk is not acceptable.

> + if (prop) {
> + len /= sizeof(__be32);
> + for (i = 0; i < len; i++) {
> + iova = be32_to_cpu(prop[i++]);
> + mem_phys = be32_to_cpu(prop[i++]);
> + mem_size = be32_to_cpu(prop[i++]);
> + access_level = be32_to_cpu(prop[i]);
> +
> + if (access_level)
> + flag = IOMMU_READ | IOMMU_WRITE;
> + else
> + flag = IOMMU_READ;
> +
> + ret = iommu_map(adsp->iommu_dom, iova, mem_phys, mem_size, flag);
> + if (ret) {
> + dev_err(adsp->dev, "failed to map addr = %p mem_size = %x\n",
> + &(mem_phys), mem_size);
> + return ret;
> + }
> + }
> + } else {
> + if (!rproc->table_ptr)
> + return 0;
> +
> + for (i = 0; i < rproc->table_ptr->num; i++) {
> + offset = rproc->table_ptr->offset[i];
> + hdr = (void *)rproc->table_ptr + offset;
> + rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr);
> +
> + ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa,
> + rsc_fw->len, rsc_fw->flags);

What about filling an sgtable instead and using it?

> + if (ret) {
> + pr_err("%s; unable to map adsp memory address\n", __func__);
> + return ret;
> + }
> + }
> + }
> + return 0;
> +}
> +
> +
> static int adsp_start(struct rproc *rproc)
> {
> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc)
> ret = qcom_q6v5_prepare(&adsp->q6v5);
> if (ret)
> return ret;
> -
> + if (!adsp->is_wpss) {
> + ret = adsp_map_smmu(adsp, rproc);

Is this also applicable to cDSP? To sdm845 adsp?

> + if (ret) {
> + dev_err(adsp->dev, "ADSP smmu mapping failed\n");
> + goto adsp_smmu_unmap;
> + }
> + }
> ret = clk_prepare_enable(adsp->xo);
> if (ret)
> goto disable_irqs;
> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc)
> clk_disable_unprepare(adsp->xo);
> disable_irqs:
> qcom_q6v5_unprepare(&adsp->q6v5);
> +adsp_smmu_unmap:
> + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size);
> + iommu_domain_free(adsp->iommu_dom);
>
> return ret;
> }


--
With best wishes
Dmitry

2022-08-06 20:47:12

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 5/8] remoteproc: qcom: Add efuse evb selection control

On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote:
> Add efuse evb selection control and enable it for starting ADSP.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>

Is the lpass_efuse region used solely by the ADSP or is it shared with
anybody else (e.g. other sound-related devices)? If the latter is true,
then please use syscon for the lpass_efuse region.

> ---
> drivers/remoteproc/qcom_q6v5_adsp.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index a9fcb5c..201cc21 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;
> @@ -85,6 +86,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;
> @@ -366,6 +368,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);
>
> @@ -520,6 +525,11 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
> return PTR_ERR(adsp->qdsp6ss_base);
> }
>
> + adsp->lpass_efuse = devm_platform_ioremap_resource_byname(pdev, "lpass_efuse");
> + if (IS_ERR(adsp->lpass_efuse)) {
> + adsp->lpass_efuse = NULL;
> + dev_dbg(adsp->dev, "failed to map LPASS efuse registers\n");
> + }
> 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");


--
With best wishes
Dmitry

2022-08-06 20:54:23

by Christophe JAILLET

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

Hi,

the error handling below looks odd.

Le 03/08/2022 à 16:21, Srinivasa Rao Mandadapu a écrit :
> Add memory sandbox support for ADSP based platforms secure booting.
>
> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
> ---
> drivers/remoteproc/qcom_q6v5_adsp.c | 101 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 99 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 3dbd035..f81da47 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c

> static int adsp_start(struct rproc *rproc)
> {
> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc)
> ret = qcom_q6v5_prepare(&adsp->q6v5);
> if (ret)
> return ret;
> -
> + if (!adsp->is_wpss) {
> + ret = adsp_map_smmu(adsp, rproc);
> + if (ret) {
> + dev_err(adsp->dev, "ADSP smmu mapping failed\n");
> + goto adsp_smmu_unmap;
goto disable_irqs;?

> + }
> + }
> ret = clk_prepare_enable(adsp->xo);
> if (ret)
> goto disable_irqs;

goto adsp_smmu_unmap;?

> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc)
> clk_disable_unprepare(adsp->xo);
> disable_irqs:
> qcom_q6v5_unprepare(&adsp->q6v5);
> +adsp_smmu_unmap:
> + iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size);
> + iommu_domain_free(adsp->iommu_dom);

Should this new hunk be above disable_irqs?
And I think that it should be guardd with a "if (!adsp->is_wpss)".

CJ

>
> return ret;
> }

2022-08-06 21:06:19

by Dmitry Baryshkov

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

On 03/08/2022 17:21, 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]>
> ---
> drivers/remoteproc/qcom_q6v5_adsp.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 2f3b9f5..bb4494c 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -697,6 +697,24 @@ 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.mbn",
> + .load_state = "adsp",
> + .ssr_name = "lpass",
> + .sysmon_name = "adsp",
> + .ssctl_id = 0x14,
> + .is_wpss = false,
> + .auto_boot = true,
> + .clk_ids = (const char*[]) {
> + "gcc_cfg_noc_lpass", NULL

The clock is not mentioned in dt bindings.

> + },
> + .num_clks = 1,
> + .proxy_pd_names = (const char*[]) {
> + NULL
> + },

Is the empty array necessary?

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


--
With best wishes
Dmitry

2022-08-09 06:43:05

by Srinivasa Rao Mandadapu

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


On 8/7/2022 2:09 AM, Christophe JAILLET wrote:
Thanks for Your Time CJ!!!
> Hi,
>
> the error handling below looks odd.
>
> Le 03/08/2022 à 16:21, Srinivasa Rao Mandadapu a écrit :
>> Add memory sandbox support for ADSP based platforms secure booting.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu
>> <[email protected]>
>> ---
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 101
>> +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 99 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index 3dbd035..f81da47 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>
>>   static int adsp_start(struct rproc *rproc)
>>   {
>>       struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
>> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc)
>>       ret = qcom_q6v5_prepare(&adsp->q6v5);
>>       if (ret)
>>           return ret;
>> -
>> +    if (!adsp->is_wpss) {
>> +        ret = adsp_map_smmu(adsp, rproc);
>> +        if (ret) {
>> +            dev_err(adsp->dev, "ADSP smmu mapping failed\n");
>> +            goto adsp_smmu_unmap;
> goto disable_irqs;?
Yes. will correct it accordingly.
>
>> +        }
>> +    }
>>       ret = clk_prepare_enable(adsp->xo);
>>       if (ret)
>>           goto disable_irqs;
>
> goto adsp_smmu_unmap;?
Yes. will correct it accordingly.
>
>> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc)
>>       clk_disable_unprepare(adsp->xo);
>>   disable_irqs:
>>       qcom_q6v5_unprepare(&adsp->q6v5);
>> +adsp_smmu_unmap:
>> +    iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size);
>> +    iommu_domain_free(adsp->iommu_dom);
>
> Should this new hunk be above disable_irqs?
> And I think that it should be guardd with a "if (!adsp->is_wpss)".
Yes. will correct it accordingly.
>
> CJ
>
>>         return ret;
>>   }
>

2022-08-09 06:54:12

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: Re: [PATCH 5/8] remoteproc: qcom: Add efuse evb selection control


On 8/7/2022 1:56 AM, Dmitry Baryshkov wrote:
Thanks for Your Time Dmitry!!!
> On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote:
>> Add efuse evb selection control and enable it for starting ADSP.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>
> Is the lpass_efuse region used solely by the ADSP or is it shared with
> anybody else (e.g. other sound-related devices)? If the latter is
> true, then please use syscon for the lpass_efuse region.
This region is being used by ADS PIL driver only.
>
>> ---
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index a9fcb5c..201cc21 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;
>> @@ -85,6 +86,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;
>> @@ -366,6 +368,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);
>>   @@ -520,6 +525,11 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
>>           return PTR_ERR(adsp->qdsp6ss_base);
>>       }
>>   +    adsp->lpass_efuse =
>> devm_platform_ioremap_resource_byname(pdev, "lpass_efuse");
>> +    if (IS_ERR(adsp->lpass_efuse)) {
>> +        adsp->lpass_efuse = NULL;
>> +        dev_dbg(adsp->dev, "failed to map LPASS efuse registers\n");
>> +    }
>>       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-08-09 08:29:50

by Srinivasa Rao Mandadapu

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


On 8/7/2022 2:04 AM, Dmitry Baryshkov wrote:
Thanks for your time and Valuable inputs Dmitry!!!
> On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote:
>> Add memory sandbox support for ADSP based platforms secure booting.
>
> This repeats commit subject. Please replace it with proper commit
> message text describing what is done and why.
Okay. Will update it.
>
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> ---
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 101
>> +++++++++++++++++++++++++++++++++++-
>>   1 file changed, 99 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index 3dbd035..f81da47 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
>> @@ -77,7 +80,7 @@ struct adsp_pil_data {
>>   struct qcom_adsp {
>>       struct device *dev;
>>       struct rproc *rproc;
>> -
>> +    struct iommu_domain *iommu_dom;
>>       struct qcom_q6v5 q6v5;
>>         struct clk *xo;
>> @@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const
>> struct firmware *fw)
>>       return 0;
>>   }
>>   +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc *rproc)
>> +{
>> +    struct of_phandle_args args;
>> +    int ret, rc, i;
>> +    long long sid;
>> +
>> +    unsigned long mem_phys;
>> +    unsigned long iova;
>> +    const __be32 *prop;
>> +    int access_level;
>> +    uint32_t len, flag, mem_size;
>> +    int offset;
>> +    struct fw_rsc_hdr *hdr;
>> +    struct fw_rsc_devmem *rsc_fw;
>> +
>> +    rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node,
>> "iommus", 1, 0, &args);
>
> Please do not add implicit dependency on #iommu-cells value.
Okay. Will change it to "of_parse_phandle_with_args()"
>
>> +    if (rc < 0)
>> +        sid = -1;
>> +    else
>> +        sid = args.args[0] & SID_MASK_DEFAULT;
>> +
>> +    adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type);
>
> please use adsp->dev->bus instead of platform_bus_type here.
Okay. will update it.
>
>> +    if (!adsp->iommu_dom) {
>> +        dev_err(adsp->dev, "failed to allocate iommu domain\n");
>> +        return -ENOMEM;
>> +    }
>> +
>> +    ret = iommu_attach_device(adsp->iommu_dom, adsp->dev);
>> +    if (ret) {
>> +        dev_err(adsp->dev, "could not attach device ret = %d\n", ret);
>> +        return -EBUSY;
>> +    }
>> +
>> +    /* Add SID configuration for ADSP Firmware to SMMU */
>> +    adsp->mem_phys =  adsp->mem_phys | (sid << 32);
>> +
>> +    ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, 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;
>> +    }
>> +
>> +    prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory",
>> &len);
>
> Non-documented property. So, this chunk is not acceptable.
Okay. Will add it in dt-bindings too.
>
>> +    if (prop) {
>> +        len /= sizeof(__be32);
>> +        for (i = 0; i < len; i++) {
>> +            iova = be32_to_cpu(prop[i++]);
>> +            mem_phys = be32_to_cpu(prop[i++]);
>> +            mem_size = be32_to_cpu(prop[i++]);
>> +            access_level = be32_to_cpu(prop[i]);
>> +
>> +            if (access_level)
>> +                flag = IOMMU_READ | IOMMU_WRITE;
>> +            else
>> +                flag = IOMMU_READ;
>> +
>> +            ret = iommu_map(adsp->iommu_dom, iova, mem_phys,
>> mem_size, flag);
>> +            if (ret) {
>> +                dev_err(adsp->dev, "failed to map addr = %p mem_size
>> = %x\n",
>> +                        &(mem_phys), mem_size);
>> +                return ret;
>> +            }
>> +        }
>> +    } else {
>> +        if (!rproc->table_ptr)
>> +            return 0;
>> +
>> +        for (i = 0; i < rproc->table_ptr->num; i++) {
>> +            offset = rproc->table_ptr->offset[i];
>> +            hdr = (void *)rproc->table_ptr + offset;
>> +            rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr);
>> +
>> +            ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa,
>> +                        rsc_fw->len, rsc_fw->flags);
>
> What about filling an sgtable instead and using it?

Here we are just doing IO mapping and allowing ADSP to access the
specified memory.

I am not sure,  sg_table applicable here or not as it's not any DMA
activity.

Please correct me if my understanding is not enough and It would help me
a lot, if any good example shared.

>
>> +            if (ret) {
>> +                pr_err("%s; unable to map adsp memory address\n",
>> __func__);
>> +                return ret;
>> +            }
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>> +
>>   static int adsp_start(struct rproc *rproc)
>>   {
>>       struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
>> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc)
>>       ret = qcom_q6v5_prepare(&adsp->q6v5);
>>       if (ret)
>>           return ret;
>> -
>> +    if (!adsp->is_wpss) {
>> +        ret = adsp_map_smmu(adsp, rproc);
>
> Is this also applicable to cDSP? To sdm845 adsp?

It's applicable to all ADSP SoC variants. I think it's better to add
adsp flag("is_adsp") for

distinguishing adsp use cases. Please suggest here.

>
>> +        if (ret) {
>> +            dev_err(adsp->dev, "ADSP smmu mapping failed\n");
>> +            goto adsp_smmu_unmap;
>> +        }
>> +    }
>>       ret = clk_prepare_enable(adsp->xo);
>>       if (ret)
>>           goto disable_irqs;
>> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc)
>>       clk_disable_unprepare(adsp->xo);
>>   disable_irqs:
>>       qcom_q6v5_unprepare(&adsp->q6v5);
>> +adsp_smmu_unmap:
>> +    iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size);
>> +    iommu_domain_free(adsp->iommu_dom);
>>         return ret;
>>   }
>
>

2022-08-09 10:04:45

by Srinivasa Rao Mandadapu

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


On 8/7/2022 2:27 AM, Dmitry Baryshkov wrote:
Thanks for Your Time Dimtry!!!
> On 03/08/2022 17:21, 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]>
>> ---
>>   drivers/remoteproc/qcom_q6v5_adsp.c | 19 +++++++++++++++++++
>>   1 file changed, 19 insertions(+)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>> index 2f3b9f5..bb4494c 100644
>> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
>> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
>> @@ -697,6 +697,24 @@ 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.mbn",
>> +    .load_state = "adsp",
>> +    .ssr_name = "lpass",
>> +    .sysmon_name = "adsp",
>> +    .ssctl_id = 0x14,
>> +    .is_wpss = false,
>> +    .auto_boot = true,
>> +    .clk_ids = (const char*[]) {
>> +        "gcc_cfg_noc_lpass", NULL
>
> The clock is not mentioned in dt bindings.
Will update in dt bindings and re post it.
>
>> +    },
>> +    .num_clks = 1,
>> +    .proxy_pd_names = (const char*[]) {
>> +        NULL
>> +    },
>
> Is the empty array necessary?
Okay. Will remove it.
>
>> +};
>> +
>>   static const struct adsp_pil_data cdsp_resource_init = {
>>       .crash_reason_smem = 601,
>>       .firmware_name = "cdsp.mdt",
>> @@ -737,6 +755,7 @@ static const struct of_device_id adsp_of_match[] = {
>>       { .compatible = "qcom,qcs404-cdsp-pil", .data =
>> &cdsp_resource_init },
>>       { .compatible = "qcom,sc7280-wpss-pil", .data =
>> &wpss_resource_init },
>>       { .compatible = "qcom,sdm845-adsp-pil", .data =
>> &adsp_resource_init },
>> +    { .compatible = "qcom,sc7280-adsp-pil", .data =
>> &adsp_sc7280_resource_init },
>>       { },
>>   };
>>   MODULE_DEVICE_TABLE(of, adsp_of_match);
>
>

2022-08-09 10:55:14

by Srinivasa Rao Mandadapu

[permalink] [raw]
Subject: Re: [PATCH 1/8] dt-bindings: remoteproc: qcom: adsp: Make ADSP pil loader as generic


On 8/5/2022 1:17 AM, Caleb Connolly wrote:
Thanks for Your TIme Celeb!!
>
> Hi,
> On 03/08/2022 15:21, Srinivasa Rao Mandadapu wrote:
>> Rename sdm845 adsp pil bindings to generic name, for using same binings
>> file for subsequent SoCs.
>>
>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>> ---
>>   .../bindings/remoteproc/qcom,lpass-adsp-pil.yaml   | 160
>> +++++++++++++++++++++
>>   .../bindings/remoteproc/qcom,sdm845-adsp-pil.yaml  | 160
>> ---------------------
>>   2 files changed, 160 insertions(+), 160 deletions(-)
>>   create mode 100644
>> Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
>>   delete mode 100644
>> Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
> This should be a rename, I think passing the -M flag to "git
> format-patch" detects it.
Okay. Will do accordingly in next version of patches.
>>
>> diff --git
>> a/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
>> b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
>> new file mode 100644
>> index 0000000..9f11332
>> --- /dev/null
>> +++
>> b/Documentation/devicetree/bindings/remoteproc/qcom,lpass-adsp-pil.yaml
>> @@ -0,0 +1,160 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/remoteproc/qcom,lpass-adsp-pil.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm LPASS ADSP Peripheral Image Loader
>> +
>> +maintainers:
>> +  - Bjorn Andersson <[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,sdm845-adsp-pil
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description:
>> +      The base address and size of the qdsp6ss register
>> +
>> +  interrupts:
>> +    items:
>> +      - description: Watchdog interrupt
>> +      - description: Fatal interrupt
>> +      - description: Ready interrupt
>> +      - description: Handover interrupt
>> +      - description: Stop acknowledge interrupt
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: wdog
>> +      - const: fatal
>> +      - const: ready
>> +      - const: handover
>> +      - const: stop-ack
>> +
>> +  clocks:
>> +    items:
>> +      - description: XO clock
>> +      - description: SWAY 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: sway_cbcr
>> +      - const: lpass_ahbs_aon_cbcr
>> +      - const: lpass_ahbm_aon_cbcr
>> +      - const: qdsp6ss_xo
>> +      - const: qdsp6ss_sleep
>> +      - const: qdsp6ss_core
>> +
>> +  power-domains:
>> +    items:
>> +      - description: CX 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
>> +      three offsets within syscon for q6, modem and nc 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-sdm845.h>
>> +    #include <dt-bindings/clock/qcom,lpass-sdm845.h>
>> +    #include <dt-bindings/power/qcom-rpmpd.h>
>> +    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
>> +    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>> +    remoteproc@17300000 {
>> +        compatible = "qcom,sdm845-adsp-pil";
>> +        reg = <0x17300000 0x40c>;
>> +
>> +        interrupts-extended = <&intc GIC_SPI 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>;
>> +        interrupt-names = "wdog", "fatal", "ready",
>> +                "handover", "stop-ack";
>> +
>> +        clocks = <&rpmhcc RPMH_CXO_CLK>,
>> +                 <&gcc GCC_LPASS_SWAY_CLK>,
>> +                 <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>,
>> +                 <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>,
>> +                 <&lpasscc LPASS_QDSP6SS_XO_CLK>,
>> +                 <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
>> +                 <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
>> +        clock-names = "xo", "sway_cbcr",
>> +                "lpass_ahbs_aon_cbcr",
>> +                "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
>> +                "qdsp6ss_sleep", "qdsp6ss_core";
>> +
>> +        power-domains = <&rpmhpd SDM845_CX>;
>> +
>> +        resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
>> +                 <&aoss_reset AOSS_CC_LPASS_RESTART>;
>> +        reset-names = "pdc_sync", "cc_lpass";
>> +
>> +        qcom,halt-regs = <&tcsr_mutex_regs 0x22000>;
>> +
>> +        memory-region = <&pil_adsp_mem>;
>> +
>> +        qcom,smem-states = <&adsp_smp2p_out 0>;
>> +        qcom,smem-state-names = "stop";
>> +    };
>> diff --git
>> a/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
>> b/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
>> deleted file mode 100644
>> index 1535bbb..0000000
>> ---
>> a/Documentation/devicetree/bindings/remoteproc/qcom,sdm845-adsp-pil.yaml
>> +++ /dev/null
>> @@ -1,160 +0,0 @@
>> -# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> -%YAML 1.2
>> ----
>> -$id:
>> http://devicetree.org/schemas/remoteproc/qcom,sdm845-adsp-pil.yaml#
>> -$schema: http://devicetree.org/meta-schemas/core.yaml#
>> -
>> -title: Qualcomm SDM845 ADSP Peripheral Image Loader
>> -
>> -maintainers:
>> -  - Bjorn Andersson <[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,sdm845-adsp-pil
>> -
>> -  reg:
>> -    maxItems: 1
>> -    description:
>> -      The base address and size of the qdsp6ss register
>> -
>> -  interrupts:
>> -    items:
>> -      - description: Watchdog interrupt
>> -      - description: Fatal interrupt
>> -      - description: Ready interrupt
>> -      - description: Handover interrupt
>> -      - description: Stop acknowledge interrupt
>> -
>> -  interrupt-names:
>> -    items:
>> -      - const: wdog
>> -      - const: fatal
>> -      - const: ready
>> -      - const: handover
>> -      - const: stop-ack
>> -
>> -  clocks:
>> -    items:
>> -      - description: XO clock
>> -      - description: SWAY 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: sway_cbcr
>> -      - const: lpass_ahbs_aon_cbcr
>> -      - const: lpass_ahbm_aon_cbcr
>> -      - const: qdsp6ss_xo
>> -      - const: qdsp6ss_sleep
>> -      - const: qdsp6ss_core
>> -
>> -  power-domains:
>> -    items:
>> -      - description: CX 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
>> -      three offsets within syscon for q6, modem and nc 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-sdm845.h>
>> -    #include <dt-bindings/clock/qcom,lpass-sdm845.h>
>> -    #include <dt-bindings/power/qcom-rpmpd.h>
>> -    #include <dt-bindings/reset/qcom,sdm845-pdc.h>
>> -    #include <dt-bindings/reset/qcom,sdm845-aoss.h>
>> -    remoteproc@17300000 {
>> -        compatible = "qcom,sdm845-adsp-pil";
>> -        reg = <0x17300000 0x40c>;
>> -
>> -        interrupts-extended = <&intc GIC_SPI 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>;
>> -        interrupt-names = "wdog", "fatal", "ready",
>> -                "handover", "stop-ack";
>> -
>> -        clocks = <&rpmhcc RPMH_CXO_CLK>,
>> -                 <&gcc GCC_LPASS_SWAY_CLK>,
>> -                 <&lpasscc LPASS_Q6SS_AHBS_AON_CLK>,
>> -                 <&lpasscc LPASS_Q6SS_AHBM_AON_CLK>,
>> -                 <&lpasscc LPASS_QDSP6SS_XO_CLK>,
>> -                 <&lpasscc LPASS_QDSP6SS_SLEEP_CLK>,
>> -                 <&lpasscc LPASS_QDSP6SS_CORE_CLK>;
>> -        clock-names = "xo", "sway_cbcr",
>> -                "lpass_ahbs_aon_cbcr",
>> -                "lpass_ahbm_aon_cbcr", "qdsp6ss_xo",
>> -                "qdsp6ss_sleep", "qdsp6ss_core";
>> -
>> -        power-domains = <&rpmhpd SDM845_CX>;
>> -
>> -        resets = <&pdc_reset PDC_AUDIO_SYNC_RESET>,
>> -                 <&aoss_reset AOSS_CC_LPASS_RESTART>;
>> -        reset-names = "pdc_sync", "cc_lpass";
>> -
>> -        qcom,halt-regs = <&tcsr_mutex_regs 0x22000>;
>> -
>> -        memory-region = <&pil_adsp_mem>;
>> -
>> -        qcom,smem-states = <&adsp_smp2p_out 0>;
>> -        qcom,smem-state-names = "stop";
>> -    };
>> --
>> 2.7.4
>>
>

2022-08-09 11:14:05

by Srinivasa Rao Mandadapu

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


On 8/9/2022 1:52 PM, Srinivasa Rao Mandadapu wrote:
>
> On 8/7/2022 2:04 AM, Dmitry Baryshkov wrote:
> Thanks for your time and Valuable inputs Dmitry!!!
>> On 03/08/2022 17:21, Srinivasa Rao Mandadapu wrote:
>>> Add memory sandbox support for ADSP based platforms secure booting.
>>
>> This repeats commit subject. Please replace it with proper commit
>> message text describing what is done and why.
> Okay. Will update it.
>>
>>>
>>> Signed-off-by: Srinivasa Rao Mandadapu <[email protected]>
>>> ---
>>>   drivers/remoteproc/qcom_q6v5_adsp.c | 101
>>> +++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 99 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
>>> b/drivers/remoteproc/qcom_q6v5_adsp.c
>>> index 3dbd035..f81da47 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
>>> @@ -77,7 +80,7 @@ struct adsp_pil_data {
>>>   struct qcom_adsp {
>>>       struct device *dev;
>>>       struct rproc *rproc;
>>> -
>>> +    struct iommu_domain *iommu_dom;
>>>       struct qcom_q6v5 q6v5;
>>>         struct clk *xo;
>>> @@ -332,6 +335,91 @@ static int adsp_load(struct rproc *rproc, const
>>> struct firmware *fw)
>>>       return 0;
>>>   }
>>>   +static int adsp_map_smmu(struct qcom_adsp *adsp, struct rproc
>>> *rproc)
>>> +{
>>> +    struct of_phandle_args args;
>>> +    int ret, rc, i;
>>> +    long long sid;
>>> +
>>> +    unsigned long mem_phys;
>>> +    unsigned long iova;
>>> +    const __be32 *prop;
>>> +    int access_level;
>>> +    uint32_t len, flag, mem_size;
>>> +    int offset;
>>> +    struct fw_rsc_hdr *hdr;
>>> +    struct fw_rsc_devmem *rsc_fw;
>>> +
>>> +    rc = of_parse_phandle_with_fixed_args(adsp->dev->of_node,
>>> "iommus", 1, 0, &args);
>>
>> Please do not add implicit dependency on #iommu-cells value.
> Okay. Will change it to "of_parse_phandle_with_args()"
>>
>>> +    if (rc < 0)
>>> +        sid = -1;
>>> +    else
>>> +        sid = args.args[0] & SID_MASK_DEFAULT;
>>> +
>>> +    adsp->iommu_dom = iommu_domain_alloc(&platform_bus_type);
>>
>> please use adsp->dev->bus instead of platform_bus_type here.
> Okay. will update it.
>>
>>> +    if (!adsp->iommu_dom) {
>>> +        dev_err(adsp->dev, "failed to allocate iommu domain\n");
>>> +        return -ENOMEM;
>>> +    }
>>> +
>>> +    ret = iommu_attach_device(adsp->iommu_dom, adsp->dev);
>>> +    if (ret) {
>>> +        dev_err(adsp->dev, "could not attach device ret = %d\n", ret);
>>> +        return -EBUSY;
>>> +    }
>>> +
>>> +    /* Add SID configuration for ADSP Firmware to SMMU */
>>> +    adsp->mem_phys =  adsp->mem_phys | (sid << 32);
>>> +
>>> +    ret = iommu_map(adsp->iommu_dom, adsp->mem_phys, 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;
>>> +    }
>>> +
>>> +    prop = of_get_property(adsp->dev->of_node, "qcom,adsp-memory",
>>> &len);
>>
>> Non-documented property. So, this chunk is not acceptable.
> Okay. Will add it in dt-bindings too.
>>
>>> +    if (prop) {
>>> +        len /= sizeof(__be32);
>>> +        for (i = 0; i < len; i++) {
>>> +            iova = be32_to_cpu(prop[i++]);
>>> +            mem_phys = be32_to_cpu(prop[i++]);
>>> +            mem_size = be32_to_cpu(prop[i++]);
>>> +            access_level = be32_to_cpu(prop[i]);
>>> +
>>> +            if (access_level)
>>> +                flag = IOMMU_READ | IOMMU_WRITE;
>>> +            else
>>> +                flag = IOMMU_READ;
>>> +
>>> +            ret = iommu_map(adsp->iommu_dom, iova, mem_phys,
>>> mem_size, flag);
>>> +            if (ret) {
>>> +                dev_err(adsp->dev, "failed to map addr = %p
>>> mem_size = %x\n",
>>> +                        &(mem_phys), mem_size);
>>> +                return ret;
>>> +            }
>>> +        }
>>> +    } else {
>>> +        if (!rproc->table_ptr)
>>> +            return 0;
>>> +
>>> +        for (i = 0; i < rproc->table_ptr->num; i++) {
>>> +            offset = rproc->table_ptr->offset[i];
>>> +            hdr = (void *)rproc->table_ptr + offset;
>>> +            rsc_fw = (struct fw_rsc_devmem *)hdr + sizeof(*hdr);
>>> +
>>> +            ret = iommu_map(rproc->domain, rsc_fw->da, rsc_fw->pa,
>>> +                        rsc_fw->len, rsc_fw->flags);
>>
>> What about filling an sgtable instead and using it?
>
> Here we are just doing IO mapping and allowing ADSP to access the
> specified memory.
>
> I am not sure,  sg_table applicable here or not as it's not any DMA
> activity.
>
> Please correct me if my understanding is not enough and It would help
> me a lot, if any good example shared.
>
>>
>>> +            if (ret) {
>>> +                pr_err("%s; unable to map adsp memory address\n",
>>> __func__);
>>> +                return ret;
>>> +            }
>>> +        }
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>>> +
>>>   static int adsp_start(struct rproc *rproc)
>>>   {
>>>       struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
>>> @@ -341,7 +429,13 @@ static int adsp_start(struct rproc *rproc)
>>>       ret = qcom_q6v5_prepare(&adsp->q6v5);
>>>       if (ret)
>>>           return ret;
>>> -
>>> +    if (!adsp->is_wpss) {
>>> +        ret = adsp_map_smmu(adsp, rproc);
>>
>> Is this also applicable to cDSP? To sdm845 adsp?
>
> It's applicable to all ADSP SoC variants. I think it's better to add
> adsp flag("is_adsp") for
>
> distinguishing adsp use cases. Please suggest here.

Verified with sdm845 developer, and got to know that, even though it's
applicable there too,

Somehow for them it was working without memory sandboxing.

Maybe, it was due to SMMU security levels were different.

>
>>
>>> +        if (ret) {
>>> +            dev_err(adsp->dev, "ADSP smmu mapping failed\n");
>>> +            goto adsp_smmu_unmap;
>>> +        }
>>> +    }
>>>       ret = clk_prepare_enable(adsp->xo);
>>>       if (ret)
>>>           goto disable_irqs;
>>> @@ -402,6 +496,9 @@ static int adsp_start(struct rproc *rproc)
>>>       clk_disable_unprepare(adsp->xo);
>>>   disable_irqs:
>>>       qcom_q6v5_unprepare(&adsp->q6v5);
>>> +adsp_smmu_unmap:
>>> +    iommu_unmap(adsp->iommu_dom, adsp->mem_phys, adsp->mem_size);
>>> +    iommu_domain_free(adsp->iommu_dom);
>>>         return ret;
>>>   }
>>
>>