[PATCH v2 0/3] regulator: handle regulator late cleanup race with PM suspend

[PATCH v2 0/3] regulator: handle regulator late cleanup race with PM suspend

[Joy Zou]

The regulator_init_complete_work fires ~30s after boot to disable
unused regulators via I2C. When this work races with PM suspend, the
I2C adapter may already be suspended, causing a -ESHUTDOWN warning
dump.

This series addresses the race and adds proper suspend power
management for unused LDO regulators.

Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>
---
Changes in v2:
- trim the oversized register dump from the backtrace in the commit
  message, keeping only the relevant call trace.
- rename the callback to pfuze100_ldo_set_suspend_disable since it is
  only wired into the LDO ops.
- use the per-regulator stby_reg/stby_mask instead of a hardcoded
  VGENxSTBY/VGENxLPWR bit.
- clarify that the change targets suspend power reduction and remove the
  redundant race-condition wording so the commit message is clearer.
- Link to v1: https://patch.msgid.link/20260716-b4-regulator-pf01-v1-0-8c1519d54bd4@oss.nxp.com

---
Joy Zou (3):
      regulator: core: use system_freezable_wq for init complete work
      regulator: pfuze100: add set_suspend_disable for LDO ops
      arm64: dts: imx8mq-evk: add regulator-off-in-suspend for VGEN1/VGEN6

 arch/arm64/boot/dts/freescale/imx8mq-evk.dts |  6 ++++++
 drivers/regulator/core.c                     |  6 ++++--
 drivers/regulator/pfuze100-regulator.c       | 20 ++++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)
---
base-commit: 34cf6dafc47441dfb6b356a095b89c3585a93714
change-id: 20260716-b4-regulator-pf01-5db03fba4a92

Best regards,
--  
Joy Zou <joy.zou@oss.nxp.com>

[PATCH v2 1/3] regulator: core: use system_freezable_wq for init complete work

[Joy Zou]

schedule_delayed_work() uses system_wq, which is non-freezable, allowing
regulator_init_complete_work to run concurrently with system suspend. This
work fires ~30s after boot to disable unused regulators via I2C. When it
races with PM suspend, the I2C adapter may already be suspended, triggering
a -ESHUTDOWN warning in __i2c_transfer():

  WARNING: ... at __i2c_transfer+0x36c/0x3c8
  Call trace:
   __i2c_transfer
   i2c_transfer
   regmap_i2c_write
   _regmap_update_bits
   regulator_disable_regmap
   _regulator_do_disable
   regulator_late_cleanup
   regulator_init_complete_work_function
   process_one_work

Switch to system_freezable_wq so the work is frozen before any device
is suspended, eliminating the race.

Fixes: 55576cf18537 ("regulator: Defer init completion for a while after late_initcall")
Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v2:
- Trim the oversized register dump from the backtrace in the commit
  message, keeping only the relevant call trace.
- Link to v1: https://patch.msgid.link/20260716-b4-regulator-pf01-v1-0-8c1519d54bd4@oss.nxp.com
---
 drivers/regulator/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 1797929dfe56..d5d24a940f31 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -27,6 +27,7 @@
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
 #include <linux/module.h>
+#include <linux/workqueue.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/regulator.h>
@@ -6882,8 +6883,9 @@ static int __init regulator_init_complete(void)
 	 * we'd only do this on systems that need it, and a kernel
 	 * command line option might be useful.
 	 */
-	schedule_delayed_work(&regulator_init_complete_work,
-			      msecs_to_jiffies(30000));
+	queue_delayed_work(system_freezable_wq,
+			   &regulator_init_complete_work,
+			   msecs_to_jiffies(30000));
 
 	return 0;
 }

-- 
2.34.1

Re: [PATCH v2 1/3] regulator: core: use system_freezable_wq for init complete work

[sashiko-bot]

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

Pre-existing issues:
- [High] regulator_unregister() uses flush_work() on a delayed_work, failing to cancel pending timers and leading to Use-After-Free.
- [High] Delayed work 'oc_work' is never canceled on unbind in mt6363-regulator, risking timer list corruption or Use-After-Free.
- [High] Delayed works 'sc_recovery_work' and 'ocp_recovery_work' are never canceled on unbind in qcom-labibb-regulator.
--

--- Patch [1]: [PATCH v2 1/3] regulator: core: use system_freezable_wq for init complete work ---
commit 7f543933219647c14311f3aa93aefaa15b1b5acf
Author: Joy Zou <joy.zou@oss.nxp.com>

regulator: core: use system_freezable_wq for init complete work

This patch fixes a race condition between the delayed regulator init
completion work and system suspend. By switching from system_wq to
system_freezable_wq, the init work is now frozen during suspend,
preventing it from running when the I2C adapter is already suspended.

> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 2e61606fc1d05..6a4008f387b5e 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
[ ... ]
> @@ -6899,8 +6900,9 @@ static int __init regulator_init_complete(void)
>  	 * we'd only do this on systems that need it, and a kernel
>  	 * command line option might be useful.
>  	 */
> -	schedule_delayed_work(&regulator_init_complete_work,
> -			      msecs_to_jiffies(30000));
> +	queue_delayed_work(system_freezable_wq,
> +			   &regulator_init_complete_work,
> +			   msecs_to_jiffies(30000));
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the use of
workqueues and timers in the regulator subsystem, I noticed that
regulator_unregister() uses flush_work() on a delayed_work instead of
cancel_delayed_work_sync():

drivers/regulator/core.c:regulator_unregister() {
...
	if (rdev->supply) {
		regulator_unregister_notifier(rdev->supply,
					      &rdev->supply_fwd_nb);

		while (rdev->use_count--)
			regulator_disable(rdev->supply);
		regulator_put(rdev->supply);
	}

	flush_work(&rdev->disable_work.work);
...
}

Because flush_work() returns immediately if the delayed_work timer is
pending but not yet queued on a workqueue, this fails to cancel the pending
timer.

When the timer eventually expires, it dereferences the freed rdev memory.

Can this lead to a use-after-free during hardware removal or module unloading?

[Severity: High]
This isn't a bug introduced by this patch, but I also noticed missing
teardown for delayed work in the mt6363-regulator driver:

drivers/regulator/mt6363-regulator.c:mt6363_regulator_probe() {
...
		config.driver_data = info;
		INIT_DELAYED_WORK(&info->oc_work, mt6363_oc_irq_enable_work);

		rdev = devm_regulator_register(dev, &info->desc, &config);
...
}

The delayed work 'oc_work' is initialized on a static array but is never
canceled on unbind.

If an overcurrent interrupt fires and schedules oc_work, and the device is
unbound before the timer expires, the timer remains pending.

If unbound and re-bound quickly, could the subsequent INIT_DELAYED_WORK()
overwrite the active timer's list node and corrupt the timer tree?

Or, if the module is unloaded, could the timer fire and execute code in
unmapped memory, causing a use-after-free?

[Severity: High]
This isn't a bug introduced by this patch, but there is a similar missing
teardown issue in the qcom-labibb-regulator driver:

drivers/regulator/qcom-labibb-regulator.c:qcom_labibb_regulator_probe() {
...
		INIT_DELAYED_WORK(&vreg->sc_recovery_work,
				  qcom_labibb_sc_recovery_worker);

		if (vreg->ocp_irq > 0)
			INIT_DELAYED_WORK(&vreg->ocp_recovery_work,
					  qcom_labibb_ocp_recovery_worker);
...
}

The delayed works 'sc_recovery_work' and 'ocp_recovery_work' point to a
devm-allocated structure.

Since there is no devm action or explicit teardown to cancel these delayed
works on unbind, devres will automatically free the vreg structure.

If the device unbinds while these timers are active, will the timers expire
and dereference the freed vreg pointer, causing a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-b4-regulator-pf01-v2-0-a406c8737fdb@oss.nxp.com?part=1

[PATCH v2 2/3] regulator: pfuze100: add set_suspend_disable for LDO ops

[Joy Zou]

Add a set_suspend_disable callback to pfuze100_ldo_regulator_ops to
support the regulator-off-in-suspend DTS property for the VGEN LDO
regulators. This allows unused LDO regulators to be properly disabled
during system suspend, reducing power consumption.

The callback is only used by the LDO ops, so name it accordingly:
pfuze100_ldo_set_suspend_disable. It uses the per-regulator
stby_reg/stby_mask that already describe the standby control for
each LDO, so it works for every LDO covered by these ops.

Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v2:
- Rename the callback to pfuze100_ldo_set_suspend_disable since it is
  only wired into the LDO ops.
- Use the per-regulator stby_reg/stby_mask instead of a hardcoded
  VGENxSTBY/VGENxLPWR bit.
- Link to v1: https://patch.msgid.link/20260716-b4-regulator-pf01-v1-0-8c1519d54bd4@oss.nxp.com
---
 drivers/regulator/pfuze100-regulator.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c
index 7d56c22b5e40..d3a0e2505524 100644
--- a/drivers/regulator/pfuze100-regulator.c
+++ b/drivers/regulator/pfuze100-regulator.c
@@ -158,6 +158,25 @@ static int pfuze100_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
 	return ret;
 }
 
