[PATCH v2 0/4] hwmon: chipcap2: various bug fixes

[PATCH v2 0/4] hwmon: chipcap2: various bug fixes

[Javier Carrasco]

The original issue (race condition for the alarms) was found by Sashiko[1]
when an unrelated patch affected chipcap2.c. The issue is real as the
access to the variables is not protected by any locking mechanism
although 2 different sources (threaded interrupts and sysfs) could
modify them.

After sending the fix, Sashiko found new issues[2] that have been added to
the series:
- Wrong channel number in hwmon_notify_event(): pass 0 as the channel
  number.
- IRQ teardown ordering: split the current implementation to provide the
  available interrupts before registering the hwmon device, and register
  the IRQs after the hwmon device has been registered.
- Enable IRQ processing when the regulator is already enabled.

The fixes have been validated on real hardware with an Amphenol
ChipCap 2 CC2D23S sensor.

[1] Link: https://lore.kernel.org/linux-hwmon/20260625162114.417EA1F000E9@smtp.kernel.org/
[2] Link: https://lore.kernel.org/linux-hwmon/20260821093706.D1EB11F000E9@smtp.kernel.org/

To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>

Changes in v2:
- Add fixes to the new issues reported by Sashiko.
- Use hwmon subsystem lock.
- Link to v1: https://lore.kernel.org/r/20260821-chipcap2_locks-v1-1-1d8ccabfc1b2@gmail.com

---
Javier Carrasco (4):
      hwmon: chipcap2: fix channels in humidity alarm notifications
      hwmon: chipcap2: fix IRQ teardown ordering
      hwmon: chipcap2: enable IRQ processing when regulator is already enabled
      hwmon: chipcap2: serialize access to low/high_alarm indicators

 drivers/hwmon/chipcap2.c | 76 ++++++++++++++++++++++++------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)
---
base-commit: 388b607d107c07aaade04c7f22f344cab6bdccd3
change-id: 20260820-chipcap2_locks-c01013a24a0c

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@gmail.com>

[PATCH v2 1/4] hwmon: chipcap2: fix channels in humidity alarm notifications

[Javier Carrasco]

hwmon_notify_event() expects the channel number as its last argument,
taken into account with the type parameter that it is a humidity sensor
type. Given that this device only provides one humidity channel, 0 must
be passed. The custom construct to enumerate the channels makes wrong
assumptions by listing all types together (temperature and humidity).

Remove the custom channel enumeration and pass the right channel to
hwmon_notify_event() for hwmon_humidity_min_alarm and
hwmon_humidity_max_alarm.

Fixes: 3af350929e75 ("hwmon: Add support for Amphenol ChipCap 2")
Cc: stable@vger.kernel.org
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/hwmon/chipcap2.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 086571d556b7..9bef767b589e 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -92,11 +92,6 @@ struct cc2_data {
 	bool process_irqs;
 };
 
-enum cc2_chan_addr {
-	CC2_CHAN_TEMP = 0,
-	CC2_CHAN_HUMIDITY,
-};
-
 /* %RH as a per cent mille from a register value */
 static long cc2_rh_convert(u16 data)
 {
@@ -499,7 +494,7 @@ static irqreturn_t cc2_low_interrupt(int irq, void *data)
 
 	if (cc2->process_irqs) {
 		hwmon_notify_event(cc2->hwmon, hwmon_humidity,
-				   hwmon_humidity_min_alarm, CC2_CHAN_HUMIDITY);
+				   hwmon_humidity_min_alarm, 0);
 		cc2->rh_alarm.low_alarm = true;
 	}
 
@@ -512,7 +507,7 @@ static irqreturn_t cc2_high_interrupt(int irq, void *data)
 
 	if (cc2->process_irqs) {
 		hwmon_notify_event(cc2->hwmon, hwmon_humidity,
-				   hwmon_humidity_max_alarm, CC2_CHAN_HUMIDITY);
+				   hwmon_humidity_max_alarm, 0);
 		cc2->rh_alarm.high_alarm = true;
 	}
 

