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

[Mark Brown]

On Tue, Mar 31, 2026 at 09:42:07PM -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.

> +static int tas675x_dapm_sleep_event(struct snd_soc_dapm_widget *widget,
> +				    struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component = snd_soc_dapm_to_component(widget->dapm);
> +	int ret = 0;
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		ret = pm_runtime_resume_and_get(component->dev);
> +		break;
> +	case SND_SOC_DAPM_PRE_PMD:
> +		pm_runtime_mark_last_busy(component->dev);
> +		pm_runtime_put_autosuspend(component->dev);
> +		break;
> +	}
> +	return ret < 0 ? ret : 0;
> +}

I'm confused what's going on here, there's runtime PM management in the
ASoC core - isn't this just duplicating what's there?

> +static int tas675x_runtime_resume(struct device *dev)
> +{

> +       if (!to_i2c_client(dev)->irq)
> +               schedule_delayed_work(&tas->fault_check_work,
> +                                     msecs_to_jiffies(TAS675X_FAULT_CHECK_INTERVAL_MS));

This is the only place where we start the fault checking but runtime PM
can be disabled in configuration and we also start runtime PM in the
active state so potentially might never suspend and resume.  probe()
should kick off the work as well.

> +static int tas675x_init_device(struct tas675x_priv *tas)
> +{
> +	struct regmap *regmap = tas->regmap;
> +	unsigned int val;
> +	int ret, i;
> +
> +	/* Clear POR fault flag to prevent IRQ storm */
> +	regmap_read(regmap, TAS675X_POWER_FAULT_LATCHED_REG, &val);


> +static int tas675x_i2c_probe(struct i2c_client *client)
> +{

> +	if (client->irq) {
> +		ret = devm_request_threaded_irq(tas->dev, client->irq, NULL,
> +						tas675x_irq_handler,
> +						IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
> +						"tas675x-fault", tas);
> +		if (ret)
> +			return dev_err_probe(tas->dev, ret, "Failed to request IRQ\n");
> +	}

We request the interrupt here...

> +	INIT_DELAYED_WORK(&tas->fault_check_work, tas675x_fault_check_work);

> +	ret = tas675x_power_on(tas);
> +	if (ret)
> +		return ret;

...before we do _power_on() which is what calls _hw_init() and clears any
faults that were latched in the registers.  This means that if there is
something there we'll handle it through the normal interrupt handling
flow and the rest won't buy is much, probably better to reorder the init
vs the interrupt request.

Attachment: signature.asc
Description: PGP signature