[PATCH v7 0/9] Extend regulator notification support

[PATCH v7 0/9] Extend regulator notification support

[Matti Vaittinen]

Extend regulator notification support

This series extends the regulator notification and error flag support.
Initial discussion on the topic can be found here:
https://lore.kernel.org/lkml/6046836e22b8252983f08d5621c35ececb97820d.camel@fi.rohmeurope.com/

This series is built on top of the BD9576MUF support patch series v9
which is currently in MFD tree at immutable branch ib-mfd-watchdog-5.13
https://lore.kernel.org/lkml/cover.1615219345.git.matti.vaittinen@fi.rohmeurope.com/
(The series should apply without those patches but there is compile time
dependency to definitions brought in at the last patch of the BD9576
series. This should be Ok though as there is a Kconfig dependency in
BD9576 regulator driver)

In a nutshell - the series adds:

1. WARNING level events/error flags. (Patch 3)
  Current regulator 'ERROR' event notifications for over/under
  voltage, over current and over temperature are used to indicate
  condition where monitored entity is so badly "off" that it actually
  indicates a hardware error which can not be recovered. The most
  typical hanling for that is believed to be a (graceful)
  system-shutdown. Here we add set of 'WARNING' level flags to allow
  sending notifications to consumers before things are 'that badly off'
  so that consumer drivers can implement recovery-actions.
2. Device-tree properties for specifying limit values. (Patches 1, 5)
  Add limits for above mentioned 'ERROR' and 'WARNING' levels (which
  send notifications to consumers) and also for a 'PROTECTION' level
  (which will be used to immediately shut-down the regulator(s) W/O
  informing consumer drivers. Typically implemented by hardware).
  Property parsing is implemented in regulator core which then calls
  callback operations for limit setting from the IC drivers. A
  warning is emitted if protection is requested by device tree but the
  underlying IC does not support configuring requested protection.
3. Helpers which can be registered by IC. (Patch 4)
  Target is to avoid implementing IRQ handling and IRQ storm protection
  in each IC driver. (Many of the ICs implementin these IRQs do not allow
  masking or acking the IRQ but keep the IRQ asserted for the whole
  duration of problem keeping the processor in IRQ handling loop).
4. Emergency poweroff function (refactored out of the thermal_core to
  kernel/reboot.c) which is called if IC fires error IRQs but IC reading
  fails and given retry-count is exceeded. (Patches 2, 4)
  Please note that the mutex in the emergency shutdown was replaced by a
  spinlock to allow calls from any context.

The helper was attempted to be done so it could be used to implement
roughly same logic as is used in qcom-labibb regulator. This means
amongst other things a safety shut-down if IC registers are not readable.
Using these shut-down retry counters are optional. The idea is that the
helper could be also used by simpler ICs which do not provide status
register(s) which can be used to check if error is still active.

ICs which do not have such status register can simply omit the 'renable'
callback (and retry-counts etc) - and helper assumes the situation is Ok
and re-enables IRQ after given time period. If problem persists the
handler is ran again and another notification is sent - but at least the
delay allows processor to avoid IRQ loop.

Patch 7 takes this notification support in use at BD9576MUF.
Patch 8 is related to MFD change which is not really related to the RFC
here. It was added to this series in order to avoid potential conflicts.
Patch 9 adds a maintainers entry.

Changelog v7:
  general:
   - rebased on v5.12-rc7
   - new patch for refactoring the hw-failure reboot logic out of
     thermal_core.c for others to use.
  notification helpers:
   - fix regulator error_flags query
   - grammar/typos
   - do not BUG() but attempt to shut-down the system
   - use BITS_PER_TYPE()

Changelog v6:
  Add MAINTAINERS entry
  Changes to IRQ notifiers
   - move devm functions to drivers/regulator/devres.c
   - drop irq validity check
   - use devm_add_action_or_reset()
   - fix styling issues
   - fix kerneldocs

Changelog v5:
   - Fix the badly formatted pr_emerg() call.

Changelog v4:
   - rebased on v5.12-rc6
   - dropped RFC
   - fix external FET DT-binding.
   - improve prints for cases when expecting HW failure.
   - styling and typos

Changelog v3:
  Regulator core:
   - Fix dangling pointer access at regulator_irq_helper()
  stpmic1_regulator:
   - fix function prototype (compile error)
  bd9576-regulator:
   - Update over current limits to what was given in new data-sheet
     (REV00K)
   - Allow over-current monitoring without external FET. Set limits to
     values given in data-sheet (REV00K).

Changelog v2:
  Generic:
  - rebase on v5.12-rc2 + BD9576 series
  - Split devm variant of delayed wq to own series
  Regulator framework:
  - Provide non devm variant of IRQ notification helpers
  - shorten dt-property names as suggested by Rob
  - unconditionally call map_event in IRQ handling and require it to be
    populated
  BD9576 regulators:
  - change the FET resistance property to micro-ohms
  - fix voltage computation in OC limit setting

--

