| Message ID | 20240414-arm-psci-system_reset2-vendor-reboots-v2-2-da9a055a648f@quicinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Implement vendor resets for PSCI SYSTEM_RESET2 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 4/14/24 21:30, Elliot Berman wrote: > 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> > --- [...] > + > + // Case 5: SYSTEM_RESET2 vendor resets > + psci { > + compatible = "arm,psci-1.0"; > + method = "smc"; > + > + mode-edl = <0>; > + mode-bootloader = <1 2>; These properties seem overly generic, and with PSCI only growing every now and then, I think a reboot-mode (or similar) subnode would be in order here Konrad
On Sun, Apr 14, 2024 at 12:30:25PM -0700, Elliot Berman wrote: > Add bindings to describe vendor-specific reboot modes. Values here > correspond to valid parameters to vendor-specific reset types in PSCI > SYSTEM_RESET2 call. > IIUC, PSCI SYSTEM_RESET will be always supported, so the choice of using SYSTEM_RESET2 sounds like a system policy and must not have any information in the device tree. All required support from PSCI is discoverable and the policy choice must be userspace driven. That's my opinion.
On Tue, 16 Apr 2024 at 12:30, Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Sun, Apr 14, 2024 at 12:30:25PM -0700, Elliot Berman wrote: > > Add bindings to describe vendor-specific reboot modes. Values here > > correspond to valid parameters to vendor-specific reset types in PSCI > > SYSTEM_RESET2 call. > > > > IIUC, PSCI SYSTEM_RESET will be always supported, so the choice of using > SYSTEM_RESET2 sounds like a system policy and must not have any information > in the device tree. All required support from PSCI is discoverable and > the policy choice must be userspace driven. That's my opinion. Well, we are talking about the vendor-specific resets, which are not discoverable. The spec defines them as "implementation defined". For example, PCSI spec doesn't define a way to add "reset to bootloader" or "reset to recovery" kinds of resets. I think the bindings at least should make it clear that the vendor bit it being set automatically. I won't comment whether this is a good decision or not.
diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml index cbb012e217ab..ac778274b3ac 100644 --- a/Documentation/devicetree/bindings/arm/psci.yaml +++ b/Documentation/devicetree/bindings/arm/psci.yaml @@ -137,8 +137,31 @@ allOf: required: - cpu_off - cpu_on - -additionalProperties: false + - if: + properties: + compatible: + contains: + const: arm,psci-1.0 + then: + allOf: + - $ref: /schemas/power/reset/reboot-mode.yaml# + - properties: + # "mode-normal" is just SYSTEM_RESET + mode-normal: false + - patternProperties: + "^mode-.*$": + items: + 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: - |+ @@ -261,4 +284,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>; + }; ...
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 | 38 +++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-)