[PATCH v2] hwmon: (sht15) Stop deferred work before releasing its resources

[Myeonghun Pak <mhun512@gmail.com>]

The IRQ handler can queue read_work, which accesses GPIOs and may re-enable
that same IRQ.  A measurement can return while read_work is still pending
or running.  The regulator notifier can also queue update_supply_work,
which accesses the regulator.  Neither work item was drained on removal
or on all probe failure paths before the managed resources were released.

Use request_irq() and free_irq() since the IRQ must be released at an
explicit point in the teardown sequence.  Disable and drain read_work
before freeing the IRQ.  Unregister the regulator notifier before draining
update_supply_work and disabling the regulator.

Use IRQF_NO_AUTOEN to ensure that the IRQ stays disabled until the first
measurement, rather than enabling it briefly between request_irq() and
disable_irq_nosync().  Cache the IRQ number and check errors from
gpiod_to_irq().

Fixes: 251eb40f5ccd ("hwmon: sht15 humidity sensor driver")
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Compile-tested (W=1). No runtime testing.

Changes in v2:
- Replace managed IRQ request/free with request_irq()/free_irq().
- Use IRQF_NO_AUTOEN and cache the data GPIO IRQ.
- Preserve asynchronous work drain ordering on probe failure and remove.

 drivers/hwmon/sht15.c | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/sht15.c b/drivers/hwmon/sht15.c
index 7bb2105d1ae223e7131ba4cc0523e4e074e01302..c95acec7ae2f8f57a10bd72f65a2905e64c3c58c 100644
--- a/drivers/hwmon/sht15.c
+++ b/drivers/hwmon/sht15.c
@@ -31,6 +31,7 @@
 #include <linux/bitrev.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of.h>
+#include <linux/workqueue.h>
 
 /* Commands */
 #define SHT15_MEASURE_TEMP		0x03
@@ -120,6 +121,7 @@ static const u8 sht15_crc8_table[] = {
  * struct sht15_data - device instance specific data
  * @sck:		clock GPIO line
  * @data:		data GPIO line
+ * @irq:		interrupt number for the data GPIO line.
  * @read_work:		bh of interrupt handler.
  * @wait_queue:		wait queue for getting values from device.
  * @val_temp:		last temperature value read from device.
@@ -149,6 +151,7 @@ static const u8 sht15_crc8_table[] = {
 struct sht15_data {
 	struct gpio_desc		*sck;
 	struct gpio_desc		*data;
+	int				irq;
 	struct work_struct		read_work;
 	wait_queue_head_t		wait_queue;
 	uint16_t			val_temp;
@@ -532,9 +535,9 @@ static int sht15_measurement(struct sht15_data *data,
 		return ret;
 	atomic_set(&data->interrupt_handled, 0);
 
-	enable_irq(gpiod_to_irq(data->data));
+	enable_irq(data->irq);
 	if (gpiod_get_value(data->data) == 0) {
-		disable_irq_nosync(gpiod_to_irq(data->data));
+		disable_irq_nosync(data->irq);
 		/* Only relevant if the interrupt hasn't occurred. */
 		if (!atomic_read(&data->interrupt_handled))
 			schedule_work(&data->read_work);
@@ -546,7 +549,7 @@ static int sht15_measurement(struct sht15_data *data,
 		data->state = SHT15_READING_NOTHING;
 		return -EIO;
 	} else if (ret == 0) { /* timeout occurred */
-		disable_irq_nosync(gpiod_to_irq(data->data));
+		disable_irq_nosync(data->irq);
 		ret = sht15_connection_reset(data);
 		if (ret)
 			return ret;
@@ -825,7 +828,7 @@ static void sht15_bh_read_data(struct work_struct *work_s)
 		 * have gone low in meantime so verify it hasn't!
 		 */
 		atomic_set(&data->interrupt_handled, 0);
-		enable_irq(gpiod_to_irq(data->data));
+		enable_irq(data->irq);
 		/* If still not occurred or another handler was scheduled */
 		if (gpiod_get_value(data->data)
 		    || atomic_read(&data->interrupt_handled))
@@ -976,25 +979,31 @@ static int sht15_probe(struct platform_device *pdev)
 		goto err_release_reg;
 	}
 
-	ret = devm_request_irq(&pdev->dev, gpiod_to_irq(data->data),
-			       sht15_interrupt_fired,
-			       IRQF_TRIGGER_FALLING,
-			       "sht15 data",
-			       data);
-	if (ret)
+	data->irq = gpiod_to_irq(data->data);
+	if (data->irq < 0) {
+		ret = data->irq;
+		goto err_release_reg;
+	}
+
+	ret = request_irq(data->irq, sht15_interrupt_fired,
+			  IRQF_TRIGGER_FALLING | IRQF_NO_AUTOEN,
+			  "sht15 data", data);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get irq for data line\n");
 		goto err_release_reg;
-	disable_irq_nosync(gpiod_to_irq(data->data));
+	}
+	/* IRQ remains disabled until a measurement starts. */
 	ret = sht15_connection_reset(data);
 	if (ret)
-		goto err_release_reg;
+		goto err_free_irq;
 	ret = sht15_soft_reset(data);
 	if (ret)
-		goto err_release_reg;
+		goto err_free_irq;
 
 	ret = sysfs_create_group(&pdev->dev.kobj, &sht15_attr_group);
 	if (ret) {
 		dev_err(&pdev->dev, "sysfs create failed\n");
-		goto err_release_reg;
+		goto err_free_irq;
 	}
 
 	data->hwmon_dev = hwmon_device_register(data->dev);
@@ -1007,9 +1016,14 @@ static int sht15_probe(struct platform_device *pdev)
 
 err_release_sysfs_group:
 	sysfs_remove_group(&pdev->dev.kobj, &sht15_attr_group);
+err_free_irq:
+	/* The read worker can re-enable the IRQ, so drain it first. */
+	disable_work_sync(&data->read_work);
+	free_irq(data->irq, data);
 err_release_reg:
 	if (!IS_ERR(data->reg)) {
 		regulator_unregister_notifier(data->reg, &data->nb);
+		cancel_work_sync(&data->update_supply_work);
 		regulator_disable(data->reg);
 	}
 	return ret;
@@ -1023,12 +1037,17 @@ static void sht15_remove(struct platform_device *pdev)
 	hwmon_device_unregister(data->hwmon_dev);
 	sysfs_remove_group(&pdev->dev.kobj, &sht15_attr_group);
 
+	/* The worker can re-enable the IRQ, so stop it before free_irq(). */
+	disable_work_sync(&data->read_work);
+	free_irq(data->irq, data);
+
 	ret = sht15_soft_reset(data);
 	if (ret)
 		dev_err(&pdev->dev, "Failed to reset device (%pe)\n", ERR_PTR(ret));
 
 	if (!IS_ERR(data->reg)) {
 		regulator_unregister_notifier(data->reg, &data->nb);
+		cancel_work_sync(&data->update_supply_work);
 		regulator_disable(data->reg);
 	}
 }
-- 
2.47.3