[PATCH v3 0/3] ARM PSCI: Add support for vendor-specific SYSTEM_RESET2

[PATCH v3 0/3] ARM PSCI: Add support for vendor-specific SYSTEM_RESET2

[Elliot Berman]

This patch adds support for vendor-specific SYSTEM_RESET2 to support
Qualcomm target use cases of rebooting into a RAM dump download mode.

This patch series applies on top of [1].

[1]: https://lore.kernel.org/patchwork/cover/1185759/

Changes since v2:
 - dt property forces vendor-specifc reset type

Changes since v1:
 - Address Sudeep's comments

Changes since RFC v2:
 - None, tested on SM8250 MTP

Elliot Berman (3):
  dt: psci: Add arm,psci-sys-reset2-vendor-param property
  firmware: psci: Add support for dt-supplied SYSTEM_RESET2 type
  arm64: dts: qcom: sm8250: Add vendor-specific PSCI system reset2 type

 Documentation/devicetree/bindings/arm/psci.yaml |  7 +++++++
 arch/arm64/boot/dts/qcom/sm8250.dtsi            |  1 +
 drivers/firmware/psci/psci.c                    | 21 +++++++++++++++++----
 include/uapi/linux/psci.h                       |  5 +++++
 4 files changed, 30 insertions(+), 4 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 1/3] dt: psci: Add arm,psci-sys-reset2-vendor-param property

[Elliot Berman]

Some implementors of PSCI may wish to use a different reset type than
SYSTEM_WARM_RESET. For instance, Qualcomm SoCs support an alternate
reset_type which may be used in more warm reboot scenarios than
SYSTEM_WARM_RESET permits (e.g. to reboot into recovery mode).

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Elliot Berman <eberman@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/psci.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
index 8ef8542..1a9d2dd 100644
--- a/Documentation/devicetree/bindings/arm/psci.yaml
+++ b/Documentation/devicetree/bindings/arm/psci.yaml
@@ -102,6 +102,13 @@ properties:
       [1] Kernel documentation - ARM idle states bindings
         Documentation/devicetree/bindings/arm/idle-states.txt
 
+  arm,psci-sys-reset2-vendor-param:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+        Vendor-specific reset type parameter to use for SYSTEM_RESET2 during
+        a warm or soft reboot. If no value is provided, then architectural
+        reset type SYSTEM_WARM_RESET is used.
+
   "#power-domain-cells":
     description:
       The number of cells in a PM domain specifier as per binding in [3].
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 2/3] firmware: psci: Add support for dt-supplied SYSTEM_RESET2 type

[Elliot Berman]

Some implementors of PSCI may wish to use a different reset type than
SYSTEM_WARM_RESET. For instance, Qualcomm SoCs support an alternate
reset_type which may be used in more warm reboot scenarios than
SYSTEM_WARM_RESET permits (e.g. to reboot into recovery mode).

Signed-off-by: Elliot Berman <eberman@codeaurora.org>
---
 drivers/firmware/psci/psci.c | 21 +++++++++++++++++----
 include/uapi/linux/psci.h    |  5 +++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 2937d44..43fe3af 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -90,6 +90,8 @@ static u32 psci_function_id[PSCI_FN_MAX];
 
 static u32 psci_cpu_suspend_feature;
 static bool psci_system_reset2_supported;
+static u32 psci_sys_reset2_reset_param =
+	PSCI_1_1_SYSTEM_RESET2_SYSTEM_WARM_RESET;
 
 static inline bool psci_has_ext_power_state(void)
 {
@@ -272,11 +274,10 @@ static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
 	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
 	    psci_system_reset2_supported) {
 		/*
-		 * reset_type[31] = 0 (architectural)
-		 * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
 		 * cookie = 0 (ignored by the implementation)
 		 */
-		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), 0, 0, 0);
+		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
+			       psci_sys_reset2_reset_param, 0, 0);
 	} else {
 		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
 	}
