[v2,2/2] regulator: fixed: add support for under-voltage IRQ
Commit Message
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(+)
Comments
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).
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
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.
@@ -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;
}