Matti Vaittinen (9):
  dt_bindings: Add protection limit properties
  reboot: thermal: Export hardware protection shutdown
  regulator: add warning flags
  regulator: IRQ based event/error notification helpers
  regulator: add property parsing and callbacks to set protection limits
  dt-bindings: regulator: bd9576 add FET ON-resistance for OCW
  regulator: bd9576: Support error reporting
  regulator: bd9576: Fix the driver name in id table
  MAINTAINERS: Add reviewer for regulator irq_helpers

 .../bindings/regulator/regulator.yaml         |   82 ++
 .../regulator/rohm,bd9576-regulator.yaml      |    6 +
 MAINTAINERS                                   |    4 +
 drivers/regulator/Makefile                    |    2 +-
 drivers/regulator/bd9576-regulator.c          | 1060 +++++++++++++++--
 drivers/regulator/core.c                      |  151 ++-
 drivers/regulator/devres.c                    |   52 +
 drivers/regulator/irq_helpers.c               |  394 ++++++
 drivers/regulator/of_regulator.c              |   58 +
 drivers/regulator/qcom-labibb-regulator.c     |   10 +-
 drivers/regulator/qcom_spmi-regulator.c       |    6 +-
 drivers/regulator/stpmic1_regulator.c         |   20 +-
 drivers/thermal/thermal_core.c                |   63 +-
 include/linux/reboot.h                        |    1 +
 include/linux/regulator/consumer.h            |   14 +
 include/linux/regulator/driver.h              |  176 ++-
 include/linux/regulator/machine.h             |   26 +
 kernel/reboot.c                               |   86 ++
 18 files changed, 2006 insertions(+), 205 deletions(-)
 create mode 100644 drivers/regulator/irq_helpers.c


base-commit: d434405aaab7d0ebc516b68a8fc4100922d7f5ef
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

[PATCH v7 2/9] reboot: thermal: Export hardware protection shutdown

[Matti Vaittinen]

Thermal core contains a logic for safety shutdown. System is attempted to
be powered off if temperature exceeds safety limits.

Currently this can be also utilized by regulator subsystem as a final
protection measure if PMICs report dangerous over-voltage, over-current or
over-temperature and if per regulator counter measures fail or do not
exist.

Move this logic to kernel/reboot.c and export the functionality for other
subsystems to use. Also replace the mutex with a spinlock to allow using
the function from any context.

Also the EMIF bus code has implemented a safety shut-down. EMIF does not
attempt orderly_poweroff at all. Thus the EMIF code is not converted to use
this new function.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changelog
 v7:
  - new patch

Please note - this patch has received only a minimal amount of testing.
(The new API call was tested to shut-down my system at driver probe but
no odd corner-cases have been tested).

Any testing for thermal shutdown is appreciated.
---
 drivers/thermal/thermal_core.c | 63 ++-----------------------
 include/linux/reboot.h         |  1 +
 kernel/reboot.c                | 86 ++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 59 deletions(-)

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 996c038f83a4..b1444845af38 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -36,10 +36,8 @@ static LIST_HEAD(thermal_governor_list);
 
 static DEFINE_MUTEX(thermal_list_lock);
 static DEFINE_MUTEX(thermal_governor_lock);
-static DEFINE_MUTEX(poweroff_lock);
 
 static atomic_t in_suspend;
-static bool power_off_triggered;
 
 static struct thermal_governor *def_governor;
 
@@ -327,70 +325,18 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz, int trip)
 		       def_governor->throttle(tz, trip);
 }
 
-/**
- * thermal_emergency_poweroff_func - emergency poweroff work after a known delay
- * @work: work_struct associated with the emergency poweroff function
- *
- * This function is called in very critical situations to force
- * a kernel poweroff after a configurable timeout value.
- */
-static void thermal_emergency_poweroff_func(struct work_struct *work)
-{
-	/*
-	 * We have reached here after the emergency thermal shutdown
-	 * Waiting period has expired. This means orderly_poweroff has
-	 * not been able to shut off the system for some reason.
-	 * Try to shut down the system immediately using kernel_power_off
-	 * if populated
-	 */
-	WARN(1, "Attempting kernel_power_off: Temperature too high\n");
-	kernel_power_off();
-
-	/*
-	 * Worst of the worst case trigger emergency restart
-	 */
-	WARN(1, "Attempting emergency_restart: Temperature too high\n");
-	emergency_restart();
-}
-
-static DECLARE_DELAYED_WORK(thermal_emergency_poweroff_work,
-			    thermal_emergency_poweroff_func);
-
-/**
- * thermal_emergency_poweroff - Trigger an emergency system poweroff
- *
- * This may be called from any critical situation to trigger a system shutdown
- * after a known period of time. By default this is not scheduled.
- */
-static void thermal_emergency_poweroff(void)
+void thermal_zone_device_critical(struct thermal_zone_device *tz)
 {
-	int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
 	/*
 	 * poweroff_delay_ms must be a carefully profiled positive value.
-	 * Its a must for thermal_emergency_poweroff_work to be scheduled
+	 * Its a must for forced_emergency_poweroff_work to be scheduled.
 	 */
-	if (poweroff_delay_ms <= 0)
-		return;
-	schedule_delayed_work(&thermal_emergency_poweroff_work,
-			      msecs_to_jiffies(poweroff_delay_ms));
-}
+	int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
 
