[PATCH v2 0/2] regulator: fixed: add under-voltage support

[PATCH v2 0/2] regulator: fixed: add under-voltage support

[Oleksij Rempel]

This series add under-voltage IRQ support to the fixed regulator
at same time by enforcing emergency shut down policy.

changes v2:
- drop event forwarding support
- use emergency shutdown directly instead of generating under-voltage
  error event.
- fix devicetree patch
- drop interrupt-names support

Oleksij Rempel (2):
  regulator: dt-bindings: fixed-regulator: Add under-voltage interrupt
    support
  regulator: fixed: add support for under-voltage IRQ

 .../bindings/regulator/fixed-regulator.yaml   |  5 ++
 drivers/regulator/fixed.c                     | 47 +++++++++++++++++++
 2 files changed, 52 insertions(+)

-- 
2.39.2

[PATCH v2 1/2] regulator: dt-bindings: fixed-regulator: Add under-voltage interrupt support

[Oleksij Rempel]

Add under-voltage interrupt support. This can be used with simple
regulators having no other way to communicate an under-voltage event
except as by toggling some GPIO line.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 .../devicetree/bindings/regulator/fixed-regulator.yaml       | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
index ac0281b1cceb..7cc4d9209491 100644
--- a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
@@ -100,6 +100,11 @@ properties:
   vin-supply:
     description: Input supply phandle.
 
+  interrupts:
+    maxItems: 1
+    description:
+      Under-voltage interrupt
+
 required:
   - compatible
   - regulator-name
-- 
2.39.2

[PATCH v2 2/2] regulator: fixed: add support for under-voltage IRQ

[Oleksij Rempel]

Add interrupt support for under-voltage notification. This functionality
can be used on systems capable to detect under-voltage state and having
enough capacity to let the SoC do some emergency preparation.

This change enforce default policy to shutdown system as soon as
interrupt is triggered.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/regulator/fixed.c | 47 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 55130efae9b8..c60ea7ac7762 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_opp.h>
+#include <linux/reboot.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/fixed.h>
 #include <linux/gpio/consumer.h>
@@ -29,6 +30,8 @@
 #include <linux/regulator/machine.h>
 #include <linux/clk.h>
 
+/* Default time in millisecond to wait for emergency shutdown */
+#define FV_DEF_EMERG_SHUTDWN_TMO	10
 
 struct fixed_voltage_data {
 	struct regulator_desc desc;
@@ -105,6 +108,46 @@ static int reg_is_enabled(struct regulator_dev *rdev)
 	return priv->enable_counter > 0;
 }
 
