Re: [PATCH v2 2/2] hwmon: add Axiado AX3000 and AX3005 PWM fan controller driver

[Guenter Roeck <linux@roeck-us.net>]

Petar,

On 9/3/26 01:48, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The interrupt handler incorrectly returns IRQ_HANDLED for unhandled device interrupts, potentially causing an IRQ storm.
> --
> 
> commit ac0edffb2c72d464c05c340c5df147fdcf10b0f4
> Author: Petar Stepanovic <pstepanovic@axiado.com>
> 
> hwmon: add Axiado AX3000 and AX3005 PWM fan controller driver
> 
> This commit adds support for the PWM fan controller found on the Axiado AX3000
> and AX3005 SoCs. It measures fan speed via a hardware tachometer interrupt and
> controls the fan via an external PWM signal.
> 
>> diff --git a/drivers/hwmon/axiado-pwm-fan.c b/drivers/hwmon/axiado-pwm-fan.c
>> new file mode 100644
>> index 0000000000000..ac153afded47a
>> --- /dev/null
>> +++ b/drivers/hwmon/axiado-pwm-fan.c
> [ ... ]
>> +static irqreturn_t axiado_tach_irq_handler(int irq, void *dev)
>> +{
>> +	struct axiado_pwm_fan_ctx *ctx = dev;
>> +	u32 status;
>> +
>> +	status = ioread32(ctx->tach_base + AX_TACH_INT_STATUS_REG);
>> +
>> +	if (!status)
>> +		return IRQ_NONE;
> 
> [Severity: High]
> Will this handler cause an IRQ storm if the hardware asserts an interrupt
> where the status is non-zero but AX_TACH_INT_PENDING is not set?
> 
> Since the condition only checks if status is zero, an unexpected bit could
> bypass the IRQ_NONE return path. The handler would then proceed to clear only
> the AX_TACH_INT_PENDING bit and return IRQ_HANDLED, leaving the actual
> hardware interrupt active and bypassing the kernel's spurious IRQ protection.
> 
> Should this explicitly check if the AX_TACH_INT_PENDING bit is set before
> processing the interrupt?
> 

Please address this problem, or explain why it does not apply.

Thanks,
Guenter

>> +
>> +	scoped_guard(spinlock_irqsave, &ctx->tach_lock) {
>> +		ctx->tach.count = ioread32(ctx->tach_base + AX_TACH_COUNT_REG);
>> +	}
>> +
>> +	iowrite32(AX_TACH_INT_PENDING, ctx->tach_base + AX_TACH_INT_STATUS_REG);
>> +
>> +	return IRQ_HANDLED;
>> +}
>