-void thermal_zone_device_critical(struct thermal_zone_device *tz)
-{
 	dev_emerg(&tz->device, "%s: critical temperature reached, "
 		  "shutting down\n", tz->type);
 
-	mutex_lock(&poweroff_lock);
-	if (!power_off_triggered) {
-		/*
-		 * Queue a backup emergency shutdown in the event of
-		 * orderly_poweroff failure
-		 */
-		thermal_emergency_poweroff();
-		orderly_poweroff(true);
-		power_off_triggered = true;
-	}
-	mutex_unlock(&poweroff_lock);
+	hw_protection_shutdown("Temperature too high", poweroff_delay_ms);
 }
 EXPORT_SYMBOL(thermal_zone_device_critical);
 
@@ -1549,7 +1495,6 @@ static int __init thermal_init(void)
 	ida_destroy(&thermal_cdev_ida);
 	mutex_destroy(&thermal_list_lock);
 	mutex_destroy(&thermal_governor_lock);
-	mutex_destroy(&poweroff_lock);
 	return result;
 }
 postcore_initcall(thermal_init);
diff --git a/include/linux/reboot.h b/include/linux/reboot.h
index 3734cd8f38a8..af907a3d68d1 100644
--- a/include/linux/reboot.h
+++ b/include/linux/reboot.h
@@ -79,6 +79,7 @@ extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
 
 extern void orderly_poweroff(bool force);
 extern void orderly_reboot(void);
