| Message ID | 20250113-hw_protection-reboot-v2-1-161d3fc734f0@pengutronix.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | reboot: support runtime configuration of emergency hw_protection action | expand |
On Mon, Jan 13, 2025 at 05:25:26PM +0100, Ahmad Fatoum wrote: > Currently __hw_protection_shutdown() either reboots or shuts down the > system according to its shutdown argument. > > To make the logic easier to follow, both inside __hw_protection_shutdown > and at caller sites, lets replace the bool parameter with an enum. > > This will be extra useful, when in a later commit, a third action is > added to the enumeration. > > No functional change. > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> With a minor question, Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org> > @@ -1009,10 +1007,10 @@ void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shut > * orderly_poweroff failure > */ > hw_failure_emergency_poweroff(ms_until_forced); > - if (shutdown) > - orderly_poweroff(true); > - else > + if (action == HWPROT_ACT_REBOOT) > orderly_reboot(); > + else > + orderly_poweroff(true); It probably doesn't really matter. Does it intend to change the branch order? As s/shutdown/action == HWPROT_ACT_SHUTDOWN/ should be more intuitive for the hunk to me.
Hello, On 20.01.25 08:10, Tzung-Bi Shih wrote: > On Mon, Jan 13, 2025 at 05:25:26PM +0100, Ahmad Fatoum wrote: >> Currently __hw_protection_shutdown() either reboots or shuts down the >> system according to its shutdown argument. >> >> To make the logic easier to follow, both inside __hw_protection_shutdown >> and at caller sites, lets replace the bool parameter with an enum. >> >> This will be extra useful, when in a later commit, a third action is >> added to the enumeration. >> >> No functional change. >> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > > With a minor question, > Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org> Thanks for your review. >> @@ -1009,10 +1007,10 @@ void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shut >> * orderly_poweroff failure >> */ >> hw_failure_emergency_poweroff(ms_until_forced); >> - if (shutdown) >> - orderly_poweroff(true); >> - else >> + if (action == HWPROT_ACT_REBOOT) >> orderly_reboot(); >> + else >> + orderly_poweroff(true); > > It probably doesn't really matter. Does it intend to change the branch > order? As s/shutdown/action == HWPROT_ACT_SHUTDOWN/ should be more > intuitive for the hunk to me. My thinking was that having poweroff in the else branch underlines that it's the default, i.e. either reboot was explicitly asked for or we fall back to the poweroff default. I see that this is subjective. I can change it for v3 if that's preferred. Cheers, Ahmad >
diff --git a/include/linux/reboot.h b/include/linux/reboot.h index abcdde4df697969a8027bcb052efc00daabbbf6a..e97f6b8e858685b4b527daa8920a31eabcf91622 100644 --- a/include/linux/reboot.h +++ b/include/linux/reboot.h @@ -177,16 +177,28 @@ void ctrl_alt_del(void); extern void orderly_poweroff(bool force); extern void orderly_reboot(void); -void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shutdown); + +/** + * enum hw_protection_action - Hardware protection action + * + * @HWPROT_ACT_SHUTDOWN: + * The system should be shut down (powered off) for HW protection. + * @HWPROT_ACT_REBOOT: + * The system should be rebooted for HW protection. + */ +enum hw_protection_action { HWPROT_ACT_SHUTDOWN, HWPROT_ACT_REBOOT }; + +void __hw_protection_shutdown(const char *reason, int ms_until_forced, + enum hw_protection_action action); static inline void hw_protection_reboot(const char *reason, int ms_until_forced) { - __hw_protection_shutdown(reason, ms_until_forced, false); + __hw_protection_shutdown(reason, ms_until_forced, HWPROT_ACT_REBOOT); } static inline void hw_protection_shutdown(const char *reason, int ms_until_forced) { - __hw_protection_shutdown(reason, ms_until_forced, true); + __hw_protection_shutdown(reason, ms_until_forced, HWPROT_ACT_SHUTDOWN); } /* diff --git a/kernel/reboot.c b/kernel/reboot.c index a701000bab3470df28665e8c9591cd82a033c6c2..847ac5d17a659981c6765699eac323f5e87f48c1 100644 --- a/kernel/reboot.c +++ b/kernel/reboot.c @@ -983,10 +983,7 @@ static void hw_failure_emergency_poweroff(int poweroff_delay_ms) * @ms_until_forced: Time to wait for orderly shutdown or reboot before * triggering it. Negative value disables the forced * shutdown or reboot. - * @shutdown: If true, indicates that a shutdown will happen - * after the critical tempeature is reached. - * If false, indicates that a reboot will happen - * after the critical tempeature is reached. + * @action: The hardware protection action to be taken. * * Initiate an emergency system shutdown or reboot in order to protect * hardware from further damage. Usage examples include a thermal protection. @@ -994,7 +991,8 @@ static void hw_failure_emergency_poweroff(int poweroff_delay_ms) * pending even if the previous request has given a large timeout for forced * shutdown/reboot. */ -void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shutdown) +void __hw_protection_shutdown(const char *reason, int ms_until_forced, + enum hw_protection_action action) { static atomic_t allow_proceed = ATOMIC_INIT(1); @@ -1009,10 +1007,10 @@ void __hw_protection_shutdown(const char *reason, int ms_until_forced, bool shut * orderly_poweroff failure */ hw_failure_emergency_poweroff(ms_until_forced); - if (shutdown) - orderly_poweroff(true); - else + if (action == HWPROT_ACT_REBOOT) orderly_reboot(); + else + orderly_poweroff(true); } EXPORT_SYMBOL_GPL(__hw_protection_shutdown);
Currently __hw_protection_shutdown() either reboots or shuts down the system according to its shutdown argument. To make the logic easier to follow, both inside __hw_protection_shutdown and at caller sites, lets replace the bool parameter with an enum. This will be extra useful, when in a later commit, a third action is added to the enumeration. No functional change. Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- include/linux/reboot.h | 18 +++++++++++++++--- kernel/reboot.c | 14 ++++++-------- 2 files changed, 21 insertions(+), 11 deletions(-)