Skip to contentSkip to conversation

ASoC: codecs: add Texas Instruments TAS2557 smart amplifier driver

PATCH Superseded Gianluca Boiano 4 participants 8 messages Jul 11 – Jul 13

Subsystems (4)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover ASoC: codecs: add Texas Instruments TAS2557 smart amplifier driver

The TAS2557 is a mono Class-D smart amplifier with an on-chip DSP for speaker protection, controlled over I2C with audio carried over I2S/TDM. The DSP requires a board-specific firmware image generated with TI's PurePath Console tuning tools; without it the device passes no audio.

Patch 1 adds the devicetree binding. Patch 2 adds the ASoC codec
driver: mono and dual-device stereo operation, per-device ti,channel tuning, and fault-interrupt recovery that reloads the firmware configuration on clock loss, over-current, brownout or over-temperature.

Tested in mono configuration on Xiaomi Mi A2 (SDM660, jasmine), Xiaomi Redmi Note 6 Pro (SDM636, tulip) and Xiaomi Redmi 5 Plus (MSM8953, vince), driving the top speaker, and in dual-device stereo configuration on Xiaomi Mi Pad 4 (SDM660, clover).

Changes in v2:
- add the four mandatory supplies (vbat/iovdd/avdd/dvdd) to the devicetree binding's "required" list (Wang Sen)
- propagate register-write failures out of hw_params() instead of only logging them (Wang Sen)
- return IRQ_NONE instead of IRQ_HANDLED from the fault interrupt handler when the device isn't in a state to have raised it, so shared-IRQ accounting stays correct (Mark Brown)
- re-apply cached runtime state (DAI format, sense enables, DAC gain, ASI slot offsets) after every device reset, including fault recovery, instead of letting a hardcoded startup sequence silently override set_dai_fmt()/the ISENSE/VSENSE mixer controls (Mark Brown)
- honour explicit snd_soc_dai_set_tdm_slot() configuration in hw_params, and restore TX (OFFSET2) slot offsets after reset
- wait for the asynchronous firmware load on first stream start instead of failing with -ENODEV
- make mute_stream direction-aware so capture-only IV-sense streams are unmuted and stopping playback no longer mutes a live capture
- corrected a comment implying the ASI2/GPIO5-8 wiring in tas2557_startup_data comes from a TI reference sequence; it does not (TI's own driver never touches ASI2)

Link to v1: https://lore.kernel.org/linux-sound/[email protected]/

Gianluca Boiano (2):
dt-bindings: sound: add Texas Instruments TAS2557
ASoC: codecs: add TAS2557 mono smart amplifier driver

 .../devicetree/bindings/sound/ti,tas2557.yaml |  127 +
 MAINTAINERS                                   |    1 +
 sound/soc/codecs/Kconfig                      |   11 +
 sound/soc/codecs/Makefile                     |    2 +
 sound/soc/codecs/tas2557.c                    | 2648 +++++++++++++++++
 sound/soc/codecs/tas2557.h                    |  218 ++
 6 files changed, 3007 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2557.yaml
 create mode 100644 sound/soc/codecs/tas2557.c
 create mode 100644 sound/soc/codecs/tas2557.h

base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482

Signature · 1 line
2.55.0
#1 lore raw

PATCH v2 1/2 dt-bindings: sound: add Texas Instruments TAS2557

Document the TAS2557 mono Class-D smart amplifier with an integrated DSP for speaker protection. The device is controlled over I2C and receives audio over I2S/TDM (ASI). It can drive a single mono speaker or, as two devices, a stereo pair; in that case the ti,channel property selects the audio slot each device reproduces and the per-device tuning it applies.

The on-chip DSP boots from a firmware image generated per speaker design with the TI PurePath Console tuning tools; the optional firmware-name property selects a board-specific image.

---
v2: add the four mandatory supplies to the "required" list (Wang Sen)

 .../devicetree/bindings/sound/ti,tas2557.yaml | 127 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2557.yaml
Documentation/devicetree/bindings/sound/ti,tas2557.yaml new file +127 -0
new file mode 100644
index 000000000000..862ea4036cb8
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2026 Gianluca Boiano <[email protected]>
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ti,tas2557.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments TAS2557 Mono Smart Amplifier
+
+maintainers:
+  - Gianluca Boiano <[email protected]>
+
+description: |
+  The TAS2557 is a mono, digital-input Class-D amplifier with an
+  integrated DSP for speaker protection. Audio is carried over I2S/TDM
+  (ASI). The on-chip DSP boots from a firmware image that is generated
+  for a specific speaker design using the TI PurePath Console tuning
+  tools.
+
+  Stereo playback is built from two devices, each reproducing one audio
+  slot selected through the ti,channel property and applying the
+  matching per-device tuning contained in a stereo-tuned firmware
+  image.
+
+  Datasheet: https://www.ti.com/lit/gpn/tas2557
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    const: ti,tas2557
+
+  reg:
+    maxItems: 1
+
+  '#sound-dai-cells':
+    const: 0
+
+  reset-gpios:
+    maxItems: 1
+    description: GPIO connected to the active-low RESET pin.
+
+  interrupts:
+    maxItems: 1
+    description: Fault interrupt output (INT).
+
+  vbat-supply:
+    description: Battery/boost input supply (VBAT), 2.9 V to 5.5 V.
+
+  iovdd-supply:
+    description: Digital I/O supply, 1.62 V to 3.6 V.
+
+  avdd-supply:
+    description: Analog supply, 1.65 V to 1.95 V.
+
+  dvdd-supply:
+    description: Digital core supply, 1.65 V to 1.95 V.
+
+  ti,imon-slot-no:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: TDM TX time slot for current-sense data.
+    default: 0
+    minimum: 0
+    maximum: 7
+
+  ti,vmon-slot-no:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: TDM TX time slot for voltage-sense data.
+    default: 2
+    minimum: 0
+    maximum: 7
+
+  ti,channel:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Audio slot of the stereo stream reproduced by this device (0 =
+      left, 1 = right). With stereo firmware this also selects which
+      per-device tuning set is applied.
+    enum: [0, 1]
+    default: 0
+
+  firmware-name:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: |
+      Name of the DSP firmware image to load. Defaults to a name
+      derived from the silicon revision. Stereo systems typically use
+      a stereo-tuned image shared by both devices.
+
+required:
+  - compatible
+  - reg
+  - '#sound-dai-cells'
+  - vbat-supply
+  - iovdd-supply
+  - avdd-supply
+  - dvdd-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        audio-codec@4c {
+            compatible = "ti,tas2557";
+            reg = <0x4c>;
+            #sound-dai-cells = <0>;
+            reset-gpios = <&tlmm 77 GPIO_ACTIVE_LOW>;
+            interrupt-parent = <&tlmm>;
+            interrupts = <73 IRQ_TYPE_LEVEL_HIGH>;
+
+            vbat-supply = <&vph_pwr>;
+            iovdd-supply = <&vreg_l6b_1p8>;
+            avdd-supply = <&vreg_l6b_1p8>;
+            dvdd-supply = <&vreg_l6b_1p8>;
+
+            ti,imon-slot-no = <0>;
+            ti,vmon-slot-no = <2>;
+            ti,channel = <0>;
+        };
+    };
MAINTAINERS +1 -0
index 15011f5752a9..8a824da3979e 100644
@@ -26677,6 +26677,7 @@ M:	Sen Wang <[email protected]>
 L:	[email protected]
 S:	Maintained
 F:	Documentation/devicetree/bindings/sound/ti,tas2552.yaml