+void hw_protection_shutdown(const char *reason, int ms_until_forced);
 
 /*
  * Emergency restart, callable from an interrupt handler.
diff --git a/kernel/reboot.c b/kernel/reboot.c
index a6ad5eb2fa73..1b5fa6d213d4 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -518,6 +518,92 @@ void orderly_reboot(void)
 }
 EXPORT_SYMBOL_GPL(orderly_reboot);
 
+/**
+ * hw_failure_emergency_poweroff_func - emergency poweroff work after a known delay
+ * @work: work_struct associated with the emergency poweroff function
+ *
+ * This function is called in very critical situations to force
+ * a kernel poweroff after a configurable timeout value.
+ */
+static void hw_failure_emergency_poweroff_func(struct work_struct *work)
+{
+	/*
+	 * We have reached here after the emergency shutdown waiting period has
+	 * expired. This means orderly_poweroff has not been able to shut off
+	 * the system for some reason.
+	 *
+	 * Try to shut down the system immediately using kernel_power_off
+	 * if populated
+	 */
+	WARN(1, "Hardware protection timed-out. Trying forced poweroff\n");
+	kernel_power_off();
+
+	/*
+	 * Worst of the worst case trigger emergency restart
+	 */
+	WARN(1,
+	     "Hardware protection shutdown failed. Trying emergency restart\n");
+	emergency_restart();
+}
+
+static DECLARE_DELAYED_WORK(hw_failure_emergency_poweroff_work,
+			    hw_failure_emergency_poweroff_func);
+
+/**
+ * hw_failure_emergency_poweroff - Trigger an emergency system poweroff
+ *
+ * This may be called from any critical situation to trigger a system shutdown
+ * after a given period of time. If time is negative this is not scheduled.
+ */
+static void hw_failure_emergency_poweroff(int poweroff_delay_ms)
+{
+	if (poweroff_delay_ms <= 0)
+		return;
+	schedule_delayed_work(&hw_failure_emergency_poweroff_work,
+			      msecs_to_jiffies(poweroff_delay_ms));
+}
+
+static bool prot_power_off_triggered;
+static DEFINE_SPINLOCK(poweroff_lock);
+
+/**
+ * hw_protection_shutdown - Trigger an emergency system poweroff
+ *
+ * @reason:		Reason of emergency shutdown to be printed.
+ * @ms_until_forced:	Time to wait for orderly shutdown before tiggering a
+ *			forced shudown. Negative value disables the forced
+ *			shutdown.
+ *
+ * Initiate an emergency system shutdown in order to protect hardware from
+ * further damage. Usage examples include a thermal protection or a voltage or
+ * current regulator failures.
+ * NOTE: The request is ignored if protection shutdown is already pending even
+ * if the previous request has given a large timeout for forced shutdown.
+ * Can be called from any context.
+ */
+void hw_protection_shutdown(const char *reason, int ms_until_forced)
+{
+	unsigned long flags;
+
+	pr_emerg("HARDWARE PROTECTION shutdown (%s)\n", reason);
+
+	spin_lock_irqsave(&poweroff_lock, flags);
+	if (prot_power_off_triggered) {
+		spin_unlock(&poweroff_lock);
+		return;
+	}
+	prot_power_off_triggered = true;
+	spin_unlock_irqrestore(&poweroff_lock, flags);
+
+	/*
+	 * Queue a backup emergency shutdown in the event of
+	 * orderly_poweroff failure
+	 */
+	hw_failure_emergency_poweroff(ms_until_forced);
+	orderly_poweroff(true);
+}
+EXPORT_SYMBOL_GPL(hw_protection_shutdown);
+
 static int __init reboot_setup(char *str)
 {
 	for (;;) {
-- 
2.25.4


-- 
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =] 

Re: [PATCH v7 2/9] reboot: thermal: Export hardware protection shutdown

[Daniel Lezcano]

On 14/04/2021 07:52, Matti Vaittinen wrote:
> Thermal core contains a logic for safety shutdown. System is attempted to
> be powered off if temperature exceeds safety limits.
> 
> Currently this can be also utilized by regulator subsystem as a final
> protection measure if PMICs report dangerous over-voltage, over-current or
> over-temperature and if per regulator counter measures fail or do not
> exist.
> 
> Move this logic to kernel/reboot.c and export the functionality for other
> subsystems to use. Also replace the mutex with a spinlock to allow using
> the function from any context.
> 
> Also the EMIF bus code has implemented a safety shut-down. EMIF does not
> attempt orderly_poweroff at all. Thus the EMIF code is not converted to use
> this new function.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> Changelog
>  v7:
>   - new patch
> 
> Please note - this patch has received only a minimal amount of testing.
> (The new API call was tested to shut-down my system at driver probe but
> no odd corner-cases have been tested).
> 
> Any testing for thermal shutdown is appreciated.

You can test it easily by enabling the option CONFIG_THERMAL_EMULATION

Then in any thermal zone:

Assuming the critical temp is below the one specified in the command:

echo 100000 > /sys/class/thermal/thermal_zone0/emul_temp

> ---
>  drivers/thermal/thermal_core.c | 63 ++-----------------------
>  include/linux/reboot.h         |  1 +
>  kernel/reboot.c                | 86 ++++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 996c038f83a4..b1444845af38 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -36,10 +36,8 @@ static LIST_HEAD(thermal_governor_list);
>  
>  static DEFINE_MUTEX(thermal_list_lock);
>  static DEFINE_MUTEX(thermal_governor_lock);
> -static DEFINE_MUTEX(poweroff_lock);
>  
>  static atomic_t in_suspend;
> -static bool power_off_triggered;
>  
>  static struct thermal_governor *def_governor;
>  
> @@ -327,70 +325,18 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz, int trip)
>  		       def_governor->throttle(tz, trip);
>  }
>  
> -/**
> - * thermal_emergency_poweroff_func - emergency poweroff work after a known delay
> - * @work: work_struct associated with the emergency poweroff function
> - *
> - * This function is called in very critical situations to force
> - * a kernel poweroff after a configurable timeout value.
> - */
> -static void thermal_emergency_poweroff_func(struct work_struct *work)
> -{
> -	/*
> -	 * We have reached here after the emergency thermal shutdown
> -	 * Waiting period has expired. This means orderly_poweroff has
> -	 * not been able to shut off the system for some reason.
> -	 * Try to shut down the system immediately using kernel_power_off
> -	 * if populated
> -	 */
> -	WARN(1, "Attempting kernel_power_off: Temperature too high\n");
> -	kernel_power_off();
> -
> -	/*
> -	 * Worst of the worst case trigger emergency restart
> -	 */
> -	WARN(1, "Attempting emergency_restart: Temperature too high\n");
> -	emergency_restart();
> -}
> -
> -static DECLARE_DELAYED_WORK(thermal_emergency_poweroff_work,
> -			    thermal_emergency_poweroff_func);
> -
> -/**
> - * thermal_emergency_poweroff - Trigger an emergency system poweroff
> - *
> - * This may be called from any critical situation to trigger a system shutdown
> - * after a known period of time. By default this is not scheduled.
> - */
> -static void thermal_emergency_poweroff(void)
> +void thermal_zone_device_critical(struct thermal_zone_device *tz)
>  {
> -	int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
>  	/*
>  	 * poweroff_delay_ms must be a carefully profiled positive value.
> -	 * Its a must for thermal_emergency_poweroff_work to be scheduled
> +	 * Its a must for forced_emergency_poweroff_work to be scheduled.
>  	 */
> -	if (poweroff_delay_ms <= 0)
> -		return;
> -	schedule_delayed_work(&thermal_emergency_poweroff_work,
> -			      msecs_to_jiffies(poweroff_delay_ms));
> -}
> +	int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
>  
> -void thermal_zone_device_critical(struct thermal_zone_device *tz)
> -{
>  	dev_emerg(&tz->device, "%s: critical temperature reached, "
>  		  "shutting down\n", tz->type);
>  
> -	mutex_lock(&poweroff_lock);
> -	if (!power_off_triggered) {
> -		/*
> -		 * Queue a backup emergency shutdown in the event of
> -		 * orderly_poweroff failure
> -		 */
> -		thermal_emergency_poweroff();
> -		orderly_poweroff(true);
> -		power_off_triggered = true;
> -	}
> -	mutex_unlock(&poweroff_lock);
> +	hw_protection_shutdown("Temperature too high", poweroff_delay_ms);
>  }
>  EXPORT_SYMBOL(thermal_zone_device_critical);
>  
> @@ -1549,7 +1495,6 @@ static int __init thermal_init(void)
>  	ida_destroy(&thermal_cdev_ida);
>  	mutex_destroy(&thermal_list_lock);
>  	mutex_destroy(&thermal_governor_lock);
> -	mutex_destroy(&poweroff_lock);
>  	return result;
>  }
>  postcore_initcall(thermal_init);
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index 3734cd8f38a8..af907a3d68d1 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -79,6 +79,7 @@ extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
>  
>  extern void orderly_poweroff(bool force);
>  extern void orderly_reboot(void);
> +void hw_protection_shutdown(const char *reason, int ms_until_forced);
>  
>  /*
>   * Emergency restart, callable from an interrupt handler.
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index a6ad5eb2fa73..1b5fa6d213d4 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -518,6 +518,92 @@ void orderly_reboot(void)
>  }
>  EXPORT_SYMBOL_GPL(orderly_reboot);
>  
> +/**
> + * hw_failure_emergency_poweroff_func - emergency poweroff work after a known delay
> + * @work: work_struct associated with the emergency poweroff function
> + *
> + * This function is called in very critical situations to force
> + * a kernel poweroff after a configurable timeout value.
> + */
> +static void hw_failure_emergency_poweroff_func(struct work_struct *work)
> +{
> +	/*
> +	 * We have reached here after the emergency shutdown waiting period has
> +	 * expired. This means orderly_poweroff has not been able to shut off
> +	 * the system for some reason.
> +	 *
> +	 * Try to shut down the system immediately using kernel_power_off
> +	 * if populated
> +	 */
> +	WARN(1, "Hardware protection timed-out. Trying forced poweroff\n");
> +	kernel_power_off();
> +
> +	/*
> +	 * Worst of the worst case trigger emergency restart
> +	 */
> +	WARN(1,
> +	     "Hardware protection shutdown failed. Trying emergency restart\n");
> +	emergency_restart();
> +}
> +
> +static DECLARE_DELAYED_WORK(hw_failure_emergency_poweroff_work,
> +			    hw_failure_emergency_poweroff_func);
> +
> +/**
> + * hw_failure_emergency_poweroff - Trigger an emergency system poweroff
> + *
> + * This may be called from any critical situation to trigger a system shutdown
> + * after a given period of time. If time is negative this is not scheduled.
> + */
> +static void hw_failure_emergency_poweroff(int poweroff_delay_ms)
> +{
> +	if (poweroff_delay_ms <= 0)
> +		return;
> +	schedule_delayed_work(&hw_failure_emergency_poweroff_work,
> +			      msecs_to_jiffies(poweroff_delay_ms));
> +}
> +
> +static bool prot_power_off_triggered;
> +static DEFINE_SPINLOCK(poweroff_lock);
> +
> +/**
> + * hw_protection_shutdown - Trigger an emergency system poweroff
> + *
> + * @reason:		Reason of emergency shutdown to be printed.
> + * @ms_until_forced:	Time to wait for orderly shutdown before tiggering a
> + *			forced shudown. Negative value disables the forced
> + *			shutdown.
> + *
> + * Initiate an emergency system shutdown in order to protect hardware from
> + * further damage. Usage examples include a thermal protection or a voltage or
> + * current regulator failures.
> + * NOTE: The request is ignored if protection shutdown is already pending even
> + * if the previous request has given a large timeout for forced shutdown.
> + * Can be called from any context.
> + */
> +void hw_protection_shutdown(const char *reason, int ms_until_forced)
> +{
> +	unsigned long flags;
> +
> +	pr_emerg("HARDWARE PROTECTION shutdown (%s)\n", reason);
> +
> +	spin_lock_irqsave(&poweroff_lock, flags);
> +	if (prot_power_off_triggered) {
> +		spin_unlock(&poweroff_lock);
> +		return;
> +	}
> +	prot_power_off_triggered = true;
> +	spin_unlock_irqrestore(&poweroff_lock, flags);
> +
> +	/*
> +	 * Queue a backup emergency shutdown in the event of
> +	 * orderly_poweroff failure
> +	 */
> +	hw_failure_emergency_poweroff(ms_until_forced);
> +	orderly_poweroff(true);
> +}
> +EXPORT_SYMBOL_GPL(hw_protection_shutdown);
> +
>  static int __init reboot_setup(char *str)
>  {
>  	for (;;) {
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v7 2/9] reboot: thermal: Export hardware protection shutdown

[Daniel Lezcano]

On 14/04/2021 07:52, Matti Vaittinen wrote:
> Thermal core contains a logic for safety shutdown. System is attempted to
> be powered off if temperature exceeds safety limits.
> 
> Currently this can be also utilized by regulator subsystem as a final
> protection measure if PMICs report dangerous over-voltage, over-current or
> over-temperature and if per regulator counter measures fail or do not
> exist.
> 
> Move this logic to kernel/reboot.c and export the functionality for other
> subsystems to use. Also replace the mutex with a spinlock to allow using
> the function from any context.
> 
> Also the EMIF bus code has implemented a safety shut-down. EMIF does not
> attempt orderly_poweroff at all. Thus the EMIF code is not converted to use
> this new function.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> Changelog
>  v7:
>   - new patch
> 
> Please note - this patch has received only a minimal amount of testing.
> (The new API call was tested to shut-down my system at driver probe but
> no odd corner-cases have been tested).
> 
> Any testing for thermal shutdown is appreciated.
> ---
>  drivers/thermal/thermal_core.c | 63 ++-----------------------
>  include/linux/reboot.h         |  1 +
>  kernel/reboot.c                | 86 ++++++++++++++++++++++++++++++++++

Please send a patch implementing the reboot/shutdown and then another
one replacing the thermal shutdown code by a call to the new API.

>  3 files changed, 91 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 996c038f83a4..b1444845af38 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -36,10 +36,8 @@ static LIST_HEAD(thermal_governor_list);
>  
>  static DEFINE_MUTEX(thermal_list_lock);
>  static DEFINE_MUTEX(thermal_governor_lock);
> -static DEFINE_MUTEX(poweroff_lock);
>  
>  static atomic_t in_suspend;
> -static bool power_off_triggered;
>  
>  static struct thermal_governor *def_governor;
>  
> @@ -327,70 +325,18 @@ static void handle_non_critical_trips(struct thermal_zone_device *tz, int trip)
>  		       def_governor->throttle(tz, trip);
>  }
>  
> -/**
> - * thermal_emergency_poweroff_func - emergency poweroff work after a known delay
> - * @work: work_struct associated with the emergency poweroff function
> - *
> - * This function is called in very critical situations to force
> - * a kernel poweroff after a configurable timeout value.
> - */
> -static void thermal_emergency_poweroff_func(struct work_struct *work)
> -{
> -	/*
> -	 * We have reached here after the emergency thermal shutdown
> -	 * Waiting period has expired. This means orderly_poweroff has
> -	 * not been able to shut off the system for some reason.
> -	 * Try to shut down the system immediately using kernel_power_off
> -	 * if populated
> -	 */
> -	WARN(1, "Attempting kernel_power_off: Temperature too high\n");
> -	kernel_power_off();
> -
> -	/*
> -	 * Worst of the worst case trigger emergency restart
> -	 */
> -	WARN(1, "Attempting emergency_restart: Temperature too high\n");
> -	emergency_restart();
> -}
> -
> -static DECLARE_DELAYED_WORK(thermal_emergency_poweroff_work,
> -			    thermal_emergency_poweroff_func);
> -
> -/**
> - * thermal_emergency_poweroff - Trigger an emergency system poweroff
> - *
> - * This may be called from any critical situation to trigger a system shutdown
> - * after a known period of time. By default this is not scheduled.
> - */
> -static void thermal_emergency_poweroff(void)
> +void thermal_zone_device_critical(struct thermal_zone_device *tz)
>  {
> -	int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
>  	/*
>  	 * poweroff_delay_ms must be a carefully profiled positive value.
> -	 * Its a must for thermal_emergency_poweroff_work to be scheduled
> +	 * Its a must for forced_emergency_poweroff_work to be scheduled.
>  	 */
> -	if (poweroff_delay_ms <= 0)
> -		return;
> -	schedule_delayed_work(&thermal_emergency_poweroff_work,
> -			      msecs_to_jiffies(poweroff_delay_ms));
> -}
> +	int poweroff_delay_ms = CONFIG_THERMAL_EMERGENCY_POWEROFF_DELAY_MS;
>  
> -void thermal_zone_device_critical(struct thermal_zone_device *tz)
> -{
>  	dev_emerg(&tz->device, "%s: critical temperature reached, "
>  		  "shutting down\n", tz->type);
>  
> -	mutex_lock(&poweroff_lock);
> -	if (!power_off_triggered) {
> -		/*
> -		 * Queue a backup emergency shutdown in the event of
> -		 * orderly_poweroff failure
> -		 */
> -		thermal_emergency_poweroff();
> -		orderly_poweroff(true);
> -		power_off_triggered = true;
> -	}
> -	mutex_unlock(&poweroff_lock);
> +	hw_protection_shutdown("Temperature too high", poweroff_delay_ms);
>  }
>  EXPORT_SYMBOL(thermal_zone_device_critical);
>  
> @@ -1549,7 +1495,6 @@ static int __init thermal_init(void)
>  	ida_destroy(&thermal_cdev_ida);
>  	mutex_destroy(&thermal_list_lock);
>  	mutex_destroy(&thermal_governor_lock);
> -	mutex_destroy(&poweroff_lock);
>  	return result;
>  }
>  postcore_initcall(thermal_init);
> diff --git a/include/linux/reboot.h b/include/linux/reboot.h
> index 3734cd8f38a8..af907a3d68d1 100644
> --- a/include/linux/reboot.h
> +++ b/include/linux/reboot.h
> @@ -79,6 +79,7 @@ extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
>  
>  extern void orderly_poweroff(bool force);
>  extern void orderly_reboot(void);
> +void hw_protection_shutdown(const char *reason, int ms_until_forced);
>  
>  /*
>   * Emergency restart, callable from an interrupt handler.
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index a6ad5eb2fa73..1b5fa6d213d4 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -518,6 +518,92 @@ void orderly_reboot(void)
>  }
>  EXPORT_SYMBOL_GPL(orderly_reboot);
>  
> +/**
> + * hw_failure_emergency_poweroff_func - emergency poweroff work after a known delay
> + * @work: work_struct associated with the emergency poweroff function
> + *
> + * This function is called in very critical situations to force
> + * a kernel poweroff after a configurable timeout value.
> + */
> +static void hw_failure_emergency_poweroff_func(struct work_struct *work)
> +{
> +	/*
> +	 * We have reached here after the emergency shutdown waiting period has
> +	 * expired. This means orderly_poweroff has not been able to shut off
> +	 * the system for some reason.
> +	 *
> +	 * Try to shut down the system immediately using kernel_power_off
> +	 * if populated
> +	 */
> +	WARN(1, "Hardware protection timed-out. Trying forced poweroff\n");
> +	kernel_power_off();
> +
> +	/*
> +	 * Worst of the worst case trigger emergency restart
> +	 */
> +	WARN(1,
> +	     "Hardware protection shutdown failed. Trying emergency restart\n");
> +	emergency_restart();
> +}
> +
> +static DECLARE_DELAYED_WORK(hw_failure_emergency_poweroff_work,
> +			    hw_failure_emergency_poweroff_func);
> +
> +/**
> + * hw_failure_emergency_poweroff - Trigger an emergency system poweroff
> + *
> + * This may be called from any critical situation to trigger a system shutdown
> + * after a given period of time. If time is negative this is not scheduled.
> + */
> +static void hw_failure_emergency_poweroff(int poweroff_delay_ms)
> +{
> +	if (poweroff_delay_ms <= 0)
> +		return;
> +	schedule_delayed_work(&hw_failure_emergency_poweroff_work,
> +			      msecs_to_jiffies(poweroff_delay_ms));
> +}
> +
> +static bool prot_power_off_triggered;
> +static DEFINE_SPINLOCK(poweroff_lock);
> +
> +/**
> + * hw_protection_shutdown - Trigger an emergency system poweroff
> + *
> + * @reason:		Reason of emergency shutdown to be printed.
> + * @ms_until_forced:	Time to wait for orderly shutdown before tiggering a
> + *			forced shudown. Negative value disables the forced
> + *			shutdown.
> + *
> + * Initiate an emergency system shutdown in order to protect hardware from
> + * further damage. Usage examples include a thermal protection or a voltage or
> + * current regulator failures.
> + * NOTE: The request is ignored if protection shutdown is already pending even
> + * if the previous request has given a large timeout for forced shutdown.
> + * Can be called from any context.
> + */
> +void hw_protection_shutdown(const char *reason, int ms_until_forced)
> +{
> +	unsigned long flags;
> +
> +	pr_emerg("HARDWARE PROTECTION shutdown (%s)\n", reason);
> +
> +	spin_lock_irqsave(&poweroff_lock, flags);
> +	if (prot_power_off_triggered) {
> +		spin_unlock(&poweroff_lock);

Why not spin_unlock_irqrestore() ?

> +		return;
> +	}
> +	prot_power_off_triggered = true;
> +	spin_unlock_irqrestore(&poweroff_lock, flags);

Why not take the spin_lock definitively for all the procedure ?

eg.

{
	...

	pr_emerg( ... );

	if (spin_trylock(&lock))
		return;

	hw_failure_emergency_poweroff(ms_until_forced);

	orderly_poweroff(true);
}

No need of prot_power_off_triggered and the spin_lock can be declared
static inside the function.

> +	/*
> +	 * Queue a backup emergency shutdown in the event of
> +	 * orderly_poweroff failure
> +	 */
> +	hw_failure_emergency_poweroff(ms_until_forced);
> +	orderly_poweroff(true);
> +}
> +EXPORT_SYMBOL_GPL(hw_protection_shutdown);
> +
>  static int __init reboot_setup(char *str)
>  {
>  	for (;;) {
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v7 2/9] reboot: thermal: Export hardware protection shutdown

[Vaittinen, Matti]

On Sat, 2021-04-17 at 06:57 +0200, Daniel Lezcano wrote:
> On 14/04/2021 07:52, Matti Vaittinen wrote:
> > Thermal core contains a logic for safety shutdown. System is
> > attempted to
> > be powered off if temperature exceeds safety limits.
> > 
> > Currently this can be also utilized by regulator subsystem as a
> > final
> > protection measure if PMICs report dangerous over-voltage, over-
> > current or
> > over-temperature and if per regulator counter measures fail or do
> > not
> > exist.
> > 
> > Move this logic to kernel/reboot.c and export the functionality for
> > other
> > subsystems to use. Also replace the mutex with a spinlock to allow
> > using
> > the function from any context.
> > 
> > Also the EMIF bus code has implemented a safety shut-down. EMIF
> > does not
> > attempt orderly_poweroff at all. Thus the EMIF code is not
> > converted to use
> > this new function.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> > Changelog
> >  v7:
> >   - new patch
> > 
> > Please note - this patch has received only a minimal amount of
> > testing.
> > (The new API call was tested to shut-down my system at driver probe
> > but
> > no odd corner-cases have been tested).
> > 
> > Any testing for thermal shutdown is appreciated.
> 
> You can test it easily by enabling the option
> CONFIG_THERMAL_EMULATION
> 
> Then in any thermal zone:
> 
> Assuming the critical temp is below the one specified in the command:
> 
> echo 100000 > /sys/class/thermal/thermal_zone0/emul_temp
> 

Thanks Daniel, I will see how that works when I create the next version
:)

