linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v9 00/10] Extend regulator notification support
@ 2021-05-10 11:26 Matti Vaittinen
  2021-05-10 11:27 ` [PATCH v9 01/10] dt_bindings: Add protection limit properties Matti Vaittinen
                   ` (9 more replies)
  0 siblings, 10 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:26 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt,
	Daniel Lezcano, Amit Kucheria, Matteo Croce, Andrew Morton,
	Petr Mladek, Rafael J. Wysocki, Mike Rapoport, Josef Bacik,
	Kai-Heng Feng, linux-pm

[-- Attachment #1: Type: text/plain, Size: 7839 bytes --]

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
  simple atomic in order to allow call 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 v9:
   - rebases on v5.13-rc1
   - Update thermal documentation
   - Fix regulator notification event number
Changelog v8:
   - split shutdown API adding and thermal core taking it in use to
     own patches.
   - replace the spinlock with atomic when ensuring the emergency
     shutdown is only called once.
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 (10):
  dt_bindings: Add protection limit properties
  reboot: Add hardware protection power-off
  thermal: Use generic HW-protection shutdown API
  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 +
 .../driver-api/thermal/sysfs-api.rst          |   24 +-
 MAINTAINERS                                   |    4 +
 drivers/regulator/Makefile                    |    2 +-
 drivers/regulator/bd9576-regulator.c          | 1054 +++++++++++++++--
 drivers/regulator/core.c                      |  151 ++-
 drivers/regulator/devres.c                    |   52 +
 drivers/regulator/irq_helpers.c               |  398 +++++++
 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                               |   80 ++
 19 files changed, 2010 insertions(+), 217 deletions(-)
 create mode 100644 drivers/regulator/irq_helpers.c


base-commit: 6efb943b8616ec53a5e444193dccf1af9ad627b5
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v9 01/10] dt_bindings: Add protection limit properties
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
@ 2021-05-10 11:27 ` Matti Vaittinen
  2021-05-10 11:28 ` [PATCH v9 02/10] reboot: Add hardware protection power-off Matti Vaittinen
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:27 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 6540 bytes --]

Support specifying protection/error/warning limits for regulator
over current, over temperature and over/under voltage.

Most of the PMICs support only "protection" feature but few
setups do also support error/warning level indications.

On many ICs most of the protection limits can't actually be set.
But for example the ampere limit for over-current protection on ROHM
BD9576 can be configured - or feature can be completely disabled.

Provide limit setting for all protections/errors for the sake of
the completeness and do that using own properties for all so that
not all users would need to set all levels when only one or few are
supported.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>

---
No changes since RFC-v2
---
 .../bindings/regulator/regulator.yaml         | 82 +++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/regulator.yaml b/Documentation/devicetree/bindings/regulator/regulator.yaml
index 6d0bc9cd4040..a6ae9ecae5cc 100644
--- a/Documentation/devicetree/bindings/regulator/regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/regulator.yaml
@@ -117,6 +117,88 @@ properties:
     description: Enable over current protection.
     type: boolean
 
+  regulator-oc-protection-microamp:
+    description: Set over current protection limit. This is a limit where
+      hardware performs emergency shutdown. Zero can be passed to disable
+      protection and value '1' indicates that protection should be enabled but
+      limit setting can be omitted.
+
+  regulator-oc-error-microamp:
+    description: Set over current error limit. This is a limit where part of
+      the hardware propably is malfunctional and damage prevention is requested.
+      Zero can be passed to disable error detection and value '1' indicates
+      that detection should be enabled but limit setting can be omitted.
+
+  regulator-oc-warn-microamp:
+    description: Set over current warning limit. This is a limit where hardware
+      is assumed still to be functional but approaching limit where it gets
+      damaged. Recovery actions should be initiated. Zero can be passed to
+      disable detection and value '1' indicates that detection should
+      be enabled but limit setting can be omitted.
+
+  regulator-ov-protection-microvolt:
+    description: Set over voltage protection limit. This is a limit where
+      hardware performs emergency shutdown. Zero can be passed to disable
+      protection and value '1' indicates that protection should be enabled but
+      limit setting can be omitted. Limit is given as microvolt offset from
+      voltage set to regulator.
+
+  regulator-ov-error-microvolt:
+    description: Set over voltage error limit. This is a limit where part of
+      the hardware propably is malfunctional and damage prevention is requested
+      Zero can be passed to disable error detection and value '1' indicates
+      that detection should be enabled but limit setting can be omitted. Limit
+      is given as microvolt offset from voltage set to regulator.
+
+  regulator-ov-warn-microvolt:
+    description: Set over voltage warning limit. This is a limit where hardware
+      is assumed still to be functional but approaching limit where it gets
+      damaged. Recovery actions should be initiated. Zero can be passed to
+      disable detection and value '1' indicates that detection should
+      be enabled but limit setting can be omitted. Limit is given as microvolt
+      offset from voltage set to regulator.
+
+  regulator-uv-protection-microvolt:
+    description: Set over under voltage protection limit. This is a limit where
+      hardware performs emergency shutdown. Zero can be passed to disable
+      protection and value '1' indicates that protection should be enabled but
+      limit setting can be omitted. Limit is given as microvolt offset from
+      voltage set to regulator.
+
+  regulator-uv-error-microvolt:
+    description: Set under voltage error limit. This is a limit where part of
+      the hardware propably is malfunctional and damage prevention is requested
+      Zero can be passed to disable error detection and value '1' indicates
+      that detection should be enabled but limit setting can be omitted. Limit
+      is given as microvolt offset from voltage set to regulator.
+
+  regulator-uv-warn-microvolt:
+    description: Set over under voltage warning limit. This is a limit where
+      hardware is assumed still to be functional but approaching limit where
+      it gets damaged. Recovery actions should be initiated. Zero can be passed
+      to disable detection and value '1' indicates that detection should
+      be enabled but limit setting can be omitted. Limit is given as microvolt
+      offset from voltage set to regulator.
+
+  regulator-temp-protection-kelvin:
+    description: Set over temperature protection limit. This is a limit where
+      hardware performs emergency shutdown. Zero can be passed to disable
+      protection and value '1' indicates that protection should be enabled but
+      limit setting can be omitted.
+
+  regulator-temp-error-kelvin:
+    description: Set over temperature error limit. This is a limit where part of
+      the hardware propably is malfunctional and damage prevention is requested
+      Zero can be passed to disable error detection and value '1' indicates
+      that detection should be enabled but limit setting can be omitted.
+
+  regulator-temp-warn-kelvin:
+    description: Set over temperature warning limit. This is a limit where
+      hardware is assumed still to be functional but approaching limit where it
+      gets damaged. Recovery actions should be initiated. Zero can be passed to
+      disable detection and value '1' indicates that detection should
+      be enabled but limit setting can be omitted.
+
   regulator-active-discharge:
     description: |
       tristate, enable/disable active discharge of regulators. The values are:
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 02/10] reboot: Add hardware protection power-off
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
  2021-05-10 11:27 ` [PATCH v9 01/10] dt_bindings: Add protection limit properties Matti Vaittinen
@ 2021-05-10 11:28 ` Matti Vaittinen
  2021-05-12  8:20   ` Petr Mladek
  2021-05-10 11:29 ` [PATCH v9 03/10] thermal: Use generic HW-protection shutdown API Matti Vaittinen
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:28 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt,
	Daniel Lezcano, Amit Kucheria, Matteo Croce, Andrew Morton,
	Petr Mladek, Rafael J. Wysocki, Mike Rapoport, Josef Bacik,
	Kai-Heng Feng, linux-pm

[-- Attachment #1: Type: text/plain, Size: 5244 bytes --]

There can be few cases when we need to shut-down the system in order to
protect the hardware. Currently this is done at east by the thermal core
when temperature raises over certain limit.

Some PMICs can also generate interrupts for example for over-current or
over-voltage, voltage drops, short-circuit, ... etc. On some systems
these are a sign of hardware failure and only thing to do is try to
protect the rest of the hardware by shutting down the system.

Add shut-down logic which can be used by all subsystems instead of
implementing the shutdown in each subsystem. The logic is stolen from
thermal_core with difference of using atomic_t instead of a mutex in
order to allow calls directly from IRQ context.

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

---

Changelog:
v8: (changes suggested by Daniel Lezcano)
 - replace a protection implemented by a flag + spin_lock_irqsave() with
   simple atomic_dec_and_test().
 - Split thermal-core changes and adding the new API to separate patches
v7:
 - New patch
---
 include/linux/reboot.h |  1 +
 kernel/reboot.c        | 80 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)

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..5da8c80a2647 100644
--- a/kernel/reboot.c
+++ b/kernel/reboot.c
@@ -7,6 +7,7 @@
 
 #define pr_fmt(fmt)	"reboot: " fmt
 
+#include <linux/atomic.h>
 #include <linux/ctype.h>
 #include <linux/export.h>
 #include <linux/kexec.h>
@@ -518,6 +519,85 @@ 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));
+}
+
+/**
+ * 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)
+{
+	static atomic_t allow_proceed = ATOMIC_INIT(1);
+
+	pr_emerg("HARDWARE PROTECTION shutdown (%s)\n", reason);
+
+	/* Shutdown should be initiated only once. */
+	if (!atomic_dec_and_test(&allow_proceed))
+		return;
+
+	/*
+	 * 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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 03/10] thermal: Use generic HW-protection shutdown API
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
  2021-05-10 11:27 ` [PATCH v9 01/10] dt_bindings: Add protection limit properties Matti Vaittinen
  2021-05-10 11:28 ` [PATCH v9 02/10] reboot: Add hardware protection power-off Matti Vaittinen
@ 2021-05-10 11:29 ` Matti Vaittinen
  2021-05-10 11:29 ` [PATCH v9 04/10] regulator: add warning flags Matti Vaittinen
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:29 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt,
	Daniel Lezcano, Amit Kucheria, Matteo Croce, Andrew Morton,
	Petr Mladek, Rafael J. Wysocki, Mike Rapoport, Josef Bacik,
	Kai-Heng Feng, linux-pm

[-- Attachment #1: Type: text/plain, Size: 6387 bytes --]

The hardware shutdown function was exported from kernel/reboot for
other subsystems to use. Logic is copied from the thermal_core. The
protection mutex is replaced by an atomic_t to allow calls also from
an IRQ context.

Use the exported API instead of implementing own just for the
thermal_core.

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

---
Changelog:
v9:
 - Update the thermal documentation
v8:
 - new patch (change added in v7, splitted in own patch at v8)

Use the exported API instead
---
 .../driver-api/thermal/sysfs-api.rst          | 24 +++----
 drivers/thermal/thermal_core.c                | 63 ++-----------------
 2 files changed, 13 insertions(+), 74 deletions(-)

diff --git a/Documentation/driver-api/thermal/sysfs-api.rst b/Documentation/driver-api/thermal/sysfs-api.rst
index 4b638c14bc16..c93fa5e961a0 100644
--- a/Documentation/driver-api/thermal/sysfs-api.rst
+++ b/Documentation/driver-api/thermal/sysfs-api.rst
@@ -740,21 +740,15 @@ possible.
 5. thermal_emergency_poweroff
 =============================
 
-On an event of critical trip temperature crossing. Thermal framework
-allows the system to shutdown gracefully by calling orderly_poweroff().
-In the event of a failure of orderly_poweroff() to shut down the system
-we are in danger of keeping the system alive at undesirably high
-temperatures. To mitigate this high risk scenario we program a work
-queue to fire after a pre-determined number of seconds to start
-an emergency shutdown of the device using the kernel_power_off()
-function. In case kernel_power_off() fails then finally
-emergency_restart() is called in the worst case.
+On an event of critical trip temperature crossing the thermal framework
+shuts down the system by calling hw_protection_shutdown(). The
+hw_protection_shutdown() first attempts to perform an orderly shutdown
+but accepts a delay after which it proceeds doing a forced power-off
+or as last resort an emergency_restart.
 
 The delay should be carefully profiled so as to give adequate time for
-orderly_poweroff(). In case of failure of an orderly_poweroff() the
-emergency poweroff kicks in after the delay has elapsed and shuts down
-the system.
+orderly poweroff.
 
-If set to 0 emergency poweroff will not be supported. So a carefully
-profiled non-zero positive value is a must for emergency poweroff to be
-triggered.
+If the delay is set to 0 emergency poweroff will not be supported. So a
+carefully profiled non-zero positive value is a must for emergency
+poweroff to be triggered.
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index d20b25f40d19..10a2d8e1cacf 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);
 
@@ -1538,7 +1484,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);
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 04/10] regulator: add warning flags
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
                   ` (2 preceding siblings ...)
  2021-05-10 11:29 ` [PATCH v9 03/10] thermal: Use generic HW-protection shutdown API Matti Vaittinen
@ 2021-05-10 11:29 ` Matti Vaittinen
  2021-05-10 11:29 ` [PATCH v9 05/10] regulator: IRQ based event/error notification helpers Matti Vaittinen
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:29 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

Add 'warning' level events and error flags to regulator core.
Current regulator core notifications are used to inform consumers
about errors where HW is misbehaving in such way it is assumed to
be broken/unrecoverable.

There are PMICs which are designed for system(s) that may have use
for regulator indications sent before HW is damaged so that some
board/consumer specific recovery-event can be performed while
continuing most of the normal operations.

Add new WARNING level events and notifications to be used for
that purpose.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
No changes since RFC-v2
---
 include/linux/regulator/consumer.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 20e84a84fb77..f72ca73631be 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -119,6 +119,16 @@ struct regulator_dev;
 #define REGULATOR_EVENT_PRE_DISABLE		0x400
 #define REGULATOR_EVENT_ABORT_DISABLE		0x800
 #define REGULATOR_EVENT_ENABLE			0x1000
+/*
+ * Following notifications should be emitted only if detected condition
+ * is such that the HW is likely to still be working but consumers should
+ * take a recovery action to prevent problems esacalating into errors.
+ */
+#define REGULATOR_EVENT_UNDER_VOLTAGE_WARN	0x2000
+#define REGULATOR_EVENT_OVER_CURRENT_WARN	0x4000
+#define REGULATOR_EVENT_OVER_VOLTAGE_WARN	0x8000
+#define REGULATOR_EVENT_OVER_TEMP_WARN		0x10000
+#define REGULATOR_EVENT_WARN_MASK		0x1E000
 
 /*
  * Regulator errors that can be queried using regulator_get_error_flags
@@ -138,6 +148,10 @@ struct regulator_dev;
 #define REGULATOR_ERROR_FAIL			BIT(4)
 #define REGULATOR_ERROR_OVER_TEMP		BIT(5)
 
+#define REGULATOR_ERROR_UNDER_VOLTAGE_WARN	BIT(6)
+#define REGULATOR_ERROR_OVER_CURRENT_WARN	BIT(7)
+#define REGULATOR_ERROR_OVER_VOLTAGE_WARN	BIT(8)
+#define REGULATOR_ERROR_OVER_TEMP_WARN		BIT(9)
 
 /**
  * struct pre_voltage_change_data - Data sent with PRE_VOLTAGE_CHANGE event
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 05/10] regulator: IRQ based event/error notification helpers
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
                   ` (3 preceding siblings ...)
  2021-05-10 11:29 ` [PATCH v9 04/10] regulator: add warning flags Matti Vaittinen
@ 2021-05-10 11:29 ` Matti Vaittinen
  2021-05-10 17:35   ` kernel test robot
                     ` (2 more replies)
  2021-05-10 11:30 ` [PATCH v9 06/10] regulator: add property parsing and callbacks to set protection limits Matti Vaittinen
                   ` (4 subsequent siblings)
  9 siblings, 3 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:29 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 24165 bytes --]

Provide helper function for IC's implementing regulator notifications
when an IRQ fires. The helper also works for IRQs which can not be acked.
Helper can be set to disable the IRQ at handler and then re-enabling it
on delayed work later. The helper also adds regulator_get_error_flags()
errors in cache for the duration of IRQ disabling.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

---
v9:
 - Fix the notification error number
v7 (mostly more fixes pointed by Andy)
 - fix regulator error_flags query
 - grammar/typos
 - do not BUG() but attempt to shut-down the system
 - use BITS_PER_TYPE()
v6 (fix issues noted by Andy):
 - remove unnecessary variable
 - use BIT(foo) instead of 1 << foo
 - use devm_add_action_or_reset()
 - do not check the irq parameter validity, leave that to
   request_threaded_irq()
 - put resource-managed function in devres.c
 - fix the kerneldocs for the new IRQ helpers
v5:
 - fix the pr_emerg print
v4:
 - Comment block styling
 - Added prints to point the potential HW failure before BUG()
 - Corrected typo from kerneldoc
 - added missing newlines
---
 drivers/regulator/Makefile       |   2 +-
 drivers/regulator/core.c         |  29 ++-
 drivers/regulator/devres.c       |  52 ++++
 drivers/regulator/irq_helpers.c  | 398 +++++++++++++++++++++++++++++++
 include/linux/regulator/driver.h | 135 +++++++++++
 5 files changed, 608 insertions(+), 8 deletions(-)
 create mode 100644 drivers/regulator/irq_helpers.c

diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 580b015296ea..534fc0163bc4 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -4,7 +4,7 @@
 #
 
 
-obj-$(CONFIG_REGULATOR) += core.o dummy.o fixed-helper.o helpers.o devres.o
+obj-$(CONFIG_REGULATOR) += core.o dummy.o fixed-helper.o helpers.o devres.o irq_helpers.o
 obj-$(CONFIG_OF) += of_regulator.o
 obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
 obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f192bf19492e..87f53f5fbbd0 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -4380,22 +4380,36 @@ unsigned int regulator_get_mode(struct regulator *regulator)
 }
 EXPORT_SYMBOL_GPL(regulator_get_mode);
 
+static int rdev_get_cached_err_flags(struct regulator_dev *rdev)
+{
+	int ret = 0;
+
+	if (rdev->use_cached_err) {
+		spin_lock(&rdev->err_lock);
+		ret = rdev->cached_err;
+		spin_unlock(&rdev->err_lock);
+	}
+	return ret;
+}
+
 static int _regulator_get_error_flags(struct regulator_dev *rdev,
 					unsigned int *flags)
 {
-	int ret;
+	int cached_flags, ret = 0;
 
 	regulator_lock(rdev);
 
-	/* sanity check */
-	if (!rdev->desc->ops->get_error_flags) {
+	cached_flags = rdev_get_cached_err_flags(rdev);
+
+	if (rdev->desc->ops->get_error_flags)
+		ret = rdev->desc->ops->get_error_flags(rdev, flags);
+	else if (!rdev->use_cached_err)
 		ret = -EINVAL;
-		goto out;
-	}
 
