[PATCH] regulator: pf1550: fix which regulator is notified

[PATCH] regulator: pf1550: fix which regulator is notified

[Donggeun Yoo]

The interrupt handler distinguishes the rail that reported the fault, but
the body ignores it. Every SW interrupt walks the regulator array looking
for the name "SW3" and every LDO interrupt looks for "LDO3", so an
over-current on SW1 is reported to the consumers of SW3 while the
consumers of SW1 hear nothing.

The lookup itself is unreliable as well. rdev_get_name() returns the
device tree regulator-name property whenever the board supplies one, and
only falls back to the name in the driver descriptor when it does not.
The binding example for this device sets regulator-name to "sw3" and
"ldo3", which strcmp() does not match against the upper case literals
used here, so a board that follows the documentation gets no over-current
notification at all. A board that names its rails after the schematic
does not match either. No other driver in the tree selects a notification
target this way.

Replace the name lookup with rdev_get_id(), which returns the descriptor
id set by the driver and cannot be overridden from the device tree, and
take both the id and the event from a table indexed by the interrupt.
The die temperature interrupts keep notifying every regulator since they
report a chip wide condition.

Fixes: 7320d41c29bb ("regulator: pf1550: Add support for regulator")
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
---
 drivers/regulator/pf1550-regulator.c | 82 ++++++++++++++--------------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/drivers/regulator/pf1550-regulator.c b/drivers/regulator/pf1550-regulator.c
index 610eac9bb9cb..ceee553a84b2 100644
--- a/drivers/regulator/pf1550-regulator.c
+++ b/drivers/regulator/pf1550-regulator.c
@@ -283,63 +283,61 @@ static struct pf1550_desc pf1550_regulators[] = {
 	PF_LDO1(PF1550, "ldo3", LDO3, 0x1f, pf1550_ldo13_volts),
 };
 
