All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Petar Stepanovic" <pstepanovic@axiado.com>
Cc: conor+dt@kernel.org, robh@kernel.org, devicetree@vger.kernel.org,
	linux-hwmon@vger.kernel.org
Subject: Re: [PATCH 2/2] hwmon: add Axiado AX3000 and AX3005 PWM fan controller driver
Date: Thu, 06 Aug 2026 09:20:48 +0000	[thread overview]
Message-ID: <20260806092049.D078D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260806-axiado-ax3000-pwm-fan-v1-2-f6db4cf2166b@axiado.com>

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 <pstepanovic@axiado.com>

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/20260806-axiado-ax3000-pwm-fan-v1-0-f6db4cf2166b@axiado.com?part=2

  reply	other threads:[~2026-08-06  9:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06  9:01 [PATCH 0/2] hwmon: add Axiado AX3000 and AX3005 PWM fan support Petar Stepanovic
2026-08-06  9:01 ` [PATCH 1/2] dt-bindings: hwmon: add Axiado AX3000 and AX3005 PWM fan controller Petar Stepanovic
2026-08-06  9:13   ` sashiko-bot
2026-08-06 10:41   ` Rob Herring (Arm)
2026-08-06 13:16   ` Rob Herring
2026-08-06  9:01 ` [PATCH 2/2] hwmon: add Axiado AX3000 and AX3005 PWM fan controller driver Petar Stepanovic
2026-08-06  9:20   ` sashiko-bot [this message]
2026-08-06 10:04   ` Uwe Kleine-König
2026-08-06 15:34     ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806092049.D078D1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=pstepanovic@axiado.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.