[PATCH v2 0/4] firmware: qcom: scm: Add minidump SRAM destination support

[PATCH v2 0/4] firmware: qcom: scm: Add minidump SRAM destination support

[Mukesh Ojha]

On most Qualcomm SoCs where minidump is supported, a word in always-on
SRAM is shared between the kernel and boot firmware. Before DDR is
initialised on the warm reset following a crash, firmware reads this
word to decide if minidump is enabled and collect a minidump and where
to deliver it (USB upload to a host, or save to local storage).

This series wires that mechanism into the SCM driver:

- The SRAM word location is described via a 'sram' phandle on the
  SCM DT node.

- Trivial change for consistency.

- A 'minidump_dest' module parameter (default: usb) selects the
  destination.  Custom kernel_param_ops expose it as the human-
  readable strings "usb" or "storage".

- Add the support for Kaanapali.

Changes in v2: https://lore.kernel.org/lkml/20260507080727.3227367-1-mukesh.ojha@oss.qualcomm.com/
 - Remove the restriction on the binding change done in v1.
 - Remove sram-name from binding.
 - sram definition is introduced and merged, so removed the refs from
   v1.
 - Minor change in the log as per comment s/find/get/
 - remove reference of sram-names
 - use minidump-sram instead of minidump-config.

Mukesh Ojha (4):
  dt-bindings: firmware: qcom,scm: Add minidump SRAM property
  firmware: qcom: scm: use dev_err_probe() for dload address failure
  firmware: qcom: scm: Add minidump SRAM support
  arm64: dts: qcom: kaanapali: Add minidump SRAM config to SCM node

 .../bindings/firmware/qcom,scm.yaml           | 16 ++++
 arch/arm64/boot/dts/qcom/kaanapali.dtsi       |  5 +
 drivers/firmware/qcom/qcom_scm.c              | 94 ++++++++++++++++++-
 3 files changed, 114 insertions(+), 1 deletion(-)

-- 
2.53.0

[PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Add minidump SRAM property

[Mukesh Ojha]

On most Qualcomm SoCs where minidump is supported, a word in always-on
SRAM is shared between the kernel and boot firmware. Before DDR is
initialised on the warm reset following a crash, firmware reads this
word to decide if minidump is enabled and collect a minidump and where
 to deliver it (USB upload to a host, or save to local storage).

Add a 'sram' property to the SCM binding to describe a region in
always-on SRAM where the minidump download destination value could be
written. Boot firmware reads it before DDR is initialised on a warm
reset to decide where to store the minidump either to host PC or to
on device storage.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 .../devicetree/bindings/firmware/qcom,scm.yaml   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
index 25f62bacbc91..27422d00b8fc 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
@@ -129,6 +129,13 @@ properties:
           - description: offset of the download mode control register
     description: TCSR hardware block
 
+  sram:
+    description:
+      Phandle to a region in always-on SRAM used to store the download
+      mode value for boot firmware to read before DDR is initialised on
+      the next warm reset.
+    maxItems: 1
+
 allOf:
   # Clocks
   - if:
@@ -250,3 +257,12 @@ examples:
             clock-names = "core", "bus", "iface";
         };
     };
+
+  - |
+    firmware {
+        scm {
+            compatible = "qcom,scm-kaanapali", "qcom,scm";
+            sram = <&minidump_config>;
+            #reset-cells = <1>;
+        };
+    };
-- 
2.53.0

[PATCH v2 2/4] firmware: qcom: scm: use dev_err_probe() for dload address failure

[Mukesh Ojha]

Replace the bare `return ret` after qcom_scm_find_dload_address() with
dev_err_probe() to produce a consistent, deferred-probe-aware error
message when the download-mode address cannot be resolved.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 9b06a69d3a6d..52432469badc 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -2747,7 +2747,8 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	scm->dev = &pdev->dev;
 	ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr);
 	if (ret < 0)
-		return ret;
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to get download mode address\n");
 
 	mutex_init(&scm->scm_bw_lock);
 
-- 
2.53.0

[PATCH v2 3/4] firmware: qcom: scm: Add minidump SRAM support

[Mukesh Ojha]

On most Qualcomm SoCs where minidump is supported, a word in always-on
SRAM is shared between the kernel and boot firmware. Before DDR is
initialised on the warm reset following a crash, firmware reads this
word to decide if minidump is enabled and collect a minidump and where
to deliver it (USB upload to a host, or save to local storage).

The SRAM region is described by a 'sram' phandle on the SCM DT node.
If the property is absent the feature is silently disabled, keeping
existing SoCs unaffected.