-	ret = rdev->desc->ops->get_error_flags(rdev, flags);
-out:
+	*flags |= cached_flags;
+
 	regulator_unlock(rdev);
+
 	return ret;
 }
 
@@ -5228,6 +5242,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
 		goto rinse;
 	}
 	device_initialize(&rdev->dev);
+	spin_lock_init(&rdev->err_lock);
 
 	/*
 	 * Duplicate the config so the driver could override it after
diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 3091210889e3..a8de0aa88bad 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -481,3 +481,55 @@ void devm_regulator_unregister_notifier(struct regulator *regulator,
 		WARN_ON(rc);
 }
 EXPORT_SYMBOL_GPL(devm_regulator_unregister_notifier);
+
+static void regulator_irq_helper_drop(void *res)
+{
+	regulator_irq_helper_cancel(&res);
+}
+
+/**
+ * devm_regulator_irq_helper - resource managed registration of IRQ based
+ * regulator event/error notifier
+ *
+ * @dev:		device to which lifetime the helper's lifetime is
+ *			bound.
+ * @d:			IRQ helper descriptor.
+ * @irq:		IRQ used to inform events/errors to be notified.
+ * @irq_flags:		Extra IRQ flags to be OR'ed with the default
+ *			IRQF_ONESHOT when requesting the (threaded) irq.
+ * @common_errs:	Errors which can be flagged by this IRQ for all rdevs.
+ *			When IRQ is re-enabled these errors will be cleared
+ *			from all associated regulators
+ * @per_rdev_errs:	Optional error flag array describing errors specific
+ *			for only some of the regulators. These errors will be
+ *			or'ed with common errors. If this is given the array
+ *			should contain rdev_amount flags. Can be set to NULL
+ *			if there is no regulator specific error flags for this
+ *			IRQ.
+ * @rdev:		Array of pointers to regulators associated with this
+ *			IRQ.
+ * @rdev_amount:	Amount of regulators associated with this IRQ.
+ *
+ * Return: handle to irq_helper or an ERR_PTR() encoded error code.
+ */
+void *devm_regulator_irq_helper(struct device *dev,
+				const struct regulator_irq_desc *d, int irq,
+				int irq_flags, int common_errs,
+				int *per_rdev_errs,
+				struct regulator_dev **rdev, int rdev_amount)
+{
+	void *ptr;
+	int ret;
+
+	ptr = regulator_irq_helper(dev, d, irq, irq_flags, common_errs,
+				    per_rdev_errs, rdev, rdev_amount);
+	if (IS_ERR(ptr))
+		return ptr;
+
+	ret = devm_add_action_or_reset(dev, regulator_irq_helper_drop, ptr);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return ptr;
+}
+EXPORT_SYMBOL_GPL(devm_regulator_irq_helper);
diff --git a/drivers/regulator/irq_helpers.c b/drivers/regulator/irq_helpers.c
new file mode 100644
index 000000000000..d771688a4c64
--- /dev/null
+++ b/drivers/regulator/irq_helpers.c
@@ -0,0 +1,398 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2021 ROHM Semiconductors
+// regulator IRQ based event notification helpers
+//
+// Logic has been partially adapted from qcom-labibb driver.
+//
+// Author: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/regulator/driver.h>
+
+#define REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS 10000
+
+struct regulator_irq {
+	struct regulator_irq_data rdata;
+	struct regulator_irq_desc desc;
+	int irq;
+	int retry_cnt;
+	struct delayed_work isr_work;
+};
+
+/*
+ * Should only be called from threaded handler to prevent potential deadlock
+ */
+static void rdev_flag_err(struct regulator_dev *rdev, int err)
+{
+	spin_lock(&rdev->err_lock);
+	rdev->cached_err |= err;
+	spin_unlock(&rdev->err_lock);
+}
+
+static void rdev_clear_err(struct regulator_dev *rdev, int err)
+{
+	spin_lock(&rdev->err_lock);
+	rdev->cached_err &= ~err;
+	spin_unlock(&rdev->err_lock);
+}
+
+static void regulator_notifier_isr_work(struct work_struct *work)
+{
+	struct regulator_irq *h;
+	struct regulator_irq_desc *d;
+	struct regulator_irq_data *rid;
+	int ret = 0;
+	int tmo, i;
+	int num_rdevs;
+
+	h = container_of(work, struct regulator_irq,
+			    isr_work.work);
+	d = &h->desc;
+	rid = &h->rdata;
+	num_rdevs = rid->num_states;
+
+reread:
+	if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
+		if (!d->die)
+			return hw_protection_shutdown("Regulator HW failure? - no IC recovery",
+						      REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
+		ret = d->die(rid);
+		/*
+		 * If the 'last resort' IC recovery failed we will have
+		 * nothing else left to do...
+		 */
+		if (ret)
+			return hw_protection_shutdown("Regulator HW failure. IC recovery failed",
+						      REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
+
+		/*
+		 * If h->die() was implemented we assume recovery has been
+		 * attempted (probably regulator was shut down) and we
+		 * just enable IRQ and bail-out.
+		 */
+		goto enable_out;
+	}
+	if (d->renable) {
+		ret = d->renable(rid);
+
+		if (ret == REGULATOR_FAILED_RETRY) {
+			/* Driver could not get current status */
+			h->retry_cnt++;
+			if (!d->reread_ms)
+				goto reread;
+
+			tmo = d->reread_ms;
+			goto reschedule;
+		}
+
+		if (ret) {
+			/*
+			 * IC status reading succeeded. update error info
+			 * just in case the renable changed it.
+			 */
+			for (i = 0; i < num_rdevs; i++) {
+				struct regulator_err_state *stat;
+				struct regulator_dev *rdev;
+
+				stat = &rid->states[i];
+				rdev = stat->rdev;
+				rdev_clear_err(rdev, (~stat->errors) &
+						      stat->possible_errs);
+			}
+			h->retry_cnt++;
+			/*
+			 * The IC indicated problem is still ON - no point in
+			 * re-enabling the IRQ. Retry later.
+			 */
+			tmo = d->irq_off_ms;
+			goto reschedule;
+		}
+	}
+
+	/*
+	 * Either IC reported problem cleared or no status checker was provided.
+	 * If problems are gone - good. If not - then the IRQ will fire again
+	 * and we'll have a new nice loop. In any case we should clear error
+	 * flags here and re-enable IRQs.
+	 */
+	for (i = 0; i < num_rdevs; i++) {
+		struct regulator_err_state *stat;
+		struct regulator_dev *rdev;
+
+		stat = &rid->states[i];
+		rdev = stat->rdev;
+		rdev_clear_err(rdev, stat->possible_errs);
+	}
+
+	/*
+	 * Things have been seemingly successful => zero retry-counter.
+	 */
+	h->retry_cnt = 0;
+
+enable_out:
+	enable_irq(h->irq);
+
+	return;
+
+reschedule:
+	if (!d->high_prio)
+		mod_delayed_work(system_wq, &h->isr_work,
+				 msecs_to_jiffies(tmo));
+	else
+		mod_delayed_work(system_highpri_wq, &h->isr_work,
+				 msecs_to_jiffies(tmo));
+}
+
+static irqreturn_t regulator_notifier_isr(int irq, void *data)
+{
+	struct regulator_irq *h = data;
+	struct regulator_irq_desc *d;
+	struct regulator_irq_data *rid;
+	unsigned long rdev_map = 0;
+	int num_rdevs;
+	int ret, i, j;
+
+	d = &h->desc;
+	rid = &h->rdata;
+	num_rdevs = rid->num_states;
+
+	if (d->fatal_cnt)
+		h->retry_cnt++;
+
+	/*
+	 * we spare a few cycles by not clearing statuses prior to this call.
+	 * The IC driver must initialize the status buffers for rdevs
+	 * which it indicates having active events via rdev_map.
+	 *
+	 * Maybe we should just to be on a safer side(?)
+	 */
+	ret = d->map_event(irq, rid, &rdev_map);
+
+	/*
+	 * If status reading fails (which is unlikely) we don't ack/disable
+	 * IRQ but just increase fail count and retry when IRQ fires again.
+	 * If retry_count exceeds the given safety limit we call IC specific die
+	 * handler which can try disabling regulator(s).
+	 *
+	 * If no die handler is given we will just bug() as a last resort.
+	 *
+	 * We could try disabling all associated rdevs - but we might shoot
+	 * ourselves in the head and leave the problematic regulator enabled. So
+	 * if IC has no die-handler populated we just assume the regulator
+	 * can't be disabled.
+	 */
+	if (unlikely(ret == REGULATOR_FAILED_RETRY))
+		goto fail_out;
+
+	h->retry_cnt = 0;
+	/*
+	 * Let's not disable IRQ if there were no status bits for us. We'd
+	 * better leave spurious IRQ handling to genirq
+	 */
+	if (ret || !rdev_map)
+		return IRQ_NONE;
+
+	/*
+	 * Some events are bogus if the regulator is disabled. Skip such events
+	 * if all relevant regulators are disabled
+	 */
+	if (d->skip_off) {
+		for_each_set_bit(i, &rdev_map, num_rdevs) {
+			struct regulator_dev *rdev;
+			const struct regulator_ops *ops;
+
+			rdev = rid->states[i].rdev;
+			ops = rdev->desc->ops;
+
+			/*
+			 * If any of the flagged regulators is enabled we do
+			 * handle this
+			 */
+			if (ops->is_enabled(rdev))
+				break;
+		}
+		if (i == num_rdevs)
+			return IRQ_NONE;
+	}
+
+	/* Disable IRQ if HW keeps line asserted */
+	if (d->irq_off_ms)
+		disable_irq_nosync(irq);
+
+	/*
+	 * IRQ seems to be for us. Let's fire correct notifiers / store error
+	 * flags
+	 */
+	for_each_set_bit(i, &rdev_map, num_rdevs) {
+		unsigned long evt;
+		struct regulator_err_state *stat;
+		struct regulator_dev *rdev;
+
+		stat = &rid->states[i];
+		rdev = stat->rdev;
+
+		for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat->notifs))
+			evt =  BIT(j);
+			pr_dbg("Sending regulator notification EVT 0x%lx\r\n",
+			       stat->notifs, evt);
+			regulator_notifier_call_chain(rdev, evt, NULL);
+
+		rdev_flag_err(rdev, stat->errors);
+	}
+
+	if (d->irq_off_ms) {
+		if (!d->high_prio)
+			schedule_delayed_work(&h->isr_work,
+					      msecs_to_jiffies(d->irq_off_ms));
+		else
+			mod_delayed_work(system_highpri_wq,
+					 &h->isr_work,
+					 msecs_to_jiffies(d->irq_off_ms));
+	}
+
+	return IRQ_HANDLED;
+
+fail_out:
+	if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
+		/* If we have no recovery, just try shut down straight away */
+		if (!d->die) {
+			hw_protection_shutdown("Regulator failure. Retry count exceeded",
+					       REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
+		} else {
+			ret = d->die(rid);
+			/* If die() failed shut down as a last attempt to save the HW */
+			if (ret)
+				hw_protection_shutdown("Regulator failure. Recovery failed",
+						       REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
+		}
+	}
+
+	return IRQ_NONE;
+}
+
+static int init_rdev_state(struct device *dev, struct regulator_irq *h,
+			   struct regulator_dev **rdev, int common_err,
+			   int *rdev_err, int rdev_amount)
+{
+	int i;
+
+	h->rdata.states = devm_kzalloc(dev, sizeof(*h->rdata.states) *
+				       rdev_amount, GFP_KERNEL);
+	if (!h->rdata.states)
+		return -ENOMEM;
+
+	h->rdata.num_states = rdev_amount;
+	h->rdata.data = h->desc.data;
+
+	for (i = 0; i < rdev_amount; i++) {
+		h->rdata.states[i].possible_errs = common_err;
+		if (rdev_err)
+			h->rdata.states[i].possible_errs |= *rdev_err++;
+		h->rdata.states[i].rdev = *rdev++;
+	}
+
+	return 0;
+}
+
+static void init_rdev_errors(struct regulator_irq *h)
+{
+	int i;
+
+	for (i = 0; i < h->rdata.num_states; i++)
+		if (h->rdata.states[i].possible_errs)
+			h->rdata.states[i].rdev->use_cached_err = true;
+}
+
+/**
+ * regulator_irq_helper - register IRQ based regulator event/error notifier
+ *
+ * @dev:		device providing the IRQs
+ * @d:			IRQ helper descriptor.
+ * @irq:		IRQ used to inform events/errors to be notified.
+ * @irq_flags:		Extra IRQ flags to be OR'ed with the default
+ *			IRQF_ONESHOT when requesting the (threaded) irq.
+ * @common_errs:	Errors which can be flagged by this IRQ for all rdevs.
+ *			When IRQ is re-enabled these errors will be cleared
+ *			from all associated regulators
+ * @per_rdev_errs:	Optional error flag array describing errors specific
+ *			for only some of the regulators. These errors will be
+ *			or'ed with common errors. If this is given the array
+ *			should contain rdev_amount flags. Can be set to NULL
+ *			if there is no regulator specific error flags for this
+ *			IRQ.
+ * @rdev:		Array of pointers to regulators associated with this
+ *			IRQ.
+ * @rdev_amount:	Amount of regulators associated with this IRQ.
+ *
+ * Return: handle to irq_helper or an ERR_PTR() encoded error code.
+ */
+void *regulator_irq_helper(struct device *dev,
+			   const struct regulator_irq_desc *d, int irq,
+			   int irq_flags, int common_errs, int *per_rdev_errs,
+			   struct regulator_dev **rdev, int rdev_amount)
+{
+	struct regulator_irq *h;
+	int ret;
+
+	if (!rdev_amount || !d || !d->map_event || !d->name)
+		return ERR_PTR(-EINVAL);
+
+	h = devm_kzalloc(dev, sizeof(*h), GFP_KERNEL);
+	if (!h)
+		return ERR_PTR(-ENOMEM);
+
+	h->irq = irq;
+	h->desc = *d;
+
+	ret = init_rdev_state(dev, h, rdev, common_errs, per_rdev_errs,
+			      rdev_amount);
+	if (ret)
+		return ERR_PTR(ret);
+
+	init_rdev_errors(h);
+
+	if (h->desc.irq_off_ms)
+		INIT_DELAYED_WORK(&h->isr_work, regulator_notifier_isr_work);
+
+	ret = request_threaded_irq(h->irq, NULL, regulator_notifier_isr,
+				   IRQF_ONESHOT | irq_flags, h->desc.name, h);
+	if (ret) {
+		dev_err(dev, "Failed to request IRQ %d\n", irq);
+
+		return ERR_PTR(ret);
+	}
+
+	return h;
+}
+EXPORT_SYMBOL_GPL(regulator_irq_helper);
+
+/**
+ * regulator_irq_helper_cancel - drop IRQ based regulator event/error notifier
+ *
+ * @handle:		Pointer to handle returned by a successful call to
+ *			regulator_irq_helper(). Will be NULLed upon return.
+ *
+ * The associated IRQ is released and work is cancelled when the function
+ * returns.
+ */
+void regulator_irq_helper_cancel(void **handle)
+{
+	if (handle && *handle) {
+		struct regulator_irq *h = *handle;
+
+		free_irq(h->irq, h);
+		if (h->desc.irq_off_ms)
+			cancel_delayed_work_sync(&h->isr_work);
+
+		h = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(regulator_irq_helper_cancel);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 4ea520c248e9..be301b28838a 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -413,6 +413,128 @@ struct regulator_config {
 	struct gpio_desc *ena_gpiod;
 };
 
+/**
+ * struct regulator_err_state - regulator error/notification status
+ *
+ * @rdev:		Regulator which status the struct indicates.
+ * @notifs:		Events which have occurred on the regulator.
+ * @errors:		Errors which are active on the regulator.
+ * @possible_errs:	Errors which can be signaled (by given IRQ).
+ */
+struct regulator_err_state {
+	struct regulator_dev *rdev;
+	unsigned long notifs;
+	unsigned long errors;
+	int possible_errs;
+};
+
+/**
+ * struct regulator_irq_data - regulator error/notification status date
+ *
+ * @states:	Status structs for each of the associated regulators.
+ * @num_states:	Amount of associated regulators.
+ * @data:	Driver data pointer given at regulator_irq_desc.
+ * @opaque:	Value storage for IC driver. Core does not update this. ICs
+ *		may want to store status register value here at map_event and
+ *		compare contents at 'renable' callback to see if new problems
+ *		have been added to status. If that is the case it may be
+ *		desirable to return REGULATOR_ERROR_CLEARED and not
+ *		REGULATOR_ERROR_ON to allow IRQ fire again and to generate
+ *		notifications also for the new issues.
+ *
+ * This structure is passed to 'map_event' and 'renable' callbacks for
+ * reporting regulator status to core.
+ */
+struct regulator_irq_data {
+	struct regulator_err_state *states;
+	int num_states;
+	void *data;
+	long opaque;
+};
+
+/**
+ * struct regulator_irq_desc - notification sender for IRQ based events.
+ *
+ * @name:	The visible name for the IRQ
+ * @fatal_cnt:	If this IRQ is used to signal HW damaging condition it may be
+ *		best to shut-down regulator(s) or reboot the SOC if error
+ *		handling is repeatedly failing. If fatal_cnt is given the IRQ
+ *		handling is aborted if it fails for fatal_cnt times and die()
+ *		callback (if populated) or BUG() is called to try to prevent
+ *		further damage.
+ * @reread_ms:	The time which is waited before attempting to re-read status
+ *		at the worker if IC reading fails. Immediate re-read is done
+ *		if time is not specified.
+ * @irq_off_ms:	The time which IRQ is kept disabled before re-evaluating the
+ *		status for devices which keep IRQ disabled for duration of the
+ *		error. If this is not given the IRQ is left enabled and renable
+ *		is not called.
+ * @skip_off:	If set to true the IRQ handler will attempt to check if any of
+ *		the associated regulators are enabled prior to taking other
+ *		actions. If no regulators are enabled and this is set to true
+ *		a spurious IRQ is assumed and IRQ_NONE is returned.
+ * @high_prio:	Boolean to indicate that high priority WQ should be used.
+ * @data:	Driver private data pointer which will be passed as such to
+ *		the renable, map_event and die callbacks in regulator_irq_data.
+ * @die:	Protection callback. If IC status reading or recovery actions
+ *		fail fatal_cnt times this callback or BUG() is called. This
+ *		callback should implement a final protection attempt like
+ *		disabling the regulator. If protection succeeded this may
+ *		return 0. If anything else is returned the core assumes final
+ *		protection failed and calls BUG() as a last resort.
+ * @map_event:	Driver callback to map IRQ status into regulator devices with
+ *		events / errors. NOTE: callback MUST initialize both the
+ *		errors and notifs for all rdevs which it signals having
+ *		active events as core does not clean the map data.
+ *		REGULATOR_FAILED_RETRY can be returned to indicate that the
+ *		status reading from IC failed. If this is repeated for
+ *		fatal_cnt times the core will call die() callback or BUG()
+ *		as a last resort to protect the HW.
+ * @renable:	Optional callback to check status (if HW supports that) before
+ *		re-enabling IRQ. If implemented this should clear the error
+ *		flags so that errors fetched by regulator_get_error_flags()
+ *		are updated. If callback is not implemented then errors are
+ *		assumed to be cleared and IRQ is re-enabled.
+ *		REGULATOR_FAILED_RETRY can be returned to
+ *		indicate that the status reading from IC failed. If this is
+ *		repeated for 'fatal_cnt' times the core will call die()
+ *		callback or BUG() as a last resort to protect the HW.
+ *		Returning zero indicates that the problem in HW has been solved
+ *		and IRQ will be re-enabled. Returning REGULATOR_ERROR_ON
+ *		indicates the error condition is still active and keeps IRQ
+ *		disabled. Please note that returning REGULATOR_ERROR_ON does
+ *		not retrigger evaluating what events are active or resending
+ *		notifications. If this is needed you probably want to return
+ *		zero and allow IRQ to retrigger causing events to be
+ *		re-evaluated and re-sent.
+ *
+ * This structure is used for registering regulator IRQ notification helper.
+ */
+struct regulator_irq_desc {
+	const char *name;
+	int irq_flags;
+	int fatal_cnt;
+	int reread_ms;
+	int irq_off_ms;
+	bool skip_off;
+	bool high_prio;
+	void *data;
+
+	int (*die)(struct regulator_irq_data *rid);
+	int (*map_event)(int irq, struct regulator_irq_data *rid,
+			  unsigned long *dev_mask);
+	int (*renable)(struct regulator_irq_data *rid);
+};
+
+/*
+ * Return values for regulator IRQ helpers.
+ */
+enum {
+	REGULATOR_ERROR_CLEARED,
+	REGULATOR_FAILED_RETRY,
+	REGULATOR_ERROR_ON,
+};
+
 /*
  * struct coupling_desc
  *
@@ -477,6 +599,9 @@ struct regulator_dev {
 
 	/* time when this regulator was disabled last time */
 	ktime_t last_off;
+	int cached_err;
+	bool use_cached_err;
+	spinlock_t err_lock;
 };
 
 struct regulator_dev *
@@ -491,6 +616,16 @@ void devm_regulator_unregister(struct device *dev, struct regulator_dev *rdev);
 
 int regulator_notifier_call_chain(struct regulator_dev *rdev,
 				  unsigned long event, void *data);
+void *devm_regulator_irq_helper(struct device *dev,
+				const struct regulator_irq_desc *d, int irq,
+				int irq_flags, int common_errs,
+				int *per_rdev_errs, struct regulator_dev **rdev,
+				int rdev_amount);
+void *regulator_irq_helper(struct device *dev,
+			   const struct regulator_irq_desc *d, int irq,
+			   int irq_flags, int common_errs, int *per_rdev_errs,
+			   struct regulator_dev **rdev, int rdev_amount);
+void regulator_irq_helper_cancel(void **handle);
 
 void *rdev_get_drvdata(struct regulator_dev *rdev);
 struct device *rdev_get_dev(struct regulator_dev *rdev);
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 06/10] regulator: add property parsing and callbacks to set protection limits
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
                   ` (4 preceding siblings ...)
  2021-05-10 11:29 ` [PATCH v9 05/10] regulator: IRQ based event/error notification helpers Matti Vaittinen
