From: David Heidelberg david@ixit.cz
[ Upstream commit c0461f8e842495041c18b2c67647501d55c17441 ]
If regmap_write() fails in al3000a_init(), al3000a_set_pwr_off is not called.
In order to avoid such a situation, move the devm_add_action_or_reset() which calls al3000a_set_pwr_off right after a successful al3000a_set_pwr_on.
Signed-off-by: David Heidelberg david@ixit.cz Link: https://patch.msgid.link/20250402-al3010-iio-regmap-v4-2-d189bea87261@ixit.c... Signed-off-by: Jonathan Cameron Jonathan.Cameron@huawei.com Signed-off-by: Sasha Levin sashal@kernel.org ---
Now let me analyze the commit thoroughly:
## Analysis
**YES**
This commit should be backported to stable kernel trees. Here's my detailed analysis:
### 1. **Bug Fix Nature** This commit fixes a clear resource management bug in the error handling path. Looking at the code changes:
**Before the fix (problematic code flow):** ```c static int al3000a_init(struct al3000a_data *data) { ret = al3000a_set_pwr_on(data); // Enables regulator + device if (ret) return ret;
ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_RESET); if (ret) return ret; // BUG: Power stays on if this fails
return regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_ENABLE); // BUG: Power stays on if this fails too } ```
**After the fix:** ```c static int al3000a_init(struct al3000a_data *data) { ret = al3000a_set_pwr_on(data); if (ret) return ret;
ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data); if (ret) return ret; // Now power gets turned off automatically
ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_RESET); if (ret) return ret; // Power automatically cleaned up by devm
return regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_ENABLE); } ```
### 2. **Resource Leak Impact** This fixes a resource leak where: - The regulator remains enabled if `regmap_write()` operations fail - The device remains powered on unnecessarily - This can affect power management and battery life on portable devices - Could potentially cause issues for subsequent probe attempts
### 3. **Pattern Consistency** Examining the similar commit #1 (al3010 driver) with "Backport Status: YES", this fix follows the exact same pattern: - Both fix resource management in error paths - Both move `devm_add_action_or_reset()` immediately after power-on - Both ensure cleanup happens automatically if subsequent operations fail - Both are minimal, targeted fixes
Looking at `/home/sasha/linux/drivers/iio/light/al3010.c:93-97`, I can see the al3010 driver already implements this pattern correctly, confirming this is the established best practice.
### 4. **Minimal Risk Assessment** - **Change scope**: Very limited, only affects error handling in probe path - **No functional changes**: Normal operation flow remains identical - **No architectural changes**: Just moves existing cleanup code earlier - **Backward compatible**: No API or behavior changes for successful cases
### 5. **Stable Tree Criteria Compliance** ✅ **Important bugfix**: Fixes resource leak affecting all users of this driver ✅ **Minimal risk**: Change is very small and well-contained ✅ **No new features**: Pure bug fix ✅ **Confined to subsystem**: Only affects IIO light sensor driver ✅ **Clear side effects**: Only positive effect (proper resource cleanup)
### 6. **User Impact** - Affects any system using the AL3000A ambient light sensor - Resource leaks can accumulate over time with repeated probe failures - Power management improvements are important for embedded/mobile systems
### 7. **Historical Context** The similar commit reference #1 for al3010 driver shows this exact same pattern was deemed worthy of backporting, establishing precedent for these types of resource management fixes in IIO light drivers.
This commit fits all the criteria for stable backporting: it's a clear bugfix, has minimal risk, follows established patterns, and improves system reliability without introducing new features or architectural changes.
drivers/iio/light/al3000a.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/iio/light/al3000a.c b/drivers/iio/light/al3000a.c index e2fbb1270040f..6d5115b2a06c5 100644 --- a/drivers/iio/light/al3000a.c +++ b/drivers/iio/light/al3000a.c @@ -85,12 +85,17 @@ static void al3000a_set_pwr_off(void *_data)
static int al3000a_init(struct al3000a_data *data) { + struct device *dev = regmap_get_device(data->regmap); int ret;
ret = al3000a_set_pwr_on(data); if (ret) return ret;
+ ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data); + if (ret) + return dev_err_probe(dev, ret, "failed to add action\n"); + ret = regmap_write(data->regmap, AL3000A_REG_SYSTEM, AL3000A_CONFIG_RESET); if (ret) return ret; @@ -157,10 +162,6 @@ static int al3000a_probe(struct i2c_client *client) if (ret) return dev_err_probe(dev, ret, "failed to init ALS\n");
- ret = devm_add_action_or_reset(dev, al3000a_set_pwr_off, data); - if (ret) - return dev_err_probe(dev, ret, "failed to add action\n"); - return devm_iio_device_register(dev, indio_dev); }