+F:	Documentation/devicetree/bindings/sound/ti,tas2557.yaml
 F:	Documentation/devicetree/bindings/sound/ti,tas2562.yaml
 F:	Documentation/devicetree/bindings/sound/ti,tas2770.yaml
 F:	Documentation/devicetree/bindings/sound/ti,tas27xx.yaml
Signature · 1 line
2.55.0
#2 lore raw

Re: [PATCH v2 1/2] dt-bindings: sound: add Texas Instruments TAS2557

Quoted 5 lines · Gianluca Boiano
Documentation/devicetree/bindings/sound/ti,tas2557.yaml new file +5 -0
+  ti,imon-slot-no:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: TDM TX time slot for current-sense data.
+    default: 0
+    minimum: 0

Documentation/devicetree/bindings/sound/ti,tas2557.yaml line 74

Quoted 10 lines
Documentation/devicetree/bindings/sound/ti,tas2557.yaml new file +10 -0
+    maximum: 7
+
+  ti,vmon-slot-no:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: TDM TX time slot for voltage-sense data.
+    default: 2
+    minimum: 0
+    maximum: 7
+
+  ti,channel:

ti,audio-channel

Documentation/devicetree/bindings/sound/ti,tas2557.yaml line 81

Quoted 7 lines
Documentation/devicetree/bindings/sound/ti,tas2557.yaml new file +7 -0
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Audio slot of the stereo stream reproduced by this device (0 =
+      left, 1 = right). With stereo firmware this also selects which
+      per-device tuning set is applied.
+    enum: [0, 1]
+    default: 0

