ASoC: codecs: add Texas Instruments TAS2557 smart amplifier driver
v3 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. A single node manages one physical TAS2557, or two of them sharing one ASI bus and one reset/IRQ pair, using the reg-array convention of ti,tas2781.yaml: the device at reg index 0 is DEV_A and the device at index 1 is DEV_B, so no channel-selection property is needed. The driver covers mono and dual-device stereo operation and fault-interrupt recovery that reloads the firmware configuration on clock loss, over-current, brownout or over-temperature.
The driver has been exercised across development of this series 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 on Xiaomi Mi Pad 4 (SDM660, clover). The v3 ASI word-length and data-slot-offset changes follow hardware reports from vince (16-bit, offset 0) and have not yet been re-confirmed on the 32-bit/offset-1 boards above.
Changes in v3:
- rework the binding and driver to a single-node, reg-array model that manages up to two devices, matching ti,tas2781.yaml; drop the ti,channel property entirely and derive DEV_A/DEV_B tuning from the device order in reg (Krzysztof Kozlowski)
- rename ti,imon-slot-no/ti,vmon-slot-no to the plural ti,imon-slot-nos/ti,vmon-slot-nos arrays (one entry per reg entry), and constrain firmware-name to maxItems: 1 (Krzysztof Kozlowski)
- derive the ASI word length from the stream (params_width) and re-apply it after each reset instead of hardcoding 32-bit, so 16-bit boards are no longer forced to 32-bit
- set symmetric_rate on the DAI (Mark Brown), and add symmetric_sample_bits since one firmware config (one rate) is active at a time and playback and capture share the single ASI word length
- apply the same write-then-cache ordering used for the DAC-gain control to the ISENSE/VSENSE mixer setters, so a failed POWER_CTRL2 write no longer leaves the cached enable state stale
- only cache the DAC gain after the hardware write succeeds, so a failed write no longer leaves the cache out of sync (Mark Brown)
- default the ASI data-slot offset to 0; an explicit snd_soc_dai_set_tdm_slot() still takes precedence
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
Link to v2: https://lore.kernel.org/linux-sound/[email protected]/
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 | 174 + MAINTAINERS | 1 + sound/soc/codecs/Kconfig | 11 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/tas2557.c | 3067 +++++++++++++++++ sound/soc/codecs/tas2557.h | 228 ++ 6 files changed, 3483 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: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
Signature · 1 line
2.55.0
PATCH v3 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). One node can describe either a single mono device or, via a two-entry reg, a stereo pair sharing one bus: the physical device at each reg index applies that half (DEV_A/DEV_B) of a stereo-tuned firmware image, so no separate channel-selection property is needed -- device order in reg is the only configuration required, matching the convention already used by ti,tas2781.yaml/ti,tas2563.yaml for the same TI firmware toolchain. ti,imon-slot-nos/ti,vmon-slot-nos follow the same per-index convention for the TDM sense-data slots. reg is capped at two entries because the uCDSP firmware container only defines mono or two-device (DEV_A/DEV_B) types -- there is no third slot to extend into regardless of DT shape.
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, shared by every device in the group.
Addresses review from Krzysztof Kozlowski, who questioned a per-node channel-selection property (an earlier ti,channel/ ti,audio-channel design) on the grounds that it doesn't scale and isn't how other multi-device TI amplifiers in-tree solve the same problem.
---
Changes in v3:
- single-node reg-array model (up to two devices), matching ti,tas2781.yaml; drop ti,channel, derive DEV_A/DEV_B from reg order
- rename ti,imon-slot-no/ti,vmon-slot-no to the plural ti,imon-slot-nos/ti,vmon-slot-nos arrays; firmware-name maxItems: 1
Changes in v2:
- add the four mandatory supplies to "required" (Wang Sen)
.../devicetree/bindings/sound/ti,tas2557.yaml | 174 ++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 175 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/ti,tas2557.yaml
Documentation/devicetree/bindings/sound/ti,tas2557.yaml new file +174 -0
@@ -0,0 +1,174 @@ +# 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. + + A single node describes either one physical TAS2557, or up to two of + them sharing one ASI bus and one reset/IRQ pair, using the same + reg-array convention as + Documentation/devicetree/bindings/sound/ti,tas2781.yaml. When two + devices are listed, `reg` carries both I2C addresses and the + physical device at each index is implicitly assigned one half of a + stereo-tuned firmware image: the device at index 0 is DEV_A and the + device at index 1 is DEV_B. No explicit channel-selection property + exists or is needed -- the order of addresses in `reg` is the only + configuration required. `ti,imon-slot-nos` and `ti,vmon-slot-nos` + follow the same indexing: the item at array index N sets the TDM TX + slot used by the device at the Nth `reg` entry. + + The uCDSP firmware format defines only the DEV_A and DEV_B device + types, so `reg` (and the two TDM slot properties) cannot list more + than two entries, regardless of how many physical speakers the board + has. + + Datasheet: https://www.ti.com/lit/gpn/tas2557 + +allOf: + - $ref: dai-common.yaml# + +properties: + compatible: + const: ti,tas2557 + + reg: + minItems: 1 + maxItems: 2 + description: + I2C address of each physical device sharing this node. One + entry configures a single mono TAS2557. Two entries configure a + device pair; the device at index 0 is DEV_A and the device at + index 1 is DEV_B. + + '#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-nos: + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 1 + maxItems: 2 + items: + minimum: 0 + maximum: 7 + description: + TDM TX time slot(s) for current-sense data, one entry per `reg` + entry. The item at array index N configures the device at the + Nth `reg` entry. + + ti,vmon-slot-nos: + $ref: /schemas/types.yaml#/definitions/uint32-array + minItems: 1 + maxItems: 2 + items: + minimum: 0 + maximum: 7 + description: + TDM TX time slot(s) for voltage-sense data, one entry per `reg` + entry. The item at array index N configures the device at the + Nth `reg` entry. + + firmware-name: + maxItems: 1 + description: | + Name of the DSP firmware image to load. Defaults to a name + derived from the silicon revision. A device pair shares one + image; each device's half of it is selected by its `reg` index, + not by this property. + +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-nos = <0>; + ti,vmon-slot-nos = <2>; + }; + }; + - | + #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>, <0x4d>; + #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-nos = <0 1>; + ti,vmon-slot-nos = <2 3>; + firmware-name = "tas2557-stereo.bin"; + }; + }; +...
MAINTAINERS +1 -0
@@ -26679,6 +26679,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
Re: [PATCH v3 1/2] dt-bindings: sound: add Texas Instruments TAS2557
Quoted 20 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). One node can describe either a single mono device or, via a two-entry
reg, a stereo pair sharing one bus: the physical device at eachregindex applies that half (DEV_A/DEV_B) of a stereo-tuned firmware image, so no separate channel-selection property is needed -- device order inregis the only configuration required, matching the convention already used byti,tas2781.yaml/ti,tas2563.yamlfor the same TI firmware toolchain.ti,imon-slot-nos/ti,vmon-slot-nosfollow the same per-index convention for the TDM sense-data slots.regis capped at two entries because the uCDSP firmware container only defines mono or two-device (DEV_A/DEV_B) types -- there is no third slot to extend into regardless of DT shape.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, shared by every device in the group.
Quoted 5 lines
Addresses review from Krzysztof Kozlowski, who questioned a per-node channel-selection property (an earlier ti,channel/ ti,audio-channel design) on the grounds that it doesn't scale and isn't how other multi-device TI amplifiers in-tree solve the same problem.
This paragraph does not look useful in commit log. This is part of changelog rather.
Quoted 9 lines
Signed-off-by Gianluca Boiano <[email protected]>---
Changes in v3:
- single-node reg-array model (up to two devices), matching ti,tas2781.yaml; drop ti,channel, derive DEV_A/DEV_B from reg order
- rename ti,imon-slot-no/ti,vmon-slot-no to the plural ti,imon-slot-nos/ti,vmon-slot-nos arrays; firmware-name maxItems: 1
Quoted 6 lines
Changes in v2:
- add the four mandatory supplies to "required" (Wang Sen).../devicetree/bindings/sound/ti,tas2557.yaml | 174 ++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 175 insertions(+)
Best regards,
Krzysztof
PATCH v3 2/2 ASoC: codecs: add TAS2557 mono smart amplifier driver
In reply to Gianluca BoianoThe 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.
A node manages 1 or 2 physical chips (reg cardinality), mirroring the multi-device architecture already used by tas2781.c/tas2781-fmwlib.c for the same TI firmware toolchain: one shared regmap retargets its I2C address per device before each transfer (tas2557_change_book_page()), one shared firmware request serves the whole group, and per-device DSP tuning (DEV_A/DEV_B) is dispatched purely by device index -- no channel-selection devicetree property exists or is needed. All per-device state (book/page cache, program/config indices, ASI sense-slot assignment) lives in a per-device array; group-level state (firmware, DAI format, TDM configuration, power/mute state) stays singular, since one DAI and one firmware image drive the whole group.
Amplifier power is managed through DAPM. A fault interrupt handler polls every device's fault registers and recovers the group 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, applied uniformly across the group. IRQ, reset and supply lines are shared across the group, matching common board wiring where both chips are strapped to the same GPIOs and rails.
Addresses review from Krzysztof Kozlowski (see the binding commit) by replacing the earlier per-node ti,channel/ti,audio-channel property with device-index dispatch, matching TAS2781's established convention instead of inventing a new one.
---
Changes in v3:
- rework to a single-node, reg-array driver managing up to two devices (per-device book/page/program/config state, group DAI/IRQ)
- derive the ASI word length from params_width() and re-apply it after each reset instead of hardcoding 32-bit
- set symmetric_rate on the DAI (Mark Brown), and symmetric_sample_bits (playback and capture share the single ASI word length)
- commit the DAC-gain cache only after the hardware write succeeds (Mark Brown), and apply the same ordering to the ISENSE/VSENSE mixer setters
- default the ASI data-slot offset to 0; explicit snd_soc_dai_set_tdm_slot() still takes precedence
Changes in v2:
- propagate hw_params() register-write failures (Wang Sen)
- return IRQ_NONE when the device was not in a state to raise the fault IRQ, for correct shared-IRQ accounting (Mark Brown)
- re-apply cached runtime state after every reset, including fault recovery (Mark Brown)
- honour explicit snd_soc_dai_set_tdm_slot(); restore TX slot offsets after reset
- wait for the async firmware load on first stream start
- make mute_stream direction-aware
sound/soc/codecs/Kconfig | 11 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/tas2557.c | 3067 ++++++++++++++++++++++++++++++++++++ sound/soc/codecs/tas2557.h | 228 +++ 4 files changed, 3308 insertions(+) create mode 100644 sound/soc/codecs/tas2557.c create mode 100644 sound/soc/codecs/tas2557.h
sound/soc/codecs/Kconfig +11 -0
@@ -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
@@ -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 +3067 -0
2,963 lines not on this page · raw message
sound/soc/codecs/tas2557.h new file +228 -0
228 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.55.0
Part of the diff is not shown here. View the raw message.
[email protected]
AI reviewRe: [PATCH v3 2/2] ASoC: codecs: add TAS2557 mono smart amplifier driver
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Unreserved secondary I2C addresses and data race on `client->addr` via single shared regmap. --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unreserved secondary I2C addresses and data race on client->addr via single shared regmap. --
--- Patch [2]: [PATCH v3 2/2] ASoC: codecs: add TAS2557 mono smart amplifier driver --- commit 6ad2a6c10e4ed5ed98d85d212f482c1f0987454d
Author: Gianluca Boiano <[email protected]>
ASoC: codecs: add TAS2557 mono smart amplifier driver
This commit adds support for the TAS2557 mono smart amplifier. It uses a shared regmap architecture to manage up to two physical chips by dynamically retargeting the I2C client address before each transfer.
Quoted 6 lines
sound/soc/codecs/tas2557.c new file +0
-0@@ -331,12 +331,12 @@ static int tas2557_change_book_page(struct tas2557_priv *tas2557, unsigned int chn,
sound/soc/codecs/tas2557.c static int tas2557_change_book_page(struct tas2557_priv *tas2557, · line 341
Quoted 10 lines
sound/soc/codecs/tas2557.c new file +0
-0unsigned char book, unsigned char page) {struct i2c_client *client =to_i2c_client (tas2557->dev);struct tas2557_dev *dv = &tas2557->devs[chn];int ret;if (client->addr != dv->addr) { client->addr = dv->addr; ret =regmap_write (tas2557->regmap,TAS2557_PAGE_REG ,0 );
[Severity: High]
Can this mutation of client->addr cause a data race with concurrent regmap accesses?
If userspace accesses regmap debugfs concurrently, it acquires the regmap's internal mutex to perform I2C transfers. Because client->addr is modified here under dev_lock but before acquiring the regmap lock, could this result in debugfs I2C transactions inadvertently hitting the wrong physical device?
[ ... ]
Quoted 14 lines
sound/soc/codecs/tas2557.c new file +0
-0@@ -2875,13 +2875,13 @@ static int tas2557_parse_dt(struct tas2557_priv *tas2557, struct i2c_client *client) {struct device *dev = tas2557->dev;struct device_node *np = dev->of_node;unsigned int imon_slots[TAS2557_MAX_DEV ] = {0 ,0 };unsigned int vmon_slots[TAS2557_MAX_DEV ] = {2 ,2 };unsigned int i;int ndev =0 ;if (np) {u64 addr;for (i =0 ; i <TAS2557_MAX_DEV ; i++) {if (of_property_read_reg (np, i, &addr,NULL ))
sound/soc/codecs/tas2557.c
Quoted 4 lines
sound/soc/codecs/tas2557.c new file +0
-0break ; tas2557->devs[ndev++].addr = addr; } }
[Severity: High]
Are these secondary I2C addresses left unreserved with the I2C subsystem?
Since there is no call to i2c_new_dummy_device() or a similar API to claim the additional addresses parsed from the reg array, is it possible that another driver or a userspace process could accidentally bind to them and cause bus collisions?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Re: [PATCH v3 2/2] ASoC: codecs: add TAS2557 mono smart amplifier driver
In reply to Gianluca Boiano…
Quoted 2 lines
+++
b/sound/soc/codecs/tas2557.c(quoted diff) +0
-0@@ -0,0 +1,3067 @@
…
Quoted 3 lines
sound/soc/codecs/tas2557.c new file +3
-0+static int tas2557_dev_read (struct tas2557_priv *tas2557,unsigned int chn, +unsigned int reg,unsigned int *value) +{
…
Quoted 3 lines
sound/soc/codecs/tas2557.c new file +3
-0+mutex_lock (&tas2557->dev_lock); + ret =tas2557_change_book_page (tas2557, chn,TAS2557_BOOK_ID (reg), +TAS2557_PAGE_ID (reg));
…
sound/soc/codecs/tas2557.c line 405
Quoted 3 lines
sound/soc/codecs/tas2557.c new file +3
-0+mutex_unlock (&tas2557->dev_lock); +return ret; +}
…
Under which circumstances would you become interested to apply a statement like “guard(mutex)(&tas2557->dev_lock);”?
https://elixir.bootlin.com/linux/v7.2-rc3/source/include/linux/mutex.h#L253
Regards,
Markus
Mark Brown
maintainerRe: [PATCH v3 2/2] ASoC: codecs: add TAS2557 mono smart amplifier driver
In reply to Gianluca Boianosound/soc/codecs/tas2557.c line 244
Quoted 12 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 +8
-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 */
This looks awfully board specific....
sound/soc/codecs/tas2557.c line 456
Quoted 8 lines
sound/soc/codecs/tas2557.c new file +8
-0+static int tas2557_dev_bulk_write (struct tas2557_priv *tas2557,unsigned int chn, +unsigned int reg,const u8 *data,size_t len) +{ +int ret; + +/* The chip auto-increments past reg 0x7f into the page register */ +if (TAS2557_PAGE_REG_ADDR (reg) + len >128 ) +return -EINVAL ;
I didn't check but is there room for sharing code with the other multi-book TI devices?
sound/soc/codecs/tas2557.c line 691
Quoted 13 lines
sound/soc/codecs/tas2557.c new file +12
-0+static int fw_parse_header (struct tas2557_priv *tas2557, +struct tas2557_firmware *fw, +const u8 *data,size_t size) +{ + fw->device =get_unaligned_be32 (data); data +=4 ; +if (fw->device !=TAS2557_FW_DEVICE_MONO && + fw->device !=TAS2557_FW_DEVICE_STEREO ) { +dev_err (tas2557->dev, +"unsupported device %u in firmware (expected 2=mono or 3=stereo)\n" , + fw->device); +return -EINVAL ; + }
The tas2781 checks to see if there are one or two devices being controlled by the driver and rejects a mismatch. Also, is there some overlap in firmware formats here? Again I didn't actually check.
sound/soc/codecs/tas2557.c line 1410
Quoted 9 lines
sound/soc/codecs/tas2557.c new file +8
-0+static int tas2557_set_program (struct tas2557_priv *tas2557, +unsigned int prog_idx,int config_idx) +/* Load PLL configuration for this config (firmware-driven) */ + if (config->pll < fw->num_plls) { + ret =tas2557_load_pll (tas2557, config->pll); +if (ret <0 ) +return ret; + }
Shouldn't an out of range value be an error?
Quoted 4 lines
sound/soc/codecs/tas2557.c new file +3
-0+static int tas2557_enable (struct tas2557_priv *tas2557,bool enable) +{ + } else if (!enable && tas2557->powered) {
sound/soc/codecs/tas2557.c line 2107
Quoted 10 lines
sound/soc/codecs/tas2557.c new file +10
-0+ ret =tas2557_load_data_all (tas2557, tas2557_shutdown_data); +if (ret <0 ) +dev_err (tas2557->dev,"shutdown failed: %d\n" , ret); + + tas2557->powered = false; + tas2557->dac_muted = true; + tas2557->sense_muted = true; + +dev_dbg (tas2557->dev,"amplifier group powered off\n" ); + }
Does sense_muted get handled properly for suspend? The core mutes and unmutes playback streams over suspend but not capture.
Review
Quoted 7 lines
(quoted diff) +6
-0+static int tas2557_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) + /* Commit cached stream state now the hardware writes have succeeded */ + tas2557->sample_rate = rate; + tas2557->wordlength = wordlength;
This won't actually take effect if the device doesn't power down - you should probably not set use_pmdown_time to force a power down/up, though that's annoying when the configuration is the same.