+/*
+ * The _LS interrupts indicate an over-current event. The _HS
+ * interrupts, which are more accurate and can detect catastrophic
+ * faults, issue an error event. The current limit FAULT interrupt is
+ * similar to the _HS.
+ */
+static const struct pf1550_regulator_irq {
+	unsigned int event;
+	u8 id;
+} pf1550_regulator_irqs[] = {
+	[PF1550_PMIC_IRQ_SW1_LS]     = { REGULATOR_EVENT_OVER_CURRENT_WARN, PF1550_SW1 },
+	[PF1550_PMIC_IRQ_SW2_LS]     = { REGULATOR_EVENT_OVER_CURRENT_WARN, PF1550_SW2 },
+	[PF1550_PMIC_IRQ_SW3_LS]     = { REGULATOR_EVENT_OVER_CURRENT_WARN, PF1550_SW3 },
+	[PF1550_PMIC_IRQ_SW1_HS]     = { REGULATOR_EVENT_OVER_CURRENT, PF1550_SW1 },
+	[PF1550_PMIC_IRQ_SW2_HS]     = { REGULATOR_EVENT_OVER_CURRENT, PF1550_SW2 },
+	[PF1550_PMIC_IRQ_SW3_HS]     = { REGULATOR_EVENT_OVER_CURRENT, PF1550_SW3 },
+	[PF1550_PMIC_IRQ_LDO1_FAULT] = { REGULATOR_EVENT_OVER_CURRENT, PF1550_LDO1 },
+	[PF1550_PMIC_IRQ_LDO2_FAULT] = { REGULATOR_EVENT_OVER_CURRENT, PF1550_LDO2 },
+	[PF1550_PMIC_IRQ_LDO3_FAULT] = { REGULATOR_EVENT_OVER_CURRENT, PF1550_LDO3 },
+};
+
 static irqreturn_t pf1550_regulator_irq_handler(int irq, void *data)
 {
+	const struct pf1550_regulator_irq *map;
 	struct pf1550_regulator_info *info = data;
 	struct device *dev = info->dev;
 	struct platform_device *pdev = to_platform_device(dev);
 	int i, irq_type = -1;
-	unsigned int event;
 
 	for (i = 0; i < PF1550_REGULATOR_IRQ_NR; i++)
 		if (irq == platform_get_irq(pdev, i))
 			irq_type = i;
 
-	switch (irq_type) {
-	/* The _LS interrupts indicate over-current event. The _HS interrupts
-	 * which are more accurate and can detect catastrophic faults, issue
-	 * an error event. The current limit FAULT interrupt is similar to the
-	 * _HS'
-	 */
-	case PF1550_PMIC_IRQ_SW1_LS:
-	case PF1550_PMIC_IRQ_SW2_LS:
-	case PF1550_PMIC_IRQ_SW3_LS:
-		event = REGULATOR_EVENT_OVER_CURRENT_WARN;
-		for (i = 0; i < PF1550_MAX_REGULATOR; i++)
-			if (!strcmp(rdev_get_name(info->rdevs[i]), "SW3"))
-				regulator_notifier_call_chain(info->rdevs[i],
-							      event, NULL);
-		break;
-	case PF1550_PMIC_IRQ_SW1_HS:
-	case PF1550_PMIC_IRQ_SW2_HS:
-	case PF1550_PMIC_IRQ_SW3_HS:
-		event = REGULATOR_EVENT_OVER_CURRENT;
-		for (i = 0; i < PF1550_MAX_REGULATOR; i++)
-			if (!strcmp(rdev_get_name(info->rdevs[i]), "SW3"))
-				regulator_notifier_call_chain(info->rdevs[i],
-							      event, NULL);
-		break;
-	case PF1550_PMIC_IRQ_LDO1_FAULT:
-	case PF1550_PMIC_IRQ_LDO2_FAULT:
-	case PF1550_PMIC_IRQ_LDO3_FAULT:
-		event = REGULATOR_EVENT_OVER_CURRENT;
-		for (i = 0; i < PF1550_MAX_REGULATOR; i++)
-			if (!strcmp(rdev_get_name(info->rdevs[i]), "LDO3"))
-				regulator_notifier_call_chain(info->rdevs[i],
-							      event, NULL);
-		break;
-	case PF1550_PMIC_IRQ_TEMP_110:
-	case PF1550_PMIC_IRQ_TEMP_125:
-		event = REGULATOR_EVENT_OVER_TEMP;
+	/* The die temperature concerns every rail. */
+	if (irq_type == PF1550_PMIC_IRQ_TEMP_110 ||
+	    irq_type == PF1550_PMIC_IRQ_TEMP_125) {
 		for (i = 0; i < PF1550_MAX_REGULATOR; i++)
 			regulator_notifier_call_chain(info->rdevs[i],
-						      event, NULL);
-		break;
-	default:
-		dev_err(dev, "regulator interrupt: irq %d occurred\n",
-			irq_type);
+						      REGULATOR_EVENT_OVER_TEMP,
+						      NULL);
+		return IRQ_HANDLED;
+	}
+
+	if (irq_type < 0 || irq_type >= (int)ARRAY_SIZE(pf1550_regulator_irqs)) {
+		dev_err(dev, "regulator interrupt: irq %d occurred\n", irq_type);
+		return IRQ_HANDLED;
 	}
 
+	map = &pf1550_regulator_irqs[irq_type];
+
+	for (i = 0; i < PF1550_MAX_REGULATOR; i++)
+		if (rdev_get_id(info->rdevs[i]) == map->id)
+			regulator_notifier_call_chain(info->rdevs[i],
+						      map->event, NULL);
+
 	return IRQ_HANDLED;
 }
 
-- 
2.53.0

Re: [PATCH] regulator: pf1550: fix which regulator is notified

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The interrupt handler unconditionally returns IRQ_HANDLED even when it completely fails to recognize the interrupt, defeating spurious interrupt detection.
--

commit 756013e824bd37aa338c50b32f4e6024e5f64dfc
Author: Donggeun Yoo <donggeunyoo.kernel@gmail.com>

    regulator: pf1550: fix which regulator is notified

