* [PATCH v6 1/5] dt-bindings: power: reset: Convert mode-.* properties to array
2024-10-18 19:39 [PATCH v6 0/5] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
@ 2024-10-18 19:39 ` Elliot Berman
2024-10-18 19:39 ` [PATCH v6 2/5] dt-bindings: arm: Document reboot mode magic Elliot Berman
` (3 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Elliot Berman @ 2024-10-18 19:39 UTC (permalink / raw)
To: Bjorn Andersson, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Andy Yan,
Lorenzo Pieralisi, Mark Rutland, Bartosz Golaszewski,
Arnd Bergmann, Olof Johansson, Catalin Marinas, Will Deacon,
cros-qcom-dts-watchers, Krzysztof Kozlowski, Konrad Dybcio,
Konrad Dybcio
Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm, 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. Users of the
reboot-mode schema will need to specify the maxItems of the mode-.*
properties. Existing users will all be 1.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
.../devicetree/bindings/power/reset/nvmem-reboot-mode.yaml | 4 ++++
Documentation/devicetree/bindings/power/reset/qcom,pon.yaml | 7 +++++++
Documentation/devicetree/bindings/power/reset/reboot-mode.yaml | 4 ++--
.../devicetree/bindings/power/reset/syscon-reboot-mode.yaml | 4 ++++
4 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml
index 627f8a6078c2..7f5f94673e9c 100644
--- a/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml
+++ b/Documentation/devicetree/bindings/power/reset/nvmem-reboot-mode.yaml
@@ -31,6 +31,10 @@ properties:
allOf:
- $ref: reboot-mode.yaml#
+patternProperties:
+ "^mode-.*$":
+ maxItems: 1
+
required:
- compatible
- nvmem-cells
diff --git a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
index fc8105a7b9b2..3da3d02a6690 100644
--- a/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
+++ b/Documentation/devicetree/bindings/power/reset/qcom,pon.yaml
@@ -54,6 +54,10 @@ required:
- compatible
- reg
+patternProperties:
+ "^mode-.*$":
+ maxItems: 1
+
unevaluatedProperties: false
allOf:
@@ -75,6 +79,9 @@ allOf:
reg-names:
items:
- const: pon
+ else:
+ patternProperties:
+ "^mode-.*$": false
# Special case for pm8941, which doesn't store reset mode
- if:
diff --git a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
index ad0a0b95cec1..3ddac06cec72 100644
--- a/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
+++ b/Documentation/devicetree/bindings/power/reset/reboot-mode.yaml
@@ -28,13 +28,13 @@ description: |
properties:
mode-normal:
- $ref: /schemas/types.yaml#/definitions/uint32
+ $ref: /schemas/types.yaml#/definitions/uint32-array
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
additionalProperties: true
diff --git a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
index b6acff199cde..79ffc78b23ea 100644
--- a/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
+++ b/Documentation/devicetree/bindings/power/reset/syscon-reboot-mode.yaml
@@ -32,6 +32,10 @@ properties:
allOf:
- $ref: reboot-mode.yaml#
+patternProperties:
+ "^mode-.*$":
+ maxItems: 1
+
unevaluatedProperties: false
required:
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v6 2/5] dt-bindings: arm: Document reboot mode magic
2024-10-18 19:39 [PATCH v6 0/5] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
2024-10-18 19:39 ` [PATCH v6 1/5] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
@ 2024-10-18 19:39 ` Elliot Berman
2024-10-18 19:39 ` [PATCH v6 3/5] firmware: psci: Read and use vendor reset types Elliot Berman
` (2 subsequent siblings)
4 siblings, 0 replies; 17+ messages in thread
From: Elliot Berman @ 2024-10-18 19:39 UTC (permalink / raw)
To: Bjorn Andersson, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Andy Yan,
Lorenzo Pieralisi, Mark Rutland, Bartosz Golaszewski,
Arnd Bergmann, Olof Johansson, Catalin Marinas, Will Deacon,
cros-qcom-dts-watchers, Krzysztof Kozlowski, Konrad Dybcio,
Konrad Dybcio
Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm, 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.
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
Documentation/devicetree/bindings/arm/psci.yaml | 43 +++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/psci.yaml b/Documentation/devicetree/bindings/arm/psci.yaml
index cbb012e217ab..5e07c62fe5d7 100644
--- a/Documentation/devicetree/bindings/arm/psci.yaml
+++ b/Documentation/devicetree/bindings/arm/psci.yaml
@@ -98,6 +98,27 @@ properties:
[1] Kernel documentation - ARM idle states bindings
Documentation/devicetree/bindings/cpu/idle-states.yaml
+ reset-types:
+ type: object
+ $ref: /schemas/power/reset/reboot-mode.yaml#
+ unevaluatedProperties: false
+ properties:
+ # "mode-normal" is just SYSTEM_RESET
+ mode-normal: false
+ patternProperties:
+ "^mode-.*$":
+ minItems: 1
+ 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.
+ The 31st bit (vendor-resets) will be implicitly set by the driver.
+
patternProperties:
"^power-domain-":
$ref: /schemas/power/power-domain.yaml#
@@ -137,6 +158,15 @@ allOf:
required:
- cpu_off
- cpu_on
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ const: arm,psci-1.0
+ then:
+ properties:
+ reset-types: false
additionalProperties: false
@@ -261,4 +291,17 @@ examples:
domain-idle-states = <&CLUSTER_RET>, <&CLUSTER_PWRDN>;
};
};
+
+ - |+
+
+ // Case 5: SYSTEM_RESET2 vendor resets
+ psci {
+ compatible = "arm,psci-1.0";
+ method = "smc";
+
+ reset-types {
+ mode-edl = <0>;
+ mode-bootloader = <1 2>;
+ };
+ };
...
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-10-18 19:39 [PATCH v6 0/5] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
2024-10-18 19:39 ` [PATCH v6 1/5] dt-bindings: power: reset: Convert mode-.* properties to array Elliot Berman
2024-10-18 19:39 ` [PATCH v6 2/5] dt-bindings: arm: Document reboot mode magic Elliot Berman
@ 2024-10-18 19:39 ` Elliot Berman
2024-10-19 5:42 ` Stephen Boyd
2024-10-18 19:39 ` [PATCH v6 4/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp Elliot Berman
2024-10-18 19:39 ` [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride Elliot Berman
4 siblings, 1 reply; 17+ messages in thread
From: Elliot Berman @ 2024-10-18 19:39 UTC (permalink / raw)
To: Bjorn Andersson, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Andy Yan,
Lorenzo Pieralisi, Mark Rutland, Bartosz Golaszewski,
Arnd Bergmann, Olof Johansson, Catalin Marinas, Will Deacon,
cros-qcom-dts-watchers, Krzysztof Kozlowski, Konrad Dybcio,
Konrad Dybcio
Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm, 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.
A separate initcall is needed to parse the devicetree, instead of using
psci_dt_init because mm isn't sufficiently set up to allocate memory.
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.
Tested-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
drivers/firmware/psci/psci.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 2328ca58bba6..60bc285622ce 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,9 +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;
+ unsigned long ret;
+ size_t i;
+
+ for (i = 0; i < num_psci_reset_params; i++) {
+ if (!strcmp(psci_reset_params[i].mode, cmd)) {
+ ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
+ psci_reset_params[i].reset_type,
+ psci_reset_params[i].cookie, 0);
+ pr_err("failed to perform reset \"%s\": %ld\n",
+ cmd, (long)ret);
+ }
+ }
+}
+
static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
void *data)
{
+ if (data && num_psci_reset_params)
+ psci_vendor_sys_reset2(action, data);
+
if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
psci_system_reset2_supported) {
/*
@@ -750,6 +780,68 @@ 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 *psci_np __free(device_node) = NULL;
+ struct device_node *np __free(device_node) = NULL;
+ struct property *prop;
+ size_t count = 0;
+ u32 magic[2];
+ int num;
+
+ if (!psci_system_reset2_supported)
+ return 0;
+
+ psci_np = of_find_matching_node(NULL, psci_of_match);
+ if (!psci_np)
+ return 0;
+
+ np = of_find_node_by_name(psci_np, "reset-types");
+ if (!np)
+ return 0;
+
+ for_each_property_of_node(np, prop) {
+ if (strncmp(prop->name, REBOOT_PREFIX, len))
+ continue;
+ num = of_property_count_elems_of_size(np, prop->name, sizeof(magic[0]));
+ if (num != 1 && num != 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;
+
+ num = of_property_read_variable_u32_array(np, prop->name, magic, 1, 2);
+ if (num < 0) {
+ pr_warn("Failed to parse vendor reboot mode %s\n", param->mode);
+ kfree_const(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 = num == 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.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-10-18 19:39 ` [PATCH v6 3/5] firmware: psci: Read and use vendor reset types Elliot Berman
@ 2024-10-19 5:42 ` Stephen Boyd
2024-10-23 16:30 ` Elliot Berman
0 siblings, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2024-10-19 5:42 UTC (permalink / raw)
To: Andy Yan, Arnd Bergmann, Bartosz Golaszewski, Bjorn Andersson,
Catalin Marinas, Conor Dooley, Elliot Berman, Konrad Dybcio,
Krzysztof Kozlowski, Krzysztof Kozlowski, Lorenzo Pieralisi,
Mark Rutland, Olof Johansson, Rob Herring, Sebastian Reichel,
Vinod Koul, Will Deacon, cros-qcom-dts-watchers
Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
Quoting Elliot Berman (2024-10-18 12:39:48)
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> index 2328ca58bba6..60bc285622ce 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-"
Maybe move this near the function that uses it.
> +
> /*
> * 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.
> @@ -305,9 +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;
> + unsigned long ret;
> + size_t i;
> +
> + for (i = 0; i < num_psci_reset_params; i++) {
> + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> + psci_reset_params[i].reset_type,
> + psci_reset_params[i].cookie, 0);
> + pr_err("failed to perform reset \"%s\": %ld\n",
> + cmd, (long)ret);
Do this intentionally return? Should it be some other function that's
__noreturn instead and a while (1) if the firmware returns back to the
kernel?
> + }
> + }
> +}
> +
> static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> void *data)
> {
> + if (data && num_psci_reset_params)
> + psci_vendor_sys_reset2(action, data);
> +
> if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> psci_system_reset2_supported) {
> /*
> @@ -750,6 +780,68 @@ 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 *psci_np __free(device_node) = NULL;
> + struct device_node *np __free(device_node) = NULL;
> + struct property *prop;
> + size_t count = 0;
> + u32 magic[2];
> + int num;
> +
> + if (!psci_system_reset2_supported)
> + return 0;
> +
> + psci_np = of_find_matching_node(NULL, psci_of_match);
> + if (!psci_np)
> + return 0;
> +
> + np = of_find_node_by_name(psci_np, "reset-types");
> + if (!np)
> + return 0;
> +
> + for_each_property_of_node(np, prop) {
> + if (strncmp(prop->name, REBOOT_PREFIX, len))
> + continue;
> + num = of_property_count_elems_of_size(np, prop->name, sizeof(magic[0]));
Use of_property_count_u32_elems()?
> + if (num != 1 && num != 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;
> +
> + num = of_property_read_variable_u32_array(np, prop->name, magic, 1, 2);
ARRAY_SIZE(magic)?
> + if (num < 0) {
Should this be less than 1?
> + pr_warn("Failed to parse vendor reboot mode %s\n", param->mode);
> + kfree_const(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 = num == 2 ? magic[1] : 0;
ARRAY_SIZE(magic)?
> + param++;
> + num_psci_reset_params++;
> + }
> +
> + return 0;
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-10-19 5:42 ` Stephen Boyd
@ 2024-10-23 16:30 ` Elliot Berman
2024-10-23 19:02 ` Stephen Boyd
2024-11-15 13:35 ` Lorenzo Pieralisi
0 siblings, 2 replies; 17+ messages in thread
From: Elliot Berman @ 2024-10-23 16:30 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andy Yan, Arnd Bergmann, Bartosz Golaszewski, Bjorn Andersson,
Catalin Marinas, Conor Dooley, Konrad Dybcio, Krzysztof Kozlowski,
Krzysztof Kozlowski, Lorenzo Pieralisi, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
> Quoting Elliot Berman (2024-10-18 12:39:48)
> > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > index 2328ca58bba6..60bc285622ce 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-"
>
> Maybe move this near the function that uses it.
>
> > +
> > /*
> > * 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.
> > @@ -305,9 +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;
> > + unsigned long ret;
> > + size_t i;
> > +
> > + for (i = 0; i < num_psci_reset_params; i++) {
> > + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > + psci_reset_params[i].reset_type,
> > + psci_reset_params[i].cookie, 0);
> > + pr_err("failed to perform reset \"%s\": %ld\n",
> > + cmd, (long)ret);
>
> Do this intentionally return? Should it be some other function that's
> __noreturn instead and a while (1) if the firmware returns back to the
> kernel?
>
Yes, I think it's best to make sure we fall back to the architectural
reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
since device would reboot then.
> > + }
> > + }
> > +}
> > +
> > static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > void *data)
> > {
> > + if (data && num_psci_reset_params)
> > + psci_vendor_sys_reset2(action, data);
> > +
> > if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> > psci_system_reset2_supported) {
> > /*
> > @@ -750,6 +780,68 @@ 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 *psci_np __free(device_node) = NULL;
> > + struct device_node *np __free(device_node) = NULL;
> > + struct property *prop;
> > + size_t count = 0;
> > + u32 magic[2];
> > + int num;
> > +
> > + if (!psci_system_reset2_supported)
> > + return 0;
> > +
> > + psci_np = of_find_matching_node(NULL, psci_of_match);
> > + if (!psci_np)
> > + return 0;
> > +
> > + np = of_find_node_by_name(psci_np, "reset-types");
> > + if (!np)
> > + return 0;
> > +
> > + for_each_property_of_node(np, prop) {
> > + if (strncmp(prop->name, REBOOT_PREFIX, len))
> > + continue;
> > + num = of_property_count_elems_of_size(np, prop->name, sizeof(magic[0]));
>
> Use of_property_count_u32_elems()?
>
> > + if (num != 1 && num != 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;
> > +
> > + num = of_property_read_variable_u32_array(np, prop->name, magic, 1, 2);
>
> ARRAY_SIZE(magic)?
>
> > + if (num < 0) {
>
> Should this be less than 1?
>
of_property_read_variable_u32_array should return -EOVERFLOW (or maybe
-ENODATA) if the array is empty. I don't see it's possible for
of_property_read_variable_u32_array() to return a non-negative value
that's not 1 or 2.
> > + pr_warn("Failed to parse vendor reboot mode %s\n", param->mode);
> > + kfree_const(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 = num == 2 ? magic[1] : 0;
>
> ARRAY_SIZE(magic)?
>
> > + param++;
> > + num_psci_reset_params++;
> > + }
> > +
> > + return 0;
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-10-23 16:30 ` Elliot Berman
@ 2024-10-23 19:02 ` Stephen Boyd
2024-10-23 22:16 ` Elliot Berman
2024-11-15 13:35 ` Lorenzo Pieralisi
1 sibling, 1 reply; 17+ messages in thread
From: Stephen Boyd @ 2024-10-23 19:02 UTC (permalink / raw)
To: Elliot Berman
Cc: Andy Yan, Arnd Bergmann, Bartosz Golaszewski, Bjorn Andersson,
Catalin Marinas, Conor Dooley, Konrad Dybcio, Krzysztof Kozlowski,
Krzysztof Kozlowski, Lorenzo Pieralisi, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
Quoting Elliot Berman (2024-10-23 09:30:21)
> On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
> > Quoting Elliot Berman (2024-10-18 12:39:48)
> > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > index 2328ca58bba6..60bc285622ce 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -305,9 +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;
> > > + unsigned long ret;
> > > + size_t i;
> > > +
> > > + for (i = 0; i < num_psci_reset_params; i++) {
> > > + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > + psci_reset_params[i].reset_type,
> > > + psci_reset_params[i].cookie, 0);
> > > + pr_err("failed to perform reset \"%s\": %ld\n",
> > > + cmd, (long)ret);
> >
> > Do this intentionally return? Should it be some other function that's
> > __noreturn instead and a while (1) if the firmware returns back to the
> > kernel?
> >
>
> Yes, I think it's best to make sure we fall back to the architectural
> reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
> since device would reboot then.
Ok. Please add a comment in the code so we know that it's intentional.
>
> > > + }
> > > + }
> > > +}
> > > +
> > > static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > > void *data)
> > > {
> > > + if (data && num_psci_reset_params)
> > > + psci_vendor_sys_reset2(action, data);
> > > +
I'd add a comment here as well indicating that a fallback is used.
> > > if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> > > psci_system_reset2_supported) {
> > > /*
> > > @@ -750,6 +780,68 @@ static const struct of_device_id psci_of_match[] __initconst = {
> > > {},
[...]
> > > + continue;
> > > +
> > > + num = of_property_read_variable_u32_array(np, prop->name, magic, 1, 2);
> >
> > ARRAY_SIZE(magic)?
> >
> > > + if (num < 0) {
> >
> > Should this be less than 1?
> >
>
> of_property_read_variable_u32_array should return -EOVERFLOW (or maybe
> -ENODATA) if the array is empty. I don't see it's possible for
> of_property_read_variable_u32_array() to return a non-negative value
> that's not 1 or 2.
I think you're saying a return value of 0 is impossible? Ok. I was
mostly looking at the usage of the return value later on in this patch
and trying to understand why 0 would be allowed as a possible return
value without looking at the details of
of_property_read_variable_u32_array(). I guess the 1, 2 are the min/max
though so it's fine.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-10-23 19:02 ` Stephen Boyd
@ 2024-10-23 22:16 ` Elliot Berman
0 siblings, 0 replies; 17+ messages in thread
From: Elliot Berman @ 2024-10-23 22:16 UTC (permalink / raw)
To: Stephen Boyd
Cc: Andy Yan, Arnd Bergmann, Bartosz Golaszewski, Bjorn Andersson,
Catalin Marinas, Conor Dooley, Konrad Dybcio, Krzysztof Kozlowski,
Krzysztof Kozlowski, Lorenzo Pieralisi, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
On Wed, Oct 23, 2024 at 12:02:19PM -0700, Stephen Boyd wrote:
> Quoting Elliot Berman (2024-10-23 09:30:21)
> > On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
> > > Quoting Elliot Berman (2024-10-18 12:39:48)
> > > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > > index 2328ca58bba6..60bc285622ce 100644
> > > > --- a/drivers/firmware/psci/psci.c
> > > > +++ b/drivers/firmware/psci/psci.c
> > > > @@ -305,9 +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;
> > > > + unsigned long ret;
> > > > + size_t i;
> > > > +
> > > > + for (i = 0; i < num_psci_reset_params; i++) {
> > > > + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > > + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > > + psci_reset_params[i].reset_type,
> > > > + psci_reset_params[i].cookie, 0);
> > > > + pr_err("failed to perform reset \"%s\": %ld\n",
> > > > + cmd, (long)ret);
> > >
> > > Do this intentionally return? Should it be some other function that's
> > > __noreturn instead and a while (1) if the firmware returns back to the
> > > kernel?
> > >
> >
> > Yes, I think it's best to make sure we fall back to the architectural
> > reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
> > since device would reboot then.
>
> Ok. Please add a comment in the code so we know that it's intentional.
>
> >
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > > > void *data)
> > > > {
> > > > + if (data && num_psci_reset_params)
> > > > + psci_vendor_sys_reset2(action, data);
> > > > +
>
> I'd add a comment here as well indicating that a fallback is used.
>
Ack.
Thanks for the feedback!
- Elliot
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-10-23 16:30 ` Elliot Berman
2024-10-23 19:02 ` Stephen Boyd
@ 2024-11-15 13:35 ` Lorenzo Pieralisi
2024-11-15 18:53 ` Florian Fainelli
2024-11-15 19:08 ` Elliot Berman
1 sibling, 2 replies; 17+ messages in thread
From: Lorenzo Pieralisi @ 2024-11-15 13:35 UTC (permalink / raw)
To: Elliot Berman
Cc: Stephen Boyd, Andy Yan, Arnd Bergmann, Bartosz Golaszewski,
Bjorn Andersson, Catalin Marinas, Conor Dooley, Konrad Dybcio,
Krzysztof Kozlowski, Krzysztof Kozlowski, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
On Wed, Oct 23, 2024 at 09:30:21AM -0700, Elliot Berman wrote:
> On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
> > Quoting Elliot Berman (2024-10-18 12:39:48)
> > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > index 2328ca58bba6..60bc285622ce 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-"
> >
> > Maybe move this near the function that uses it.
> >
> > > +
> > > /*
> > > * 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.
> > > @@ -305,9 +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;
> > > + unsigned long ret;
> > > + size_t i;
> > > +
> > > + for (i = 0; i < num_psci_reset_params; i++) {
> > > + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > + psci_reset_params[i].reset_type,
> > > + psci_reset_params[i].cookie, 0);
> > > + pr_err("failed to perform reset \"%s\": %ld\n",
> > > + cmd, (long)ret);
> >
> > Do this intentionally return? Should it be some other function that's
> > __noreturn instead and a while (1) if the firmware returns back to the
> > kernel?
> >
>
> Yes, I think it's best to make sure we fall back to the architectural
> reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
> since device would reboot then.
Well, that's one of the doubts I have about enabling this code. From
userspace we are requesting a reboot (I don't even think that user
space knows which reboot modes are actually implemented (?)) and we may
end up issuing one with completely different semantics ?
Are these "reset types" exported to user space ?
Lorenzo
> > > + }
> > > + }
> > > +}
> > > +
> > > static int psci_sys_reset(struct notifier_block *nb, unsigned long action,
> > > void *data)
> > > {
> > > + if (data && num_psci_reset_params)
> > > + psci_vendor_sys_reset2(action, data);
> > > +
> > > if ((reboot_mode == REBOOT_WARM || reboot_mode == REBOOT_SOFT) &&
> > > psci_system_reset2_supported) {
> > > /*
> > > @@ -750,6 +780,68 @@ 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 *psci_np __free(device_node) = NULL;
> > > + struct device_node *np __free(device_node) = NULL;
> > > + struct property *prop;
> > > + size_t count = 0;
> > > + u32 magic[2];
> > > + int num;
> > > +
> > > + if (!psci_system_reset2_supported)
> > > + return 0;
> > > +
> > > + psci_np = of_find_matching_node(NULL, psci_of_match);
> > > + if (!psci_np)
> > > + return 0;
> > > +
> > > + np = of_find_node_by_name(psci_np, "reset-types");
> > > + if (!np)
> > > + return 0;
> > > +
> > > + for_each_property_of_node(np, prop) {
> > > + if (strncmp(prop->name, REBOOT_PREFIX, len))
> > > + continue;
> > > + num = of_property_count_elems_of_size(np, prop->name, sizeof(magic[0]));
> >
> > Use of_property_count_u32_elems()?
> >
> > > + if (num != 1 && num != 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;
> > > +
> > > + num = of_property_read_variable_u32_array(np, prop->name, magic, 1, 2);
> >
> > ARRAY_SIZE(magic)?
> >
> > > + if (num < 0) {
> >
> > Should this be less than 1?
> >
>
> of_property_read_variable_u32_array should return -EOVERFLOW (or maybe
> -ENODATA) if the array is empty. I don't see it's possible for
> of_property_read_variable_u32_array() to return a non-negative value
> that's not 1 or 2.
>
> > > + pr_warn("Failed to parse vendor reboot mode %s\n", param->mode);
> > > + kfree_const(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 = num == 2 ? magic[1] : 0;
> >
> > ARRAY_SIZE(magic)?
> >
> > > + param++;
> > > + num_psci_reset_params++;
> > > + }
> > > +
> > > + return 0;
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-11-15 13:35 ` Lorenzo Pieralisi
@ 2024-11-15 18:53 ` Florian Fainelli
2024-11-15 19:08 ` Elliot Berman
1 sibling, 0 replies; 17+ messages in thread
From: Florian Fainelli @ 2024-11-15 18:53 UTC (permalink / raw)
To: Lorenzo Pieralisi, Elliot Berman
Cc: Stephen Boyd, Andy Yan, Arnd Bergmann, Bartosz Golaszewski,
Bjorn Andersson, Catalin Marinas, Conor Dooley, Konrad Dybcio,
Krzysztof Kozlowski, Krzysztof Kozlowski, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, linux-pm,
linux-arm-msm
On 11/15/24 05:35, Lorenzo Pieralisi wrote:
> On Wed, Oct 23, 2024 at 09:30:21AM -0700, Elliot Berman wrote:
>> On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
>>> Quoting Elliot Berman (2024-10-18 12:39:48)
>>>> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
>>>> index 2328ca58bba6..60bc285622ce 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-"
>>>
>>> Maybe move this near the function that uses it.
>>>
>>>> +
>>>> /*
>>>> * 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.
>>>> @@ -305,9 +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;
>>>> + unsigned long ret;
>>>> + size_t i;
>>>> +
>>>> + for (i = 0; i < num_psci_reset_params; i++) {
>>>> + if (!strcmp(psci_reset_params[i].mode, cmd)) {
>>>> + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
>>>> + psci_reset_params[i].reset_type,
>>>> + psci_reset_params[i].cookie, 0);
>>>> + pr_err("failed to perform reset \"%s\": %ld\n",
>>>> + cmd, (long)ret);
>>>
>>> Do this intentionally return? Should it be some other function that's
>>> __noreturn instead and a while (1) if the firmware returns back to the
>>> kernel?
>>>
>>
>> Yes, I think it's best to make sure we fall back to the architectural
>> reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
>> since device would reboot then.
>
> Well, that's one of the doubts I have about enabling this code. From
> userspace we are requesting a reboot (I don't even think that user
> space knows which reboot modes are actually implemented (?)) and we may
> end up issuing one with completely different semantics ?
>
> Are these "reset types" exported to user space ?
AFAICT, they are not, but arguably you already need custom user space
which is capable of doing:
syscall(SYS_reboot, LINUX_REBOOT_MAGIC1, LINUX_REBOOT_MAGIC2,
LINUX_REBOOT_CMD_RESTART2, reboot_cmd);
in order to utilize the custom reboot mode. I could imagine that with a
discovery mechanism, a wrapper could be written to check that the
specified command is actually supported before issuing the system call,
or even have the system call do that under the hood.
I don't personally feel like this is very important in the sense that as
long as a fallback exists for an unsupported reboot command specified,
the system does reboot.
--
Florian
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-11-15 13:35 ` Lorenzo Pieralisi
2024-11-15 18:53 ` Florian Fainelli
@ 2024-11-15 19:08 ` Elliot Berman
2024-11-18 15:13 ` Lorenzo Pieralisi
1 sibling, 1 reply; 17+ messages in thread
From: Elliot Berman @ 2024-11-15 19:08 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Boyd, Andy Yan, Arnd Bergmann, Bartosz Golaszewski,
Bjorn Andersson, Catalin Marinas, Conor Dooley, Konrad Dybcio,
Krzysztof Kozlowski, Krzysztof Kozlowski, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
On Fri, Nov 15, 2024 at 02:35:52PM +0100, Lorenzo Pieralisi wrote:
> On Wed, Oct 23, 2024 at 09:30:21AM -0700, Elliot Berman wrote:
> > On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
> > > Quoting Elliot Berman (2024-10-18 12:39:48)
> > > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > > index 2328ca58bba6..60bc285622ce 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-"
> > >
> > > Maybe move this near the function that uses it.
> > >
> > > > +
> > > > /*
> > > > * 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.
> > > > @@ -305,9 +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;
> > > > + unsigned long ret;
> > > > + size_t i;
> > > > +
> > > > + for (i = 0; i < num_psci_reset_params; i++) {
> > > > + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > > + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > > + psci_reset_params[i].reset_type,
> > > > + psci_reset_params[i].cookie, 0);
> > > > + pr_err("failed to perform reset \"%s\": %ld\n",
> > > > + cmd, (long)ret);
> > >
> > > Do this intentionally return? Should it be some other function that's
> > > __noreturn instead and a while (1) if the firmware returns back to the
> > > kernel?
> > >
> >
> > Yes, I think it's best to make sure we fall back to the architectural
> > reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
> > since device would reboot then.
>
> Well, that's one of the doubts I have about enabling this code. From
> userspace we are requesting a reboot (I don't even think that user
> space knows which reboot modes are actually implemented (?)) and we may
> end up issuing one with completely different semantics ?
You're right here, userspace issue a "reboot bootloader" and if kernel
doesn't have the support to set up the right cookie, the device would do
a normal reboot and not stop at the bootloader. This problem exists
today and I think whether this is an issue to solve is out of scope here.
>
> Are these "reset types" exported to user space ?
>
No mechanism exists to do that. We could do something specific for PSCI
or do something generic for everybody. I don't think something specific
for PSCI is the right approach because it's a general problem. I don't
think there's enough interest to change reboot command plumbing to
advertise valid reset types to userspace.
- Elliot
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-11-15 19:08 ` Elliot Berman
@ 2024-11-18 15:13 ` Lorenzo Pieralisi
2024-11-18 19:27 ` Elliot Berman
0 siblings, 1 reply; 17+ messages in thread
From: Lorenzo Pieralisi @ 2024-11-18 15:13 UTC (permalink / raw)
To: Elliot Berman
Cc: Stephen Boyd, Andy Yan, Arnd Bergmann, Bartosz Golaszewski,
Bjorn Andersson, Catalin Marinas, Conor Dooley, Konrad Dybcio,
Krzysztof Kozlowski, Krzysztof Kozlowski, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
On Fri, Nov 15, 2024 at 11:08:22AM -0800, Elliot Berman wrote:
> On Fri, Nov 15, 2024 at 02:35:52PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Oct 23, 2024 at 09:30:21AM -0700, Elliot Berman wrote:
> > > On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
> > > > Quoting Elliot Berman (2024-10-18 12:39:48)
> > > > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > > > index 2328ca58bba6..60bc285622ce 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-"
> > > >
> > > > Maybe move this near the function that uses it.
> > > >
> > > > > +
> > > > > /*
> > > > > * 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.
> > > > > @@ -305,9 +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;
> > > > > + unsigned long ret;
> > > > > + size_t i;
> > > > > +
> > > > > + for (i = 0; i < num_psci_reset_params; i++) {
> > > > > + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > > > + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > > > + psci_reset_params[i].reset_type,
> > > > > + psci_reset_params[i].cookie, 0);
> > > > > + pr_err("failed to perform reset \"%s\": %ld\n",
> > > > > + cmd, (long)ret);
> > > >
> > > > Do this intentionally return? Should it be some other function that's
> > > > __noreturn instead and a while (1) if the firmware returns back to the
> > > > kernel?
> > > >
> > >
> > > Yes, I think it's best to make sure we fall back to the architectural
> > > reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
> > > since device would reboot then.
> >
> > Well, that's one of the doubts I have about enabling this code. From
> > userspace we are requesting a reboot (I don't even think that user
> > space knows which reboot modes are actually implemented (?)) and we may
> > end up issuing one with completely different semantics ?
>
> You're right here, userspace issue a "reboot bootloader" and if kernel
> doesn't have the support to set up the right cookie, the device would do
> a normal reboot and not stop at the bootloader. This problem exists
> today and I think whether this is an issue to solve is out of scope here.
That's true. It is the same issue we have with reboot_mode anyway.
Is it a fair statement to say that currently when we request a reboot,
the reboot mode is the one set through /sys/kernel/reboot/mode ?
Does user space use that file today ?
I guess userspace does not take specific actions according to the
reset it thinks it issues - it is a question.
> > Are these "reset types" exported to user space ?
> >
>
> No mechanism exists to do that. We could do something specific for PSCI
> or do something generic for everybody. I don't think something specific
> for PSCI is the right approach because it's a general problem. I don't
> think there's enough interest to change reboot command plumbing to
> advertise valid reset types to userspace.
That's for sure. I suppose the most important bit is making sure that
all resets comply with the kernel semantics expected from a *reset*;
I appreciate that's a vague statement (and I have no idea how to enforce
it) but that's the gist of this discussion.
Another thing I am worried about is device drivers restart handlers
(ie having to parse a command that might be platform specific in a
generic driver to grok what reset was actually issued and what action
should be taken).
I admit it is a tough nut to crack this one - apologies for the time
it is taking to reach an agreement.
Lorenzo
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v6 3/5] firmware: psci: Read and use vendor reset types
2024-11-18 15:13 ` Lorenzo Pieralisi
@ 2024-11-18 19:27 ` Elliot Berman
0 siblings, 0 replies; 17+ messages in thread
From: Elliot Berman @ 2024-11-18 19:27 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Stephen Boyd, Andy Yan, Arnd Bergmann, Bartosz Golaszewski,
Bjorn Andersson, Catalin Marinas, Conor Dooley, Konrad Dybcio,
Krzysztof Kozlowski, Krzysztof Kozlowski, Mark Rutland,
Olof Johansson, Rob Herring, Sebastian Reichel, Vinod Koul,
Will Deacon, cros-qcom-dts-watchers,
Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
On Mon, Nov 18, 2024 at 04:13:47PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Nov 15, 2024 at 11:08:22AM -0800, Elliot Berman wrote:
> > On Fri, Nov 15, 2024 at 02:35:52PM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Oct 23, 2024 at 09:30:21AM -0700, Elliot Berman wrote:
> > > > On Fri, Oct 18, 2024 at 10:42:46PM -0700, Stephen Boyd wrote:
> > > > > Quoting Elliot Berman (2024-10-18 12:39:48)
> > > > > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > > > > index 2328ca58bba6..60bc285622ce 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-"
> > > > >
> > > > > Maybe move this near the function that uses it.
> > > > >
> > > > > > +
> > > > > > /*
> > > > > > * 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.
> > > > > > @@ -305,9 +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;
> > > > > > + unsigned long ret;
> > > > > > + size_t i;
> > > > > > +
> > > > > > + for (i = 0; i < num_psci_reset_params; i++) {
> > > > > > + if (!strcmp(psci_reset_params[i].mode, cmd)) {
> > > > > > + ret = invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
> > > > > > + psci_reset_params[i].reset_type,
> > > > > > + psci_reset_params[i].cookie, 0);
> > > > > > + pr_err("failed to perform reset \"%s\": %ld\n",
> > > > > > + cmd, (long)ret);
> > > > >
> > > > > Do this intentionally return? Should it be some other function that's
> > > > > __noreturn instead and a while (1) if the firmware returns back to the
> > > > > kernel?
> > > > >
> > > >
> > > > Yes, I think it's best to make sure we fall back to the architectural
> > > > reset (whether it's the SYSTEM_RESET or architectural SYSTEM_RESET2)
> > > > since device would reboot then.
> > >
> > > Well, that's one of the doubts I have about enabling this code. From
> > > userspace we are requesting a reboot (I don't even think that user
> > > space knows which reboot modes are actually implemented (?)) and we may
> > > end up issuing one with completely different semantics ?
> >
> > You're right here, userspace issue a "reboot bootloader" and if kernel
> > doesn't have the support to set up the right cookie, the device would do
> > a normal reboot and not stop at the bootloader. This problem exists
> > today and I think whether this is an issue to solve is out of scope here.
>
> That's true. It is the same issue we have with reboot_mode anyway.
>
> Is it a fair statement to say that currently when we request a reboot,
> the reboot mode is the one set through /sys/kernel/reboot/mode ?
>
> Does user space use that file today ?
>
> I guess userspace does not take specific actions according to the
> reset it thinks it issues - it is a question.
>
Yes, user space can write to that file. User space has to configure both
the mode and command to get the desired reboot configuration. I view the
vendor reset types as replacing "both", in the sense userspace may not
need to configure the reboot mode anymore. If "reboot bootloader" or
"reboot edl" requires a warm reset, the firmware knows that's how the
PMIC needs to be configured. I don't currently see any need for Linux to
be aware that a particular vendor reset type is "like a soft" or "like a
warm" or "like a cold" reset.
> > > Are these "reset types" exported to user space ?
> > >
> >
> > No mechanism exists to do that. We could do something specific for PSCI
> > or do something generic for everybody. I don't think something specific
> > for PSCI is the right approach because it's a general problem. I don't
> > think there's enough interest to change reboot command plumbing to
> > advertise valid reset types to userspace.
>
> That's for sure. I suppose the most important bit is making sure that
> all resets comply with the kernel semantics expected from a *reset*;
> I appreciate that's a vague statement (and I have no idea how to enforce
> it) but that's the gist of this discussion.
>
> Another thing I am worried about is device drivers restart handlers
> (ie having to parse a command that might be platform specific in a
> generic driver to grok what reset was actually issued and what action
> should be taken).
Right, I got your point! I haven't seen any drivers that care about it,
besides the ones that actually do the resetting.
I'm okay to say that all vendor SYSTEM_RESET2 need to be treated like a
REBOOT_COLD, but we might run into issue in future where we might want
some vendor SYSTEM_RESET2 to act be closer to some other mode. I suppose
then a device-specific driver is needed there.
In the hypothetical situation where we need reboot_mode to be a specific
value for a vendor SYSTEM_RESET2, I like my current approach. Userspace
already needs to align the mode and command without the kernel enforcing
it, so it's possible we could still use the generic PSCI driver without
needing to write a device-specific driver to issue the vendor
SYSTEM_RESET2. If we hard-code that vendor SYSTEM_RESET2 must be like a
cold reset, then we definitely need a device-specific driver if we'd
rather it be like a warm or soft mode.
> I admit it is a tough nut to crack this one - apologies for the time
> it is taking to reach an agreement.
>
This is a weird one :) It seems simple at first but it flexes the design
of reboot mode and command. I appreciate the time you've taken to look
at this!
- Elliot
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v6 4/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp
2024-10-18 19:39 [PATCH v6 0/5] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
` (2 preceding siblings ...)
2024-10-18 19:39 ` [PATCH v6 3/5] firmware: psci: Read and use vendor reset types Elliot Berman
@ 2024-10-18 19:39 ` Elliot Berman
2024-10-18 19:39 ` [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride Elliot Berman
4 siblings, 0 replies; 17+ messages in thread
From: Elliot Berman @ 2024-10-18 19:39 UTC (permalink / raw)
To: Bjorn Andersson, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Andy Yan,
Lorenzo Pieralisi, Mark Rutland, Bartosz Golaszewski,
Arnd Bergmann, Olof Johansson, Catalin Marinas, Will Deacon,
cros-qcom-dts-watchers, Krzysztof Kozlowski, Konrad Dybcio,
Konrad Dybcio
Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm, Elliot Berman
Add nodes for the vendor-defined system resets. "bootloader" will cause
device to reboot and stop in the bootloader's fastboot mode. "edl" will
cause device to reboot into "emergency download mode", which permits
loading images via the Firehose protocol.
Co-developed-by: Shivendra Pratap <quic_spratap@quicinc.com>
Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
arch/arm64/boot/dts/qcom/qcm6490-idp.dts | 7 +++++++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
index a0668f767e4b..9c141244a7b2 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-idp.dts
@@ -617,6 +617,13 @@ &pon_resin {
status = "okay";
};
+&psci {
+ reset-types {
+ mode-bootloader = <0x10001 0x2>;
+ mode-edl = <0 0x1>;
+ };
+};
+
&qupv3_id_0 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 3d8410683402..5360d0e51a65 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -850,7 +850,7 @@ pmu {
interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
};
- psci {
+ psci: psci {
compatible = "arm,psci-1.0";
method = "smc";
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride
2024-10-18 19:39 [PATCH v6 0/5] Implement vendor resets for PSCI SYSTEM_RESET2 Elliot Berman
` (3 preceding siblings ...)
2024-10-18 19:39 ` [PATCH v6 4/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for qcm6490-idp Elliot Berman
@ 2024-10-18 19:39 ` Elliot Berman
2024-10-18 19:51 ` Konrad Dybcio
2024-10-20 19:52 ` kernel test robot
4 siblings, 2 replies; 17+ messages in thread
From: Elliot Berman @ 2024-10-18 19:39 UTC (permalink / raw)
To: Bjorn Andersson, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Andy Yan,
Lorenzo Pieralisi, Mark Rutland, Bartosz Golaszewski,
Arnd Bergmann, Olof Johansson, Catalin Marinas, Will Deacon,
cros-qcom-dts-watchers, Krzysztof Kozlowski, Konrad Dybcio,
Konrad Dybcio
Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm, Elliot Berman
Add nodes for the vendor-defined system resets. "bootloader" will cause
device to reboot and stop in the bootloader's fastboot mode. "edl" will
cause device to reboot into "emergency download mode", which permits
loading images via the Firehose protocol.
Co-developed-by: Shivendra Pratap <quic_spratap@quicinc.com>
Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
---
arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 7 +++++++
arch/arm64/boot/dts/qcom/sa8775p.dtsi | 2 +-
include/linux/arm-smccc.h | 5 +++++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
index 2a6170623ea9..b0eb779b3ec5 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi
@@ -498,6 +498,13 @@ &pmm8654au_3_gpios {
"GNSS_BOOT_MODE";
};
+&psci {
+ reset-types {
+ mode-bootloader = <0x10001 0x2>;
+ mode-edl <0 0x1>;
+ };
+};
+
&qupv3_id_1 {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sa8775p.dtsi b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
index 23f1b2e5e624..dd36eea80f7c 100644
--- a/arch/arm64/boot/dts/qcom/sa8775p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8775p.dtsi
@@ -329,7 +329,7 @@ pmu {
interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
};
- psci {
+ psci: psci {
compatible = "arm,psci-1.0";
method = "smc";
};
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 083f85653716..bdc974b76df8 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -85,6 +85,11 @@
ARM_SMCCC_SMC_32, \
0, 2)
+#define ARM_SMCCC_ARCH_FEATURE_AVAILABILITY_ID \
+ ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
+ ARM_SMCCC_SMC_32, \
+ 0, 3)
+
#define ARM_SMCCC_ARCH_WORKAROUND_1 \
ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
ARM_SMCCC_SMC_32, \
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride
2024-10-18 19:39 ` [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride Elliot Berman
@ 2024-10-18 19:51 ` Konrad Dybcio
2024-10-20 19:52 ` kernel test robot
1 sibling, 0 replies; 17+ messages in thread
From: Konrad Dybcio @ 2024-10-18 19:51 UTC (permalink / raw)
To: Elliot Berman, Bjorn Andersson, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Andy Yan,
Lorenzo Pieralisi, Mark Rutland, Bartosz Golaszewski,
Arnd Bergmann, Olof Johansson, Catalin Marinas, Will Deacon,
cros-qcom-dts-watchers, Krzysztof Kozlowski, Konrad Dybcio
Cc: Satya Durga Srinivasu Prabhala, Melody Olvera, Shivendra Pratap,
devicetree, linux-kernel, linux-arm-kernel, Florian Fainelli,
linux-pm, linux-arm-msm
On 18.10.2024 9:39 PM, Elliot Berman wrote:
> Add nodes for the vendor-defined system resets. "bootloader" will cause
> device to reboot and stop in the bootloader's fastboot mode. "edl" will
> cause device to reboot into "emergency download mode", which permits
> loading images via the Firehose protocol.
>
> Co-developed-by: Shivendra Pratap <quic_spratap@quicinc.com>
> Signed-off-by: Shivendra Pratap <quic_spratap@quicinc.com>
> Signed-off-by: Elliot Berman <quic_eberman@quicinc.com>
> ---
> arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi | 7 +++++++
> arch/arm64/boot/dts/qcom/sa8775p.dtsi | 2 +-
> include/linux/arm-smccc.h | 5 +++++
[...]
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 083f85653716..bdc974b76df8 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -85,6 +85,11 @@
> ARM_SMCCC_SMC_32, \
> 0, 2)
>
> +#define ARM_SMCCC_ARCH_FEATURE_AVAILABILITY_ID \
> + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> + ARM_SMCCC_SMC_32, \
> + 0, 3)
> +
> #define ARM_SMCCC_ARCH_WORKAROUND_1 \
> ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> ARM_SMCCC_SMC_32, \
>
Unused hunk
Konrad
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride
2024-10-18 19:39 ` [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride Elliot Berman
2024-10-18 19:51 ` Konrad Dybcio
@ 2024-10-20 19:52 ` kernel test robot
1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2024-10-20 19:52 UTC (permalink / raw)
To: Elliot Berman, Bjorn Andersson, Sebastian Reichel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Andy Yan,
Lorenzo Pieralisi, Mark Rutland, Bartosz Golaszewski,
Arnd Bergmann, Olof Johansson, Catalin Marinas, Will Deacon,
cros-qcom-dts-watchers, Konrad Dybcio
Cc: llvm, oe-kbuild-all, Satya Durga Srinivasu Prabhala,
Melody Olvera, Shivendra Pratap, devicetree, linux-kernel,
linux-arm-kernel, Florian Fainelli, linux-pm, linux-arm-msm,
Elliot Berman
Hi Elliot,
kernel test robot noticed the following build errors:
[auto build test ERROR on 98f7e32f20d28ec452afb208f9cffc08448a2652]
url: https://github.com/intel-lab-lkp/linux/commits/Elliot-Berman/dt-bindings-power-reset-Convert-mode-properties-to-array/20241019-034308
base: 98f7e32f20d28ec452afb208f9cffc08448a2652
patch link: https://lore.kernel.org/r/20241018-arm-psci-system_reset2-vendor-reboots-v6-5-50cbe88b0a24%40quicinc.com
patch subject: [PATCH v6 5/5] arm64: dts: qcom: Add PSCI SYSTEM_RESET2 types for sa8775p-ride
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241021/202410210333.KGHmzwGN-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project bfe84f7085d82d06d61c632a7bad1e692fd159e4)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241021/202410210333.KGHmzwGN-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/202410210333.KGHmzwGN-lkp@intel.com/
All errors (new ones prefixed by >>):
>> Error: arch/arm64/boot/dts/qcom/sa8775p-ride.dtsi:504.12-13 syntax error
FATAL ERROR: Unable to parse input tree
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 17+ messages in thread