2022-12-28 17:00:14

by Samuel Holland

Subject: [PATCH] mfd: axp20x: Switch to the sys-off handler API

This removes a layer of indirection through pm_power_off() and allows
the PMIC handler to be used as a fallback when firmware power off fails.
This happens on boards like the Clockwork DevTerm R-01 where OpenSBI
does not know how to use the PMIC to power off the board.

Move the check for AXP288 to avoid registering a dummy handler.

Signed-off-by: Samuel Holland <[email protected]>
---
Note: this patch depends on updating the PSCI driver first[1] to keep
the existing callback priority.

[1]: https://lore.kernel.org/lkml/[email protected]/

drivers/mfd/axp20x.c | 26 +++++++++++---------------
1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
index 47fd700f284f..58f82f1f4b43 100644
--- a/drivers/mfd/axp20x.c
+++ b/drivers/mfd/axp20x.c
@@ -23,7 +23,7 @@
#include <linux/mfd/core.h>
#include <linux/module.h>
#include <linux/of_device.h>
-#include <linux/pm_runtime.h>
+#include <linux/reboot.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>

@@ -825,17 +825,17 @@ static const struct mfd_cell axp813_cells[] = {
},
};

-static struct axp20x_dev *axp20x_pm_power_off;
-static void axp20x_power_off(void)
+static int axp20x_power_off(struct sys_off_data *data)
{
- if (axp20x_pm_power_off->variant == AXP288_ID)
- return;
+ struct axp20x_dev *axp20x = data->cb_data;

- regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL,
+ regmap_write(axp20x->regmap, AXP20X_OFF_CTRL,
AXP20X_OFF);

/* Give capacitors etc. time to drain to avoid kernel panic msg. */
mdelay(500);
+
+ return NOTIFY_DONE;
}

int axp20x_match_device(struct axp20x_dev *axp20x)
@@ -1002,10 +1002,11 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
return ret;
}

- if (!pm_power_off) {
- axp20x_pm_power_off = axp20x;
- pm_power_off = axp20x_power_off;
- }
+ if (axp20x->variant != AXP288_ID)
+ devm_register_sys_off_handler(axp20x->dev,
+ SYS_OFF_MODE_POWER_OFF,
+ SYS_OFF_PRIO_DEFAULT,
+ axp20x_power_off, axp20x);

dev_info(axp20x->dev, "AXP20X driver loaded\n");

@@ -1015,11 +1016,6 @@ EXPORT_SYMBOL(axp20x_device_probe);

void axp20x_device_remove(struct axp20x_dev *axp20x)
{
- if (axp20x == axp20x_pm_power_off) {
- axp20x_pm_power_off = NULL;
- pm_power_off = NULL;
- }
-
mfd_remove_devices(axp20x->dev);
regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
}
--
2.37.4

2023-01-09 17:28:58

by Lee Jones

Subject: Re: [PATCH] mfd: axp20x: Switch to the sys-off handler API

On Wed, 28 Dec 2022, Samuel Holland wrote:

> This removes a layer of indirection through pm_power_off() and allows
> the PMIC handler to be used as a fallback when firmware power off fails.
> This happens on boards like the Clockwork DevTerm R-01 where OpenSBI
> does not know how to use the PMIC to power off the board.
>
> Move the check for AXP288 to avoid registering a dummy handler.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
> Note: this patch depends on updating the PSCI driver first[1] to keep
> the existing callback priority.
>
> [1]: https://lore.kernel.org/lkml/[email protected]/
>
> drivers/mfd/axp20x.c | 26 +++++++++++---------------
> 1 file changed, 11 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> index 47fd700f284f..58f82f1f4b43 100644
> --- a/drivers/mfd/axp20x.c
> +++ b/drivers/mfd/axp20x.c
> @@ -23,7 +23,7 @@
> #include <linux/mfd/core.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
> -#include <linux/pm_runtime.h>
> +#include <linux/reboot.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
>
> @@ -825,17 +825,17 @@ static const struct mfd_cell axp813_cells[] = {
> },
> };
>
> -static struct axp20x_dev *axp20x_pm_power_off;
> -static void axp20x_power_off(void)
> +static int axp20x_power_off(struct sys_off_data *data)
> {
> - if (axp20x_pm_power_off->variant == AXP288_ID)
> - return;
> + struct axp20x_dev *axp20x = data->cb_data;
>
> - regmap_write(axp20x_pm_power_off->regmap, AXP20X_OFF_CTRL,
> + regmap_write(axp20x->regmap, AXP20X_OFF_CTRL,
> AXP20X_OFF);

Nit: Can you pop this on the line above now, thanks.

In fact, forget it, I'll do it.

> /* Give capacitors etc. time to drain to avoid kernel panic msg. */
> mdelay(500);
> +
> + return NOTIFY_DONE;
> }
>
> int axp20x_match_device(struct axp20x_dev *axp20x)
> @@ -1002,10 +1002,11 @@ int axp20x_device_probe(struct axp20x_dev *axp20x)
> return ret;
> }
>
> - if (!pm_power_off) {
> - axp20x_pm_power_off = axp20x;
> - pm_power_off = axp20x_power_off;
> - }
> + if (axp20x->variant != AXP288_ID)
> + devm_register_sys_off_handler(axp20x->dev,
> + SYS_OFF_MODE_POWER_OFF,
> + SYS_OFF_PRIO_DEFAULT,
> + axp20x_power_off, axp20x);
>
> dev_info(axp20x->dev, "AXP20X driver loaded\n");
>
> @@ -1015,11 +1016,6 @@ EXPORT_SYMBOL(axp20x_device_probe);
>
> void axp20x_device_remove(struct axp20x_dev *axp20x)
> {
> - if (axp20x == axp20x_pm_power_off) {
> - axp20x_pm_power_off = NULL;
> - pm_power_off = NULL;
> - }
> -
> mfd_remove_devices(axp20x->dev);
> regmap_del_irq_chip(axp20x->irq, axp20x->regmap_irqc);
> }

Applied, thanks

--
Lee Jones [李琼斯]