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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `scoped_guard` macro is improperly terminated with a trailing semicolon, resulting in an empty critical section that leaves the alarm state updates unprotected.
--

--- Patch [4]: [PATCH v2 4/4] hwmon: chipcap2: serialize access to low/high_alarm indicators ---
commit a0d28bd8208a1d69e597290c8ebb0fdf0731e68b
Author: Javier Carrasco <javier.carrasco.cruz@gmail.com>

hwmon: chipcap2: serialize access to low/high_alarm indicators

This commit attempts to serialize concurrent access to the hardware monitoring
alarm indicators. It introduces scoped_guard usage within the threaded
interrupt handlers to protect the low and high alarm state updates.

> diff --git a/drivers/hwmon/chipcap2.c b/drivers/hwmon/chipcap2.c
> index 57d22d65612c2..0a56c55e93942 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);

[Severity: High]
Does this trailing semicolon create an empty critical section?

Because scoped_guard expands to a for loop, placing a semicolon immediately
after it makes the loop body empty. This means the lock is acquired and
immediately released.

The subsequent assignment to cc2->rh_alarm.low_alarm executes entirely outside
the lock, leaving the data race unmitigated. Should the assignment be wrapped
in a compound block without the trailing semicolon?

> +			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);

[Severity: High]
Does the same empty critical section issue apply here for the high_alarm
update?

> +			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;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260823-chipcap2_locks-v2-0-6a26c8e9e2fc@gmail.com?part=4