Best Regards
	Matti Vaittinen

Re: [PATCH v7 2/9] reboot: thermal: Export hardware protection shutdown

[Matti Vaittinen]

Hi Daniel,

Thank you for the review. Much appreciated!

On Sat, 2021-04-17 at 07:32 +0200, Daniel Lezcano wrote:
> On 14/04/2021 07:52, Matti Vaittinen wrote:
> > Thermal core contains a logic for safety shutdown. System is
> > attempted to
> > be powered off if temperature exceeds safety limits.
> > 
> > Currently this can be also utilized by regulator subsystem as a
> > final
> > protection measure if PMICs report dangerous over-voltage, over-
> > current or
> > over-temperature and if per regulator counter measures fail or do
> > not
> > exist.
> > 
> > Move this logic to kernel/reboot.c and export the functionality for
> > other
> > subsystems to use. Also replace the mutex with a spinlock to allow
> > using
> > the function from any context.
> > 
> > Also the EMIF bus code has implemented a safety shut-down. EMIF
> > does not
> > attempt orderly_poweroff at all. Thus the EMIF code is not
> > converted to use
> > this new function.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > ---
> > Changelog
> >  v7:
> >   - new patch
> > 
> > Please note - this patch has received only a minimal amount of
> > testing.
> > (The new API call was tested to shut-down my system at driver probe
> > but
> > no odd corner-cases have been tested).
> > 
> > Any testing for thermal shutdown is appreciated.
> > ---
> >  drivers/thermal/thermal_core.c | 63 ++-----------------------
> >  include/linux/reboot.h         |  1 +
> >  kernel/reboot.c                | 86
> > ++++++++++++++++++++++++++++++++++
> 
> Please send a patch implementing the reboot/shutdown and then another
> one replacing the thermal shutdown code by a call to the new API.

