devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2
@ 2023-11-17 21:18 Elliot Berman
  2023-11-17 21:18 ` [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Elliot Berman @ 2023-11-17 21:18 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
	Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli, Elliot Berman

The PSCI SYSTEM_RESET2 call allows vendor firmware to define additional
reset types which could be mapped to the reboot argument.

Setting up reboot on Qualcomm devices can be inconsistent from chipset
to chipset.  Generally, there is a PMIC register that gets written to
decide the reboot type. There is also sometimes a cookie that can be
written to indicate that the bootloader should behave differently than a
regular boot. These knobs evolve over product generations and require 
more drivers. Qualcomm firmwares are beginning to expose vendor
SYSTEM_RESET2 types to simplify driver requirements from Linux.

Add support in PSCI to statically wire reboot mode commands from
userspace to a vendor reset and cookie value using the device tree. The
DT bindings are similar to reboot mode framework except that 2
integers are accepted (the type and cookie). Also, reboot mode framework
is intended to program, but not reboot, the host. PSCI SYSTEM_RESET2
does both. I've not added support for reading ACPI tables since I don't
have any device which provides them + firmware that supports vendor
SYSTEM_RESET2 types.

Previous discussions around SYSTEM_RESET2:
- https://lore.kernel.org/lkml/20230724223057.1208122-2-quic_eberman@quicinc.com/T/
- https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/

This RFC approach differs from the one sent in July by:
- Not using the reboot mode framework
- Added support to control both reset type and cookie
- Implicitly dropped "normal" reboot command, which is always just
  SYSTEM_RESET anyway.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
Changes since RFC:
- Reference reboot-mode bindings as suggeted by Rob.
- Link to RFC: https://lore.kernel.org/r/20231030-arm-psci-system_reset2-vendor-reboots-v1-0-dcdd63352ad1@quicinc.com

---
Elliot Berman (3):
      dt-bindings: power: reset: Convert mode-.* properties to array
      dt-bindings: arm: Document reboot mode magic
      firmware: psci: Read and use vendor reset types

 Documentation/devicetree/bindings/arm/psci.yaml    | 36 ++++++++-
 .../bindings/power/reset/reboot-mode.yaml          |  7 +-
 drivers/firmware/psci/psci.c                       | 87 +++++++++++++++++++++-
 3 files changed, 125 insertions(+), 5 deletions(-)
---
base-commit: f86128050d2d854035bfa461aadf36e6951b2bac
change-id: 20231016-arm-psci-system_reset2-vendor-reboots-cc3ad456c070

Best regards,
-- 
Elliot Berman <quic_eberman@quicinc.com>


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

* [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array
  2023-11-17 21:18 [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
@ 2023-11-17 21:18 ` Elliot Berman
  2023-11-20 10:52   ` Krzysztof Kozlowski
  2023-11-20 15:27   ` Rob Herring
  2023-11-17 21:18 ` [PATCH 2/3] dt-bindings: arm: Document reboot mode magic Elliot Berman
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Elliot Berman @ 2023-11-17 21:18 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
	Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli, Elliot Berman

PSCI reboot mode will map a mode name to multiple magic values instead
of just one. Convert the mode-.* property to an array with default
number of items limited to 1.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 Documentation/devicetree/bindings/power/reset/reboot-mode.yaml | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
index ad0a0b95cec1..2c786e783464 100644
--- a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
+++ b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
@@ -28,13 +28,16 @@ description: |
 
 properties:
   mode-normal:
-    $ref: /schemas/types.yaml#/definitions/uint32
+    $ref: "#/patternProperties/^mode-.*$"
     description:
       Default value to set on a reboot if no command was provided.
 
 patternProperties:
   "^mode-.*$":
-    $ref: /schemas/types.yaml#/definitions/uint32
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    # Default to one value. Bindings that reference this schema could override.
+    minItems: 1
+    maxItems: 1
 
 additionalProperties: true
 

-- 
2.41.0


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

* [PATCH 2/3] dt-bindings: arm: Document reboot mode magic
  2023-11-17 21:18 [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
  2023-11-17 21:18 ` [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
@ 2023-11-17 21:18 ` Elliot Berman
  2023-11-20 10:56   ` Krzysztof Kozlowski
  2023-11-17 21:18 ` [PATCH 3/3] firmware: psci: Read and use vendor reset types Elliot Berman
  2023-11-20 10:55 ` [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Krzysztof Kozlowski
  3 siblings, 1 reply; 14+ messages in thread
From: Elliot Berman @ 2023-11-17 21:18 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
	Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli, Elliot Berman

Add bindings to describe vendor-specific reboot modes. Values here
correspond to valid parameters to vendor-specific reset types in PSCI
SYSTEM_RESET2 call.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 Documentation/devicetree/bindings/arm/psci.yaml | 36 +++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
index 0c5381e081bd..ac95c1610287 100644
--- a/Documentation/devicetree/bindings/arm/psci.yaml
+++ b/Documentation/devicetree/bindings/arm/psci.yaml
@@ -136,8 +136,29 @@ allOf:
       required:
         - cpu_off
         - cpu_on
-
-additionalProperties: false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: arm,psci-1.0
+    then:
+      $ref: /schemas/power/reset/reboot-mode.yaml#
+      properties:
+        # "mode-normal" is just SYSTEM_RESET
+        mode-normal: false
+      patternProperties:
+        "^mode-.*$":
+          maxItems: 2
+          description: |
+            Describes a vendor-specific reset type. The string after "mode-"
+            maps a reboot mode to the parameters in the PSCI SYSTEM_RESET2 call.
+
+            Parameters are named mode-xxx = <type[, cookie]>, where xxx
+            is the name of the magic reboot mode, type is the lower 31 bits
+            of the reset_type, and, optionally, the cookie value. If the cookie
+            is not provided, it is defaulted to zero.
+
+unevaluatedProperties: false
 
 examples:
   - |+
@@ -260,4 +281,15 @@ examples:
         domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWRDN>;
       };
     };