Most speakers do not need such property, especially that what do you do in 4-speaker configuration? I found only two references: awinic,aw87390 and awinic,aw88395.yaml.

+

Documentation/devicetree/bindings/sound/ti,tas2557.yaml line 84

Quoted 2 lines
Documentation/devicetree/bindings/sound/ti,tas2557.yaml new file +2 -0
+  firmware-name:
+    $ref: /schemas/types.yaml#/definitions/string

Wrong type - there is basically no syntax like that (except a few left-overs which I remove now), so please kindly do not upstrem some old code. Drop.

You need maxItems.

Best regards,
Krzysztof

#3 lore raw

Re: [PATCH v2 1/2] dt-bindings: sound: add Texas Instruments TAS2557

Quoted 7 lines

+ ti,channel:

ti,audio-channel

Most speakers do not need such property, especially that what do you do in 4-speaker configuration? I found only two references: awinic,aw87390 and awinic,aw88395.yaml.

Dropped in v3. A node now takes a reg array of 1 or 2 addresses; the device at each index applies that half (DEV_A/DEV_B) of the stereo firmware. Device order is the only configuration needed - no property. This matches tas2781.c/tas2781-fmwlib.c's architecture for the same firmware toolchain.

reg is capped at two because the firmware container itself only defines two device types (DEV_A/DEV_B). A 4-speaker board would need two independent stereo pairs on two nodes, same as with tas2781.

Quoted 7 lines

+ $ref: /schemas/types.yaml#/definitions/string

Wrong type - there is basically no syntax like that (except a few left-overs which I remove now), so please kindly do not upstrem some old code. Drop.

You need maxItems.

Fixed in v3: maxItems: 1.

Gianluca

maintainer
#4 lore raw

Re: [PATCH v2 1/2] dt-bindings: sound: add Texas Instruments TAS2557

In reply to Gianluca Boiano
Quoted 6 lines · Gianluca Boiano

Document the TAS2557 mono Class-D smart amplifier with an integrated DSP for speaker protection. The device is controlled over I2C and receives audio over I2S/TDM (ASI). It can drive a single mono speaker or, as two devices, a stereo pair; in that case the ti,channel property selects the audio slot each device reproduces and the per-device tuning it applies.

Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone.

#5 lore raw

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

In reply to Gianluca Boiano

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.

The register map is book/page indirected behind an 8-bit regmap; the driver layers composite book/page/register addressing on top of it. The TI "uCDSP" firmware format (PLL, program and configuration blocks) is parsed with bounds checking, and program 0 with a sample-rate matched configuration is applied whenever the amplifier powers up; the device cannot pass audio without this firmware. Firmware images are produced per speaker design with TI's PurePath Console tuning tools, so they are board specific; the default image name is derived from the detected silicon revision and can be overridden with the firmware-name device tree property.

Amplifier power is managed through DAPM. A fault interrupt handler recovers the device from clock loss, over-current, brownout and over-temperature events by reloading the firmware configuration. Speaker volume and I/V-sense switches are exposed as ALSA controls. Stereo is supported as two cooperating instances using stereo (device 3) firmware, with per-instance DEV_A/DEV_B tuning selected by the ti,channel property.

---
v2:
- propagate register-write failures out of hw_params() instead of only logging them (Wang Sen)
- return IRQ_NONE instead of IRQ_HANDLED when the device isn't in a state to have raised the interrupt (Mark Brown)
- re-apply cached runtime state (DAI format, sense enables, DAC gain, ASI slot offsets) after every reset, including fault recovery, instead of a hardcoded sequence silently overriding set_dai_fmt() and the ISENSE/VSENSE mixer controls (Mark Brown)
- honour explicit snd_soc_dai_set_tdm_slot() configuration in hw_params, restore TX (OFFSET2) slot offsets after reset
- wait for the asynchronous firmware load on first stream start instead of failing with -ENODEV
- make mute_stream direction-aware so capture-only IV-sense streams are unmuted and stopping playback doesn't mute a live capture
- corrected a comment implying ASI2/GPIO5-8 wiring in tas2557_startup_data comes from a TI reference sequence

 sound/soc/codecs/Kconfig   |   11 +
 sound/soc/codecs/Makefile  |    2 +
 sound/soc/codecs/tas2557.c | 2648 ++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/tas2557.h |  218 +++
 4 files changed, 2879 insertions(+)
 create mode 100644 sound/soc/codecs/tas2557.c
 create mode 100644 sound/soc/codecs/tas2557.h
