Problem
-------
SoC devices require power-off call chaining functionality from kernel.
We have a widely used restart chaining provided by restart notifier API,
but nothing for power-off.
Solution
--------
Introduce new API that provides both restart and power-off call chains.
Why combine restart with power-off? Because drivers often do both.
More practical to have API that provides both under the same roof.
The new API is designed with simplicity and extensibility in mind.
It's built upon the existing restart and reboot APIs. The simplicity
is in new helper functions that are convenient for drivers. The
extensibility is in the design that doesn't hardcode callback
arguments, making easy to add new parameters and remove old.
This is a third attempt to introduce the new API. First was made by
Guenter Roeck back in 2014, second was made by Thierry Reding in 2017.
In fact the work didn't stop and recently arm_pm_restart() was removed
from v5.14 kernel, which was a part of preparatory work started by
Guenter Roeck. I took into account experience and ideas from the
previous attempts, extended and polished them.
Adoption plan
-------------
This patchset introduces the new API. It also converts multiple drivers
and arch code to the new API to demonstrate how it all looks in practice.
The plan is:
1. Merge new API (patches 1-8). This API will co-exist with the old APIs.
2. Convert arch code to do_kernel_power_off() (patches 9-21).
3. Convert drivers and platform code to the new API.
4. Remove obsolete pm_power_off and pm_power_off_prepare variables.
5. Make restart-notifier API private to kernel/reboot.c once no users left.
Results
-------
1. Devices can be powered off properly.
2. Global variables are removed from drivers.
3. Global pm_power_off and pm_power_off_prepare callback variables are
removed once all users are converted to the new API. The latter callback
is removed by patch #25 of this series.
4. Ambiguous call chain ordering is prohibited. See patch #5 which adds
verification of restart handlers priorities, ensuring that they are unique.
Changelog:
v2: - Replaced standalone power-off call chain demo-API with the combined
power-off+restart API because this is what drivers want. It's a more
comprehensive solution.
- Converted multiple drivers and arch code to the new API. Suggested by
Andy Shevchenko. I skimmed through the rest of drivers, verifying that
new API suits them. The rest of the drivers will be converted once we
will settle on the new API, otherwise will be too many patches here.
- v2 API doesn't expose notifier to users and require handlers to
have unique priority. Suggested by Guenter Roeck.
- v2 API has power-off chaining disabled by default and require
drivers to explicitly opt-in to the chaining. This preserves old
behaviour for existing drivers once they are converted to the new
API.
Dmitry Osipenko (45):
notifier: Remove extern annotation from function prototypes
notifier: Add blocking_notifier_call_chain_empty()
notifier: Add atomic/blocking_notifier_has_unique_priority()
reboot: Correct typo in a comment
reboot: Warn if restart handler has duplicated priority
reboot: Warn if unregister_restart_handler() fails
reboot: Remove extern annotation from function prototypes
kernel: Add combined power-off+restart handler call chain API
xen/x86: Use do_kernel_power_off()
ARM: Use do_kernel_power_off()
arm64: Use do_kernel_power_off()
csky: Use do_kernel_power_off()
ia64: Use do_kernel_power_off()
mips: Use do_kernel_power_off()
nds32: Use do_kernel_power_off()
parisc: Use do_kernel_power_off()
powerpc: Use do_kernel_power_off()
riscv: Use do_kernel_power_off()
sh: Use do_kernel_power_off()
x86: Use do_kernel_power_off()
m68k: Switch to new power-handler API
memory: emif: Use kernel_can_power_off()
ACPI: power: Switch to power-handler API
regulator: pfuze100: Use devm_register_power_handler()
reboot: Remove pm_power_off_prepare()
soc/tegra: pmc: Utilize power-handler API to power off Nexus 7
properly
mfd: ntxec: Use devm_register_power_handler()
mfd: rn5t618: Use devm_register_power_handler()
mfd: acer-a500: Use devm_register_power_handler()
mfd: ene-kb3930: Use devm_register_power_handler()
mfd: axp20x: Use register_simple_power_off_handler()
mfd: retu: Use devm_register_simple_power_off_handler()
mfd: rk808: Use devm_register_simple_power_off_handler()
mfd: palmas: Use devm_register_simple_power_off_handler()
mfd: max8907: Use devm_register_simple_power_off_handler()
mfd: tps6586x: Use devm_register_simple_power_off_handler()
mfd: tps65910: Use devm_register_simple_power_off_handler()
mfd: max77620: Use devm_register_simple_power_off_handler()
mfd: dm355evm_msp: Use devm_register_trivial_power_off_handler()
mfd: twl4030: Use devm_register_trivial_power_off_handler()
mfd: ab8500: Use devm_register_trivial_power_off_handler()
reset: ath79: Use devm_register_simple_restart_handler()
reset: intel-gw: Use devm_register_simple_restart_handler()
reset: lpc18xx: Use devm_register_prioritized_restart_handler()
reset: npcm: Use devm_register_prioritized_restart_handler()
arch/arm/kernel/reboot.c | 4 +-
arch/arm64/kernel/process.c | 3 +-
arch/csky/kernel/power.c | 6 +-
arch/ia64/kernel/process.c | 4 +-
arch/m68k/emu/natfeat.c | 3 +-
arch/m68k/include/asm/machdep.h | 1 -
arch/m68k/kernel/process.c | 5 +-
arch/m68k/kernel/setup_mm.c | 1 -
arch/m68k/kernel/setup_no.c | 1 -
arch/m68k/mac/config.c | 4 +-
arch/mips/kernel/reset.c | 3 +-
arch/nds32/kernel/process.c | 3 +-
arch/parisc/kernel/process.c | 4 +-
arch/powerpc/kernel/setup-common.c | 4 +-
arch/powerpc/xmon/xmon.c | 3 +-
arch/riscv/kernel/reset.c | 12 +-
arch/sh/kernel/reboot.c | 3 +-
arch/x86/kernel/reboot.c | 4 +-
arch/x86/xen/enlighten_pv.c | 4 +-
drivers/acpi/sleep.c | 25 +-
drivers/memory/emif.c | 2 +-
drivers/mfd/ab8500-sysctrl.c | 17 +-
drivers/mfd/acer-ec-a500.c | 52 +--
drivers/mfd/axp20x.c | 22 +-
drivers/mfd/dm355evm_msp.c | 20 +-
drivers/mfd/ene-kb3930.c | 45 +-
drivers/mfd/max77620.c | 21 +-
drivers/mfd/max8907.c | 22 +-
drivers/mfd/ntxec.c | 50 +-
drivers/mfd/palmas.c | 24 +-
drivers/mfd/retu-mfd.c | 31 +-
drivers/mfd/rk808.c | 23 +-
drivers/mfd/rn5t618.c | 56 +--
drivers/mfd/tps6586x.c | 21 +-
drivers/mfd/tps65910.c | 17 +-
drivers/mfd/twl4030-power.c | 10 +-
drivers/regulator/pfuze100-regulator.c | 39 +-
drivers/reset/reset-ath79.c | 15 +-
drivers/reset/reset-intel-gw.c | 13 +-
drivers/reset/reset-lpc18xx.c | 14 +-
drivers/reset/reset-npcm.c | 14 +-
drivers/soc/tegra/pmc.c | 54 ++-
include/linux/mfd/axp20x.h | 1 +
include/linux/notifier.h | 37 +-
include/linux/pm.h | 1 -
include/linux/reboot.h | 216 ++++++++-
kernel/notifier.c | 88 ++++
kernel/power/hibernate.c | 2 +-
kernel/reboot.c | 615 ++++++++++++++++++++++++-
49 files changed, 1209 insertions(+), 430 deletions(-)
--
2.33.1
Kernel now supports chained power-off handlers. Use do_kernel_power_off()
that invokes chained power-off handlers. It also invokes legacy
pm_power_off() for now, which will be removed once all drivers will
be converted to the new power-off API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/x86/xen/enlighten_pv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4f63117f09bb..4a0b9b7baf02 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -32,6 +32,7 @@
#include <linux/gfp.h>
#include <linux/edd.h>
#include <linux/objtool.h>
+#include <linux/reboot.h>
#include <xen/xen.h>
#include <xen/events.h>
@@ -1087,8 +1088,7 @@ static void xen_machine_halt(void)
static void xen_machine_power_off(void)
{
- if (pm_power_off)
- pm_power_off();
+ do_kernel_power_off();
xen_reboot(SHUTDOWN_poweroff);
}
--
2.33.1
Kernel now supports chained power-off handlers. Use do_kernel_power_off()
that invokes chained power-off handlers. It also invokes legacy
pm_power_off() for now, which will be removed once all drivers will
be converted to the new power-off API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/csky/kernel/power.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/csky/kernel/power.c b/arch/csky/kernel/power.c
index 923ee4e381b8..86ee202906f8 100644
--- a/arch/csky/kernel/power.c
+++ b/arch/csky/kernel/power.c
@@ -9,16 +9,14 @@ EXPORT_SYMBOL(pm_power_off);
void machine_power_off(void)
{
local_irq_disable();
- if (pm_power_off)
- pm_power_off();
+ do_kernel_power_off();
asm volatile ("bkpt");
}
void machine_halt(void)
{
local_irq_disable();
- if (pm_power_off)
- pm_power_off();
+ do_kernel_power_off();
asm volatile ("bkpt");
}
--
2.33.1
Emit warning if unregister_restart_handler() fails since it never should
fail. This will ease further API development by catching mistakes early.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
kernel/reboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/reboot.c b/kernel/reboot.c
index d39e599c3c99..291d44082f42 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -210,7 +210,7 @@ EXPORT_SYMBOL(register_restart_handler);
*/
int unregister_restart_handler(struct notifier_block *nb)
{
- return atomic_notifier_chain_unregister(&restart_handler_list, nb);
+ return WARN_ON(atomic_notifier_chain_unregister(&restart_handler_list, nb));
}
EXPORT_SYMBOL(unregister_restart_handler);
--
2.33.1
Kernel now supports chained power-off handlers. Use do_kernel_power_off()
that invokes chained power-off handlers. It also invokes legacy
pm_power_off() for now, which will be removed once all drivers will
be converted to the new power-off API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/x86/kernel/reboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 0a40df66a40d..cd7d9416d81a 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -747,10 +747,10 @@ static void native_machine_halt(void)
static void native_machine_power_off(void)
{
- if (pm_power_off) {
+ if (kernel_can_power_off()) {
if (!reboot_force)
machine_shutdown();
- pm_power_off();
+ do_kernel_power_off();
}
/* A fallback in case there is no PM info available */
tboot_shutdown(TB_SHUTDOWN_HALT);
--
2.33.1
Kernel now supports chained power-off handlers. Use do_kernel_power_off()
that invokes chained power-off handlers. It also invokes legacy
pm_power_off() for now, which will be removed once all drivers will
be converted to the new power-off API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/nds32/kernel/process.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/nds32/kernel/process.c b/arch/nds32/kernel/process.c
index 49fab9e39cbf..0936dcd7db1b 100644
--- a/arch/nds32/kernel/process.c
+++ b/arch/nds32/kernel/process.c
@@ -54,8 +54,7 @@ EXPORT_SYMBOL(machine_halt);
void machine_power_off(void)
{
- if (pm_power_off)
- pm_power_off();
+ do_kernel_power_off();
}
EXPORT_SYMBOL(machine_power_off);
--
2.33.1
Nexus 7 Android tablet can be turned off using a special bootloader
command which is conveyed to bootloader by putting magic value into
specific scratch register and then rebooting normally. This power-off
method should be invoked if USB cable is connected. Bootloader then will
display battery status and power off the device. This behaviour is
borrowed from downstream kernel and matches user expectations, otherwise
it looks like device got hung during power-off and it may wake up on
USB disconnect.
Switch PMC driver to power-handler API, which provides drivers with
combined power-off+restart call chains functionality, replacing the
restart-only call chain API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/soc/tegra/pmc.c | 54 +++++++++++++++++++++++++++--------------
1 file changed, 36 insertions(+), 18 deletions(-)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 575d6d5b4294..a01330099e1a 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -39,6 +39,7 @@
#include <linux/platform_device.h>
#include <linux/pm_domain.h>
#include <linux/pm_opp.h>
+#include <linux/power_supply.h>
#include <linux/reboot.h>
#include <linux/regmap.h>
#include <linux/reset.h>
@@ -107,6 +108,7 @@
#define PMC_USB_DEBOUNCE_DEL 0xec
#define PMC_USB_AO 0xf0
+#define PMC_SCRATCH37 0x130
#define PMC_SCRATCH41 0x140
#define PMC_WAKE2_MASK 0x160
@@ -1064,10 +1066,8 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid)
return tegra_powergate_remove_clamping(id);
}
-static int tegra_pmc_restart_notify(struct notifier_block *this,
- unsigned long action, void *data)
+static void tegra_pmc_restart(const char *cmd)
{
- const char *cmd = data;
u32 value;
value = tegra_pmc_scratch_readl(pmc, pmc->soc->regs->scratch0);
@@ -1090,13 +1090,33 @@ static int tegra_pmc_restart_notify(struct notifier_block *this,
value = tegra_pmc_readl(pmc, PMC_CNTRL);
value |= PMC_CNTRL_MAIN_RST;
tegra_pmc_writel(pmc, value, PMC_CNTRL);
+}
- return NOTIFY_DONE;
+static void tegra_pmc_restart_handler(struct restart_data *data)
+{
+ tegra_pmc_restart(data->cmd);
}
-static struct notifier_block tegra_pmc_restart_handler = {
- .notifier_call = tegra_pmc_restart_notify,
- .priority = 128,
+static void tegra_pmc_power_off_handler(struct power_off_data *data)
+{
+ /*
+ * Reboot Nexus 7 into special bootloader mode if USB cable is
+ * connected in order to display battery status and power off.
+ */
+ if (of_machine_is_compatible("asus,grouper") &&
+ power_supply_is_system_supplied()) {
+ const u32 go_to_charger_mode = 0xa5a55a5a;
+
+ tegra_pmc_writel(pmc, go_to_charger_mode, PMC_SCRATCH37);
+ tegra_pmc_restart(NULL);
+ }
+}
+
+static struct power_handler tegra_pmc_power_handler = {
+ .restart_cb = tegra_pmc_restart_handler,
+ .power_off_cb = tegra_pmc_power_off_handler,
+ .power_off_priority = POWEROFF_PRIO_FIRMWARE,
+ .power_off_chaining_allowed = true,
};
static int powergate_show(struct seq_file *s, void *data)
@@ -2859,6 +2879,13 @@ static int tegra_pmc_probe(struct platform_device *pdev)
pmc->clk = NULL;
}
+ err = devm_register_power_handler(&pdev->dev, &tegra_pmc_power_handler);
+ if (err) {
+ dev_err(&pdev->dev, "unable to register power handler, %d\n",
+ err);
+ return err;
+ }
+
/*
* PCLK clock rate can't be retrieved using CLK API because it
* causes lockup if CPU enters LP2 idle state from some other
@@ -2890,20 +2917,13 @@ static int tegra_pmc_probe(struct platform_device *pdev)
goto cleanup_sysfs;
}
- err = register_restart_handler(&tegra_pmc_restart_handler);
- if (err) {
- dev_err(&pdev->dev, "unable to register restart handler, %d\n",
- err);
- goto cleanup_debugfs;
- }
-
err = tegra_pmc_pinctrl_init(pmc);
if (err)
- goto cleanup_restart_handler;
+ goto cleanup_debugfs;
err = tegra_pmc_regmap_init(pmc);
if (err < 0)
- goto cleanup_restart_handler;
+ goto cleanup_debugfs;
err = tegra_powergate_init(pmc, pdev->dev.of_node);
if (err < 0)
@@ -2926,8 +2946,6 @@ static int tegra_pmc_probe(struct platform_device *pdev)
cleanup_powergates:
tegra_powergate_remove_all(pdev->dev.of_node);
-cleanup_restart_handler:
- unregister_restart_handler(&tegra_pmc_restart_handler);
cleanup_debugfs:
debugfs_remove(pmc->debugfs);
cleanup_sysfs:
--
2.33.1
Use devm_register_power_handler() that replaces global pm_power_off
variable and allows to register multiple power-off handlers. It also
provides restart-handler support, i.e. all in one API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/ene-kb3930.c | 45 ++++++++++++++--------------------------
1 file changed, 15 insertions(+), 30 deletions(-)
diff --git a/drivers/mfd/ene-kb3930.c b/drivers/mfd/ene-kb3930.c
index 1b73318d1f1f..6a3c5f48e5e1 100644
--- a/drivers/mfd/ene-kb3930.c
+++ b/drivers/mfd/ene-kb3930.c
@@ -31,10 +31,9 @@ struct kb3930 {
struct i2c_client *client;
struct regmap *ram_regmap;
struct gpio_descs *off_gpios;
+ struct power_handler power_handler;
};
-static struct kb3930 *kb3930_power_off;
-
#define EC_GPIO_WAVE 0
#define EC_GPIO_OFF_MODE 1
@@ -60,21 +59,19 @@ static void kb3930_off(struct kb3930 *ddata, int off_mode)
}
}
-static int kb3930_restart(struct notifier_block *this,
- unsigned long mode, void *cmd)
+static void kb3930_restart(struct restart_data *data)
{
- kb3930_off(kb3930_power_off, EC_OFF_MODE_REBOOT);
- return NOTIFY_DONE;
+ struct kb3930 *ddata = data->cb_data;
+
+ kb3930_off(ddata, EC_OFF_MODE_REBOOT);
}
-static void kb3930_pm_power_off(void)
+static void kb3930_power_off(struct power_off_data *data)
{
- kb3930_off(kb3930_power_off, EC_OFF_MODE_POWER);
-}
+ struct kb3930 *ddata = data->cb_data;
-static struct notifier_block kb3930_restart_nb = {
- .notifier_call = kb3930_restart,
-};
+ kb3930_off(ddata, EC_OFF_MODE_POWER);
+}
static const struct mfd_cell ariel_ec_cells[] = {
{ .name = "dell-wyse-ariel-led", },
@@ -131,7 +128,6 @@ static int kb3930_probe(struct i2c_client *client)
if (!ddata)
return -ENOMEM;
- kb3930_power_off = ddata;
ddata->client = client;
i2c_set_clientdata(client, ddata);
@@ -169,24 +165,14 @@ static int kb3930_probe(struct i2c_client *client)
}
if (ddata->off_gpios) {
- register_restart_handler(&kb3930_restart_nb);
- if (!pm_power_off)
- pm_power_off = kb3930_pm_power_off;
- }
+ ddata->power_handler.cb_data = ddata;
+ ddata->power_handler.restart_cb = kb3930_restart;
+ ddata->power_handler.power_off_cb = kb3930_power_off;
- return 0;
-}
-
-static int kb3930_remove(struct i2c_client *client)
-{
- struct kb3930 *ddata = i2c_get_clientdata(client);
-
- if (ddata->off_gpios) {
- if (pm_power_off == kb3930_pm_power_off)
- pm_power_off = NULL;
- unregister_restart_handler(&kb3930_restart_nb);
+ ret = devm_register_power_handler(dev, &ddata->power_handler);
+ if (ret)
+ return ret;
}
- kb3930_power_off = NULL;
return 0;
}
@@ -199,7 +185,6 @@ MODULE_DEVICE_TABLE(of, kb3930_dt_ids);
static struct i2c_driver kb3930_driver = {
.probe_new = kb3930_probe,
- .remove = kb3930_remove,
.driver = {
.name = "ene-kb3930",
.of_match_table = kb3930_dt_ids,
--
2.33.1
Use devm_register_power_handler() that replaces global pm_power_off
variable and allows to register multiple power-off handlers. It also
provides restart-handler support, i.e. all in one API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/ntxec.c | 50 ++++++++++-----------------------------------
1 file changed, 11 insertions(+), 39 deletions(-)
diff --git a/drivers/mfd/ntxec.c b/drivers/mfd/ntxec.c
index b711e73eedcb..fd6410cbe153 100644
--- a/drivers/mfd/ntxec.c
+++ b/drivers/mfd/ntxec.c
@@ -32,12 +32,11 @@
#define NTXEC_POWERKEEP_VALUE 0x0800
#define NTXEC_RESET_VALUE 0xff00
-static struct i2c_client *poweroff_restart_client;
-
-static void ntxec_poweroff(void)
+static void ntxec_poweroff(struct power_off_data *data)
{
int res;
u8 buf[3] = { NTXEC_REG_POWEROFF };
+ struct i2c_client *poweroff_restart_client = data->cb_data;
struct i2c_msg msgs[] = {
{
.addr = poweroff_restart_client->addr,
@@ -62,8 +61,7 @@ static void ntxec_poweroff(void)
msleep(5000);
}
-static int ntxec_restart(struct notifier_block *nb,
- unsigned long action, void *data)
+static void ntxec_restart(struct restart_data *data)
{
int res;
u8 buf[3] = { NTXEC_REG_RESET };
@@ -72,6 +70,7 @@ static int ntxec_restart(struct notifier_block *nb,
* it causes an I2C error. (The reset handler in the downstream driver
* does send the full two-byte value, but doesn't check the result).
*/
+ struct i2c_client *poweroff_restart_client = data->cb_data;
struct i2c_msg msgs[] = {
{
.addr = poweroff_restart_client->addr,
@@ -87,13 +86,11 @@ static int ntxec_restart(struct notifier_block *nb,
if (res < 0)
dev_warn(&poweroff_restart_client->dev,
"Failed to restart (err = %d)\n", res);
-
- return NOTIFY_DONE;
}
-static struct notifier_block ntxec_restart_handler = {
- .notifier_call = ntxec_restart,
- .priority = 128,
+static struct power_handler ntxec_power_handler = {
+ .restart_cb = ntxec_restart,
+ .power_off_cb = ntxec_poweroff,
};
static int regmap_ignore_write(void *context,
@@ -208,25 +205,12 @@ static int ntxec_probe(struct i2c_client *client)
if (res < 0)
return res;
- if (poweroff_restart_client)
- /*
- * Another instance of the driver already took
- * poweroff/restart duties.
- */
- dev_err(ec->dev, "poweroff_restart_client already assigned\n");
- else
- poweroff_restart_client = client;
-
- if (pm_power_off)
- /* Another driver already registered a poweroff handler. */
- dev_err(ec->dev, "pm_power_off already assigned\n");
- else
- pm_power_off = ntxec_poweroff;
-
- res = register_restart_handler(&ntxec_restart_handler);
+ ntxec_power_handler.cb_data = client;
+
+ res = devm_register_power_handler(ec->dev, &ntxec_power_handler);
if (res)
dev_err(ec->dev,
- "Failed to register restart handler: %d\n", res);
+ "Failed to register power handler: %d\n", res);
}
i2c_set_clientdata(client, ec);
@@ -239,17 +223,6 @@ static int ntxec_probe(struct i2c_client *client)
return res;
}
-static int ntxec_remove(struct i2c_client *client)
-{
- if (client == poweroff_restart_client) {
- poweroff_restart_client = NULL;
- pm_power_off = NULL;
- unregister_restart_handler(&ntxec_restart_handler);
- }
-
- return 0;
-}
-
static const struct of_device_id of_ntxec_match_table[] = {
{ .compatible = "netronix,ntxec", },
{}
@@ -262,7 +235,6 @@ static struct i2c_driver ntxec_driver = {
.of_match_table = of_ntxec_match_table,
},
.probe_new = ntxec_probe,
- .remove = ntxec_remove,
};
module_i2c_driver(ntxec_driver);
--
2.33.1
Kernel now supports chained power-off handlers. Use
register_power_off_handler() that registers power-off handlers and
do_kernel_power_off() that invokes chained power-off handlers. Legacy
pm_power_off() will be removed once all drivers will be converted to
the new power-off API.
Normally arch code should adopt only the do_kernel_power_off() at first,
but m68k is a special case because it uses pm_power_off() "inside out",
i.e. pm_power_off() invokes machine_power_off() [in fact it does nothing],
while it's machine_power_off() that should invoke the pm_power_off(), and
thus, we can't convert platforms to the new API separately. There are only
two platforms changed here, so it's not a big deal.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/m68k/emu/natfeat.c | 3 ++-
arch/m68k/include/asm/machdep.h | 1 -
arch/m68k/kernel/process.c | 5 ++---
arch/m68k/kernel/setup_mm.c | 1 -
arch/m68k/kernel/setup_no.c | 1 -
arch/m68k/mac/config.c | 4 +++-
6 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/m68k/emu/natfeat.c b/arch/m68k/emu/natfeat.c
index 71b78ecee75c..b19dc00026d9 100644
--- a/arch/m68k/emu/natfeat.c
+++ b/arch/m68k/emu/natfeat.c
@@ -15,6 +15,7 @@
#include <linux/string.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/reboot.h>
#include <linux/io.h>
#include <asm/machdep.h>
#include <asm/natfeat.h>
@@ -90,5 +91,5 @@ void __init nf_init(void)
pr_info("NatFeats found (%s, %lu.%lu)\n", buf, version >> 16,
version & 0xffff);
- mach_power_off = nf_poweroff;
+ register_platform_power_off(nf_poweroff);
}
diff --git a/arch/m68k/include/asm/machdep.h b/arch/m68k/include/asm/machdep.h
index 8fd80ef1b77e..8d8c3ee2069f 100644
--- a/arch/m68k/include/asm/machdep.h
+++ b/arch/m68k/include/asm/machdep.h
@@ -24,7 +24,6 @@ extern int (*mach_get_rtc_pll)(struct rtc_pll_info *);
extern int (*mach_set_rtc_pll)(struct rtc_pll_info *);
extern void (*mach_reset)( void );
extern void (*mach_halt)( void );
-extern void (*mach_power_off)( void );
extern unsigned long (*mach_hd_init) (unsigned long, unsigned long);
extern void (*mach_hd_setup)(char *, int *);
extern void (*mach_heartbeat) (int);
diff --git a/arch/m68k/kernel/process.c b/arch/m68k/kernel/process.c
index a6030dbaa089..e160a7c57bd3 100644
--- a/arch/m68k/kernel/process.c
+++ b/arch/m68k/kernel/process.c
@@ -67,12 +67,11 @@ void machine_halt(void)
void machine_power_off(void)
{
- if (mach_power_off)
- mach_power_off();
+ do_kernel_power_off();
for (;;);
}
-void (*pm_power_off)(void) = machine_power_off;
+void (*pm_power_off)(void);
EXPORT_SYMBOL(pm_power_off);
void show_regs(struct pt_regs * regs)
diff --git a/arch/m68k/kernel/setup_mm.c b/arch/m68k/kernel/setup_mm.c
index 4b51bfd38e5f..50f4f120a4ff 100644
--- a/arch/m68k/kernel/setup_mm.c
+++ b/arch/m68k/kernel/setup_mm.c
@@ -98,7 +98,6 @@ EXPORT_SYMBOL(mach_get_rtc_pll);
EXPORT_SYMBOL(mach_set_rtc_pll);
void (*mach_reset)( void );
void (*mach_halt)( void );
-void (*mach_power_off)( void );
#ifdef CONFIG_HEARTBEAT
void (*mach_heartbeat) (int);
EXPORT_SYMBOL(mach_heartbeat);
diff --git a/arch/m68k/kernel/setup_no.c b/arch/m68k/kernel/setup_no.c
index 5e4104f07a44..00bf82258233 100644
--- a/arch/m68k/kernel/setup_no.c
+++ b/arch/m68k/kernel/setup_no.c
@@ -55,7 +55,6 @@ int (*mach_hwclk) (int, struct rtc_time*);
/* machine dependent reboot functions */
void (*mach_reset)(void);
void (*mach_halt)(void);
-void (*mach_power_off)(void);
#ifdef CONFIG_M68000
#if defined(CONFIG_M68328)
diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index 5d16f9b47aa9..727320dedf08 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -12,6 +12,7 @@
#include <linux/errno.h>
#include <linux/module.h>
+#include <linux/reboot.h>
#include <linux/types.h>
#include <linux/mm.h>
#include <linux/tty.h>
@@ -139,7 +140,6 @@ void __init config_mac(void)
mach_hwclk = mac_hwclk;
mach_reset = mac_reset;
mach_halt = mac_poweroff;
- mach_power_off = mac_poweroff;
#if IS_ENABLED(CONFIG_INPUT_M68K_BEEP)
mach_beep = mac_mksound;
#endif
@@ -159,6 +159,8 @@ void __init config_mac(void)
if (macintosh_config->ident == MAC_MODEL_IICI)
mach_l2_flush = via_l2_flush;
+
+ register_platform_power_off(mac_poweroff);
}
--
2.33.1
Use devm_register_power_handler() that replaces global pm_power_off
variable and allows to register multiple power-off handlers. It also
provides restart-handler support, i.e. all in one API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/acer-ec-a500.c | 52 ++++++++++++++------------------------
1 file changed, 19 insertions(+), 33 deletions(-)
diff --git a/drivers/mfd/acer-ec-a500.c b/drivers/mfd/acer-ec-a500.c
index 80c2fdd14fc4..fc864abc0049 100644
--- a/drivers/mfd/acer-ec-a500.c
+++ b/drivers/mfd/acer-ec-a500.c
@@ -31,8 +31,6 @@ enum {
REG_COLD_REBOOT = 0x55,
};
-static struct i2c_client *a500_ec_client_pm_off;
-
static int a500_ec_read(void *context, const void *reg_buf, size_t reg_size,
void *val_buf, size_t val_sizel)
{
@@ -104,32 +102,35 @@ static const struct regmap_bus a500_ec_regmap_bus = {
.max_raw_read = 2,
};
-static void a500_ec_poweroff(void)
+static void a500_ec_power_off_handler(struct power_off_data *data)
{
- i2c_smbus_write_word_data(a500_ec_client_pm_off,
- REG_SHUTDOWN, CMD_SHUTDOWN);
+ struct i2c_client *client = data->cb_data;
+
+ i2c_smbus_write_word_data(client, REG_SHUTDOWN, CMD_SHUTDOWN);
mdelay(A500_EC_POWER_CMD_TIMEOUT);
}
-static int a500_ec_restart_notify(struct notifier_block *this,
- unsigned long reboot_mode, void *data)
+static void a500_ec_restart_handler(struct restart_data *data)
{
- if (reboot_mode == REBOOT_WARM)
- i2c_smbus_write_word_data(a500_ec_client_pm_off,
+ struct i2c_client *client = data->cb_data;
+
+ if (data->mode == REBOOT_WARM)
+ i2c_smbus_write_word_data(client,
REG_WARM_REBOOT, CMD_WARM_REBOOT);
else
- i2c_smbus_write_word_data(a500_ec_client_pm_off,
+ i2c_smbus_write_word_data(client,
REG_COLD_REBOOT, CMD_COLD_REBOOT);
mdelay(A500_EC_POWER_CMD_TIMEOUT);
-
- return NOTIFY_DONE;
}
-static struct notifier_block a500_ec_restart_handler = {
- .notifier_call = a500_ec_restart_notify,
- .priority = 200,
+static struct power_handler a500_ec_power_handler = {
+ .restart_cb = a500_ec_restart_handler,
+ .restart_priority = RESTART_PRIO_HIGH,
+
+ .power_off_cb = a500_ec_power_off_handler,
+ .power_off_priority = POWEROFF_PRIO_HIGH,
};
static const struct mfd_cell a500_ec_cells[] = {
@@ -156,26 +157,12 @@ static int a500_ec_probe(struct i2c_client *client)
}
if (of_device_is_system_power_controller(client->dev.of_node)) {
- a500_ec_client_pm_off = client;
+ a500_ec_power_handler.cb_data = client;
- err = register_restart_handler(&a500_ec_restart_handler);
+ err = devm_register_power_handler(&client->dev,
+ &a500_ec_power_handler);
if (err)
return err;
-
- if (!pm_power_off)
- pm_power_off = a500_ec_poweroff;
- }
-
- return 0;
-}
-
-static int a500_ec_remove(struct i2c_client *client)
-{
- if (of_device_is_system_power_controller(client->dev.of_node)) {
- if (pm_power_off == a500_ec_poweroff)
- pm_power_off = NULL;
-
- unregister_restart_handler(&a500_ec_restart_handler);
}
return 0;
@@ -193,7 +180,6 @@ static struct i2c_driver a500_ec_driver = {
.of_match_table = a500_ec_match,
},
.probe_new = a500_ec_probe,
- .remove = a500_ec_remove,
};
module_i2c_driver(a500_ec_driver);
--
2.33.1
Use devm_register_power_handler() that replaces global pm_power_off
variable and allows to register multiple power-off handlers. It also
provides restart-handler support, i.e. all in one API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/rn5t618.c | 56 ++++++++++++++++---------------------------
1 file changed, 21 insertions(+), 35 deletions(-)
diff --git a/drivers/mfd/rn5t618.c b/drivers/mfd/rn5t618.c
index 384acb459427..12d7b2339bbe 100644
--- a/drivers/mfd/rn5t618.c
+++ b/drivers/mfd/rn5t618.c
@@ -84,9 +84,6 @@ static const struct regmap_irq_chip rc5t619_irq_chip = {
.mask_invert = true,
};
-static struct i2c_client *rn5t618_pm_power_off;
-static struct notifier_block rn5t618_restart_handler;
-
static int rn5t618_irq_init(struct rn5t618 *rn5t618)
{
const struct regmap_irq_chip *irq_chip = NULL;
@@ -115,7 +112,9 @@ static int rn5t618_irq_init(struct rn5t618 *rn5t618)
return ret;
}
-static void rn5t618_trigger_poweroff_sequence(bool repower)
+static void
+rn5t618_trigger_poweroff_sequence(struct i2c_client *rn5t618_pm_power_off,
+ bool repower)
{
int ret;
@@ -151,25 +150,31 @@ static void rn5t618_trigger_poweroff_sequence(bool repower)
dev_alert(&rn5t618_pm_power_off->dev, "Failed to shutdown (err = %d)\n", ret);
}
-static void rn5t618_power_off(void)
+static void rn5t618_power_off(struct power_off_data *data)
{
- rn5t618_trigger_poweroff_sequence(false);
+ struct i2c_client *client = data->cb_data;
+
+ rn5t618_trigger_poweroff_sequence(client, false);
}
-static int rn5t618_restart(struct notifier_block *this,
- unsigned long mode, void *cmd)
+static void rn5t618_restart(struct restart_data *data)
{
- rn5t618_trigger_poweroff_sequence(true);
+ struct i2c_client *client = data->cb_data;
+
+ rn5t618_trigger_poweroff_sequence(client, true);
/*
* Re-power factor detection on PMIC side is not instant. 1ms
* proved to be enough time until reset takes effect.
*/
mdelay(1);
-
- return NOTIFY_DONE;
}
+static struct power_handler rn5t618_power_handler = {
+ .restart_cb = rn5t618_restart,
+ .restart_priority = RESTART_PRIO_HIGH,
+};
+
static const struct of_device_id rn5t618_of_match[] = {
{ .compatible = "ricoh,rn5t567", .data = (void *)RN5T567 },
{ .compatible = "ricoh,rn5t618", .data = (void *)RN5T618 },
@@ -221,38 +226,20 @@ static int rn5t618_i2c_probe(struct i2c_client *i2c)
return ret;
}
- rn5t618_pm_power_off = i2c;
- if (of_device_is_system_power_controller(i2c->dev.of_node)) {
- if (!pm_power_off)
- pm_power_off = rn5t618_power_off;
- else
- dev_warn(&i2c->dev, "Poweroff callback already assigned\n");
- }
+ if (of_device_is_system_power_controller(i2c->dev.of_node))
+ rn5t618_power_handler.power_off_cb = rn5t618_power_off;
- rn5t618_restart_handler.notifier_call = rn5t618_restart;
- rn5t618_restart_handler.priority = 192;
+ rn5t618_power_handler.cb_data = i2c;
- ret = register_restart_handler(&rn5t618_restart_handler);
+ ret = devm_register_power_handler(&i2c->dev, &rn5t618_power_handler);
if (ret) {
- dev_err(&i2c->dev, "cannot register restart handler, %d\n", ret);
+ dev_err(&i2c->dev, "failed to register power handler: %d\n", ret);
return ret;
}
return rn5t618_irq_init(priv);
}
-static int rn5t618_i2c_remove(struct i2c_client *i2c)
-{
- if (i2c == rn5t618_pm_power_off) {
- rn5t618_pm_power_off = NULL;
- pm_power_off = NULL;
- }
-
- unregister_restart_handler(&rn5t618_restart_handler);
-
- return 0;
-}
-
static int __maybe_unused rn5t618_i2c_suspend(struct device *dev)
{
struct rn5t618 *priv = dev_get_drvdata(dev);
@@ -284,7 +271,6 @@ static struct i2c_driver rn5t618_i2c_driver = {
.pm = &rn5t618_i2c_dev_pm_ops,
},
.probe_new = rn5t618_i2c_probe,
- .remove = rn5t618_i2c_remove,
};
module_i2c_driver(rn5t618_i2c_driver);
--
2.33.1
Kernel now supports chained power-off handlers. Use do_kernel_power_off()
that invokes chained power-off handlers. It also invokes legacy
pm_power_off() for now, which will be removed once all drivers will
be converted to the new power-off API.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/arm64/kernel/process.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index aacf2f5559a8..f8db031afa7d 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -110,8 +110,7 @@ void machine_power_off(void)
{
local_irq_disable();
smp_send_stop();
- if (pm_power_off)
- pm_power_off();
+ do_kernel_power_off();
}
/*
--
2.33.1
Use devm_register_simple_power_off_handler() that replaces global
pm_power_off variable and allows to register multiple power-off handlers.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/rk808.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index b181fe401330..3bf369469053 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -18,6 +18,7 @@
#include <linux/mfd/core.h>
#include <linux/module.h>
#include <linux/of_device.h>
+#include <linux/reboot.h>
#include <linux/regmap.h>
struct rk808_reg_data {
@@ -526,12 +527,11 @@ static const struct regmap_irq_chip rk818_irq_chip = {
.init_ack_masked = true,
};
-static struct i2c_client *rk808_i2c_client;
-
-static void rk808_pm_power_off(void)
+static void rk808_pm_power_off(void *data)
{
int ret;
unsigned int reg, bit;
+ struct i2c_client *rk808_i2c_client = data;
struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
switch (rk808->variant) {
@@ -725,8 +725,14 @@ static int rk808_probe(struct i2c_client *client,
}
if (of_property_read_bool(np, "rockchip,system-power-controller")) {
- rk808_i2c_client = client;
- pm_power_off = rk808_pm_power_off;
+ ret = devm_register_simple_power_off_handler(&client->dev,
+ rk808_pm_power_off,
+ client);
+ if (ret) {
+ dev_err(&client->dev,
+ "failed to register power-off handler %d\n", ret);
+ goto err_irq;
+ }
}
return 0;
@@ -742,13 +748,6 @@ static int rk808_remove(struct i2c_client *client)
regmap_del_irq_chip(client->irq, rk808->irq_data);
- /**
- * pm_power_off may points to a function from another module.
- * Check if the pointer is set by us and only then overwrite it.
- */
- if (pm_power_off == rk808_pm_power_off)
- pm_power_off = NULL;
-
return 0;
}
--
2.33.1
Use devm_register_simple_power_off_handler() that replaces global
pm_power_off variable and allows to register multiple power-off handlers.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/tps65910.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index 6e105cca27d4..8fab30dc84e5 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -16,6 +16,7 @@
#include <linux/irq.h>
#include <linux/irqdomain.h>
#include <linux/mfd/core.h>
+#include <linux/reboot.h>
#include <linux/regmap.h>
#include <linux/mfd/tps65910.h>
#include <linux/of.h>
@@ -429,9 +430,9 @@ struct tps65910_board *tps65910_parse_dt(struct i2c_client *client,
}
#endif
-static struct i2c_client *tps65910_i2c_client;
-static void tps65910_power_off(void)
+static void tps65910_power_off(void *data)
{
+ struct i2c_client *tps65910_i2c_client = data;
struct tps65910 *tps65910;
tps65910 = dev_get_drvdata(&tps65910_i2c_client->dev);
@@ -503,9 +504,15 @@ static int tps65910_i2c_probe(struct i2c_client *i2c,
tps65910_ck32k_init(tps65910, pmic_plat_data);
tps65910_sleepinit(tps65910, pmic_plat_data);
- if (pmic_plat_data->pm_off && !pm_power_off) {
- tps65910_i2c_client = i2c;
- pm_power_off = tps65910_power_off;
+ if (pmic_plat_data->pm_off) {
+ ret = devm_register_simple_power_off_handler(&i2c->dev,
+ tps65910_power_off,
+ i2c);
+ if (ret) {
+ dev_err(&i2c->dev,
+ "failed to register power-off handler: %d\n", ret);
+ return ret;
+ }
}
ret = devm_mfd_add_devices(tps65910->dev, -1,
--
2.33.1
Use devm_register_power_handler() that replaces global pm_power_off_prepare
variable and allows to register multiple power-off handlers.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/regulator/pfuze100-regulator.c | 39 ++++++++++----------------
1 file changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c
index d60d7d1b7fa2..73d5baf9d3ea 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -10,6 +10,7 @@
#include <linux/of_device.h>
#include <linux/regulator/of_regulator.h>
#include <linux/platform_device.h>
+#include <linux/reboot.h>
#include <linux/regulator/driver.h>
#include <linux/regulator/machine.h>
#include <linux/regulator/pfuze100.h>
@@ -76,6 +77,7 @@ struct pfuze_chip {
struct pfuze_regulator regulator_descs[PFUZE100_MAX_REGULATOR];
struct regulator_dev *regulators[PFUZE100_MAX_REGULATOR];
struct pfuze_regulator *pfuze_regulators;
+ struct power_handler power_handler;
};
static const int pfuze100_swbst[] = {
@@ -569,10 +571,10 @@ static inline struct device_node *match_of_node(int index)
return pfuze_matches[index].of_node;
}
-static struct pfuze_chip *syspm_pfuze_chip;
-
-static void pfuze_power_off_prepare(void)
+static void pfuze_power_off_prepare(struct power_off_prep_data *data)
{
+ struct pfuze_chip *syspm_pfuze_chip = data->cb_data;
+
dev_info(syspm_pfuze_chip->dev, "Configure standby mode for power off");
/* Switch from default mode: APS/APS to APS/Off */
@@ -611,24 +613,24 @@ static void pfuze_power_off_prepare(void)
static int pfuze_power_off_prepare_init(struct pfuze_chip *pfuze_chip)
{
+ int err;
+
if (pfuze_chip->chip_id != PFUZE100) {
dev_warn(pfuze_chip->dev, "Requested pm_power_off_prepare handler for not supported chip\n");
return -ENODEV;
}
- if (pm_power_off_prepare) {
- dev_warn(pfuze_chip->dev, "pm_power_off_prepare is already registered.\n");
- return -EBUSY;
- }
+ pfuze_chip->power_handler.power_off_prepare_cb = pfuze_power_off_prepare;
+ pfuze_chip->power_handler.cb_data = pfuze_chip;
- if (syspm_pfuze_chip) {
- dev_warn(pfuze_chip->dev, "syspm_pfuze_chip is already set.\n");
- return -EBUSY;
+ err = devm_register_power_handler(pfuze_chip->dev,
+ &pfuze_chip->power_handler);
+ if (err) {
+ dev_err(pfuze_chip->dev,
+ "failed to register power handler: %d\n", err);
+ return err;
}
- syspm_pfuze_chip = pfuze_chip;
- pm_power_off_prepare = pfuze_power_off_prepare;
-
return 0;
}
@@ -837,23 +839,12 @@ static int pfuze100_regulator_probe(struct i2c_client *client,
return 0;
}
-static int pfuze100_regulator_remove(struct i2c_client *client)
-{
- if (syspm_pfuze_chip) {
- syspm_pfuze_chip = NULL;
- pm_power_off_prepare = NULL;
- }
-
- return 0;
-}
-
static struct i2c_driver pfuze_driver = {
.driver = {
.name = "pfuze100-regulator",
.of_match_table = pfuze_dt_ids,
},
.probe = pfuze100_regulator_probe,
- .remove = pfuze100_regulator_remove,
};
module_i2c_driver(pfuze_driver);
--
2.33.1
Use devm_register_simple_power_off_handler() that replaces global
pm_power_off variable and allows to register multiple power-off handlers.
Nexus 7 Android tablet can be powered off using MAX77663 PMIC and using
a special bootloader command. At first the bootloader option should be
tried, it will have a higher priority than of PMIC that uses default
priority.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/max77620.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/max77620.c b/drivers/mfd/max77620.c
index fec2096474ad..29487ccc191a 100644
--- a/drivers/mfd/max77620.c
+++ b/drivers/mfd/max77620.c
@@ -31,11 +31,10 @@
#include <linux/init.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/reboot.h>
#include <linux/regmap.h>
#include <linux/slab.h>
-static struct max77620_chip *max77620_scratch;
-
static const struct resource gpio_resources[] = {
DEFINE_RES_IRQ(MAX77620_IRQ_TOP_GPIO),
};
@@ -483,13 +482,13 @@ static int max77620_read_es_version(struct max77620_chip *chip)
return ret;
}
-static void max77620_pm_power_off(void)
+static void max77620_pm_power_off(void *data)
{
- struct max77620_chip *chip = max77620_scratch;
+ struct max77620_chip *chip = data;
regmap_update_bits(chip->rmap, MAX77620_REG_ONOFFCNFG1,
- MAX77620_ONOFFCNFG1_SFT_RST,
- MAX77620_ONOFFCNFG1_SFT_RST);
+ MAX77620_ONOFFCNFG1_SFT_RST,
+ MAX77620_ONOFFCNFG1_SFT_RST);
}
static int max77620_probe(struct i2c_client *client,
@@ -566,9 +565,13 @@ static int max77620_probe(struct i2c_client *client,
}
pm_off = of_device_is_system_power_controller(client->dev.of_node);
- if (pm_off && !pm_power_off) {
- max77620_scratch = chip;
- pm_power_off = max77620_pm_power_off;
+ if (pm_off) {
+ ret = devm_register_simple_power_off_handler(chip->dev,
+ max77620_pm_power_off,
+ chip);
+ if (ret < 0)
+ dev_err(chip->dev,
+ "Failed to register power-off handler: %d\n", ret);
}
return 0;
--
2.33.1
Use devm_register_simple_power_off_handler() that replaces global
pm_power_off variable and allows to register multiple power-off handlers.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/mfd/max8907.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/mfd/max8907.c b/drivers/mfd/max8907.c
index 41f566e6a096..58699510311b 100644
--- a/drivers/mfd/max8907.c
+++ b/drivers/mfd/max8907.c
@@ -16,6 +16,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_device.h>
+#include <linux/reboot.h>
#include <linux/regmap.h>
#include <linux/slab.h>
@@ -174,9 +175,10 @@ static const struct regmap_irq_chip max8907_rtc_irq_chip = {
.num_irqs = ARRAY_SIZE(max8907_rtc_irqs),
};
-static struct max8907 *max8907_pm_off;
-static void max8907_power_off(void)
+static void max8907_power_off(void *data)
{
+ struct max8907 *max8907_pm_off = data;
+
regmap_update_bits(max8907_pm_off->regmap_gen, MAX8907_REG_RESET_CNFG,
MAX8907_MASK_POWER_OFF, MAX8907_MASK_POWER_OFF);
}
@@ -214,6 +216,17 @@ static int max8907_i2c_probe(struct i2c_client *i2c,
goto err_regmap_gen;
}
+ if (pm_off) {
+ ret = devm_register_simple_power_off_handler(&i2c->dev,
+ max8907_power_off,
+ max8907);
+ if (ret) {
+ dev_err(&i2c->dev,
+ "failed to register power-off handler: %d\n", ret);
+ return ret;
+ }
+ }
+
max8907->i2c_rtc = i2c_new_dummy_device(i2c->adapter, MAX8907_RTC_I2C_ADDR);
if (IS_ERR(max8907->i2c_rtc)) {
ret = PTR_ERR(max8907->i2c_rtc);
@@ -260,11 +273,6 @@ static int max8907_i2c_probe(struct i2c_client *i2c,
goto err_add_devices;
}
- if (pm_off && !pm_power_off) {
- max8907_pm_off = max8907;
- pm_power_off = max8907_power_off;
- }
-
return 0;
err_add_devices:
--
2.33.1
There is no need to annotate function prototypes with 'extern', it makes
code less readable. Remove unnecessary annotations from <notifier.h>.
Signed-off-by: Dmitry Osipenko <[email protected]>
---
include/linux/notifier.h | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 87069b8459af..4b80a815b666 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -90,7 +90,7 @@ struct srcu_notifier_head {
} while (0)
/* srcu_notifier_heads must be cleaned up dynamically */
-extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
+void srcu_init_notifier_head(struct srcu_notifier_head *nh);
#define srcu_cleanup_notifier_head(name) \
cleanup_srcu_struct(&(name)->srcu);
@@ -141,36 +141,36 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
#ifdef __KERNEL__
-extern int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
+int atomic_notifier_chain_register(struct atomic_notifier_head *nh,
struct notifier_block *nb);
-extern int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
+int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
struct notifier_block *nb);
-extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
+int raw_notifier_chain_register(struct raw_notifier_head *nh,
struct notifier_block *nb);
-extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
+int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
struct notifier_block *nb);
-extern int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
+int atomic_notifier_chain_unregister(struct atomic_notifier_head *nh,
struct notifier_block *nb);
-extern int blocking_notifier_chain_unregister(struct blocking_notifier_head *nh,
+int blocking_notifier_chain_unregister(struct blocking_notifier_head *nh,
struct notifier_block *nb);
-extern int raw_notifier_chain_unregister(struct raw_notifier_head *nh,
+int raw_notifier_chain_unregister(struct raw_notifier_head *nh,
struct notifier_block *nb);
-extern int srcu_notifier_chain_unregister(struct srcu_notifier_head *nh,
+int srcu_notifier_chain_unregister(struct srcu_notifier_head *nh,
struct notifier_block *nb);
-extern int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
+int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
unsigned long val, void *v);
-extern int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
+int blocking_notifier_call_chain(struct blocking_notifier_head *nh,
unsigned long val, void *v);
-extern int raw_notifier_call_chain(struct raw_notifier_head *nh,
+int raw_notifier_call_chain(struct raw_notifier_head *nh,
unsigned long val, void *v);
-extern int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
+int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
unsigned long val, void *v);
-extern int blocking_notifier_call_chain_robust(struct blocking_notifier_head *nh,
+int blocking_notifier_call_chain_robust(struct blocking_notifier_head *nh,
unsigned long val_up, unsigned long val_down, void *v);
-extern int raw_notifier_call_chain_robust(struct raw_notifier_head *nh,
+int raw_notifier_call_chain_robust(struct raw_notifier_head *nh,
unsigned long val_up, unsigned long val_down, void *v);
#define NOTIFY_DONE 0x0000 /* Don't care */
--
2.33.1
On 27.10.21 23:16, Dmitry Osipenko wrote:
> Kernel now supports chained power-off handlers. Use do_kernel_power_off()
> that invokes chained power-off handlers. It also invokes legacy
> pm_power_off() for now, which will be removed once all drivers will
> be converted to the new power-off API.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Juergen Gross <[email protected]>
Juergen
On Thu, Oct 28, 2021 at 12:16:54AM +0300, Dmitry Osipenko wrote:
> Use devm_register_power_handler() that replaces global pm_power_off_prepare
> variable and allows to register multiple power-off handlers.
Acked-by: Mark Brown <[email protected]>
Dmitry Osipenko <[email protected]> 於 2021年10月28日 週四 上午5:18寫道：
>
> Kernel now supports chained power-off handlers. Use do_kernel_power_off()
> that invokes chained power-off handlers. It also invokes legacy
> pm_power_off() for now, which will be removed once all drivers will
> be converted to the new power-off API.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> arch/nds32/kernel/process.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/nds32/kernel/process.c b/arch/nds32/kernel/process.c
> index 49fab9e39cbf..0936dcd7db1b 100644
> --- a/arch/nds32/kernel/process.c
> +++ b/arch/nds32/kernel/process.c
> @@ -54,8 +54,7 @@ EXPORT_SYMBOL(machine_halt);
>
> void machine_power_off(void)
> {
> - if (pm_power_off)
> - pm_power_off();
> + do_kernel_power_off();
> }
>
> EXPORT_SYMBOL(machine_power_off);
> --
> 2.33.1
>
Loop in Alan and KC
28.10.2021 00:16, Dmitry Osipenko пишет:
> mfd: ab8500: Use devm_register_trivial_power_off_handler()
> reset: ath79: Use devm_register_simple_restart_handler()
> reset: intel-gw: Use devm_register_simple_restart_handler()
> reset: lpc18xx: Use devm_register_prioritized_restart_handler()
> reset: npcm: Use devm_register_prioritized_restart_handler()
These patches got lost because Gmail gave me ban after 40's email. I
think it doesn't worth to re-send them now since you should get an idea
about how API usage looks like without the lost patches.
Only for this patch, Acked-by: Guo Ren <[email protected]>
On Thu, Oct 28, 2021 at 5:18 AM Dmitry Osipenko <[email protected]> wrote:
>
> Kernel now supports chained power-off handlers. Use do_kernel_power_off()
> that invokes chained power-off handlers. It also invokes legacy
> pm_power_off() for now, which will be removed once all drivers will
> be converted to the new power-off API.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> arch/csky/kernel/power.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/csky/kernel/power.c b/arch/csky/kernel/power.c
> index 923ee4e381b8..86ee202906f8 100644
> --- a/arch/csky/kernel/power.c
> +++ b/arch/csky/kernel/power.c
> @@ -9,16 +9,14 @@ EXPORT_SYMBOL(pm_power_off);
> void machine_power_off(void)
> {
> local_irq_disable();
> - if (pm_power_off)
> - pm_power_off();
> + do_kernel_power_off();
> asm volatile ("bkpt");
> }
>
> void machine_halt(void)
> {
> local_irq_disable();
> - if (pm_power_off)
> - pm_power_off();
> + do_kernel_power_off();
> asm volatile ("bkpt");
> }
>
> --
> 2.33.1
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
On Thu, Oct 28, 2021 at 12:16:41AM +0300, Dmitry Osipenko wrote:
> Kernel now supports chained power-off handlers. Use do_kernel_power_off()
> that invokes chained power-off handlers. It also invokes legacy
> pm_power_off() for now, which will be removed once all drivers will
> be converted to the new power-off API.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Catalin Marinas <[email protected]>
Hi,
On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
> Use devm_register_power_handler() that replaces global pm_power_off
> variable and allows to register multiple power-off handlers. It also
> provides restart-handler support, i.e. all in one API.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
When I boot with (most of) this patchset applied, I get the warning at
kernel/reboot.c:187:
/*
* Handler must have unique priority. Otherwise call order is
* determined by registration order, which is unreliable.
*/
WARN_ON(!atomic_notifier_has_unique_priority(&restart_handler_list, nb));
As the NTXEC driver doesn't specify a priority, I think this is an issue
to be fixed elsewhere.
Other than that, it works and looks good, as far as I can tell.
For this patch:
Reviewed-by: Jonathan Neuschäfer <[email protected]>
Tested-by: Jonathan Neuschäfer <[email protected]>
Best regards,
Jonathan
---
Full Oops log:
[ 3.523294] ------------[ cut here ]------------
[ 3.528193] WARNING: CPU: 0 PID: 1 at kernel/reboot.c:187 register_restart_handler+0x4c/0x58
[ 3.536975] Modules linked in:
[ 3.540312] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.15.0-00021-gcb24c628b307 #622
[ 3.548214] Hardware name: Freescale i.MX50 (Device Tree Support)
[ 3.554357] [<c0111540>] (unwind_backtrace) from [<c010cdd0>] (show_stack+0x10/0x14)
[ 3.562183] [<c010cdd0>] (show_stack) from [<c0bf240c>] (dump_stack_lvl+0x58/0x70)
[ 3.569824] [<c0bf240c>] (dump_stack_lvl) from [<c0127604>] (__warn+0xd4/0x154)
[ 3.577191] [<c0127604>] (__warn) from [<c0bec844>] (warn_slowpath_fmt+0x74/0xa8)
[ 3.584727] [<c0bec844>] (warn_slowpath_fmt) from [<c01593c8>] (register_restart_handler+0x4c/0x58)
[ 3.593823] [<c01593c8>] (register_restart_handler) from [<c08676c8>] (__watchdog_register_device+0x13c/0x27c)
[ 3.603889] [<c08676c8>] (__watchdog_register_device) from [<c0867868>] (watchdog_register_device+0x60/0xb4)
[ 3.613764] [<c0867868>] (watchdog_register_device) from [<c08678f8>] (devm_watchdog_register_device+0x3c/0x84)
[ 3.623898] [<c08678f8>] (devm_watchdog_register_device) from [<c1146454>] (imx2_wdt_probe+0x254/0x2ac)
[ 3.633346] [<c1146454>] (imx2_wdt_probe) from [<c06feb74>] (platform_probe+0x58/0xb8)
[ 3.641314] [<c06feb74>] (platform_probe) from [<c06fb2f8>] (call_driver_probe+0x24/0x108)
[ 3.649636] [<c06fb2f8>] (call_driver_probe) from [<c06fbe08>] (really_probe.part.0+0xa8/0x358)
[ 3.658384] [<c06fbe08>] (really_probe.part.0) from [<c06fc1c4>] (__driver_probe_device+0x94/0x208)
[ 3.667470] [<c06fc1c4>] (__driver_probe_device) from [<c06fc368>] (driver_probe_device+0x30/0xc8)
[ 3.676468] [<c06fc368>] (driver_probe_device) from [<c06fcb0c>] (__driver_attach+0xe0/0x1c4)
[ 3.685032] [<c06fcb0c>] (__driver_attach) from [<c06f9a20>] (bus_for_each_dev+0x74/0xc0)
[ 3.693253] [<c06f9a20>] (bus_for_each_dev) from [<c06faeb8>] (bus_add_driver+0x100/0x208)
[ 3.701563] [<c06faeb8>] (bus_add_driver) from [<c06fd8a0>] (driver_register+0x88/0x118)
[ 3.709696] [<c06fd8a0>] (driver_register) from [<c06fe920>] (__platform_driver_probe+0x44/0xdc)
[ 3.718522] [<c06fe920>] (__platform_driver_probe) from [<c01022ac>] (do_one_initcall+0x78/0x388)
[ 3.727444] [<c01022ac>] (do_one_initcall) from [<c1101708>] (do_initcalls+0xcc/0x110)
[ 3.735413] [<c1101708>] (do_initcalls) from [<c110198c>] (kernel_init_freeable+0x1ec/0x250)
[ 3.743896] [<c110198c>] (kernel_init_freeable) from [<c0bfe724>] (kernel_init+0x10/0x128)
[ 3.752224] [<c0bfe724>] (kernel_init) from [<c010011c>] (ret_from_fork+0x14/0x38)
[ 3.759844] Exception stack(0xc40adfb0 to 0xc40adff8)
[ 3.764933] dfa0: 00000000 00000000 00000000 00000000
[ 3.773143] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 3.781351] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 3.788347] irq event stamp: 143613
[ 3.792102] hardirqs last enabled at (143623): [<c01a3ebc>] __up_console_sem+0x50/0x60
[ 3.800397] hardirqs last disabled at (143632): [<c01a3ea8>] __up_console_sem+0x3c/0x60
[ 3.808491] softirqs last enabled at (143612): [<c0101518>] __do_softirq+0x2f8/0x5b0
[ 3.816591] softirqs last disabled at (143603): [<c01307dc>] __irq_exit_rcu+0x160/0x1d8
[ 3.825014] ---[ end trace 7f6709d2c89774b4 ]---
06.11.2021 23:54, Jonathan Neuschäfer пишет:
> Hi,
>
> On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
>> Use devm_register_power_handler() that replaces global pm_power_off
>> variable and allows to register multiple power-off handlers. It also
>> provides restart-handler support, i.e. all in one API.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>
> When I boot with (most of) this patchset applied, I get the warning at
> kernel/reboot.c:187:
>
> /*
> * Handler must have unique priority. Otherwise call order is
> * determined by registration order, which is unreliable.
> */
> WARN_ON(!atomic_notifier_has_unique_priority(&restart_handler_list, nb));
>
> As the NTXEC driver doesn't specify a priority, I think this is an issue
> to be fixed elsewhere.
>
> Other than that, it works and looks good, as far as I can tell.
>
>
> For this patch:
>
> Reviewed-by: Jonathan Neuschäfer <[email protected]>
> Tested-by: Jonathan Neuschäfer <[email protected]>
Thank you. You have conflicting restart handlers, apparently NTXEC
driver should have higher priority than the watchdog driver. It should
be a common problem for the watchdog drivers, I will lower watchdog's
default priority to fix it.
On 11/7/21 8:53 AM, Dmitry Osipenko wrote:
> 06.11.2021 23:54, Jonathan Neuschäfer пишет:
>> Hi,
>>
>> On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
>>> Use devm_register_power_handler() that replaces global pm_power_off
>>> variable and allows to register multiple power-off handlers. It also
>>> provides restart-handler support, i.e. all in one API.
>>>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>
>> When I boot with (most of) this patchset applied, I get the warning at
>> kernel/reboot.c:187:
>>
>> /*
>> * Handler must have unique priority. Otherwise call order is
>> * determined by registration order, which is unreliable.
>> */
>> WARN_ON(!atomic_notifier_has_unique_priority(&restart_handler_list, nb));
>>
>> As the NTXEC driver doesn't specify a priority, I think this is an issue
>> to be fixed elsewhere.
>>
>> Other than that, it works and looks good, as far as I can tell.
>>
>>
>> For this patch:
>>
>> Reviewed-by: Jonathan Neuschäfer <[email protected]>
>> Tested-by: Jonathan Neuschäfer <[email protected]>
>
> Thank you. You have conflicting restart handlers, apparently NTXEC
> driver should have higher priority than the watchdog driver. It should
> be a common problem for the watchdog drivers, I will lower watchdog's
> default priority to fix it.
>
The watchdog subsystem already uses "0" as default priority, which was
intended as priority of last resort for restart handlers. I do not see
a reason to change that.
Guenter
07.11.2021 20:08, Guenter Roeck пишет:
> On 11/7/21 8:53 AM, Dmitry Osipenko wrote:
>> 06.11.2021 23:54, Jonathan Neuschäfer пишет:
>>> Hi,
>>>
>>> On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
>>>> Use devm_register_power_handler() that replaces global pm_power_off
>>>> variable and allows to register multiple power-off handlers. It also
>>>> provides restart-handler support, i.e. all in one API.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>> ---
>>>
>>> When I boot with (most of) this patchset applied, I get the warning at
>>> kernel/reboot.c:187:
>>>
>>> /*
>>> * Handler must have unique priority. Otherwise call order is
>>> * determined by registration order, which is unreliable.
>>> */
>>> WARN_ON(!atomic_notifier_has_unique_priority(&restart_handler_list,
>>> nb));
>>>
>>> As the NTXEC driver doesn't specify a priority, I think this is an issue
>>> to be fixed elsewhere.
>>>
>>> Other than that, it works and looks good, as far as I can tell.
>>>
>>>
>>> For this patch:
>>>
>>> Reviewed-by: Jonathan Neuschäfer <[email protected]>
>>> Tested-by: Jonathan Neuschäfer <[email protected]>
>>
>> Thank you. You have conflicting restart handlers, apparently NTXEC
>> driver should have higher priority than the watchdog driver. It should
>> be a common problem for the watchdog drivers, I will lower watchdog's
>> default priority to fix it.
>>
>
> The watchdog subsystem already uses "0" as default priority, which was
> intended as priority of last resort for restart handlers. I do not see
> a reason to change that.
Right, I meant that watchdog drivers which use restart handler set the
level to the default 128 [1]. Although, maybe it's a problem only for
i.MX drivers in practice, I'll take a closer look at the other drivers.
[1]
https://elixir.bootlin.com/linux/latest/source/drivers/watchdog/imx2_wdt.c#L318
On 11/7/21 9:16 AM, Dmitry Osipenko wrote:
> 07.11.2021 20:08, Guenter Roeck пишет:
>> On 11/7/21 8:53 AM, Dmitry Osipenko wrote:
>>> 06.11.2021 23:54, Jonathan Neuschäfer пишет:
>>>> Hi,
>>>>
>>>> On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
>>>>> Use devm_register_power_handler() that replaces global pm_power_off
>>>>> variable and allows to register multiple power-off handlers. It also
>>>>> provides restart-handler support, i.e. all in one API.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>>> ---
>>>>
>>>> When I boot with (most of) this patchset applied, I get the warning at
>>>> kernel/reboot.c:187:
>>>>
>>>> /*
>>>> * Handler must have unique priority. Otherwise call order is
>>>> * determined by registration order, which is unreliable.
>>>> */
>>>> WARN_ON(!atomic_notifier_has_unique_priority(&restart_handler_list,
>>>> nb));
>>>>
>>>> As the NTXEC driver doesn't specify a priority, I think this is an issue
>>>> to be fixed elsewhere.
>>>>
>>>> Other than that, it works and looks good, as far as I can tell.
>>>>
>>>>
>>>> For this patch:
>>>>
>>>> Reviewed-by: Jonathan Neuschäfer <[email protected]>
>>>> Tested-by: Jonathan Neuschäfer <[email protected]>
>>>
>>> Thank you. You have conflicting restart handlers, apparently NTXEC
>>> driver should have higher priority than the watchdog driver. It should
>>> be a common problem for the watchdog drivers, I will lower watchdog's
>>> default priority to fix it.
>>>
>>
>> The watchdog subsystem already uses "0" as default priority, which was
>> intended as priority of last resort for restart handlers. I do not see
>> a reason to change that.
>
> Right, I meant that watchdog drivers which use restart handler set the
> level to the default 128 [1]. Although, maybe it's a problem only for
> i.MX drivers in practice, I'll take a closer look at the other drivers.
>
They don't have to do that. The default is priority 0. It is the decision
of the driver author to set the watchdog's restart priority. So it is wrong
to claim that this would be "a common problem for the watchdog drivers",
because it isn't. Presumably there was a reason for the driver author
to select the default priority of 128. If there is a platform which has
a better means to restart the system, it should select a priority of
129 or higher instead of affecting _all_ platforms using the imx watchdog
to reset the system.
Sure, you can negotiate that with the driver author, but the default should
really be to change the priority for less affected platforms.
Guenter
07.11.2021 20:32, Guenter Roeck пишет:
> On 11/7/21 9:16 AM, Dmitry Osipenko wrote:
>> 07.11.2021 20:08, Guenter Roeck пишет:
>>> On 11/7/21 8:53 AM, Dmitry Osipenko wrote:
>>>> 06.11.2021 23:54, Jonathan Neuschäfer пишет:
>>>>> Hi,
>>>>>
>>>>> On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
>>>>>> Use devm_register_power_handler() that replaces global pm_power_off
>>>>>> variable and allows to register multiple power-off handlers. It also
>>>>>> provides restart-handler support, i.e. all in one API.
>>>>>>
>>>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>>>> ---
>>>>>
>>>>> When I boot with (most of) this patchset applied, I get the warning at
>>>>> kernel/reboot.c:187:
>>>>>
>>>>> /*
>>>>> * Handler must have unique priority. Otherwise call order is
>>>>> * determined by registration order, which is unreliable.
>>>>> */
>>>>> WARN_ON(!atomic_notifier_has_unique_priority(&restart_handler_list,
>>>>>
>>>>> nb));
>>>>>
>>>>> As the NTXEC driver doesn't specify a priority, I think this is an
>>>>> issue
>>>>> to be fixed elsewhere.
>>>>>
>>>>> Other than that, it works and looks good, as far as I can tell.
>>>>>
>>>>>
>>>>> For this patch:
>>>>>
>>>>> Reviewed-by: Jonathan Neuschäfer <[email protected]>
>>>>> Tested-by: Jonathan Neuschäfer <[email protected]>
>>>>
>>>> Thank you. You have conflicting restart handlers, apparently NTXEC
>>>> driver should have higher priority than the watchdog driver. It should
>>>> be a common problem for the watchdog drivers, I will lower watchdog's
>>>> default priority to fix it.
>>>>
>>>
>>> The watchdog subsystem already uses "0" as default priority, which was
>>> intended as priority of last resort for restart handlers. I do not see
>>> a reason to change that.
>>
>> Right, I meant that watchdog drivers which use restart handler set the
>> level to the default 128 [1]. Although, maybe it's a problem only for
>> i.MX drivers in practice, I'll take a closer look at the other drivers.
>>
>
> They don't have to do that. The default is priority 0. It is the decision
> of the driver author to set the watchdog's restart priority. So it is wrong
> to claim that this would be "a common problem for the watchdog drivers",
> because it isn't. Presumably there was a reason for the driver author
> to select the default priority of 128. If there is a platform which has
> a better means to restart the system, it should select a priority of
> 129 or higher instead of affecting _all_ platforms using the imx watchdog
> to reset the system.
>
> Sure, you can negotiate that with the driver author, but the default should
> really be to change the priority for less affected platforms.
Yes, looks like there is no common problem for watchdog drivers.
Initially I was recalling that watchdog core uses 128 by default and
typed the message without verifying it. I see now that it's incorrect,
my bad.
EC drivers tend to use higher priority in general. Jonathan, could you
please confirm that NTXEC driver is a more preferable restart method
than the watchdog?
08.11.2021 14:22, Jonathan Neuschäfer пишет:
> On Sun, Nov 07, 2021 at 08:42:33PM +0300, Dmitry Osipenko wrote:
> [...]
>> EC drivers tend to use higher priority in general. Jonathan, could you
>> please confirm that NTXEC driver is a more preferable restart method
>> than the watchdog?
>
> Yes. The original firmware uses the NTXEC to restart, and it works well,
> so I do think it's preferable.
Thank you, then I'll update the NTXEC patch like this:
https://github.com/grate-driver/linux/commit/22da3d91f1734d9a0ed036220ad4ea28465af988
On Sun, Nov 07, 2021 at 08:42:33PM +0300, Dmitry Osipenko wrote:
[...]
> EC drivers tend to use higher priority in general. Jonathan, could you
> please confirm that NTXEC driver is a more preferable restart method
> than the watchdog?
Yes. The original firmware uses the NTXEC to restart, and it works well,
so I do think it's preferable.
Best regards,
Jonathan
On Mon, Nov 08, 2021 at 02:36:42PM +0300, Dmitry Osipenko wrote:
> 08.11.2021 14:22, Jonathan Neuschäfer пишет:
> > On Sun, Nov 07, 2021 at 08:42:33PM +0300, Dmitry Osipenko wrote:
> > [...]
> >> EC drivers tend to use higher priority in general. Jonathan, could you
> >> please confirm that NTXEC driver is a more preferable restart method
> >> than the watchdog?
> >
> > Yes. The original firmware uses the NTXEC to restart, and it works well,
> > so I do think it's preferable.
>
> Thank you, then I'll update the NTXEC patch like this:
>
> https://github.com/grate-driver/linux/commit/22da3d91f1734d9a0ed036220ad4ea28465af988
I tested again, but sys_off_handler_reboot called a bogus pointer
(probably reboot_prepare_cb). I think it was left uninitialized in
ntxec_probe, which uses devm_kmalloc. I guess we could switch it to
devm_kzalloc:
diff --git a/drivers/mfd/ntxec.c b/drivers/mfd/ntxec.c
index 1f55dfce14308..30364beb4b1d0 100644
--- a/drivers/mfd/ntxec.c
+++ b/drivers/mfd/ntxec.c
@@ -144,7 +144,7 @@ static int ntxec_probe(struct i2c_client *client)
const struct mfd_cell *subdevs;
size_t n_subdevs;
- ec = devm_kmalloc(&client->dev, sizeof(*ec), GFP_KERNEL);
+ ec = devm_kzalloc(&client->dev, sizeof(*ec), GFP_KERNEL);
if (!ec)
return -ENOMEM;
With that done, it works flawlessly.
Thanks,
Jonathan
10.11.2021 13:43, Jonathan Neuschäfer пишет:
> On Mon, Nov 08, 2021 at 02:36:42PM +0300, Dmitry Osipenko wrote:
>> 08.11.2021 14:22, Jonathan Neuschäfer пишет:
>>> On Sun, Nov 07, 2021 at 08:42:33PM +0300, Dmitry Osipenko wrote:
>>> [...]
>>>> EC drivers tend to use higher priority in general. Jonathan, could you
>>>> please confirm that NTXEC driver is a more preferable restart method
>>>> than the watchdog?
>>>
>>> Yes. The original firmware uses the NTXEC to restart, and it works well,
>>> so I do think it's preferable.
>>
>> Thank you, then I'll update the NTXEC patch like this:
>>
>> https://github.com/grate-driver/linux/commit/22da3d91f1734d9a0ed036220ad4ea28465af988
>
> I tested again, but sys_off_handler_reboot called a bogus pointer
> (probably reboot_prepare_cb). I think it was left uninitialized in
> ntxec_probe, which uses devm_kmalloc. I guess we could switch it to
> devm_kzalloc:
>
> diff --git a/drivers/mfd/ntxec.c b/drivers/mfd/ntxec.c
> index 1f55dfce14308..30364beb4b1d0 100644
> --- a/drivers/mfd/ntxec.c
> +++ b/drivers/mfd/ntxec.c
> @@ -144,7 +144,7 @@ static int ntxec_probe(struct i2c_client *client)
> const struct mfd_cell *subdevs;
> size_t n_subdevs;
>
> - ec = devm_kmalloc(&client->dev, sizeof(*ec), GFP_KERNEL);
> + ec = devm_kzalloc(&client->dev, sizeof(*ec), GFP_KERNEL);
> if (!ec)
> return -ENOMEM;
>
>
>
> With that done, it works flawlessly.
Good catch, thank you! I'll correct this patch and add yours t-b.
On Thu, 28 Oct 2021, Dmitry Osipenko wrote:
> Use devm_register_power_handler() that replaces global pm_power_off
> variable and allows to register multiple power-off handlers. It also
> provides restart-handler support, i.e. all in one API.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/mfd/rn5t618.c | 56 ++++++++++++++++---------------------------
> 1 file changed, 21 insertions(+), 35 deletions(-)
For my own reference (apply this as-is to your sign-off block):
Acked-for-MFD-by: Lee Jones <[email protected]>
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
29.11.2021 14:55, Lee Jones пишет:
> On Thu, 28 Oct 2021, Dmitry Osipenko wrote:
>
>> Use devm_register_power_handler() that replaces global pm_power_off
>> variable and allows to register multiple power-off handlers. It also
>> provides restart-handler support, i.e. all in one API.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> drivers/mfd/rn5t618.c | 56 ++++++++++++++++---------------------------
>> 1 file changed, 21 insertions(+), 35 deletions(-)
>
> For my own reference (apply this as-is to your sign-off block):
>
> Acked-for-MFD-by: Lee Jones <[email protected]>
>
Thanks you. This and other driver patches will be slightly changed
because the power-handler was renamed to sys-off handler starting with
the v3 of this series, but yours ack still will be valid here.