This commit fixes the regulator notification logic in the pf1550 driver.
It replaces string-based matching of regulator names with ID-based lookups
to reliably notify the correct consumer when a fault occurs.

> diff --git a/drivers/regulator/pf1550-regulator.c b/drivers/regulator/pf1550-regulator.c
> index 610eac9bb9cb2..ceee553a84b23 100644
> --- a/drivers/regulator/pf1550-regulator.c
> +++ b/drivers/regulator/pf1550-regulator.c
[ ... ]
> @@ -283,63 +283,61 @@ static struct pf1550_desc pf1550_regulators[] = {
[ ... ]
>  static irqreturn_t pf1550_regulator_irq_handler(int irq, void *data)
>  {
> +	const struct pf1550_regulator_irq *map;
>  	struct pf1550_regulator_info *info = data;
>  	struct device *dev = info->dev;
>  	struct platform_device *pdev = to_platform_device(dev);
>  	int i, irq_type = -1;
> -	unsigned int event;
>  
>  	for (i = 0; i < PF1550_REGULATOR_IRQ_NR; i++)
>  		if (irq == platform_get_irq(pdev, i))
>  			irq_type = i;
>  
[ ... ]
> +	if (irq_type < 0 || irq_type >= (int)ARRAY_SIZE(pf1550_regulator_irqs)) {
> +		dev_err(dev, "regulator interrupt: irq %d occurred\n", irq_type);
> +		return IRQ_HANDLED;
                ^^^^^^^^^^^^^^^^^^^
[Severity: High]
This is a pre-existing issue, but returning IRQ_HANDLED when the driver
completely fails to recognize the interrupt defeats the kernel's spurious
interrupt detection.

If pf1550_regulator_irq_handler() is invoked with an IRQ that does not match
any index in the loop, irq_type remains -1 and this branch is taken. Since
IRQ_HANDLED is returned, the spurious interrupt detection is falsely told
the interrupt was handled successfully and will not mask the virtual IRQ line.

If the PMIC continuously asserts an unhandled fault, could the parent
regmap-irq thread loop endlessly resulting in an interrupt storm and CPU
lockup? Should this return IRQ_NONE instead?

>  	}
>  
> +	map = &pf1550_regulator_irqs[irq_type];
> +
> +	for (i = 0; i < PF1550_MAX_REGULATOR; i++)
> +		if (rdev_get_id(info->rdevs[i]) == map->id)
> +			regulator_notifier_call_chain(info->rdevs[i],
> +						      map->event, NULL);
> +
>  	return IRQ_HANDLED;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904105624.48577-1-donggeunyoo.kernel@gmail.com?part=1

Re: [PATCH] regulator: pf1550: fix which regulator is notified

[Donggeun Yoo]

On Fri, Sep 04, 2026 at 11:10:38AM +0000, sashiko-bot@kernel.org wrote:
> This is a pre-existing issue, but returning IRQ_HANDLED when the driver
> completely fails to recognize the interrupt defeats the kernel's spurious
> interrupt detection.
>
> If the PMIC continuously asserts an unhandled fault, could the parent
> regmap-irq thread loop endlessly resulting in an interrupt storm and CPU
> lockup? Should this return IRQ_NONE instead?

The branch is not reachable.

probe() requests the handler for the eleven interrupts returned by
platform_get_irq(pdev, 0..PF1550_REGULATOR_IRQ_NR-1), without IRQF_SHARED,
and the lookup loop rebuilds that list with the same calls. The cell has no
.of_compatible, so platform_get_irq() returns the number mfd_add_device()
stored in the resource once, through irq_create_mapping(). Both loops see
the same numbers, so irq_type is always in range.

An interrupt the driver has no entry for cannot reach the handler either.
regmap_irq_thread() dispatches only the bits registered in the irq chip, so
there is nothing for the described storm to come from.

The point about handle_nested_irq() feeding note_interrupt() is right in
general, it just does not apply here.

Thanks,
Donggeun