Re: [PATCH v2 2/4] ASoC: codecs: Add TAS675x quad-channel audio amplifier driver

Mark Brown
2026-04-02T17:10:30Z

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

On Wed, Apr 01, 2026 at 05:28:43PM -0500, Sen Wang wrote:
> The TAS675x (TAS6754, TAS67524) are quad-channel, digital-input
> Class-D amplifiers with an integrated DSP, controlled over I2C.
> They support I2S and TDM serial audio interfaces.

One issue that didn't get noticed last time, sorry:

> +static int tas675x_runtime_suspend(struct device *dev)
> +{
> + struct tas675x_priv *tas = dev_get_drvdata(dev);
> +
> + cancel_delayed_work_sync(&tas->fault_check_work);
> + tas675x_set_state_all(tas, TAS675X_STATE_SLEEP_BOTH);
> +
> + return 0;
> +}

This cancels the work, completing any that's already running, but...

> +static void tas675x_fault_check_work(struct work_struct *work)
> +{
> + struct tas675x_priv *tas = container_of(work, struct tas675x_priv,
> + fault_check_work.work);
> +
> + if (tas675x_check_faults(tas))
> + regmap_write(tas->regmap, TAS675X_RESET_REG, TAS675X_FAULT_CLEAR);
> +
> + schedule_delayed_work(&tas->fault_check_work,
> + msecs_to_jiffies(TAS675X_FAULT_CHECK_INTERVAL_MS));
> +}

...the work unconditionally rearms itself so we might race and requeue
(we cancel *then* wait) with the device powered off. There's the
disable_delayed_work_sync() API which should be a better fit.

> +static irqreturn_t tas675x_irq_handler(int irq, void *data)
> +{
> + struct tas675x_priv *tas = data;
> +
> + tas675x_check_faults(tas);
> +
> + /* Clear the FAULT pin latch as something latched */
> + regmap_write(tas->regmap, TAS675X_RESET_REG, TAS675X_FAULT_CLEAR);
> +
> + return IRQ_HANDLED;
> +}

Also, this should return IRQ_NONE if no faults were seen (to allow for
interrupt sharing and the genirq core's handling of hardware faults).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]