[PATCH v3 0/9] MDM9607/MSM8917/MSM8937/MSM8940 MSS

[PATCH v3 0/9] MDM9607/MSM8917/MSM8937/MSM8940 MSS

[Barnabás Czémán]

This patch series add support for MDM9607/MSM8917/MSM8937/MSM8940 mss.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
Changes in v3:
- qcom_q6v5_mss: Rework inrush current mitigation
- Link to v2: https://lore.kernel.org/r/20251231-mss-v2-0-ae5eafd835c4@mainlining.org

Changes in v2:
- qcom_q6v5_mss:
  - Intorduce need_pas_mem_setup flag
  - Use mss-supply as a regulator for MSM8917, MSM8937 and MSM8940.
- qcom,msm8916-mss-pil:
  - Add compatibles where they needed
  - Adjust mss-supply description
- Link to v1: https://lore.kernel.org/r/20251228-mss-v1-0-aeb36b1f7a3f@mainlining.org

---
Barnabás Czémán (8):
      remoteproc: qcom_q6v5_mss: Introduce need_pas_mem_setup
      dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MDM9607
      dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8917
      remoteproc: qcom_q6v5_mss: Add MSM8917
      dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8937
      remoteproc: qcom_q6v5_mss: Add MSM8937
      dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8940
      remoteproc: qcom_q6v5_mss: Add MSM8940

Stephan Gerhold (1):
      remoteproc: qcom_q6v5_mss: Add MDM9607

 .../bindings/remoteproc/qcom,msm8916-mss-pil.yaml  |  13 +-
 drivers/remoteproc/qcom_q6v5_mss.c                 | 254 +++++++++++++++++++--
 2 files changed, 251 insertions(+), 16 deletions(-)
---
base-commit: cc3aa43b44bdb43dfbac0fcb51c56594a11338a8
change-id: 20251228-mss-1fa61b7092b5

Best regards,
-- 
Barnabás Czémán <barnabas.czeman@mainlining.org>

[PATCH v3 1/9] remoteproc: qcom_q6v5_mss: Introduce need_pas_mem_setup

[Barnabás Czémán]