sound/soc/codecs/Kconfig +11 -0
index 252f683be3c1..cf56b554dcca 100644
@@ -268,6 +268,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_STI_SAS
 	imply SND_SOC_TAC5XX2_SDW
 	imply SND_SOC_TAS2552
+	imply SND_SOC_TAS2557
 	imply SND_SOC_TAS2562
 	imply SND_SOC_TAS2764
 	imply SND_SOC_TAS2770
@@ -2167,6 +2168,16 @@ config SND_SOC_TAS2552
 	tristate "Texas Instruments TAS2552 Mono Audio amplifier"
 	depends on I2C
 
+config SND_SOC_TAS2557
+	tristate "Texas Instruments TAS2557 Mono Smart Amplifier"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Enable support for the Texas Instruments TAS2557 mono smart
+	  amplifier with integrated DSP. The on-chip DSP requires firmware
+	  (tas2557_uCDSP.bin) to be present in the firmware search path;
+	  audio cannot pass through the device without it.
+
 config SND_SOC_TAS2562
 	tristate "Texas Instruments TAS2562 Mono Audio amplifier"
 	depends on I2C
sound/soc/codecs/Makefile +2 -0
index aa0396e5b575..b8a2f639a24d 100644
@@ -433,6 +433,7 @@ snd-soc-max98504-y := max98504.o
 snd-soc-simple-amplifier-y := simple-amplifier.o
 snd-soc-tpa6130a2-y := tpa6130a2.o
 snd-soc-tas2552-y := tas2552.o
+snd-soc-tas2557-y := tas2557.o
 snd-soc-tas2562-y := tas2562.o
 snd-soc-tas2764-y := tas2764.o
 snd-soc-tas2780-y := tas2780.o
@@ -754,6 +755,7 @@ obj-$(CONFIG_SND_SOC_STAC9766)	+= snd-soc-stac9766.o
 obj-$(CONFIG_SND_SOC_STI_SAS)	+= snd-soc-sti-sas.o
 obj-$(CONFIG_SND_SOC_TAC5XX2_SDW)	+= snd-soc-tac5xx2-sdw.o
 obj-$(CONFIG_SND_SOC_TAS2552)	+= snd-soc-tas2552.o
+obj-$(CONFIG_SND_SOC_TAS2557)	+= snd-soc-tas2557.o
 obj-$(CONFIG_SND_SOC_TAS2562)	+= snd-soc-tas2562.o
 obj-$(CONFIG_SND_SOC_TAS2764)	+= snd-soc-tas2764.o
 obj-$(CONFIG_SND_SOC_TAS2780)	+= snd-soc-tas2780.o
sound/soc/codecs/tas2557.c new file +2648 -0

2,648 lines not on this page · raw message

sound/soc/codecs/tas2557.h new file +218 -0

218 lines not on this page · raw message

Signature · 1 line
2.55.0
#6 lore raw

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

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] Incorrect ASI data-slot offset calculation for the right channel skips the correct audio slot.…

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Incorrect ASI data-slot offset calculation for the right channel skips the correct audio slot.
- [Medium] Missing SNS_CTRL_REG configuration when enabling ISENSE/VSENSE ALSA controls at runtime.
- [Medium] tas2557_failsafe_recovery unintentionally unmutes the capture path upon fault recovery.
- [Medium] tas2557_set_dai_fmt() updates DAC format registers but fails to update ADC format registers.
- [Medium] tas2557_startup_data hardcodes ASI2 DAC format but omits ASI1 DAC format configuration. --

--- Patch [2]: [PATCH v2 2/2] ASoC: codecs: add TAS2557 mono smart amplifier driver --- commit d1471370278591a7f73ca18bb2ac75981e6b8a92
Author: Gianluca Boiano <[email protected]>
Subject: ASoC: codecs: add TAS2557 mono smart amplifier driver

The TAS2557 is a mono Class-D smart amplifier with an on-chip DSP for speaker protection. It uses I2S/TDM and requires board-specific firmware loaded per speaker design. Amplifier power is managed through DAPM and includes fault interrupt recovery and volume controls.