Expose a 'minidump_dest' module parameter (default: usb) so the user can
select the destination. Only the string names "usb" or "storage" are
accepted; an invalid value is rejected with -EINVAL. Changing the
destination while minidump mode is already active updates SRAM immediately.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 91 ++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 52432469badc..e3d5973bf567 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -57,6 +57,7 @@ struct qcom_scm {
 	int scm_vote_count;
 
 	u64 dload_mode_addr;
+	void __iomem *minidump_sram;
 
 	struct qcom_tzmem_pool *mempool;
 	unsigned int wq_cnt;
@@ -141,6 +142,18 @@ static const u8 qcom_scm_cpu_warm_bits[QCOM_SCM_BOOT_MAX_CPUS] = {
 #define QCOM_DLOAD_MINIDUMP	2
 #define QCOM_DLOAD_BOTHDUMP	3
 
+/* Minidump destination values written to always-on SRAM for boot firmware */
+#define QCOM_MINIDUMP_DEST_USB		0x0
+#define QCOM_MINIDUMP_DEST_STORAGE	0x2
+
+static u32 minidump_dest = QCOM_MINIDUMP_DEST_USB;
+
+static const char * const minidump_dest_name[] = { "usb", "storage" };
+static const u32 minidump_dest_val[] = {
+	QCOM_MINIDUMP_DEST_USB,
+	QCOM_MINIDUMP_DEST_STORAGE,
+};
+
 #define QCOM_SCM_DEFAULT_WAITQ_COUNT 1
 
 static const char * const qcom_scm_convention_names[] = {
@@ -568,6 +581,14 @@ static void qcom_scm_set_download_mode(u32 dload_mode)
 
 	if (ret)
 		dev_err(__scm->dev, "failed to set download mode: %d\n", ret);
+
+	/*
+	 * Write the destination into the always-on SRAM so boot firmware
+	 * can read it before DDR is initialised on the next warm reset.
+	 * Only written when minidump is active;
+	 */
+	if (__scm->minidump_sram && (dload_mode & QCOM_DLOAD_MINIDUMP))
+		writel_relaxed(minidump_dest, __scm->minidump_sram);
 }
 
 /**
@@ -2040,6 +2061,37 @@ int qcom_scm_gpu_init_regs(u32 gpu_req)
 }
 EXPORT_SYMBOL_GPL(qcom_scm_gpu_init_regs);
 
+static int qcom_scm_map_minidump_sram(struct device *dev, void __iomem **out)
+{
+	struct device_node *np = dev->of_node;
+	struct device_node *sram_np;
+	struct resource res;
+	int ret;
+
+	if (!of_property_present(np, "sram"))
+		return 0;
+
+	sram_np = of_parse_phandle(np, "sram", 0);
+	if (!sram_np)
+		return -EINVAL;
+
+	ret = of_address_to_resource(sram_np, 0, &res);
+	of_node_put(sram_np);
+	if (ret)
+		return ret;
+
+	if (resource_size(&res) < sizeof(u32)) {
+		dev_err(dev, "minidump SRAM region too small\n");
+		return -EINVAL;
+	}
+
+	*out = devm_ioremap(dev, res.start, resource_size(&res));
+	if (!*out)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int qcom_scm_find_dload_address(struct device *dev, u64 *addr)
 {
 	struct device_node *tcsr;
@@ -2733,6 +2785,40 @@ static const struct kernel_param_ops download_mode_param_ops = {
 module_param_cb(download_mode, &download_mode_param_ops, NULL, 0644);
 MODULE_PARM_DESC(download_mode, "download mode: off/0/N for no dump mode, full/on/1/Y for full dump mode, mini for minidump mode and full,mini for both full and minidump mode together are acceptable values");
 
+static int get_minidump_dest(char *buffer, const struct kernel_param *kp)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(minidump_dest_val); i++)
+		if (minidump_dest == minidump_dest_val[i])
+			return sysfs_emit(buffer, "%s\n", minidump_dest_name[i]);
+
+	return sysfs_emit(buffer, "unknown\n");
+}
+
+static int set_minidump_dest(const char *val, const struct kernel_param *kp)
+{
+	int i;
+
+	i = sysfs_match_string(minidump_dest_name, val);
+	if (i < 0)
+		return -EINVAL;
+
+	minidump_dest = minidump_dest_val[i];
+	if (__scm && __scm->minidump_sram && (download_mode & QCOM_DLOAD_MINIDUMP))
+		writel_relaxed(minidump_dest, __scm->minidump_sram);
+
+	return 0;
+}
+
+static const struct kernel_param_ops minidump_dest_param_ops = {
+	.get = get_minidump_dest,
+	.set = set_minidump_dest,
+};
+
+module_param_cb(minidump_dest, &minidump_dest_param_ops, NULL, 0644);
+MODULE_PARM_DESC(minidump_dest, "Minidump SRAM destination: usb (default) or storage");
+
 static int qcom_scm_probe(struct platform_device *pdev)
 {
 	struct qcom_tzmem_pool_config pool_config;
@@ -2750,6 +2836,11 @@ static int qcom_scm_probe(struct platform_device *pdev)
 		return dev_err_probe(&pdev->dev, ret,
 				     "Failed to get download mode address\n");
 
+	ret = qcom_scm_map_minidump_sram(&pdev->dev, &scm->minidump_sram);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "Failed to map minidump SRAM\n");
+
 	mutex_init(&scm->scm_bw_lock);
 
 	scm->path = devm_of_icc_get(&pdev->dev, NULL);
-- 
2.53.0

[PATCH v2 4/4] arm64: dts: qcom: kaanapali: Add minidump SRAM config to SCM node

[Mukesh Ojha]

Point the SCM node at the minidump config slot in the always-on SRAM.
Boot firmware reads this word before DDR is initialised on a warm reset
to decide where to deliver the minidump.

Signed-off-by: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/kaanapali.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
index bcd1cee31356..425b532c53be 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
+++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
@@ -224,6 +224,7 @@ scm: scm {
 			qcom,dload-mode = <&tcsr 0x19000>;
 			interconnects = <&aggre_noc MASTER_CRYPTO QCOM_ICC_TAG_ALWAYS
 					 &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>;
+			sram = <&minidump_config>;
 		};
 
 		scmi: scmi {
@@ -5448,6 +5449,10 @@ sram@14680000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
 
+			minidump_config: minidump-sram@1c {
+				reg = <0x1c 0x4>;
+			};
+
 			pil-sram@94c {
 				compatible = "qcom,pil-reloc-info";
 				reg = <0x94c 0xc8>;
-- 
2.53.0

Re: [PATCH v2 1/4] dt-bindings: firmware: qcom,scm: Add minidump SRAM property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing type definition ($ref) for the `sram` device tree property.
- [Low] Violation of device tree naming conventions by using the generic name `sram` for a vendor-specific property.
--

commit 657534134734a0a9c527c2bb59cf5ebc948cf8b5
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

dt-bindings: firmware: qcom,scm: Add minidump SRAM property

This commit adds a sram property to the Qualcomm SCM binding. It describes
a region in always-on SRAM where the minidump download destination value is
written so boot firmware can read it on warm reset.

> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> @@ -127,6 +127,13 @@ properties:
>            - description: offset of the download mode control register
>      description: TCSR hardware block
>  
> +  sram:

[Severity: Low]
Would it be better to use a vendor-specific name for this property, such as
qcom,minidump-sram or qcom,dload-sram?

Using the generic sram name for a highly specialized firmware feature might
prevent the qcom,scm node from using sram for general-purpose SRAM attachments
in the future.

> +    description:
> +      Phandle to a region in always-on SRAM used to store the download
> +      mode value for boot firmware to read before DDR is initialised on
> +      the next warm reset.
> +    maxItems: 1

[Severity: Low]
Should this property explicitly reference a standard type, such as
$ref: /schemas/types.yaml#/definitions/phandle?

Because the property name lacks a standard type-implying suffix, specifying
maxItems: 1 alone might treat it as an untyped JSON array. Devicetree
validation tooling could fail to enforce that the value is a valid phandle,
allowing malformed values to pass the dt_binding_check silently.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519171442.1582987-1-mukesh.ojha@oss.qualcomm.com?part=1

Re: [PATCH v2 3/4] firmware: qcom: scm: Add minidump SRAM support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The global `__scm` pointer is read concurrently in `set_minidump_dest()` without the required `smp_load_acquire()` or `READ_ONCE()` barrier, creating a data race.
--

commit 46d7aebbb8abe9c9e0e6c6cfdb3495908a439bbd
Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>

firmware: qcom: scm: Add minidump SRAM support

This commit adds support for writing minidump destination values to
always-on SRAM. It exposes a module parameter to let users select the
destination, which updates the SRAM immediately if minidump mode is active.

> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
[ ... ]
> @@ -2733,6 +2785,40 @@ static const struct kernel_param_ops download_mode_param_ops = {
[ ... ]
> +static int set_minidump_dest(const char *val, const struct kernel_param *kp)
> +{
> +	int i;
> +
> +	i = sysfs_match_string(minidump_dest_name, val);
> +	if (i < 0)
> +		return -EINVAL;
> +
> +	minidump_dest = minidump_dest_val[i];
> +	if (__scm && __scm->minidump_sram && (download_mode & QCOM_DLOAD_MINIDUMP))

[Severity: Medium]
Could this plain read of __scm result in a data race?

In qcom_scm_probe(), the global __scm pointer is published using
smp_store_release(&__scm, scm). Since module parameters are exposed in
sysfs early, this setter might be executed concurrently while the probe
function is running.

Without a matching smp_load_acquire() or READ_ONCE(), is it possible for
the compiler or CPU to reorder the memory accesses? This might result in
observing a non-NULL __scm but an uninitialized __scm->minidump_sram value,
which could then lead to an arbitrary memory write via writel_relaxed().

Would it be safer to safely acquire the pointer first, for example using:
struct qcom_scm *scm = smp_load_acquire(&__scm);

> +		writel_relaxed(minidump_dest, __scm->minidump_sram);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519171442.1582987-1-mukesh.ojha@oss.qualcomm.com?part=3