@@ -493,6 +494,7 @@ typedef int (*psci_initcall_t)(const struct device_node *);
 static int __init psci_0_2_init(struct device_node *np)
 {
 	int err;
+	u32 param;
 
 	err = get_set_conduit_method(np);
 	if (err)
@@ -505,7 +507,18 @@ static int __init psci_0_2_init(struct device_node *np)
 	 * can be carried out according to the specific version reported
 	 * by firmware
 	 */
-	return psci_probe();
+	err = psci_probe();
+	if (err)
+		return err;
+
+	if (psci_system_reset2_supported &&
+	    !of_property_read_u32(np, "arm,psci-sys-reset2-vendor-param", &param)) {
+		psci_sys_reset2_reset_param = param |
+			(PSCI_1_1_SYSTEM_RESET2_OWNER_VENDOR <<
+			 PSCI_1_1_SYSTEM_RESET2_OWNER_SHIFT);
+	}
+
+	return 0;
 }
 
 /*
diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
index 2fcad1d..0829175 100644
--- a/include/uapi/linux/psci.h
+++ b/include/uapi/linux/psci.h
@@ -55,6 +55,11 @@
 #define PSCI_1_0_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
 #define PSCI_1_1_FN64_SYSTEM_RESET2		PSCI_0_2_FN64(18)
 
+#define PSCI_1_1_SYSTEM_RESET2_OWNER_SHIFT		31
+#define PSCI_1_1_SYSTEM_RESET2_OWNER_ARCH		0
+#define PSCI_1_1_SYSTEM_RESET2_OWNER_VENDOR		1
+#define PSCI_1_1_SYSTEM_RESET2_SYSTEM_WARM_RESET	0
+
 /* PSCI v0.2 power state encoding for CPU_SUSPEND function */
 #define PSCI_0_2_POWER_STATE_ID_MASK		0xffff
 #define PSCI_0_2_POWER_STATE_ID_SHIFT		0
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v3 3/3] arm64: dts: qcom: sm8250: Add vendor-specific PSCI system reset2 type

[Elliot Berman]

Qualcomm Technologies, Inc. SoCs do not guarantee that an architectural
warm reset boots back into Linux kernel. For instance, if download mode
or reboot reason cookies are set, the SoC would do a warm reset into an
alternate exception level (e.g. a mode to collect RAM dumps) or an alternate
EL1 application (e.g. fastboot mode). Thus, Qualcomm Technologies,
Inc. SoCs support a vendor-specific warm reset type that can be used in
all instances of warm/soft reboots.

Signed-off-by: Elliot Berman <eberman@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sm8250.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index f63df12..11cede1 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -162,6 +162,7 @@
 	psci {
 		compatible = "arm,psci-1.0";
 		method = "smc";
+		arm,psci-sys-reset2-vendor-param = <0>;
 	};
 
 	reserved_memory: reserved-memory {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v3 1/3] dt: psci: Add arm,psci-sys-reset2-vendor-param property

[Mark Rutland]

Hi Elliot,

On Thu, Mar 05, 2020 at 11:05:27AM -0800, Elliot Berman wrote:
> Some implementors of PSCI may wish to use a different reset type than
> SYSTEM_WARM_RESET. For instance, Qualcomm SoCs support an alternate
> reset_type which may be used in more warm reboot scenarios than
> SYSTEM_WARM_RESET permits (e.g. to reboot into recovery mode).

To be honest, I'm still confused by this series, and I think that these
patches indicate a larger problem that we cannot solve generally (e.g.
on other platofrms and/or with ACPI).

I think the underlying issue is whether the semantics for:

a) Linux's RESET_WARM and RESET_SOFT
b) PSCI's SYSTEM_RESET2 SYSTEM_WARM_RESET

... actually align in practice, which this series suggests is not the
case.

If those don't align, then I think that commit:

  4302e381a870aafb ("firmware/psci: add support for SYSTEM_RESET2")

... is not actually reliable, and not something we can support by
default, and we should rethink the code introduce in that commit.