I guess your suggestion makes sense. That way if the change causes any
problems in thermal-core it can be reverted without impacting other
potential users of this API. My original thinking was that this was
more of an move of functionality than adding an API. Having the move as
one patch makes sense as it shows where the code came from.

> 
> >  3 files changed, 91 insertions(+), 59 deletions(-)
> > 
> > diff --git a/drivers/thermal/thermal_core.c
> > b/drivers/thermal/thermal_core.c
> > index 996c038f83a4..b1444845af38 100644
> > --- a/drivers/thermal/thermal_core.c
> > +++ b/drivers/thermal/thermal_core.c
> > @@ -36,10 +36,8 @@ static LIST_HEAD(thermal_governor_list);
> >  
> > 

...

> > +static bool prot_power_off_triggered;
> > +static DEFINE_SPINLOCK(poweroff_lock);
> > +
> > +/**
> > + * hw_protection_shutdown - Trigger an emergency system poweroff
> > + *
> > + * @reason:		Reason of emergency shutdown to be
> > printed.
> > + * @ms_until_forced:	Time to wait for orderly shutdown
> > before tiggering a
> > + *			forced shudown. Negative value disables the
> > forced
> > + *			shutdown.
> > + *
> > + * Initiate an emergency system shutdown in order to protect
> > hardware from
> > + * further damage. Usage examples include a thermal protection or
> > a voltage or
> > + * current regulator failures.
> > + * NOTE: The request is ignored if protection shutdown is already
> > pending even
> > + * if the previous request has given a large timeout for forced
> > shutdown.
> > + * Can be called from any context.
> > + */
> > +void hw_protection_shutdown(const char *reason, int
> > ms_until_forced)
> > +{
> > +	unsigned long flags;
> > +
> > +	pr_emerg("HARDWARE PROTECTION shutdown (%s)\n", reason);
> > +
> > +	spin_lock_irqsave(&poweroff_lock, flags);
> > +	if (prot_power_off_triggered) {
> > +		spin_unlock(&poweroff_lock);
> 
> Why not spin_unlock_irqrestore() ?
> 

Well spotted It for sure must be spin_unlock_irqrestore. My bad.

> > +		return;
> > +	}
> > +	prot_power_off_triggered = true;
> > +	spin_unlock_irqrestore(&poweroff_lock, flags);
> 
> Why not take the spin_lock definitively for all the procedure ?
> 
> eg.
> 
> {
> 	...
> 
> 	pr_emerg( ... );
> 
> 	if (spin_trylock(&lock))
> 		return;
> 
> 	hw_failure_emergency_poweroff(ms_until_forced);
> 
> 	orderly_poweroff(true);
> }
> 
> No need of prot_power_off_triggered and the spin_lock can be declared
> static inside the function.

I think this makes perfect sense. My thinking just jammed to replacing
the mutex thermal-core used with a spin-lock using similar logic. I
guess this could even be just an atomic cmpxchg (or equivalent, I don't
remember what atomic abstractions we have) just to return if function
has been previously executed. Well, the spin_trylock() should work just
fine as far as I can say. So - thanks.


Best Regards
	Matti Vaittinen