Some platforms like MSM8953 and MSM8937 TZ needs to be
informed of the modem start address and pas_id.
Lets introduce need_pas_mem_setup flag for handle this case.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/remoteproc/qcom_q6v5_mss.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 91940977ca89..3c404118b322 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -162,6 +162,7 @@ struct rproc_hexagon_res {
 	char **proxy_pd_names;
 	int version;
 	bool need_mem_protection;
+	bool need_pas_mem_setup;
 	bool has_alt_reset;
 	bool has_mba_logs;
 	bool has_spare_reg;
@@ -240,6 +241,7 @@ struct q6v5 {
 	struct qcom_sysmon *sysmon;
 	struct platform_device *bam_dmux;
 	bool need_mem_protection;
+	bool need_pas_mem_setup;
 	bool has_alt_reset;
 	bool has_mba_logs;
 	bool has_spare_reg;
@@ -1441,7 +1443,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
 			max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
 	}
 
-	if (qproc->version == MSS_MSM8953) {
+	if (qproc->need_pas_mem_setup) {
 		ret = qcom_scm_pas_mem_setup(MPSS_PAS_ID, qproc->mpss_phys, qproc->mpss_size);
 		if (ret) {
 			dev_err(qproc->dev,
@@ -2224,6 +2226,7 @@ static const struct rproc_hexagon_res sc7180_mss = {
 		NULL
 	},
 	.need_mem_protection = true,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = true,
 	.has_spare_reg = true,
@@ -2253,6 +2256,7 @@ static const struct rproc_hexagon_res sc7280_mss = {
 		NULL
 	},
 	.need_mem_protection = true,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = true,
 	.has_spare_reg = false,
@@ -2285,6 +2289,7 @@ static const struct rproc_hexagon_res sdm660_mss = {
 			NULL
 	},
 	.need_mem_protection = true,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2321,6 +2326,7 @@ static const struct rproc_hexagon_res sdm845_mss = {
 			NULL
 	},
 	.need_mem_protection = true,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = true,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2353,6 +2359,7 @@ static const struct rproc_hexagon_res msm8998_mss = {
 			NULL
 	},
 	.need_mem_protection = true,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2392,6 +2399,7 @@ static const struct rproc_hexagon_res msm8996_mss = {
 			NULL
 	},
 	.need_mem_protection = true,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2427,6 +2435,7 @@ static const struct rproc_hexagon_res msm8909_mss = {
 		NULL
 	},
 	.need_mem_protection = false,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2473,6 +2482,7 @@ static const struct rproc_hexagon_res msm8916_mss = {
 		NULL
 	},
 	.need_mem_protection = false,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2509,6 +2519,7 @@ static const struct rproc_hexagon_res msm8953_mss = {
 		NULL
 	},
 	.need_mem_protection = false,
+	.need_pas_mem_setup = true,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2562,6 +2573,7 @@ static const struct rproc_hexagon_res msm8974_mss = {
 		NULL
 	},
 	.need_mem_protection = false,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2600,6 +2612,7 @@ static const struct rproc_hexagon_res msm8226_mss = {
 		NULL
 	},
 	.need_mem_protection = false,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,
@@ -2646,6 +2659,7 @@ static const struct rproc_hexagon_res msm8926_mss = {
 		NULL
 	},
 	.need_mem_protection = false,
+	.need_pas_mem_setup = false,
 	.has_alt_reset = false,
 	.has_mba_logs = false,
 	.has_spare_reg = false,

-- 
2.52.0

[PATCH v3 2/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MDM9607

[Barnabás Czémán]

Add the compatible for MSS as found on the MDM9607 platform.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
index c179b560572b..4e0d2fe0e46c 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
@@ -17,6 +17,7 @@ properties:
   compatible:
     oneOf:
       - enum:
+          - qcom,mdm9607-mss-pil
           - qcom,msm8226-mss-pil
           - qcom,msm8909-mss-pil
           - qcom,msm8916-mss-pil
@@ -226,6 +227,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - qcom,mdm9607-mss-pil
               - qcom,msm8909-mss-pil
               - qcom,msm8916-mss-pil
     then:

-- 
2.52.0

[PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Barnabás Czémán]

From: Stephan Gerhold <stephan@gerhold.net>

Add support for MDM9607 MSS it have different ACC settings
and it needs mitigation for inrush current issue.

Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Reword the commit, add necessary flags, rework inrush current mitigation]
Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/remoteproc/qcom_q6v5_mss.c | 89 ++++++++++++++++++++++++++++++++------
 1 file changed, 75 insertions(+), 14 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 3c404118b322..19863c576d72 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -124,6 +124,7 @@
 #define QDSP6v56_CLAMP_QMC_MEM		BIT(22)
 #define QDSP6SS_XO_CBCR		0x0038
 #define QDSP6SS_ACC_OVERRIDE_VAL		0x20
+#define QDSP6SS_ACC_OVERRIDE_VAL_9607	0x80800000
 #define QDSP6v55_BHS_EN_REST_ACK	BIT(0)
 
 /* QDSP6v65 parameters */
@@ -256,6 +257,7 @@ struct q6v5 {
 };
 
 enum {
+	MSS_MDM9607,
 	MSS_MSM8226,
 	MSS_MSM8909,
 	MSS_MSM8916,
@@ -747,15 +749,19 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 			return ret;
 		}
 		goto pbl_wait;
-	} else if (qproc->version == MSS_MSM8909 ||
+	} else if (qproc->version == MSS_MDM9607 ||
+		   qproc->version == MSS_MSM8909 ||
 		   qproc->version == MSS_MSM8953 ||
 		   qproc->version == MSS_MSM8996 ||
 		   qproc->version == MSS_MSM8998 ||
 		   qproc->version == MSS_SDM660) {
 
-		if (qproc->version != MSS_MSM8909 &&
-		    qproc->version != MSS_MSM8953)
-			/* Override the ACC value if required */
+		/* Override the ACC value if required */
+		if (qproc->version == MSS_MDM9607)
+			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
+			       qproc->reg_base + QDSP6SS_STRAP_ACC);
+		else if (qproc->version != MSS_MSM8909 &&
+			 qproc->version != MSS_MSM8953)
 			writel(QDSP6SS_ACC_OVERRIDE_VAL,
 			       qproc->reg_base + QDSP6SS_STRAP_ACC);
 
@@ -800,7 +806,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 		writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
 
 		if (qproc->version != MSS_MSM8909) {
-			int mem_pwr_ctl;
+			int mem_pwr_ctl, reverse = 0;
 
 			/* Deassert QDSP6 compiler memory clamp */
 			val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
@@ -812,7 +818,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 			writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
 
 			/* Turn on L1, L2, ETB and JU memories 1 at a time */
-			if (qproc->version == MSS_MSM8953 ||
+			if (qproc->version == MSS_MDM9607 ||
+			    qproc->version == MSS_MSM8953 ||
 			    qproc->version == MSS_MSM8996) {
 				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
 				i = 19;
@@ -822,16 +829,34 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 				i = 28;
 			}
 			val = readl(qproc->reg_base + mem_pwr_ctl);
-			for (; i >= 0; i--) {
-				val |= BIT(i);
-				writel(val, qproc->reg_base + mem_pwr_ctl);
+			if (qproc->version == MSS_MDM9607) {
 				/*
-				 * Read back value to ensure the write is done then
-				 * wait for 1us for both memory peripheral and data
-				 * array to turn on.
+				 * Set first 5 bits in reverse to avoid
+				 * "inrush current" issues.
 				 */
-				val |= readl(qproc->reg_base + mem_pwr_ctl);
-				udelay(1);
+				reverse = 6;
+				for (; i >= reverse; i--) {
+					val |= BIT(i);
+					writel(val, qproc->reg_base + mem_pwr_ctl);
+					udelay(1);
+				}
+				for (i = 0; i < reverse; i++) {
+					val |= BIT(i);
+					writel(val, qproc->reg_base + mem_pwr_ctl);
+					udelay(1);
+				}
+			} else {
+				for (; i >= 0; i--) {
+					val |= BIT(i);
+					writel(val, qproc->reg_base + mem_pwr_ctl);
+					/*
+					 * Read back value to ensure the write is done then
+					 * wait for 1us for both memory peripheral and data
+					 * array to turn on.
+					 */
+					val |= readl(qproc->reg_base + mem_pwr_ctl);
+					udelay(1);
+				}
 			}
 		} else {
 			/* Turn on memories */
@@ -2410,6 +2435,41 @@ static const struct rproc_hexagon_res msm8996_mss = {
 	.version = MSS_MSM8996,
 };
 
+static const struct rproc_hexagon_res mdm9607_mss = {
+	.hexagon_mba_image = "mba.mbn",
+	.proxy_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "pll",
+			.uA = 100000,
+		},
+		{}
+	},
+	.proxy_clk_names = (char*[]){
+		"xo",
+		NULL
+	},
+	.active_clk_names = (char*[]){
+		"iface",
+		"bus",
+		"mem",
+		NULL
+	},
+	.proxy_pd_names = (char*[]){
+		"mx",
+		"cx",
+		NULL
+	},
+	.need_mem_protection = false,
+	.has_alt_reset = false,
+	.has_mba_logs = false,
+	.has_spare_reg = false,
+	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
+	.has_ext_cntl_regs = false,
+	.has_vq6 = false,
+	.version = MSS_MDM9607,
+};
+
 static const struct rproc_hexagon_res msm8909_mss = {
 	.hexagon_mba_image = "mba.mbn",
 	.proxy_supply = (struct qcom_mss_reg_res[]) {
@@ -2672,6 +2732,7 @@ static const struct rproc_hexagon_res msm8926_mss = {
 
 static const struct of_device_id q6v5_of_match[] = {
 	{ .compatible = "qcom,q6v5-pil", .data = &msm8916_mss},
+	{ .compatible = "qcom,mdm9607-mss-pil", .data = &mdm9607_mss},
 	{ .compatible = "qcom,msm8226-mss-pil", .data = &msm8226_mss},
 	{ .compatible = "qcom,msm8909-mss-pil", .data = &msm8909_mss},
 	{ .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},

-- 
2.52.0

[PATCH v3 4/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8917

[Barnabás Czémán]

Add the compatible for MSS as found on the MSM8917 platform.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 .../devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml         | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
index 4e0d2fe0e46c..74202dd34703 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
@@ -21,6 +21,7 @@ properties:
           - qcom,msm8226-mss-pil
           - qcom,msm8909-mss-pil
           - qcom,msm8916-mss-pil
+          - qcom,msm8917-mss-pil
           - qcom,msm8926-mss-pil
           - qcom,msm8953-mss-pil
           - qcom,msm8974-mss-pil
@@ -90,7 +91,7 @@ properties:
     description: PLL proxy supply (control handed over after startup)
 
   mss-supply:
-    description: MSS power domain supply (only valid for qcom,msm8974-mss-pil)
+    description: MSS power domain supply
 
   resets:
     items:
@@ -230,6 +231,7 @@ allOf:
               - qcom,mdm9607-mss-pil
               - qcom,msm8909-mss-pil
               - qcom,msm8916-mss-pil
+              - qcom,msm8917-mss-pil
     then:
       properties:
         power-domains:
@@ -273,6 +275,7 @@ allOf:
           contains:
             enum:
               - qcom,msm8926-mss-pil
+              - qcom,msm8917-mss-pil
               - qcom,msm8974-mss-pil
     then:
       required:

-- 
2.52.0

[PATCH v3 5/9] remoteproc: qcom_q6v5_mss: Add MSM8917

[Barnabás Czémán]

Add support for MSM8917 MSS it is similar for MDM9607 MSS
only difference is the mss supply.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/remoteproc/qcom_q6v5_mss.c | 54 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 19863c576d72..a36447a92274 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -261,6 +261,7 @@ enum {
 	MSS_MSM8226,
 	MSS_MSM8909,
 	MSS_MSM8916,
+	MSS_MSM8917,
 	MSS_MSM8926,
 	MSS_MSM8953,
 	MSS_MSM8974,
@@ -751,13 +752,15 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 		goto pbl_wait;
 	} else if (qproc->version == MSS_MDM9607 ||
 		   qproc->version == MSS_MSM8909 ||
+		   qproc->version == MSS_MSM8917 ||
 		   qproc->version == MSS_MSM8953 ||
 		   qproc->version == MSS_MSM8996 ||
 		   qproc->version == MSS_MSM8998 ||
 		   qproc->version == MSS_SDM660) {
 
 		/* Override the ACC value if required */
-		if (qproc->version == MSS_MDM9607)
+		if (qproc->version == MSS_MDM9607 ||
+		    qproc->version == MSS_MSM8917)
 			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
 			       qproc->reg_base + QDSP6SS_STRAP_ACC);
 		else if (qproc->version != MSS_MSM8909 &&
@@ -819,6 +822,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 
 			/* Turn on L1, L2, ETB and JU memories 1 at a time */
 			if (qproc->version == MSS_MDM9607 ||
+			    qproc->version == MSS_MSM8917 ||
 			    qproc->version == MSS_MSM8953 ||
 			    qproc->version == MSS_MSM8996) {
 				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
@@ -829,7 +833,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 				i = 28;
 			}
 			val = readl(qproc->reg_base + mem_pwr_ctl);
-			if (qproc->version == MSS_MDM9607) {
+			if (qproc->version == MSS_MDM9607 ||
+			    qproc->version == MSS_MSM8917) {
 				/*
 				 * Set first 5 bits in reverse to avoid
 				 * "inrush current" issues.
@@ -2553,6 +2558,50 @@ static const struct rproc_hexagon_res msm8916_mss = {
 	.version = MSS_MSM8916,
 };
 
+static const struct rproc_hexagon_res msm8917_mss = {
+	.hexagon_mba_image = "mba.mbn",
+	.proxy_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "pll",
+			.uA = 100000,
+		},
+		{}
+	},
+	.active_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "mss",
+			.uV = 1050000,
+			.uA = 100000,
+		},
+		{}
+	},
+	.proxy_clk_names = (char*[]){
+		"xo",
+		NULL
+	},
+	.active_clk_names = (char*[]){
+		"iface",
+		"bus",
+		"mem",
+		NULL
+	},
+	.proxy_pd_names = (char*[]) {
+		"cx",
+		"mx",
+		NULL
+	},
+	.need_mem_protection = false,
+	.need_pas_mem_setup = false,
+	.has_alt_reset = false,
+	.has_mba_logs = false,
+	.has_spare_reg = false,
+	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
+	.has_ext_cntl_regs = false,
+	.has_vq6 = false,
+	.version = MSS_MSM8917,
+};
+
 static const struct rproc_hexagon_res msm8953_mss = {
 	.hexagon_mba_image = "mba.mbn",
 	.proxy_supply = (struct qcom_mss_reg_res[]) {
@@ -2736,6 +2785,7 @@ static const struct of_device_id q6v5_of_match[] = {
 	{ .compatible = "qcom,msm8226-mss-pil", .data = &msm8226_mss},
 	{ .compatible = "qcom,msm8909-mss-pil", .data = &msm8909_mss},
 	{ .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
+	{ .compatible = "qcom,msm8917-mss-pil", .data = &msm8917_mss},
 	{ .compatible = "qcom,msm8926-mss-pil", .data = &msm8926_mss},
 	{ .compatible = "qcom,msm8953-mss-pil", .data = &msm8953_mss},
 	{ .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss},

-- 
2.52.0

[PATCH v3 6/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8937

[Barnabás Czémán]

Add the compatible for MSS as found on the MSM8937 platform.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
index 74202dd34703..b4a1b5852896 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
@@ -23,6 +23,7 @@ properties:
           - qcom,msm8916-mss-pil
           - qcom,msm8917-mss-pil
           - qcom,msm8926-mss-pil
+          - qcom,msm8937-mss-pil
           - qcom,msm8953-mss-pil
           - qcom,msm8974-mss-pil
 
@@ -232,6 +233,7 @@ allOf:
               - qcom,msm8909-mss-pil
               - qcom,msm8916-mss-pil
               - qcom,msm8917-mss-pil
+              - qcom,msm8937-mss-pil
     then:
       properties:
         power-domains:
@@ -276,6 +278,7 @@ allOf:
             enum:
               - qcom,msm8926-mss-pil
               - qcom,msm8917-mss-pil
+              - qcom,msm8937-mss-pil
               - qcom,msm8974-mss-pil
     then:
       required:

-- 
2.52.0

[PATCH v3 7/9] remoteproc: qcom_q6v5_mss: Add MSM8937

[Barnabás Czémán]

Add support for MSM8937 MSS it is similar to MSM8917 MSS.
It differs primarily in that TZ needs to be informed of
the modem start address and pas_id.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/remoteproc/qcom_q6v5_mss.c | 54 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 52 insertions(+), 2 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index a36447a92274..390b35b5a69d 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -263,6 +263,7 @@ enum {
 	MSS_MSM8916,
 	MSS_MSM8917,
 	MSS_MSM8926,
+	MSS_MSM8937,
 	MSS_MSM8953,
 	MSS_MSM8974,
 	MSS_MSM8996,
@@ -753,6 +754,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 	} else if (qproc->version == MSS_MDM9607 ||
 		   qproc->version == MSS_MSM8909 ||
 		   qproc->version == MSS_MSM8917 ||
+		   qproc->version == MSS_MSM8937 ||
 		   qproc->version == MSS_MSM8953 ||
 		   qproc->version == MSS_MSM8996 ||
 		   qproc->version == MSS_MSM8998 ||
@@ -760,7 +762,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 
 		/* Override the ACC value if required */
 		if (qproc->version == MSS_MDM9607 ||
-		    qproc->version == MSS_MSM8917)
+		    qproc->version == MSS_MSM8917 ||
+		    qproc->version == MSS_MSM8937)
 			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
 			       qproc->reg_base + QDSP6SS_STRAP_ACC);
 		else if (qproc->version != MSS_MSM8909 &&
@@ -823,6 +826,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 			/* Turn on L1, L2, ETB and JU memories 1 at a time */
 			if (qproc->version == MSS_MDM9607 ||
 			    qproc->version == MSS_MSM8917 ||
+			    qproc->version == MSS_MSM8937 ||
 			    qproc->version == MSS_MSM8953 ||
 			    qproc->version == MSS_MSM8996) {
 				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
@@ -834,7 +838,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 			}
 			val = readl(qproc->reg_base + mem_pwr_ctl);
 			if (qproc->version == MSS_MDM9607 ||
-			    qproc->version == MSS_MSM8917) {
+			    qproc->version == MSS_MSM8917 ||
+			    qproc->version == MSS_MSM8937) {
 				/*
 				 * Set first 5 bits in reverse to avoid
 				 * "inrush current" issues.
@@ -2602,6 +2607,50 @@ static const struct rproc_hexagon_res msm8917_mss = {
 	.version = MSS_MSM8917,
 };
 
+static const struct rproc_hexagon_res msm8937_mss = {
+	.hexagon_mba_image = "mba.mbn",
+	.proxy_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "pll",
+			.uA = 100000,
+		},
+		{}
+	},
+	.active_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "mss",
+			.uV = 1050000,
+			.uA = 100000,
+		},
+		{}
+	},
+	.proxy_clk_names = (char*[]){
+		"xo",
+		NULL
+	},
+	.active_clk_names = (char*[]){
+		"iface",
+		"bus",
+		"mem",
+		NULL
+	},
+	.proxy_pd_names = (char*[]) {
+		"cx",
+		"mx",
+		NULL
+	},
+	.need_mem_protection = false,
+	.need_pas_mem_setup = true,
+	.has_alt_reset = false,
+	.has_mba_logs = false,
+	.has_spare_reg = false,
+	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
+	.has_ext_cntl_regs = false,
+	.has_vq6 = false,
+	.version = MSS_MSM8937,
+};
+
 static const struct rproc_hexagon_res msm8953_mss = {
 	.hexagon_mba_image = "mba.mbn",
 	.proxy_supply = (struct qcom_mss_reg_res[]) {
@@ -2787,6 +2836,7 @@ static const struct of_device_id q6v5_of_match[] = {
 	{ .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
 	{ .compatible = "qcom,msm8917-mss-pil", .data = &msm8917_mss},
 	{ .compatible = "qcom,msm8926-mss-pil", .data = &msm8926_mss},
+	{ .compatible = "qcom,msm8937-mss-pil", .data = &msm8937_mss},
 	{ .compatible = "qcom,msm8953-mss-pil", .data = &msm8953_mss},
 	{ .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss},
 	{ .compatible = "qcom,msm8996-mss-pil", .data = &msm8996_mss},

-- 
2.52.0

[PATCH v3 8/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8940

[Barnabás Czémán]

Add the compatible for MSS as found on the MSM8940 platform.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
index b4a1b5852896..8c0ff4dfad10 100644
--- a/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
+++ b/Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml
@@ -24,6 +24,7 @@ properties:
           - qcom,msm8917-mss-pil
           - qcom,msm8926-mss-pil
           - qcom,msm8937-mss-pil
+          - qcom,msm8940-mss-pil
           - qcom,msm8953-mss-pil
           - qcom,msm8974-mss-pil
 
@@ -234,6 +235,7 @@ allOf:
               - qcom,msm8916-mss-pil
               - qcom,msm8917-mss-pil
               - qcom,msm8937-mss-pil
+              - qcom,msm8940-mss-pil
     then:
       properties:
         power-domains:
@@ -279,6 +281,7 @@ allOf:
               - qcom,msm8926-mss-pil
               - qcom,msm8917-mss-pil
               - qcom,msm8937-mss-pil
+              - qcom,msm8940-mss-pil
               - qcom,msm8974-mss-pil
     then:
       required:

-- 
2.52.0

[PATCH v3 9/9] remoteproc: qcom_q6v5_mss: Add MSM8940

[Barnabás Czémán]

Add support for MSM8940 MSS it is similar for MSM8937 MSS
without inrush current mitigation.

Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
---
 drivers/remoteproc/qcom_q6v5_mss.c | 51 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 390b35b5a69d..fe3a8e2208f4 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -264,6 +264,7 @@ enum {
 	MSS_MSM8917,
 	MSS_MSM8926,
 	MSS_MSM8937,
+	MSS_MSM8940,
 	MSS_MSM8953,
 	MSS_MSM8974,
 	MSS_MSM8996,
@@ -755,6 +756,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 		   qproc->version == MSS_MSM8909 ||
 		   qproc->version == MSS_MSM8917 ||
 		   qproc->version == MSS_MSM8937 ||
+		   qproc->version == MSS_MSM8940 ||
 		   qproc->version == MSS_MSM8953 ||
 		   qproc->version == MSS_MSM8996 ||
 		   qproc->version == MSS_MSM8998 ||
@@ -763,7 +765,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 		/* Override the ACC value if required */
 		if (qproc->version == MSS_MDM9607 ||
 		    qproc->version == MSS_MSM8917 ||
-		    qproc->version == MSS_MSM8937)
+		    qproc->version == MSS_MSM8937 ||
+		    qproc->version == MSS_MSM8940)
 			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
 			       qproc->reg_base + QDSP6SS_STRAP_ACC);
 		else if (qproc->version != MSS_MSM8909 &&
@@ -827,6 +830,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
 			if (qproc->version == MSS_MDM9607 ||
 			    qproc->version == MSS_MSM8917 ||
 			    qproc->version == MSS_MSM8937 ||
+			    qproc->version == MSS_MSM8940 ||
 			    qproc->version == MSS_MSM8953 ||
 			    qproc->version == MSS_MSM8996) {
 				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
@@ -2651,6 +2655,50 @@ static const struct rproc_hexagon_res msm8937_mss = {
 	.version = MSS_MSM8937,
 };
 
+static const struct rproc_hexagon_res msm8940_mss = {
+	.hexagon_mba_image = "mba.mbn",
+	.proxy_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "pll",
+			.uA = 100000,
+		},
+		{}
+	},
+	.active_supply = (struct qcom_mss_reg_res[]) {
+		{
+			.supply = "mss",
+			.uV = 1050000,
+			.uA = 100000,
+		},
+		{}
+	},
+	.proxy_clk_names = (char*[]){
+		"xo",
+		NULL
+	},
+	.active_clk_names = (char*[]){
+		"iface",
+		"bus",
+		"mem",
+		NULL
+	},
+	.proxy_pd_names = (char*[]) {
+		"cx",
+		"mx",
+		NULL
+	},
+	.need_mem_protection = false,
+	.need_pas_mem_setup = true,
+	.has_alt_reset = false,
+	.has_mba_logs = false,
+	.has_spare_reg = false,
+	.has_qaccept_regs = false,
+	.has_ext_bhs_reg = false,
+	.has_ext_cntl_regs = false,
+	.has_vq6 = false,
+	.version = MSS_MSM8940,
+};
+
 static const struct rproc_hexagon_res msm8953_mss = {
 	.hexagon_mba_image = "mba.mbn",
 	.proxy_supply = (struct qcom_mss_reg_res[]) {
@@ -2837,6 +2885,7 @@ static const struct of_device_id q6v5_of_match[] = {
 	{ .compatible = "qcom,msm8917-mss-pil", .data = &msm8917_mss},
 	{ .compatible = "qcom,msm8926-mss-pil", .data = &msm8926_mss},
 	{ .compatible = "qcom,msm8937-mss-pil", .data = &msm8937_mss},
+	{ .compatible = "qcom,msm8940-mss-pil", .data = &msm8940_mss},
 	{ .compatible = "qcom,msm8953-mss-pil", .data = &msm8953_mss},
 	{ .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss},
 	{ .compatible = "qcom,msm8996-mss-pil", .data = &msm8996_mss},

-- 
2.52.0

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Bryan O'Donoghue]

On 31/12/2025 16:30, Barnabás Czémán wrote:
> From: Stephan Gerhold <stephan@gerhold.net>
> 
> Add support for MDM9607 MSS it have different ACC settings
> and it needs mitigation for inrush current issue.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> [Reword the commit, add necessary flags, rework inrush current mitigation]
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
>   drivers/remoteproc/qcom_q6v5_mss.c | 89 ++++++++++++++++++++++++++++++++------
>   1 file changed, 75 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 3c404118b322..19863c576d72 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -124,6 +124,7 @@
>   #define QDSP6v56_CLAMP_QMC_MEM		BIT(22)
>   #define QDSP6SS_XO_CBCR		0x0038
>   #define QDSP6SS_ACC_OVERRIDE_VAL		0x20
> +#define QDSP6SS_ACC_OVERRIDE_VAL_9607	0x80800000
>   #define QDSP6v55_BHS_EN_REST_ACK	BIT(0)
> 
>   /* QDSP6v65 parameters */
> @@ -256,6 +257,7 @@ struct q6v5 {
>   };
> 
>   enum {
> +	MSS_MDM9607,
>   	MSS_MSM8226,
>   	MSS_MSM8909,
>   	MSS_MSM8916,
> @@ -747,15 +749,19 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>   			return ret;
>   		}
>   		goto pbl_wait;
> -	} else if (qproc->version == MSS_MSM8909 ||
> +	} else if (qproc->version == MSS_MDM9607 ||
> +		   qproc->version == MSS_MSM8909 ||
>   		   qproc->version == MSS_MSM8953 ||
>   		   qproc->version == MSS_MSM8996 ||
>   		   qproc->version == MSS_MSM8998 ||
>   		   qproc->version == MSS_SDM660) {
> 
> -		if (qproc->version != MSS_MSM8909 &&
> -		    qproc->version != MSS_MSM8953)
> -			/* Override the ACC value if required */
> +		/* Override the ACC value if required */
> +		if (qproc->version == MSS_MDM9607)
> +			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
> +			       qproc->reg_base + QDSP6SS_STRAP_ACC);
> +		else if (qproc->version != MSS_MSM8909 &&
> +			 qproc->version != MSS_MSM8953)
>   			writel(QDSP6SS_ACC_OVERRIDE_VAL,
>   			       qproc->reg_base + QDSP6SS_STRAP_ACC);
> 
> @@ -800,7 +806,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>   		writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> 
>   		if (qproc->version != MSS_MSM8909) {
> -			int mem_pwr_ctl;
> +			int mem_pwr_ctl, reverse = 0;
> 
>   			/* Deassert QDSP6 compiler memory clamp */
>   			val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> @@ -812,7 +818,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>   			writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> 
>   			/* Turn on L1, L2, ETB and JU memories 1 at a time */
> -			if (qproc->version == MSS_MSM8953 ||
> +			if (qproc->version == MSS_MDM9607 ||
> +			    qproc->version == MSS_MSM8953 ||
>   			    qproc->version == MSS_MSM8996) {
>   				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
>   				i = 19;
> @@ -822,16 +829,34 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>   				i = 28;
>   			}
>   			val = readl(qproc->reg_base + mem_pwr_ctl);
> -			for (; i >= 0; i--) {
> -				val |= BIT(i);
> -				writel(val, qproc->reg_base + mem_pwr_ctl);
> +			if (qproc->version == MSS_MDM9607) {
>   				/*
> -				 * Read back value to ensure the write is done then
> -				 * wait for 1us for both memory peripheral and data
> -				 * array to turn on.
> +				 * Set first 5 bits in reverse to avoid
> +				 * "inrush current" issues.
>   				 */
> -				val |= readl(qproc->reg_base + mem_pwr_ctl);
> -				udelay(1);
> +				reverse = 6;
> +				for (; i >= reverse; i--) {
> +					val |= BIT(i);
> +					writel(val, qproc->reg_base + mem_pwr_ctl);
> +					udelay(1);
> +				}
> +				for (i = 0; i < reverse; i++) {
> +					val |= BIT(i);
> +					writel(val, qproc->reg_base + mem_pwr_ctl);
> +					udelay(1);
> +				}
> +			} else {
> +				for (; i >= 0; i--) {
> +					val |= BIT(i);
> +					writel(val, qproc->reg_base + mem_pwr_ctl);
> +					/*
> +					 * Read back value to ensure the write is done then
> +					 * wait for 1us for both memory peripheral and data
> +					 * array to turn on.
> +					 */
> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
> +					udelay(1);

Isn't the logic here inverted ?

i.e. you've written a thing and ostensibly require a delay for that 
thing to take effect, the power to switch on in this case.

It makes more sense to write, delay and read back rather than write, 
readback and delay surely...


> +				}
>   			}
>   		} else {
>   			/* Turn on memories */
> @@ -2410,6 +2435,41 @@ static const struct rproc_hexagon_res msm8996_mss = {
>   	.version = MSS_MSM8996,
>   };
> 
> +static const struct rproc_hexagon_res mdm9607_mss = {
> +	.hexagon_mba_image = "mba.mbn",
> +	.proxy_supply = (struct qcom_mss_reg_res[]) {
> +		{
> +			.supply = "pll",
> +			.uA = 100000,
> +		},
> +		{}
> +	},
> +	.proxy_clk_names = (char*[]){
> +		"xo",
> +		NULL
> +	},
> +	.active_clk_names = (char*[]){
> +		"iface",
> +		"bus",
> +		"mem",
> +		NULL
> +	},
> +	.proxy_pd_names = (char*[]){
> +		"mx",
> +		"cx",
> +		NULL
> +	},
> +	.need_mem_protection = false,
> +	.has_alt_reset = false,
> +	.has_mba_logs = false,
> +	.has_spare_reg = false,
> +	.has_qaccept_regs = false,
> +	.has_ext_bhs_reg = false,
> +	.has_ext_cntl_regs = false,
> +	.has_vq6 = false,
> +	.version = MSS_MDM9607,
> +};
> +
>   static const struct rproc_hexagon_res msm8909_mss = {
>   	.hexagon_mba_image = "mba.mbn",
>   	.proxy_supply = (struct qcom_mss_reg_res[]) {
> @@ -2672,6 +2732,7 @@ static const struct rproc_hexagon_res msm8926_mss = {
> 
>   static const struct of_device_id q6v5_of_match[] = {
>   	{ .compatible = "qcom,q6v5-pil", .data = &msm8916_mss},
> +	{ .compatible = "qcom,mdm9607-mss-pil", .data = &mdm9607_mss},
>   	{ .compatible = "qcom,msm8226-mss-pil", .data = &msm8226_mss},
>   	{ .compatible = "qcom,msm8909-mss-pil", .data = &msm8909_mss},
>   	{ .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
> 
> --
> 2.52.0
> 
> 

Re: [PATCH v3 1/9] remoteproc: qcom_q6v5_mss: Introduce need_pas_mem_setup

[Bryan O'Donoghue]

On 31/12/2025 16:30, Barnabás Czémán wrote:
> Some platforms like MSM8953 and MSM8937 TZ needs to be
> informed of the modem start address and pas_id.
> Lets introduce need_pas_mem_setup flag for handle this case.
> 
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
>   drivers/remoteproc/qcom_q6v5_mss.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 91940977ca89..3c404118b322 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -162,6 +162,7 @@ struct rproc_hexagon_res {
>   	char **proxy_pd_names;
>   	int version;
>   	bool need_mem_protection;
> +	bool need_pas_mem_setup;
>   	bool has_alt_reset;
>   	bool has_mba_logs;
>   	bool has_spare_reg;
> @@ -240,6 +241,7 @@ struct q6v5 {
>   	struct qcom_sysmon *sysmon;
>   	struct platform_device *bam_dmux;
>   	bool need_mem_protection;
> +	bool need_pas_mem_setup;
>   	bool has_alt_reset;
>   	bool has_mba_logs;
>   	bool has_spare_reg;
> @@ -1441,7 +1443,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
>   			max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
>   	}
> 
> -	if (qproc->version == MSS_MSM8953) {
> +	if (qproc->need_pas_mem_setup) {
>   		ret = qcom_scm_pas_mem_setup(MPSS_PAS_ID, qproc->mpss_phys, qproc->mpss_size);
>   		if (ret) {
>   			dev_err(qproc->dev,
> @@ -2224,6 +2226,7 @@ static const struct rproc_hexagon_res sc7180_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = true,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = true,
>   	.has_spare_reg = true,
> @@ -2253,6 +2256,7 @@ static const struct rproc_hexagon_res sc7280_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = true,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = true,
>   	.has_spare_reg = false,
> @@ -2285,6 +2289,7 @@ static const struct rproc_hexagon_res sdm660_mss = {
>   			NULL
>   	},
>   	.need_mem_protection = true,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2321,6 +2326,7 @@ static const struct rproc_hexagon_res sdm845_mss = {
>   			NULL
>   	},
>   	.need_mem_protection = true,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = true,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2353,6 +2359,7 @@ static const struct rproc_hexagon_res msm8998_mss = {
>   			NULL
>   	},
>   	.need_mem_protection = true,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2392,6 +2399,7 @@ static const struct rproc_hexagon_res msm8996_mss = {
>   			NULL
>   	},
>   	.need_mem_protection = true,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2427,6 +2435,7 @@ static const struct rproc_hexagon_res msm8909_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = false,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2473,6 +2482,7 @@ static const struct rproc_hexagon_res msm8916_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = false,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2509,6 +2519,7 @@ static const struct rproc_hexagon_res msm8953_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = false,
> +	.need_pas_mem_setup = true,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2562,6 +2573,7 @@ static const struct rproc_hexagon_res msm8974_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = false,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2600,6 +2612,7 @@ static const struct rproc_hexagon_res msm8226_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = false,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> @@ -2646,6 +2659,7 @@ static const struct rproc_hexagon_res msm8926_mss = {
>   		NULL
>   	},
>   	.need_mem_protection = false,
> +	.need_pas_mem_setup = false,
>   	.has_alt_reset = false,
>   	.has_mba_logs = false,
>   	.has_spare_reg = false,
> 
> --
> 2.52.0
> 
> 

Reviewed-by: Bryan O'Donoghue <bod@kernel.org>

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Bryan O'Donoghue]

On 01/01/2026 13:13, Bryan O'Donoghue wrote:
> On 31/12/2025 16:30, Barnabás Czémán wrote:
>> From: Stephan Gerhold <stephan@gerhold.net>
>>
>> Add support for MDM9607 MSS it have different ACC settings
>> and it needs mitigation for inrush current issue.
>>
>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>> [Reword the commit, add necessary flags, rework inrush current mitigation]
>> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
>> ---
>>    drivers/remoteproc/qcom_q6v5_mss.c | 89 ++++++++++++++++++++++++++++++++------
>>    1 file changed, 75 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
>> index 3c404118b322..19863c576d72 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>> @@ -124,6 +124,7 @@
>>    #define QDSP6v56_CLAMP_QMC_MEM		BIT(22)
>>    #define QDSP6SS_XO_CBCR		0x0038
>>    #define QDSP6SS_ACC_OVERRIDE_VAL		0x20
>> +#define QDSP6SS_ACC_OVERRIDE_VAL_9607	0x80800000
>>    #define QDSP6v55_BHS_EN_REST_ACK	BIT(0)
>>
>>    /* QDSP6v65 parameters */
>> @@ -256,6 +257,7 @@ struct q6v5 {
>>    };
>>
>>    enum {
>> +	MSS_MDM9607,
>>    	MSS_MSM8226,
>>    	MSS_MSM8909,
>>    	MSS_MSM8916,
>> @@ -747,15 +749,19 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>    			return ret;
>>    		}
>>    		goto pbl_wait;
>> -	} else if (qproc->version == MSS_MSM8909 ||
>> +	} else if (qproc->version == MSS_MDM9607 ||
>> +		   qproc->version == MSS_MSM8909 ||
>>    		   qproc->version == MSS_MSM8953 ||
>>    		   qproc->version == MSS_MSM8996 ||
>>    		   qproc->version == MSS_MSM8998 ||
>>    		   qproc->version == MSS_SDM660) {
>>
>> -		if (qproc->version != MSS_MSM8909 &&
>> -		    qproc->version != MSS_MSM8953)
>> -			/* Override the ACC value if required */
>> +		/* Override the ACC value if required */
>> +		if (qproc->version == MSS_MDM9607)
>> +			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
>> +			       qproc->reg_base + QDSP6SS_STRAP_ACC);
>> +		else if (qproc->version != MSS_MSM8909 &&
>> +			 qproc->version != MSS_MSM8953)
>>    			writel(QDSP6SS_ACC_OVERRIDE_VAL,
>>    			       qproc->reg_base + QDSP6SS_STRAP_ACC);
>>
>> @@ -800,7 +806,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>    		writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>>
>>    		if (qproc->version != MSS_MSM8909) {
>> -			int mem_pwr_ctl;
>> +			int mem_pwr_ctl, reverse = 0;
>>
>>    			/* Deassert QDSP6 compiler memory clamp */
>>    			val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> @@ -812,7 +818,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>    			writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>>
>>    			/* Turn on L1, L2, ETB and JU memories 1 at a time */
>> -			if (qproc->version == MSS_MSM8953 ||
>> +			if (qproc->version == MSS_MDM9607 ||
>> +			    qproc->version == MSS_MSM8953 ||
>>    			    qproc->version == MSS_MSM8996) {
>>    				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
>>    				i = 19;
>> @@ -822,16 +829,34 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>    				i = 28;
>>    			}
>>    			val = readl(qproc->reg_base + mem_pwr_ctl);
>> -			for (; i >= 0; i--) {
>> -				val |= BIT(i);
>> -				writel(val, qproc->reg_base + mem_pwr_ctl);
>> +			if (qproc->version == MSS_MDM9607) {
>>    				/*
>> -				 * Read back value to ensure the write is done then
>> -				 * wait for 1us for both memory peripheral and data
>> -				 * array to turn on.
>> +				 * Set first 5 bits in reverse to avoid
>> +				 * "inrush current" issues.
>>    				 */
>> -				val |= readl(qproc->reg_base + mem_pwr_ctl);
>> -				udelay(1);
>> +				reverse = 6;
>> +				for (; i >= reverse; i--) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
>> +				}
>> +				for (i = 0; i < reverse; i++) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
>> +				}
>> +			} else {
>> +				for (; i >= 0; i--) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					/*
>> +					 * Read back value to ensure the write is done then
>> +					 * wait for 1us for both memory peripheral and data
>> +					 * array to turn on.
>> +					 */
>> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
> 
> Isn't the logic here inverted ?
> 
> i.e. you've written a thing and ostensibly require a delay for that
> thing to take effect, the power to switch on in this case.
> 
> It makes more sense to write, delay and read back rather than write,
> readback and delay surely...
> 
Also now that I look at this, shouldn't you interrogate the bit gets set 
as per your write ?

Does this bit mean "yes I acknowledge your request" or "yes I carried 
out your request" ?

In the first case you care that the bit indicates something useful, in 
the second case it barely indicates anything at all.

---
bod

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Bryan O'Donoghue]

On 01/01/2026 13:19, Bryan O'Donoghue wrote:
> In the first case you care that the bit indicates something useful, in
> the second case it barely indicates anything at all.

Swap these bits, the first case is an acknowledgement of receiving a 
command the second case is a status bit from carrying out the command.

Either way it seems a bit mindless to just or back in whatever bit you 
read back here... in fact shouldn't val be _equal_ to the value - 
instead of or-ing the value in ?

I think it probably should and I also think if you are error checking 
you should check your flagged bit actually appears in the read-back dword.

---
bod

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[barnabas.czeman]

On 2026-01-01 14:13, Bryan O'Donoghue wrote:
> On 31/12/2025 16:30, Barnabás Czémán wrote:
>> From: Stephan Gerhold <stephan@gerhold.net>
>> 
>> Add support for MDM9607 MSS it have different ACC settings
>> and it needs mitigation for inrush current issue.
>> 
>> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>> [Reword the commit, add necessary flags, rework inrush current 
>> mitigation]
>> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
>> ---
>>   drivers/remoteproc/qcom_q6v5_mss.c | 89 
>> ++++++++++++++++++++++++++++++++------
>>   1 file changed, 75 insertions(+), 14 deletions(-)
>> 
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c 
>> b/drivers/remoteproc/qcom_q6v5_mss.c
>> index 3c404118b322..19863c576d72 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>> @@ -124,6 +124,7 @@
>>   #define QDSP6v56_CLAMP_QMC_MEM		BIT(22)
>>   #define QDSP6SS_XO_CBCR		0x0038
>>   #define QDSP6SS_ACC_OVERRIDE_VAL		0x20
>> +#define QDSP6SS_ACC_OVERRIDE_VAL_9607	0x80800000
>>   #define QDSP6v55_BHS_EN_REST_ACK	BIT(0)
>> 
>>   /* QDSP6v65 parameters */
>> @@ -256,6 +257,7 @@ struct q6v5 {
>>   };
>> 
>>   enum {
>> +	MSS_MDM9607,
>>   	MSS_MSM8226,
>>   	MSS_MSM8909,
>>   	MSS_MSM8916,
>> @@ -747,15 +749,19 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   			return ret;
>>   		}
>>   		goto pbl_wait;
>> -	} else if (qproc->version == MSS_MSM8909 ||
>> +	} else if (qproc->version == MSS_MDM9607 ||
>> +		   qproc->version == MSS_MSM8909 ||
>>   		   qproc->version == MSS_MSM8953 ||
>>   		   qproc->version == MSS_MSM8996 ||
>>   		   qproc->version == MSS_MSM8998 ||
>>   		   qproc->version == MSS_SDM660) {
>> 
>> -		if (qproc->version != MSS_MSM8909 &&
>> -		    qproc->version != MSS_MSM8953)
>> -			/* Override the ACC value if required */
>> +		/* Override the ACC value if required */
>> +		if (qproc->version == MSS_MDM9607)
>> +			writel(QDSP6SS_ACC_OVERRIDE_VAL_9607,
>> +			       qproc->reg_base + QDSP6SS_STRAP_ACC);
>> +		else if (qproc->version != MSS_MSM8909 &&
>> +			 qproc->version != MSS_MSM8953)
>>   			writel(QDSP6SS_ACC_OVERRIDE_VAL,
>>   			       qproc->reg_base + QDSP6SS_STRAP_ACC);
>> 
>> @@ -800,7 +806,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   		writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> 
>>   		if (qproc->version != MSS_MSM8909) {
>> -			int mem_pwr_ctl;
>> +			int mem_pwr_ctl, reverse = 0;
>> 
>>   			/* Deassert QDSP6 compiler memory clamp */
>>   			val = readl(qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> @@ -812,7 +818,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   			writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
>> 
>>   			/* Turn on L1, L2, ETB and JU memories 1 at a time */
>> -			if (qproc->version == MSS_MSM8953 ||
>> +			if (qproc->version == MSS_MDM9607 ||
>> +			    qproc->version == MSS_MSM8953 ||
>>   			    qproc->version == MSS_MSM8996) {
>>   				mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
>>   				i = 19;
>> @@ -822,16 +829,34 @@ static int q6v5proc_reset(struct q6v5 *qproc)
>>   				i = 28;
>>   			}
>>   			val = readl(qproc->reg_base + mem_pwr_ctl);
>> -			for (; i >= 0; i--) {
>> -				val |= BIT(i);
>> -				writel(val, qproc->reg_base + mem_pwr_ctl);
>> +			if (qproc->version == MSS_MDM9607) {
>>   				/*
>> -				 * Read back value to ensure the write is done then
>> -				 * wait for 1us for both memory peripheral and data
>> -				 * array to turn on.
>> +				 * Set first 5 bits in reverse to avoid
>> +				 * "inrush current" issues.
>>   				 */
>> -				val |= readl(qproc->reg_base + mem_pwr_ctl);
>> -				udelay(1);
>> +				reverse = 6;
>> +				for (; i >= reverse; i--) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
>> +				}
>> +				for (i = 0; i < reverse; i++) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
>> +				}
>> +			} else {
>> +				for (; i >= 0; i--) {
>> +					val |= BIT(i);
>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>> +					/*
>> +					 * Read back value to ensure the write is done then
>> +					 * wait for 1us for both memory peripheral and data
>> +					 * array to turn on.
>> +					 */
>> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
>> +					udelay(1);
> 
> Isn't the logic here inverted ?
> 
> i.e. you've written a thing and ostensibly require a delay for that 
> thing to take effect, the power to switch on in this case.
> 
> It makes more sense to write, delay and read back rather than write, 
> readback and delay surely...
This is the original reset sequence without modification, i have just 
moved it in a else case when it is not an MDM9607, MSM8917 or MSM8937.
> 
> 
>> +				}
>>   			}
>>   		} else {
>>   			/* Turn on memories */
>> @@ -2410,6 +2435,41 @@ static const struct rproc_hexagon_res 
>> msm8996_mss = {
>>   	.version = MSS_MSM8996,
>>   };
>> 
>> +static const struct rproc_hexagon_res mdm9607_mss = {
>> +	.hexagon_mba_image = "mba.mbn",
>> +	.proxy_supply = (struct qcom_mss_reg_res[]) {
>> +		{
>> +			.supply = "pll",
>> +			.uA = 100000,
>> +		},
>> +		{}
>> +	},
>> +	.proxy_clk_names = (char*[]){
>> +		"xo",
>> +		NULL
>> +	},
>> +	.active_clk_names = (char*[]){
>> +		"iface",
>> +		"bus",
>> +		"mem",
>> +		NULL
>> +	},
>> +	.proxy_pd_names = (char*[]){
>> +		"mx",
>> +		"cx",
>> +		NULL
>> +	},
>> +	.need_mem_protection = false,
>> +	.has_alt_reset = false,
>> +	.has_mba_logs = false,
>> +	.has_spare_reg = false,
>> +	.has_qaccept_regs = false,
>> +	.has_ext_bhs_reg = false,
>> +	.has_ext_cntl_regs = false,
>> +	.has_vq6 = false,
>> +	.version = MSS_MDM9607,
>> +};
>> +
>>   static const struct rproc_hexagon_res msm8909_mss = {
>>   	.hexagon_mba_image = "mba.mbn",
>>   	.proxy_supply = (struct qcom_mss_reg_res[]) {
>> @@ -2672,6 +2732,7 @@ static const struct rproc_hexagon_res 
>> msm8926_mss = {
>> 
>>   static const struct of_device_id q6v5_of_match[] = {
>>   	{ .compatible = "qcom,q6v5-pil", .data = &msm8916_mss},
>> +	{ .compatible = "qcom,mdm9607-mss-pil", .data = &mdm9607_mss},
>>   	{ .compatible = "qcom,msm8226-mss-pil", .data = &msm8226_mss},
>>   	{ .compatible = "qcom,msm8909-mss-pil", .data = &msm8909_mss},
>>   	{ .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
>> 
>> --
>> 2.52.0
>> 
>> 

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Bryan O'Donoghue]

On 01/01/2026 13:50, barnabas.czeman@mainlining.org wrote:
>>> +				for (; i >= 0; i--) {
>>> +					val |= BIT(i);
>>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>>> +					/*
>>> +					 * Read back value to ensure the write is done then
>>> +					 * wait for 1us for both memory peripheral and data
>>> +					 * array to turn on.
>>> +					 */
>>> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
>>> +					udelay(1);
>> Isn't the logic here inverted ?
>>
>> i.e. you've written a thing and ostensibly require a delay for that
>> thing to take effect, the power to switch on in this case.
>>
>> It makes more sense to write, delay and read back rather than write,
>> readback and delay surely...
> This is the original reset sequence without modification, i have just
> moved it in a else case when it is not an MDM9607, MSM8917 or MSM8937.

Doesn't make it correct, we fix upstream logic bugs all the time...

For example a read-back to ensure write completion is only required for 
posted memory transactions.

Is this a posted write ?

Is there an io-fabric in the world which exceeds 1 microsecond to 
perform a write transaction ?

Anyway leaving that aside the bit that's really objectionable and IMO 
obvious a bug is val |= readl();

Why or the bit back in ? and then why not check the bit was set on the 
read ?

val = readl() is a lot less janky and shouldn't it matter that the bit 
we tried to set is actually reflected in the read-back ?

Failure to set the bit would certainly be a problem...

---
bod

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[barnabas.czeman]

On 2026-01-01 21:58, Bryan O'Donoghue wrote:
> On 01/01/2026 13:50, barnabas.czeman@mainlining.org wrote:
>>>> +				for (; i >= 0; i--) {
>>>> +					val |= BIT(i);
>>>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>>>> +					/*
>>>> +					 * Read back value to ensure the write is done then
>>>> +					 * wait for 1us for both memory peripheral and data
>>>> +					 * array to turn on.
>>>> +					 */
>>>> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
>>>> +					udelay(1);
>>> Isn't the logic here inverted ?
>>> 
>>> i.e. you've written a thing and ostensibly require a delay for that
>>> thing to take effect, the power to switch on in this case.
>>> 
>>> It makes more sense to write, delay and read back rather than write,
>>> readback and delay surely...
>> This is the original reset sequence without modification, i have just
>> moved it in a else case when it is not an MDM9607, MSM8917 or MSM8937.
> 
> Doesn't make it correct, we fix upstream logic bugs all the time...
Here is the original upstream logic
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/remoteproc/qcom_q6v5_mss.c?h=next-20251219#n823
and here is the same at downstream 3.18
https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LA.UM.6.6.c32-05500-89xx.0/drivers/soc/qcom/pil-q6v5.c#L451
and same from downstream 4.9
https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.10.6.2.c26-01500-89xx.0/drivers/soc/qcom/pil-q6v5.c#L518

> 
> For example a read-back to ensure write completion is only required for 
> posted memory transactions.
> 
> Is this a posted write ?
> 
> Is there an io-fabric in the world which exceeds 1 microsecond to 
> perform a write transaction ?
> 
> Anyway leaving that aside the bit that's really objectionable and IMO 
> obvious a bug is val |= readl();
> 
> Why or the bit back in ? and then why not check the bit was set on the 
> read ?
> 
> val = readl() is a lot less janky and shouldn't it matter that the bit 
> we tried to set is actually reflected in the read-back ?
> 
> Failure to set the bit would certainly be a problem...
> 
> ---
> bod

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Bryan O'Donoghue]

On 01/01/2026 21:57, barnabas.czeman@mainlining.org wrote:
> On 2026-01-01 21:58, Bryan O'Donoghue wrote:
>> On 01/01/2026 13:50, barnabas.czeman@mainlining.org wrote:
>>>>> +				for (; i >= 0; i--) {
>>>>> +					val |= BIT(i);
>>>>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>>>>> +					/*
>>>>> +					 * Read back value to ensure the write is done then
>>>>> +					 * wait for 1us for both memory peripheral and data
>>>>> +					 * array to turn on.
>>>>> +					 */
>>>>> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
>>>>> +					udelay(1);
>>>> Isn't the logic here inverted ?
>>>>
>>>> i.e. you've written a thing and ostensibly require a delay for that
>>>> thing to take effect, the power to switch on in this case.
>>>>
>>>> It makes more sense to write, delay and read back rather than write,
>>>> readback and delay surely...
>>> This is the original reset sequence without modification, i have just
>>> moved it in a else case when it is not an MDM9607, MSM8917 or MSM8937.
>>
>> Doesn't make it correct, we fix upstream logic bugs all the time...
> Here is the original upstream logic
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/remoteproc/qcom_q6v5_mss.c?h=next-20251219#n823
> and here is the same at downstream 3.18
> https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LA.UM.6.6.c32-05500-89xx.0/drivers/soc/qcom/pil-q6v5.c#L451
> and same from downstream 4.9
> https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.10.6.2.c26-01500-89xx.0/drivers/soc/qcom/pil-q6v5.c#L518

Plenty of downstream bugs...

Let's assume those are posted writes i.e. the IO fabric equivalent of 
UDP - I'm not sure I'd say the downstream code is consistent in its 
treatement of write transactions..

But aside from just pointing at downstream - how is val |= readl() a 
correct thing versus val = readl();

...

I mean its just not

> 
>>
>> For example a read-back to ensure write completion is only required for
>> posted memory transactions.
>>
>> Is this a posted write ?
>>
>> Is there an io-fabric in the world which exceeds 1 microsecond to
>> perform a write transaction ?
>>
>> Anyway leaving that aside the bit that's really objectionable and IMO
>> obvious a bug is val |= readl();
>>
>> Why or the bit back in ? and then why not check the bit was set on the
>> read ?
>>
>> val = readl() is a lot less janky and shouldn't it matter that the bit
>> we tried to set is actually reflected in the read-back ?
>>
>> Failure to set the bit would certainly be a problem...
>>
>> ---
>> bod

Re: [PATCH v3 2/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MDM9607

[Krzysztof Kozlowski]

On Wed, Dec 31, 2025 at 05:30:12PM +0100, Barnabás Czémán wrote:
> Add the compatible for MSS as found on the MDM9607 platform.
> 
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
>  Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v3 4/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8917

[Krzysztof Kozlowski]

On Wed, Dec 31, 2025 at 05:30:14PM +0100, Barnabás Czémán wrote:
> Add the compatible for MSS as found on the MSM8917 platform.
> 
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
>  .../devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml         | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v3 6/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8937

[Krzysztof Kozlowski]

On Wed, Dec 31, 2025 at 05:30:16PM +0100, Barnabás Czémán wrote:
> Add the compatible for MSS as found on the MSM8937 platform.
> 
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
>  Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v3 8/9] dt-bindings: remoteproc: qcom,msm8916-mss-pil: Add MSM8940

[Krzysztof Kozlowski]

On Wed, Dec 31, 2025 at 05:30:18PM +0100, Barnabás Czémán wrote:
> Add the compatible for MSS as found on the MSM8940 platform.
> 
> Signed-off-by: Barnabás Czémán <barnabas.czeman@mainlining.org>
> ---
>  Documentation/devicetree/bindings/remoteproc/qcom,msm8916-mss-pil.yaml | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Konrad Dybcio]

On 1/1/26 9:58 PM, Bryan O'Donoghue wrote:
> On 01/01/2026 13:50, barnabas.czeman@mainlining.org wrote:
>>>> +                for (; i >= 0; i--) {
>>>> +                    val |= BIT(i);
>>>> +                    writel(val, qproc->reg_base + mem_pwr_ctl);
>>>> +                    /*
>>>> +                     * Read back value to ensure the write is done then
>>>> +                     * wait for 1us for both memory peripheral and data
>>>> +                     * array to turn on.
>>>> +                     */
>>>> +                    val |= readl(qproc->reg_base + mem_pwr_ctl);
>>>> +                    udelay(1);
>>> Isn't the logic here inverted ?
>>>
>>> i.e. you've written a thing and ostensibly require a delay for that
>>> thing to take effect, the power to switch on in this case.
>>>
>>> It makes more sense to write, delay and read back rather than write,
>>> readback and delay surely...
>> This is the original reset sequence without modification, i have just
>> moved it in a else case when it is not an MDM9607, MSM8917 or MSM8937.
> 
> Doesn't make it correct, we fix upstream logic bugs all the time...
> 
> For example a read-back to ensure write completion is only required for posted memory transactions.
> 
> Is this a posted write ?
> 
> Is there an io-fabric in the world which exceeds 1 microsecond to perform a write transaction ?

Writes on arm64 aren't usually observable from the remote endpoint when
you would expect them to, they can be buffered unless there's an explicit
readback right afterwards (which creates a dependency that the processor
will fulfill)

Now I don't like that this driver is going

val |= BIT(i);
writel(val, foo);
// val is "altered" but not really
val |= readl(foo);

I didn't notice we were just doing a readback for the sake of a readback
in the last revision. MDM9607 should most definitely have it too..
Perhaps I should have just read the comment

Konrad

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[barnabas.czeman]

On 2026-01-02 10:55, Bryan O'Donoghue wrote:
> On 01/01/2026 21:57, barnabas.czeman@mainlining.org wrote:
>> On 2026-01-01 21:58, Bryan O'Donoghue wrote:
>>> On 01/01/2026 13:50, barnabas.czeman@mainlining.org wrote:
>>>>>> +				for (; i >= 0; i--) {
>>>>>> +					val |= BIT(i);
>>>>>> +					writel(val, qproc->reg_base + mem_pwr_ctl);
>>>>>> +					/*
>>>>>> +					 * Read back value to ensure the write is done then
>>>>>> +					 * wait for 1us for both memory peripheral and data
>>>>>> +					 * array to turn on.
>>>>>> +					 */
>>>>>> +					val |= readl(qproc->reg_base + mem_pwr_ctl);
>>>>>> +					udelay(1);
>>>>> Isn't the logic here inverted ?
>>>>> 
>>>>> i.e. you've written a thing and ostensibly require a delay for that
>>>>> thing to take effect, the power to switch on in this case.
>>>>> 
>>>>> It makes more sense to write, delay and read back rather than 
>>>>> write,
>>>>> readback and delay surely...
>>>> This is the original reset sequence without modification, i have 
>>>> just
>>>> moved it in a else case when it is not an MDM9607, MSM8917 or 
>>>> MSM8937.
>>> 
>>> Doesn't make it correct, we fix upstream logic bugs all the time...
>> Here is the original upstream logic
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/remoteproc/qcom_q6v5_mss.c?h=next-20251219#n823
>> and here is the same at downstream 3.18
>> https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/blob/LA.UM.6.6.c32-05500-89xx.0/drivers/soc/qcom/pil-q6v5.c#L451
>> and same from downstream 4.9
>> https://git.codelinaro.org/clo/la/kernel/msm-4.9/-/blob/LA.UM.10.6.2.c26-01500-89xx.0/drivers/soc/qcom/pil-q6v5.c#L518
> 
> Plenty of downstream bugs...
Here is the commit where that line was introduced 
https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/ffc6ee0242ec4caa69687848ad3ac5f376b3d005
> 
> Let's assume those are posted writes i.e. the IO fabric equivalent of 
> UDP - I'm not sure I'd say the downstream code is consistent in its 
> treatement of write transactions..
> 
> But aside from just pointing at downstream - how is val |= readl() a 
> correct thing versus val = readl();
> 
> ...
> 
> I mean its just not
> 
>> 
>>> 
>>> For example a read-back to ensure write completion is only required 
>>> for
>>> posted memory transactions.
>>> 
>>> Is this a posted write ?
>>> 
>>> Is there an io-fabric in the world which exceeds 1 microsecond to
>>> perform a write transaction ?
>>> 
>>> Anyway leaving that aside the bit that's really objectionable and IMO
>>> obvious a bug is val |= readl();
>>> 
>>> Why or the bit back in ? and then why not check the bit was set on 
>>> the
>>> read ?
>>> 
>>> val = readl() is a lot less janky and shouldn't it matter that the 
>>> bit
>>> we tried to set is actually reflected in the read-back ?
>>> 
>>> Failure to set the bit would certainly be a problem...
>>> 
>>> ---
>>> bod

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Bryan O'Donoghue]

On 02/01/2026 12:00, Konrad Dybcio wrote:
>> Is there an io-fabric in the world which exceeds 1 microsecond to perform a write transaction ?
> Writes on arm64 aren't usually observable from the remote endpoint when
> you would expect them to, they can be buffered unless there's an explicit
> readback right afterwards (which creates a dependency that the processor
> will fulfill)

I don't mean write-combining cache, I mean posted versus non-posted 
writes which is a feature of the front-side-bus :)

---
bod

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Bryan O'Donoghue]

On 02/01/2026 12:00, Konrad Dybcio wrote:
> Now I don't like that this driver is going
> 
> val |= BIT(i);
> writel(val, foo);
> // val is "altered" but not really
> val |= readl(foo);
> 
> I didn't notice we were just doing a readback for the sake of a readback
> in the last revision. MDM9607 should most definitely have it too..
> Perhaps I should have just read the comment

Yeah this just looks dodgy and inconsistent in this code.

And anyway, why OR those bits in...

---
bod

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Konrad Dybcio]

On 1/2/26 1:59 PM, Bryan O'Donoghue wrote:
> On 02/01/2026 12:00, Konrad Dybcio wrote:
>>> Is there an io-fabric in the world which exceeds 1 microsecond to perform a write transaction ?
>> Writes on arm64 aren't usually observable from the remote endpoint when
>> you would expect them to, they can be buffered unless there's an explicit
>> readback right afterwards (which creates a dependency that the processor
>> will fulfill)
> 
> I don't mean write-combining cache, I mean posted versus non-posted writes which is a feature of the front-side-bus :)

Writes are posted (Device-nGnRE, note the E attribute is set)

https://documentation-service.arm.com/static/63a43e333f28e5456434e18b

Konrad

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[Konrad Dybcio]

On 1/2/26 2:00 PM, Bryan O'Donoghue wrote:
> On 02/01/2026 12:00, Konrad Dybcio wrote:
>> Now I don't like that this driver is going
>>
>> val |= BIT(i);
>> writel(val, foo);
>> // val is "altered" but not really
>> val |= readl(foo);
>>
>> I didn't notice we were just doing a readback for the sake of a readback
>> in the last revision. MDM9607 should most definitely have it too..
>> Perhaps I should have just read the comment
> 
> Yeah this just looks dodgy and inconsistent in this code.
> 
> And anyway, why OR those bits in...

Changing |= to just = would make it easier to follow indeed

Konrad

Re: [PATCH v3 3/9] remoteproc: qcom_q6v5_mss: Add MDM9607

[barnabas.czeman]

On 2026-01-02 13:00, Konrad Dybcio wrote:
> On 1/1/26 9:58 PM, Bryan O'Donoghue wrote:
>> On 01/01/2026 13:50, barnabas.czeman@mainlining.org wrote:
>>>>> +                for (; i >= 0; i--) {
>>>>> +                    val |= BIT(i);
>>>>> +                    writel(val, qproc->reg_base + mem_pwr_ctl);
>>>>> +                    /*
>>>>> +                     * Read back value to ensure the write is done 
>>>>> then
>>>>> +                     * wait for 1us for both memory peripheral and 
>>>>> data
>>>>> +                     * array to turn on.
>>>>> +                     */
>>>>> +                    val |= readl(qproc->reg_base + mem_pwr_ctl);
>>>>> +                    udelay(1);
>>>> Isn't the logic here inverted ?
>>>> 
>>>> i.e. you've written a thing and ostensibly require a delay for that
>>>> thing to take effect, the power to switch on in this case.
>>>> 
>>>> It makes more sense to write, delay and read back rather than write,
>>>> readback and delay surely...
>>> This is the original reset sequence without modification, i have just
>>> moved it in a else case when it is not an MDM9607, MSM8917 or 
>>> MSM8937.
>> 
>> Doesn't make it correct, we fix upstream logic bugs all the time...
>> 
>> For example a read-back to ensure write completion is only required 
>> for posted memory transactions.
>> 
>> Is this a posted write ?
>> 
>> Is there an io-fabric in the world which exceeds 1 microsecond to 
>> perform a write transaction ?
> 
> Writes on arm64 aren't usually observable from the remote endpoint when
> you would expect them to, they can be buffered unless there's an 
> explicit
> readback right afterwards (which creates a dependency that the 
> processor
> will fulfill)
> 
> Now I don't like that this driver is going
> 
> val |= BIT(i);
> writel(val, foo);
> // val is "altered" but not really
> val |= readl(foo);
> 
> I didn't notice we were just doing a readback for the sake of a 
> readback
> in the last revision. MDM9607 should most definitely have it too..
In this case I should go back to previous inrush current mitigation from 
v2.
> Perhaps I should have just read the comment
> 
> Konrad