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
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 <digetx@gmail.com>
---
include/linux/notifier.h | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
There is no need to annotate function prototypes with 'extern', it makes
code less readable. Remove unnecessary annotations from <reboot.h>.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
include/linux/reboot.h | 38 +++++++++++++++++++-------------------
1 file changed, 19 insertions(+), 19 deletions(-)
@@ -40,36 +40,36 @@ extern int reboot_cpu;externintreboot_force;-externintregister_reboot_notifier(structnotifier_block*);-externintunregister_reboot_notifier(structnotifier_block*);+intregister_reboot_notifier(structnotifier_block*);+intunregister_reboot_notifier(structnotifier_block*);-externintdevm_register_reboot_notifier(structdevice*,structnotifier_block*);+intdevm_register_reboot_notifier(structdevice*,structnotifier_block*);-externintregister_restart_handler(structnotifier_block*);-externintunregister_restart_handler(structnotifier_block*);-externvoiddo_kernel_restart(char*cmd);+intregister_restart_handler(structnotifier_block*);+intunregister_restart_handler(structnotifier_block*);+voiddo_kernel_restart(char*cmd);/**Architecture-specificimplementationsofsys_rebootcommands.*/-externvoidmigrate_to_reboot_cpu(void);-externvoidmachine_restart(char*cmd);-externvoidmachine_halt(void);-externvoidmachine_power_off(void);+voidmigrate_to_reboot_cpu(void);+voidmachine_restart(char*cmd);+voidmachine_halt(void);+voidmachine_power_off(void);-externvoidmachine_shutdown(void);+voidmachine_shutdown(void);structpt_regs;-externvoidmachine_crash_shutdown(structpt_regs*);+voidmachine_crash_shutdown(structpt_regs*);/**Architectureindependentimplementationsofsys_rebootcommands.*/-externvoidkernel_restart_prepare(char*cmd);-externvoidkernel_restart(char*cmd);-externvoidkernel_halt(void);-externvoidkernel_power_off(void);+voidkernel_restart_prepare(char*cmd);+voidkernel_restart(char*cmd);+voidkernel_halt(void);+voidkernel_power_off(void);externintC_A_D;/* for sysctl */voidctrl_alt_del(void);
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 <digetx@gmail.com>
---
arch/x86/xen/enlighten_pv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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 <digetx@gmail.com>
---
arch/arm/kernel/reboot.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
SoC platforms often have multiple options of how to perform system's
power-off and restart operations. Meanwhile today's kernel is limited to
a single option. Add combined power-off+restart handler call chain API,
which is inspired by the restart API. The new API provides both power-off
and restart functionality.
The old pm_power_off method will be kept around till all users are
converted to the new API.
Current restart API will be replaced by the new unified API since
new API is its superset. The restart functionality of the power-handler
API is built upon the existing restart-notifier APIs.
In order to ease conversion to the new API, convenient helpers are added
for the common use-cases. They will reduce amount of boilerplate code and
remove global variables. These helpers preserve old behaviour for cases
where only one power-off handler is executed, this is what existing
drivers want, and thus, they could be easily converted to the new API.
Users of the new API should explicitly enable power-off chaining by
setting corresponding flag of the power_handler structure.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
include/linux/reboot.h | 176 +++++++++++-
kernel/power/hibernate.c | 2 +-
kernel/reboot.c | 601 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 768 insertions(+), 11 deletions(-)
@@ -8,10 +8,16 @@structdevice;-#define SYS_DOWN 0x0001 /* Notify of system down */-#define SYS_RESTART SYS_DOWN-#define SYS_HALT 0x0002 /* Notify of system halt */-#define SYS_POWER_OFF 0x0003 /* Notify of system power off */+enumreboot_prepare_mode{+SYS_DOWN=1,/* Notify of system down */+SYS_RESTART=SYS_DOWN,+SYS_HALT,/* Notify of system halt */+SYS_POWER_OFF,/* Notify of system power off */+};++#define RESTART_PRIO_RESERVED 0+#define RESTART_PRIO_DEFAULT 128+#define RESTART_PRIO_HIGH 192enumreboot_mode{REBOOT_UNDEFINED=-1,
@@ -189,9 +189,8 @@ int register_restart_handler(struct notifier_block *nb)returnret;/*-*Handlermusthaveuniquepriority.Otherwiseinvocationorderis-*determinedbytheregistrationorder,whichispresumedtobe-*unreliable.+*Handlermusthaveuniquepriority.Otherwisecallorderis+*determinedbyregistrationorder,whichisunreliable.*/WARN_ON(!atomic_notifier_has_unique_priority(&restart_handler_list,nb));
@@ -294,6 +293,587 @@ void kernel_halt(void)}EXPORT_SYMBOL_GPL(kernel_halt);+/*+*Notifierlistforkernelcodewhichwantstobecalled+*topoweroffthesystem.+*/+staticBLOCKING_NOTIFIER_HEAD(power_off_handler_list);++staticvoiddummy_pm_power_off(void)+{+/* temporary stub until pm_power_off() is gone, see more below */+}++staticstructnotifier_block*pm_power_off_nb;++/**+*register_power_off_handler-Registerfunctiontobecalledtopoweroff+*thesystem+*@nb:Infoabouthandlerfunctiontobecalled+*@nb->priority:Handlerpriority.Handlersshouldfollowthe+*followingguidelinesforsettingpriorities.+*0:Power-offhandleroflastresort,+*withlimitedpower-offcapabilities+*128:Defaultpower-offhandler;useifnoother+*power-offhandlerisexpectedtobeavailable,+*and/orifpower-offfunctionalityis+*sufficienttopower-offtheentiresystem+*255:Highestprioritypower-offhandler,will+*preemptallotherpower-offhandlers+*+*Registersafunctionwithcodetobecalledtopoweroffthe+*system.+*+*Registeredfunctionswillbecalledaslaststepofthepower-off+*sequence.+*+*Registeredfunctionsareexpectedtopoweroffthesystemimmediately.+*Ifmorethanonefunctionisregistered,thepower-offhandlerpriority+*selectswhichfunctionwillbecalledfirst.+*+*Power-offhandlersareexpectedtoberegisteredfromnon-architecture+*code,typicallyfromdrivers.Atypicalusecasewouldbeasystem+*wherepower-offfunctionalityisprovidedthroughaPMIC.Multiple+*power-offhandlersmayexist;forexample,onepower-offhandlermight+*turnofftheentiresystem,whileanotheronlyturnsoffpartof+*system.Insuchcases,thepower-offhandlerwhichonlydisablespart+*ofthehardwareisexpectedtoregisterwithlowprioritytoensure+*thatitonlyrunsifnoothermeanstopoweroffthesystemis+*available.+*+*Currentlyalwaysreturnszero,asblocking_notifier_chain_register()+*alwaysreturnszero.+*/+staticintregister_power_off_handler(structnotifier_block*nb)+{+intret;++ret=blocking_notifier_chain_register(&power_off_handler_list,nb);+if(ret)+returnret;++/*+*Handlermusthaveuniquepriority.Otherwiseinvocationorderis+*determinedbytheregistrationorder,whichispresumedtobe+*unreliable.+*/+WARN_ON(!blocking_notifier_has_unique_priority(&power_off_handler_list,nb));++/*+*Somedriverscheckwhetherpm_power_offwasalreadyinstalled.+*InstalldummycallbackusingnewAPItopreserveoldbehaviour+*forthosedriversduringperiodoftransitiontothenewAPI.+*/+if(!pm_power_off){+pm_power_off=dummy_pm_power_off;+pm_power_off_nb=nb;+}++return0;+}++staticvoidunregister_power_off_handler(structnotifier_block*nb)+{+if(nb==pm_power_off_nb){+/*+*Checkwhethersomebodyreplacedpm_power_offbehind+*outback.+*/+if(!WARN_ON(pm_power_off!=dummy_pm_power_off))+pm_power_off=NULL;++pm_power_off_nb=NULL;+}++WARN_ON(blocking_notifier_chain_unregister(&power_off_handler_list,nb));+}++staticvoiddevm_unregister_power_off_handler(void*data)+{+structnotifier_block*nb=data;++unregister_power_off_handler(nb);+}++staticintdevm_register_power_off_handler(structdevice*dev,+structnotifier_block*nb)+{+interr;++err=register_power_off_handler(nb);+if(err)+returnerr;++returndevm_add_action_or_reset(dev,devm_unregister_power_off_handler,+nb);+}++staticintpower_handler_power_off(structnotifier_block*nb,+unsignedlongmode,void*unused)+{+structpower_off_prep_dataprep_data={};+structpower_handler_private_data*priv;+structpower_off_datadata={};+structpower_handler*h;+intret=NOTIFY_DONE;++priv=container_of(nb,structpower_handler_private_data,power_off_nb);+h=container_of(priv,structpower_handler,priv);+prep_data.cb_data=h->cb_data;+data.cb_data=h->cb_data;++switch(mode){+casePOWEROFF_NORMAL:+if(h->power_off_cb)+h->power_off_cb(&data);++if(priv->simple_power_off_cb)+priv->simple_power_off_cb(priv->simple_power_off_cb_data);++if(priv->trivial_power_off_cb)+priv->trivial_power_off_cb();++if(!h->power_off_chaining_allowed)+ret=NOTIFY_STOP;++break;++casePOWEROFF_PREPARE:+if(h->power_off_prepare_cb)+h->power_off_prepare_cb(&prep_data);++break;++default:+unreachable();+}++returnret;+}++staticintpower_handler_restart(structnotifier_block*nb,+unsignedlongmode,void*cmd)+{+structpower_handler_private_data*priv;+structrestart_datadata={};+structpower_handler*h;++priv=container_of(nb,structpower_handler_private_data,restart_nb);+h=container_of(priv,structpower_handler,priv);++data.cb_data=h->cb_data;+data.mode=mode;+data.cmd=cmd;++h->restart_cb(&data);++returnNOTIFY_DONE;+}++staticintpower_handler_restart_prep(structnotifier_block*nb,+unsignedlongmode,void*cmd)+{+structpower_handler_private_data*priv;+structreboot_prep_datadata={};+structpower_handler*h;++priv=container_of(nb,structpower_handler_private_data,reboot_prep_nb);+h=container_of(priv,structpower_handler,priv);++data.cb_data=h->cb_data;+data.mode=mode;+data.cmd=cmd;++h->reboot_prepare_cb(&data);++returnNOTIFY_DONE;+}++staticstructpower_handler_private_data*+power_handler_private_data(structpower_handler*handler)+{+return(structpower_handler_private_data*)&handler->priv;+}++/**+*devm_register_power_handler-Registerpowerhandler+*@dev:Devicethatregistershandler+*@handler:Powerhandlerdescriptor+*+*Registerspowerhandlerthatwillbecalledaslaststepofthe+*power-offandrestartsequences.+*+*Returnszeroonsuccess,orerrorcodeonfailure.+*/+intregister_power_handler(structpower_handler*handler)+{+structpower_handler_private_data*priv=power_handler_private_data(handler);+interr,priority;++/* sanity-check whether handler is registered twice */+if(WARN_ON(priv->registered))+return-EBUSY;++if(handler->power_off_cb||handler->power_off_prepare_cb){+if(handler->power_off_priority==POWEROFF_PRIO_RESERVED)+priority=POWEROFF_PRIO_DEFAULT;+else+priority=handler->power_off_priority;++priv->power_off_nb.notifier_call=power_handler_power_off;+priv->power_off_nb.priority=priority;++err=register_power_off_handler(&priv->power_off_nb);+if(err)+gotoreset_power_handler;+}++if(handler->restart_cb){+if(handler->restart_priority==RESTART_PRIO_RESERVED)+priority=RESTART_PRIO_DEFAULT;+else+priority=handler->restart_priority;++priv->restart_nb.notifier_call=power_handler_restart;+priv->restart_nb.priority=priority;++err=register_restart_handler(&priv->restart_nb);+if(err)+gotounreg_power_off_handler;+}++if(handler->reboot_prepare_cb){+priv->reboot_prep_nb.notifier_call=power_handler_restart_prep;+priv->reboot_prep_nb.priority=0;++err=register_reboot_notifier(&priv->reboot_prep_nb);+if(err)+gotounreg_restart_handler;+}++priv->registered=true;++return0;++unreg_restart_handler:+if(handler->restart_cb)+unregister_restart_handler(&priv->restart_nb);++unreg_power_off_handler:+if(handler->power_off_cb)+unregister_power_off_handler(&priv->power_off_nb);++reset_power_handler:+memset(priv,0,sizeof(*priv));++returnerr;+}+EXPORT_SYMBOL(register_power_handler);++/**+*unregister_power_handler-Unregisterpowerhandler+*@handler:Powerhandlerdescriptor+*+*Unregistersapreviouslyregisteredpowerhandler.Doesnothingif+*handlerisNULL.+*/+voidunregister_power_handler(structpower_handler*handler)+{+structpower_handler_private_data*priv;++if(!handler)+return;++priv=power_handler_private_data(handler);++/* sanity-check whether handler is unregistered twice */+if(WARN_ON(!priv->registered))+return;++if(handler->reboot_prepare_cb)+unregister_reboot_notifier(&priv->reboot_prep_nb);++if(handler->restart_cb)+unregister_restart_handler(&priv->restart_nb);++if(handler->power_off_cb)+unregister_power_off_handler(&priv->power_off_nb);++memset(priv,0,sizeof(*priv));+}+EXPORT_SYMBOL(unregister_power_handler);++staticvoiddevm_unregister_power_handler(void*data)+{+structpower_handler*handler=data;++unregister_power_handler(handler);+}++/**+*devm_register_power_handler-Registerpowerhandler+*@dev:Devicethatregistershandler+*@handler:Powerhandlerdescriptor+*+*Resource-managedvariantofregister_power_handler();+*+*Returnszeroonsuccess,orerrorcodeonfailure.+*/+intdevm_register_power_handler(structdevice*dev,+structpower_handler*handler)+{+interr;++err=register_power_handler(handler);+if(err)+returnerr;++returndevm_add_action_or_reset(dev,devm_unregister_power_handler,+handler);+}+EXPORT_SYMBOL(devm_register_power_handler);++/**+*register_simple_power_off_handler-Registersimplepower-offcallback+*@dev:Devicethatregisterscallback+*@callback:Callbackfunction+*@data:Callback'sargument+*+*Registerspower-offcallbackwithdefaultpriority,itwillbecalled+*aslaststepofthepower-offsequence.+*+*Returnspower_handlerpointeronsuccess,orERR_PTRonfailure.+*/+structpower_handler*+register_simple_power_off_handler(void(*callback)(void*data),void*data)+{+structpower_handler_private_data*priv;+structpower_handler*handler;+interr;++handler=kzalloc(sizeof(*handler),GFP_KERNEL);+if(!handler)+returnERR_PTR(-ENOMEM);++priv=power_handler_private_data(handler);++priv->power_off_nb.notifier_call=power_handler_power_off;+priv->power_off_nb.priority=POWEROFF_PRIO_DEFAULT;+priv->simple_power_off_cb_data=data;+priv->simple_power_off_cb=callback;++err=register_power_off_handler(&priv->power_off_nb);+if(err){+kfree(handler);+returnERR_PTR(err);+}++returnhandler;+}+EXPORT_SYMBOL(register_simple_power_off_handler);++/**+*unregister_power_handler-Unregistersimplepower-offhandler+*@handler:Powerhandlerdescriptor+*+*Unregisterspowerhandlerthatwasregisteredby+*register_simple_power_off_handler().Doesnothingifhandleris+*errororNULL.+*/+voidunregister_simple_power_off_handler(structpower_handler*handler)+{+structpower_handler_private_data*priv;++if(!IS_ERR_OR_NULL(handler)){+priv=power_handler_private_data(handler);+unregister_power_off_handler(&priv->power_off_nb);+kfree(handler);+}+}+EXPORT_SYMBOL(unregister_simple_power_off_handler);++/**+*devm_register_simple_power_off_handler-Registersimplepower-offcallback+*@dev:Devicethatregisterscallback+*@callback:Callbackfunction+*@data:Callback'sargument+*+*Registersresource-managedpower-offcallbackwithdefaultpriority,+*itwillbecalledaslaststepofthepower-offsequence.Further+*lowerprioritycallbackswon'tbeexecutedifthis@callbackfails.+*+*Returnszeroonsuccess,orerrorcodeonfailure.+*/+intdevm_register_simple_power_off_handler(structdevice*dev,+void(*callback)(void*data),+void*data)+{+structpower_handler_private_data*priv;+structpower_handler*handler;++handler=devm_kzalloc(dev,sizeof(*handler),GFP_KERNEL);+if(!handler)+return-ENOMEM;++priv=power_handler_private_data(handler);++priv->power_off_nb.notifier_call=power_handler_power_off;+priv->power_off_nb.priority=POWEROFF_PRIO_DEFAULT;+priv->simple_power_off_cb_data=data;+priv->simple_power_off_cb=callback;++returndevm_register_power_off_handler(dev,&priv->power_off_nb);+}+EXPORT_SYMBOL(devm_register_simple_power_off_handler);++/**+*devm_register_trivial_power_off_handler-Registertrivialpower-offcallback+*@dev:Devicethatregisterscallback+*@desc:Callbackdescriptor+*+*Sameasdevm_register_simple_power_off_handler(),butcallback+*doesn'ttakeargument.Furtherlowerprioritycallbackswon'tbe+*executedifthis@callbackfails.+*+*Returnszeroonsuccess,orerrorcodeonfailure.+*/+intdevm_register_trivial_power_off_handler(structdevice*dev,+void(*callback)(void))+{+structpower_handler_private_data*priv;+structpower_handler*handler;++handler=devm_kzalloc(dev,sizeof(*handler),GFP_KERNEL);+if(!handler)+return-ENOMEM;++priv=power_handler_private_data(handler);++priv->power_off_nb.notifier_call=power_handler_power_off;+priv->power_off_nb.priority=POWEROFF_PRIO_DEFAULT;+priv->trivial_power_off_cb=callback;++returndevm_register_power_off_handler(dev,&priv->power_off_nb);+}+EXPORT_SYMBOL(devm_register_trivial_power_off_handler);++/**+*devm_register_simple_restart_handler-Registersimplerestartcallback+*@dev:Devicethatregisterscallback+*@callback:Callbackfunction+*@data:Callback'sargument+*+*Registersresource-managedrestartcallbackwithdefaultpriority,+*itwillbecalledaslaststepoftherestartsequence.+*+*Returnszeroonsuccess,orerrorcodeonfailure.+*/+intdevm_register_simple_restart_handler(structdevice*dev,+void(*callback)(structrestart_data*data),+void*data)+{+returndevm_register_prioritized_restart_handler(dev,+RESTART_PRIO_DEFAULT,+callback,data);+}+EXPORT_SYMBOL(devm_register_simple_restart_handler);++/**+*devm_register_prioritized_restart_handler-Registerprioritizedrestartcallback+*@dev:Devicethatregisterscallback+*@priority:Callback'spriority+*@callback:Callbackfunction+*@data:Callback'sargument+*+*Registersresource-managedrestartcallbackwithagivenpriority,+*itwillbecalledaslaststepoftherestartsequence.+*+*Returnszeroonsuccess,orerrorcodeonfailure.+*/+intdevm_register_prioritized_restart_handler(structdevice*dev,+intpriority,+void(*callback)(structrestart_data*data),+void*data)+{+structpower_handler*handler;++handler=devm_kzalloc(dev,sizeof(*handler),GFP_KERNEL);+if(!handler)+return-ENOMEM;++handler->restart_priority=priority;+handler->restart_cb=callback;+handler->cb_data=data;++returndevm_register_power_handler(dev,handler);+}+EXPORT_SYMBOL(devm_register_prioritized_restart_handler);++staticstructpower_handlerplatform_power_off_handler={+.priv={+.power_off_nb={+.notifier_call=power_handler_power_off,+.priority=POWEROFF_PRIO_PLATFORM,+},+},+};++/**+*register_platform_power_off-Registerplatform-levelpower-offcallback+*@power_off:Power-offcallback+*+*Registerspower-offcallbackthatwillbecalledaslaststep+*ofthepower-offsequence.Thiscallbackisexpectedtobeinvoked+*forthelastresort.Furtherlowerprioritycallbackswon'tbe+*executedif@power_offfails.Onlyoneplatformpower-offcallback+*isallowedtoberegistered.+*+*Returnszeroonsuccess,orerrorcodeonfailure.+*/+intregister_platform_power_off(void(*power_off)(void))+{+structpower_handler_private_data*priv;++/* this function is allowed to be called only once */+if(WARN_ON(platform_power_off_handler.priv.trivial_power_off_cb))+return-EBUSY;++priv=power_handler_private_data(&platform_power_off_handler);+priv->trivial_power_off_cb=power_off;++returnregister_power_off_handler(&priv->power_off_nb);+}++/**+*do_kernel_power_off-Executekernelpower-offhandlercallchain+*+*Callsfunctionsregisteredwithregister_power_off_handler.+*+*Expectedtobecalledaslaststepofthepower-offsequence.+*+*Powersoffthesystemimmediatelyifapower-offhandlerfunctionhas+*beenregistered.Otherwisedoesnothing.+*/+voiddo_kernel_power_off(void)+{+/* legacy pm_power_off() is unchained and it has highest priority */+if(pm_power_off&&pm_power_off!=dummy_pm_power_off)+returnpm_power_off();++blocking_notifier_call_chain(&power_off_handler_list,POWEROFF_NORMAL,+NULL);+}++staticvoiddo_kernel_power_off_prepare(void)+{+/* legacy pm_power_off_prepare() is unchained and it has highest priority */+if(pm_power_off_prepare)+returnpm_power_off_prepare();++blocking_notifier_call_chain(&power_off_handler_list,POWEROFF_PREPARE,+NULL);+}+/***kernel_power_off-power_offthesystem*
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 <digetx@gmail.com>
---
kernel/reboot.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Add sanity check which ensures that there are no two restart handlers
registered with the same priority. Normally it's a direct sign of a
problem if two handlers use the same priority.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
kernel/reboot.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
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 <digetx@gmail.com>
---
arch/csky/kernel/power.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 <digetx@gmail.com>
---
arch/arm64/kernel/process.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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 <digetx@gmail.com>
---
arch/ia64/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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 <digetx@gmail.com>
---
arch/mips/kernel/reset.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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 <digetx@gmail.com>
---
arch/nds32/kernel/process.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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 <digetx@gmail.com>
---
arch/powerpc/kernel/setup-common.c | 4 +---
arch/powerpc/xmon/xmon.c | 3 +--
2 files changed, 2 insertions(+), 5 deletions(-)
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 <digetx@gmail.com>
---
arch/parisc/kernel/process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -114,8 +115,7 @@ void machine_power_off(void)pdc_chassis_send_status(PDC_CHASSIS_DIRECT_SHUTDOWN);/* ipmi_poweroff may have been installed. */-if(pm_power_off)-pm_power_off();+do_kernel_power_off();/* It seems we have no way to power the system off via*software.Theuserhastopressthebuttonhimself.*/
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 <digetx@gmail.com>
---
arch/riscv/kernel/reset.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
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 <digetx@gmail.com>
---
arch/sh/kernel/reboot.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
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 <digetx@gmail.com>
---
arch/x86/kernel/reboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -747,10 +747,10 @@ static void native_machine_halt(void)staticvoidnative_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);
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 <digetx@gmail.com>
---
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(-)
@@ -630,7 +630,7 @@ static irqreturn_t emif_threaded_isr(int irq, void *dev_id)dev_emerg(emif->dev,"SDRAM temperature exceeds operating limit.. Needs shut down!!!\n");/* If we have Power OFF ability, use it, else try restarting */-if(pm_power_off){+if(kernel_can_power_off()){kernel_power_off();}else{WARN(1,"FIXME: NO pm_power_off!!! trying restart\n");
@@ -1016,7 +1008,7 @@ static void acpi_sleep_hibernate_setup(void)staticinlinevoidacpi_sleep_hibernate_setup(void){}#endif /* !CONFIG_HIBERNATION */-staticvoidacpi_power_off_prepare(void)+staticvoidacpi_power_off_prepare(structpower_off_prep_data*data){/* Prepare to power off the system */acpi_sleep_prepare(ACPI_STATE_S5);
@@ -1024,7 +1016,7 @@ static void acpi_power_off_prepare(void)acpi_os_wait_events_complete();}-staticvoidacpi_power_off(void)+staticvoidacpi_power_off(structpower_off_data*data){/* acpi_sleep_prepare(ACPI_STATE_S5) should have already been called */pr_debug("%s called\n",__func__);
@@ -20,7 +20,6 @@*Callbacksforplatformdriverstoimplement.*/externvoid(*pm_power_off)(void);-externvoid(*pm_power_off_prepare)(void);structdevice;/* we have a circular dep with device.h */#ifdef CONFIG_VT_CONSOLE_SLEEP
@@ -48,13 +48,6 @@ int reboot_cpu;enumreboot_typereboot_type=BOOT_ACPI;intreboot_force;-/*-*Ifset,thisisusedforpreparingthesystemtopoweroff.-*/--void(*pm_power_off_prepare)(void);-EXPORT_SYMBOL_GPL(pm_power_off_prepare);-/***emergency_restart-rebootthesystem*
@@ -866,10 +859,6 @@ void do_kernel_power_off(void)staticvoiddo_kernel_power_off_prepare(void){-/* legacy pm_power_off_prepare() is unchained and it has highest priority */-if(pm_power_off_prepare)-returnpm_power_off_prepare();-blocking_notifier_call_chain(&power_off_handler_list,POWEROFF_PREPARE,NULL);}
@@ -569,10 +571,10 @@ static inline struct device_node *match_of_node(int index)returnpfuze_matches[index].of_node;}-staticstructpfuze_chip*syspm_pfuze_chip;--staticvoidpfuze_power_off_prepare(void)+staticvoidpfuze_power_off_prepare(structpower_off_prep_data*data){+structpfuze_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)staticintpfuze_power_off_prepare_init(structpfuze_chip*pfuze_chip){+interr;+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);+returnerr;}-syspm_pfuze_chip=pfuze_chip;-pm_power_off_prepare=pfuze_power_off_prepare;-return0;}
@@ -837,23 +839,12 @@ static int pfuze100_regulator_probe(struct i2c_client *client,return0;}-staticintpfuze100_regulator_remove(structi2c_client*client)-{-if(syspm_pfuze_chip){-syspm_pfuze_chip=NULL;-pm_power_off_prepare=NULL;-}--return0;-}-staticstructi2c_driverpfuze_driver={.driver={.name="pfuze100-regulator",.of_match_table=pfuze_dt_ids,},.probe=pfuze100_regulator_probe,-.remove=pfuze100_regulator_remove,};module_i2c_driver(pfuze_driver);
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 <digetx@gmail.com>
---
drivers/soc/tegra/pmc.c | 54 +++++++++++++++++++++++++++--------------
1 file changed, 36 insertions(+), 18 deletions(-)
@@ -1064,10 +1066,8 @@ int tegra_pmc_cpu_remove_clamping(unsigned int cpuid)returntegra_powergate_remove_clamping(id);}-staticinttegra_pmc_restart_notify(structnotifier_block*this,-unsignedlongaction,void*data)+staticvoidtegra_pmc_restart(constchar*cmd){-constchar*cmd=data;u32value;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);+}-returnNOTIFY_DONE;+staticvoidtegra_pmc_restart_handler(structrestart_data*data)+{+tegra_pmc_restart(data->cmd);}-staticstructnotifier_blocktegra_pmc_restart_handler={-.notifier_call=tegra_pmc_restart_notify,-.priority=128,+staticvoidtegra_pmc_power_off_handler(structpower_off_data*data)+{+/*+*RebootNexus7intospecialbootloadermodeifUSBcableis+*connectedinordertodisplaybatterystatusandpoweroff.+*/+if(of_machine_is_compatible("asus,grouper")&&+power_supply_is_system_supplied()){+constu32go_to_charger_mode=0xa5a55a5a;++tegra_pmc_writel(pmc,go_to_charger_mode,PMC_SCRATCH37);+tegra_pmc_restart(NULL);+}+}++staticstructpower_handlertegra_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,};staticintpowergate_show(structseq_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);+returnerr;+}+/**PCLKclockratecan'tberetrievedusingCLKAPIbecauseit*causeslockupifCPUentersLP2idlestatefromsomeother
@@ -2890,20 +2917,13 @@ static int tegra_pmc_probe(struct platform_device *pdev)gotocleanup_sysfs;}-err=register_restart_handler(&tegra_pmc_restart_handler);-if(err){-dev_err(&pdev->dev,"unable to register restart handler, %d\n",-err);-gotocleanup_debugfs;-}-err=tegra_pmc_pinctrl_init(pmc);if(err)-gotocleanup_restart_handler;+gotocleanup_debugfs;err=tegra_pmc_regmap_init(pmc);if(err<0)-gotocleanup_restart_handler;+gotocleanup_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:
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 <digetx@gmail.com>
---
drivers/mfd/ntxec.c | 50 ++++++++++-----------------------------------
1 file changed, 11 insertions(+), 39 deletions(-)
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 <digetx@gmail.com>
---
drivers/mfd/acer-ec-a500.c | 52 ++++++++++++++------------------------
1 file changed, 19 insertions(+), 33 deletions(-)
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 <digetx@gmail.com>
---
drivers/mfd/rn5t618.c | 56 ++++++++++++++++---------------------------
1 file changed, 21 insertions(+), 35 deletions(-)
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 <digetx@gmail.com>
---
drivers/mfd/ene-kb3930.c | 45 ++++++++++++++--------------------------
1 file changed, 15 insertions(+), 30 deletions(-)
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 <digetx@gmail.com>
---
drivers/mfd/max77620.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
@@ -923,7 +924,7 @@ static int twl4030_power_probe(struct platform_device *pdev)}/* Board has to be wired properly to use this feature */-if(twl4030_power_use_poweroff(pdata,node)&&!pm_power_off){+if(twl4030_power_use_poweroff(pdata,node)){/* Default for SEQ_OFFSYNC is set, lets ensure this */err=twl_i2c_read_u8(TWL_MODULE_PM_MASTER,&val,TWL4030_PM_MASTER_CFG_P123_TRANSITION);
@@ -939,7 +940,12 @@ static int twl4030_power_probe(struct platform_device *pdev)}}-pm_power_off=twl4030_power_off;+err=devm_register_trivial_power_off_handler(&pdev->dev,+twl4030_power_off);+if(err){+dev_err(&pdev->dev,"Failed to register power-off handler\n");+gotorelock;+}}relock:
On Wed, 27 Oct 2021 14:16:48 PDT (-0700), digetx@gmail.com wrote:
quoted hunk
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 <digetx@gmail.com>
---
arch/riscv/kernel/reset.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
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 <digetx@gmail.com>
@@ -114,8 +115,7 @@ void machine_power_off(void)pdc_chassis_send_status(PDC_CHASSIS_DIRECT_SHUTDOWN);/* ipmi_poweroff may have been installed. */-if(pm_power_off)-pm_power_off();+do_kernel_power_off();/* It seems we have no way to power the system off via*software.Theuserhastopressthebuttonhimself.*/
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 <digetx@gmail.com>
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2021-10-28 09:53:40
On Wed, Oct 27, 2021 at 11:18 PM Dmitry Osipenko [off-list ref] wrote:
quoted hunk
SoC platforms often have multiple options of how to perform system's
power-off and restart operations. Meanwhile today's kernel is limited to
a single option. Add combined power-off+restart handler call chain API,
which is inspired by the restart API. The new API provides both power-off
and restart functionality.
The old pm_power_off method will be kept around till all users are
converted to the new API.
Current restart API will be replaced by the new unified API since
new API is its superset. The restart functionality of the power-handler
API is built upon the existing restart-notifier APIs.
In order to ease conversion to the new API, convenient helpers are added
for the common use-cases. They will reduce amount of boilerplate code and
remove global variables. These helpers preserve old behaviour for cases
where only one power-off handler is executed, this is what existing
drivers want, and thus, they could be easily converted to the new API.
Users of the new API should explicitly enable power-off chaining by
setting corresponding flag of the power_handler structure.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
include/linux/reboot.h | 176 +++++++++++-
kernel/power/hibernate.c | 2 +-
kernel/reboot.c | 601 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 768 insertions(+), 11 deletions(-)
@@ -8,10 +8,16 @@structdevice;-#define SYS_DOWN 0x0001 /* Notify of system down */-#define SYS_RESTART SYS_DOWN-#define SYS_HALT 0x0002 /* Notify of system halt */-#define SYS_POWER_OFF 0x0003 /* Notify of system power off */+enumreboot_prepare_mode{+SYS_DOWN=1,/* Notify of system down */+SYS_RESTART=SYS_DOWN,+SYS_HALT,/* Notify of system halt */+SYS_POWER_OFF,/* Notify of system power off */+};++#define RESTART_PRIO_RESERVED 0+#define RESTART_PRIO_DEFAULT 128+#define RESTART_PRIO_HIGH 192enumreboot_mode{REBOOT_UNDEFINED=-1,
The name of this structure is too generic IMV. There are many things
that it might apply to in principle.
What about calling power_off_handler or sys_off_handler as it need not
be about power at all?
From: "Rafael J. Wysocki" <rafael@kernel.org> Date: 2021-10-28 09:59:45
On Wed, Oct 27, 2021 at 11:18 PM Dmitry Osipenko [off-list ref] wrote:
quoted hunk
SoC platforms often have multiple options of how to perform system's
power-off and restart operations. Meanwhile today's kernel is limited to
a single option. Add combined power-off+restart handler call chain API,
which is inspired by the restart API. The new API provides both power-off
and restart functionality.
The old pm_power_off method will be kept around till all users are
converted to the new API.
Current restart API will be replaced by the new unified API since
new API is its superset. The restart functionality of the power-handler
API is built upon the existing restart-notifier APIs.
In order to ease conversion to the new API, convenient helpers are added
for the common use-cases. They will reduce amount of boilerplate code and
remove global variables. These helpers preserve old behaviour for cases
where only one power-off handler is executed, this is what existing
drivers want, and thus, they could be easily converted to the new API.
Users of the new API should explicitly enable power-off chaining by
setting corresponding flag of the power_handler structure.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
include/linux/reboot.h | 176 +++++++++++-
kernel/power/hibernate.c | 2 +-
kernel/reboot.c | 601 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 768 insertions(+), 11 deletions(-)
@@ -8,10 +8,16 @@structdevice;-#define SYS_DOWN 0x0001 /* Notify of system down */-#define SYS_RESTART SYS_DOWN-#define SYS_HALT 0x0002 /* Notify of system halt */-#define SYS_POWER_OFF 0x0003 /* Notify of system power off */+enumreboot_prepare_mode{+SYS_DOWN=1,/* Notify of system down */+SYS_RESTART=SYS_DOWN,+SYS_HALT,/* Notify of system halt */+SYS_POWER_OFF,/* Notify of system power off */+};++#define RESTART_PRIO_RESERVED 0+#define RESTART_PRIO_DEFAULT 128+#define RESTART_PRIO_HIGH 192enumreboot_mode{REBOOT_UNDEFINED=-1,
Also I'm wondering why these particular numbers were chosen, here and above?
+
+enum poweroff_mode {
+ POWEROFF_NORMAL = 0,
+ POWEROFF_PREPARE,
+};
+
+struct power_off_data {
+ void *cb_data;
+};
+
+struct power_off_prep_data {
+ void *cb_data;
+};
+
+struct restart_data {
+ void *cb_data;
+ const char *cmd;
+ enum reboot_mode mode;
+};
+
+struct reboot_prep_data {
+ void *cb_data;
+ const char *cmd;
+ enum reboot_prepare_mode mode;
+};
+
+struct power_handler_private_data {
+ struct notifier_block reboot_prep_nb;
+ struct notifier_block power_off_nb;
+ struct notifier_block restart_nb;
+ void (*trivial_power_off_cb)(void);
+ void (*simple_power_off_cb)(void *data);
+ void *simple_power_off_cb_data;
+ bool registered;
+};
+
+/**
+ * struct power_handler - Machine power-off + restart handler
+ *
+ * Describes power-off and restart handlers which are invoked by kernel
+ * to power off or restart this machine. Supports prioritized chaining for
+ * both restart and power-off handlers. Callback's priority must be unique.
+ * Intended to be used by device drivers that are responsible for restarting
+ * and powering off hardware which kernel is running on.
+ *
+ * Struct power_handler can be static. Members of this structure must not be
+ * altered while handler is registered.
+ *
+ * Fill the structure members and pass it to register_power_handler().
+ */
+struct power_handler {
+ /**
+ * @cb_data:
+ *
+ * User data included in callback's argument.
+ */
And here I would document the structure fields in the main kerneldoc
comment above.
As is, it is a bit hard to grasp the whole definition.
+ void *cb_data;
+
+ /**
+ * @power_off_cb:
+ *
+ * Callback that should turn off machine. Inactive if NULL.
+ */
+ void (*power_off_cb)(struct power_off_data *data);
+
+ /**
+ * @power_off_prepare_cb:
+ *
+ * Power-off preparation callback. All power-off preparation callbacks
+ * are invoked before @restart_cb. Inactive if NULL.
+ */
+ void (*power_off_prepare_cb)(struct power_off_prep_data *data);
+
+ /**
+ * @power_off_priority:
+ *
+ * Power-off callback priority, must be unique. Zero value is
+ * reassigned to default priority. Inactive if @power_off_cb is NULL.
+ */
+ int power_off_priority;
+
+ /**
+ * @power_off_chaining_allowed:
+ *
+ * False if callbacks execution should stop when @power_off_cb fails
+ * to power off machine. True if further lower priority power-off
+ * callback should be executed.
+ */
+ bool power_off_chaining_allowed;
+
+ /**
+ * @restart_cb:
+ *
+ * Callback that should reboot machine. Inactive if NULL.
+ */
+ void (*restart_cb)(struct restart_data *data);
+
+ /**
+ * @restart_priority:
+ *
+ * Restart callback priority, must be unique. Zero value is reassigned
+ * to default priority. Inactive if @restart_cb is NULL.
+ */
+ int restart_priority;
+
+ /**
+ * @reboot_prepare_cb:
+ *
+ * Reboot preparation callback. All reboot preparation callbacks are
+ * invoked before @restart_cb. Inactive if NULL.
+ */
+ void (*reboot_prepare_cb)(struct reboot_prep_data *data);
+
+ /**
+ * @priv:
+ *
+ * Internal data. Shouldn't be touched.
+ */
+ const struct power_handler_private_data priv;
+};
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Date: 2021-10-28 11:01:15
On Thu, Oct 28, 2021 at 12:16:33AM +0300, Dmitry Osipenko wrote:
Add atomic/blocking_notifier_has_unique_priority() helpers which return
true if given handler has unique priority.
...
+/**
+ * atomic_notifier_has_unique_priority - Checks whether notifier's priority is unique
+ * @nh: Pointer to head of the atomic notifier chain
+ * @n: Entry in notifier chain to check
+ *
+ * Checks whether there is another notifier in the chain with the same priority.
+ * Must be called in process context.
+ *
+ * Returns true if priority is unique, false otherwise.
From: "Russell King (Oracle)" <linux@armlinux.org.uk> Date: 2021-10-28 12:06:47
On Thu, Oct 28, 2021 at 12:16:40AM +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 <digetx@gmail.com>
Reviewed-by: Russell King (Oracle) <redacted>
Thanks!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
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 <digetx@gmail.com>
---
arch/nds32/kernel/process.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
On Thu, Oct 28, 2021 at 12:16:33AM +0300, Dmitry Osipenko wrote:
quoted
Add atomic/blocking_notifier_has_unique_priority() helpers which return
true if given handler has unique priority.
...
quoted
+/**
+ * atomic_notifier_has_unique_priority - Checks whether notifier's priority is unique
+ * @nh: Pointer to head of the atomic notifier chain
+ * @n: Entry in notifier chain to check
+ *
+ * Checks whether there is another notifier in the chain with the same priority.
+ * Must be called in process context.
+ *
+ * Returns true if priority is unique, false otherwise.
Why this indentation?
This is the same doc-comment style used by this file in general. I
haven't tried to invent anything new.
...
quoted
+ /*
+ * This code gets used during boot-up, when task switching is
+ * not yet working and interrupts must remain disabled. At
One space is enough.
This comment is replicated multiple times over this source file. You can
find it before each down_write(). I borrowed the text as-is, for
consistency.
+/**
+ * struct power_handler - Machine power-off + restart handler
+ *
+ * Describes power-off and restart handlers which are invoked by kernel
+ * to power off or restart this machine. Supports prioritized chaining for
+ * both restart and power-off handlers. Callback's priority must be unique.
+ * Intended to be used by device drivers that are responsible for restarting
+ * and powering off hardware which kernel is running on.
+ *
+ * Struct power_handler can be static. Members of this structure must not be
+ * altered while handler is registered.
+ *
+ * Fill the structure members and pass it to register_power_handler().
+ */
+struct power_handler {
The name of this structure is too generic IMV. There are many things
that it might apply to in principle.
What about calling power_off_handler or sys_off_handler as it need not
be about power at all?
I didn't like much the 'power' either, but couldn't come up with a
better variant. Will change it in v3, thank you.
Also I'm wondering why these particular numbers were chosen, here and above?
These values are chosen based on priorities that drivers already use. I looked thorough them all and ended with this scheme that fulfills the needs of the current API users.
I'll add these comments in v3:
/*
* Standard restart priority levels. Intended to be set in the
* sys_off_handler.restart_priority field.
*
* Use `RESTART_PRIO_XXX +- prio` style for additional levels.
*
* RESTART_PRIO_RESERVED: Falls back to RESTART_PRIO_DEFAULT.
* Drivers may leave priority initialized
* to zero, to auto-set it to the default level.
*
* RESTART_PRIO_DEFAULT: Use this for generic handler.
*
* RESTART_PRIO_HIGH: Use this if you have multiple handlers and
* this handler has higher priority than the
* default handler.
*/
/*
* Standard power-off priority levels. Intended to be set in the
* sys_off_handler.power_off_priority field.
*
* Use `POWEROFF_PRIO_XXX +- prio` style for additional levels.
*
* POWEROFF_PRIO_RESERVED: Falls back to POWEROFF_PRIO_DEFAULT.
* Drivers may leave priority initialized
* to zero, to auto-set it to the default level.
*
* POWEROFF_PRIO_PLATFORM: Intended to be used by platform-level handler.
* Has lowest priority since device drivers are
* expected to take over platform handler which
* doesn't allow further callback chaining.
*
* POWEROFF_PRIO_DEFAULT: Use this for generic handler.
*
* POWEROFF_PRIO_HIGH: Use this if you have multiple handlers and
* this handler has higher priority than the
* default handler.
*
* POWEROFF_PRIO_FIRMWARE: Use this if handler uses firmware call.
* Has highest priority since firmware is expected
* to know best how to power-off hardware properly.
*/
+/**
+ * struct power_handler - Machine power-off + restart handler
+ *
+ * Describes power-off and restart handlers which are invoked by kernel
+ * to power off or restart this machine. Supports prioritized chaining for
+ * both restart and power-off handlers. Callback's priority must be unique.
+ * Intended to be used by device drivers that are responsible for restarting
+ * and powering off hardware which kernel is running on.
+ *
+ * Struct power_handler can be static. Members of this structure must not be
+ * altered while handler is registered.
+ *
+ * Fill the structure members and pass it to register_power_handler().
+ */
+struct power_handler {
+ /**
+ * @cb_data:
+ *
+ * User data included in callback's argument.
+ */
And here I would document the structure fields in the main kerneldoc
comment above.
As is, it is a bit hard to grasp the whole definition.
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 [off-list ref]
On Thu, Oct 28, 2021 at 5:18 AM Dmitry Osipenko [off-list ref] wrote:
quoted hunk
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 <digetx@gmail.com>
---
arch/csky/kernel/power.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
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 <digetx@gmail.com>
From: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Date: 2021-11-06 20:56:45
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 <digetx@gmail.com>
---
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 <j.neuschaefer@gmx.net>
Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
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 ]---
Hi,
On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
quoted
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 <digetx@gmail.com>
---
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 <j.neuschaefer@gmx.net>
Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
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.
Hi,
On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
quoted
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 <digetx@gmail.com>
---
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 <j.neuschaefer@gmx.net>
Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
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
Hi,
On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
quoted
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 <digetx@gmail.com>
---
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 <j.neuschaefer@gmx.net>
Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
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.
Hi,
On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
quoted
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 <digetx@gmail.com>
---
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 <j.neuschaefer@gmx.net>
Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
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
Hi,
On Thu, Oct 28, 2021 at 12:16:57AM +0300, Dmitry Osipenko wrote:
quoted
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 <digetx@gmail.com>
---
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 <j.neuschaefer@gmx.net>
Tested-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
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?
From: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Date: 2021-11-08 11:25:07
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 Sun, Nov 07, 2021 at 08:42:33PM +0300, Dmitry Osipenko wrote:
[...]
quoted
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.
From: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Date: 2021-11-10 10:46:12
On Mon, Nov 08, 2021 at 02:36:42PM +0300, Dmitry Osipenko wrote:
08.11.2021 14:22, Jonathan Neuschäfer пишет:
quoted
On Sun, Nov 07, 2021 at 08:42:33PM +0300, Dmitry Osipenko wrote:
[...]
quoted
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.
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:
On Mon, Nov 08, 2021 at 02:36:42PM +0300, Dmitry Osipenko wrote:
quoted
08.11.2021 14:22, Jonathan Neuschäfer пишет:
quoted
On Sun, Nov 07, 2021 at 08:42:33PM +0300, Dmitry Osipenko wrote:
[...]
quoted
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.
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:
From: Lee Jones <hidden> Date: 2021-11-29 13:12:58
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 <digetx@gmail.com>
---
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 [off-list ref]
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
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 <digetx@gmail.com>
---
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 [off-list ref]
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.