If (a) and (b) are supposed to align, and the behaviour on your platform
is an erratum, then I think we should treat it as such rather than
adding a property that is open to abuse.

Thoughts?

Thanks,
Mark.

> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Elliot Berman <eberman@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/psci.yaml | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
> index 8ef8542..1a9d2dd 100644
> --- a/Documentation/devicetree/bindings/arm/psci.yaml
> +++ b/Documentation/devicetree/bindings/arm/psci.yaml
> @@ -102,6 +102,13 @@ properties:
>        [1] Kernel documentation - ARM idle states bindings
>          Documentation/devicetree/bindings/arm/idle-states.txt
>  
> +  arm,psci-sys-reset2-vendor-param:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +        Vendor-specific reset type parameter to use for SYSTEM_RESET2 during
> +        a warm or soft reboot. If no value is provided, then architectural
> +        reset type SYSTEM_WARM_RESET is used.
> +
>    "#power-domain-cells":
>      description:
>        The number of cells in a PM domain specifier as per binding in [3].
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH v3 2/3] firmware: psci: Add support for dt-supplied SYSTEM_RESET2 type

[Mark Rutland]

On Thu, Mar 05, 2020 at 11:05:28AM -0800, Elliot Berman wrote:
> Some implementors of PSCI may wish to use a different reset type than
> SYSTEM_WARM_RESET. For instance, Qualcomm SoCs support an alternate
> reset_type which may be used in more warm reboot scenarios than
> SYSTEM_WARM_RESET permits (e.g. to reboot into recovery mode).
> 
> Signed-off-by: Elliot Berman <eberman@codeaurora.org>

I think we need to discuss the expected semantics on patch 1, and as
things stand, I do not want to take this patch until we understand and
agree to how things should behave.

Thanks,
Mark.

> ---
>  drivers/firmware/psci/psci.c | 21 +++++++++++++++++----
>  include/uapi/linux/psci.h    |  5 +++++
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> index 2937d44..43fe3af 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -90,6 +90,8 @@ static u32 psci_function_id[PSCI_FN_MAX];
>  
>  static u32 psci_cpu_suspend_feature;
>  static bool psci_system_reset2_supported;
> +static u32 psci_sys_reset2_reset_param =
> +	PSCI_1_1_SYSTEM_RESET2_SYSTEM_WARM_RESET;
>  
>  static inline bool psci_has_ext_power_state(void)
>  {
> @@ -272,11 +274,10 @@ static void psci_sys_reset(enum reboot_mode reboot_mode, const char *cmd)
>  	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
>  	    psci_system_reset2_supported) {
>  		/*
> -		 * reset_type[31] = 0 (architectural)
> -		 * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
>  		 * cookie = 0 (ignored by the implementation)
>  		 */
> -		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2), 0, 0, 0);
> +		invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> +			       psci_sys_reset2_reset_param, 0, 0);
>  	} else {
>  		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
>  	}
> @@ -493,6 +494,7 @@ typedef int (*psci_initcall_t)(const struct device_node *);
>  static int __init psci_0_2_init(struct device_node *np)
>  {
>  	int err;
> +	u32 param;
>  
>  	err = get_set_conduit_method(np);
>  	if (err)
> @@ -505,7 +507,18 @@ static int __init psci_0_2_init(struct device_node *np)
>  	 * can be carried out according to the specific version reported
>  	 * by firmware
>  	 */
> -	return psci_probe();
> +	err = psci_probe();
> +	if (err)
> +		return err;
> +
> +	if (psci_system_reset2_supported &&
> +	    !of_property_read_u32(np, "arm,psci-sys-reset2-vendor-param", &param)) {
> +		psci_sys_reset2_reset_param = param |
> +			(PSCI_1_1_SYSTEM_RESET2_OWNER_VENDOR <<
> +			 PSCI_1_1_SYSTEM_RESET2_OWNER_SHIFT);
> +	}
> +
> +	return 0;
>  }
>  
>  /*
> diff --git a/include/uapi/linux/psci.h b/include/uapi/linux/psci.h
> index 2fcad1d..0829175 100644
> --- a/include/uapi/linux/psci.h
> +++ b/include/uapi/linux/psci.h
> @@ -55,6 +55,11 @@
>  #define PSCI_1_0_FN64_SYSTEM_SUSPEND		PSCI_0_2_FN64(14)
>  #define PSCI_1_1_FN64_SYSTEM_RESET2		PSCI_0_2_FN64(18)
>  
> +#define PSCI_1_1_SYSTEM_RESET2_OWNER_SHIFT		31
> +#define PSCI_1_1_SYSTEM_RESET2_OWNER_ARCH		0
> +#define PSCI_1_1_SYSTEM_RESET2_OWNER_VENDOR		1
> +#define PSCI_1_1_SYSTEM_RESET2_SYSTEM_WARM_RESET	0
> +
>  /* PSCI v0.2 power state encoding for CPU_SUSPEND function */
>  #define PSCI_0_2_POWER_STATE_ID_MASK		0xffff
>  #define PSCI_0_2_POWER_STATE_ID_SHIFT		0
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH v3 1/3] dt: psci: Add arm,psci-sys-reset2-vendor-param property

