Re: [PATCH v2 2/2] ASoC: codecs: add TAS2557 mono smart amplifier driver

[Mark Brown]

On Sat, Jul 11, 2026 at 04:57:21PM +0200, Gianluca Boiano wrote:

> The TAS2557 is a mono Class-D smart amplifier with an on-chip DSP for
> speaker protection. Audio is received over I2S/TDM (ASI) and processed
> by DSP firmware before reaching the Class-D output stage.

> +static int tas2557_set_program(struct tas2557_priv *tas2557,
> +			       unsigned int prog_idx, int config_idx)
> +{

> +	/* Auto-select config: match program + sample_rate (0 = accept any) */
> +	if (config_idx < 0) {
> +		config_idx = -1;
> +		for (cfg_idx = 0; cfg_idx < fw->num_configs; cfg_idx++) {
> +			if (fw->configs[cfg_idx].program != prog_idx)
> +				continue;
> +			if (tas2557->sample_rate == 0 ||
> +			    tas2557->sample_rate ==
> +					fw->configs[cfg_idx].sample_rate) {
> +				config_idx = (int)cfg_idx;
> +				break;
> +			}
> +		}
> +		if (config_idx < 0) {
> +			dev_err(tas2557->dev,
> +				"no config for program %u at %u Hz\n",
> +				prog_idx, tas2557->sample_rate);
> +			return -EINVAL;

This will reject attempts to start audio based on the sample rates
supported by the firmware but there's nothing that validates these
sample rates in hw_params(), nor anything that tells userspace about the
constraints coming from the firmware.

Also does the fact that we're using a single firmware config mean that
the driver needs to flag symmetric_rates?

> +static int tas2557_volume_put(struct snd_kcontrol *kc,
> +			      struct snd_ctl_elem_value *ucontrol)
> +{

> +	tas2557->dac_gain = gain;
> +
> +	if (tas2557->powered)
> +		ret = tas2557_dev_update_bits(tas2557, TAS2557_SPK_CTRL_REG,
> +					      TAS2557_DAC_GAIN_MASK,
> +					      gain << TAS2557_DAC_GAIN_SHIFT);
> +
> +	mutex_unlock(&tas2557->lock);
> +
> +	if (ret < 0)
> +		return ret;

It'd be better to avoid updating the cached gain if the write failed,
keep things consistent as much as possible?

Attachment: signature.asc
Description: PGP signature