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

[Mark Brown]

On Fri, Jul 10, 2026 at 12:13:29AM +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 const unsigned int tas2557_startup_data[] = {
> +	TAS2557_GPI_PIN_REG,              0x15,	/* enable DIN, MCLK, CCI */
> +	TAS2557_GPIO1_PIN_REG,            0x01,	/* enable BCLK (ASI1) */
> +	TAS2557_GPIO2_PIN_REG,            0x01,	/* enable WCLK (ASI1) */
> +	/* ASI2 GPIO wiring; arm the BCLK source (GPIO5) before its dividers */
> +	TAS2557_GPIO6_PIN_REG,            0x01,	/* GPIO6 = ASI2 WCLK input */
> +	TAS2557_GPIO8_PIN_REG,            0x02,	/* GPIO8 = ASI2 DIN */
> +	TAS2557_GPIO5_PIN_REG,            0x01,	/* GPIO5 = ASI2 BCLK input */

This looks suspicously board specific, it should probably come from
device tree.

> +	TAS2557_ASI2_DAC_FORMAT_REG,      0x18,	/* ASI2: 32-bit I2S */

This should be set_dai_fmt().

> +	TAS2557_ASI2_BDIV_CLK_SEL_REG,   0x01,
> +	TAS2557_ASI2_BDIV_CLK_RATIO_REG, 0x01,
> +	TAS2557_ASI2_BDIV_CLK_RATIO_REG, 0x81,	/* power up BDIV */
> +	TAS2557_ASI2_WDIV_CLK_RATIO_REG, 0x40,
> +	TAS2557_ASI2_WDIV_CLK_RATIO_REG, 0xc0,	/* power up WDIV */

Can't these be worked out dynamically?

> +	/* Power sequencing: Class-D + Boost first, then DSP/PLL */
> +	TAS2557_POWER_CTRL2_REG, 0xA0,		/* Class-D, Boost power up */
> +	TAS2557_POWER_CTRL2_REG, 0xA3,		/* Class-D, Boost, IV-sense power up */

tas2557_isense_put() also writes to this register, there's a disconnect
with blind writing.  

> +/* =========================================================================
> + * Firmware parsing
> + *
> + * Format (all multi-byte fields are big-endian):

Is there any overlap with other TI firmware parsers?

> +static irqreturn_t tas2557_irq_handler(int irq, void *data)
> +{
> +	struct tas2557_priv *tas2557 = data;
> +	unsigned int int1 = 0, int2 = 0, pwr_flag = 0;
> +	int ret;
> +
> +	mutex_lock(&tas2557->lock);
> +
> +	if (!tas2557->fw || !tas2557->powered) {
> +		mutex_unlock(&tas2557->lock);
> +		return IRQ_HANDLED;
> +	}

We definitely didn't handle an IRQ in this case - this will break
sharing and debug features in the interrupt core.

> +	/* Gate the chip's INT output while diagnosing the fault */
> +	tas2557_dev_write(tas2557, TAS2557_GPIO4_PIN_REG, 0x00);

Is GPIO4 the only line that can be an interrupt source?

Attachment: signature.asc
Description: PGP signature