[Elliot Berman]

Reviving this very old thread. In case folks don't have in their 
mailbox, here is archive link:

https://lore.kernel.org/all/20200320120105.GA36658@C02TD0UTHF1T.local/

On 3/20/2020 5:01 AM, Mark Rutland wrote:
> Hi Elliot,
> 
> On Thu, Mar 05, 2020 at 11:05:27AM -0800, Elliot Berman wrote:
>> Some implementors of PSCI may wish to use a different reset type than
>> SYSTEM_WARM_RESET. For instance, Qualcomm SoCs support an alternate
>> reset_type which may be used in more warm reboot scenarios than
>> SYSTEM_WARM_RESET permits (e.g. to reboot into recovery mode).
> 
> To be honest, I'm still confused by this series, and I think that these
> patches indicate a larger problem that we cannot solve generally (e.g.
> on other platofrms and/or with ACPI).
> 
> I think the underlying issue is whether the semantics for:
> 
> a) Linux's RESET_WARM and RESET_SOFT
> b) PSCI's SYSTEM_RESET2 SYSTEM_WARM_RESET
> 
> ... actually align in practice, which this series suggests is not the
> case.
> 
> If those don't align, then I think that commit:
> 
>    4302e381a870aafb ("firmware/psci: add support for SYSTEM_RESET2")
> 
> ... is not actually reliable, and not something we can support by
> default, and we should rethink the code introduce in that commit.
> 
> If (a) and (b) are supposed to align, and the behaviour on your platform
> is an erratum, then I think we should treat it as such rather than
> adding a property that is open to abuse.
> 
> Thoughts?
> 
I think it's ok for (a) and (b) to align (that invalidates this series). 
PSCI SYSTEM_RESET2 supports vendor-specific reset types and the PSCI 
driver doesn't have any way for these vendor-specific resets to happen.

Would a better way be to export a psci_system_reset2 function? I can 
implement a reboot-mode driver for PSCI system_reset2 for DT-based 
platforms.

Thanks,
Elliot