Quoted 4 lines
sound/soc/codecs/tas2557.c new file +0 -0
new file mode 100644

[ ... ]

sound/soc/codecs/tas2557.c line 205

Quoted 9 lines
sound/soc/codecs/tas2557.c new file +9 -0
+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 */
+	TAS2557_ASI2_DAC_FORMAT_REG,      0x18,	/* ASI2: 32-bit I2S */

[Severity: Medium]
Is it intentional to omit the equivalent formatting register for ASI1 here?

The comment above mentions both ASI interfaces are enabled and configured identically, but if the board wiring uses ASI1, it looks like it would be left at its reset default word length while ASI2 is explicitly set to 32-bit.

Quoted 1 line
sound/soc/codecs/tas2557.c new file +1 -0
+	TAS2557_ASI2_BDIV_CLK_SEL_REG,   0x01,

[ ... ]

Quoted 2 lines
sound/soc/codecs/tas2557.c new file +2 -0
+static int tas2557_failsafe_recovery(struct tas2557_priv *tas2557)
+{

[ ... ]

Quoted 8 lines
sound/soc/codecs/tas2557.c new file +8 -0
+	if (tas2557->powered) {
+		ret = tas2557_load_data(tas2557, tas2557_startup_data);
+		if (ret < 0)
+			return ret;
+
+		ret = tas2557_apply_runtime_state(tas2557);
+		if (ret < 0)
+			return ret;

sound/soc/codecs/tas2557.c line 1559

Quoted 10 lines
sound/soc/codecs/tas2557.c new file +10 -0
+
+		ret = tas2557_load_config_post_power(tas2557);
+		if (ret < 0)
+			return ret;
+
+		if (!tas2557->dac_muted) {
+			ret = tas2557_load_data(tas2557, tas2557_unmute_data);
+			if (ret < 0)
+				return ret;
+		} else {

[Severity: Medium]
Does this sequence inadvertently unmute the capture path upon fault recovery?

When the DAC is unmuted but the capture stream is muted, calling tas2557_load_data with tas2557_unmute_data clears the mute bits for both the DAC and ISENSE paths.

Should this check tas2557->sense_muted and re-assert the capture mutes like it does in tas2557_enable?

Quoted 2 lines
sound/soc/codecs/tas2557.c new file +2 -0
+			/* Muted: clocks not guaranteed, keep detection off */
+			ret = tas2557_clk_err_detect(tas2557, false);

[ ... ]

Quoted 4 lines
sound/soc/codecs/tas2557.c new file +4 -0
+static int tas2557_hw_params(struct snd_pcm_substream *substream,
+			     struct snd_pcm_hw_params *params,
+			     struct snd_soc_dai *dai)
+{

[ ... ]

sound/soc/codecs/tas2557.c line 1881

Quoted 5 lines
sound/soc/codecs/tas2557.c new file +5 -0
+	if (!tas2557->tdm_configured) {
+		width = tas2557->tdm_slot_width > 0 ?
+				tas2557->tdm_slot_width :
+				params_physical_width(params);
+		offset = (tas2557->channel == 0) ? 1 : 1 + width * 2;

[Severity: High]
Can this offset calculation skip the correct audio slot for the right channel?

For a 32-bit word, 1 + width * 2 evaluates to 65 BCLKs, which falls completely outside the standard 64-bit stereo I2S frame where the right channel is expected at offset 33.

Quoted 2 lines
sound/soc/codecs/tas2557.c new file +2 -0
+		tas2557->asi_offset = offset;
+	}

[ ... ]

Quoted 2 lines
sound/soc/codecs/tas2557.c new file +2 -0
+static int tas2557_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{

[ ... ]

sound/soc/codecs/tas2557.c line 1948

Quoted 8 lines
sound/soc/codecs/tas2557.c new file +8 -0
+	mutex_lock(&tas2557->lock);
+	tas2557->dai_fmt = asi_fmt;
+	ret = tas2557_dev_update_bits(tas2557, TAS2557_ASI1_DAC_FORMAT_REG,
+				      TAS2557_FORMAT_MASK, asi_fmt);
+	if (ret >= 0)
+		ret = tas2557_dev_update_bits(tas2557,
+					      TAS2557_ASI2_DAC_FORMAT_REG,
+					      TAS2557_FORMAT_MASK, asi_fmt);

[Severity: Medium]
Should the ADC format registers be updated here as well?

This currently updates the DAC format registers (TAS2557_ASI1_DAC_FORMAT_REG and TAS2557_ASI2_DAC_FORMAT_REG) but leaves the corresponding ADC format registers unmodified at their reset defaults. Could this cause the capture stream to use the wrong ASI data format?

Quoted 1 line
sound/soc/codecs/tas2557.c new file +1 -0
+	mutex_unlock(&tas2557->lock);

[ ... ]

Quoted 3 lines
sound/soc/codecs/tas2557.c new file +3 -0
+static int tas2557_isense_put(struct snd_kcontrol *kc,
+			      struct snd_ctl_elem_value *ucontrol)
+{

[ ... ]

sound/soc/codecs/tas2557.c line 2196

Quoted 6 lines
sound/soc/codecs/tas2557.c new file +6 -0
+	tas2557->isense_enabled = en;
+
+	if (tas2557->powered)
+		ret = tas2557_dev_update_bits(tas2557, TAS2557_POWER_CTRL2_REG,
+					      TAS2557_ISENSE_ENABLE,
+					      en ? TAS2557_ISENSE_ENABLE : 0);

[Severity: Medium]
If the amplifier powers up with ISENSE and VSENSE disabled, does enabling them later via this control miss updating TAS2557_SNS_CTRL_REG?

In tas2557_apply_runtime_state, the slot routing is only written if at least one sense line is already enabled. If the ALSA switches are turned on later, the hardware might transmit on the default slot instead of the DT-configured slot.

Quoted 2 lines
sound/soc/codecs/tas2557.c new file +2 -0
+
+	mutex_unlock(&tas2557->lock);

[ ... ]

Quoted 3 lines
sound/soc/codecs/tas2557.c new file +3 -0
+static int tas2557_vsense_put(struct snd_kcontrol *kc,
+			      struct snd_ctl_elem_value *ucontrol)
+{

[ ... ]

sound/soc/codecs/tas2557.c line 2236

Quoted 6 lines
sound/soc/codecs/tas2557.c new file +6 -0
+	tas2557->vsense_enabled = en;
+
+	if (tas2557->powered)
+		ret = tas2557_dev_update_bits(tas2557, TAS2557_POWER_CTRL2_REG,
+					      TAS2557_VSENSE_ENABLE,
+					      en ? TAS2557_VSENSE_ENABLE : 0);

[Severity: Medium]
Similarly, does toggling VSENSE power at runtime need to configure the SNS_CTRL_REG routing if it wasn't done during startup?

Quoted 2 lines
sound/soc/codecs/tas2557.c new file +2 -0
+
+	mutex_unlock(&tas2557->lock);

[ ... ]

Quoted 2 lines
sound/soc/codecs/tas2557.c new file +2 -0
+static int tas2557_i2c_probe(struct i2c_client *client)
+{

[ ... ]

sound/soc/codecs/tas2557.c line 2555

Quoted 6 lines
sound/soc/codecs/tas2557.c new file +6 -0
+	/*
+	 * Seed the cached ASI data-slot offset before any hw_params() or
+	 * set_tdm_slot() call arrives; 32 is the fixed ASI word length
+	 * used by tas2557_startup_data.
+	 */
+	tas2557->asi_offset = (tas2557->channel == 0) ? 1 : 1 + 32 * 2;

[Severity: High]
Does this seed the same incorrect right channel offset mentioned earlier in tas2557_hw_params?

Quoted 3 lines
sound/soc/codecs/tas2557.c new file +3 -0
+
+	ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(tas2557_supplies),
+					     tas2557_supplies);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
maintainer
#7 lore raw

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

In reply to Gianluca Boiano
Quoted 16 lines · Gianluca Boiano

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.

sound/soc/codecs/tas2557.c new file +11 -0
+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 ==

sound/soc/codecs/tas2557.c line 1179

Quoted 10 lines
sound/soc/codecs/tas2557.c new file +10 -0
+					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?

Quoted 5 lines
sound/soc/codecs/tas2557.c new file +4 -0
+static int tas2557_volume_put(struct snd_kcontrol *kc,
+			      struct snd_ctl_elem_value *ucontrol)
+{
 
+	tas2557->dac_gain = gain;

sound/soc/codecs/tas2557.c line 2161

Quoted 10 lines
sound/soc/codecs/tas2557.c new file +10 -0
+
+	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?