@ 2021-05-10 11:30 ` Matti Vaittinen
  2021-05-10 11:30 ` [PATCH v9 07/10] dt-bindings: regulator: bd9576 add FET ON-resistance for OCW Matti Vaittinen
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:30 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 17612 bytes --]

Add DT property parsing code and setting callback for regulator over/under
voltage, over-current and temperature error limits.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
No changes since RFC-v3
---
 drivers/regulator/core.c                  | 122 +++++++++++++++++++++-
 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 +++-
 include/linux/regulator/driver.h          |  41 +++++++-
 include/linux/regulator/machine.h         |  26 +++++
 7 files changed, 274 insertions(+), 9 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 87f53f5fbbd0..e40cb8c338dc 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1315,6 +1315,52 @@ static int machine_constraints_current(struct regulator_dev *rdev,
 
 static int _regulator_do_enable(struct regulator_dev *rdev);
 
+static int notif_set_limit(struct regulator_dev *rdev,
+			   int (*set)(struct regulator_dev *, int, int, bool),
+			   int limit, int severity)
+{
+	bool enable;
+
+	if (limit == REGULATOR_NOTIF_LIMIT_DISABLE) {
+		enable = false;
+		limit = 0;
+	} else {
+		enable = true;
+	}
+
+	if (limit == REGULATOR_NOTIF_LIMIT_ENABLE)
+		limit = 0;
+
+	return set(rdev, limit, severity, enable);
+}
+
+static int handle_notify_limits(struct regulator_dev *rdev,
+			int (*set)(struct regulator_dev *, int, int, bool),
+			struct notification_limit *limits)
+{
+	int ret = 0;
+
+	if (!set)
+		return -EOPNOTSUPP;
+
+	if (limits->prot)
+		ret = notif_set_limit(rdev, set, limits->prot,
+				      REGULATOR_SEVERITY_PROT);
+	if (ret)
+		return ret;
+
+	if (limits->err)
+		ret = notif_set_limit(rdev, set, limits->err,
+				      REGULATOR_SEVERITY_ERR);
+	if (ret)
+		return ret;
+
+	if (limits->warn)
+		ret = notif_set_limit(rdev, set, limits->warn,
+				      REGULATOR_SEVERITY_WARN);
+
+	return ret;
+}
 /**
  * set_machine_constraints - sets regulator constraints
  * @rdev: regulator source
@@ -1400,9 +1446,27 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 		}
 	}
 
+	/*
+	 * Existing logic does not warn if over_current_protection is given as
+	 * a constraint but driver does not support that. I think we should
+	 * warn about this type of issues as it is possible someone changes
+	 * PMIC on board to another type - and the another PMIC's driver does
+	 * not support setting protection. Board composer may happily believe
+	 * the DT limits are respected - especially if the new PMIC HW also
+	 * supports protection but the driver does not. I won't change the logic
+	 * without hearing more experienced opinion on this though.
+	 *
+	 * If warning is seen as a good idea then we can merge handling the
+	 * over-curret protection and detection and get rid of this special
+	 * handling.
+	 */
 	if (rdev->constraints->over_current_protection
 		&& ops->set_over_current_protection) {
-		ret = ops->set_over_current_protection(rdev);
+		int lim = rdev->constraints->over_curr_limits.prot;
+
+		ret = ops->set_over_current_protection(rdev, lim,
+						       REGULATOR_SEVERITY_PROT,
+						       true);
 		if (ret < 0) {
 			rdev_err(rdev, "failed to set over current protection: %pe\n",
 				 ERR_PTR(ret));
@@ -1410,6 +1474,62 @@ static int set_machine_constraints(struct regulator_dev *rdev)
 		}
 	}
 
+	if (rdev->constraints->over_current_detection)
+		ret = handle_notify_limits(rdev,
+					   ops->set_over_current_protection,
+					   &rdev->constraints->over_curr_limits);
+	if (ret) {
+		if (ret != -EOPNOTSUPP) {
+			rdev_err(rdev, "failed to set over current limits: %pe\n",
+				 ERR_PTR(ret));
+			return ret;
+		}
+		rdev_warn(rdev,
+			  "IC does not support requested over-current limits\n");
+	}
+
+	if (rdev->constraints->over_voltage_detection)
+		ret = handle_notify_limits(rdev,
+					   ops->set_over_voltage_protection,
+					   &rdev->constraints->over_voltage_limits);
+	if (ret) {
+		if (ret != -EOPNOTSUPP) {
+			rdev_err(rdev, "failed to set over voltage limits %pe\n",
+				 ERR_PTR(ret));
+			return ret;
+		}
+		rdev_warn(rdev,
+			  "IC does not support requested over voltage limits\n");
+	}
+
+	if (rdev->constraints->under_voltage_detection)
+		ret = handle_notify_limits(rdev,
+					   ops->set_under_voltage_protection,
+					   &rdev->constraints->under_voltage_limits);
+	if (ret) {
+		if (ret != -EOPNOTSUPP) {
+			rdev_err(rdev, "failed to set under voltage limits %pe\n",
+				 ERR_PTR(ret));
+			return ret;
+		}
+		rdev_warn(rdev,
+			  "IC does not support requested under voltage limits\n");
+	}
+
+	if (rdev->constraints->over_temp_detection)
+		ret = handle_notify_limits(rdev,
+					   ops->set_thermal_protection,
+					   &rdev->constraints->temp_limits);
+	if (ret) {
+		if (ret != -EOPNOTSUPP) {
+			rdev_err(rdev, "failed to set temperature limits %pe\n",
+				 ERR_PTR(ret));
+			return ret;
+		}
+		rdev_warn(rdev,
+			  "IC does not support requested temperature limits\n");
+	}
+
 	if (rdev->constraints->active_discharge && ops->set_active_discharge) {
 		bool ad_state = (rdev->constraints->active_discharge ==
 			      REGULATOR_ACTIVE_DISCHARGE_ENABLE) ? true : false;
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c
index 49f6c05fee34..f54d4f176882 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -21,6 +21,62 @@ static const char *const regulator_states[PM_SUSPEND_MAX + 1] = {
 	[PM_SUSPEND_MAX]	= "regulator-state-disk",
 };
 
+static void fill_limit(int *limit, int val)
+{
+	if (val)
+		if (val == 1)
+			*limit = REGULATOR_NOTIF_LIMIT_ENABLE;
+		else
+			*limit = val;
+	else
+		*limit = REGULATOR_NOTIF_LIMIT_DISABLE;
+}
+
+static void of_get_regulator_prot_limits(struct device_node *np,
+				struct regulation_constraints *constraints)
+{
+	u32 pval;
+	int i;
+	static const char *const props[] = {
+		"regulator-oc-%s-microamp",
+		"regulator-ov-%s-microvolt",
+		"regulator-temp-%s-kelvin",
+		"regulator-uv-%s-microvolt",
+	};
+	struct notification_limit *limits[] = {
+		&constraints->over_curr_limits,
+		&constraints->over_voltage_limits,
+		&constraints->temp_limits,
+		&constraints->under_voltage_limits,
+	};
+	bool set[4] = {0};
+
+	/* Protection limits: */
+	for (i = 0; i < ARRAY_SIZE(props); i++) {
+		char prop[255];
+		bool found;
+		int j;
+		static const char *const lvl[] = {
+			"protection", "error", "warn"
+		};
+		int *l[] = {
+			&limits[i]->prot, &limits[i]->err, &limits[i]->warn,
+		};
+
+		for (j = 0; j < ARRAY_SIZE(lvl); j++) {
+			snprintf(prop, 255, props[i], lvl[j]);
+			found = !of_property_read_u32(np, prop, &pval);
+			if (found)
+				fill_limit(l[j], pval);
+			set[i] |= found;
+		}
+	}
+	constraints->over_current_detection = set[0];
+	constraints->over_voltage_detection = set[1];
+	constraints->over_temp_detection = set[2];
+	constraints->under_voltage_detection = set[3];
+}
+
 static int of_get_regulation_constraints(struct device *dev,
 					struct device_node *np,
 					struct regulator_init_data **init_data,
@@ -188,6 +244,8 @@ static int of_get_regulation_constraints(struct device *dev,
 	constraints->over_current_protection = of_property_read_bool(np,
 					"regulator-over-current-protection");
 
+	of_get_regulator_prot_limits(np, constraints);
+
 	for (i = 0; i < ARRAY_SIZE(regulator_states); i++) {
 		switch (i) {
 		case PM_SUSPEND_MEM:
diff --git a/drivers/regulator/qcom-labibb-regulator.c b/drivers/regulator/qcom-labibb-regulator.c
index de25e3279b4b..b3da0dc58782 100644
--- a/drivers/regulator/qcom-labibb-regulator.c
+++ b/drivers/regulator/qcom-labibb-regulator.c
@@ -307,13 +307,21 @@ static irqreturn_t qcom_labibb_ocp_isr(int irq, void *chip)
 	return IRQ_HANDLED;
 }
 
-static int qcom_labibb_set_ocp(struct regulator_dev *rdev)
+static int qcom_labibb_set_ocp(struct regulator_dev *rdev, int lim,
+			       int severity, bool enable)
 {
 	struct labibb_regulator *vreg = rdev_get_drvdata(rdev);
 	char *ocp_irq_name;
 	u32 irq_flags = IRQF_ONESHOT;
 	int irq_trig_low, ret;
 
+	/*
+	 * labibb supports only protection - and does not support setting
+	 * limit. Furthermore, we don't support disabling protection.
+	 */
+	if (lim || severity != REGULATOR_SEVERITY_PROT || !enable)
+		return -EINVAL;
+
 	/* If there is no OCP interrupt, there's nothing to set */
 	if (vreg->ocp_irq <= 0)
 		return -EINVAL;
diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index 95677c51c1fa..41424a3366d0 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -595,11 +595,15 @@ static int spmi_regulator_vs_enable(struct regulator_dev *rdev)
 	return regulator_enable_regmap(rdev);
 }
 
-static int spmi_regulator_vs_ocp(struct regulator_dev *rdev)
+static int spmi_regulator_vs_ocp(struct regulator_dev *rdev, int lim_uA,
+				 int severity, bool enable)
 {
 	struct spmi_regulator *vreg = rdev_get_drvdata(rdev);
 	u8 reg = SPMI_VS_OCP_OVERRIDE;
 
+	if (lim_uA || !enable || severity != REGULATOR_SEVERITY_PROT)
+		return -EINVAL;
+
 	return spmi_vreg_write(vreg, SPMI_VS_REG_OCP, &reg, 1);
 }
 
diff --git a/drivers/regulator/stpmic1_regulator.c b/drivers/regulator/stpmic1_regulator.c
index cf10fdb72e32..2d7597c76e4a 100644
--- a/drivers/regulator/stpmic1_regulator.c
+++ b/drivers/regulator/stpmic1_regulator.c
@@ -32,7 +32,8 @@ struct stpmic1_regulator_cfg {
 
 static int stpmic1_set_mode(struct regulator_dev *rdev, unsigned int mode);
 static unsigned int stpmic1_get_mode(struct regulator_dev *rdev);
-static int stpmic1_set_icc(struct regulator_dev *rdev);
+static int stpmic1_set_icc(struct regulator_dev *rdev, int lim, int severity,
+			   bool enable);
 static unsigned int stpmic1_map_mode(unsigned int mode);
 
 enum {
@@ -491,11 +492,26 @@ static int stpmic1_set_mode(struct regulator_dev *rdev, unsigned int mode)
 				  STPMIC1_BUCK_MODE_LP, value);
 }
 
-static int stpmic1_set_icc(struct regulator_dev *rdev)
+static int stpmic1_set_icc(struct regulator_dev *rdev, int lim, int severity,
+			   bool enable)
 {
 	struct stpmic1_regulator_cfg *cfg = rdev_get_drvdata(rdev);
 	struct regmap *regmap = rdev_get_regmap(rdev);
 
+	/*
+	 * The code seems like one bit in a register controls whether OCP is
+	 * enabled. So we might be able to turn it off here is if that
+	 * was requested. I won't support this because I don't have the HW.
+	 * Feel free to try and implement if you have the HW and need kernel
+	 * to disable this.
+	 *
+	 * Also, I don't know if limit can be configured or if we support
+	 * error/warning instead of protect. So I just keep existing logic
+	 * and assume no.
+	 */
+	if (lim || severity != REGULATOR_SEVERITY_PROT || !enable)
+		return -EINVAL;
+
 	/* enable switch off in case of over current */
 	return regmap_update_bits(regmap, cfg->icc_reg, cfg->icc_mask,
 				  cfg->icc_mask);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index be301b28838a..eb979b810639 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -40,6 +40,15 @@ enum regulator_status {
 	REGULATOR_STATUS_UNDEFINED,
 };
 
+enum regulator_detection_severity {
+	/* Hardware shut down voltage outputs if condition is detected */
+	REGULATOR_SEVERITY_PROT,
+	/* Hardware is probably damaged/inoperable */
+	REGULATOR_SEVERITY_ERR,
+	/* Hardware is still recoverable but recovery action must be taken */
+	REGULATOR_SEVERITY_WARN,
+};
+
 /* Initialize struct linear_range for regulators */
 #define REGULATOR_LINEAR_RANGE(_min_uV, _min_sel, _max_sel, _step_uV)	\
 {									\
@@ -78,8 +87,25 @@ enum regulator_status {
  * @get_current_limit: Get the configured limit for a current-limited regulator.
  * @set_input_current_limit: Configure an input limit.
  *
- * @set_over_current_protection: Support capability of automatically shutting
- *                               down when detecting an over current event.
+ * @set_over_current_protection: Support enabling of and setting limits for over
+ *	current situation detection. Detection can be configured for three
+ *	levels of severity.
+ *	REGULATOR_SEVERITY_PROT should automatically shut down the regulator(s).
+ *	REGULATOR_SEVERITY_ERR should indicate that over-current situation is
+ *		caused by an unrecoverable error but HW does not perform
+ *		automatic shut down.
+ *	REGULATOR_SEVERITY_WARN should indicate situation where hardware is
+ *		still believed to not be damaged but that a board sepcific
+ *		recovery action is needed. If lim_uA is 0 the limit should not
+ *		be changed but the detection should just be enabled/disabled as
+ *		is requested.
+ * @set_over_voltage_protection: Support enabling of and setting limits for over
+ *	voltage situation detection. Detection can be configured for same
+ *	severities as over current protection.
+ * @set_under_voltage_protection: Support enabling of and setting limits for
+ *	under situation detection.
+ * @set_thermal_protection: Support enabling of and setting limits for over
+ *	temperature situation detection.
  *
  * @set_active_discharge: Set active discharge enable/disable of regulators.
  *
@@ -143,8 +169,15 @@ struct regulator_ops {
 	int (*get_current_limit) (struct regulator_dev *);
 
 	int (*set_input_current_limit) (struct regulator_dev *, int lim_uA);
-	int (*set_over_current_protection) (struct regulator_dev *);
-	int (*set_active_discharge) (struct regulator_dev *, bool enable);
+	int (*set_over_current_protection)(struct regulator_dev *, int lim_uA,
+					   int severity, bool enable);
+	int (*set_over_voltage_protection)(struct regulator_dev *, int lim_uV,
+					   int severity, bool enable);
+	int (*set_under_voltage_protection)(struct regulator_dev *, int lim_uV,
+					    int severity, bool enable);
+	int (*set_thermal_protection)(struct regulator_dev *, int lim,
+				      int severity, bool enable);
+	int (*set_active_discharge)(struct regulator_dev *, bool enable);
 
 	/* enable/disable regulator */
 	int (*enable) (struct regulator_dev *);
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 8a56f033b6cd..68b4a514a410 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -83,6 +83,14 @@ struct regulator_state {
 	bool changeable;
 };
 
+#define REGULATOR_NOTIF_LIMIT_DISABLE -1
+#define REGULATOR_NOTIF_LIMIT_ENABLE -2
+struct notification_limit {
+	int prot;
+	int err;
+	int warn;
+};
+
 /**
  * struct regulation_constraints - regulator operating constraints.
  *
@@ -100,6 +108,11 @@ struct regulator_state {
  * @ilim_uA: Maximum input current.
  * @system_load: Load that isn't captured by any consumer requests.
  *
+ * @over_curr_limits:		Limits for acting on over current.
+ * @over_voltage_limits:	Limits for acting on over voltage.
+ * @under_voltage_limits:	Limits for acting on under voltage.
+ * @temp_limits:		Limits for acting on over temperature.
+
  * @max_spread: Max possible spread between coupled regulators
  * @max_uV_step: Max possible step change in voltage
  * @valid_modes_mask: Mask of modes which may be configured by consumers.
@@ -116,6 +129,11 @@ struct regulator_state {
  * @pull_down: Enable pull down when regulator is disabled.
  * @over_current_protection: Auto disable on over current event.
  *
+ * @over_current_detection: Configure over current limits.
+ * @over_voltage_detection: Configure over voltage limits.
+ * @under_voltage_detection: Configure under voltage limits.
+ * @over_temp_detection: Configure over temperature limits.
+ *
  * @input_uV: Input voltage for regulator when supplied by another regulator.
  *
  * @state_disk: State for regulator when system is suspended in disk mode.
@@ -172,6 +190,10 @@ struct regulation_constraints {
 	struct regulator_state state_disk;
 	struct regulator_state state_mem;
 	struct regulator_state state_standby;
+	struct notification_limit over_curr_limits;
+	struct notification_limit over_voltage_limits;
+	struct notification_limit under_voltage_limits;
+	struct notification_limit temp_limits;
 	suspend_state_t initial_state; /* suspend state to set at init */
 
 	/* mode to set on startup */
@@ -193,6 +215,10 @@ struct regulation_constraints {
 	unsigned soft_start:1;	/* ramp voltage slowly */
 	unsigned pull_down:1;	/* pull down resistor when regulator off */
 	unsigned over_current_protection:1; /* auto disable on over current */
+	unsigned over_current_detection:1; /* notify on over current */
+	unsigned over_voltage_detection:1; /* notify on over voltage */
+	unsigned under_voltage_detection:1; /* notify on under voltage */
+	unsigned over_temp_detection:1; /* notify on over temperature */
 };
 
 /**
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 07/10] dt-bindings: regulator: bd9576 add FET ON-resistance for OCW
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
                   ` (5 preceding siblings ...)
  2021-05-10 11:30 ` [PATCH v9 06/10] regulator: add property parsing and callbacks to set protection limits Matti Vaittinen
@ 2021-05-10 11:30 ` Matti Vaittinen
  2021-05-10 11:30 ` [PATCH v9 08/10] regulator: bd9576: Support error reporting Matti Vaittinen
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:30 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

BD9576MUF provides over-current protection and detection. Current is
measured as voltage loss over external FET. Allow specifying FET's on
resistance so current monitoring limits can be converted to voltages.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Reviewed-by: Rob Herring <robh@kernel.org>

---
v5 onwards:
  - No changes
v4:
  - Fixed the description indentiation
---
 .../bindings/regulator/rohm,bd9576-regulator.yaml           | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml b/Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml
index b6515a0cee62..7cb74cc8c5d9 100644
--- a/Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/rohm,bd9576-regulator.yaml
@@ -27,6 +27,12 @@ patternProperties:
       Properties for single regulator.
     $ref: "regulator.yaml#"
 
+    properties:
+      rohm,ocw-fet-ron-micro-ohms:
+        description: |
+          External FET's ON-resistance. Required if VoutS1 OCP/OCW is
+          to be set.
+
     required:
       - regulator-name
 
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 08/10] regulator: bd9576: Support error reporting
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
                   ` (6 preceding siblings ...)
  2021-05-10 11:30 ` [PATCH v9 07/10] dt-bindings: regulator: bd9576 add FET ON-resistance for OCW Matti Vaittinen
@ 2021-05-10 11:30 ` Matti Vaittinen
  2021-05-10 11:31 ` [PATCH v9 09/10] regulator: bd9576: Fix the driver name in id table Matti Vaittinen
  2021-05-10 11:31 ` [PATCH v9 10/10] MAINTAINERS: Add reviewer for regulator irq_helpers Matti Vaittinen
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:30 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 36884 bytes --]

BD9573 and BD9576 support set of "protection" interrupts for "fatal"
issues. Those lead to SOC reset as PMIC shuts the power outputs. Thus
there is no relevant IRQ handling for them.

Few "detection" interrupts were added to the BD9576 with the idea that
SOC could take some recovery-action before error gets unrecoverable.

Add support for over and under voltage detection for Vout1 ... Vout4
and VoutL1. Add over-current detection for VoutS1 and finally a
thermal warning (common for all regulators) which alerts 30 C
before temperature reaches the thermal shutdown point. This way
consumer drivers can build error-recovery mechanisms.

Unfortunately the BD9576 interrupt logic was not re-evaluated. IRQs
are not designed to be properly acknowleged - and IRQ line is kept
active for whole duration of error condition (in comparison to
informing only about state change).

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
v9:
  - Rebased on v5.13-rc1 (probe return value fixes)
No changes since RFC-v3
---
 drivers/regulator/bd9576-regulator.c | 1050 +++++++++++++++++++++++---
 1 file changed, 926 insertions(+), 124 deletions(-)

diff --git a/drivers/regulator/bd9576-regulator.c b/drivers/regulator/bd9576-regulator.c
index 204a2da054f5..6ba12af4c632 100644
--- a/drivers/regulator/bd9576-regulator.c
+++ b/drivers/regulator/bd9576-regulator.c
@@ -2,10 +2,10 @@
 // Copyright (C) 2020 ROHM Semiconductors
 // ROHM BD9576MUF/BD9573MUF regulator driver
 
-#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
+#include <linux/jiffies.h>
 #include <linux/kernel.h>
 #include <linux/mfd/rohm-bd957x.h>
 #include <linux/mfd/rohm-generic.h>
@@ -16,11 +16,18 @@
 #include <linux/regulator/machine.h>
 #include <linux/regulator/of_regulator.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
 
 #define BD957X_VOUTS1_VOLT	3300000
 #define BD957X_VOUTS4_BASE_VOLT	1030000
 #define BD957X_VOUTS34_NUM_VOLT	32
 
+#define BD9576_THERM_IRQ_MASK_TW	BIT(5)
+#define BD9576_xVD_IRQ_MASK_VOUTL1	BIT(5)
+#define BD9576_UVD_IRQ_MASK_VOUTS1_OCW	BIT(6)
+#define BD9576_xVD_IRQ_MASK_VOUT1TO4	0x0F
+
 static int vout1_volt_table[] = {5000000, 4900000, 4800000, 4700000, 4600000,
 				 4500000, 4500000, 4500000, 5000000, 5100000,
 				 5200000, 5300000, 5400000, 5500000, 5500000,
@@ -36,9 +43,85 @@ static int voutl1_volt_table[] = {2500000, 2540000, 2580000, 2620000, 2660000,
 				  2420000, 2380000, 2340000, 2300000, 2260000,
 				  2220000};
 
+static const struct linear_range vout1_xvd_ranges[] = {
+	REGULATOR_LINEAR_RANGE(225000, 0x01, 0x2b, 0),
+	REGULATOR_LINEAR_RANGE(225000, 0x2c, 0x54, 5000),
+	REGULATOR_LINEAR_RANGE(425000, 0x55, 0x7f, 0),
+};
+
+static const struct linear_range vout234_xvd_ranges[] = {
+	REGULATOR_LINEAR_RANGE(17000, 0x01, 0x0f, 0),
+	REGULATOR_LINEAR_RANGE(17000, 0x10, 0x6d, 1000),
+	REGULATOR_LINEAR_RANGE(110000, 0x6e, 0x7f, 0),
+};
+
+static const struct linear_range voutL1_xvd_ranges[] = {
+	REGULATOR_LINEAR_RANGE(34000, 0x01, 0x0f, 0),
+	REGULATOR_LINEAR_RANGE(34000, 0x10, 0x6d, 2000),
+	REGULATOR_LINEAR_RANGE(220000, 0x6e, 0x7f, 0),
+};
+
+static struct linear_range voutS1_ocw_ranges_internal[] = {
+	REGULATOR_LINEAR_RANGE(200000, 0x01, 0x04, 0),
+	REGULATOR_LINEAR_RANGE(250000, 0x05, 0x18, 50000),
+	REGULATOR_LINEAR_RANGE(1200000, 0x19, 0x3f, 0),
+};
+
+static struct linear_range voutS1_ocw_ranges[] = {
+	REGULATOR_LINEAR_RANGE(50000, 0x01, 0x04, 0),
+	REGULATOR_LINEAR_RANGE(60000, 0x05, 0x18, 10000),
+	REGULATOR_LINEAR_RANGE(250000, 0x19, 0x3f, 0),
+};
+
+static struct linear_range voutS1_ocp_ranges_internal[] = {
+	REGULATOR_LINEAR_RANGE(300000, 0x01, 0x06, 0),
+	REGULATOR_LINEAR_RANGE(350000, 0x7, 0x1b, 50000),
+	REGULATOR_LINEAR_RANGE(1350000, 0x1c, 0x3f, 0),
+};
+
+static struct linear_range voutS1_ocp_ranges[] = {
+	REGULATOR_LINEAR_RANGE(70000, 0x01, 0x06, 0),
+	REGULATOR_LINEAR_RANGE(80000, 0x7, 0x1b, 10000),
+	REGULATOR_LINEAR_RANGE(280000, 0x1c, 0x3f, 0),
+};
+
 struct bd957x_regulator_data {
 	struct regulator_desc desc;
 	int base_voltage;
+	struct regulator_dev *rdev;
+	int ovd_notif;
+	int uvd_notif;
+	int temp_notif;
+	int ovd_err;
+	int uvd_err;
+	int temp_err;
+	const struct linear_range *xvd_ranges;
+	int num_xvd_ranges;
+	bool oc_supported;
+	unsigned int ovd_reg;
+	unsigned int uvd_reg;
+	unsigned int xvd_mask;
+	unsigned int ocp_reg;
+	unsigned int ocp_mask;
+	unsigned int ocw_reg;
+	unsigned int ocw_mask;
+	unsigned int ocw_rfet;
+};
+
+#define BD9576_NUM_REGULATORS 6
+#define BD9576_NUM_OVD_REGULATORS 5
+
+struct bd957x_data {
+	struct bd957x_regulator_data regulator_data[BD9576_NUM_REGULATORS];
+	struct regmap *regmap;
+	struct delayed_work therm_irq_suppress;
+	struct delayed_work ovd_irq_suppress;
+	struct delayed_work uvd_irq_suppress;
+	unsigned int therm_irq;
+	unsigned int ovd_irq;
+	unsigned int uvd_irq;
+	spinlock_t err_lock;
+	int regulator_global_err;
 };
 
 static int bd957x_vout34_list_voltage(struct regulator_dev *rdev,
@@ -72,151 +155,784 @@ static int bd957x_list_voltage(struct regulator_dev *rdev,
 	return desc->volt_table[index];
 }
 
-static const struct regulator_ops bd957x_vout34_ops = {
+static void bd9576_fill_ovd_flags(struct bd957x_regulator_data *data,
+				  bool warn)
+{
+	if (warn) {
+		data->ovd_notif = REGULATOR_EVENT_OVER_VOLTAGE_WARN;
+		data->ovd_err = REGULATOR_ERROR_OVER_VOLTAGE_WARN;
+	} else {
+		data->ovd_notif = REGULATOR_EVENT_REGULATION_OUT;
+		data->ovd_err = REGULATOR_ERROR_REGULATION_OUT;
+	}
+}
+
+static void bd9576_fill_ocp_flags(struct bd957x_regulator_data *data,
+				  bool warn)
+{
+	if (warn) {
+		data->uvd_notif = REGULATOR_EVENT_OVER_CURRENT_WARN;
+		data->uvd_err = REGULATOR_ERROR_OVER_CURRENT_WARN;
+	} else {
+		data->uvd_notif = REGULATOR_EVENT_OVER_CURRENT;
+		data->uvd_err = REGULATOR_ERROR_OVER_CURRENT;
+	}
+}
+
+static void bd9576_fill_uvd_flags(struct bd957x_regulator_data *data,
+				  bool warn)
+{
+	if (warn) {
+		data->uvd_notif = REGULATOR_EVENT_UNDER_VOLTAGE_WARN;
+		data->uvd_err = REGULATOR_ERROR_UNDER_VOLTAGE_WARN;
+	} else {
+		data->uvd_notif = REGULATOR_EVENT_UNDER_VOLTAGE;
+		data->uvd_err = REGULATOR_ERROR_UNDER_VOLTAGE;
+	}
+}
+
+static void bd9576_fill_temp_flags(struct bd957x_regulator_data *data,
+				   bool enable, bool warn)
+{
+	if (!enable) {
+		data->temp_notif = 0;
+		data->temp_err = 0;
+	} else if (warn) {
+		data->temp_notif = REGULATOR_EVENT_OVER_TEMP_WARN;
+		data->temp_err = REGULATOR_ERROR_OVER_TEMP_WARN;
+	} else {
+		data->temp_notif = REGULATOR_EVENT_OVER_TEMP;
+		data->temp_err = REGULATOR_ERROR_OVER_TEMP;
+	}
+}
+
+static int bd9576_set_limit(const struct linear_range *r, int num_ranges,
+			    struct regmap *regmap, int reg, int mask, int lim)
+{
+	int ret;
+	bool found;
+	int sel = 0;
+
+	if (lim) {
+
+		ret = linear_range_get_selector_low_array(r, num_ranges,
+							  lim, &sel, &found);
+		if (ret)
+			return ret;
+
+		if (!found)
+			dev_warn(regmap_get_device(regmap),
+				 "limit %d out of range. Setting lower\n",
+				 lim);
+	}
+
+	return regmap_update_bits(regmap, reg, mask, sel);
+}
+
+static bool check_ocp_flag_mismatch(struct regulator_dev *rdev, int severity,
+				    struct bd957x_regulator_data *r)
+{
+	if ((severity == REGULATOR_SEVERITY_ERR &&
+	    r->uvd_notif != REGULATOR_EVENT_OVER_CURRENT) ||
+	    (severity == REGULATOR_SEVERITY_WARN &&
+	    r->uvd_notif != REGULATOR_EVENT_OVER_CURRENT_WARN)) {
+		dev_warn(rdev_get_dev(rdev),
+			 "Can't support both OCP WARN and ERR\n");
+		/* Do not overwrite ERR config with WARN */
+		if (severity == REGULATOR_SEVERITY_WARN)
+			return true;
+
+		bd9576_fill_ocp_flags(r, 0);
+	}
+
+	return false;
+}
+
+static bool check_uvd_flag_mismatch(struct regulator_dev *rdev, int severity,
+				    struct bd957x_regulator_data *r)
+{
+	if ((severity == REGULATOR_SEVERITY_ERR &&
+	     r->uvd_notif != REGULATOR_EVENT_UNDER_VOLTAGE) ||
+	     (severity == REGULATOR_SEVERITY_WARN &&
+	     r->uvd_notif != REGULATOR_EVENT_UNDER_VOLTAGE_WARN)) {
+		dev_warn(rdev_get_dev(rdev),
+			 "Can't support both UVD WARN and ERR\n");
+		if (severity == REGULATOR_SEVERITY_WARN)
+			return true;
+
+		bd9576_fill_uvd_flags(r, 0);
+	}
+
+	return false;
+}
+
+static bool check_ovd_flag_mismatch(struct regulator_dev *rdev, int severity,
+				    struct bd957x_regulator_data *r)
+{
+	if ((severity == REGULATOR_SEVERITY_ERR &&
+	     r->ovd_notif != REGULATOR_EVENT_REGULATION_OUT) ||
+	     (severity == REGULATOR_SEVERITY_WARN &&
+	     r->ovd_notif != REGULATOR_EVENT_OVER_VOLTAGE_WARN)) {
+		dev_warn(rdev_get_dev(rdev),
+			 "Can't support both OVD WARN and ERR\n");
+		if (severity == REGULATOR_SEVERITY_WARN)
+			return true;
+
+		bd9576_fill_ovd_flags(r, 0);
+	}
+
+	return false;
+}
+
+static bool check_temp_flag_mismatch(struct regulator_dev *rdev, int severity,
+				    struct bd957x_regulator_data *r)
+{
+	if ((severity == REGULATOR_SEVERITY_ERR &&
+	     r->ovd_notif != REGULATOR_EVENT_OVER_TEMP) ||
+	     (severity == REGULATOR_SEVERITY_WARN &&
+	     r->ovd_notif != REGULATOR_EVENT_OVER_TEMP_WARN)) {
+		dev_warn(rdev_get_dev(rdev),
+			 "Can't support both thermal WARN and ERR\n");
+		if (severity == REGULATOR_SEVERITY_WARN)
+			return true;
+	}
+
+	return false;
+}
+
+static int bd9576_set_ocp(struct regulator_dev *rdev, int lim_uA, int severity,
+			  bool enable)
+{
+	struct bd957x_data *d;
+	struct bd957x_regulator_data *r;
+	int reg, mask;
+	int Vfet, rfet;
+	const struct linear_range *range;
+	int num_ranges;
+
+	if ((lim_uA && !enable) || (!lim_uA && enable))
+		return -EINVAL;
+
+	r = container_of(rdev->desc, struct bd957x_regulator_data, desc);
+	if (!r->oc_supported)
+		return -EINVAL;
+
+	d = rdev_get_drvdata(rdev);
+
+	if (severity == REGULATOR_SEVERITY_PROT) {
+		reg = r->ocp_reg;
+		mask = r->ocp_mask;
+		if (r->ocw_rfet) {
+			range = voutS1_ocp_ranges;
+			num_ranges = ARRAY_SIZE(voutS1_ocp_ranges);
+			rfet = r->ocw_rfet / 1000;
+		} else {
+			range = voutS1_ocp_ranges_internal;
+			num_ranges = ARRAY_SIZE(voutS1_ocp_ranges_internal);
+			/* Internal values are already micro-amperes */
+			rfet = 1000;
+		}
+	} else {
+		reg = r->ocw_reg;
+		mask = r->ocw_mask;
+
+		if (r->ocw_rfet) {
+			range = voutS1_ocw_ranges;
+			num_ranges = ARRAY_SIZE(voutS1_ocw_ranges);
+			rfet = r->ocw_rfet / 1000;
+		} else {
+			range = voutS1_ocw_ranges_internal;
+			num_ranges = ARRAY_SIZE(voutS1_ocw_ranges_internal);
+			/* Internal values are already micro-amperes */
+			rfet = 1000;
+		}
+
+		/* We abuse uvd fields for OCW on VoutS1 */
+		if (r->uvd_notif) {
+			/*
+			 * If both warning and error are requested, prioritize
+			 * ERROR configuration
+			 */
+			if (check_ocp_flag_mismatch(rdev, severity, r))
+				return 0;
+		} else {
+			bool warn = severity == REGULATOR_SEVERITY_WARN;
+
+			bd9576_fill_ocp_flags(r, warn);
+		}
+	}
+
+	/*
+	 * limits are given in uA, rfet is mOhm
+	 * Divide lim_uA by 1000 to get Vfet in uV.
+	 * (We expect both Rfet and limit uA to be magnitude of hundreds of
+	 * milli Amperes & milli Ohms => we should still have decent accuracy)
+	 */
+	Vfet = lim_uA/1000 * rfet;
+
+	return bd9576_set_limit(range, num_ranges, d->regmap,
+				reg, mask, Vfet);
+}
+
+static int bd9576_set_uvp(struct regulator_dev *rdev, int lim_uV, int severity,
+			  bool enable)
+{
+	struct bd957x_data *d;
+	struct bd957x_regulator_data *r;
+	int mask, reg;
+
+	if (severity == REGULATOR_SEVERITY_PROT) {
+		if (!enable || lim_uV)
+			return -EINVAL;
+		return 0;
+	}
+
+	/*
+	 * BD9576 has enable control as a special value in limit reg. Can't
+	 * set limit but keep feature disabled or enable W/O given limit.
+	 */
+	if ((lim_uV && !enable) || (!lim_uV && enable))
+		return -EINVAL;
+
+	r = container_of(rdev->desc, struct bd957x_regulator_data, desc);
+	d = rdev_get_drvdata(rdev);
+
+	mask = r->xvd_mask;
+	reg = r->uvd_reg;
+	/*
+	 * Check that there is no mismatch for what the detection IRQs are to
+	 * be used.
+	 */
+	if (r->uvd_notif) {
+		if (check_uvd_flag_mismatch(rdev, severity, r))
+			return 0;
+	} else {
+		bd9576_fill_uvd_flags(r, severity == REGULATOR_SEVERITY_WARN);
+	}
+
+	return bd9576_set_limit(r->xvd_ranges, r->num_xvd_ranges, d->regmap,
+				reg, mask, lim_uV);
+}
+
+static int bd9576_set_ovp(struct regulator_dev *rdev, int lim_uV, int severity,
+			  bool enable)
+{
+	struct bd957x_data *d;
+	struct bd957x_regulator_data *r;
+	int mask, reg;
+
+	if (severity == REGULATOR_SEVERITY_PROT) {
+		if (!enable || lim_uV)
+			return -EINVAL;
+		return 0;
+	}
+
+	/*
+	 * BD9576 has enable control as a special value in limit reg. Can't
+	 * set limit but keep feature disabled or enable W/O given limit.
+	 */
+	if ((lim_uV && !enable) || (!lim_uV && enable))
+		return -EINVAL;
+
+	r = container_of(rdev->desc, struct bd957x_regulator_data, desc);
+	d = rdev_get_drvdata(rdev);
+
+	mask = r->xvd_mask;
+	reg = r->ovd_reg;
+	/*
+	 * Check that there is no mismatch for what the detection IRQs are to
+	 * be used.
+	 */
+	if (r->ovd_notif) {
+		if (check_ovd_flag_mismatch(rdev, severity, r))
+			return 0;
+	} else {
+		bd9576_fill_ovd_flags(r, severity == REGULATOR_SEVERITY_WARN);
+	}
+
+	return bd9576_set_limit(r->xvd_ranges, r->num_xvd_ranges, d->regmap,
+				reg, mask, lim_uV);
+}
+
+
+static int bd9576_set_tw(struct regulator_dev *rdev, int lim, int severity,
+			  bool enable)
+{
+	struct bd957x_data *d;
+	struct bd957x_regulator_data *r;
+	int i;
+
+	/*
+	 * BD9576MUF has fixed temperature limits
+	 * The detection can only be enabled/disabled
+	 */
+	if (lim)
+		return -EINVAL;
+
+	/* Protection can't be disabled */
+	if (severity == REGULATOR_SEVERITY_PROT) {
+		if (!enable)
+			return -EINVAL;
+		else
+			return 0;
+	}
+
+	r = container_of(rdev->desc, struct bd957x_regulator_data, desc);
+	d = rdev_get_drvdata(rdev);
+
+	/*
+	 * Check that there is no mismatch for what the detection IRQs are to
+	 * be used.
+	 */
+	if (r->temp_notif)
+		if (check_temp_flag_mismatch(rdev, severity, r))
+			return 0;
+
+	bd9576_fill_temp_flags(r, enable, severity == REGULATOR_SEVERITY_WARN);
+
+	if (enable)
+		return regmap_update_bits(d->regmap, BD957X_REG_INT_THERM_MASK,
+					 BD9576_THERM_IRQ_MASK_TW, 0);
+
+	/*
+	 * If any of the regulators is interested in thermal warning we keep IRQ
+	 * enabled.
+	 */
+	for (i = 0; i < BD9576_NUM_REGULATORS; i++)
+		if (d->regulator_data[i].temp_notif)
+			return 0;
+
+	return regmap_update_bits(d->regmap, BD957X_REG_INT_THERM_MASK,
+				  BD9576_THERM_IRQ_MASK_TW,
+				  BD9576_THERM_IRQ_MASK_TW);
+}
+
+static const struct regulator_ops bd9573_vout34_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.list_voltage = bd957x_vout34_list_voltage,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 };
 
-static const struct regulator_ops bd957X_vouts1_regulator_ops = {
+static const struct regulator_ops bd9576_vout34_ops = {
+	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = bd957x_vout34_list_voltage,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_over_voltage_protection = bd9576_set_ovp,
+	.set_under_voltage_protection = bd9576_set_uvp,
+	.set_thermal_protection = bd9576_set_tw,
+};
+
+static const struct regulator_ops bd9573_vouts1_regulator_ops = {
+	.is_enabled = regulator_is_enabled_regmap,
+};
+
+static const struct regulator_ops bd9576_vouts1_regulator_ops = {
+	.is_enabled = regulator_is_enabled_regmap,
+	.set_over_current_protection = bd9576_set_ocp,
+};
+
+static const struct regulator_ops bd9573_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
+	.list_voltage = bd957x_list_voltage,
+	.get_voltage_sel = regulator_get_voltage_sel_regmap,
 };
 
-static const struct regulator_ops bd957x_ops = {
+static const struct regulator_ops bd9576_ops = {
 	.is_enabled = regulator_is_enabled_regmap,
 	.list_voltage = bd957x_list_voltage,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_over_voltage_protection = bd9576_set_ovp,
+	.set_under_voltage_protection = bd9576_set_uvp,
+	.set_thermal_protection = bd9576_set_tw,
+};
+
+static const struct regulator_ops  *bd9573_ops_arr[] = {
+	[BD957X_VD50]	= &bd9573_ops,
+	[BD957X_VD18]	= &bd9573_ops,
+	[BD957X_VDDDR]	= &bd9573_vout34_ops,
+	[BD957X_VD10]	= &bd9573_vout34_ops,
+	[BD957X_VOUTL1]	= &bd9573_ops,
+	[BD957X_VOUTS1]	= &bd9573_vouts1_regulator_ops,
 };
 
-static struct bd957x_regulator_data bd9576_regulators[] = {
-	{
-		.desc = {
-			.name = "VD50",
-			.of_match = of_match_ptr("regulator-vd50"),
-			.regulators_node = of_match_ptr("regulators"),
-			.id = BD957X_VD50,
-			.type = REGULATOR_VOLTAGE,
-			.ops = &bd957x_ops,
-			.volt_table = &vout1_volt_table[0],
-			.n_voltages = ARRAY_SIZE(vout1_volt_table),
-			.vsel_reg = BD957X_REG_VOUT1_TUNE,
-			.vsel_mask = BD957X_MASK_VOUT1_TUNE,
-			.enable_reg = BD957X_REG_POW_TRIGGER1,
-			.enable_mask = BD957X_REGULATOR_EN_MASK,
-			.enable_val = BD957X_REGULATOR_DIS_VAL,
-			.enable_is_inverted = true,
-			.owner = THIS_MODULE,
+static const struct regulator_ops  *bd9576_ops_arr[] = {
+	[BD957X_VD50]	= &bd9576_ops,
+	[BD957X_VD18]	= &bd9576_ops,
+	[BD957X_VDDDR]	= &bd9576_vout34_ops,
+	[BD957X_VD10]	= &bd9576_vout34_ops,
+	[BD957X_VOUTL1]	= &bd9576_ops,
+	[BD957X_VOUTS1]	= &bd9576_vouts1_regulator_ops,
+};
+
+static int vouts1_get_fet_res(struct device_node *np,
+				const struct regulator_desc *desc,
+				struct regulator_config *cfg)
+{
+	struct bd957x_regulator_data *data;
+	int ret;
+	u32 uohms;
+
+	data = container_of(desc, struct bd957x_regulator_data, desc);
+
+	ret = of_property_read_u32(np, "rohm,ocw-fet-ron-micro-ohms", &uohms);
+	if (ret) {
+		if (ret != -EINVAL)
+			return ret;
+
+		return 0;
+	}
+	data->ocw_rfet = uohms;
+	return 0;
+}
+
+static struct bd957x_data bd957x_regulators = {
+	.regulator_data = {
+		{
+			.desc = {
+				.name = "VD50",
+				.of_match = of_match_ptr("regulator-vd50"),
+				.regulators_node = of_match_ptr("regulators"),
+				.id = BD957X_VD50,
+				.type = REGULATOR_VOLTAGE,
+				.volt_table = &vout1_volt_table[0],
+				.n_voltages = ARRAY_SIZE(vout1_volt_table),
+				.vsel_reg = BD957X_REG_VOUT1_TUNE,
+				.vsel_mask = BD957X_MASK_VOUT1_TUNE,
+				.enable_reg = BD957X_REG_POW_TRIGGER1,
+				.enable_mask = BD957X_REGULATOR_EN_MASK,
+				.enable_val = BD957X_REGULATOR_DIS_VAL,
+				.enable_is_inverted = true,
+				.owner = THIS_MODULE,
+			},
+			.xvd_ranges = vout1_xvd_ranges,
+			.num_xvd_ranges = ARRAY_SIZE(vout1_xvd_ranges),
+			.ovd_reg = BD9576_REG_VOUT1_OVD,
+			.uvd_reg = BD9576_REG_VOUT1_UVD,
+			.xvd_mask = BD9576_MASK_XVD,
 		},
-	},
-	{
-		.desc = {
-			.name = "VD18",
-			.of_match = of_match_ptr("regulator-vd18"),
-			.regulators_node = of_match_ptr("regulators"),
-			.id = BD957X_VD18,
-			.type = REGULATOR_VOLTAGE,
-			.ops = &bd957x_ops,
-			.volt_table = &vout2_volt_table[0],
-			.n_voltages = ARRAY_SIZE(vout2_volt_table),
-			.vsel_reg = BD957X_REG_VOUT2_TUNE,
-			.vsel_mask = BD957X_MASK_VOUT2_TUNE,
-			.enable_reg = BD957X_REG_POW_TRIGGER2,
-			.enable_mask = BD957X_REGULATOR_EN_MASK,
-			.enable_val = BD957X_REGULATOR_DIS_VAL,
-			.enable_is_inverted = true,
-			.owner = THIS_MODULE,
+		{
+			.desc = {
+				.name = "VD18",
+				.of_match = of_match_ptr("regulator-vd18"),
+				.regulators_node = of_match_ptr("regulators"),
+				.id = BD957X_VD18,
+				.type = REGULATOR_VOLTAGE,
+				.volt_table = &vout2_volt_table[0],
+				.n_voltages = ARRAY_SIZE(vout2_volt_table),
+				.vsel_reg = BD957X_REG_VOUT2_TUNE,
+				.vsel_mask = BD957X_MASK_VOUT2_TUNE,
+				.enable_reg = BD957X_REG_POW_TRIGGER2,
+				.enable_mask = BD957X_REGULATOR_EN_MASK,
+				.enable_val = BD957X_REGULATOR_DIS_VAL,
+				.enable_is_inverted = true,
+				.owner = THIS_MODULE,
+			},
+			.xvd_ranges = vout234_xvd_ranges,
+			.num_xvd_ranges = ARRAY_SIZE(vout234_xvd_ranges),
+			.ovd_reg = BD9576_REG_VOUT2_OVD,
+			.uvd_reg = BD9576_REG_VOUT2_UVD,
+			.xvd_mask = BD9576_MASK_XVD,
 		},
-	},
-	{
-		.desc = {
-			.name = "VDDDR",
-			.of_match = of_match_ptr("regulator-vdddr"),
-			.regulators_node = of_match_ptr("regulators"),
-			.id = BD957X_VDDDR,
-			.ops = &bd957x_vout34_ops,
-			.type = REGULATOR_VOLTAGE,
-			.n_voltages = BD957X_VOUTS34_NUM_VOLT,
-			.vsel_reg = BD957X_REG_VOUT3_TUNE,
-			.vsel_mask = BD957X_MASK_VOUT3_TUNE,
-			.enable_reg = BD957X_REG_POW_TRIGGER3,
-			.enable_mask = BD957X_REGULATOR_EN_MASK,
-			.enable_val = BD957X_REGULATOR_DIS_VAL,
-			.enable_is_inverted = true,
-			.owner = THIS_MODULE,
+		{
+			.desc = {
+				.name = "VDDDR",
+				.of_match = of_match_ptr("regulator-vdddr"),
+				.regulators_node = of_match_ptr("regulators"),
+				.id = BD957X_VDDDR,
+				.type = REGULATOR_VOLTAGE,
+				.n_voltages = BD957X_VOUTS34_NUM_VOLT,
+				.vsel_reg = BD957X_REG_VOUT3_TUNE,
+				.vsel_mask = BD957X_MASK_VOUT3_TUNE,
+				.enable_reg = BD957X_REG_POW_TRIGGER3,
+				.enable_mask = BD957X_REGULATOR_EN_MASK,
+				.enable_val = BD957X_REGULATOR_DIS_VAL,
+				.enable_is_inverted = true,
+				.owner = THIS_MODULE,
+			},
+			.ovd_reg = BD9576_REG_VOUT3_OVD,
+			.uvd_reg = BD9576_REG_VOUT3_UVD,
+			.xvd_mask = BD9576_MASK_XVD,
+			.xvd_ranges = vout234_xvd_ranges,
+			.num_xvd_ranges = ARRAY_SIZE(vout234_xvd_ranges),
 		},
-	},
-	{
-		.desc = {
-			.name = "VD10",
-			.of_match = of_match_ptr("regulator-vd10"),
-			.regulators_node = of_match_ptr("regulators"),
-			.id = BD957X_VD10,
-			.ops = &bd957x_vout34_ops,
-			.type = REGULATOR_VOLTAGE,
-			.fixed_uV = BD957X_VOUTS4_BASE_VOLT,
-			.n_voltages = BD957X_VOUTS34_NUM_VOLT,
-			.vsel_reg = BD957X_REG_VOUT4_TUNE,
-			.vsel_mask = BD957X_MASK_VOUT4_TUNE,
-			.enable_reg = BD957X_REG_POW_TRIGGER4,
-			.enable_mask = BD957X_REGULATOR_EN_MASK,
-			.enable_val = BD957X_REGULATOR_DIS_VAL,
-			.enable_is_inverted = true,
-			.owner = THIS_MODULE,
+		{
+			.desc = {
+				.name = "VD10",
+				.of_match = of_match_ptr("regulator-vd10"),
+				.regulators_node = of_match_ptr("regulators"),
+				.id = BD957X_VD10,
+				.type = REGULATOR_VOLTAGE,
+				.fixed_uV = BD957X_VOUTS4_BASE_VOLT,
+				.n_voltages = BD957X_VOUTS34_NUM_VOLT,
+				.vsel_reg = BD957X_REG_VOUT4_TUNE,
+				.vsel_mask = BD957X_MASK_VOUT4_TUNE,
+				.enable_reg = BD957X_REG_POW_TRIGGER4,
+				.enable_mask = BD957X_REGULATOR_EN_MASK,
+				.enable_val = BD957X_REGULATOR_DIS_VAL,
+				.enable_is_inverted = true,
+				.owner = THIS_MODULE,
+			},
+			.xvd_ranges = vout234_xvd_ranges,
+			.num_xvd_ranges = ARRAY_SIZE(vout234_xvd_ranges),
+			.ovd_reg = BD9576_REG_VOUT4_OVD,
+			.uvd_reg = BD9576_REG_VOUT4_UVD,
+			.xvd_mask = BD9576_MASK_XVD,
 		},
-	},
-	{
-		.desc = {
-			.name = "VOUTL1",
-			.of_match = of_match_ptr("regulator-voutl1"),
-			.regulators_node = of_match_ptr("regulators"),
-			.id = BD957X_VOUTL1,
-			.ops = &bd957x_ops,
-			.type = REGULATOR_VOLTAGE,
-			.volt_table = &voutl1_volt_table[0],
-			.n_voltages = ARRAY_SIZE(voutl1_volt_table),
-			.vsel_reg = BD957X_REG_VOUTL1_TUNE,
-			.vsel_mask = BD957X_MASK_VOUTL1_TUNE,
-			.enable_reg = BD957X_REG_POW_TRIGGERL1,
-			.enable_mask = BD957X_REGULATOR_EN_MASK,
-			.enable_val = BD957X_REGULATOR_DIS_VAL,
-			.enable_is_inverted = true,
-			.owner = THIS_MODULE,
+		{
+			.desc = {
+				.name = "VOUTL1",
+				.of_match = of_match_ptr("regulator-voutl1"),
+				.regulators_node = of_match_ptr("regulators"),
+				.id = BD957X_VOUTL1,
+				.type = REGULATOR_VOLTAGE,
+				.volt_table = &voutl1_volt_table[0],
+				.n_voltages = ARRAY_SIZE(voutl1_volt_table),
+				.vsel_reg = BD957X_REG_VOUTL1_TUNE,
+				.vsel_mask = BD957X_MASK_VOUTL1_TUNE,
+				.enable_reg = BD957X_REG_POW_TRIGGERL1,
+				.enable_mask = BD957X_REGULATOR_EN_MASK,
+				.enable_val = BD957X_REGULATOR_DIS_VAL,
+				.enable_is_inverted = true,
+				.owner = THIS_MODULE,
+			},
+			.xvd_ranges = voutL1_xvd_ranges,
+			.num_xvd_ranges = ARRAY_SIZE(voutL1_xvd_ranges),
+			.ovd_reg = BD9576_REG_VOUTL1_OVD,
+			.uvd_reg = BD9576_REG_VOUTL1_UVD,
+			.xvd_mask = BD9576_MASK_XVD,
 		},
-	},
-	{
-		.desc = {
-			.name = "VOUTS1",
-			.of_match = of_match_ptr("regulator-vouts1"),
-			.regulators_node = of_match_ptr("regulators"),
-			.id = BD957X_VOUTS1,
-			.ops = &bd957X_vouts1_regulator_ops,
-			.type = REGULATOR_VOLTAGE,
-			.n_voltages = 1,
-			.fixed_uV = BD957X_VOUTS1_VOLT,
-			.enable_reg = BD957X_REG_POW_TRIGGERS1,
-			.enable_mask = BD957X_REGULATOR_EN_MASK,
-			.enable_val = BD957X_REGULATOR_DIS_VAL,
-			.enable_is_inverted = true,
-			.owner = THIS_MODULE,
+		{
+			.desc = {
+				.name = "VOUTS1",
+				.of_match = of_match_ptr("regulator-vouts1"),
+				.regulators_node = of_match_ptr("regulators"),
+				.id = BD957X_VOUTS1,
+				.type = REGULATOR_VOLTAGE,
+				.n_voltages = 1,
+				.fixed_uV = BD957X_VOUTS1_VOLT,
+				.enable_reg = BD957X_REG_POW_TRIGGERS1,
+				.enable_mask = BD957X_REGULATOR_EN_MASK,
+				.enable_val = BD957X_REGULATOR_DIS_VAL,
+				.enable_is_inverted = true,
+				.owner = THIS_MODULE,
+				.of_parse_cb = vouts1_get_fet_res,
+			},
+			.oc_supported = true,
+			.ocw_reg = BD9576_REG_VOUT1S_OCW,
+			.ocw_mask = BD9576_MASK_VOUT1S_OCW,
+			.ocp_reg = BD9576_REG_VOUT1S_OCP,
+			.ocp_mask = BD9576_MASK_VOUT1S_OCP,
 		},
 	},
 };
 
+static int bd9576_renable(struct regulator_irq_data *rid, int reg, int mask)
+{
+	int val, ret;
+	struct bd957x_data *d = (struct bd957x_data *)rid->data;
+
+	ret = regmap_read(d->regmap, reg, &val);
+	if (ret)
+		return REGULATOR_FAILED_RETRY;
+
+	if (rid->opaque && rid->opaque == (val & mask)) {
+		/*
+		 * It seems we stil have same status. Ack and return
+		 * information that we are still out of limits and core
+		 * should not enable IRQ
+		 */
+		regmap_write(d->regmap, reg, mask & val);
+		return REGULATOR_ERROR_ON;
+	}
+	rid->opaque = 0;
+	/*
+	 * Status was changed. Either prolem was solved or we have new issues.
+	 * Let's re-enable IRQs and be prepared to report problems again
+	 */
+	return REGULATOR_ERROR_CLEARED;
+}
+
+static int bd9576_uvd_renable(struct regulator_irq_data *rid)
+{
+	return bd9576_renable(rid, BD957X_REG_INT_UVD_STAT, UVD_IRQ_VALID_MASK);
+}
+
+static int bd9576_ovd_renable(struct regulator_irq_data *rid)
+{
+	return bd9576_renable(rid, BD957X_REG_INT_OVD_STAT, OVD_IRQ_VALID_MASK);
+}
+
+static int bd9576_temp_renable(struct regulator_irq_data *rid)
+{
+	return bd9576_renable(rid, BD957X_REG_INT_THERM_STAT,
+			      BD9576_THERM_IRQ_MASK_TW);
+}
+
+static int bd9576_uvd_handler(int irq, struct regulator_irq_data *rid,
+			      unsigned long *dev_mask)
+{
+	int val, ret, i;
+	struct bd957x_data *d = (struct bd957x_data *)rid->data;
+
+	ret = regmap_read(d->regmap, BD957X_REG_INT_UVD_STAT, &val);
+	if (ret)
+		return REGULATOR_FAILED_RETRY;
+
+	*dev_mask = 0;
+
+	rid->opaque = val & UVD_IRQ_VALID_MASK;
+
+	/*
+	 * Go through the set status bits and report either error or warning
+	 * to the notifier depending on what was flagged in DT
+	 */
+	*dev_mask = val & BD9576_xVD_IRQ_MASK_VOUT1TO4;
+	/* There is 1 bit gap in register after Vout1 .. Vout4 statuses */
+	*dev_mask |= ((val & BD9576_xVD_IRQ_MASK_VOUTL1) >> 1);
+	/*
+	 * We (ab)use the uvd for OCW notification. DT parsing should
+	 * have added correct OCW flag to uvd_notif and uvd_err for S1
+	 */
+	*dev_mask |= ((val & BD9576_UVD_IRQ_MASK_VOUTS1_OCW) >> 1);
+
+	for_each_set_bit(i, dev_mask, 6) {
+		struct bd957x_regulator_data *rdata;
+		struct regulator_err_state *stat;
+
+		rdata = &d->regulator_data[i];
+		stat  = &rid->states[i];
+
+		stat->notifs	= rdata->uvd_notif;
+		stat->errors	= rdata->uvd_err;
+	}
+
+	ret = regmap_write(d->regmap, BD957X_REG_INT_UVD_STAT,
+			   UVD_IRQ_VALID_MASK & val);
+
+	return 0;
+}
+
+static int bd9576_ovd_handler(int irq, struct regulator_irq_data *rid,
+			      unsigned long *dev_mask)
+{
+	int val, ret, i;
+	struct bd957x_data *d = (struct bd957x_data *)rid->data;
+
+	ret = regmap_read(d->regmap, BD957X_REG_INT_OVD_STAT, &val);
+	if (ret)
+		return REGULATOR_FAILED_RETRY;
+
+	rid->opaque = val & OVD_IRQ_VALID_MASK;
+	*dev_mask = 0;
+
+	if (!(val & OVD_IRQ_VALID_MASK))
+		return 0;
+
+	*dev_mask = val & BD9576_xVD_IRQ_MASK_VOUT1TO4;
+	/* There is 1 bit gap in register after Vout1 .. Vout4 statuses */
+	*dev_mask |= ((val & BD9576_xVD_IRQ_MASK_VOUTL1) >> 1);
+
+	for_each_set_bit(i, dev_mask, 5) {
+		struct bd957x_regulator_data *rdata;
+		struct regulator_err_state *stat;
+
+		rdata = &d->regulator_data[i];
+		stat  = &rid->states[i];
+
+		stat->notifs	= rdata->ovd_notif;
+		stat->errors	= rdata->ovd_err;
+	}
+
+	/* Clear the sub-IRQ status */
+	regmap_write(d->regmap, BD957X_REG_INT_OVD_STAT,
+		     OVD_IRQ_VALID_MASK & val);
+
+	return 0;
+}
+
+#define BD9576_DEV_MASK_ALL_REGULATORS 0x3F
+
+static int bd9576_thermal_handler(int irq, struct regulator_irq_data *rid,
+				  unsigned long *dev_mask)
+{
+	int val, ret, i;
+	struct bd957x_data *d = (struct bd957x_data *)rid->data;
+
+	ret = regmap_read(d->regmap, BD957X_REG_INT_THERM_STAT, &val);
+	if (ret)
+		return REGULATOR_FAILED_RETRY;
+
+	if (!(val & BD9576_THERM_IRQ_MASK_TW)) {
+		*dev_mask = 0;
+		return 0;
+	}
+
+	*dev_mask = BD9576_DEV_MASK_ALL_REGULATORS;
+
+	for (i = 0; i < BD9576_NUM_REGULATORS; i++) {
+		struct bd957x_regulator_data *rdata;
+		struct regulator_err_state *stat;
+
+		rdata = &d->regulator_data[i];
+		stat  = &rid->states[i];
+
+		stat->notifs	= rdata->temp_notif;
+		stat->errors	= rdata->temp_err;
+	}
+
+	/* Clear the sub-IRQ status */
+	regmap_write(d->regmap, BD957X_REG_INT_THERM_STAT,
+		     BD9576_THERM_IRQ_MASK_TW);
+
+	return 0;
+}
+
 static int bd957x_probe(struct platform_device *pdev)
 {
+	int i;
+	unsigned int num_reg_data;
+	bool vout_mode, ddr_sel, may_have_irqs;
 	struct regmap *regmap;
+	struct bd957x_data *ic_data;
 	struct regulator_config config = { 0 };
-	int i;
-	bool vout_mode, ddr_sel;
-	const struct bd957x_regulator_data *reg_data = &bd9576_regulators[0];
-	unsigned int num_reg_data = ARRAY_SIZE(bd9576_regulators);
+	/* All regulators are related to UVD and thermal IRQs... */
+	struct regulator_dev *rdevs[BD9576_NUM_REGULATORS];
+	/* ...But VoutS1 is not flagged by OVD IRQ */
+	struct regulator_dev *ovd_devs[BD9576_NUM_OVD_REGULATORS];
+	static const struct regulator_irq_desc bd9576_notif_uvd = {
+		.name = "bd9576-uvd",
+		.irq_off_ms = 1000,
+		.map_event = bd9576_uvd_handler,
+		.renable = bd9576_uvd_renable,
+		.data = &bd957x_regulators,
+	};
+	static const struct regulator_irq_desc bd9576_notif_ovd = {
+		.name = "bd9576-ovd",
+		.irq_off_ms = 1000,
+		.map_event = bd9576_ovd_handler,
+		.renable = bd9576_ovd_renable,
+		.data = &bd957x_regulators,
+	};
+	static const struct regulator_irq_desc bd9576_notif_temp = {
+		.name = "bd9576-temp",
+		.irq_off_ms = 1000,
+		.map_event = bd9576_thermal_handler,
+		.renable = bd9576_temp_renable,
+		.data = &bd957x_regulators,
+	};
 	enum rohm_chip_type chip = platform_get_device_id(pdev)->driver_data;
 
+	num_reg_data = ARRAY_SIZE(bd957x_regulators.regulator_data);
+
+	ic_data = &bd957x_regulators;
+
 	regmap = dev_get_regmap(pdev->dev.parent, NULL);
 	if (!regmap) {
 		dev_err(&pdev->dev, "No regmap\n");
 		return -EINVAL;
 	}
+
+	ic_data->regmap = regmap;
 	vout_mode = of_property_read_bool(pdev->dev.parent->of_node,
 					 "rohm,vout1-en-low");
 	if (vout_mode) {
@@ -263,15 +979,17 @@ static int bd957x_probe(struct platform_device *pdev)
 	 * bytes and use bd9576_regulators directly for non-constant configs
 	 * like DDR voltage selection.
 	 */
+	platform_set_drvdata(pdev, ic_data);
 	ddr_sel =  of_property_read_bool(pdev->dev.parent->of_node,
 					 "rohm,ddr-sel-low");
 	if (ddr_sel)
-		bd9576_regulators[2].desc.fixed_uV = 1350000;
+		ic_data->regulator_data[2].desc.fixed_uV = 1350000;
 	else
-		bd9576_regulators[2].desc.fixed_uV = 1500000;
+		ic_data->regulator_data[2].desc.fixed_uV = 1500000;
 
 	switch (chip) {
 	case ROHM_CHIP_TYPE_BD9576:
+		may_have_irqs = true;
 		dev_dbg(&pdev->dev, "Found BD9576MUF\n");
 		break;
 	case ROHM_CHIP_TYPE_BD9573:
@@ -282,32 +1000,116 @@ static int bd957x_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	for (i = 0; i < num_reg_data; i++) {
+		struct regulator_desc *d;
+
+		d = &ic_data->regulator_data[i].desc;
+
+
+		if (may_have_irqs) {
+			if (d->id >= ARRAY_SIZE(bd9576_ops_arr))
+				return -EINVAL;
+
+			d->ops = bd9576_ops_arr[d->id];
+		} else {
+			if (d->id >= ARRAY_SIZE(bd9573_ops_arr))
+				return -EINVAL;
+
+			d->ops = bd9573_ops_arr[d->id];
+		}
+	}
+
 	config.dev = pdev->dev.parent;
 	config.regmap = regmap;
+	config.driver_data = ic_data;
 
 	for (i = 0; i < num_reg_data; i++) {
 
-		const struct regulator_desc *desc;
-		struct regulator_dev *rdev;
-		const struct bd957x_regulator_data *r;
+		struct bd957x_regulator_data *r = &ic_data->regulator_data[i];
+		const struct regulator_desc *desc = &r->desc;
 
-		r = &reg_data[i];
-		desc = &r->desc;
-
-		rdev = devm_regulator_register(&pdev->dev, desc, &config);
-		if (IS_ERR(rdev)) {
+		r->rdev = devm_regulator_register(&pdev->dev, desc,
+							   &config);
+		if (IS_ERR(r->rdev)) {
 			dev_err(&pdev->dev,
 				"failed to register %s regulator\n",
 				desc->name);
-			return PTR_ERR(rdev);
+			return PTR_ERR(r->rdev);
 		}
 		/*
 		 * Clear the VOUT1 GPIO setting - rest of the regulators do not
 		 * support GPIO control
 		 */
 		config.ena_gpiod = NULL;
+
+		if (!may_have_irqs)
+			continue;
+
+		rdevs[i] = r->rdev;
+		if (i < BD957X_VOUTS1)
+			ovd_devs[i] = r->rdev;
 	}
+	if (may_have_irqs) {
+		void *ret;
+		/*
+		 * We can add both the possible error and warning flags here
+		 * because the core uses these only for status clearing and
+		 * if we use warnings - errors are always clear and the other
+		 * way around. We can also add CURRENT flag for all regulators
+		 * because it is never set if it is not supported. Same applies
+		 * to setting UVD for VoutS1 - it is not accidentally cleared
+		 * as it is never set.
+		 */
+		int uvd_errs = REGULATOR_ERROR_UNDER_VOLTAGE |
+			       REGULATOR_ERROR_UNDER_VOLTAGE_WARN |
+			       REGULATOR_ERROR_OVER_CURRENT |
+			       REGULATOR_ERROR_OVER_CURRENT_WARN;
+		int ovd_errs = REGULATOR_ERROR_OVER_VOLTAGE_WARN |
+			       REGULATOR_ERROR_REGULATION_OUT;
+		int temp_errs = REGULATOR_ERROR_OVER_TEMP |
+				REGULATOR_ERROR_OVER_TEMP_WARN;
+		int irq;
+
+		irq = platform_get_irq_byname(pdev, "bd9576-uvd");
+
+		/* Register notifiers - can fail if IRQ is not given */
+		ret = devm_regulator_irq_helper(&pdev->dev, &bd9576_notif_uvd,
+						irq, 0, uvd_errs, NULL,
+						&rdevs[0],
+						BD9576_NUM_REGULATORS);
+		if (IS_ERR(ret)) {
+			if (PTR_ERR(ret) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+
+			dev_warn(&pdev->dev, "UVD disabled %pe\n", ret);
+		}
+
+		irq = platform_get_irq_byname(pdev, "bd9576-ovd");
+
+		ret = devm_regulator_irq_helper(&pdev->dev, &bd9576_notif_ovd,
+						irq, 0, ovd_errs, NULL,
+						&ovd_devs[0],
+						BD9576_NUM_OVD_REGULATORS);
+		if (IS_ERR(ret)) {
+			if (PTR_ERR(ret) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
+
+			dev_warn(&pdev->dev, "OVD disabled %pe\n", ret);
+		}
+		irq = platform_get_irq_byname(pdev, "bd9576-temp");
+
+		ret = devm_regulator_irq_helper(&pdev->dev, &bd9576_notif_temp,
+						irq, 0, temp_errs, NULL,
+						&rdevs[0],
+						BD9576_NUM_REGULATORS);
+		if (IS_ERR(ret)) {
+			if (PTR_ERR(ret) == -EPROBE_DEFER)
+				return -EPROBE_DEFER;
 
+			dev_warn(&pdev->dev, "Thermal warning disabled %pe\n",
+				 ret);
+		}
+	}
 	return 0;
 }
 
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 09/10] regulator: bd9576: Fix the driver name in id table
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
                   ` (7 preceding siblings ...)
  2021-05-10 11:30 ` [PATCH v9 08/10] regulator: bd9576: Support error reporting Matti Vaittinen
@ 2021-05-10 11:31 ` Matti Vaittinen
  2021-05-10 11:31 ` [PATCH v9 10/10] MAINTAINERS: Add reviewer for regulator irq_helpers Matti Vaittinen
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:31 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Driver name was changed in MFD cell:
https://lore.kernel.org/lkml/560b9748094392493ebf7af11b6cc558776c4fd5.1613031055.git.matti.vaittinen@fi.rohmeurope.com/
Fix the ID table to match this.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
No changes since RFC-v2
---
 drivers/regulator/bd9576-regulator.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/bd9576-regulator.c b/drivers/regulator/bd9576-regulator.c
index 6ba12af4c632..d78d6127c573 100644
--- a/drivers/regulator/bd9576-regulator.c
+++ b/drivers/regulator/bd9576-regulator.c
@@ -1114,8 +1114,8 @@ static int bd957x_probe(struct platform_device *pdev)
 }
 
 static const struct platform_device_id bd957x_pmic_id[] = {
-	{ "bd9573-pmic", ROHM_CHIP_TYPE_BD9573 },
-	{ "bd9576-pmic", ROHM_CHIP_TYPE_BD9576 },
+	{ "bd9573-regulator", ROHM_CHIP_TYPE_BD9573 },
+	{ "bd9576-regulator", ROHM_CHIP_TYPE_BD9576 },
 	{ },
 };
 MODULE_DEVICE_TABLE(platform, bd957x_pmic_id);
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v9 10/10] MAINTAINERS: Add reviewer for regulator irq_helpers
  2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
                   ` (8 preceding siblings ...)
  2021-05-10 11:31 ` [PATCH v9 09/10] regulator: bd9576: Fix the driver name in id table Matti Vaittinen
@ 2021-05-10 11:31 ` Matti Vaittinen
  9 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-10 11:31 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

Add a reviewer entry for the regulator irq_helpers.

Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
Changelog:
 v7 - onwards
  - no changes
 v6:
  - New patch
---
 MAINTAINERS | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index bd7aff0c120f..93f51caf9fb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19551,6 +19551,10 @@ F:	include/dt-bindings/regulator/
 F:	include/linux/regulator/
 K:	regulator_get_optional
 
+VOLTAGE AND CURRENT REGULATOR IRQ HELPERS
+R:	Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+F:	drivers/regulator/irq_helpers.c
+
 VRF
 M:	David Ahern <dsahern@kernel.org>
 L:	netdev@vger.kernel.org
-- 
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 =] 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 05/10] regulator: IRQ based event/error notification helpers
  2021-05-10 11:29 ` [PATCH v9 05/10] regulator: IRQ based event/error notification helpers Matti Vaittinen
@ 2021-05-10 17:35   ` kernel test robot
  2021-05-10 20:15     ` Andy Shevchenko
  2021-05-10 19:45   ` kernel test robot
  2021-05-11  4:54   ` Matti Vaittinen
  2 siblings, 1 reply; 21+ messages in thread
From: kernel test robot @ 2021-05-10 17:35 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: kbuild-all, clang-built-linux, Mark Brown, Kees Cook,
	Andy Shevchenko, Zhang Rui, Guenter Roeck, agross, devicetree,
	linux-power, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 6079 bytes --]

Hi Matti,

I love your patch! Yet something to improve:

[auto build test ERROR on 6efb943b8616ec53a5e444193dccf1af9ad627b5]

url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
base:   6efb943b8616ec53a5e444193dccf1af9ad627b5
config: x86_64-randconfig-a015-20210510 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 492173d42b32cb91d5d0d72d5ed84fcab80d059a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/904edb46fa37ac86bc1e7a1629141e037f45ebed
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
        git checkout 904edb46fa37ac86bc1e7a1629141e037f45ebed
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/regulator/irq_helpers.c:244:4: error: implicit declaration of function 'pr_dbg' [-Werror,-Wimplicit-function-declaration]
                           pr_dbg("Sending regulator notification EVT 0x%lx\r\n",
                           ^
   1 error generated.


vim +/pr_dbg +244 drivers/regulator/irq_helpers.c

   153	
   154	static irqreturn_t regulator_notifier_isr(int irq, void *data)
   155	{
   156		struct regulator_irq *h = data;
   157		struct regulator_irq_desc *d;
   158		struct regulator_irq_data *rid;
   159		unsigned long rdev_map = 0;
   160		int num_rdevs;
   161		int ret, i, j;
   162	
   163		d = &h->desc;
   164		rid = &h->rdata;
   165		num_rdevs = rid->num_states;
   166	
   167		if (d->fatal_cnt)
   168			h->retry_cnt++;
   169	
   170		/*
   171		 * we spare a few cycles by not clearing statuses prior to this call.
   172		 * The IC driver must initialize the status buffers for rdevs
   173		 * which it indicates having active events via rdev_map.
   174		 *
   175		 * Maybe we should just to be on a safer side(?)
   176		 */
   177		ret = d->map_event(irq, rid, &rdev_map);
   178	
   179		/*
   180		 * If status reading fails (which is unlikely) we don't ack/disable
   181		 * IRQ but just increase fail count and retry when IRQ fires again.
   182		 * If retry_count exceeds the given safety limit we call IC specific die
   183		 * handler which can try disabling regulator(s).
   184		 *
   185		 * If no die handler is given we will just bug() as a last resort.
   186		 *
   187		 * We could try disabling all associated rdevs - but we might shoot
   188		 * ourselves in the head and leave the problematic regulator enabled. So
   189		 * if IC has no die-handler populated we just assume the regulator
   190		 * can't be disabled.
   191		 */
   192		if (unlikely(ret == REGULATOR_FAILED_RETRY))
   193			goto fail_out;
   194	
   195		h->retry_cnt = 0;
   196		/*
   197		 * Let's not disable IRQ if there were no status bits for us. We'd
   198		 * better leave spurious IRQ handling to genirq
   199		 */
   200		if (ret || !rdev_map)
   201			return IRQ_NONE;
   202	
   203		/*
   204		 * Some events are bogus if the regulator is disabled. Skip such events
   205		 * if all relevant regulators are disabled
   206		 */
   207		if (d->skip_off) {
   208			for_each_set_bit(i, &rdev_map, num_rdevs) {
   209				struct regulator_dev *rdev;
   210				const struct regulator_ops *ops;
   211	
   212				rdev = rid->states[i].rdev;
   213				ops = rdev->desc->ops;
   214	
   215				/*
   216				 * If any of the flagged regulators is enabled we do
   217				 * handle this
   218				 */
   219				if (ops->is_enabled(rdev))
   220					break;
   221			}
   222			if (i == num_rdevs)
   223				return IRQ_NONE;
   224		}
   225	
   226		/* Disable IRQ if HW keeps line asserted */
   227		if (d->irq_off_ms)
   228			disable_irq_nosync(irq);
   229	
   230		/*
   231		 * IRQ seems to be for us. Let's fire correct notifiers / store error
   232		 * flags
   233		 */
   234		for_each_set_bit(i, &rdev_map, num_rdevs) {
   235			unsigned long evt;
   236			struct regulator_err_state *stat;
   237			struct regulator_dev *rdev;
   238	
   239			stat = &rid->states[i];
   240			rdev = stat->rdev;
   241	
   242			for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat->notifs))
   243				evt =  BIT(j);
 > 244				pr_dbg("Sending regulator notification EVT 0x%lx\r\n",
   245				       stat->notifs, evt);
   246				regulator_notifier_call_chain(rdev, evt, NULL);
   247	
   248			rdev_flag_err(rdev, stat->errors);
   249		}
   250	
   251		if (d->irq_off_ms) {
   252			if (!d->high_prio)
   253				schedule_delayed_work(&h->isr_work,
   254						      msecs_to_jiffies(d->irq_off_ms));
   255			else
   256				mod_delayed_work(system_highpri_wq,
   257						 &h->isr_work,
   258						 msecs_to_jiffies(d->irq_off_ms));
   259		}
   260	
   261		return IRQ_HANDLED;
   262	
   263	fail_out:
   264		if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
   265			/* If we have no recovery, just try shut down straight away */
   266			if (!d->die) {
   267				hw_protection_shutdown("Regulator failure. Retry count exceeded",
   268						       REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
   269			} else {
   270				ret = d->die(rid);
   271				/* If die() failed shut down as a last attempt to save the HW */
   272				if (ret)
   273					hw_protection_shutdown("Regulator failure. Recovery failed",
   274							       REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
   275			}
   276		}
   277	
   278		return IRQ_NONE;
   279	}
   280	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31016 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 05/10] regulator: IRQ based event/error notification helpers
  2021-05-10 11:29 ` [PATCH v9 05/10] regulator: IRQ based event/error notification helpers Matti Vaittinen
  2021-05-10 17:35   ` kernel test robot
@ 2021-05-10 19:45   ` kernel test robot
  2021-05-10 20:20     ` Andy Shevchenko
  2021-05-11  4:54   ` Matti Vaittinen
  2 siblings, 1 reply; 21+ messages in thread
From: kernel test robot @ 2021-05-10 19:45 UTC (permalink / raw)
  To: Matti Vaittinen, Matti Vaittinen
  Cc: kbuild-all, Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui,
	Guenter Roeck, agross, devicetree, linux-power, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 6979 bytes --]

Hi Matti,

I love your patch! Yet something to improve:

[auto build test ERROR on 6efb943b8616ec53a5e444193dccf1af9ad627b5]

url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
base:   6efb943b8616ec53a5e444193dccf1af9ad627b5
config: i386-randconfig-s002-20210510 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-341-g8af24329-dirty
        # https://github.com/0day-ci/linux/commit/904edb46fa37ac86bc1e7a1629141e037f45ebed
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
        git checkout 904edb46fa37ac86bc1e7a1629141e037f45ebed
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:12,
                    from arch/x86/include/asm/percpu.h:27,
                    from arch/x86/include/asm/current.h:6,
                    from include/linux/sched.h:12,
                    from include/linux/ratelimit.h:6,
                    from include/linux/dev_printk.h:16,
                    from include/linux/device.h:15,
                    from drivers/regulator/irq_helpers.c:10:
   drivers/regulator/irq_helpers.c: In function 'regulator_notifier_isr':
   include/linux/bitops.h:35:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
      35 |  for ((bit) = find_first_bit((addr), (size));  \
         |  ^~~
   drivers/regulator/irq_helpers.c:242:3: note: in expansion of macro 'for_each_set_bit'
     242 |   for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat->notifs))
         |   ^~~~~~~~~~~~~~~~
   drivers/regulator/irq_helpers.c:244:4: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'for'
     244 |    pr_dbg("Sending regulator notification EVT 0x%lx\r\n",
         |    ^~~~~~
>> drivers/regulator/irq_helpers.c:244:4: error: implicit declaration of function 'pr_dbg'; did you mean 'pr_debug'? [-Werror=implicit-function-declaration]
     244 |    pr_dbg("Sending regulator notification EVT 0x%lx\r\n",
         |    ^~~~~~
         |    pr_debug
   cc1: some warnings being treated as errors


vim +244 drivers/regulator/irq_helpers.c

   153	
   154	static irqreturn_t regulator_notifier_isr(int irq, void *data)
   155	{
   156		struct regulator_irq *h = data;
   157		struct regulator_irq_desc *d;
   158		struct regulator_irq_data *rid;
   159		unsigned long rdev_map = 0;
   160		int num_rdevs;
   161		int ret, i, j;
   162	
   163		d = &h->desc;
   164		rid = &h->rdata;
   165		num_rdevs = rid->num_states;
   166	
   167		if (d->fatal_cnt)
   168			h->retry_cnt++;
   169	
   170		/*
   171		 * we spare a few cycles by not clearing statuses prior to this call.
   172		 * The IC driver must initialize the status buffers for rdevs
   173		 * which it indicates having active events via rdev_map.
   174		 *
   175		 * Maybe we should just to be on a safer side(?)
   176		 */
   177		ret = d->map_event(irq, rid, &rdev_map);
   178	
   179		/*
   180		 * If status reading fails (which is unlikely) we don't ack/disable
   181		 * IRQ but just increase fail count and retry when IRQ fires again.
   182		 * If retry_count exceeds the given safety limit we call IC specific die
   183		 * handler which can try disabling regulator(s).
   184		 *
   185		 * If no die handler is given we will just bug() as a last resort.
   186		 *
   187		 * We could try disabling all associated rdevs - but we might shoot
   188		 * ourselves in the head and leave the problematic regulator enabled. So
   189		 * if IC has no die-handler populated we just assume the regulator
   190		 * can't be disabled.
   191		 */
   192		if (unlikely(ret == REGULATOR_FAILED_RETRY))
   193			goto fail_out;
   194	
   195		h->retry_cnt = 0;
   196		/*
   197		 * Let's not disable IRQ if there were no status bits for us. We'd
   198		 * better leave spurious IRQ handling to genirq
   199		 */
   200		if (ret || !rdev_map)
   201			return IRQ_NONE;
   202	
   203		/*
   204		 * Some events are bogus if the regulator is disabled. Skip such events
   205		 * if all relevant regulators are disabled
   206		 */
   207		if (d->skip_off) {
   208			for_each_set_bit(i, &rdev_map, num_rdevs) {
   209				struct regulator_dev *rdev;
   210				const struct regulator_ops *ops;
   211	
   212				rdev = rid->states[i].rdev;
   213				ops = rdev->desc->ops;
   214	
   215				/*
   216				 * If any of the flagged regulators is enabled we do
   217				 * handle this
   218				 */
   219				if (ops->is_enabled(rdev))
   220					break;
   221			}
   222			if (i == num_rdevs)
   223				return IRQ_NONE;
   224		}
   225	
   226		/* Disable IRQ if HW keeps line asserted */
   227		if (d->irq_off_ms)
   228			disable_irq_nosync(irq);
   229	
   230		/*
   231		 * IRQ seems to be for us. Let's fire correct notifiers / store error
   232		 * flags
   233		 */
   234		for_each_set_bit(i, &rdev_map, num_rdevs) {
   235			unsigned long evt;
   236			struct regulator_err_state *stat;
   237			struct regulator_dev *rdev;
   238	
   239			stat = &rid->states[i];
   240			rdev = stat->rdev;
   241	
   242			for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat->notifs))
   243				evt =  BIT(j);
 > 244				pr_dbg("Sending regulator notification EVT 0x%lx\r\n",
   245				       stat->notifs, evt);
   246				regulator_notifier_call_chain(rdev, evt, NULL);
   247	
   248			rdev_flag_err(rdev, stat->errors);
   249		}
   250	
   251		if (d->irq_off_ms) {
   252			if (!d->high_prio)
   253				schedule_delayed_work(&h->isr_work,
   254						      msecs_to_jiffies(d->irq_off_ms));
   255			else
   256				mod_delayed_work(system_highpri_wq,
   257						 &h->isr_work,
   258						 msecs_to_jiffies(d->irq_off_ms));
   259		}
   260	
   261		return IRQ_HANDLED;
   262	
   263	fail_out:
   264		if (d->fatal_cnt && h->retry_cnt > d->fatal_cnt) {
   265			/* If we have no recovery, just try shut down straight away */
   266			if (!d->die) {
   267				hw_protection_shutdown("Regulator failure. Retry count exceeded",
   268						       REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
   269			} else {
   270				ret = d->die(rid);
   271				/* If die() failed shut down as a last attempt to save the HW */
   272				if (ret)
   273					hw_protection_shutdown("Regulator failure. Recovery failed",
   274							       REGULATOR_FORCED_SAFETY_SHUTDOWN_WAIT_MS);
   275			}
   276		}
   277	
   278		return IRQ_NONE;
   279	}
   280	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34105 bytes --]

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 05/10] regulator: IRQ based event/error notification helpers
  2021-05-10 17:35   ` kernel test robot
@ 2021-05-10 20:15     ` Andy Shevchenko
  0 siblings, 0 replies; 21+ messages in thread
From: Andy Shevchenko @ 2021-05-10 20:15 UTC (permalink / raw)
  To: kernel test robot
  Cc: Matti Vaittinen, Matti Vaittinen, kbuild-all, clang-built-linux,
	Mark Brown, Kees Cook, Zhang Rui, Guenter Roeck, agross,
	devicetree, linux-power, linux-kernel

On Mon, May 10, 2021 at 8:35 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Matti,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on 6efb943b8616ec53a5e444193dccf1af9ad627b5]
>
> url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
> base:   6efb943b8616ec53a5e444193dccf1af9ad627b5
> config: x86_64-randconfig-a015-20210510 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 492173d42b32cb91d5d0d72d5ed84fcab80d059a)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install x86_64 cross compiling tool for clang build
>         # apt-get install binutils-x86-64-linux-gnu
>         # https://github.com/0day-ci/linux/commit/904edb46fa37ac86bc1e7a1629141e037f45ebed
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
>         git checkout 904edb46fa37ac86bc1e7a1629141e037f45ebed
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> >> drivers/regulator/irq_helpers.c:244:4: error: implicit declaration of function 'pr_dbg' [-Werror,-Wimplicit-function-declaration]
>                            pr_dbg("Sending regulator notification EVT 0x%lx\r\n",
>                            ^
>    1 error generated.

I believe it has to be pr_debug()

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 05/10] regulator: IRQ based event/error notification helpers
  2021-05-10 19:45   ` kernel test robot
@ 2021-05-10 20:20     ` Andy Shevchenko
  2021-05-11  3:24       ` Matti Vaittinen
  0 siblings, 1 reply; 21+ messages in thread
From: Andy Shevchenko @ 2021-05-10 20:20 UTC (permalink / raw)
  To: kernel test robot
  Cc: Matti Vaittinen, Matti Vaittinen, kbuild-all, Mark Brown,
	Kees Cook, Zhang Rui, Guenter Roeck, agross, devicetree,
	linux-power, linux-kernel

On Mon, May 10, 2021 at 10:46 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Matti,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on 6efb943b8616ec53a5e444193dccf1af9ad627b5]
>
> url:    https://github.com/0day-ci/linux/commits/Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
> base:   6efb943b8616ec53a5e444193dccf1af9ad627b5
> config: i386-randconfig-s002-20210510 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce:
>         # apt-get install sparse
>         # sparse version: v0.6.3-341-g8af24329-dirty
>         # https://github.com/0day-ci/linux/commit/904edb46fa37ac86bc1e7a1629141e037f45ebed
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Matti-Vaittinen/Extend-regulator-notification-support/20210510-203125
>         git checkout 904edb46fa37ac86bc1e7a1629141e037f45ebed
>         # save the attached .config to linux build tree
>         make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>    In file included from include/linux/kernel.h:12,
>                     from arch/x86/include/asm/percpu.h:27,
>                     from arch/x86/include/asm/current.h:6,
>                     from include/linux/sched.h:12,
>                     from include/linux/ratelimit.h:6,
>                     from include/linux/dev_printk.h:16,
>                     from include/linux/device.h:15,
>                     from drivers/regulator/irq_helpers.c:10:
>    drivers/regulator/irq_helpers.c: In function 'regulator_notifier_isr':
>    include/linux/bitops.h:35:2: warning: this 'for' clause does not guard... [-Wmisleading-indentation]
>       35 |  for ((bit) = find_first_bit((addr), (size));  \
>          |  ^~~
>    drivers/regulator/irq_helpers.c:242:3: note: in expansion of macro 'for_each_set_bit'
>      242 |   for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat->notifs))
>          |   ^~~~~~~~~~~~~~~~
>    drivers/regulator/irq_helpers.c:244:4: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'for'


Seems like missed {}

Matti, there is a serious question: how had you tested this...
(besides obvious compilation error)
Perhaps you have to fix your process somewhere to avoid missing important steps?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 05/10] regulator: IRQ based event/error notification helpers
  2021-05-10 20:20     ` Andy Shevchenko
@ 2021-05-11  3:24       ` Matti Vaittinen
  0 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-11  3:24 UTC (permalink / raw)
  To: Andy Shevchenko, kernel test robot
  Cc: kbuild-all, Mark Brown, Kees Cook, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel

Hi Andy, All,

On Mon, 2021-05-10 at 23:20 +0300, Andy Shevchenko wrote:
> On Mon, May 10, 2021 at 10:46 PM kernel test robot <lkp@intel.com>
> wrote:

> > 
> >    include/linux/bitops.h:35:2: warning: this 'for' clause does not
> > guard... [-Wmisleading-indentation]
> >       35 |  for ((bit) = find_first_bit((addr), (size));  \
> >          |  ^~~
> >    drivers/regulator/irq_helpers.c:242:3: note: in expansion of
> > macro 'for_each_set_bit'
> >      242 |   for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat-
> > >notifs))
> >          |   ^~~~~~~~~~~~~~~~
> >    drivers/regulator/irq_helpers.c:244:4: note: ...this statement,
> > but the latter is misleadingly indented as if it were guarded by
> > the 'for'
> 
> Seems like missed {}
> 
> Matti, there is a serious question: how had you tested this...

I actually did. I did not just run rebase for the series and threw new
version but I actually did run this in real HW, with real break-out
board and with a fresh info print to see the event being sent.

> (besides obvious compilation error)
> Perhaps you have to fix your process somewhere to avoid missing
> important steps?

Yes. Can't deny this. And process fix should be simple. If code/patch
needs a change (even a print removal/print severity change/parameter
change)  - then it needs to be tested again prior formatting the
patches.

Sorry folks.

--Matti


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 05/10] regulator: IRQ based event/error notification helpers
  2021-05-10 11:29 ` [PATCH v9 05/10] regulator: IRQ based event/error notification helpers Matti Vaittinen
  2021-05-10 17:35   ` kernel test robot
  2021-05-10 19:45   ` kernel test robot
@ 2021-05-11  4:54   ` Matti Vaittinen
  2 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-11  4:54 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Mark Brown, Kees Cook, Andy Shevchenko, Zhang Rui, Guenter Roeck,
	agross, devicetree, linux-power, linux-kernel, linux-renesas-soc,
	linux-arm-msm, bjorn.andersson, lgirdwood, robh+dt


On Mon, 2021-05-10 at 14:29 +0300, Matti Vaittinen wrote:
> Provide helper function for IC's implementing regulator notifications
> when an IRQ fires. The helper also works for IRQs which can not be
> acked.
> Helper can be set to disable the IRQ at handler and then re-enabling
> it
> on delayed work later. The helper also adds
> regulator_get_error_flags()
> errors in cache for the duration of IRQ disabling.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> 
> +static irqreturn_t regulator_notifier_isr(int irq, void *data)
> +{

//snip

> +	/*
> +	 * IRQ seems to be for us. Let's fire correct notifiers / store
> error
> +	 * flags
> +	 */
> +	for_each_set_bit(i, &rdev_map, num_rdevs) {
> +		unsigned long evt;
> +		struct regulator_err_state *stat;
> +		struct regulator_dev *rdev;
> +
> +		stat = &rid->states[i];
> +		rdev = stat->rdev;
> +
> +		for_each_set_bit(j, &stat->notifs, BITS_PER_TYPE(stat-
> >notifs)) {
> +			evt =  BIT(j);
> +			pr_dbg("Sending regulator notification EVT
> 0x%lx\r\n",
> +			       stat->notifs, evt);
> +			regulator_notifier_call_chain(rdev, evt, NULL);
> +		}

This construct sends own notification for each of the event flagged by
the driver. My thinking was that sending each event separately ensures
all of them are handled. OTOH, the comment in the even description
states:

> * NOTE: These events can be OR'ed together when passed into handler.

So... Should I actually simplify this and just punt out all the
stat->notifs in one event? That would get rid of this one extra loop.

> +		rdev_flag_err(rdev, stat->errors);
> +	}

Best Regards
	Matti Vaittinen



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 02/10] reboot: Add hardware protection power-off
  2021-05-10 11:28 ` [PATCH v9 02/10] reboot: Add hardware protection power-off Matti Vaittinen
@ 2021-05-12  8:20   ` Petr Mladek
  2021-05-12 12:00     ` Vaittinen, Matti
  0 siblings, 1 reply; 21+ messages in thread
From: Petr Mladek @ 2021-05-12  8:20 UTC (permalink / raw)
  To: Matti Vaittinen
  Cc: Matti Vaittinen, Mark Brown, Kees Cook, Andy Shevchenko,
	Zhang Rui, Guenter Roeck, agross, devicetree, linux-power,
	linux-kernel, linux-renesas-soc, linux-arm-msm, bjorn.andersson,
	lgirdwood, robh+dt, Daniel Lezcano, Amit Kucheria, Matteo Croce,
	Andrew Morton, Rafael J. Wysocki, Mike Rapoport, Josef Bacik,
	Kai-Heng Feng, linux-pm

On Mon 2021-05-10 14:28:30, Matti Vaittinen wrote:
> There can be few cases when we need to shut-down the system in order to
> protect the hardware. Currently this is done at east by the thermal core
> when temperature raises over certain limit.
> 
> Some PMICs can also generate interrupts for example for over-current or
> over-voltage, voltage drops, short-circuit, ... etc. On some systems
> these are a sign of hardware failure and only thing to do is try to
> protect the rest of the hardware by shutting down the system.
> 
> Add shut-down logic which can be used by all subsystems instead of
> implementing the shutdown in each subsystem. The logic is stolen from
> thermal_core with difference of using atomic_t instead of a mutex in
> order to allow calls directly from IRQ context.
> 
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> 
> diff --git a/kernel/reboot.c b/kernel/reboot.c
> index a6ad5eb2fa73..5da8c80a2647 100644
> --- a/kernel/reboot.c
> +++ b/kernel/reboot.c
> @@ -518,6 +519,85 @@ 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();

WARN() look like an overkill here. It prints many lines that are not
much useful in this case. The function is called from well-known
context (workqueue worker).

Also be aware that "panic_on_warn" commandline option will trigger
panic() here.


> +	/*
> +	 * Worst of the worst case trigger emergency restart
> +	 */
> +	WARN(1,
> +	     "Hardware protection shutdown failed. Trying emergency restart\n");
> +	emergency_restart();

Two consecutive WARN() calls are even less useful. They are eye
catching but it is hard to find the only useful line with
the custom message.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 02/10] reboot: Add hardware protection power-off
  2021-05-12  8:20   ` Petr Mladek
@ 2021-05-12 12:00     ` Vaittinen, Matti
  2021-05-13  8:34       ` Petr Mladek
  0 siblings, 1 reply; 21+ messages in thread
From: Vaittinen, Matti @ 2021-05-12 12:00 UTC (permalink / raw)
  To: pmladek
  Cc: josef, keescook, rppt, linux-arm-msm, linux-power, linux-kernel,
	bjorn.andersson, rui.zhang, linux-renesas-soc, devicetree, akpm,
	linux, daniel.lezcano, lgirdwood, robh+dt, kai.heng.feng, mcroce,
	amitk, linux-pm, andy.shevchenko, agross, broonie,
	rafael.j.wysocki

Hi Petr,

Thanks for the review!

On Wed, 2021-05-12 at 10:20 +0200, Petr Mladek wrote:
> On Mon 2021-05-10 14:28:30, Matti Vaittinen wrote:
> > There can be few cases when we need to shut-down the system in
> > order to
> > protect the hardware. Currently this is done at east by the thermal
> > core
> > when temperature raises over certain limit.
> > 
> > Some PMICs can also generate interrupts for example for over-
> > current or
> > over-voltage, voltage drops, short-circuit, ... etc. On some
> > systems
> > these are a sign of hardware failure and only thing to do is try to
> > protect the rest of the hardware by shutting down the system.
> > 
> > Add shut-down logic which can be used by all subsystems instead of
> > implementing the shutdown in each subsystem. The logic is stolen
> > from
> > thermal_core with difference of using atomic_t instead of a mutex
> > in
> > order to allow calls directly from IRQ context.
> > 
> > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > 
> > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > index a6ad5eb2fa73..5da8c80a2647 100644
> > --- a/kernel/reboot.c
> > +++ b/kernel/reboot.c
> > @@ -518,6 +519,85 @@ 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();
> 
> WARN() look like an overkill here. It prints many lines that are not
> much useful in this case. The function is called from well-known
> context (workqueue worker).

This was the existing code which I stole from the thermal_core. I kind
of think that eye-catching WARN is actually a good choice here. Doing
autonomous power-off without a WARNing does not sound good to me :)

> Also be aware that "panic_on_warn" commandline option will trigger
> panic() here.

Hmm.. If panic() hangs the system that might indeed be a problem. Now
we are (again) on a territory which I don't know well. I'd appreciate
any input from thermal folks and Mark. I don't like the idea of making
extreme things like power-off w/o well visible log-trace. Thus I would
like to have WARN()-like eye-catcher, even if the call-trace was not
too varying. It will at least point to this worker. Any better
suggestions than WARN()?

> 
> > +	/*
> > +	 * Worst of the worst case trigger emergency restart
> > +	 */
> > +	WARN(1,
> > +	     "Hardware protection shutdown failed. Trying emergency
> > restart\n");
> > +	emergency_restart();
> 
> Two consecutive WARN() calls are even less useful. They are eye
> catching but it is hard to find the only useful line with
> the custom message.

I think you are right. One WARN should be enough to point here. This
last one could be just an additional print.

Best Regards
	--Matti Vaittinen

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 02/10] reboot: Add hardware protection power-off
  2021-05-12 12:00     ` Vaittinen, Matti
@ 2021-05-13  8:34       ` Petr Mladek
  2021-05-17  4:57         ` Matti Vaittinen
  0 siblings, 1 reply; 21+ messages in thread
From: Petr Mladek @ 2021-05-13  8:34 UTC (permalink / raw)
  To: Vaittinen, Matti
  Cc: josef, keescook, rppt, linux-arm-msm, linux-power, linux-kernel,
	bjorn.andersson, rui.zhang, linux-renesas-soc, devicetree, akpm,
	linux, daniel.lezcano, lgirdwood, robh+dt, kai.heng.feng, mcroce,
	amitk, linux-pm, andy.shevchenko, agross, broonie,
	rafael.j.wysocki, rostedt, Geert Uytterhoeven

On Wed 2021-05-12 12:00:46, Vaittinen, Matti wrote:
> On Wed, 2021-05-12 at 10:20 +0200, Petr Mladek wrote:
> > On Mon 2021-05-10 14:28:30, Matti Vaittinen wrote:
> > > There can be few cases when we need to shut-down the system in
> > > order to
> > > protect the hardware. Currently this is done at east by the thermal
> > > core
> > > when temperature raises over certain limit.
> > > 
> > > Some PMICs can also generate interrupts for example for over-
> > > current or
> > > over-voltage, voltage drops, short-circuit, ... etc. On some
> > > systems
> > > these are a sign of hardware failure and only thing to do is try to
> > > protect the rest of the hardware by shutting down the system.
> > > 
> > > Add shut-down logic which can be used by all subsystems instead of
> > > implementing the shutdown in each subsystem. The logic is stolen
> > > from
> > > thermal_core with difference of using atomic_t instead of a mutex
> > > in
> > > order to allow calls directly from IRQ context.
> > > 
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > 
> > > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > > index a6ad5eb2fa73..5da8c80a2647 100644
> > > --- a/kernel/reboot.c
> > > +++ b/kernel/reboot.c
> > > @@ -518,6 +519,85 @@ 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();
> > 
> > WARN() look like an overkill here. It prints many lines that are not
> > much useful in this case. The function is called from well-known
> > context (workqueue worker).
> 
> This was the existing code which I stole from the thermal_core. I kind
> of think that eye-catching WARN is actually a good choice here. Doing
> autonomous power-off without a WARNing does not sound good to me :)
> 
> > Also be aware that "panic_on_warn" commandline option will trigger
> > panic() here.
> 
> Hmm.. If panic() hangs the system that might indeed be a problem. Now
> we are (again) on a territory which I don't know well. I'd appreciate
> any input from thermal folks and Mark. I don't like the idea of making
> extreme things like power-off w/o well visible log-trace. Thus I would
> like to have WARN()-like eye-catcher, even if the call-trace was not
> too varying. It will at least point to this worker. Any better
> suggestions than WARN()?

Heh, it might make sense to create a system wide API for these. I am
sure that WARN() is mis-used this way on many other locations.

There already are two locations that use another eye-catching text.
A common API might help to avoid duplication of the common parts,
see
https://lore.kernel.org/lkml/20210305194206.3165917-2-elver@google.com/

Well, it might be out of scope for this patchset.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v9 02/10] reboot: Add hardware protection power-off
  2021-05-13  8:34       ` Petr Mladek
@ 2021-05-17  4:57         ` Matti Vaittinen
  0 siblings, 0 replies; 21+ messages in thread
From: Matti Vaittinen @ 2021-05-17  4:57 UTC (permalink / raw)
  To: Petr Mladek
  Cc: josef, keescook, rppt, linux-arm-msm, linux-power, linux-kernel,
	bjorn.andersson, rui.zhang, linux-renesas-soc, devicetree, akpm,
	linux, daniel.lezcano, lgirdwood, robh+dt, kai.heng.feng, mcroce,
	amitk, linux-pm, andy.shevchenko, agross, broonie,
	rafael.j.wysocki, rostedt, Geert Uytterhoeven


On Thu, 2021-05-13 at 10:34 +0200, Petr Mladek wrote:
> On Wed 2021-05-12 12:00:46, Vaittinen, Matti wrote:
> > On Wed, 2021-05-12 at 10:20 +0200, Petr Mladek wrote:
> > > On Mon 2021-05-10 14:28:30, Matti Vaittinen wrote:
> > > > There can be few cases when we need to shut-down the system in
> > > > order to
> > > > protect the hardware. Currently this is done at east by the
> > > > thermal
> > > > core
> > > > when temperature raises over certain limit.
> > > > 
> > > > Some PMICs can also generate interrupts for example for over-
> > > > current or
> > > > over-voltage, voltage drops, short-circuit, ... etc. On some
> > > > systems
> > > > these are a sign of hardware failure and only thing to do is
> > > > try to
> > > > protect the rest of the hardware by shutting down the system.
> > > > 
> > > > Add shut-down logic which can be used by all subsystems instead
> > > > of
> > > > implementing the shutdown in each subsystem. The logic is
> > > > stolen
> > > > from
> > > > thermal_core with difference of using atomic_t instead of a
> > > > mutex
> > > > in
> > > > order to allow calls directly from IRQ context.
> > > > 
> > > > Signed-off-by: Matti Vaittinen <
> > > > matti.vaittinen@fi.rohmeurope.com>
> > > > 
> > > > diff --git a/kernel/reboot.c b/kernel/reboot.c
> > > > index a6ad5eb2fa73..5da8c80a2647 100644
> > > > --- a/kernel/reboot.c
> > > > +++ b/kernel/reboot.c
> > > > @@ -518,6 +519,85 @@ 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();
> > > 
> > > WARN() look like an overkill here. It prints many lines that are
> > > not
> > > much useful in this case. The function is called from well-known
> > > context (workqueue worker).
> > 
> > This was the existing code which I stole from the thermal_core. I
> > kind
> > of think that eye-catching WARN is actually a good choice here.
> > Doing
> > autonomous power-off without a WARNing does not sound good to me :)
> > 
> > > Also be aware that "panic_on_warn" commandline option will
> > > trigger
> > > panic() here.
> > 
> > Hmm.. If panic() hangs the system that might indeed be a problem.
> > Now
> > we are (again) on a territory which I don't know well. I'd
> > appreciate
> > any input from thermal folks and Mark. I don't like the idea of
> > making
> > extreme things like power-off w/o well visible log-trace. Thus I
> > would
> > like to have WARN()-like eye-catcher, even if the call-trace was
> > not
> > too varying. It will at least point to this worker. Any better
> > suggestions than WARN()?
> 
> Heh, it might make sense to create a system wide API for these. I am
> sure that WARN() is mis-used this way on many other locations.
> 
> There already are two locations that use another eye-catching text.
> A common API might help to avoid duplication of the common parts,
> see
> https://lore.kernel.org/lkml/20210305194206.3165917-2-elver@google.com/
> 
> Well, it might be out of scope for this patchset.

I just had a very brief "chat" with Geert (3 IRC messages, posted
during 4 or 5 days :]) - and Geert pointed me to this:

https://lore.kernel.org/linux-iommu/20210331093104.383705-4-geert+renesas@glider.be/

So, maybe I'll just go with simple pr_emerg() and trust that the
emerg() print should catch attention as such level print probably
should. I'll respin the patch series (probably tomorrow) - let's see
what thermal and regulator folks say :)

Thanks for all the help this far!

Best Regards
	Matti Vaittinen


^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2021-05-17  4:57 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10 11:26 [PATCH v9 00/10] Extend regulator notification support Matti Vaittinen
2021-05-10 11:27 ` [PATCH v9 01/10] dt_bindings: Add protection limit properties Matti Vaittinen
2021-05-10 11:28 ` [PATCH v9 02/10] reboot: Add hardware protection power-off Matti Vaittinen
2021-05-12  8:20   ` Petr Mladek
2021-05-12 12:00     ` Vaittinen, Matti
2021-05-13  8:34       ` Petr Mladek
2021-05-17  4:57         ` Matti Vaittinen
2021-05-10 11:29 ` [PATCH v9 03/10] thermal: Use generic HW-protection shutdown API Matti Vaittinen
2021-05-10 11:29 ` [PATCH v9 04/10] regulator: add warning flags Matti Vaittinen
2021-05-10 11:29 ` [PATCH v9 05/10] regulator: IRQ based event/error notification helpers Matti Vaittinen
2021-05-10 17:35   ` kernel test robot
2021-05-10 20:15     ` Andy Shevchenko
2021-05-10 19:45   ` kernel test robot
2021-05-10 20:20     ` Andy Shevchenko
2021-05-11  3:24       ` Matti Vaittinen
2021-05-11  4:54   ` Matti Vaittinen
2021-05-10 11:30 ` [PATCH v9 06/10] regulator: add property parsing and callbacks to set protection limits Matti Vaittinen
2021-05-10 11:30 ` [PATCH v9 07/10] dt-bindings: regulator: bd9576 add FET ON-resistance for OCW Matti Vaittinen
2021-05-10 11:30 ` [PATCH v9 08/10] regulator: bd9576: Support error reporting Matti Vaittinen
2021-05-10 11:31 ` [PATCH v9 09/10] regulator: bd9576: Fix the driver name in id table Matti Vaittinen
2021-05-10 11:31 ` [PATCH v9 10/10] MAINTAINERS: Add reviewer for regulator irq_helpers Matti Vaittinen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).