+
+  - |+
+
+    // Case 5: SYSTEM_RESET2 vendor resets
+    psci {
+      compatible = "arm,psci-1.0";
+      method = "smc";
+
+      mode-edl = <0>;
+      mode-bootloader = <1 2>;
+    };
 ...

-- 
2.41.0


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

* [PATCH 3/3] firmware: psci: Read and use vendor reset types
  2023-11-17 21:18 [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
  2023-11-17 21:18 ` [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
  2023-11-17 21:18 ` [PATCH 2/3] dt-bindings: arm: Document reboot mode magic Elliot Berman
@ 2023-11-17 21:18 ` Elliot Berman
  2023-11-20 10:55 ` [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Krzysztof Kozlowski
  3 siblings, 0 replies; 14+ messages in thread
From: Elliot Berman @ 2023-11-17 21:18 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
	Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli, Elliot Berman

SoC vendors have different types of resets and are controlled through
various registers. For instance, Qualcomm chipsets can reboot to a
"download mode" that allows a RAM dump to be collected. Another example
is they also support writing a cookie that can be read by bootloader
during next boot. PSCI offers a mechanism, SYSTEM_RESET2, for these
vendor reset types to be implemented without requiring drivers for every
register/cookie.

Add support in PSCI to statically map reboot mode commands from
userspace to a vendor reset and cookie value using the device tree.

Reboot mode framework is close but doesn't quite fit with the
design and requirements for PSCI SYSTEM_RESET2. Some of these issues can
be solved but doesn't seem reasonable in sum:
 1. reboot mode registers against the reboot_notifier_list, which is too
    early to call SYSTEM_RESET2. PSCI would need to remember the reset
    type from the reboot-mode framework callback and use it
    psci_sys_reset.
 2. reboot mode assumes only one cookie/parameter is described in the
    device tree. SYSTEM_RESET2 uses 2: one for the type and one for
    cookie.
 3. psci cpuidle driver already registers a driver against the
    arm,psci-1.0 compatible. Refactoring would be needed to have both a
    cpuidle and reboot-mode driver.

Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
 drivers/firmware/psci/psci.c | 87 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 86 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index d9629ff87861..3f6a862c3999 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -29,6 +29,8 @@
 #include <asm/smp_plat.h>
 #include <asm/suspend.h>
 
+#define REBOOT_PREFIX "mode-"
+
 /*
  * While a 64-bit OS can make calls with SMC32 calling conventions, for some
  * calls it is necessary to use SMC64 to pass or return 64-bit values.
@@ -79,6 +81,14 @@ struct psci_0_1_function_ids get_psci_0_1_function_ids(void)
 static u32 psci_cpu_suspend_feature;
 static bool psci_system_reset2_supported;
 
+struct psci_reset_param {
+	const char *mode;
+	u32 reset_type;
+	u32 cookie;
+};
+static struct psci_reset_param *psci_reset_params;
+static size_t num_psci_reset_params;
+
 static inline bool psci_has_ext_power_state(void)
 {
 	return psci_cpu_suspend_feature &
@@ -305,11 +315,29 @@ static int get_set_conduit_method(const struct device_node *np)
 	return 0;
 }
 
+static void psci_vendor_sys_reset2(unsigned long action, void *data)
+{
+	const char *cmd = data;
+	size_t i;
+
+	for (i = 0; i < num_psci_reset_params; i++) {
+		if (!strcmp(psci_reset_params[i].mode, cmd)) {
+			invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
+				       psci_reset_params[i].reset_type,
+				       psci_reset_params[i].cookie, 0);
+			break;
+		}
+	}
+}
+
 static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
 			  void *data)
 {
+	if (psci_system_reset2_supported && num_psci_reset_params)
+		psci_vendor_sys_reset2(action, data);
+
 	if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
-	    psci_system_reset2_supported) {
+		 psci_system_reset2_supported) {
 		/*
 		 * reset_type[31] = 0 (architectural)
 		 * reset_type[30:0] = 0 (SYSTEM_WARM_RESET)
@@ -748,6 +776,63 @@ static const struct of_device_id psci_of_match[] __initconst = {
 	{},
 };
 
+static int __init psci_init_system_reset2_modes(void)
+{
+	const size_t len = strlen(REBOOT_PREFIX);
+	struct psci_reset_param *param;
+	struct device_node *np;
+	struct property *prop;
+	size_t count = 0;
+	u32 magic[2];
+	int ret;
+
+	if (!psci_system_reset2_supported)
+		return 0;
+
+	np = of_find_matching_node(NULL, psci_of_match);
+	if (!np)
+		return 0;
+
+	for_each_property_of_node(np, prop) {
+		if (strncmp(prop->name, REBOOT_PREFIX, len))
+			continue;
+		ret = of_property_count_elems_of_size(np, prop->name, sizeof(magic[0]));
+		if (ret != 1 && ret != 2)
+			continue;
+
+		count++;
+	}
+
+	param = psci_reset_params = kcalloc(count, sizeof(*psci_reset_params), GFP_KERNEL);
+	if (!psci_reset_params)
+		return -ENOMEM;
+
+	for_each_property_of_node(np, prop) {
+		if (strncmp(prop->name, REBOOT_PREFIX, len))
+			continue;
+
+		param->mode = kstrdup_const(prop->name + len, GFP_KERNEL);
+		if (!param->mode)
+			continue;
+
+		ret = of_property_read_variable_u32_array(np, prop->name, magic, 1, 2);
+		if (ret < 0) {
+			pr_warn("Failed to parse vendor reboot mode %s\n", param->mode);
+			kfree(param->mode);
+			continue;
+		}
+
+		/* Force reset type to be in vendor space */
+		param->reset_type = PSCI_1_1_RESET_TYPE_VENDOR_START | magic[0];
+		param->cookie = ret == 2 ? magic[1] : 0;
+		param++;
+		num_psci_reset_params++;
+	}
+
+	return 0;
+}
+arch_initcall(psci_init_system_reset2_modes);
+
 int __init psci_dt_init(void)
 {
 	struct device_node *np;

-- 
2.41.0


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

* Re: [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array
  2023-11-17 21:18 ` [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
@ 2023-11-20 10:52   ` Krzysztof Kozlowski
  2023-11-20 15:58     ` Elliot Berman
  2023-11-20 15:27   ` Rob Herring
  1 sibling, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-20 10:52 UTC (permalink / raw)
  To: Elliot Berman, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli

On 17/11/2023 22:18, Elliot Berman wrote:
> PSCI reboot mode will map a mode name to multiple magic values instead
> of just one. Convert the mode-.* property to an array with default
> number of items limited to 1.
> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  Documentation/devicetree/bindings/power/reset/reboot-mode.yaml | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
> index ad0a0b95cec1..2c786e783464 100644
> --- a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
> @@ -28,13 +28,16 @@ description: |
>  
>  properties:
>    mode-normal:
> -    $ref: /schemas/types.yaml#/definitions/uint32
> +    $ref: "#/patternProperties/^mode-.*$"
>      description:
>        Default value to set on a reboot if no command was provided.
>  
>  patternProperties:
>    "^mode-.*$":
> -    $ref: /schemas/types.yaml#/definitions/uint32
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    # Default to one value. Bindings that reference this schema could override.
> +    minItems: 1
> +    maxItems: 1

I don't understand. Array with one value is the same as uint32.

Best regards,
Krzysztof


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

* Re: [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2
  2023-11-17 21:18 [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
                   ` (2 preceding siblings ...)
  2023-11-17 21:18 ` [PATCH 3/3] firmware: psci: Read and use vendor reset types Elliot Berman
@ 2023-11-20 10:55 ` Krzysztof Kozlowski
  2023-11-20 16:03   ` Elliot Berman
  3 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-20 10:55 UTC (permalink / raw)
  To: Elliot Berman, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli

On 17/11/2023 22:18, Elliot Berman wrote:
> The PSCI SYSTEM_RESET2 call allows vendor firmware to define additional
> reset types which could be mapped to the reboot argument.
> 
> Setting up reboot on Qualcomm devices can be inconsistent from chipset
> to chipset.  Generally, there is a PMIC register that gets written to
> decide the reboot type. There is also sometimes a cookie that can be
> written to indicate that the bootloader should behave differently than a
> regular boot. These knobs evolve over product generations and require 
> more drivers. Qualcomm firmwares are beginning to expose vendor
> SYSTEM_RESET2 types to simplify driver requirements from Linux.
> 
> Add support in PSCI to statically wire reboot mode commands from
> userspace to a vendor reset and cookie value using the device tree. The
> DT bindings are similar to reboot mode framework except that 2
> integers are accepted (the type and cookie). Also, reboot mode framework
> is intended to program, but not reboot, the host. PSCI SYSTEM_RESET2
> does both. I've not added support for reading ACPI tables since I don't
> have any device which provides them + firmware that supports vendor
> SYSTEM_RESET2 types.
> 
> Previous discussions around SYSTEM_RESET2:
> - https://lore.kernel.org/lkml/20230724223057.1208122-2-quic_eberman@quicinc.com/T/
> - https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/

Please link here upstream DTS user for this.

Best regards,
Krzysztof


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

* Re: [PATCH 2/3] dt-bindings: arm: Document reboot mode magic
  2023-11-17 21:18 ` [PATCH 2/3] dt-bindings: arm: Document reboot mode magic Elliot Berman
@ 2023-11-20 10:56   ` Krzysztof Kozlowski
  2023-11-20 15:45     ` Elliot Berman
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-20 10:56 UTC (permalink / raw)
  To: Elliot Berman, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli

On 17/11/2023 22:18, Elliot Berman wrote:
> -
> -additionalProperties: false
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: arm,psci-1.0
> +    then:
> +      $ref: /schemas/power/reset/reboot-mode.yaml#
> +      properties:
> +        # "mode-normal" is just SYSTEM_RESET
> +        mode-normal: false
> +      patternProperties:
> +        "^mode-.*$":
> +          maxItems: 2

And if you tested the patch, it would tell you it can be max 1 item.

Best regards,
Krzysztof


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

* Re: [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array
  2023-11-17 21:18 ` [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
  2023-11-20 10:52   ` Krzysztof Kozlowski
@ 2023-11-20 15:27   ` Rob Herring
  1 sibling, 0 replies; 14+ messages in thread
From: Rob Herring @ 2023-11-20 15:27 UTC (permalink / raw)
  To: Elliot Berman
  Cc: Krzysztof Kozlowski, Conor Dooley, Lorenzo Pieralisi,
	Mark Rutland, Satya Durga Srinivasu Prabhala, Melody Olvera,
	devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli

On Fri, Nov 17, 2023 at 01:18:46PM -0800, Elliot Berman wrote:
> PSCI reboot mode will map a mode name to multiple magic values instead
> of just one. Convert the mode-.* property to an array with default
> number of items limited to 1.
> 
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
>  Documentation/devicetree/bindings/power/reset/reboot-mode.yaml | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
> index ad0a0b95cec1..2c786e783464 100644
> --- a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
> +++ b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
> @@ -28,13 +28,16 @@ description: |
>  
>  properties:
>    mode-normal:
> -    $ref: /schemas/types.yaml#/definitions/uint32
> +    $ref: "#/patternProperties/^mode-.*$"
>      description:
>        Default value to set on a reboot if no command was provided.
>  
>  patternProperties:
>    "^mode-.*$":
> -    $ref: /schemas/types.yaml#/definitions/uint32
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    # Default to one value. Bindings that reference this schema could override.
> +    minItems: 1
> +    maxItems: 1

There are no overrides in json-schema, so this won't work. 

It happens to work though. It has to do with how we process the schemas 
because every integer property is decoded into a 2 dimensional array. So 
we process the schemas to convert schemas for scalars and arrays into 
a matrix. This hit a corner case where we bail on doing any fixup when 
maxItems is 1, but really it should be transformed into:

maxItems: 1
items:
  maxItems: 1

Which would then fail on your case with 2 entries. You need 'maxItems: 
1' everywhere just 1 entry is expected (e.g. mode-normal) and no 
constraints here. 

Rob


>  
>  additionalProperties: true
>  
> 
> -- 
> 2.41.0
> 

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

* Re: [PATCH 2/3] dt-bindings: arm: Document reboot mode magic
  2023-11-20 10:56   ` Krzysztof Kozlowski
@ 2023-11-20 15:45     ` Elliot Berman
  2023-11-20 16:41       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Elliot Berman @ 2023-11-20 15:45 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli



On 11/20/2023 2:56 AM, Krzysztof Kozlowski wrote:
> On 17/11/2023 22:18, Elliot Berman wrote:
>> -
>> -additionalProperties: false
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: arm,psci-1.0
>> +    then:
>> +      $ref: /schemas/power/reset/reboot-mode.yaml#
>> +      properties:
>> +        # "mode-normal" is just SYSTEM_RESET
>> +        mode-normal: false
>> +      patternProperties:
>> +        "^mode-.*$":
>> +          maxItems: 2
> 
> And if you tested the patch, it would tell you it can be max 1 item.

make dt_binding_check DT_SCHEMA_FILES=arm/psci.yaml

passes for me. Rob explained why it's working (and why it shouldn't), 
so I'll fix it according to his recommendation in v2.

Thanks,
Elliot

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

* Re: [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array
  2023-11-20 10:52   ` Krzysztof Kozlowski
@ 2023-11-20 15:58     ` Elliot Berman
  0 siblings, 0 replies; 14+ messages in thread
From: Elliot Berman @ 2023-11-20 15:58 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli

Hi Krzysztof,

On 11/20/2023 2:52 AM, Krzysztof Kozlowski wrote:
> On 17/11/2023 22:18, Elliot Berman wrote:
>> PSCI reboot mode will map a mode name to multiple magic values instead
>> of just one. Convert the mode-.* property to an array with default
>> number of items limited to 1.
>>
>> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
>> ---
>>  Documentation/devicetree/bindings/power/reset/reboot-mode.yaml | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
>> index ad0a0b95cec1..2c786e783464 100644
>> --- a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
>> +++ b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
>> @@ -28,13 +28,16 @@ description: |
>>  
>>  properties:
>>    mode-normal:
>> -    $ref: /schemas/types.yaml#/definitions/uint32
>> +    $ref: "#/patternProperties/^mode-.*$"
>>      description:
>>        Default value to set on a reboot if no command was provided.
>>  
>>  patternProperties:
>>    "^mode-.*$":
>> -    $ref: /schemas/types.yaml#/definitions/uint32
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    # Default to one value. Bindings that reference this schema could override.
>> +    minItems: 1
>> +    maxItems: 1
> 
> I don't understand. Array with one value is the same as uint32.
> 

PSCI SYSTEM_RESET2 can have multiple values per reboot type. In other words:
a given reboot mode could refer to a tuple with N values, where N
is device-specific. The current schema only allows for N=1 and PSCI
SYSTEM_RESET2 uses N=2. This patch was to update the reboot-mode.yaml
to allow me to specify N=2 in the psci bindings.

Thanks,
Elliot

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

* Re: [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2
  2023-11-20 10:55 ` [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Krzysztof Kozlowski
@ 2023-11-20 16:03   ` Elliot Berman
  2023-11-20 17:27     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 14+ messages in thread
From: Elliot Berman @ 2023-11-20 16:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli



On 11/20/2023 2:55 AM, Krzysztof Kozlowski wrote:
> On 17/11/2023 22:18, Elliot Berman wrote:
>> The PSCI SYSTEM_RESET2 call allows vendor firmware to define additional
>> reset types which could be mapped to the reboot argument.
>>
>> Setting up reboot on Qualcomm devices can be inconsistent from chipset
>> to chipset.  Generally, there is a PMIC register that gets written to
>> decide the reboot type. There is also sometimes a cookie that can be
>> written to indicate that the bootloader should behave differently than a
>> regular boot. These knobs evolve over product generations and require 
>> more drivers. Qualcomm firmwares are beginning to expose vendor
>> SYSTEM_RESET2 types to simplify driver requirements from Linux.
>>
>> Add support in PSCI to statically wire reboot mode commands from
>> userspace to a vendor reset and cookie value using the device tree. The
>> DT bindings are similar to reboot mode framework except that 2
>> integers are accepted (the type and cookie). Also, reboot mode framework
>> is intended to program, but not reboot, the host. PSCI SYSTEM_RESET2
>> does both. I've not added support for reading ACPI tables since I don't
>> have any device which provides them + firmware that supports vendor
>> SYSTEM_RESET2 types.
>>
>> Previous discussions around SYSTEM_RESET2:
>> - https://lore.kernel.org/lkml/20230724223057.1208122-2-quic_eberman@quicinc.com/T/
>> - https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/
> 
> Please link here upstream DTS user for this.

The changes are applicable for SM8650, but hasn't yet landed in arm64/for-next/core.

I'll include it in the v2 with note.

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

* Re: [PATCH 2/3] dt-bindings: arm: Document reboot mode magic
  2023-11-20 15:45     ` Elliot Berman
@ 2023-11-20 16:41       ` Krzysztof Kozlowski
  2023-11-20 17:09         ` Elliot Berman
  0 siblings, 1 reply; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-20 16:41 UTC (permalink / raw)
  To: Elliot Berman, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli

On 20/11/2023 16:45, Elliot Berman wrote:
> 
> 
> On 11/20/2023 2:56 AM, Krzysztof Kozlowski wrote:
>> On 17/11/2023 22:18, Elliot Berman wrote:
>>> -
>>> -additionalProperties: false
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: arm,psci-1.0
>>> +    then:
>>> +      $ref: /schemas/power/reset/reboot-mode.yaml#
>>> +      properties:
>>> +        # "mode-normal" is just SYSTEM_RESET
>>> +        mode-normal: false
>>> +      patternProperties:
>>> +        "^mode-.*$":
>>> +          maxItems: 2
>>
>> And if you tested the patch, it would tell you it can be max 1 item.
> 
> make dt_binding_check DT_SCHEMA_FILES=arm/psci.yaml

psci.example.dtb: psci: mode-edl: [[0]] is too short
psci.example.dtb: psci: mode-bootloader: [[1, 2]] is too short

psci.example.dtb: psci: Unevaluated properties are not allowed
('mode-bootloader', 'mode-edl' were unexpected)

> 
> passes for me. Rob explained why it's working (and why it shouldn't), 
> so I'll fix it according to his recommendation in v2.

Then you wanted uint32-matrix.


Best regards,
Krzysztof


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

* Re: [PATCH 2/3] dt-bindings: arm: Document reboot mode magic
  2023-11-20 16:41       ` Krzysztof Kozlowski
@ 2023-11-20 17:09         ` Elliot Berman
  0 siblings, 0 replies; 14+ messages in thread
From: Elliot Berman @ 2023-11-20 17:09 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli



On 11/20/2023 8:41 AM, Krzysztof Kozlowski wrote:
> On 20/11/2023 16:45, Elliot Berman wrote:
>>
>>
>> On 11/20/2023 2:56 AM, Krzysztof Kozlowski wrote:
>>> On 17/11/2023 22:18, Elliot Berman wrote:
>>>> -
>>>> -additionalProperties: false
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            const: arm,psci-1.0
>>>> +    then:
>>>> +      $ref: /schemas/power/reset/reboot-mode.yaml#
>>>> +      properties:
>>>> +        # "mode-normal" is just SYSTEM_RESET
>>>> +        mode-normal: false
>>>> +      patternProperties:
>>>> +        "^mode-.*$":
>>>> +          maxItems: 2
>>>
>>> And if you tested the patch, it would tell you it can be max 1 item.
>>
>> make dt_binding_check DT_SCHEMA_FILES=arm/psci.yaml
> 
> psci.example.dtb: psci: mode-edl: [[0]] is too short
> psci.example.dtb: psci: mode-bootloader: [[1, 2]] is too short
> 
> psci.example.dtb: psci: Unevaluated properties are not allowed
> ('mode-bootloader', 'mode-edl' were unexpected)
> 

Ah, tip of tree for dt-schema doesn't seem to report the error.

Doesn't report the error:
dt-validate --version
2023.10.dev17+g58feadb

Reports the error:
dt-validate --version
2023.9

Looks likely related to generated the processed-schema.json rather
than dt-validate itself. The tip of tree dt-validate does report
the error if processed-schema.json is generated by 2023.9 tool,
but not if the schema was also generated by tip-of-tree mkschema.

I'll try bisecting the error and report back.

>>
>> passes for me. Rob explained why it's working (and why it shouldn't), 
>> so I'll fix it according to his recommendation in v2.
> 
> Then you wanted uint32-matrix.
> 
> 
> Best regards,
> Krzysztof
> 

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

* Re: [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2
  2023-11-20 16:03   ` Elliot Berman
@ 2023-11-20 17:27     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-11-20 17:27 UTC (permalink / raw)
  To: Elliot Berman, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lorenzo Pieralisi, Mark Rutland
  Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, devicetree,
	linux-kernel, linux-arm-kernel, Florian Fainelli

On 20/11/2023 17:03, Elliot Berman wrote:
> 
> 
> On 11/20/2023 2:55 AM, Krzysztof Kozlowski wrote:
>> On 17/11/2023 22:18, Elliot Berman wrote:
>>> The PSCI SYSTEM_RESET2 call allows vendor firmware to define additional
>>> reset types which could be mapped to the reboot argument.
>>>
>>> Setting up reboot on Qualcomm devices can be inconsistent from chipset
>>> to chipset.  Generally, there is a PMIC register that gets written to
>>> decide the reboot type. There is also sometimes a cookie that can be
>>> written to indicate that the bootloader should behave differently than a
>>> regular boot. These knobs evolve over product generations and require 
>>> more drivers. Qualcomm firmwares are beginning to expose vendor
>>> SYSTEM_RESET2 types to simplify driver requirements from Linux.
>>>
>>> Add support in PSCI to statically wire reboot mode commands from
>>> userspace to a vendor reset and cookie value using the device tree. The
>>> DT bindings are similar to reboot mode framework except that 2
>>> integers are accepted (the type and cookie). Also, reboot mode framework
>>> is intended to program, but not reboot, the host. PSCI SYSTEM_RESET2
>>> does both. I've not added support for reading ACPI tables since I don't
>>> have any device which provides them + firmware that supports vendor
>>> SYSTEM_RESET2 types.
>>>
>>> Previous discussions around SYSTEM_RESET2:
>>> - https://lore.kernel.org/lkml/20230724223057.1208122-2-quic_eberman@quicinc.com/T/
>>> - https://lore.kernel.org/all/4a679542-b48d-7e11-f33a-63535a5c68cb@quicinc.com/
>>
>> Please link here upstream DTS user for this.
> 
> The changes are applicable for SM8650, but hasn't yet landed in arm64/for-next/core.
> 
> I'll include it in the v2 with note.

It's enough if you link to lore or any other upstream-oriented tree with
that user. Does not have to be merged.

Best regards,
Krzysztof


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

end of thread, other threads:[~2023-11-20 17:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-17 21:18 [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
2023-11-17 21:18 ` [PATCH 1/3] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
2023-11-20 10:52   ` Krzysztof Kozlowski
2023-11-20 15:58     ` Elliot Berman
2023-11-20 15:27   ` Rob Herring
2023-11-17 21:18 ` [PATCH 2/3] dt-bindings: arm: Document reboot mode magic Elliot Berman
2023-11-20 10:56   ` Krzysztof Kozlowski
2023-11-20 15:45     ` Elliot Berman
2023-11-20 16:41       ` Krzysztof Kozlowski
2023-11-20 17:09         ` Elliot Berman
2023-11-17 21:18 ` [PATCH 3/3] firmware: psci: Read and use vendor reset types Elliot Berman
2023-11-20 10:55 ` [PATCH 0/3] Implement vendor resets for PSCI SYSTEM_RESET2 Krzysztof Kozlowski
2023-11-20 16:03   ` Elliot Berman
2023-11-20 17:27     ` Krzysztof Kozlowski

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).