> Thanks,
> Mark.
> 
>>
>> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
>> Signed-off-by: Elliot Berman <eberman@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/arm/psci.yaml | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
>> index 8ef8542..1a9d2dd 100644
>> --- a/Documentation/devicetree/bindings/arm/psci.yaml
>> +++ b/Documentation/devicetree/bindings/arm/psci.yaml
>> @@ -102,6 +102,13 @@ properties:
>>         [1] Kernel documentation - ARM idle states bindings
>>           Documentation/devicetree/bindings/arm/idle-states.txt
>>   
>> +  arm,psci-sys-reset2-vendor-param:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: |
>> +        Vendor-specific reset type parameter to use for SYSTEM_RESET2 during
>> +        a warm or soft reboot. If no value is provided, then architectural
>> +        reset type SYSTEM_WARM_RESET is used.
>> +
>>     "#power-domain-cells":
>>       description:
>>         The number of cells in a PM domain specifier as per binding in [3].
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
> 
>  From mboxrd@z Thu Jan  1 00:00:00 1970
> Return-Path: <SRS0=iAha=5F=lists.infradead.org=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@kernel.org>
> X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on
> 	aws-us-west-2-korg-lkml-1.web.codeaurora.org
> X-Spam-Level:
> X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
> 	DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,
> 	SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
> 	version=3.4.0
> Received: from mail.kernel.org (mail.kernel.org [198.145.29.99])
> 	by smtp.lore.kernel.org (Postfix) with ESMTP id 4F3C9C43333
> 	for <infradead-linux-arm-kernel@archiver.kernel.org>; Fri, 20 Mar 2020 12:01:17 +0000 (UTC)
> Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133])
> 	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
> 	(No client certificate requested)
> 	by mail.kernel.org (Postfix) with ESMTPS id 1C65120732
> 	for <infradead-linux-arm-kernel@archiver.kernel.org>; Fri, 20 Mar 2020 12:01:16 +0000 (UTC)
> Authentication-Results: mail.kernel.org;
> 	dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="AoWzGqG2"
> DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1C65120732
> Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com
> Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org
> DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed;
> 	d=lists.infradead.org; s=bombadil.20170209; h=Sender:
> 	Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post:
> 	List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:
> 	Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description:
> 	Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:
> 	List-Owner; bh=15BYqh1L1ALv71IjF7NXoGOSeeS0sbsBf/ahe6hUVYY=; b=AoWzGqG2lL+PnT
> 	eHgf22G62sLTs4kKNimymlbuMng5lx29kBuft3E15X+B1hWZr4smk8/XlIARAMD2tVef89z6wBHTq
> 	Uj3FBiZzghCEKhiW+yJA2h/DxfBmBWBDmS2iPBQlehFOpftLMApK6uSGvHKbh4CjuYVS1FFjzli2g
> 	10leZnncASBmeyiZl/17d3H286+dalis6nDW6GfOYMzg/zhlZJi630/CPEsmlt/4TkIr6SQYPsfpI
> 	DtjiDGeQYCZEqNvtu22etrtCleRfqtsoLyLYBSD1zn2haDJTzhhDQgoZ8+8xr2dzNR9lnhfONmyHC
> 	8Q65eWvNSwTvJlb55XxQ==;
> Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org)
> 	by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux))
> 	id 1jFGKy-0004HY-Gg; Fri, 20 Mar 2020 12:01:16 +0000
> Received: from foss.arm.com ([217.140.110.172])
>   by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux))
>   id 1jFGKt-0004Gl-QZ
>   for linux-arm-kernel@lists.infradead.org; Fri, 20 Mar 2020 12:01:14 +0000
> Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])
>   by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7AA5D31B;
>   Fri, 20 Mar 2020 05:01:10 -0700 (PDT)
> Received: from C02TD0UTHF1T.local (usa-sjc-imap-foss1.foss.arm.com
>   [10.121.207.14])
>   by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1E89A3F85E;
>   Fri, 20 Mar 2020 05:01:07 -0700 (PDT)
> Date: Fri, 20 Mar 2020 12:01:05 +0000
> From: Mark Rutland <mark.rutland@arm.com>
> To: Elliot Berman <eberman@codeaurora.org>
> Subject: Re: [PATCH v3 1/3] dt: psci: Add arm,psci-sys-reset2-vendor-param
>   property
> Message-ID: <20200320120105.GA36658@C02TD0UTHF1T.local>
> References: <1583435129-31356-1-git-send-email-eberman@codeaurora.org>
>   <1583435129-31356-2-git-send-email-eberman@codeaurora.org>
> MIME-Version: 1.0
> Content-Disposition: inline
> In-Reply-To: <1583435129-31356-2-git-send-email-eberman@codeaurora.org>
> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3
> X-CRM114-CacheID: sfid-20200320_050112_965539_E60DC8A3
> X-CRM114-Status: GOOD (  19.35  )
> X-BeenThere: linux-arm-kernel@lists.infradead.org
> X-Mailman-Version: 2.1.29
> Precedence: list
> List-Id: <linux-arm-kernel.lists.infradead.org>
> List-Unsubscribe: <http://lists.infradead.org/mailman/options/linux-arm-kernel>,
>   <mailto:linux-arm-kernel-request@lists.infradead.org?subject=unsubscribe>
> List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
> List-Post: <mailto:linux-arm-kernel@lists.infradead.org>
> List-Help: <mailto:linux-arm-kernel-request@lists.infradead.org?subject=help>
> List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,
>   <mailto:linux-arm-kernel-request@lists.infradead.org?subject=subscribe>
> Cc: Trilok Soni <tsoni@codeaurora.org>,
>   Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
>   David Collins <collinsd@codeaurora.org>, linux-arm-msm@vger.kernel.org,
>   linux-kernel@vger.kernel.org, Bjorn Andersson <bjorn.andersson@linaro.org>,
>   Sudeep Holla <sudeep.holla@arm.com>, Prasad Sodagudi <psodagud@codeaurora.org>,
>   linux-arm-kernel@lists.infradead.org
> Content-Type: text/plain; charset="us-ascii"
> Content-Transfer-Encoding: 7bit
> Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@lists.infradead.org>
> Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org
> 
> Hi Elliot,
> 
> On Thu, Mar 05, 2020 at 11:05:27AM -0800, Elliot Berman wrote:
>> Some implementors of PSCI may wish to use a different reset type than
>> SYSTEM_WARM_RESET. For instance, Qualcomm SoCs support an alternate
>> reset_type which may be used in more warm reboot scenarios than
>> SYSTEM_WARM_RESET permits (e.g. to reboot into recovery mode).
> 
> To be honest, I'm still confused by this series, and I think that these
> patches indicate a larger problem that we cannot solve generally (e.g.
> on other platofrms and/or with ACPI).
> 
> I think the underlying issue is whether the semantics for:
> 
> a) Linux's RESET_WARM and RESET_SOFT
> b) PSCI's SYSTEM_RESET2 SYSTEM_WARM_RESET
> 
> ... actually align in practice, which this series suggests is not the
> case.
> 
> If those don't align, then I think that commit:
> 
>    4302e381a870aafb ("firmware/psci: add support for SYSTEM_RESET2")
> 
> ... is not actually reliable, and not something we can support by
> default, and we should rethink the code introduce in that commit.
> 
> If (a) and (b) are supposed to align, and the behaviour on your platform
> is an erratum, then I think we should treat it as such rather than
> adding a property that is open to abuse.
> 
> Thoughts?
> 
> Thanks,
> Mark.
> 
>>
>> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
>> Signed-off-by: Elliot Berman <eberman@codeaurora.org>
>> ---
>>   Documentation/devicetree/bindings/arm/psci.yaml | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
>> index 8ef8542..1a9d2dd 100644
>> --- a/Documentation/devicetree/bindings/arm/psci.yaml
>> +++ b/Documentation/devicetree/bindings/arm/psci.yaml
>> @@ -102,6 +102,13 @@ properties:
>>         [1] Kernel documentation - ARM idle states bindings
>>           Documentation/devicetree/bindings/arm/idle-states.txt
>>   
>> +  arm,psci-sys-reset2-vendor-param:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: |
>> +        Vendor-specific reset type parameter to use for SYSTEM_RESET2 during
>> +        a warm or soft reboot. If no value is provided, then architectural
>> +        reset type SYSTEM_WARM_RESET is used.
>> +
>>     "#power-domain-cells":
>>       description:
>>         The number of cells in a PM domain specifier as per binding in [3].
>> -- 
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>