-- 
2.43.0

[PATCH v2 2/4] hwmon: chipcap2: fix IRQ teardown ordering

[Javier Carrasco]

The threaded IRQs are currently requested before the hwmon device is
registered. Since devres releases resources in reverse order, the hwmon
device is unregistered before the IRQs are freed during driver teardown.

An IRQ may therefore still run after the hwmon device has been released
and access the stale data->hwmon pointer.

Separate IRQ discovery from IRQ registration so that the IRQ numbers and
alarm visibility flags are initialized before registering the hwmon
device, while the IRQ handlers are requested afterwards. This ensures
that the IRQs are released before the hwmon device during devres cleanup.

Fixes: 3af350929e75 ("hwmon: Add support for Amphenol ChipCap 2")
Cc: stable@vger.kernel.org
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/hwmon/chipcap2.c | 55 ++++++++++++++++++++++++------------------------
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 9bef767b589e..15630d1dd90a 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -631,28 +631,37 @@ static int cc2_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 	}
 }
 
-static int cc2_request_ready_irq(struct cc2_data *data, struct device *dev)
+static void cc2_get_irqs(struct cc2_data *data, struct device *dev)
 {
-	int ret = 0;
 
 	data->irq_ready = fwnode_irq_get_byname(dev_fwnode(dev), "ready");
-	if (data->irq_ready > 0) {
+	if (data->irq_ready > 0)
 		init_completion(&data->complete);
+
+	data->irq_low = fwnode_irq_get_byname(dev_fwnode(dev), "low");
+	if (data->irq_low > 0)
+		data->rh_alarm.low_alarm_visible = true;
+
+	data->irq_high = fwnode_irq_get_byname(dev_fwnode(dev), "high");
+	if (data->irq_high > 0)
+		data->rh_alarm.high_alarm_visible = true;
+}
+
+static int cc2_request_irqs(struct cc2_data *data, struct device *dev)
+{
+	int ret;
+
+	if (data->irq_ready > 0) {
 		ret = devm_request_threaded_irq(dev, data->irq_ready, NULL,
 						cc2_ready_interrupt,
 						IRQF_ONESHOT |
 						IRQF_TRIGGER_RISING,
 						dev_name(dev), data);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to request ready irq\n");
 	}
 
-	return ret;
-}
-
-static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
-{
-	int ret = 0;
-
-	data->irq_low = fwnode_irq_get_byname(dev_fwnode(dev), "low");
 	if (data->irq_low > 0) {
 		ret = devm_request_threaded_irq(dev, data->irq_low, NULL,
 						cc2_low_interrupt,
@@ -660,12 +669,10 @@ static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
 						IRQF_TRIGGER_RISING,
 						dev_name(dev), data);
 		if (ret)
-			return ret;
-
-		data->rh_alarm.low_alarm_visible = true;
+			return dev_err_probe(dev, ret,
+					     "Failed to request low alarm irq\n");
 	}
 
-	data->irq_high = fwnode_irq_get_byname(dev_fwnode(dev), "high");
 	if (data->irq_high > 0) {
 		ret = devm_request_threaded_irq(dev, data->irq_high, NULL,
 						cc2_high_interrupt,
@@ -673,12 +680,11 @@ static int cc2_request_alarm_irqs(struct cc2_data *data, struct device *dev)
 						IRQF_TRIGGER_RISING,
 						dev_name(dev), data);
 		if (ret)
-			return ret;
-
-		data->rh_alarm.high_alarm_visible = true;
+			return dev_err_probe(dev, ret,
+					     "Failed to request high alarm irq\n");
 	}
 
-	return ret;
+	return 0;
 }
 
 static const struct hwmon_channel_info *cc2_info[] = {
@@ -706,7 +712,6 @@ static int cc2_probe(struct i2c_client *client)
 {
 	struct cc2_data *data;
 	struct device *dev = &client->dev;
-	int ret;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
 		return -EOPNOTSUPP;
@@ -726,13 +731,7 @@ static int cc2_probe(struct i2c_client *client)
 
 	device_property_read_string(dev, "label", &data->label);
 
-	ret = cc2_request_ready_irq(data, dev);
-	if (ret)
-		return dev_err_probe(dev, ret, "Failed to request ready irq\n");
-
-	ret = cc2_request_alarm_irqs(data, dev);
-	if (ret)
-		return dev_err_probe(dev, ret, "Failed to request alarm irqs\n");
+	cc2_get_irqs(data, dev);
 
 	data->hwmon = devm_hwmon_device_register_with_info(dev, client->name,
 							   data, &cc2_chip_info,
@@ -741,7 +740,7 @@ static int cc2_probe(struct i2c_client *client)
 		return dev_err_probe(dev, PTR_ERR(data->hwmon),
 				     "Failed to register hwmon device\n");
 
-	return 0;
+	return cc2_request_irqs(data, dev);
 }
 
 static void cc2_remove(struct i2c_client *client)

-- 
2.43.0

[PATCH v2 3/4] hwmon: chipcap2: enable IRQ processing when regulator is already enabled

[Javier Carrasco]

cc2_enable() returns early when the regulator is already enabled.
This can happen if a previous regulator_disable() failed or if the
regulator was configured as always-on.

In this case, process_irqs remains false, causing the ready interrupt
to be ignored. Subsequent operations waiting for the ready completion
then time out.

Set process_irqs before returning when the regulator is already
enabled, so that the driver continues to process ready interrupts.

Fixes: 3af350929e75 ("hwmon: Add support for Amphenol ChipCap 2")
Cc: stable@vger.kernel.org
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/hwmon/chipcap2.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 15630d1dd90a..57d22d65612c 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -119,8 +119,10 @@ static int cc2_enable(struct cc2_data *data)
 	int ret;
 
 	/* exclusive regulator, check in case a disable failed */
-	if (regulator_is_enabled(data->regulator))
+	if (regulator_is_enabled(data->regulator)) {
+		data->process_irqs = true;
 		return 0;
+	}
 
 	/* clear any pending completion */
 	try_wait_for_completion(&data->complete);

-- 
2.43.0

[PATCH v2 4/4] hwmon: chipcap2: serialize access to low/high_alarm indicators

[Javier Carrasco]

Access to low_alarm and high_alarm from the threaded interrupt handlers
and sysfs is not protected by any locking mechanism at the moment, which
can lead to missed events.

Use the hwmon subsystem lock to serialize access to the alarm indicators
and update their state before generating the corresponding event to keep
a consistent state.

Fixes: 3af350929e75 ("hwmon: Add support for Amphenol ChipCap 2")
Cc: stable@vger.kernel.org
Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
---
 drivers/hwmon/chipcap2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
index 57d22d65612c..0a56c55e9394 100644
--- a/drivers/hwmon/chipcap2.c
+++ b/drivers/hwmon/chipcap2.c
@@ -495,9 +495,11 @@ static irqreturn_t cc2_low_interrupt(int irq, void *data)
 	struct cc2_data *cc2 = data;
 
 	if (cc2->process_irqs) {
+		scoped_guard(hwmon_lock, cc2->hwmon);
+			cc2->rh_alarm.low_alarm = true;
+
 		hwmon_notify_event(cc2->hwmon, hwmon_humidity,
 				   hwmon_humidity_min_alarm, 0);
-		cc2->rh_alarm.low_alarm = true;
 	}
 
 	return IRQ_HANDLED;
@@ -508,9 +510,11 @@ static irqreturn_t cc2_high_interrupt(int irq, void *data)
 	struct cc2_data *cc2 = data;
 
 	if (cc2->process_irqs) {
+		scoped_guard(hwmon_lock, cc2->hwmon);
+			cc2->rh_alarm.high_alarm = true;
+
 		hwmon_notify_event(cc2->hwmon, hwmon_humidity,
 				   hwmon_humidity_max_alarm, 0);
-		cc2->rh_alarm.high_alarm = true;
 	}
 
 	return IRQ_HANDLED;

-- 
2.43.0