PSCI implements a restart notifier for architectural defined resets.
The SYSTEM_RESET2 call allows vendor firmware to define additional reset
types which could be mapped to the reboot reason.
Implement a driver to wire the reboot-mode framework to make vendor
SYSTEM_RESET2 calls on reboot.
This is a continuation from https://lore.kernel.org/all/[email protected]/
Elliot Berman (4):
power: reset: reboot-mode: Allow magic to be 0
power: reset: reboot-mode: Wire reboot_mode enum to magic
dt-bindings: power: reset: Document arm,psci-vendor-reset
power: reset: Implement a PSCI SYSTEM_RESET2 reboot-mode driver
.../power/reset/arm,psci-vendor-reset.yaml | 35 +++++++++++++
MAINTAINERS | 2 +
drivers/firmware/psci/psci.c | 9 ++++
drivers/power/reset/Kconfig | 9 ++++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/psci-vendor-reset.c | 49 +++++++++++++++++++
drivers/power/reset/reboot-mode.c | 44 ++++++++++++-----
include/linux/psci.h | 2 +
8 files changed, 139 insertions(+), 12 deletions(-)
create mode 100644 Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml
create mode 100644 drivers/power/reset/psci-vendor-reset.c
base-commit: 6eaae198076080886b9e7d57f4ae06fa782f90ef
--
2.41.0
Allow magic from the reboot-mode driver to be defined, but 0. This is
useful when the register/command to trigger reboot needs 0 to be
written. This is the case when the "default" reboot mode is to enter a
crash dump collection mode (e.g. when triggered by a watchdog) and Linux
doing a normal reboot requires the setting to be explicitly reset to
zero.
Signed-off-by: Elliot Berman <[email protected]>
---
drivers/power/reset/reboot-mode.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index b4076b10b893..a646aefa041b 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -19,11 +19,10 @@ struct mode_info {
struct list_head list;
};
-static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
- const char *cmd)
+static bool get_reboot_mode_magic(struct reboot_mode_driver *reboot,
+ const char *cmd, unsigned int *magic)
{
const char *normal = "normal";
- int magic = 0;
struct mode_info *info;
if (!cmd)
@@ -31,12 +30,12 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
list_for_each_entry(info, &reboot->head, list) {
if (!strcmp(info->mode, cmd)) {
- magic = info->magic;
- break;
+ *magic = info->magic;
+ return true;
}
}
- return magic;
+ return false;
}
static int reboot_mode_notify(struct notifier_block *this,
@@ -46,8 +45,7 @@ static int reboot_mode_notify(struct notifier_block *this,
unsigned int magic;
reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
- magic = get_reboot_mode_magic(reboot, cmd);
- if (magic)
+ if (get_reboot_mode_magic(reboot, cmd, &magic))
reboot->write(reboot, magic);
return NOTIFY_DONE;
--
2.41.0
Allow the reboot mode type to be wired to magic.
Signed-off-by: Elliot Berman <[email protected]>
---
drivers/power/reset/reboot-mode.c | 30 ++++++++++++++++++++++++++----
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
index a646aefa041b..4ea97ccbaf51 100644
--- a/drivers/power/reset/reboot-mode.c
+++ b/drivers/power/reset/reboot-mode.c
@@ -22,12 +22,8 @@ struct mode_info {
static bool get_reboot_mode_magic(struct reboot_mode_driver *reboot,
const char *cmd, unsigned int *magic)
{
- const char *normal = "normal";
struct mode_info *info;
- if (!cmd)
- cmd = normal;
-
list_for_each_entry(info, &reboot->head, list) {
if (!strcmp(info->mode, cmd)) {
*magic = info->magic;
@@ -45,6 +41,32 @@ static int reboot_mode_notify(struct notifier_block *this,
unsigned int magic;
reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
+
+ if (!cmd) {
+ switch (mode) {
+ case REBOOT_COLD:
+ cmd = "cold";
+ break;
+ case REBOOT_WARM:
+ cmd = "warm";
+ break;
+ case REBOOT_HARD:
+ cmd = "hard";
+ break;
+ case REBOOT_SOFT:
+ cmd = "soft";
+ break;
+ case REBOOT_GPIO:
+ cmd = "gpio";
+ break;
+ }
+ if (get_reboot_mode_magic(reboot, cmd, &magic)) {
+ reboot->write(reboot, magic);
+ return NOTIFY_DONE;
+ }
+ cmd = "normal";
+ }
+
if (get_reboot_mode_magic(reboot, cmd, &magic))
reboot->write(reboot, magic);
--
2.41.0
PSCI implements a restart notifier for architectural defined resets.
The SYSTEM_RESET2 allows vendor firmware to define additional reset
types which could be mapped to the reboot reason.
Implement a driver to wire the reboot-mode framework to make vendor
SYSTEM_RESET2 calls on reboot.
Signed-off-by: Elliot Berman <[email protected]>
---
MAINTAINERS | 1 +
drivers/firmware/psci/psci.c | 9 +++++
drivers/power/reset/Kconfig | 9 +++++
drivers/power/reset/Makefile | 1 +
drivers/power/reset/psci-vendor-reset.c | 49 +++++++++++++++++++++++++
include/linux/psci.h | 2 +
6 files changed, 71 insertions(+)
create mode 100644 drivers/power/reset/psci-vendor-reset.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2da4c5f1917b..214b14c1da63 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16984,6 +16984,7 @@ L: [email protected] (moderated for non-subscribers)
S: Maintained
F: Documentation/devicetree/bindings/power/reset/arm,psci-vendor-reset.yaml
F: drivers/firmware/psci/
+F: drivers/power/reset/psci-vendor-reset.c
F: include/linux/psci.h
F: include/uapi/linux/psci.h
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index d9629ff87861..6db73f9d2304 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -328,6 +328,15 @@ static struct notifier_block psci_sys_reset_nb = {
.priority = 129,
};
+void psci_vendor_sys_reset2(u32 reset_type, u32 cookie)
+{
+ if (psci_system_reset2_supported)
+ invoke_psci_fn(PSCI_FN_NATIVE(1_1, SYSTEM_RESET2),
+ reset_type | BIT_ULL(31),
+ cookie, 0);
+}
+EXPORT_SYMBOL_GPL(psci_vendor_sys_reset2);
+
static void psci_sys_poweroff(void)
{
invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
diff --git a/drivers/power/reset/Kconfig b/drivers/power/reset/Kconfig
index fff07b2bd77b..1474b9d51089 100644
--- a/drivers/power/reset/Kconfig
+++ b/drivers/power/reset/Kconfig
@@ -311,4 +311,13 @@ config POWER_MLXBF
help
This driver supports reset or low power mode handling for Mellanox BlueField.
+config POWER_RESET_PSCI_VENDOR_RESET
+ tristate "PSCI Vendor SYSTEM_RESET2 driver"
+ depends on ARM64 || ARM || COMPILE_TEST
+ select ARM_PSCI_FW
+ select REBOOT_MODE
+ help
+ Say y/m here to enable driver to use Vendor SYSTEM_RESET2 types on
+ chips which have firmware implementing custom SYSTEM_RESET2 types.
+
endif
diff --git a/drivers/power/reset/Makefile b/drivers/power/reset/Makefile
index d763e6735ee3..d09243966b74 100644
--- a/drivers/power/reset/Makefile
+++ b/drivers/power/reset/Makefile
@@ -37,3 +37,4 @@ obj-$(CONFIG_SYSCON_REBOOT_MODE) += syscon-reboot-mode.o
obj-$(CONFIG_POWER_RESET_SC27XX) += sc27xx-poweroff.o
obj-$(CONFIG_NVMEM_REBOOT_MODE) += nvmem-reboot-mode.o
obj-$(CONFIG_POWER_MLXBF) += pwr-mlxbf.o
+obj-$(CONFIG_POWER_RESET_PSCI_VENDOR_RESET) += psci-vendor-reset.o
diff --git a/drivers/power/reset/psci-vendor-reset.c b/drivers/power/reset/psci-vendor-reset.c
new file mode 100644
index 000000000000..95d027225185
--- /dev/null
+++ b/drivers/power/reset/psci-vendor-reset.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/**
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/reboot-mode.h>
+#include <linux/psci.h>
+
+static int psci_vendor_system_reset2(struct reboot_mode_driver *reboot, unsigned int magic)
+{
+ psci_vendor_sys_reset2(magic, 0);
+
+ return NOTIFY_DONE;
+}
+
+static int psci_vendor_reset_probe(struct platform_device *pdev)
+{
+ struct reboot_mode_driver *reboot;
+
+ reboot = devm_kzalloc(&pdev->dev, sizeof(*reboot), GFP_KERNEL);
+ if (!reboot)
+ return -ENOMEM;
+
+ reboot->write = psci_vendor_system_reset2;
+
+ return devm_reboot_mode_register(&pdev->dev, reboot);
+}
+
+static const struct of_device_id psci_vendor_reset_id_table[] = {
+ { .compatible = "arm,psci-vendor-reset" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, psci_vendor_reset_id_table);
+
+static struct platform_driver psci_vendor_reset_driver = {
+ .probe = psci_vendor_reset_probe,
+ .driver = {
+ .name = "psci-vendor-reset",
+ .of_match_table = of_match_ptr(psci_vendor_reset_id_table),
+ },
+};
+module_platform_driver(psci_vendor_reset_driver);
+
+MODULE_DESCRIPTION("PSCI Vendor SYSTEM_RESET2 Driver");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/psci.h b/include/linux/psci.h
index 4ca0060a3fc4..0c652c12e0a8 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -21,6 +21,8 @@ bool psci_power_state_is_valid(u32 state);
int psci_set_osi_mode(bool enable);
bool psci_has_osi_support(void);
+void psci_vendor_sys_reset2(u32 reset_type, u32 cookie);
+
struct psci_operations {
u32 (*get_version)(void);
int (*cpu_suspend)(u32 state, unsigned long entry_point);
--
2.41.0
On 25/07/2023 00:30, Elliot Berman wrote:
> PSCI implements a restart notifier for architectural defined resets.
> The SYSTEM_RESET2 allows vendor firmware to define additional reset
> types which could be mapped to the reboot reason.
>
> +
> +static const struct of_device_id psci_vendor_reset_id_table[] = {
> + { .compatible = "arm,psci-vendor-reset" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, psci_vendor_reset_id_table);
> +
> +static struct platform_driver psci_vendor_reset_driver = {
> + .probe = psci_vendor_reset_probe,
> + .driver = {
> + .name = "psci-vendor-reset",
> + .of_match_table = of_match_ptr(psci_vendor_reset_id_table),
Drop of_match_ptr() - it is useless and if used, then it must be
balanced with conditional table.
> + },
> +};
> +module_platform_driver(psci_vendor_reset_driver);
> +
Best regards,
Krzysztof
On 7/25/2023 4:00 AM, Elliot Berman wrote:
> Allow the reboot mode type to be wired to magic.
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> drivers/power/reset/reboot-mode.c | 30 ++++++++++++++++++++++++++----
> 1 file changed, 26 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index a646aefa041b..4ea97ccbaf51 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -22,12 +22,8 @@ struct mode_info {
> static bool get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> const char *cmd, unsigned int *magic)
> {
> - const char *normal = "normal";
> struct mode_info *info;
>
> - if (!cmd)
> - cmd = normal;
> -
> list_for_each_entry(info, &reboot->head, list) {
> if (!strcmp(info->mode, cmd)) {
> *magic = info->magic;
> @@ -45,6 +41,32 @@ static int reboot_mode_notify(struct notifier_block *this,
> unsigned int magic;
>
> reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
> +
> + if (!cmd) {
> + switch (mode) {
IIUC, mode will be filled up with reboot_mode during restart
notifier and not reboot notifiers ?
> + case REBOOT_COLD:
> + cmd = "cold";
> + break;
> + case REBOOT_WARM:
> + cmd = "warm";
> + break;
> + case REBOOT_HARD:
> + cmd = "hard";
> + break;
> + case REBOOT_SOFT:
> + cmd = "soft";
> + break;
> + case REBOOT_GPIO:
> + cmd = "gpio";
These strings are already there kernel/reboot.c
Can it be reused ?
#define REBOOT_COLD_STR "cold"
#define REBOOT_WARM_STR "warm"
#define REBOOT_HARD_STR "hard"
#define REBOOT_SOFT_STR "soft"
#define REBOOT_GPIO_STR "gpio"
#define REBOOT_UNDEFINED_STR "undefined"
> + break;
> + }
> + if (get_reboot_mode_magic(reboot, cmd, &magic)) {
Is info->mode is going to filled up with mode-cold, mode-warm and so
on from DT to compare against cmd?
What if , cmd is not among the one above switch, NULL pointer during
strcmp ?
-Mukesh
> + reboot->write(reboot, magic);
> + return NOTIFY_DONE;
> + }
> + cmd = "normal";
> + }
> +
> if (get_reboot_mode_magic(reboot, cmd, &magic))
> reboot->write(reboot, magic);
>
Hello,
On 7/24/23 15:30, Elliot Berman wrote:
> PSCI implements a restart notifier for architectural defined resets.
> The SYSTEM_RESET2 call allows vendor firmware to define additional reset
> types which could be mapped to the reboot reason.
>
> Implement a driver to wire the reboot-mode framework to make vendor
> SYSTEM_RESET2 calls on reboot.
>
> This is a continuation from https://lore.kernel.org/all/[email protected]/
Would appreciate being CC'd on a the non-RFC postings of this patch.
FWIW, my use case is better described with this earlier submission:
https://lore.kernel.org/lkml/[email protected]/T/#m74e4243c1af3a8d896e19b573b58f562fa09961d
It would be neat if I could leverage your driver in order to implement
this custom "reboot powercycle" implementation. Towards that goal, we
would likely need to specify the desired reboot "sub" operation
alongside its PSCI SYSTEM_RESET2 reboot type argument?
Thanks!
--
Florian
On 7/25/2023 12:12 PM, Florian Fainelli wrote:
> Hello,
>
> On 7/24/23 15:30, Elliot Berman wrote:
>> PSCI implements a restart notifier for architectural defined resets.
>> The SYSTEM_RESET2 call allows vendor firmware to define additional reset
>> types which could be mapped to the reboot reason.
>>
>> Implement a driver to wire the reboot-mode framework to make vendor
>> SYSTEM_RESET2 calls on reboot.
>>
>> This is a continuation from
>> https://lore.kernel.org/all/[email protected]/
>
> Would appreciate being CC'd on a the non-RFC postings of this patch.
> FWIW, my use case is better described with this earlier submission:
>
> https://lore.kernel.org/lkml/[email protected]/T/#m74e4243c1af3a8d896e19b573b58f562fa09961d
>
> It would be neat if I could leverage your driver in order to implement
> this custom "reboot powercycle" implementation. Towards that goal, we
> would likely need to specify the desired reboot "sub" operation
> alongside its PSCI SYSTEM_RESET2 reboot type argument?
>
> Thanks!
I think you you want to describe the PSCI vendor reset under a warm
reboot with command "powercycle"? In other words, my series only lets DT
describe either reboot_mode (warm) or cmd (powercycle) but not both
simultaneously?
Please correct me if I got it wrong! Otherwise, I can incorporate way to
describe vendor reset type matching both reboot_mode and cmd in the DT.
- Elliot
On 7/25/2023 3:03 AM, Mukesh Ojha wrote:
>
>
> On 7/25/2023 4:00 AM, Elliot Berman wrote:
>> Allow the reboot mode type to be wired to magic.
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>> ---
>> drivers/power/reset/reboot-mode.c | 30 ++++++++++++++++++++++++++----
>> 1 file changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/power/reset/reboot-mode.c
>> b/drivers/power/reset/reboot-mode.c
>> index a646aefa041b..4ea97ccbaf51 100644
>> --- a/drivers/power/reset/reboot-mode.c
>> +++ b/drivers/power/reset/reboot-mode.c
>> @@ -22,12 +22,8 @@ struct mode_info {
>> static bool get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>> const char *cmd, unsigned int *magic)
>> {
>> - const char *normal = "normal";
>> struct mode_info *info;
>> - if (!cmd)
>> - cmd = normal;
>> -
>> list_for_each_entry(info, &reboot->head, list) {
>> if (!strcmp(info->mode, cmd)) {
>> *magic = info->magic;
>> @@ -45,6 +41,32 @@ static int reboot_mode_notify(struct notifier_block
>> *this,
>> unsigned int magic;
>> reboot = container_of(this, struct reboot_mode_driver,
>> reboot_notifier);
>> +
>> + if (!cmd) {
>> + switch (mode) {
>
> IIUC, mode will be filled up with reboot_mode during restart
> notifier and not reboot notifiers ?
>
Ah, you're correct. I should register a restart notifier and not use
reboot mode framework. I'll follow similar bindings.
>> + case REBOOT_COLD:
>> + cmd = "cold";
>> + break;
>> + case REBOOT_WARM:
>> + cmd = "warm";
>> + break;
>> + case REBOOT_HARD:
>> + cmd = "hard";
>> + break;
>> + case REBOOT_SOFT:
>> + cmd = "soft";
>> + break;
>> + case REBOOT_GPIO:
>> + cmd = "gpio";
>
> These strings are already there kernel/reboot.c
> Can it be reused ?
>
> #define REBOOT_COLD_STR "cold"
> #define REBOOT_WARM_STR "warm"
> #define REBOOT_HARD_STR "hard"
> #define REBOOT_SOFT_STR "soft"
> #define REBOOT_GPIO_STR "gpio"
> #define REBOOT_UNDEFINED_STR "undefined"
>
>
One set of constants are "binding" for devicetree and the other is for
sysfs. I think they should be kept separate.
>> + break;
>> + }
>> + if (get_reboot_mode_magic(reboot, cmd, &magic)) {
>
> Is info->mode is going to filled up with mode-cold, mode-warm and so
> on from DT to compare against cmd?
>
> What if , cmd is not among the one above switch, NULL pointer during
> strcmp ?
> > -Mukesh
>
>> + reboot->write(reboot, magic);
>> + return NOTIFY_DONE;
>> + }
>> + cmd = "normal";
>> + }
>> +
>> if (get_reboot_mode_magic(reboot, cmd, &magic))
>> reboot->write(reboot, magic);
On Tue, Jul 25, 2023 at 02:04:28PM -0700, Elliot Berman wrote:
>
>
> On 7/25/2023 3:03 AM, Mukesh Ojha wrote:
> >
> >
> > On 7/25/2023 4:00 AM, Elliot Berman wrote:
> > > Allow the reboot mode type to be wired to magic.
> > >
> > > Signed-off-by: Elliot Berman <[email protected]>
> > > ---
> > > ? drivers/power/reset/reboot-mode.c | 30 ++++++++++++++++++++++++++----
> > > ? 1 file changed, 26 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/power/reset/reboot-mode.c
> > > b/drivers/power/reset/reboot-mode.c
> > > index a646aefa041b..4ea97ccbaf51 100644
> > > --- a/drivers/power/reset/reboot-mode.c
> > > +++ b/drivers/power/reset/reboot-mode.c
> > > @@ -22,12 +22,8 @@ struct mode_info {
> > > ? static bool get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> > > ??????????????????????? const char *cmd, unsigned int *magic)
> > > ? {
> > > -??? const char *normal = "normal";
> > > ????? struct mode_info *info;
> > > -??? if (!cmd)
> > > -??????? cmd = normal;
> > > -
> > > ????? list_for_each_entry(info, &reboot->head, list) {
> > > ????????? if (!strcmp(info->mode, cmd)) {
> > > ????????????? *magic = info->magic;
> > > @@ -45,6 +41,32 @@ static int reboot_mode_notify(struct
> > > notifier_block *this,
> > > ????? unsigned int magic;
> > > ????? reboot = container_of(this, struct reboot_mode_driver,
> > > reboot_notifier);
> > > +
> > > +??? if (!cmd) {
> > > +??????? switch (mode) {
> >
> > IIUC, mode will be filled up with reboot_mode during restart
> > notifier and not reboot notifiers ?
> >
>
I went through the patch in isolation and came to the same conclusion on
why you are using mode directly here. Now that it is clarified, why
not use reboot_mode directly instead of introducing restart notifiers
here?
Also you might want to clarify that we are using reboot_mode as fallback
to wire the magic.
Thanks,
Pavan
On Mon, Jul 24, 2023 at 03:30:51PM -0700, Elliot Berman wrote:
> Allow magic from the reboot-mode driver to be defined, but 0. This is
> useful when the register/command to trigger reboot needs 0 to be
> written. This is the case when the "default" reboot mode is to enter a
> crash dump collection mode (e.g. when triggered by a watchdog) and Linux
> doing a normal reboot requires the setting to be explicitly reset to
> zero.
Sorry, it is not clear to me. The current code does not treat 0 as a
valid value for magic. Since our platform has a meaning for 0 as a magic
value, we are doing this correct?
Basically we are allowing
reboot-mode {
mode-normal = <0x0>;
...
}
What is "default" reboot mode you are referring here? are you referring
to
enum reboot_mode reboot_mode DEFAULT_REBOOT_MODE
defined in kernel/reboot.c?
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> drivers/power/reset/reboot-mode.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/power/reset/reboot-mode.c b/drivers/power/reset/reboot-mode.c
> index b4076b10b893..a646aefa041b 100644
> --- a/drivers/power/reset/reboot-mode.c
> +++ b/drivers/power/reset/reboot-mode.c
> @@ -19,11 +19,10 @@ struct mode_info {
> struct list_head list;
> };
>
> -static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> - const char *cmd)
> +static bool get_reboot_mode_magic(struct reboot_mode_driver *reboot,
> + const char *cmd, unsigned int *magic)
> {
> const char *normal = "normal";
> - int magic = 0;
> struct mode_info *info;
>
> if (!cmd)
> @@ -31,12 +30,12 @@ static unsigned int get_reboot_mode_magic(struct reboot_mode_driver *reboot,
>
> list_for_each_entry(info, &reboot->head, list) {
> if (!strcmp(info->mode, cmd)) {
> - magic = info->magic;
> - break;
> + *magic = info->magic;
> + return true;
> }
> }
>
> - return magic;
> + return false;
> }
>
> static int reboot_mode_notify(struct notifier_block *this,
> @@ -46,8 +45,7 @@ static int reboot_mode_notify(struct notifier_block *this,
> unsigned int magic;
>
> reboot = container_of(this, struct reboot_mode_driver, reboot_notifier);
> - magic = get_reboot_mode_magic(reboot, cmd);
> - if (magic)
> + if (get_reboot_mode_magic(reboot, cmd, &magic))
> reboot->write(reboot, magic);
>
> return NOTIFY_DONE;
> --
> 2.41.0
>
Thanks,
Pavan
On Mon, Jul 24, 2023 at 03:30:54PM -0700, Elliot Berman wrote:
> PSCI implements a restart notifier for architectural defined resets.
> The SYSTEM_RESET2 allows vendor firmware to define additional reset
> types which could be mapped to the reboot reason.
>
> Implement a driver to wire the reboot-mode framework to make vendor
> SYSTEM_RESET2 calls on reboot.
>
> Signed-off-by: Elliot Berman <[email protected]>
Do we need to skip the PSCI call from the existing PSCI restart notifier
which gets called after your newly introduced callback from reboot mode
notifier?
Thanks,
Pavan
On 7/26/2023 3:41 AM, Pavan Kondeti wrote:
> On Mon, Jul 24, 2023 at 03:30:54PM -0700, Elliot Berman wrote:
>> PSCI implements a restart notifier for architectural defined resets.
>> The SYSTEM_RESET2 allows vendor firmware to define additional reset
>> types which could be mapped to the reboot reason.
>>
>> Implement a driver to wire the reboot-mode framework to make vendor
>> SYSTEM_RESET2 calls on reboot.
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>
> Do we need to skip the PSCI call from the existing PSCI restart notifier
> which gets called after your newly introduced callback from reboot mode
> notifier?
>
No need, the vendor SYSTEM_RESET2 call shouldn't return if the call worked.
> Thanks,
> Pavan
On 7/25/23 13:27, Elliot Berman wrote:
>
>
> On 7/25/2023 12:12 PM, Florian Fainelli wrote:
>> Hello,
>>
>> On 7/24/23 15:30, Elliot Berman wrote:
>>> PSCI implements a restart notifier for architectural defined resets.
>>> The SYSTEM_RESET2 call allows vendor firmware to define additional reset
>>> types which could be mapped to the reboot reason.
>>>
>>> Implement a driver to wire the reboot-mode framework to make vendor
>>> SYSTEM_RESET2 calls on reboot.
>>>
>>> This is a continuation from
>>> https://lore.kernel.org/all/[email protected]/
>>
>> Would appreciate being CC'd on a the non-RFC postings of this patch.
>> FWIW, my use case is better described with this earlier submission:
>>
>> https://lore.kernel.org/lkml/[email protected]/T/#m74e4243c1af3a8d896e19b573b58f562fa09961d
>>
>> It would be neat if I could leverage your driver in order to implement
>> this custom "reboot powercycle" implementation. Towards that goal, we
>> would likely need to specify the desired reboot "sub" operation
>> alongside its PSCI SYSTEM_RESET2 reboot type argument?
>>
>> Thanks!
>
> I think you you want to describe the PSCI vendor reset under a warm
> reboot with command "powercycle"? In other words, my series only lets DT
> describe either reboot_mode (warm) or cmd (powercycle) but not both
> simultaneously?
I did not give a lot of thoughts into the different types of reboot
(warm, soft, cold) and just went with an extension of whichever reboot
type we have to be supplemented by the "powercycle" command. It seems
like we should support both the reboot type and command, it would be
fine with me if I had to specify reboot_mode + cmd for each reboot_mode.
--
Florian
On Wed, Jul 26, 2023 at 10:19:01AM -0700, Elliot Berman wrote:
>
>
> On 7/26/2023 3:41 AM, Pavan Kondeti wrote:
> > On Mon, Jul 24, 2023 at 03:30:54PM -0700, Elliot Berman wrote:
> > > PSCI implements a restart notifier for architectural defined resets.
> > > The SYSTEM_RESET2 allows vendor firmware to define additional reset
> > > types which could be mapped to the reboot reason.
> > >
> > > Implement a driver to wire the reboot-mode framework to make vendor
> > > SYSTEM_RESET2 calls on reboot.
> > >
> > > Signed-off-by: Elliot Berman <[email protected]>
> >
> > Do we need to skip the PSCI call from the existing PSCI restart notifier
> > which gets called after your newly introduced callback from reboot mode
> > notifier?
> >
>
> No need, the vendor SYSTEM_RESET2 call shouldn't return if the call worked.
>
From the documentation of restart handlers, restarting from reboot
notifiers may not be correct. Since you hooked it up with reboot-mode
driver framework, you may restart the system much early before the
actual machine restart kicks in. Pls check.
Thanks,
Pavan