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

From: Elliot Berman
Date: Tue Jun 13 2023 - 16:55:23 EST



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@xxxxxxx>
Signed-off-by: Elliot Berman <eberman@xxxxxxxxxxxxxx>
---
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@xxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxx>; 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@xxxxxxxxxxxxxxxxxxx>; 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@xxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxxxxxx; 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@xxxxxxx>
To: Elliot Berman <eberman@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx>
<1583435129-31356-2-git-send-email-eberman@xxxxxxxxxxxxxx>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <1583435129-31356-2-git-send-email-eberman@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxx
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@xxxxxxxxxxxxxxxxxxx?subject=unsubscribe>
List-Archive: <http://lists.infradead.org/pipermail/linux-arm-kernel/>
List-Post: <mailto:linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>
List-Help: <mailto:linux-arm-kernel-request@xxxxxxxxxxxxxxxxxxx?subject=help>
List-Subscribe: <http://lists.infradead.org/mailman/listinfo/linux-arm-kernel>,
<mailto:linux-arm-kernel-request@xxxxxxxxxxxxxxxxxxx?subject=subscribe>
Cc: Trilok Soni <tsoni@xxxxxxxxxxxxxx>,
Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>,
David Collins <collinsd@xxxxxxxxxxxxxx>, linux-arm-msm@xxxxxxxxxxxxxxx,
linux-kernel@xxxxxxxxxxxxxxx, Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>,
Sudeep Holla <sudeep.holla@xxxxxxx>, Prasad Sodagudi <psodagud@xxxxxxxxxxxxxx>,
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Sender: "linux-arm-kernel" <linux-arm-kernel-bounces@xxxxxxxxxxxxxxxxxxx>
Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@xxxxxxxxxxxxxxxxxxx

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@xxxxxxx>
Signed-off-by: Elliot Berman <eberman@xxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel