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

["Javier Carrasco" <javier.carrasco.cruz@gmail.com>]

On Sun Aug 23, 2026 at 8:06 PM CEST, sashiko-bot wrote:
> 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?
>

This feedback is correct. The trailing semicolon is for guard() and not
scoped_guard(). I will fix that for V3.

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

Same here.

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

Best regards,
Javier