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

[Mark Brown]

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: signature.asc
Description: PGP signature