+static int pfuze100_ldo_set_suspend_disable(struct regulator_dev *rdev)
+{
+	struct pfuze_chip *pfuze100 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	struct pfuze_regulator *desc = &pfuze100->regulator_descs[id];
+
+	/*
+	 * Set the standby bit so the LDO output is turned off when the PMIC
+	 * receives a STANDBY event, using the per-regulator stby_reg/stby_mask
+	 * that describe the standby control for each LDO.
+	 *
+	 * The stby_mask only covers the VGENxSTBY bit. The VGENxLPWR stays at
+	 * its reset value of 0, so the LDO is switched off rather than put
+	 * into low-power mode.
+	 */
+	return regmap_update_bits(pfuze100->regmap, desc->stby_reg,
+				  desc->stby_mask, desc->stby_mask);
+}
+
 static const struct regulator_ops pfuze100_ldo_regulator_ops = {
 	.enable = regulator_enable_regmap,
 	.disable = regulator_disable_regmap,
@@ -165,6 +184,7 @@ static const struct regulator_ops pfuze100_ldo_regulator_ops = {
 	.list_voltage = regulator_list_voltage_linear,
 	.set_voltage_sel = regulator_set_voltage_sel_regmap,
 	.get_voltage_sel = regulator_get_voltage_sel_regmap,
+	.set_suspend_disable = pfuze100_ldo_set_suspend_disable,
 };
 
 static const struct regulator_ops pfuze100_fixed_regulator_ops = {

-- 
2.34.1

Re: [PATCH v2 2/3] regulator: pfuze100: add set_suspend_disable for LDO ops

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The callback `pfuze100_ldo_set_suspend_disable` does not explicitly clear the Low Power mode bit (`VGENxLPWR`), relying on the fragile assumption that the bootloader left it at its reset value of 0. If it is 1, the LDO enters low-power mode instead of powering off.
--

--- Patch [2]: [PATCH v2 2/3] regulator: pfuze100: add set_suspend_disable for LDO ops ---
commit b5ef15eff59cfdbe79ee9305f34ea04f79252ea1
Author: Joy Zou <joy.zou@oss.nxp.com>

regulator: pfuze100: add set_suspend_disable for LDO ops

This commit adds a set_suspend_disable callback to pfuze100_ldo_regulator_ops.
This supports the regulator-off-in-suspend DTS property for the VGEN LDO
regulators, allowing unused LDO regulators to be properly disabled during
system suspend.

> diff --git a/drivers/regulator/pfuze100-regulator.c b/drivers/regulator/pfuze100-regulator.c
> index 7d56c22b5e400..d3a0e25055248 100644
> --- a/drivers/regulator/pfuze100-regulator.c
> +++ b/drivers/regulator/pfuze100-regulator.c

[ ... ]

> @@ -158,6 +158,25 @@ static int pfuze100_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
>  	return ret;
>  }
>  
> +static int pfuze100_ldo_set_suspend_disable(struct regulator_dev *rdev)
> +{
> +	struct pfuze_chip *pfuze100 = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	struct pfuze_regulator *desc = &pfuze100->regulator_descs[id];
> +
> +	/*
> +	 * Set the standby bit so the LDO output is turned off when the PMIC
> +	 * receives a STANDBY event, using the per-regulator stby_reg/stby_mask
> +	 * that describe the standby control for each LDO.
> +	 *
> +	 * The stby_mask only covers the VGENxSTBY bit. The VGENxLPWR stays at
> +	 * its reset value of 0, so the LDO is switched off rather than put
> +	 * into low-power mode.
> +	 */
> +	return regmap_update_bits(pfuze100->regmap, desc->stby_reg,
> +				  desc->stby_mask, desc->stby_mask);

[Severity: Medium]
Will this sequence reliably turn off the regulator if the bootloader has
previously set the VGENxLPWR bit to 1?

Since this only updates the VGENxSTBY bit defined in stby_mask, it relies on
the VGENxLPWR bit remaining at its reset value of 0.

If the bootloader modified this bit, does the LDO enter low-power mode
instead of completely powering off during system suspend?

Could the VGENxLPWR bit be explicitly cleared here to guarantee the regulator
turns off, similar to how it is handled in pfuze_power_off_prepare()?

> +}
> +
>  static const struct regulator_ops pfuze100_ldo_regulator_ops = {
>  	.enable = regulator_enable_regmap,
>  	.disable = regulator_disable_regmap,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-b4-regulator-pf01-v2-0-a406c8737fdb@oss.nxp.com?part=2

[PATCH v2 3/3] arm64: dts: imx8mq-evk: add regulator-off-in-suspend for VGEN1/VGEN6

[Joy Zou]

VGEN1 and VGEN6 have no consumers and are not always-on. Mark them as
regulator-off-in-suspend to disable them properly during suspend and
reduce power consumption.

Signed-off-by: Joy Zou <joy.zou@oss.nxp.com>
---
Changes in v2:
- The patch is aimed at reducing power consumption during suspend. It is
not related to the race condition. So remove the redundant wording so the
commit message is clearer.
- Link to v1: https://patch.msgid.link/20260716-b4-regulator-pf01-v1-0-8c1519d54bd4@oss.nxp.com
---
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 5ea04703ddd7..fc0303e44433 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -299,6 +299,9 @@ vref_reg: vrefddr {
 			vgen1_reg: vgen1 {
 				regulator-min-microvolt = <800000>;
 				regulator-max-microvolt = <1550000>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 
 			vgen2_reg: vgen2 {
@@ -328,6 +331,9 @@ vgen5_reg: vgen5 {
 			vgen6_reg: vgen6 {
 				regulator-min-microvolt = <1800000>;
 				regulator-max-microvolt = <3300000>;
+				regulator-state-mem {
+					regulator-off-in-suspend;
+				};
 			};
 		};
 	};

-- 
2.34.1