devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Add EDL reboot and warm reset support for QRB2210
@ 2025-11-03 18:20 Loic Poulain
  2025-11-03 18:20 ` [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset Loic Poulain
                   ` (4 more replies)
  0 siblings, 5 replies; 34+ messages in thread
From: Loic Poulain @ 2025-11-03 18:20 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, sre
  Cc: linux-arm-msm, devicetree, linux-pm, Loic Poulain

This patch series adds support for Emergency Download (EDL) reboot mode
via the qcom-scm interface, allowing the platform to enter a recovery
mode through the primary bootloader. Additionally, since this 'mode'
requires warm reset and platforms like the QRB2210-RB1 lacks PSCI
warm reset support, a fallback mechanism using the PMIC is introduced.

The series includes:
- Documentation updates for new DT bindings.
- SCM firmware support for EDL reboot.
- PMIC warm reset support in the qcom-pon driver.
- Device tree updates for the QRB2210-RB1 platform.

Tested on QRB2210-RB1.

Loic Poulain (5):
  dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset
  power: reset: qcom-pon: Add support for WARM reset
  dt-bindings: firmware: qcom,scm: Document reboot mode
  firmware: qcom: scm: Support for EDL reboot mode
  arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot

 .../bindings/firmware/qcom,scm.yaml           |  4 ++
 .../bindings/power/reset/qcom,pon.yaml        |  7 +++
 arch/arm64/boot/dts/qcom/pm4125.dtsi          |  2 +-
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts      |  8 ++++
 drivers/firmware/qcom/qcom_scm.c              | 22 +++++++++
 drivers/power/reset/qcom-pon.c                | 47 +++++++++++++++++++
 6 files changed, 89 insertions(+), 1 deletion(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset
  2025-11-03 18:20 [PATCH 0/5] Add EDL reboot and warm reset support for QRB2210 Loic Poulain
@ 2025-11-03 18:20 ` Loic Poulain
  2025-11-04  2:17   ` Bjorn Andersson
  2025-11-04 14:03   ` Krzysztof Kozlowski
  2025-11-03 18:20 ` [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset Loic Poulain
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 34+ messages in thread
From: Loic Poulain @ 2025-11-03 18:20 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, sre
  Cc: linux-arm-msm, devicetree, linux-pm, Loic Poulain

This property can be used as a fallback mechanism when a warm reset
cannot be achieved through the standard firmware interface (SCPI).

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 .../devicetree/bindings/power/reset/qcom,pon.yaml          | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
index 979a377cb4ff..ad8691c87f4f 100644
--- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
+++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
@@ -38,6 +38,13 @@ properties:
     minItems: 1
     maxItems: 2
 
+  qcom,warm-reset:
+    description: |
+      The PON (Power-On) peripheral provides support for warm reset, which can
+      be used as a fallback mechanism when a warm reset cannot be achieved
+      through the standard firmware interface.
+    type: boolean
+
   pwrkey:
     type: object
     $ref: /schemas/input/qcom,pm8941-pwrkey.yaml#
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-03 18:20 [PATCH 0/5] Add EDL reboot and warm reset support for QRB2210 Loic Poulain
  2025-11-03 18:20 ` [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset Loic Poulain
@ 2025-11-03 18:20 ` Loic Poulain
  2025-11-04  2:19   ` Bjorn Andersson
                     ` (2 more replies)
  2025-11-03 18:20 ` [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode Loic Poulain
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 34+ messages in thread
From: Loic Poulain @ 2025-11-03 18:20 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, sre
  Cc: linux-arm-msm, devicetree, linux-pm, Loic Poulain

This mechanism can be used when firmware lacks proper warm-reset handling,
for example, when the PSCI SYSTEM_RESET2 function is not implemented.
It enables the warm reset functionality via the PMIC.

This fallback is only enabled if qcom,warm-reset property is present.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
index 7e108982a582..684e9fe9987d 100644
--- a/drivers/power/reset/qcom-pon.c
+++ b/drivers/power/reset/qcom-pon.c
@@ -19,12 +19,20 @@
 
 #define NO_REASON_SHIFT			0
 
+#define PON_SW_RESET_S2_CTL				0x62
+#define		PON_SW_RESET_S2_CTL_WARM_RST	0x01
+#define PON_SW_RESET_S2_CTL2				0x63
+#define		PON_SW_RESET_S2_CTL2_RST_EN	BIT(7)
+#define PON_SW_RESET_GO					0x64
+#define		PON_SW_RESET_GO_MAGIC		0xa5
+
 struct qcom_pon {
 	struct device *dev;
 	struct regmap *regmap;
 	u32 baseaddr;
 	struct reboot_mode_driver reboot_mode;
 	long reason_shift;
+	bool warm_reset;
 };
 
 static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
@@ -44,6 +52,37 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
 	return ret;
 }
 
+static int pm8916_pon_reset(struct sys_off_data *data)
+{
+	struct qcom_pon *pon = data->cb_data;
+	int ret;
+
+	if (!pon->warm_reset || data->mode != REBOOT_WARM)
+		return NOTIFY_DONE;
+
+	ret = regmap_write(pon->regmap,
+			   pon->baseaddr + PON_SW_RESET_S2_CTL,
+			   PON_SW_RESET_S2_CTL_WARM_RST);
+	if (ret)
+		return NOTIFY_BAD;
+
+	ret = regmap_update_bits(pon->regmap,
+				 pon->baseaddr + PON_SW_RESET_S2_CTL2,
+				 PON_SW_RESET_S2_CTL2_RST_EN,
+				 PON_SW_RESET_S2_CTL2_RST_EN);
+	if (ret)
+		return NOTIFY_BAD;
+
+	ret = regmap_write(pon->regmap, pon->baseaddr + PON_SW_RESET_GO,
+			   PON_SW_RESET_GO_MAGIC);
+	if (ret)
+		return NOTIFY_BAD;
+
+	mdelay(100);
+
+	return NOTIFY_DONE;
+}
+
 static int qcom_pon_probe(struct platform_device *pdev)
 {
 	struct qcom_pon *pon;
@@ -80,8 +119,16 @@ static int qcom_pon_probe(struct platform_device *pdev)
 		}
 	}
 
+	pon->warm_reset = of_property_read_bool(pdev->dev.of_node, "qcom,warm-reset");
+
 	platform_set_drvdata(pdev, pon);
 
+	/* Higher priority than psci to handle warm-reset properly */
+	error = devm_register_sys_off_handler(&pdev->dev, SYS_OFF_MODE_RESTART, SYS_OFF_PRIO_HIGH,
+					      pm8916_pon_reset, pon);
+	if (error)
+		return dev_err_probe(&pdev->dev, error, "reboot registration fail\n");
+
 	return devm_of_platform_populate(&pdev->dev);
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-03 18:20 [PATCH 0/5] Add EDL reboot and warm reset support for QRB2210 Loic Poulain
  2025-11-03 18:20 ` [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset Loic Poulain
  2025-11-03 18:20 ` [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset Loic Poulain
@ 2025-11-03 18:20 ` Loic Poulain
  2025-11-04  2:16   ` Bjorn Andersson
  2025-11-03 18:20 ` [PATCH 4/5] firmware: qcom: scm: Support for EDL " Loic Poulain
  2025-11-03 18:20 ` [PATCH 5/5] arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot Loic Poulain
  4 siblings, 1 reply; 34+ messages in thread
From: Loic Poulain @ 2025-11-03 18:20 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, sre
  Cc: linux-arm-msm, devicetree, linux-pm, Loic Poulain

SCM can be used to support reboot mode such as Emergency Recovery Mode.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
index b913192219e4..c8bb7dacd900 100644
--- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
+++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
@@ -121,6 +121,10 @@ properties:
           - description: offset of the download mode control register
     description: TCSR hardware block
 
+patternProperties:
+  "^mode-.*$":
+    maxItems: 1
+
 allOf:
   # Clocks
   - if:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 4/5] firmware: qcom: scm: Support for EDL reboot mode
  2025-11-03 18:20 [PATCH 0/5] Add EDL reboot and warm reset support for QRB2210 Loic Poulain
                   ` (2 preceding siblings ...)
  2025-11-03 18:20 ` [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode Loic Poulain
@ 2025-11-03 18:20 ` Loic Poulain
  2025-11-04  2:22   ` Bjorn Andersson
                     ` (2 more replies)
  2025-11-03 18:20 ` [PATCH 5/5] arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot Loic Poulain
  4 siblings, 3 replies; 34+ messages in thread
From: Loic Poulain @ 2025-11-03 18:20 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, sre
  Cc: linux-arm-msm, devicetree, linux-pm, Loic Poulain

When the dload register is specified, it can also be used to boot the
platform into Emergency Download Mode (EDL). This mode is implemented
by the primary ROM bootloader and provides a recovery mechanism.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/firmware/qcom/qcom_scm.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
index 26cd0458aacd..e697ef14619f 100644
--- a/drivers/firmware/qcom/qcom_scm.c
+++ b/drivers/firmware/qcom/qcom_scm.c
@@ -26,6 +26,7 @@
 #include <linux/of_platform.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/platform_device.h>
+#include <linux/reboot-mode.h>
 #include <linux/reset-controller.h>
 #include <linux/sizes.h>
 #include <linux/types.h>
@@ -43,6 +44,7 @@ struct qcom_scm {
 	struct icc_path *path;
 	struct completion waitq_comp;
 	struct reset_controller_dev reset;
+	struct reboot_mode_driver reboot_mode;
 
 	/* control access to the interconnect path */
 	struct mutex scm_bw_lock;
@@ -130,6 +132,8 @@ static const u8 qcom_scm_cpu_warm_bits[QCOM_SCM_BOOT_MAX_CPUS] = {
 #define QCOM_DLOAD_MINIDUMP	2
 #define QCOM_DLOAD_BOTHDUMP	3
 
+#define QCOM_EDL_MASK		BIT(0)
+
 static const char * const qcom_scm_convention_names[] = {
 	[SMC_CONVENTION_UNKNOWN] = "unknown",
 	[SMC_CONVENTION_ARM_32] = "smc arm 32",
@@ -2206,6 +2210,18 @@ static const struct kernel_param_ops download_mode_param_ops = {
 	.set = set_download_mode,
 };
 
+static int qcom_scm_reboot_mode_write(struct reboot_mode_driver *reboot,
+				      unsigned int magic)
+{
+	struct qcom_scm *scm = container_of(reboot, struct qcom_scm, reboot_mode);
+	int ret = -EOPNOTSUPP;
+
+	if (scm->dload_mode_addr)
+		ret = qcom_scm_io_rmw(scm->dload_mode_addr, QCOM_EDL_MASK, magic);
+
+	return ret;
+}
+
 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");
 
@@ -2251,6 +2267,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	scm->reboot_mode.dev = &pdev->dev;
+	scm->reboot_mode.write = qcom_scm_reboot_mode_write;
+	ret = devm_reboot_mode_register(&pdev->dev, &scm->reboot_mode);
+	if (ret)
+		return ret;
+
 	/* vote for max clk rate for highest performance */
 	ret = clk_set_rate(scm->core_clk, INT_MAX);
 	if (ret)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [PATCH 5/5] arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot
  2025-11-03 18:20 [PATCH 0/5] Add EDL reboot and warm reset support for QRB2210 Loic Poulain
                   ` (3 preceding siblings ...)
  2025-11-03 18:20 ` [PATCH 4/5] firmware: qcom: scm: Support for EDL " Loic Poulain
@ 2025-11-03 18:20 ` Loic Poulain
  2025-11-04  2:20   ` Dmitry Baryshkov
  4 siblings, 1 reply; 34+ messages in thread
From: Loic Poulain @ 2025-11-03 18:20 UTC (permalink / raw)
  To: andersson, konradybcio, robh, krzk+dt, conor+dt, sre
  Cc: linux-arm-msm, devicetree, linux-pm, Loic Poulain

EDL reboot mode can be configured via qcom-scm, enabling the platform
to enter Emergency Download Mode for recovery purposes. Additionally,
we enable PMIC-driven warm reset as the RB1 platform's PSCI interface
lacks support for warm reset.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/pm4125.dtsi     | 2 +-
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
index cf8c822e80ce..5d84a3250481 100644
--- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
@@ -15,7 +15,7 @@ pmic@0 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		pon@800 {
+		pon: pon@800 {
 			compatible = "qcom,pm8916-pon";
 			reg = <0x800>;
 
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
index b2e0fc5501c1..70c6c929bbd3 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
@@ -322,6 +322,10 @@ &pm4125_vbus {
 	status = "okay";
 };
 
+&pon {
+	qcom,warm-reset;
+};
+
 &qupv3_id_0 {
 	status = "okay";
 };
@@ -510,6 +514,11 @@ pm4125_l22: l22 {
 	};
 };
 
+&scm {
+	qcom,dload-mode = <&tcsr_regs 0x13000>;
+	mode-edl = <0x1>;
+};
+
 &sdhc_1 {
 	vmmc-supply = <&pm4125_l20>;
 	vqmmc-supply = <&pm4125_l14>;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-03 18:20 ` [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode Loic Poulain
@ 2025-11-04  2:16   ` Bjorn Andersson
  2025-11-04  2:19     ` Dmitry Baryshkov
  2025-11-04 21:19     ` Loic Poulain
  0 siblings, 2 replies; 34+ messages in thread
From: Bjorn Andersson @ 2025-11-04  2:16 UTC (permalink / raw)
  To: Loic Poulain
  Cc: konradybcio, robh, krzk+dt, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
> SCM can be used to support reboot mode such as Emergency Recovery Mode.

"such as"? Do we have any other useful bits in here?

> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> index b913192219e4..c8bb7dacd900 100644
> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> @@ -121,6 +121,10 @@ properties:
>            - description: offset of the download mode control register
>      description: TCSR hardware block
>  
> +patternProperties:
> +  "^mode-.*$":

I'd only ever expect mode-edl = <1>. Do we have additional modes that
warrant the generic nature of this?

Regards,
Bjorn

> +    maxItems: 1
> +
>  allOf:
>    # Clocks
>    - if:
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset
  2025-11-03 18:20 ` [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset Loic Poulain
@ 2025-11-04  2:17   ` Bjorn Andersson
  2025-11-04 14:03   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 34+ messages in thread
From: Bjorn Andersson @ 2025-11-04  2:17 UTC (permalink / raw)
  To: Loic Poulain
  Cc: konradybcio, robh, krzk+dt, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

On Mon, Nov 03, 2025 at 07:20:02PM +0100, Loic Poulain wrote:
> This property can be used as a fallback mechanism when a warm reset
> cannot be achieved through the standard firmware interface (SCPI).

PSCI

> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> ---
>  .../devicetree/bindings/power/reset/qcom,pon.yaml          | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> index 979a377cb4ff..ad8691c87f4f 100644
> --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> @@ -38,6 +38,13 @@ properties:
>      minItems: 1
>      maxItems: 2
>  
> +  qcom,warm-reset:
> +    description: |
> +      The PON (Power-On) peripheral provides support for warm reset, which can
> +      be used as a fallback mechanism when a warm reset cannot be achieved
> +      through the standard firmware interface.
> +    type: boolean
> +
>    pwrkey:
>      type: object
>      $ref: /schemas/input/qcom,pm8941-pwrkey.yaml#
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-03 18:20 ` [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset Loic Poulain
@ 2025-11-04  2:19   ` Bjorn Andersson
  2025-11-04 11:50   ` Konrad Dybcio
  2025-11-04 14:04   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 34+ messages in thread
From: Bjorn Andersson @ 2025-11-04  2:19 UTC (permalink / raw)
  To: Loic Poulain
  Cc: konradybcio, robh, krzk+dt, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

On Mon, Nov 03, 2025 at 07:20:03PM +0100, Loic Poulain wrote:
> This mechanism can be used when firmware lacks proper warm-reset handling,
> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
> It enables the warm reset functionality via the PMIC.
> 
> This fallback is only enabled if qcom,warm-reset property is present.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

Reviewed-by: Bjorn Andersson <andersson@kernel.org>

Regards,
Bjorn

> ---
>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> index 7e108982a582..684e9fe9987d 100644
> --- a/drivers/power/reset/qcom-pon.c
> +++ b/drivers/power/reset/qcom-pon.c
> @@ -19,12 +19,20 @@
>  
>  #define NO_REASON_SHIFT			0
>  
> +#define PON_SW_RESET_S2_CTL				0x62
> +#define		PON_SW_RESET_S2_CTL_WARM_RST	0x01
> +#define PON_SW_RESET_S2_CTL2				0x63
> +#define		PON_SW_RESET_S2_CTL2_RST_EN	BIT(7)
> +#define PON_SW_RESET_GO					0x64
> +#define		PON_SW_RESET_GO_MAGIC		0xa5
> +
>  struct qcom_pon {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	u32 baseaddr;
>  	struct reboot_mode_driver reboot_mode;
>  	long reason_shift;
> +	bool warm_reset;
>  };
>  
>  static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
> @@ -44,6 +52,37 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  	return ret;
>  }
>  
> +static int pm8916_pon_reset(struct sys_off_data *data)
> +{
> +	struct qcom_pon *pon = data->cb_data;
> +	int ret;
> +
> +	if (!pon->warm_reset || data->mode != REBOOT_WARM)
> +		return NOTIFY_DONE;
> +
> +	ret = regmap_write(pon->regmap,
> +			   pon->baseaddr + PON_SW_RESET_S2_CTL,
> +			   PON_SW_RESET_S2_CTL_WARM_RST);
> +	if (ret)
> +		return NOTIFY_BAD;
> +
> +	ret = regmap_update_bits(pon->regmap,
> +				 pon->baseaddr + PON_SW_RESET_S2_CTL2,
> +				 PON_SW_RESET_S2_CTL2_RST_EN,
> +				 PON_SW_RESET_S2_CTL2_RST_EN);
> +	if (ret)
> +		return NOTIFY_BAD;
> +
> +	ret = regmap_write(pon->regmap, pon->baseaddr + PON_SW_RESET_GO,
> +			   PON_SW_RESET_GO_MAGIC);
> +	if (ret)
> +		return NOTIFY_BAD;
> +
> +	mdelay(100);
> +
> +	return NOTIFY_DONE;
> +}
> +
>  static int qcom_pon_probe(struct platform_device *pdev)
>  {
>  	struct qcom_pon *pon;
> @@ -80,8 +119,16 @@ static int qcom_pon_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	pon->warm_reset = of_property_read_bool(pdev->dev.of_node, "qcom,warm-reset");
> +
>  	platform_set_drvdata(pdev, pon);
>  
> +	/* Higher priority than psci to handle warm-reset properly */
> +	error = devm_register_sys_off_handler(&pdev->dev, SYS_OFF_MODE_RESTART, SYS_OFF_PRIO_HIGH,
> +					      pm8916_pon_reset, pon);
> +	if (error)
> +		return dev_err_probe(&pdev->dev, error, "reboot registration fail\n");
> +
>  	return devm_of_platform_populate(&pdev->dev);
>  }
>  
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-04  2:16   ` Bjorn Andersson
@ 2025-11-04  2:19     ` Dmitry Baryshkov
  2025-11-04  2:45       ` Bjorn Andersson
  2025-11-04 21:19     ` Loic Poulain
  1 sibling, 1 reply; 34+ messages in thread
From: Dmitry Baryshkov @ 2025-11-04  2:19 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Loic Poulain, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Mon, Nov 03, 2025 at 08:16:30PM -0600, Bjorn Andersson wrote:
> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
> > SCM can be used to support reboot mode such as Emergency Recovery Mode.
> 
> "such as"? Do we have any other useful bits in here?
> 
> > 
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > index b913192219e4..c8bb7dacd900 100644
> > --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > @@ -121,6 +121,10 @@ properties:
> >            - description: offset of the download mode control register
> >      description: TCSR hardware block
> >  
> > +patternProperties:
> > +  "^mode-.*$":
> 
> I'd only ever expect mode-edl = <1>. Do we have additional modes that
> warrant the generic nature of this?

fastboot / bootloader?

> 
> Regards,
> Bjorn
> 
> > +    maxItems: 1
> > +
> >  allOf:
> >    # Clocks
> >    - if:
> > -- 
> > 2.34.1
> > 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/5] arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot
  2025-11-03 18:20 ` [PATCH 5/5] arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot Loic Poulain
@ 2025-11-04  2:20   ` Dmitry Baryshkov
  2025-11-04 10:23     ` Loic Poulain
  0 siblings, 1 reply; 34+ messages in thread
From: Dmitry Baryshkov @ 2025-11-04  2:20 UTC (permalink / raw)
  To: Loic Poulain
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Mon, Nov 03, 2025 at 07:20:06PM +0100, Loic Poulain wrote:
> EDL reboot mode can be configured via qcom-scm, enabling the platform
> to enter Emergency Download Mode for recovery purposes. Additionally,
> we enable PMIC-driven warm reset as the RB1 platform's PSCI interface
> lacks support for warm reset.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/pm4125.dtsi     | 2 +-
>  arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 9 +++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> index cf8c822e80ce..5d84a3250481 100644
> --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> @@ -15,7 +15,7 @@ pmic@0 {
>  		#address-cells = <1>;
>  		#size-cells = <0>;
>  
> -		pon@800 {
> +		pon: pon@800 {
>  			compatible = "qcom,pm8916-pon";
>  			reg = <0x800>;
>  
> diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> index b2e0fc5501c1..70c6c929bbd3 100644
> --- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> +++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> @@ -322,6 +322,10 @@ &pm4125_vbus {
>  	status = "okay";
>  };
>  
> +&pon {
> +	qcom,warm-reset;
> +};
> +
>  &qupv3_id_0 {
>  	status = "okay";
>  };
> @@ -510,6 +514,11 @@ pm4125_l22: l22 {
>  	};
>  };
>  
> +&scm {
> +	qcom,dload-mode = <&tcsr_regs 0x13000>;
> +	mode-edl = <0x1>;

Why are these a part of the board DT file rather than the SoC DT? I'd
assume that at least dload-mode is generic to all devices.

> +};
> +
>  &sdhc_1 {
>  	vmmc-supply = <&pm4125_l20>;
>  	vqmmc-supply = <&pm4125_l14>;
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/5] firmware: qcom: scm: Support for EDL reboot mode
  2025-11-03 18:20 ` [PATCH 4/5] firmware: qcom: scm: Support for EDL " Loic Poulain
@ 2025-11-04  2:22   ` Bjorn Andersson
  2025-11-04 12:09   ` kernel test robot
  2025-11-04 13:13   ` kernel test robot
  2 siblings, 0 replies; 34+ messages in thread
From: Bjorn Andersson @ 2025-11-04  2:22 UTC (permalink / raw)
  To: Loic Poulain
  Cc: konradybcio, robh, krzk+dt, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

On Mon, Nov 03, 2025 at 07:20:05PM +0100, Loic Poulain wrote:
> When the dload register is specified, it can also be used to boot the
> platform into Emergency Download Mode (EDL). This mode is implemented
> by the primary ROM bootloader and provides a recovery mechanism.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/firmware/qcom/qcom_scm.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> index 26cd0458aacd..e697ef14619f 100644
> --- a/drivers/firmware/qcom/qcom_scm.c
> +++ b/drivers/firmware/qcom/qcom_scm.c
> @@ -26,6 +26,7 @@
>  #include <linux/of_platform.h>
>  #include <linux/of_reserved_mem.h>
>  #include <linux/platform_device.h>
> +#include <linux/reboot-mode.h>
>  #include <linux/reset-controller.h>
>  #include <linux/sizes.h>
>  #include <linux/types.h>
> @@ -43,6 +44,7 @@ struct qcom_scm {
>  	struct icc_path *path;
>  	struct completion waitq_comp;
>  	struct reset_controller_dev reset;
> +	struct reboot_mode_driver reboot_mode;
>  
>  	/* control access to the interconnect path */
>  	struct mutex scm_bw_lock;
> @@ -130,6 +132,8 @@ static const u8 qcom_scm_cpu_warm_bits[QCOM_SCM_BOOT_MAX_CPUS] = {
>  #define QCOM_DLOAD_MINIDUMP	2
>  #define QCOM_DLOAD_BOTHDUMP	3
>  
> +#define QCOM_EDL_MASK		BIT(0)
> +
>  static const char * const qcom_scm_convention_names[] = {
>  	[SMC_CONVENTION_UNKNOWN] = "unknown",
>  	[SMC_CONVENTION_ARM_32] = "smc arm 32",
> @@ -2206,6 +2210,18 @@ static const struct kernel_param_ops download_mode_param_ops = {
>  	.set = set_download_mode,
>  };
>  
> +static int qcom_scm_reboot_mode_write(struct reboot_mode_driver *reboot,
> +				      unsigned int magic)
> +{
> +	struct qcom_scm *scm = container_of(reboot, struct qcom_scm, reboot_mode);
> +	int ret = -EOPNOTSUPP;
> +
> +	if (scm->dload_mode_addr)
> +		ret = qcom_scm_io_rmw(scm->dload_mode_addr, QCOM_EDL_MASK, magic);
> +
> +	return ret;
> +}
> +
>  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");
>  
> @@ -2251,6 +2267,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	scm->reboot_mode.dev = &pdev->dev;
> +	scm->reboot_mode.write = qcom_scm_reboot_mode_write;
> +	ret = devm_reboot_mode_register(&pdev->dev, &scm->reboot_mode);
> +	if (ret)
> +		return ret;

I think it would be sufficient with a dev_err() here. You're loosing the
ability to set the "edl" bit but you don't break the whole system - e.g.
in the event that you're making a typo in your mode definition.


Other than that, I think this looks good.

Regards,
Bjorn

> +
>  	/* vote for max clk rate for highest performance */
>  	ret = clk_set_rate(scm->core_clk, INT_MAX);
>  	if (ret)
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-04  2:19     ` Dmitry Baryshkov
@ 2025-11-04  2:45       ` Bjorn Andersson
  0 siblings, 0 replies; 34+ messages in thread
From: Bjorn Andersson @ 2025-11-04  2:45 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Loic Poulain, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Tue, Nov 04, 2025 at 04:19:14AM +0200, Dmitry Baryshkov wrote:
> On Mon, Nov 03, 2025 at 08:16:30PM -0600, Bjorn Andersson wrote:
> > On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
> > > SCM can be used to support reboot mode such as Emergency Recovery Mode.
> > 
> > "such as"? Do we have any other useful bits in here?
> > 
> > > 
> > > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > > ---
> > >  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > > index b913192219e4..c8bb7dacd900 100644
> > > --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > > +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > > @@ -121,6 +121,10 @@ properties:
> > >            - description: offset of the download mode control register
> > >      description: TCSR hardware block
> > >  
> > > +patternProperties:
> > > +  "^mode-.*$":
> > 
> > I'd only ever expect mode-edl = <1>. Do we have additional modes that
> > warrant the generic nature of this?
> 
> fastboot / bootloader?
> 

They go in the PON_SOFT_RB_SPARE register, in the pon driver. But that
apparently doesn't tickle the EDL bit.

But it's a good question, I'd like for it to be answered in one of the
commit messages.

Regards,
Bjorn

> > 
> > Regards,
> > Bjorn
> > 
> > > +    maxItems: 1
> > > +
> > >  allOf:
> > >    # Clocks
> > >    - if:
> > > -- 
> > > 2.34.1
> > > 
> 
> -- 
> With best wishes
> Dmitry

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 5/5] arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot
  2025-11-04  2:20   ` Dmitry Baryshkov
@ 2025-11-04 10:23     ` Loic Poulain
  0 siblings, 0 replies; 34+ messages in thread
From: Loic Poulain @ 2025-11-04 10:23 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Tue, Nov 4, 2025 at 3:20 AM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Mon, Nov 03, 2025 at 07:20:06PM +0100, Loic Poulain wrote:
> > EDL reboot mode can be configured via qcom-scm, enabling the platform
> > to enter Emergency Download Mode for recovery purposes. Additionally,
> > we enable PMIC-driven warm reset as the RB1 platform's PSCI interface
> > lacks support for warm reset.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  arch/arm64/boot/dts/qcom/pm4125.dtsi     | 2 +-
> >  arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 9 +++++++++
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/pm4125.dtsi b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > index cf8c822e80ce..5d84a3250481 100644
> > --- a/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pm4125.dtsi
> > @@ -15,7 +15,7 @@ pmic@0 {
> >               #address-cells = <1>;
> >               #size-cells = <0>;
> >
> > -             pon@800 {
> > +             pon: pon@800 {
> >                       compatible = "qcom,pm8916-pon";
> >                       reg = <0x800>;
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> > index b2e0fc5501c1..70c6c929bbd3 100644
> > --- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> > +++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
> > @@ -322,6 +322,10 @@ &pm4125_vbus {
> >       status = "okay";
> >  };
> >
> > +&pon {
> > +     qcom,warm-reset;
> > +};
> > +
> >  &qupv3_id_0 {
> >       status = "okay";
> >  };
> > @@ -510,6 +514,11 @@ pm4125_l22: l22 {
> >       };
> >  };
> >
> > +&scm {
> > +     qcom,dload-mode = <&tcsr_regs 0x13000>;
> > +     mode-edl = <0x1>;
>
> Why are these a part of the board DT file rather than the SoC DT? I'd
> assume that at least dload-mode is generic to all devices.

There’s no strong reason, it’s firmware-dependent (ROM/XBL). But
indeed, this behavior appears to be common across all QCM2290-based
boards as far as I know. I’ll address it in version 2.

Thanks,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-03 18:20 ` [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset Loic Poulain
  2025-11-04  2:19   ` Bjorn Andersson
@ 2025-11-04 11:50   ` Konrad Dybcio
  2025-11-04 15:01     ` Loic Poulain
  2025-11-04 14:04   ` Krzysztof Kozlowski
  2 siblings, 1 reply; 34+ messages in thread
From: Konrad Dybcio @ 2025-11-04 11:50 UTC (permalink / raw)
  To: Loic Poulain, andersson, konradybcio, robh, krzk+dt, conor+dt,
	sre
  Cc: linux-arm-msm, devicetree, linux-pm

On 11/3/25 7:20 PM, Loic Poulain wrote:
> This mechanism can be used when firmware lacks proper warm-reset handling,
> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
> It enables the warm reset functionality via the PMIC.
> 
> This fallback is only enabled if qcom,warm-reset property is present.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> index 7e108982a582..684e9fe9987d 100644
> --- a/drivers/power/reset/qcom-pon.c
> +++ b/drivers/power/reset/qcom-pon.c
> @@ -19,12 +19,20 @@
>  
>  #define NO_REASON_SHIFT			0
>  
> +#define PON_SW_RESET_S2_CTL				0x62
> +#define		PON_SW_RESET_S2_CTL_WARM_RST	0x01
> +#define PON_SW_RESET_S2_CTL2				0x63
> +#define		PON_SW_RESET_S2_CTL2_RST_EN	BIT(7)
> +#define PON_SW_RESET_GO					0x64
> +#define		PON_SW_RESET_GO_MAGIC		0xa5

Going back to msm8974 where the SPMI arbiter first showed up, these
values are all seemingly valid, so I think we can drop the dt property.
The restart reasons are set in stone too, and you can find more of them
in the register description.

That said, we're circumventing PS_HOLD this way - is that intended?

And do we need to take any special care where there's more than one
PMIC onboard to make sure that the SoC gets properly reset?

Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/5] firmware: qcom: scm: Support for EDL reboot mode
  2025-11-03 18:20 ` [PATCH 4/5] firmware: qcom: scm: Support for EDL " Loic Poulain
  2025-11-04  2:22   ` Bjorn Andersson
@ 2025-11-04 12:09   ` kernel test robot
  2025-11-04 13:13   ` kernel test robot
  2 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2025-11-04 12:09 UTC (permalink / raw)
  To: Loic Poulain, andersson, konradybcio, robh, krzk+dt, conor+dt,
	sre
  Cc: oe-kbuild-all, linux-arm-msm, devicetree, linux-pm, Loic Poulain

Hi Loic,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on sre-power-supply/for-next linus/master v6.18-rc4 next-20251103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Loic-Poulain/dt-bindings-power-reset-qcom-pon-Document-qcom-warm-reset/20251104-022140
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20251103182006.1158383-5-loic.poulain%40oss.qualcomm.com
patch subject: [PATCH 4/5] firmware: qcom: scm: Support for EDL reboot mode
config: sparc-randconfig-001-20251104 (https://download.01.org/0day-ci/archive/20251104/202511041928.at1dqqZH-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251104/202511041928.at1dqqZH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511041928.at1dqqZH-lkp@intel.com/

All errors (new ones prefixed by >>):

   sparc64-linux-ld: drivers/spi/spi-amlogic-spifc-a4.o: in function `aml_sfc_set_bus_width':
   spi-amlogic-spifc-a4.c:(.text+0x3e8): undefined reference to `__ffsdi2'
   sparc64-linux-ld: drivers/firmware/qcom/qcom_scm.o: in function `qcom_scm_probe':
>> qcom_scm.c:(.text+0x2488): undefined reference to `devm_reboot_mode_register'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 4/5] firmware: qcom: scm: Support for EDL reboot mode
  2025-11-03 18:20 ` [PATCH 4/5] firmware: qcom: scm: Support for EDL " Loic Poulain
  2025-11-04  2:22   ` Bjorn Andersson
  2025-11-04 12:09   ` kernel test robot
@ 2025-11-04 13:13   ` kernel test robot
  2 siblings, 0 replies; 34+ messages in thread
From: kernel test robot @ 2025-11-04 13:13 UTC (permalink / raw)
  To: Loic Poulain, andersson, konradybcio, robh, krzk+dt, conor+dt,
	sre
  Cc: oe-kbuild-all, linux-arm-msm, devicetree, linux-pm, Loic Poulain

Hi Loic,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on sre-power-supply/for-next linus/master v6.18-rc4 next-20251104]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Loic-Poulain/dt-bindings-power-reset-qcom-pon-Document-qcom-warm-reset/20251104-022140
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20251103182006.1158383-5-loic.poulain%40oss.qualcomm.com
patch subject: [PATCH 4/5] firmware: qcom: scm: Support for EDL reboot mode
config: arm64-randconfig-003-20251104 (https://download.01.org/0day-ci/archive/20251104/202511042044.FqeSCxnF-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.4.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251104/202511042044.FqeSCxnF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511042044.FqeSCxnF-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "devm_reboot_mode_register" [drivers/firmware/qcom/qcom-scm.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset
  2025-11-03 18:20 ` [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset Loic Poulain
  2025-11-04  2:17   ` Bjorn Andersson
@ 2025-11-04 14:03   ` Krzysztof Kozlowski
  2025-11-04 15:12     ` Loic Poulain
  1 sibling, 1 reply; 34+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-04 14:03 UTC (permalink / raw)
  To: Loic Poulain
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Mon, Nov 03, 2025 at 07:20:02PM +0100, Loic Poulain wrote:
> This property can be used as a fallback mechanism when a warm reset
> cannot be achieved through the standard firmware interface (SCPI).
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  .../devicetree/bindings/power/reset/qcom,pon.yaml          | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> index 979a377cb4ff..ad8691c87f4f 100644
> --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> @@ -38,6 +38,13 @@ properties:
>      minItems: 1
>      maxItems: 2
>  
> +  qcom,warm-reset:
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      The PON (Power-On) peripheral provides support for warm reset, which can
> +      be used as a fallback mechanism when a warm reset cannot be achieved
> +      through the standard firmware interface.

You described the desired Linux feature or behavior, not the actual
hardware. The bindings are about the latter, so instead you need to
rephrase the property and its description to match actual hardware
capabilities/features/configuration etc.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-03 18:20 ` [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset Loic Poulain
  2025-11-04  2:19   ` Bjorn Andersson
  2025-11-04 11:50   ` Konrad Dybcio
@ 2025-11-04 14:04   ` Krzysztof Kozlowski
  2 siblings, 0 replies; 34+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-04 14:04 UTC (permalink / raw)
  To: Loic Poulain
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Mon, Nov 03, 2025 at 07:20:03PM +0100, Loic Poulain wrote:
> This mechanism can be used when firmware lacks proper warm-reset handling,
> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
> It enables the warm reset functionality via the PMIC.
> 
> This fallback is only enabled if qcom,warm-reset property is present.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> index 7e108982a582..684e9fe9987d 100644
> --- a/drivers/power/reset/qcom-pon.c
> +++ b/drivers/power/reset/qcom-pon.c
> @@ -19,12 +19,20 @@
>  
>  #define NO_REASON_SHIFT			0
>  
> +#define PON_SW_RESET_S2_CTL				0x62
> +#define		PON_SW_RESET_S2_CTL_WARM_RST	0x01
> +#define PON_SW_RESET_S2_CTL2				0x63
> +#define		PON_SW_RESET_S2_CTL2_RST_EN	BIT(7)
> +#define PON_SW_RESET_GO					0x64
> +#define		PON_SW_RESET_GO_MAGIC		0xa5
> +
>  struct qcom_pon {
>  	struct device *dev;
>  	struct regmap *regmap;
>  	u32 baseaddr;
>  	struct reboot_mode_driver reboot_mode;
>  	long reason_shift;
> +	bool warm_reset;
>  };
>  
>  static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
> @@ -44,6 +52,37 @@ static int qcom_pon_reboot_mode_write(struct reboot_mode_driver *reboot,
>  	return ret;
>  }
>  
> +static int pm8916_pon_reset(struct sys_off_data *data)
> +{
> +	struct qcom_pon *pon = data->cb_data;
> +	int ret;
> +
> +	if (!pon->warm_reset || data->mode != REBOOT_WARM)
> +		return NOTIFY_DONE;

And here is proof from the binding - you just tell what the drivers
should do.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-04 11:50   ` Konrad Dybcio
@ 2025-11-04 15:01     ` Loic Poulain
  2025-11-04 15:20       ` Konrad Dybcio
  0 siblings, 1 reply; 34+ messages in thread
From: Loic Poulain @ 2025-11-04 15:01 UTC (permalink / raw)
  To: Konrad Dybcio, krzk+dt
  Cc: andersson, konradybcio, robh, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

Hi Konrad, Krzysztof,

On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 11/3/25 7:20 PM, Loic Poulain wrote:
> > This mechanism can be used when firmware lacks proper warm-reset handling,
> > for example, when the PSCI SYSTEM_RESET2 function is not implemented.
> > It enables the warm reset functionality via the PMIC.
> >
> > This fallback is only enabled if qcom,warm-reset property is present.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> >
> > diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> > index 7e108982a582..684e9fe9987d 100644
> > --- a/drivers/power/reset/qcom-pon.c
> > +++ b/drivers/power/reset/qcom-pon.c
> > @@ -19,12 +19,20 @@
> >
> >  #define NO_REASON_SHIFT                      0
> >
> > +#define PON_SW_RESET_S2_CTL                          0x62
> > +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
> > +#define PON_SW_RESET_S2_CTL2                         0x63
> > +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
> > +#define PON_SW_RESET_GO                                      0x64
> > +#define              PON_SW_RESET_GO_MAGIC           0xa5
>
> Going back to msm8974 where the SPMI arbiter first showed up, these
> values are all seemingly valid, so I think we can drop the dt property.
> The restart reasons are set in stone too, and you can find more of them
> in the register description.

Yes, but this should only apply when the platform firmware does not
support warm reset via PSCI, right?
Making it unconditional would override the PSCI implementation even
when warm reset is supported.

The point is that psci_sys_reset() executes a cold reset if warm
reset isn’t available. Therefore, our PMIC reboot notifier must have a
higher priority than the PSCI handler.

So maybe the alternative could be to introduce an additional reboot
handler in psci, with the lowest priority, so that warm reset can have
a chance to run either from the psci main reboot handler or from the
PMIC reboot handler before falling back to cold reset?
[PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]

> That said, we're circumventing PS_HOLD this way - is that intended?

Well, we don’t have direct control over PS_HOLD since it’s managed by
the firmware in our case. That’s why I considered using the PMIC
software reset as an effective way to achieve this warm reset.

> And do we need to take any special care where there's more than one
> PMIC onboard to make sure that the SoC gets properly reset?

Good point, this likely only matters if the other PMIC reboot handler
performs the same type of reset and their actions overlap.
In that case, I may need to remove the mdelay from this handler.
Additionally, if we adopt the PSCI change discussed above, the system
will fall back to a cold reset when a PMIC-based reset isn’t possible.

Regards,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset
  2025-11-04 14:03   ` Krzysztof Kozlowski
@ 2025-11-04 15:12     ` Loic Poulain
  0 siblings, 0 replies; 34+ messages in thread
From: Loic Poulain @ 2025-11-04 15:12 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Tue, Nov 4, 2025 at 3:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Nov 03, 2025 at 07:20:02PM +0100, Loic Poulain wrote:
> > This property can be used as a fallback mechanism when a warm reset
> > cannot be achieved through the standard firmware interface (SCPI).
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  .../devicetree/bindings/power/reset/qcom,pon.yaml          | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> > index 979a377cb4ff..ad8691c87f4f 100644
> > --- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> > +++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
> > @@ -38,6 +38,13 @@ properties:
> >      minItems: 1
> >      maxItems: 2
> >
> > +  qcom,warm-reset:
> > +    description: |
>
> Do not need '|' unless you need to preserve formatting.
>
> > +      The PON (Power-On) peripheral provides support for warm reset, which can
> > +      be used as a fallback mechanism when a warm reset cannot be achieved
> > +      through the standard firmware interface.
>
> You described the desired Linux feature or behavior, not the actual
> hardware. The bindings are about the latter, so instead you need to
> rephrase the property and its description to match actual hardware
> capabilities/features/configuration etc.

I could argue that this property is meant to describe hardware support
for warm reset, but since all platforms support it, it effectively
becomes a way to configure behavior rather than indicate hardware
capability.

I’ll try to discuss and find an alternative approach that avoids
introducing this extra property.

Thanks,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-04 15:01     ` Loic Poulain
@ 2025-11-04 15:20       ` Konrad Dybcio
  2025-11-05 21:44         ` Loic Poulain
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Dybcio @ 2025-11-04 15:20 UTC (permalink / raw)
  To: Loic Poulain, krzk+dt
  Cc: andersson, konradybcio, robh, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

On 11/4/25 4:01 PM, Loic Poulain wrote:
> Hi Konrad, Krzysztof,
> 
> On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 11/3/25 7:20 PM, Loic Poulain wrote:
>>> This mechanism can be used when firmware lacks proper warm-reset handling,
>>> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
>>> It enables the warm reset functionality via the PMIC.
>>>
>>> This fallback is only enabled if qcom,warm-reset property is present.
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>> ---
>>>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
>>>  1 file changed, 47 insertions(+)
>>>
>>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
>>> index 7e108982a582..684e9fe9987d 100644
>>> --- a/drivers/power/reset/qcom-pon.c
>>> +++ b/drivers/power/reset/qcom-pon.c
>>> @@ -19,12 +19,20 @@
>>>
>>>  #define NO_REASON_SHIFT                      0
>>>
>>> +#define PON_SW_RESET_S2_CTL                          0x62
>>> +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
>>> +#define PON_SW_RESET_S2_CTL2                         0x63
>>> +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
>>> +#define PON_SW_RESET_GO                                      0x64
>>> +#define              PON_SW_RESET_GO_MAGIC           0xa5
>>
>> Going back to msm8974 where the SPMI arbiter first showed up, these
>> values are all seemingly valid, so I think we can drop the dt property.
>> The restart reasons are set in stone too, and you can find more of them
>> in the register description.
> 
> Yes, but this should only apply when the platform firmware does not
> support warm reset via PSCI, right?
> Making it unconditional would override the PSCI implementation even
> when warm reset is supported.
> 
> The point is that psci_sys_reset() executes a cold reset if warm
> reset isn’t available. Therefore, our PMIC reboot notifier must have a
> higher priority than the PSCI handler.
> 
> So maybe the alternative could be to introduce an additional reboot
> handler in psci, with the lowest priority, so that warm reset can have
> a chance to run either from the psci main reboot handler or from the
> PMIC reboot handler before falling back to cold reset?
> [PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]

This seems like a common enough problem, perhaps the framework could
accept EOPNOTSUPP or similar and try to delegate further, coming back
with a normal restart or so, if unsupported. Trying to make a special
contract between qcom-pon and psci silently will be very fragile
otherwise.

>> That said, we're circumventing PS_HOLD this way - is that intended?
> 
> Well, we don’t have direct control over PS_HOLD since it’s managed by
> the firmware in our case. That’s why I considered using the PMIC
> software reset as an effective way to achieve this warm reset.

Hm, so is there no longer a way to assert it by writing to PMIC
registers?

>> And do we need to take any special care where there's more than one
>> PMIC onboard to make sure that the SoC gets properly reset?
> 
> Good point, this likely only matters if the other PMIC reboot handler
> performs the same type of reset and their actions overlap.

I think there used to be some logic in the older days where Linux would
manually go over all PMICs with a PON and send a reset to them. Maybe
that's too old for this platform though.

Konrad

> In that case, I may need to remove the mdelay from this handler.
> Additionally, if we adopt the PSCI change discussed above, the system
> will fall back to a cold reset when a PMIC-based reset isn’t possible.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-04  2:16   ` Bjorn Andersson
  2025-11-04  2:19     ` Dmitry Baryshkov
@ 2025-11-04 21:19     ` Loic Poulain
  2025-11-05  9:44       ` Konrad Dybcio
  1 sibling, 1 reply; 34+ messages in thread
From: Loic Poulain @ 2025-11-04 21:19 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: konradybcio, robh, krzk+dt, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

On Tue, Nov 4, 2025 at 3:12 AM Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
> > SCM can be used to support reboot mode such as Emergency Recovery Mode.
>
> "such as"? Do we have any other useful bits in here?

 I heard we may have different EDL modes supported like USB or SD
based EDL, but I don't know much about the details.

>
> >
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > index b913192219e4..c8bb7dacd900 100644
> > --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> > @@ -121,6 +121,10 @@ properties:
> >            - description: offset of the download mode control register
> >      description: TCSR hardware block
> >
> > +patternProperties:
> > +  "^mode-.*$":
>
> I'd only ever expect mode-edl = <1>. Do we have additional modes that
> warrant the generic nature of this?

We may extend this to mode-ramdump if it makes sense, but as of now
it's indeed only edl, will fix it.

Regards,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-04 21:19     ` Loic Poulain
@ 2025-11-05  9:44       ` Konrad Dybcio
  2025-11-05 20:17         ` Loic Poulain
  2025-11-12 16:36         ` Bjorn Andersson
  0 siblings, 2 replies; 34+ messages in thread
From: Konrad Dybcio @ 2025-11-05  9:44 UTC (permalink / raw)
  To: Loic Poulain, Bjorn Andersson
  Cc: konradybcio, robh, krzk+dt, conor+dt, sre, linux-arm-msm,
	devicetree, linux-pm

On 11/4/25 10:19 PM, Loic Poulain wrote:
> On Tue, Nov 4, 2025 at 3:12 AM Bjorn Andersson <andersson@kernel.org> wrote:
>>
>> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
>>> SCM can be used to support reboot mode such as Emergency Recovery Mode.
>>
>> "such as"? Do we have any other useful bits in here?
> 
>  I heard we may have different EDL modes supported like USB or SD
> based EDL, but I don't know much about the details.
> 
>>
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>> ---
>>>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>> index b913192219e4..c8bb7dacd900 100644
>>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>> @@ -121,6 +121,10 @@ properties:
>>>            - description: offset of the download mode control register
>>>      description: TCSR hardware block
>>>
>>> +patternProperties:
>>> +  "^mode-.*$":
>>
>> I'd only ever expect mode-edl = <1>. Do we have additional modes that
>> warrant the generic nature of this?
> 
> We may extend this to mode-ramdump if it makes sense, but as of now
> it's indeed only edl, will fix it.

Would adding ramdump here be a matter of:

+ mode-ramdump = <0xmagic>

?

If so, please add it too

Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-05  9:44       ` Konrad Dybcio
@ 2025-11-05 20:17         ` Loic Poulain
  2025-11-06  9:51           ` Konrad Dybcio
  2025-11-12 16:36         ` Bjorn Andersson
  1 sibling, 1 reply; 34+ messages in thread
From: Loic Poulain @ 2025-11-05 20:17 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Bjorn Andersson, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Wed, Nov 5, 2025 at 10:44 AM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 11/4/25 10:19 PM, Loic Poulain wrote:
> > On Tue, Nov 4, 2025 at 3:12 AM Bjorn Andersson <andersson@kernel.org> wrote:
> >>
> >> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
> >>> SCM can be used to support reboot mode such as Emergency Recovery Mode.
> >>
> >> "such as"? Do we have any other useful bits in here?
> >
> >  I heard we may have different EDL modes supported like USB or SD
> > based EDL, but I don't know much about the details.
> >
> >>
> >>>
> >>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>> index b913192219e4..c8bb7dacd900 100644
> >>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>> @@ -121,6 +121,10 @@ properties:
> >>>            - description: offset of the download mode control register
> >>>      description: TCSR hardware block
> >>>
> >>> +patternProperties:
> >>> +  "^mode-.*$":
> >>
> >> I'd only ever expect mode-edl = <1>. Do we have additional modes that
> >> warrant the generic nature of this?
> >
> > We may extend this to mode-ramdump if it makes sense, but as of now
> > it's indeed only edl, will fix it.
>
> Would adding ramdump here be a matter of:
>
> + mode-ramdump = <0xmagic>

Not in its current form, I’ll need to slightly adjust the qcom,scm
driver mask for the reboot mode and also prevent the dload inhibit
during reboot. I can include these changes in v2 if that would be
helpful.

Regards,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-04 15:20       ` Konrad Dybcio
@ 2025-11-05 21:44         ` Loic Poulain
  2025-11-06 12:50           ` Konrad Dybcio
  0 siblings, 1 reply; 34+ messages in thread
From: Loic Poulain @ 2025-11-05 21:44 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: krzk+dt, andersson, konradybcio, robh, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

Hi Konrad,

On Tue, Nov 4, 2025 at 4:20 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 11/4/25 4:01 PM, Loic Poulain wrote:
> > Hi Konrad, Krzysztof,
> >
> > On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
> > <konrad.dybcio@oss.qualcomm.com> wrote:
> >>
> >> On 11/3/25 7:20 PM, Loic Poulain wrote:
> >>> This mechanism can be used when firmware lacks proper warm-reset handling,
> >>> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
> >>> It enables the warm reset functionality via the PMIC.
> >>>
> >>> This fallback is only enabled if qcom,warm-reset property is present.
> >>>
> >>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> ---
> >>>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 47 insertions(+)
> >>>
> >>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> >>> index 7e108982a582..684e9fe9987d 100644
> >>> --- a/drivers/power/reset/qcom-pon.c
> >>> +++ b/drivers/power/reset/qcom-pon.c
> >>> @@ -19,12 +19,20 @@
> >>>
> >>>  #define NO_REASON_SHIFT                      0
> >>>
> >>> +#define PON_SW_RESET_S2_CTL                          0x62
> >>> +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
> >>> +#define PON_SW_RESET_S2_CTL2                         0x63
> >>> +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
> >>> +#define PON_SW_RESET_GO                                      0x64
> >>> +#define              PON_SW_RESET_GO_MAGIC           0xa5
> >>
> >> Going back to msm8974 where the SPMI arbiter first showed up, these
> >> values are all seemingly valid, so I think we can drop the dt property.
> >> The restart reasons are set in stone too, and you can find more of them
> >> in the register description.
> >
> > Yes, but this should only apply when the platform firmware does not
> > support warm reset via PSCI, right?
> > Making it unconditional would override the PSCI implementation even
> > when warm reset is supported.
> >
> > The point is that psci_sys_reset() executes a cold reset if warm
> > reset isn’t available. Therefore, our PMIC reboot notifier must have a
> > higher priority than the PSCI handler.
> >
> > So maybe the alternative could be to introduce an additional reboot
> > handler in psci, with the lowest priority, so that warm reset can have
> > a chance to run either from the psci main reboot handler or from the
> > PMIC reboot handler before falling back to cold reset?
> > [PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]
>
> This seems like a common enough problem, perhaps the framework could
> accept EOPNOTSUPP or similar and try to delegate further, coming back
> with a normal restart or so, if unsupported. Trying to make a special
> contract between qcom-pon and psci silently will be very fragile
> otherwise.

I tested the following, as described above:
https://github.com/loicpoulain/linux/commit/5c34ea54e1a21ff1192c3c341877b24eff5f80b4
The only special 'contract' is the handler priority.
If you can elaborate on another/better approach, that would be helpful.

>
> >> That said, we're circumventing PS_HOLD this way - is that intended?
> >
> > Well, we don’t have direct control over PS_HOLD since it’s managed by
> > the firmware in our case. That’s why I considered using the PMIC
> > software reset as an effective way to achieve this warm reset.
>
> Hm, so is there no longer a way to assert it by writing to PMIC
> registers?

PS_HOLD is a SoC signal, and we can maybe assert it via the
MPM_PS_HOLD register through the msm-poweroff driver if needed (well,
if access is allowed from a non-secure world).
However, this would also require coordination with the PMIC driver to
select the correct PS_HOLD action (shutdown, cold reset, warm reset).
For that reason, I’d prefer to keep PS_HOLD based logic abstracted by PSCI.
Using the SW_RST PMIC register allows us to perform a reset without
additional signal handling.

> >> And do we need to take any special care where there's more than one
> >> PMIC onboard to make sure that the SoC gets properly reset?
> >
> > Good point, this likely only matters if the other PMIC reboot handler
> > performs the same type of reset and their actions overlap.
>
> I think there used to be some logic in the older days where Linux would
> manually go over all PMICs with a PON and send a reset to them. Maybe
> that's too old for this platform though.
>
> Konrad
>
> > In that case, I may need to remove the mdelay from this handler.
> > Additionally, if we adopt the PSCI change discussed above, the system
> > will fall back to a cold reset when a PMIC-based reset isn’t possible.

Regards,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-05 20:17         ` Loic Poulain
@ 2025-11-06  9:51           ` Konrad Dybcio
  0 siblings, 0 replies; 34+ messages in thread
From: Konrad Dybcio @ 2025-11-06  9:51 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Bjorn Andersson, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On 11/5/25 9:17 PM, Loic Poulain wrote:
> On Wed, Nov 5, 2025 at 10:44 AM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 11/4/25 10:19 PM, Loic Poulain wrote:
>>> On Tue, Nov 4, 2025 at 3:12 AM Bjorn Andersson <andersson@kernel.org> wrote:
>>>>
>>>> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
>>>>> SCM can be used to support reboot mode such as Emergency Recovery Mode.
>>>>
>>>> "such as"? Do we have any other useful bits in here?
>>>
>>>  I heard we may have different EDL modes supported like USB or SD
>>> based EDL, but I don't know much about the details.
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>>> index b913192219e4..c8bb7dacd900 100644
>>>>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>>> @@ -121,6 +121,10 @@ properties:
>>>>>            - description: offset of the download mode control register
>>>>>      description: TCSR hardware block
>>>>>
>>>>> +patternProperties:
>>>>> +  "^mode-.*$":
>>>>
>>>> I'd only ever expect mode-edl = <1>. Do we have additional modes that
>>>> warrant the generic nature of this?
>>>
>>> We may extend this to mode-ramdump if it makes sense, but as of now
>>> it's indeed only edl, will fix it.
>>
>> Would adding ramdump here be a matter of:
>>
>> + mode-ramdump = <0xmagic>
> 
> Not in its current form, I’ll need to slightly adjust the qcom,scm
> driver mask for the reboot mode and also prevent the dload inhibit
> during reboot. I can include these changes in v2 if that would be
> helpful.

Let's do it incrementally, as changing the scope usually ends up
causing chaos

Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-05 21:44         ` Loic Poulain
@ 2025-11-06 12:50           ` Konrad Dybcio
  2025-11-12 11:16             ` Loic Poulain
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Dybcio @ 2025-11-06 12:50 UTC (permalink / raw)
  To: Loic Poulain
  Cc: krzk+dt, andersson, konradybcio, robh, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On 11/5/25 10:44 PM, Loic Poulain wrote:
> Hi Konrad,
> 
> On Tue, Nov 4, 2025 at 4:20 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 11/4/25 4:01 PM, Loic Poulain wrote:
>>> Hi Konrad, Krzysztof,
>>>
>>> On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
>>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>>
>>>> On 11/3/25 7:20 PM, Loic Poulain wrote:
>>>>> This mechanism can be used when firmware lacks proper warm-reset handling,
>>>>> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
>>>>> It enables the warm reset functionality via the PMIC.
>>>>>
>>>>> This fallback is only enabled if qcom,warm-reset property is present.
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>> ---
>>>>>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 47 insertions(+)
>>>>>
>>>>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
>>>>> index 7e108982a582..684e9fe9987d 100644
>>>>> --- a/drivers/power/reset/qcom-pon.c
>>>>> +++ b/drivers/power/reset/qcom-pon.c
>>>>> @@ -19,12 +19,20 @@
>>>>>
>>>>>  #define NO_REASON_SHIFT                      0
>>>>>
>>>>> +#define PON_SW_RESET_S2_CTL                          0x62
>>>>> +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
>>>>> +#define PON_SW_RESET_S2_CTL2                         0x63
>>>>> +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
>>>>> +#define PON_SW_RESET_GO                                      0x64
>>>>> +#define              PON_SW_RESET_GO_MAGIC           0xa5
>>>>
>>>> Going back to msm8974 where the SPMI arbiter first showed up, these
>>>> values are all seemingly valid, so I think we can drop the dt property.
>>>> The restart reasons are set in stone too, and you can find more of them
>>>> in the register description.
>>>
>>> Yes, but this should only apply when the platform firmware does not
>>> support warm reset via PSCI, right?
>>> Making it unconditional would override the PSCI implementation even
>>> when warm reset is supported.
>>>
>>> The point is that psci_sys_reset() executes a cold reset if warm
>>> reset isn’t available. Therefore, our PMIC reboot notifier must have a
>>> higher priority than the PSCI handler.
>>>
>>> So maybe the alternative could be to introduce an additional reboot
>>> handler in psci, with the lowest priority, so that warm reset can have
>>> a chance to run either from the psci main reboot handler or from the
>>> PMIC reboot handler before falling back to cold reset?
>>> [PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]
>>
>> This seems like a common enough problem, perhaps the framework could
>> accept EOPNOTSUPP or similar and try to delegate further, coming back
>> with a normal restart or so, if unsupported. Trying to make a special
>> contract between qcom-pon and psci silently will be very fragile
>> otherwise.
> 
> I tested the following, as described above:
> https://github.com/loicpoulain/linux/commit/5c34ea54e1a21ff1192c3c341877b24eff5f80b4
> The only special 'contract' is the handler priority.
> If you can elaborate on another/better approach, that would be helpful.

Thinking about it again, it'd be difficult to grab some sort of a handle
to the ""parent"" reboot mode, so what you propose here is good

>>>> That said, we're circumventing PS_HOLD this way - is that intended?
>>>
>>> Well, we don’t have direct control over PS_HOLD since it’s managed by
>>> the firmware in our case. That’s why I considered using the PMIC
>>> software reset as an effective way to achieve this warm reset.
>>
>> Hm, so is there no longer a way to assert it by writing to PMIC
>> registers?
> 
> PS_HOLD is a SoC signal, and we can maybe assert it via the
> MPM_PS_HOLD register through the msm-poweroff driver if needed (well,
> if access is allowed from a non-secure world).
> However, this would also require coordination with the PMIC driver to
> select the correct PS_HOLD action (shutdown, cold reset, warm reset).
> For that reason, I’d prefer to keep PS_HOLD based logic abstracted by PSCI.
> Using the SW_RST PMIC register allows us to perform a reset without
> additional signal handling.

Yeah of course we should use PSCI where functional and available

I think PS_HOLD used to be fully manual on old (msm-3.10) platforms
through PMIC registers. I see that e.g. msm-4.19 has an SCM call to
(de)assert it. There's also a "halt PMIC arbiter" call.

(via drivers/power/reset/msm-poweroff.c)

Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-06 12:50           ` Konrad Dybcio
@ 2025-11-12 11:16             ` Loic Poulain
  2025-11-12 11:20               ` Konrad Dybcio
  0 siblings, 1 reply; 34+ messages in thread
From: Loic Poulain @ 2025-11-12 11:16 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: krzk+dt, andersson, konradybcio, robh, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

Hi Konrad,

On Thu, Nov 6, 2025 at 1:50 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 11/5/25 10:44 PM, Loic Poulain wrote:
> > Hi Konrad,
> >
> > On Tue, Nov 4, 2025 at 4:20 PM Konrad Dybcio
> > <konrad.dybcio@oss.qualcomm.com> wrote:
> >>
> >> On 11/4/25 4:01 PM, Loic Poulain wrote:
> >>> Hi Konrad, Krzysztof,
> >>>
> >>> On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
> >>> <konrad.dybcio@oss.qualcomm.com> wrote:
> >>>>
> >>>> On 11/3/25 7:20 PM, Loic Poulain wrote:
> >>>>> This mechanism can be used when firmware lacks proper warm-reset handling,
> >>>>> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
> >>>>> It enables the warm reset functionality via the PMIC.
> >>>>>
> >>>>> This fallback is only enabled if qcom,warm-reset property is present.
> >>>>>
> >>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>>>> ---
> >>>>>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
> >>>>>  1 file changed, 47 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> >>>>> index 7e108982a582..684e9fe9987d 100644
> >>>>> --- a/drivers/power/reset/qcom-pon.c
> >>>>> +++ b/drivers/power/reset/qcom-pon.c
> >>>>> @@ -19,12 +19,20 @@
> >>>>>
> >>>>>  #define NO_REASON_SHIFT                      0
> >>>>>
> >>>>> +#define PON_SW_RESET_S2_CTL                          0x62
> >>>>> +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
> >>>>> +#define PON_SW_RESET_S2_CTL2                         0x63
> >>>>> +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
> >>>>> +#define PON_SW_RESET_GO                                      0x64
> >>>>> +#define              PON_SW_RESET_GO_MAGIC           0xa5
> >>>>
> >>>> Going back to msm8974 where the SPMI arbiter first showed up, these
> >>>> values are all seemingly valid, so I think we can drop the dt property.
> >>>> The restart reasons are set in stone too, and you can find more of them
> >>>> in the register description.
> >>>
> >>> Yes, but this should only apply when the platform firmware does not
> >>> support warm reset via PSCI, right?
> >>> Making it unconditional would override the PSCI implementation even
> >>> when warm reset is supported.
> >>>
> >>> The point is that psci_sys_reset() executes a cold reset if warm
> >>> reset isn’t available. Therefore, our PMIC reboot notifier must have a
> >>> higher priority than the PSCI handler.
> >>>
> >>> So maybe the alternative could be to introduce an additional reboot
> >>> handler in psci, with the lowest priority, so that warm reset can have
> >>> a chance to run either from the psci main reboot handler or from the
> >>> PMIC reboot handler before falling back to cold reset?
> >>> [PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]
> >>
> >> This seems like a common enough problem, perhaps the framework could
> >> accept EOPNOTSUPP or similar and try to delegate further, coming back
> >> with a normal restart or so, if unsupported. Trying to make a special
> >> contract between qcom-pon and psci silently will be very fragile
> >> otherwise.
> >
> > I tested the following, as described above:
> > https://github.com/loicpoulain/linux/commit/5c34ea54e1a21ff1192c3c341877b24eff5f80b4
> > The only special 'contract' is the handler priority.
> > If you can elaborate on another/better approach, that would be helpful.
>
> Thinking about it again, it'd be difficult to grab some sort of a handle
> to the ""parent"" reboot mode, so what you propose here is good
>
> >>>> That said, we're circumventing PS_HOLD this way - is that intended?
> >>>
> >>> Well, we don’t have direct control over PS_HOLD since it’s managed by
> >>> the firmware in our case. That’s why I considered using the PMIC
> >>> software reset as an effective way to achieve this warm reset.
> >>
> >> Hm, so is there no longer a way to assert it by writing to PMIC
> >> registers?
> >
> > PS_HOLD is a SoC signal, and we can maybe assert it via the
> > MPM_PS_HOLD register through the msm-poweroff driver if needed (well,
> > if access is allowed from a non-secure world).
> > However, this would also require coordination with the PMIC driver to
> > select the correct PS_HOLD action (shutdown, cold reset, warm reset).
> > For that reason, I’d prefer to keep PS_HOLD based logic abstracted by PSCI.
> > Using the SW_RST PMIC register allows us to perform a reset without
> > additional signal handling.
>
> Yeah of course we should use PSCI where functional and available
>
> I think PS_HOLD used to be fully manual on old (msm-3.10) platforms
> through PMIC registers. I see that e.g. msm-4.19 has an SCM call to
> (de)assert it. There's also a "halt PMIC arbiter" call.
>
> (via drivers/power/reset/msm-poweroff.c)

Yes I could try the SCM call to deassert PS_HOLD, is it something we
should prefer over PMIC soft reset?
Asking because the implication would be a more complex solution
(though not yet tested):
- Adding reboot mode support in qcom-scm to activate ELD mode
- Adding reset-notifier in pmic driver to modify PS_HOLD action to warm-reset
- Adding reset-notifier in qcom,scm (of lower priority than PMIC)
doing the actual SCM ps-hold deassert
- Ensuring that PSCI is still used for cold-reset and warm-reset when
supported...

Regards,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-12 11:16             ` Loic Poulain
@ 2025-11-12 11:20               ` Konrad Dybcio
  2025-11-14 15:35                 ` Loic Poulain
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Dybcio @ 2025-11-12 11:20 UTC (permalink / raw)
  To: Loic Poulain
  Cc: krzk+dt, andersson, konradybcio, robh, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On 11/12/25 12:16 PM, Loic Poulain wrote:
> Hi Konrad,
> 
> On Thu, Nov 6, 2025 at 1:50 PM Konrad Dybcio
> <konrad.dybcio@oss.qualcomm.com> wrote:
>>
>> On 11/5/25 10:44 PM, Loic Poulain wrote:
>>> Hi Konrad,
>>>
>>> On Tue, Nov 4, 2025 at 4:20 PM Konrad Dybcio
>>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>>
>>>> On 11/4/25 4:01 PM, Loic Poulain wrote:
>>>>> Hi Konrad, Krzysztof,
>>>>>
>>>>> On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
>>>>> <konrad.dybcio@oss.qualcomm.com> wrote:
>>>>>>
>>>>>> On 11/3/25 7:20 PM, Loic Poulain wrote:
>>>>>>> This mechanism can be used when firmware lacks proper warm-reset handling,
>>>>>>> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
>>>>>>> It enables the warm reset functionality via the PMIC.
>>>>>>>
>>>>>>> This fallback is only enabled if qcom,warm-reset property is present.
>>>>>>>
>>>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>>>> ---
>>>>>>>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 47 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
>>>>>>> index 7e108982a582..684e9fe9987d 100644
>>>>>>> --- a/drivers/power/reset/qcom-pon.c
>>>>>>> +++ b/drivers/power/reset/qcom-pon.c
>>>>>>> @@ -19,12 +19,20 @@
>>>>>>>
>>>>>>>  #define NO_REASON_SHIFT                      0
>>>>>>>
>>>>>>> +#define PON_SW_RESET_S2_CTL                          0x62
>>>>>>> +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
>>>>>>> +#define PON_SW_RESET_S2_CTL2                         0x63
>>>>>>> +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
>>>>>>> +#define PON_SW_RESET_GO                                      0x64
>>>>>>> +#define              PON_SW_RESET_GO_MAGIC           0xa5
>>>>>>
>>>>>> Going back to msm8974 where the SPMI arbiter first showed up, these
>>>>>> values are all seemingly valid, so I think we can drop the dt property.
>>>>>> The restart reasons are set in stone too, and you can find more of them
>>>>>> in the register description.
>>>>>
>>>>> Yes, but this should only apply when the platform firmware does not
>>>>> support warm reset via PSCI, right?
>>>>> Making it unconditional would override the PSCI implementation even
>>>>> when warm reset is supported.
>>>>>
>>>>> The point is that psci_sys_reset() executes a cold reset if warm
>>>>> reset isn’t available. Therefore, our PMIC reboot notifier must have a
>>>>> higher priority than the PSCI handler.
>>>>>
>>>>> So maybe the alternative could be to introduce an additional reboot
>>>>> handler in psci, with the lowest priority, so that warm reset can have
>>>>> a chance to run either from the psci main reboot handler or from the
>>>>> PMIC reboot handler before falling back to cold reset?
>>>>> [PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]
>>>>
>>>> This seems like a common enough problem, perhaps the framework could
>>>> accept EOPNOTSUPP or similar and try to delegate further, coming back
>>>> with a normal restart or so, if unsupported. Trying to make a special
>>>> contract between qcom-pon and psci silently will be very fragile
>>>> otherwise.
>>>
>>> I tested the following, as described above:
>>> https://github.com/loicpoulain/linux/commit/5c34ea54e1a21ff1192c3c341877b24eff5f80b4
>>> The only special 'contract' is the handler priority.
>>> If you can elaborate on another/better approach, that would be helpful.
>>
>> Thinking about it again, it'd be difficult to grab some sort of a handle
>> to the ""parent"" reboot mode, so what you propose here is good
>>
>>>>>> That said, we're circumventing PS_HOLD this way - is that intended?
>>>>>
>>>>> Well, we don’t have direct control over PS_HOLD since it’s managed by
>>>>> the firmware in our case. That’s why I considered using the PMIC
>>>>> software reset as an effective way to achieve this warm reset.
>>>>
>>>> Hm, so is there no longer a way to assert it by writing to PMIC
>>>> registers?
>>>
>>> PS_HOLD is a SoC signal, and we can maybe assert it via the
>>> MPM_PS_HOLD register through the msm-poweroff driver if needed (well,
>>> if access is allowed from a non-secure world).
>>> However, this would also require coordination with the PMIC driver to
>>> select the correct PS_HOLD action (shutdown, cold reset, warm reset).
>>> For that reason, I’d prefer to keep PS_HOLD based logic abstracted by PSCI.
>>> Using the SW_RST PMIC register allows us to perform a reset without
>>> additional signal handling.
>>
>> Yeah of course we should use PSCI where functional and available
>>
>> I think PS_HOLD used to be fully manual on old (msm-3.10) platforms
>> through PMIC registers. I see that e.g. msm-4.19 has an SCM call to
>> (de)assert it. There's also a "halt PMIC arbiter" call.
>>
>> (via drivers/power/reset/msm-poweroff.c)
> 
> Yes I could try the SCM call to deassert PS_HOLD, is it something we
> should prefer over PMIC soft reset?
> Asking because the implication would be a more complex solution
> (though not yet tested):
> - Adding reboot mode support in qcom-scm to activate ELD mode
> - Adding reset-notifier in pmic driver to modify PS_HOLD action to warm-reset
> - Adding reset-notifier in qcom,scm (of lower priority than PMIC)
> doing the actual SCM ps-hold deassert
> - Ensuring that PSCI is still used for cold-reset and warm-reset when
> supported...

My answer is unfortunately "I don't know". We should loop in some
PMIC folks that would know the difference

Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-05  9:44       ` Konrad Dybcio
  2025-11-05 20:17         ` Loic Poulain
@ 2025-11-12 16:36         ` Bjorn Andersson
  2025-11-13 11:00           ` Konrad Dybcio
  1 sibling, 1 reply; 34+ messages in thread
From: Bjorn Andersson @ 2025-11-12 16:36 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Loic Poulain, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Wed, Nov 05, 2025 at 10:44:05AM +0100, Konrad Dybcio wrote:
> On 11/4/25 10:19 PM, Loic Poulain wrote:
> > On Tue, Nov 4, 2025 at 3:12 AM Bjorn Andersson <andersson@kernel.org> wrote:
> >>
> >> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
> >>> SCM can be used to support reboot mode such as Emergency Recovery Mode.
> >>
> >> "such as"? Do we have any other useful bits in here?
> > 
> >  I heard we may have different EDL modes supported like USB or SD
> > based EDL, but I don't know much about the details.
> > 
> >>
> >>>
> >>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>> index b913192219e4..c8bb7dacd900 100644
> >>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>> @@ -121,6 +121,10 @@ properties:
> >>>            - description: offset of the download mode control register
> >>>      description: TCSR hardware block
> >>>
> >>> +patternProperties:
> >>> +  "^mode-.*$":
> >>
> >> I'd only ever expect mode-edl = <1>. Do we have additional modes that
> >> warrant the generic nature of this?
> > 
> > We may extend this to mode-ramdump if it makes sense, but as of now
> > it's indeed only edl, will fix it.
> 
> Would adding ramdump here be a matter of:
> 
> + mode-ramdump = <0xmagic>
> 
> ?
> 
> If so, please add it too
> 

But what does that mean? "Hey computer, perform a graceful shutdown and
when you're done, give me a ramdump"?

Regards,
Bjorn

> Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-12 16:36         ` Bjorn Andersson
@ 2025-11-13 11:00           ` Konrad Dybcio
  2025-11-13 17:38             ` Bjorn Andersson
  0 siblings, 1 reply; 34+ messages in thread
From: Konrad Dybcio @ 2025-11-13 11:00 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Loic Poulain, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On 11/12/25 5:36 PM, Bjorn Andersson wrote:
> On Wed, Nov 05, 2025 at 10:44:05AM +0100, Konrad Dybcio wrote:
>> On 11/4/25 10:19 PM, Loic Poulain wrote:
>>> On Tue, Nov 4, 2025 at 3:12 AM Bjorn Andersson <andersson@kernel.org> wrote:
>>>>
>>>> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
>>>>> SCM can be used to support reboot mode such as Emergency Recovery Mode.
>>>>
>>>> "such as"? Do we have any other useful bits in here?
>>>
>>>  I heard we may have different EDL modes supported like USB or SD
>>> based EDL, but I don't know much about the details.
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>>> index b913192219e4..c8bb7dacd900 100644
>>>>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
>>>>> @@ -121,6 +121,10 @@ properties:
>>>>>            - description: offset of the download mode control register
>>>>>      description: TCSR hardware block
>>>>>
>>>>> +patternProperties:
>>>>> +  "^mode-.*$":
>>>>
>>>> I'd only ever expect mode-edl = <1>. Do we have additional modes that
>>>> warrant the generic nature of this?
>>>
>>> We may extend this to mode-ramdump if it makes sense, but as of now
>>> it's indeed only edl, will fix it.
>>
>> Would adding ramdump here be a matter of:
>>
>> + mode-ramdump = <0xmagic>
>>
>> ?
>>
>> If so, please add it too
>>
> 
> But what does that mean? "Hey computer, perform a graceful shutdown and
> when you're done, give me a ramdump"?

I.. guess?

Perhaps it could be useful for registering a panic handler to reboot
into ramdump in case that's not enabled by deafult (but is that
possible with our fw?)

Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode
  2025-11-13 11:00           ` Konrad Dybcio
@ 2025-11-13 17:38             ` Bjorn Andersson
  0 siblings, 0 replies; 34+ messages in thread
From: Bjorn Andersson @ 2025-11-13 17:38 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Loic Poulain, konradybcio, robh, krzk+dt, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

On Thu, Nov 13, 2025 at 12:00:40PM +0100, Konrad Dybcio wrote:
> On 11/12/25 5:36 PM, Bjorn Andersson wrote:
> > On Wed, Nov 05, 2025 at 10:44:05AM +0100, Konrad Dybcio wrote:
> >> On 11/4/25 10:19 PM, Loic Poulain wrote:
> >>> On Tue, Nov 4, 2025 at 3:12 AM Bjorn Andersson <andersson@kernel.org> wrote:
> >>>>
> >>>> On Mon, Nov 03, 2025 at 07:20:04PM +0100, Loic Poulain wrote:
> >>>>> SCM can be used to support reboot mode such as Emergency Recovery Mode.
> >>>>
> >>>> "such as"? Do we have any other useful bits in here?
> >>>
> >>>  I heard we may have different EDL modes supported like USB or SD
> >>> based EDL, but I don't know much about the details.
> >>>
> >>>>
> >>>>>
> >>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>>>> ---
> >>>>>  Documentation/devicetree/bindings/firmware/qcom,scm.yaml | 4 ++++
> >>>>>  1 file changed, 4 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>>>> index b913192219e4..c8bb7dacd900 100644
> >>>>> --- a/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/firmware/qcom,scm.yaml
> >>>>> @@ -121,6 +121,10 @@ properties:
> >>>>>            - description: offset of the download mode control register
> >>>>>      description: TCSR hardware block
> >>>>>
> >>>>> +patternProperties:
> >>>>> +  "^mode-.*$":
> >>>>
> >>>> I'd only ever expect mode-edl = <1>. Do we have additional modes that
> >>>> warrant the generic nature of this?
> >>>
> >>> We may extend this to mode-ramdump if it makes sense, but as of now
> >>> it's indeed only edl, will fix it.
> >>
> >> Would adding ramdump here be a matter of:
> >>
> >> + mode-ramdump = <0xmagic>
> >>
> >> ?
> >>
> >> If so, please add it too
> >>
> > 
> > But what does that mean? "Hey computer, perform a graceful shutdown and
> > when you're done, give me a ramdump"?
> 
> I.. guess?
> 
> Perhaps it could be useful for registering a panic handler to reboot
> into ramdump in case that's not enabled by deafult (but is that
> possible with our fw?)
> 

That should be covered the other way around today, if the user asks for
ramdumps to be collected then we set that up at boot, which we then
clear on a clean shutdown.

So, that should cover the panic() case as well - although I've not
figured out how to load the ramdump in crash, so it's been a long while
since I had reason to test this myself.

Regards,
Bjorn

> Konrad

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset
  2025-11-12 11:20               ` Konrad Dybcio
@ 2025-11-14 15:35                 ` Loic Poulain
  0 siblings, 0 replies; 34+ messages in thread
From: Loic Poulain @ 2025-11-14 15:35 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: krzk+dt, andersson, konradybcio, robh, conor+dt, sre,
	linux-arm-msm, devicetree, linux-pm

Hi Konrad,

On Wed, Nov 12, 2025 at 12:20 PM Konrad Dybcio
<konrad.dybcio@oss.qualcomm.com> wrote:
>
> On 11/12/25 12:16 PM, Loic Poulain wrote:
> > Hi Konrad,
> >
> > On Thu, Nov 6, 2025 at 1:50 PM Konrad Dybcio
> > <konrad.dybcio@oss.qualcomm.com> wrote:
> >>
> >> On 11/5/25 10:44 PM, Loic Poulain wrote:
> >>> Hi Konrad,
> >>>
> >>> On Tue, Nov 4, 2025 at 4:20 PM Konrad Dybcio
> >>> <konrad.dybcio@oss.qualcomm.com> wrote:
> >>>>
> >>>> On 11/4/25 4:01 PM, Loic Poulain wrote:
> >>>>> Hi Konrad, Krzysztof,
> >>>>>
> >>>>> On Tue, Nov 4, 2025 at 12:50 PM Konrad Dybcio
> >>>>> <konrad.dybcio@oss.qualcomm.com> wrote:
> >>>>>>
> >>>>>> On 11/3/25 7:20 PM, Loic Poulain wrote:
> >>>>>>> This mechanism can be used when firmware lacks proper warm-reset handling,
> >>>>>>> for example, when the PSCI SYSTEM_RESET2 function is not implemented.
> >>>>>>> It enables the warm reset functionality via the PMIC.
> >>>>>>>
> >>>>>>> This fallback is only enabled if qcom,warm-reset property is present.
> >>>>>>>
> >>>>>>> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> >>>>>>> ---
> >>>>>>>  drivers/power/reset/qcom-pon.c | 47 ++++++++++++++++++++++++++++++++++
> >>>>>>>  1 file changed, 47 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/power/reset/qcom-pon.c b/drivers/power/reset/qcom-pon.c
> >>>>>>> index 7e108982a582..684e9fe9987d 100644
> >>>>>>> --- a/drivers/power/reset/qcom-pon.c
> >>>>>>> +++ b/drivers/power/reset/qcom-pon.c
> >>>>>>> @@ -19,12 +19,20 @@
> >>>>>>>
> >>>>>>>  #define NO_REASON_SHIFT                      0
> >>>>>>>
> >>>>>>> +#define PON_SW_RESET_S2_CTL                          0x62
> >>>>>>> +#define              PON_SW_RESET_S2_CTL_WARM_RST    0x01
> >>>>>>> +#define PON_SW_RESET_S2_CTL2                         0x63
> >>>>>>> +#define              PON_SW_RESET_S2_CTL2_RST_EN     BIT(7)
> >>>>>>> +#define PON_SW_RESET_GO                                      0x64
> >>>>>>> +#define              PON_SW_RESET_GO_MAGIC           0xa5
> >>>>>>
> >>>>>> Going back to msm8974 where the SPMI arbiter first showed up, these
> >>>>>> values are all seemingly valid, so I think we can drop the dt property.
> >>>>>> The restart reasons are set in stone too, and you can find more of them
> >>>>>> in the register description.
> >>>>>
> >>>>> Yes, but this should only apply when the platform firmware does not
> >>>>> support warm reset via PSCI, right?
> >>>>> Making it unconditional would override the PSCI implementation even
> >>>>> when warm reset is supported.
> >>>>>
> >>>>> The point is that psci_sys_reset() executes a cold reset if warm
> >>>>> reset isn’t available. Therefore, our PMIC reboot notifier must have a
> >>>>> higher priority than the PSCI handler.
> >>>>>
> >>>>> So maybe the alternative could be to introduce an additional reboot
> >>>>> handler in psci, with the lowest priority, so that warm reset can have
> >>>>> a chance to run either from the psci main reboot handler or from the
> >>>>> PMIC reboot handler before falling back to cold reset?
> >>>>> [PSCI-handler]->[other-handlers]->[PSCI-cold-reset-fallback-handler]
> >>>>
> >>>> This seems like a common enough problem, perhaps the framework could
> >>>> accept EOPNOTSUPP or similar and try to delegate further, coming back
> >>>> with a normal restart or so, if unsupported. Trying to make a special
> >>>> contract between qcom-pon and psci silently will be very fragile
> >>>> otherwise.
> >>>
> >>> I tested the following, as described above:
> >>> https://github.com/loicpoulain/linux/commit/5c34ea54e1a21ff1192c3c341877b24eff5f80b4
> >>> The only special 'contract' is the handler priority.
> >>> If you can elaborate on another/better approach, that would be helpful.
> >>
> >> Thinking about it again, it'd be difficult to grab some sort of a handle
> >> to the ""parent"" reboot mode, so what you propose here is good
> >>
> >>>>>> That said, we're circumventing PS_HOLD this way - is that intended?
> >>>>>
> >>>>> Well, we don’t have direct control over PS_HOLD since it’s managed by
> >>>>> the firmware in our case. That’s why I considered using the PMIC
> >>>>> software reset as an effective way to achieve this warm reset.
> >>>>
> >>>> Hm, so is there no longer a way to assert it by writing to PMIC
> >>>> registers?
> >>>
> >>> PS_HOLD is a SoC signal, and we can maybe assert it via the
> >>> MPM_PS_HOLD register through the msm-poweroff driver if needed (well,
> >>> if access is allowed from a non-secure world).
> >>> However, this would also require coordination with the PMIC driver to
> >>> select the correct PS_HOLD action (shutdown, cold reset, warm reset).
> >>> For that reason, I’d prefer to keep PS_HOLD based logic abstracted by PSCI.
> >>> Using the SW_RST PMIC register allows us to perform a reset without
> >>> additional signal handling.
> >>
> >> Yeah of course we should use PSCI where functional and available
> >>
> >> I think PS_HOLD used to be fully manual on old (msm-3.10) platforms
> >> through PMIC registers. I see that e.g. msm-4.19 has an SCM call to
> >> (de)assert it. There's also a "halt PMIC arbiter" call.
> >>
> >> (via drivers/power/reset/msm-poweroff.c)
> >
> > Yes I could try the SCM call to deassert PS_HOLD, is it something we
> > should prefer over PMIC soft reset?
> > Asking because the implication would be a more complex solution
> > (though not yet tested):
> > - Adding reboot mode support in qcom-scm to activate ELD mode
> > - Adding reset-notifier in pmic driver to modify PS_HOLD action to warm-reset
> > - Adding reset-notifier in qcom,scm (of lower priority than PMIC)
> > doing the actual SCM ps-hold deassert
> > - Ensuring that PSCI is still used for cold-reset and warm-reset when
> > supported...
>
> My answer is unfortunately "I don't know". We should loop in some
> PMIC folks that would know the difference

I think I’ll start with a simpler patch to add support for qcom,scm reboot-mode.
Then, we can discuss the PMIC changes. I’ll also check whether it
might be easier to fix the PSCI or EFI reboot implementation on the
firmware/U-Boot side so that warm reset is handled correctly.

Regards,
Loic

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2025-11-14 15:35 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-03 18:20 [PATCH 0/5] Add EDL reboot and warm reset support for QRB2210 Loic Poulain
2025-11-03 18:20 ` [PATCH 1/5] dt-bindings: power: reset: qcom-pon: Document qcom,warm-reset Loic Poulain
2025-11-04  2:17   ` Bjorn Andersson
2025-11-04 14:03   ` Krzysztof Kozlowski
2025-11-04 15:12     ` Loic Poulain
2025-11-03 18:20 ` [PATCH 2/5] power: reset: qcom-pon: Add support for WARM reset Loic Poulain
2025-11-04  2:19   ` Bjorn Andersson
2025-11-04 11:50   ` Konrad Dybcio
2025-11-04 15:01     ` Loic Poulain
2025-11-04 15:20       ` Konrad Dybcio
2025-11-05 21:44         ` Loic Poulain
2025-11-06 12:50           ` Konrad Dybcio
2025-11-12 11:16             ` Loic Poulain
2025-11-12 11:20               ` Konrad Dybcio
2025-11-14 15:35                 ` Loic Poulain
2025-11-04 14:04   ` Krzysztof Kozlowski
2025-11-03 18:20 ` [PATCH 3/5] dt-bindings: firmware: qcom,scm: Document reboot mode Loic Poulain
2025-11-04  2:16   ` Bjorn Andersson
2025-11-04  2:19     ` Dmitry Baryshkov
2025-11-04  2:45       ` Bjorn Andersson
2025-11-04 21:19     ` Loic Poulain
2025-11-05  9:44       ` Konrad Dybcio
2025-11-05 20:17         ` Loic Poulain
2025-11-06  9:51           ` Konrad Dybcio
2025-11-12 16:36         ` Bjorn Andersson
2025-11-13 11:00           ` Konrad Dybcio
2025-11-13 17:38             ` Bjorn Andersson
2025-11-03 18:20 ` [PATCH 4/5] firmware: qcom: scm: Support for EDL " Loic Poulain
2025-11-04  2:22   ` Bjorn Andersson
2025-11-04 12:09   ` kernel test robot
2025-11-04 13:13   ` kernel test robot
2025-11-03 18:20 ` [PATCH 5/5] arm64: dts: qcom: qrb2210-rb1: Add support for EDL reboot Loic Poulain
2025-11-04  2:20   ` Dmitry Baryshkov
2025-11-04 10:23     ` Loic Poulain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).