+static irqreturn_t reg_fixed_under_voltage_irq_handler(int irq, void *data)
+{
+	hw_protection_shutdown("Critical voltage drop reached",
+			       FV_DEF_EMERG_SHUTDWN_TMO);
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * reg_fixed_get_irqs - Get and register the optional IRQ for fixed voltage
+ *                      regulator.
+ * @dev: Pointer to the device structure.
+ * @priv: Pointer to fixed_voltage_data structure containing private data.
+ *
+ * This function tries to get the IRQ from the device firmware node.
+ * If it's an optional IRQ and not found, it returns 0.
+ * Otherwise, it attempts to request the threaded IRQ.
+ *
+ * Return: 0 on success, or error code on failure.
+ */
+static int reg_fixed_get_irqs(struct device *dev,
+			      struct fixed_voltage_data *priv)
+{
+	int ret;
+
+	ret = fwnode_irq_get(dev_fwnode(dev), 0);
+	/* This is optional IRQ. If not found we will get -EINVAL */
+	if (ret == -EINVAL)
+		return 0;
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to get IRQ\n");
+
+	ret = devm_request_threaded_irq(dev, ret, NULL,
+					reg_fixed_under_voltage_irq_handler,
+					IRQF_ONESHOT, "under-voltage", priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request IRQ\n");
+
+	return 0;
+}
 
 /**
  * of_get_fixed_voltage_config - extract fixed_voltage_config structure info
@@ -294,6 +337,10 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
 	dev_dbg(&pdev->dev, "%s supplying %duV\n", drvdata->desc.name,
 		drvdata->desc.fixed_uV);
 
+	ret = reg_fixed_get_irqs(dev, drvdata);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
2.39.2

Re: [PATCH v2 2/2] regulator: fixed: add support for under-voltage IRQ

[Mark Brown]

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

On Tue, Oct 24, 2023 at 03:08:42PM +0200, Oleksij Rempel wrote:

> Add interrupt support for under-voltage notification. This functionality
> can be used on systems capable to detect under-voltage state and having
> enough capacity to let the SoC do some emergency preparation.
> 
> This change enforce default policy to shutdown system as soon as
> interrupt is triggered.

...

> +static irqreturn_t reg_fixed_under_voltage_irq_handler(int irq, void *data)
> +{
> +	hw_protection_shutdown("Critical voltage drop reached",
> +			       FV_DEF_EMERG_SHUTDWN_TMO);
> +
> +	return IRQ_HANDLED;
> +}

We need a bit more policy here - the regulator could be critical to
system function but it could also be well isolated and just affecting
whatever device it's directly supplying in a way that the system can
tolerate and might even want to (eg, for something like a SD card or USB
port where end users are plugging in external hardware).

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

Re: [PATCH v2 2/2] regulator: fixed: add support for under-voltage IRQ

[Oleksij Rempel]

On Tue, Oct 24, 2023 at 02:26:24PM +0100, Mark Brown wrote:
> On Tue, Oct 24, 2023 at 03:08:42PM +0200, Oleksij Rempel wrote:
> 
> > Add interrupt support for under-voltage notification. This functionality
> > can be used on systems capable to detect under-voltage state and having
> > enough capacity to let the SoC do some emergency preparation.
> > 
> > This change enforce default policy to shutdown system as soon as
> > interrupt is triggered.
> 
> ...
> 
> > +static irqreturn_t reg_fixed_under_voltage_irq_handler(int irq, void *data)
> > +{
> > +	hw_protection_shutdown("Critical voltage drop reached",
> > +			       FV_DEF_EMERG_SHUTDWN_TMO);
> > +
> > +	return IRQ_HANDLED;
> > +}
> 
> We need a bit more policy here - the regulator could be critical to
> system function but it could also be well isolated and just affecting
> whatever device it's directly supplying in a way that the system can
> tolerate and might even want to (eg, for something like a SD card or USB
> port where end users are plugging in external hardware).

Hm, how about devicetree property to indicate system critical nature of
the regulator. For example "system-critical-regulator" or
"system-critical-undervoltage-interrupt" ?

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 2/2] regulator: fixed: add support for under-voltage IRQ

[Mark Brown]

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

On Tue, Oct 24, 2023 at 04:06:34PM +0200, Oleksij Rempel wrote:
> On Tue, Oct 24, 2023 at 02:26:24PM +0100, Mark Brown wrote:

> > We need a bit more policy here - the regulator could be critical to
> > system function but it could also be well isolated and just affecting
> > whatever device it's directly supplying in a way that the system can
> > tolerate and might even want to (eg, for something like a SD card or USB
> > port where end users are plugging in external hardware).

> Hm, how about devicetree property to indicate system critical nature of
> the regulator. For example "system-critical-regulator" or
> "system-critical-undervoltage-interrupt" ?

I'd probably go with the former.  As a code thing we probably want the
driver to generate an under voltage notification and then the core uses
that notification to trigger the power failure handling.  It feels like
we might end up doing something better in future but I'm not seeing it
right now and there's a fairly clear argument that this is a part of the
hardware design.  It shouldn't be too bad to do backwards compatibility
if required I think.

I'd put the property in the core regulator bindings then it'll work for
everything.

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