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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The tachometer IRQ handler blindly clears a hardcoded bit instead of verifying the interrupt status, potentially causing an IRQ storm.
--

--- Patch [2]: [PATCH 2/2] hwmon: add Axiado AX3000 and AX3005 PWM fan controller driver ---
commit f5b1a22a2979f32a82f8a025aa241070c85d00ae
Author: Petar Stepanovic <[email protected]>

hwmon: add Axiado AX3000 and AX3005 PWM fan controller driver

This commit adds support for the PWM fan controller on Axiado AX3000 and
AX3005 SoCs. The driver uses a hardware tachometer block and interrupt to
measure fan speed, while controlling 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
> --- /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;
> +
> +	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);

[Severity: Medium]
Does this risk an IRQ storm if the hardware sets any bit other than
AX_TACH_INT_PENDING?

If a different bit in AX_TACH_INT_STATUS_REG triggers the interrupt, this
handler will see a non-zero status, proceed to update the count, and then
blindly clear only AX_TACH_INT_PENDING (BIT(0)). The other bit would remain
set, and the interrupt line would remain asserted.

Should this check if AX_TACH_INT_PENDING is actually set in the status, and
clear the specific status bits that were read rather than hardcoding the
cleared bit?

> +
> +	return IRQ_HANDLED;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2