ASoC: codecs: nau8310: Add NAU83G10 amplifier
Subsystems (3)
cover ASoC: codecs: nau8310: Add NAU83G10 amplifier
This patch series adds support for the Nuvoton NAU83G10 audio amplifier.
The NAU83G10 is a mono boost Class-D audio amplifier with an integrated DSP and I/V-sense support. The driver supports I2C register access through regmap.
DAPM widgets and routes, TDM interface configuration, and DSP firmware loading.
The series is structured asd follows:
- Patch 1: Adds the YAML device tree bindings documentation.
- Patch 2: Adds the ASoC audio amplifier driver implementation.
Neo Chang (2):
ASoC: dt-bindings: nuvoton,nau8310: Add NAU83G10
ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
.../bindings/sound/nuvoton,nau8310.yaml | 200 ++ sound/soc/codecs/Kconfig | 13 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/nau8310-dsp.c | 690 ++++++ sound/soc/codecs/nau8310-dsp.h | 111 + sound/soc/codecs/nau8310.c | 1845 +++++++++++++++++ sound/soc/codecs/nau8310.h | 629 ++++++ 7 files changed, 3490 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml create mode 100644 sound/soc/codecs/nau8310-dsp.c create mode 100644 sound/soc/codecs/nau8310-dsp.h create mode 100644 sound/soc/codecs/nau8310.c create mode 100644 sound/soc/codecs/nau8310.h
Signature · 1 line
2.25.1
PATCH 1/2 ASoC: dt-bindings: nuvoton,nau8310: Add NAU83G10
Add device tree bindings documentation for the Nuvoton NAU83G10 audio amplifier.
--- .../bindings/sound/nuvoton,nau8310.yaml | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml new file +200 -0
@@ -0,0 +1,200 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/nuvoton,nau8310.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton NAU83G10/20 Mono Class-D Amplifier with DSP + +description: + Mono Class-D Amplifier with DSP and I/V-sense. + This device supports I2C. + +maintainers: + - Neo Chang <[email protected]> + +properties: + compatible: + enum: + - nuvoton,nau8310 + - nuvoton,nau8320 + + reg: + description: The I2C address is determined by the hardware pins. + enum: [0x10, 0x11, 0x40, 0x41] + + "#sound-dai-cells": + const: 0 + + clocks: + maxItems: 1 + + clock-names: + const: mclk + + firmware-name: + items: + - description: + DSP core firmware. + + nuvoton,vref-impedance: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + VREF Impedance Selection. + enum: + - 0 # Open + - 1 # 25 kOhm + - 2 # 125 kOhm + - 3 # 2.5 kOhm + default: 0 + + nuvoton,dac-vref: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + DAC Reference Voltage Setting. + enum: + - 0 # Voltage source directly from VDDA pin + - 1 # VDDA * 1.5/1.8V + - 2 # VDDA * 1.6/1.8V + - 3 # VDDA * 1.7/1.8V + default: 1 + + nuvoton,sar-voltage: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + SAR Gain Tracking Selection. + enum: + - 0 # VDDA + - 1 # VDDA * 1.0 + - 2 # VDDA * 1.1 + - 3 # VDDA * 1.2 + - 4 # VDDA * 1.3 + - 5 # VDDA * 1.4 + - 6 # VDDA * 1.54 + - 7 # VDDA * 3.12 + default: 0 + + nuvoton,sar-compare-time: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + SAR compare time. + enum: + - 0 # 500 ns + - 1 # 1 us + - 2 # 2 us + - 3 # 4 us + default: 1 + + nuvoton,sar-sampling-time: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + SAR sampling time. + enum: + - 0 # 2 us + - 1 # 4 us + - 2 # 8 us + - 3 # 16 us + default: 1 + + nuvoton,clock-det-disable: + type: boolean + description: + Enable Clock Detection Module. + + nuvoton,clock-det-data: + type: boolean + description: + Clock detection non-zero samples configuration for power up signal. + + nuvoton,boost-delay: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Control Hold Timer in decrement (msec). + enum: + - 0 # 0 msec + - 1 # 1 msec + - 2 # 2 msec + - 3 # 4 msec + - 4 # 8 msec + - 5 # 16 msec + - 6 # 32 msec + - 7 # 64 msec + - 8 # 128 msec + default: 0 + + nuvoton,boost-convert-enable: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Convert Enable. + enum: + - 0 # BOOST CONVERTER disabled + - 1 # BOOST CONVERTER enable gated by POWERUP signal + - 2 # BOOST CONVERTER disabled + - 3 # BOOST CONVERTER enabled + default: 0 + + nuvoton,boost-target-limit: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Control Target Value Limit. + The value ranges from 0 to 63, representing 0V to 12V with a 0.19V step. + minimum: 0 + maximum: 63 + default: 63 + + nuvoton,boost-target-margin: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Margin Value between Boost Target Voltage and peak output level. + The value ranges from 0 to 63, representing 0V to 12V with a 0.19V step. + minimum: 0 + maximum: 63 + default: 0 + + nuvoton,temp-compensation: + type: boolean + description: + Enable Temperature Compensation. + + nuvoton,normal-iis-data: + type: boolean + description: + Enable Normal I2S Audio Data mode. The I2S interface + transmits only audio data without SAR ADC data. + +required: + - compatible + - reg + - firmware-name + +allOf: + - $ref: dai-common.yaml# + +unevaluatedProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + codec@10 { + compatible = "nuvoton,nau8310"; + reg = <0x10>; + #sound-dai-cells = <0>; + firmware-name = "NAU83G10.kcs.bin"; + nuvoton,vref-impedance = <2>; + nuvoton,dac-vref = <1>; + nuvoton,sar-voltage = <0>; + nuvoton,sar-compare-time = <1>; + nuvoton,sar-sampling-time = <1>; + nuvoton,clock-det-disable; + nuvoton,clock-det-data; + nuvoton,boost-delay = <0x8>; + nuvoton,boost-convert-enable = <1>; + nuvoton,boost-target-limit = <0x32>; + nuvoton,boost-target-margin = <0x2>; + nuvoton,temp-compensation; + nuvoton,normal-iis-data; + }; + };
Signature · 1 line
2.25.1
[email protected]
AI reviewRe: [PATCH v1 1/2] ASoC: dt-bindings: nuvoton,nau8310: Add NAU83G10
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] ASoC: dt-bindings: nuvoton,nau8310: inverted clock-det-disable description - [Medium] ASoC…
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] ASoC: dt-bindings: nuvoton,nau8310: inverted clock-det-disable description
- [Medium] ASoC: dt-bindings: nuvoton,nau8310: raw register values instead of units
- [Medium] ASoC: dt-bindings: nuvoton,nau8310: unconstrained nau8320 variant properties --
commit 01b8fdb80988a85278fec4d71b7b4a47411ed6ee
Author: Neo Chang <[email protected]>
ASoC: dt-bindings: nuvoton,nau8310: Add NAU83G10
This commit adds the device tree binding schema in Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml for the Nuvoton mono class-d audio amplifier family.
Quoted 16 lines
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml new file +10
-0@@ -0,0 +1,200 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/nuvoton,nau8310.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Nuvoton NAU83G10/20 Mono Class-D Amplifier with DSP + +description: + Mono Class-D Amplifier with DSP and I/V-sense.
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml line 20
Quoted 10 lines
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml new file +10
-0+ This device supports I2C. + +maintainers: + - Neo Chang <[email protected]> + +properties: + compatible: + enum: + - nuvoton,nau8310 + - nuvoton,nau8320
[Severity: Medium]
Should Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml restrict the boost converter properties when compatible is set to nuvoton,nau8320?
In sound/soc/codecs/nau8310.c, nau8310_read_device_properties() only reads nuvoton,boost-delay, nuvoton,boost-convert-enable, nuvoton,boost-target-limit, and nuvoton,boost-target-margin for nuvoton,nau8310 and ignores them on nuvoton,nau8320:
if (nau8310->silicon_id == NAU8310_REG_SI_REV_G10) {
ret = device_property_read_u32(dev, "nuvoton,boost-delay",
&nau8310->boost_delay);
...
}Without an allOf if/then block setting those four properties to false for nuvoton,nau8320, devicetree nodes for nuvoton,nau8320 with boost converter properties will pass schema validation even though the driver ignores them. Should the commit message also mention nuvoton,nau8320 alongside nuvoton,nau8310?
[ ... ]
Quoted 53 lines
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml new file +53
-0+ nuvoton,vref-impedance: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + VREF Impedance Selection. + enum: + - 0 # Open + - 1 # 25 kOhm + - 2 # 125 kOhm + - 3 # 2.5 kOhm + default: 0 + + nuvoton,dac-vref: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + DAC Reference Voltage Setting. + enum: + - 0 # Voltage source directly from VDDA pin + - 1 # VDDA * 1.5/1.8V + - 2 # VDDA * 1.6/1.8V + - 3 # VDDA * 1.7/1.8V + default: 1 + + nuvoton,sar-voltage: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + SAR Gain Tracking Selection. + enum: + - 0 # VDDA + - 1 # VDDA * 1.0 + - 2 # VDDA * 1.1 + - 3 # VDDA * 1.2 + - 4 # VDDA * 1.3 + - 5 # VDDA * 1.4 + - 6 # VDDA * 1.54 + - 7 # VDDA * 3.12 + default: 0 + + nuvoton,sar-compare-time: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + SAR compare time. + enum: + - 0 # 500 ns + - 1 # 1 us + - 2 # 2 us + - 3 # 4 us + default: 1 + + nuvoton,sar-sampling-time: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + SAR sampling time. + enum:
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml line 102
Quoted 10 lines
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml new file +10
-0+ - 0 # 2 us + - 1 # 4 us + - 2 # 8 us + - 3 # 16 us + default: 1 + + nuvoton,clock-det-disable: + type: boolean + description: + Enable Clock Detection Module.
[Severity: Medium]
Is the description for nuvoton,clock-det-disable in Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml inverted relative to the property name and driver behavior?
In sound/soc/codecs/nau8310.c, nau8310_read_device_properties() and nau8310_init_regs() disable clock detection when nuvoton,clock-det-disable is present in the devicetree node:
nau8310->clock_detection =
!device_property_read_bool(dev, "nuvoton,clock-det-disable");
if (!nau8310->clock_detection)
regmap_update_bits(nau8310->regmap, NAU8310_REG_CLK_DET,
NAU8310_CLKPWRUP_DIS, NAU8310_CLKPWRUP_DIS);Also, Documentation/devicetree/bindings/sound/nuvoton,nau8325.yaml already defines nuvoton,clock-detection-disable for this feature. Could this binding reuse nuvoton,clock-detection-disable and state that it disables the clock detection module?
Quoted 40 lines
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml new file +40
-0+ + nuvoton,clock-det-data: + type: boolean + description: + Clock detection non-zero samples configuration for power up signal. + + nuvoton,boost-delay: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Control Hold Timer in decrement (msec). + enum: + - 0 # 0 msec + - 1 # 1 msec + - 2 # 2 msec + - 3 # 4 msec + - 4 # 8 msec + - 5 # 16 msec + - 6 # 32 msec + - 7 # 64 msec + - 8 # 128 msec + default: 0 + + nuvoton,boost-convert-enable: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Convert Enable. + enum: + - 0 # BOOST CONVERTER disabled + - 1 # BOOST CONVERTER enable gated by POWERUP signal + - 2 # BOOST CONVERTER disabled + - 3 # BOOST CONVERTER enabled + default: 0 + + nuvoton,boost-target-limit: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Control Target Value Limit. + The value ranges from 0 to 63, representing 0V to 12V with a 0.19V step. + minimum: 0 + maximum: 63
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml line 152
Quoted 10 lines
Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml new file +10
-0+ default: 63 + + nuvoton,boost-target-margin: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + Boost Margin Value between Boost Target Voltage and peak output level. + The value ranges from 0 to 63, representing 0V to 12V with a 0.19V step. + minimum: 0 + maximum: 63 + default: 0
[Severity: Medium]
Could these vendor properties in Documentation/devicetree/bindings/sound/nuvoton,nau8310.yaml use standard unit suffixes and physical values instead of raw register encodings?
Specifically:
- nuvoton,vref-impedance and nuvoton,dac-vref use raw register indices 0..3, whereas Documentation/devicetree/bindings/sound/nuvoton,nau8325.yaml already defines nuvoton,vref-impedance-ohms and nuvoton,dac-vref-microvolt.
- nuvoton,sar-compare-time, nuvoton,sar-sampling-time, nuvoton,boost-delay, nuvoton,boost-target-limit, and nuvoton,boost-target-margin expose raw register indices or step counts (0..3, 0..8, 0..63) rather than physical values with -ns, -us, -ms, and -microvolt suffixes.
- The schema default values for nuvoton,vref-impedance (0), nuvoton,boost-delay (0), nuvoton,boost-target-limit (63), and nuvoton,boost-target-margin (0) do not match the defaults applied in nau8310_read_device_properties() (2, 0x8, 0x32, and 0x2).
- nuvoton,boost-convert-enable has a boolean-style -enable suffix for a uint32 register field where both 0 and 2 mean disabled, and nuvoton,sar-voltage exposes duplicate register values 0 and 1 for the same 1.0 gain setting.
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH 2/2 ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
In reply to Neo ChangAdd support for the Nuvoton NAU83G10 audio amplifier. The NAU83G10 is a mono boost Class-D audio amplifier with integrated DSP and I/V-sense support.
--- sound/soc/codecs/Kconfig | 13 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/nau8310-dsp.c | 690 ++++++++++++ sound/soc/codecs/nau8310-dsp.h | 111 ++ sound/soc/codecs/nau8310.c | 1845 ++++++++++++++++++++++++++++++++ sound/soc/codecs/nau8310.h | 629 +++++++++++ 6 files changed, 3290 insertions(+) create mode 100644 sound/soc/codecs/nau8310-dsp.c create mode 100644 sound/soc/codecs/nau8310-dsp.h create mode 100644 sound/soc/codecs/nau8310.c create mode 100644 sound/soc/codecs/nau8310.h
sound/soc/codecs/Kconfig +13 -0
@@ -174,6 +174,7 @@ config SND_SOC_ALL_CODECS imply SND_SOC_MT6660 imply SND_SOC_NAU8315 imply SND_SOC_NAU8325 + imply SND_SOC_NAU8310 imply SND_SOC_NAU8360 imply SND_SOC_NAU8540 imply SND_SOC_NAU8810
@@ -2940,6 +2941,18 @@ config SND_SOC_NAU8825 tristate depends on I2C +config SND_SOC_NAU8310 + tristate "Nuvoton Technology Corporation NAU83G10 Mono Boosted Amplifier" + depends on I2C + select FW_LOADER + help + The NAU83G10 is a mono boosted Class-D amplifier with an integrated + DSP and I/V sense. The DSP is programmed with a speaker excursion + control algorithm that uses real-time voltage and current feedback + to prevent the speaker membrane from exceeding its rated limit. + It has a high-efficiency Class-H boost converter to provide + ample headroom for overall loudness and sound quality. + config SND_SOC_NAU8360 tristate "Nuvoton Technology Corporation NAU83G60 Stereo Smart Amplifier" depends on I2C
sound/soc/codecs/Makefile +2 -0
@@ -199,6 +199,7 @@ snd-soc-mt6359-accdet-y := mt6359-accdet.o snd-soc-mt6660-y := mt6660.o snd-soc-nau8315-y := nau8315.o snd-soc-nau8325-y := nau8325.o +snd-soc-nau8310-y := nau8310.o nau8310-dsp.o snd-soc-nau8360-y := nau8360.o nau8360-dsp.o snd-soc-nau8540-y := nau8540.o snd-soc-nau8810-y := nau8810.o
@@ -642,6 +643,7 @@ obj-$(CONFIG_SND_SOC_MT6359_ACCDET) += mt6359-accdet.o obj-$(CONFIG_SND_SOC_MT6660) += snd-soc-mt6660.o obj-$(CONFIG_SND_SOC_NAU8315) += snd-soc-nau8315.o obj-$(CONFIG_SND_SOC_NAU8325) += snd-soc-nau8325.o +obj-$(CONFIG_SND_SOC_NAU8310) += snd-soc-nau8310.o obj-$(CONFIG_SND_SOC_NAU8360) += snd-soc-nau8360.o obj-$(CONFIG_SND_SOC_NAU8540) += snd-soc-nau8540.o obj-$(CONFIG_SND_SOC_NAU8810) += snd-soc-nau8810.o
sound/soc/codecs/nau8310-dsp.c new file +690 -0
690 lines not on this page · raw message
sound/soc/codecs/nau8310-dsp.h new file +111 -0
111 lines not on this page · raw message
sound/soc/codecs/nau8310.c new file +1845 -0
1,845 lines not on this page · raw message
sound/soc/codecs/nau8310.h new file +629 -0
315 lines not on this page · raw message
The rest of this diff is not shown. View the raw message.
Signature · 1 line
2.25.1
Part of the diff is not shown here. View the raw message.
[email protected]
AI reviewRe: [PATCH v1 2/2] ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
Thank you for your contribution! Sashiko AI review found 15 potential issue(s) to consider: - [High] ASoC: nau8310: type confusion and crash in component kcontrol callbacks - [High] ASoC: nau8310…
Thank you for your contribution! Sashiko AI review found 15 potential issue(s) to consider:
- [High] ASoC: nau8310: type confusion and crash in component kcontrol callbacks
- [High] ASoC: nau8310: nau8310_dsp_init() desynchronizes DAPM switch state
- [High] ASoC: nau8310: missing cancel_work_sync() on probe error and component remove
- [High] ASoC: nau8310: lock inversion and bias_level TOCTOU in nau8310_dsp_snd_soc_dapm_put()
- [Medium] ASoC: nau8310: nau8310_dsp_set_kcs_setup() ignores nau8310->dsp_firmware
- [Medium] ASoC: nau8310: broken clock config for 64kHz/96kHz and 1x/2x multipliers
- [Medium] ASoC: nau8310: uninitialized *n1_sel read in nau8310_clksrc_choose()
- [Medium] ASoC: nau8310: nau8310_shutdown() clobbers active stream and mclk state
- [Medium] ASoC: nau8310: nau8310_dac_oversampl_enum exposes reserved register value
- [Medium] ASoC: nau8310: volatile registers in nau8310_reg_defaults synced on resume
- [Medium] ASoC: nau8310: nau8310_set_tdm_slot() bitwise-ORs encoded TX slot values
- [Medium] ASoC: nau8310: nau8310_reply_from_dsp() leaves reply buffer uninitialized
- [Medium] ASoC: nau8310: race between load_fw_work and DAPM clock events
- [Low] ASoC: nau8310: broken snd_soc_dapm_del_routes() error cleanup
- [Low] ASoC: nau8310: uninitialized kcs_setup_comp fields in nau8310_dsp_set_kcs_setup() --
commit 781c4e28ad8239f56461795282d48ebf131f9a1b
Author: Neo Chang <[email protected]>
ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
This patch adds an ASoC codec driver for the Nuvoton NAU83G10 mono boost Class-D audio amplifier with integrated dsp and I/V-sense support.
Quoted 5 lines
sound/soc/codecs/nau8310-dsp.c new file +0
-0
[ ... ]
Quoted 5 lines
sound/soc/codecs/nau8310-dsp.c new file +5
-0+static int nau8310_reply_from_dsp (struct snd_soc_component *cp, +const struct nau8310_cmd_info *cmd_info,int data_size, +void *data) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp);
sound/soc/codecs/nau8310-dsp.c line 353
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+struct device *dev = cp->dev; +int ret, frag_len, pad_len_exp, data_count =0 ; + + ret =nau8310_dsp_replied (nau8310, &frag_len); +if (ret) +return ret; +if (!frag_len || !cmd_info->reply_data) +return 0 ; +if (!data || frag_len ==1 ) +return -EINVAL ;
[Severity: Medium]
In sound/soc/codecs/nau8310-dsp.c, if nau8310_dsp_replied() returns a reply preamble with frag_len == 0 for a command where cmd_info->reply_data is true, nau8310_reply_from_dsp() returns 0 without writing anything to the data buffer.
In nau8310_dsp_kcs_setup(), kcs_rst is declared uninitialized on the stack and passed to nau8310_dsp_exec_command() for NAU8310_DSP_CMD_GET_KCS_RSLTS:
unsigned int kcs_rst; ... ret = nau8310_dsp_exec_command(cp, NAU8310_DSP_CMD_GET_KCS_RSLTS, 0, NAU8310_DSP_DATA_BYTE, NULL, NAU8310_DSP_DATA_BYTE, &kcs_rst); if (ret) return ret; if (kcs_rst != NAU8310_DSP_KCS_RSLTS_SUCCESS) return -EINVAL;
Could a zero-length reply cause nau8310_dsp_kcs_setup() to evaluate an uninitialized kcs_rst value? Should nau8310_reply_from_dsp() return an error when cmd_info->reply_data is set but frag_len == 0?
[ ... ]
Quoted 27 lines
sound/soc/codecs/nau8310-dsp.c new file +27
-0+static int nau8310_dsp_snd_soc_dapm_put (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct snd_soc_dapm_context *dapm =snd_soc_dapm_kcontrol_to_dapm (kcontrol); +struct snd_soc_component *component =snd_soc_dapm_to_component (dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component); +struct soc_mixer_control *mc = + (struct soc_mixer_control *)kcontrol->private_value; +unsigned int mask = (1 <<fls (mc->max)) -1 ; +unsigned int val; +int ret; + +if (snd_soc_dapm_get_bias_level (dapm) >SND_SOC_BIAS_STANDBY ) { +dev_warn_ratelimited (nau8310->dev,"changing path is not allowed during playback" ); +return -EBUSY ; + } + + val = (ucontrol->value.integer.value[0 ] & mask); + +mutex_lock (&nau8310->lock); +if (val ==NAU8310_DAC_SRC_DSP && !nau8310->load_fw_done) { +dev_warn_ratelimited (nau8310->dev,"Cannot enable DSP: Firmware not ready or disabled" ); +mutex_unlock (&nau8310->lock); +return -EBUSY ; + } + + ret =snd_soc_dapm_put_volsw (kcontrol, ucontrol);
sound/soc/codecs/nau8310-dsp.c line 535
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+ +if (val ==NAU8310_DAC_SRC_BYPASS ) { +/* Switch the clock source of DSP to MCLK. */ +regmap_update_bits (nau8310->regmap,NAU8310_R04_ENA_CTRL , +NAU8310_DSP_SEL_OSC ,0 ); + } +mutex_unlock (&nau8310->lock); + +return ret; +}
[Severity: High]
Can nau8310_dsp_snd_soc_dapm_put() in sound/soc/codecs/nau8310-dsp.c cause a lock inversion with nau8310_startup() and nau8310_load_fw_work()?
Here, nau8310_dsp_snd_soc_dapm_put() acquires nau8310->lock and then calls snd_soc_dapm_put_volsw(), which acquires card->dapm_mutex and card->pcm_mutex (via snd_soc_dpcm_runtime_update()).
However, when opening a pcm stream, soc_pcm_open() holds card->pcm_mutex and calls nau8310_startup(), which calls flush_work(&nau8310->load_fw_work) while nau8310_load_fw_work() acquires nau8310->lock:
Thread 1 (control write):
nau8310_dsp_snd_soc_dapm_put()
mutex_lock(&nau8310->lock)
snd_soc_dapm_put_volsw()
snd_soc_dpcm_runtime_update()
snd_soc_dpcm_mutex_lock(card) -> locks card->pcm_mutexThread 2 (pcm open):
soc_pcm_open() -> locks card->pcm_mutex
__soc_pcm_open()
snd_soc_dai_startup()
nau8310_startup()
flush_work(&nau8310->load_fw_work)
waits for nau8310_load_fw_work() -> mutex_lock(&nau8310->lock)Also, snd_soc_dapm_get_bias_level(dapm) is checked before acquiring nau8310->lock or card->dapm_mutex, so a stream can start while waiting for nau8310->lock and bypass the playback check. And when val == NAU8310_DAC_SRC_BYPASS, clearing NAU8310_DSP_SEL_OSC does not check if load_fw_work is still pending, which can switch the clock away from the internal oscillator before nau8310_load_fw_work() runs.
Quoted 26 lines
sound/soc/codecs/nau8310-dsp.c new file +26
-0+ +#define NAU8310_DSP_SOC_DAPM_SINGLE (xname, reg, shift, mask, invert) \ + SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \ + snd_soc_dapm_get_volsw, nau8310_dsp_snd_soc_dapm_put) + +static const struct snd_kcontrol_new nau8310_dacdat_select_dsp = +NAU8310_DSP_SOC_DAPM_SINGLE ("Switch" ,NAU8310_R1A_DSP_CORE_CTRL2 , +NAU8310_DAC_SEL_DSP_SFT ,1 ,0 ); + +static int nau8310_dsp_clock_event (struct snd_soc_dapm_widget *w, +struct snd_kcontrol *kcontrol,int event) +{ +struct snd_soc_component *cp =snd_soc_dapm_to_component (w->dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +int ret =0 ; + +if (SND_SOC_DAPM_EVENT_ON (event)) { + ret =nau8310_dsp_exec_command (cp,NAU8310_DSP_CMD_CLK_RESTART , +0 ,0 ,NULL ,0 ,NULL ); +if (ret) +dev_err (cp->dev,"Send DSP command %s fail (%d)" , + dsp_cmd_table[NAU8310_DSP_CMD_CLK_RESTART ], ret); + +/* Switch the clock source of DSP to MCLK. */ +regmap_update_bits (nau8310->regmap,NAU8310_R04_ENA_CTRL , +NAU8310_DSP_SEL_OSC ,0 );
sound/soc/codecs/nau8310-dsp.c line 571
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+ }else if (SND_SOC_DAPM_EVENT_OFF (event)) { + ret =nau8310_dsp_exec_command (cp,NAU8310_DSP_CMD_CLK_STOP , +0 ,0 ,NULL ,0 ,NULL ); +if (ret) +dev_err (cp->dev,"Send DSP command %s fail (%d)" , + dsp_cmd_table[NAU8310_DSP_CMD_CLK_STOP ], ret); + } + +return ret; +}
[Severity: Medium]
Can nau8310_dsp_clock_event() in sound/soc/codecs/nau8310-dsp.c race with nau8310_load_fw_work() during system resume?
In nau8310_resume(), nau8310_dsp_fw_load() switches the clock source to the internal oscillator (NAU8310_DSP_SEL_OSC) and schedules nau8310->load_fw_work asynchronously. Immediately afterwards, soc_resume_deferred() powers up dapm widgets and invokes nau8310_dsp_clock_event() while nau8310_load_fw_work() is still in nau8310_dsp_init() (which sleeps in msleep(120) and then transfers firmware over NAU8310_RF000_DSP_COMM under nau8310->lock).
Because nau8310_dsp_clock_event() and nau8310_clock_event() do not synchronize with load_fw_work or acquire nau8310->lock, won't they interleave commands on NAU8310_RF000_DSP_COMM and clear NAU8310_DSP_SEL_OSC while firmware download is still in progress?
[ ... ]
Quoted 6 lines
sound/soc/codecs/nau8310-dsp.c new file +6
-0+static int nau8310_dsp_set_kcs_setup (struct snd_soc_component *cp) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +struct regmap *regmap = nau8310->regmap; +struct nau8310_kcs_setup kcs_setup_comp, *kcs_setup = &kcs_setup_comp; +struct device *dev = cp->dev;
sound/soc/codecs/nau8310-dsp.c line 603
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+const struct firmware *fw; +int ret, status =0 , buf_off, buf_len; + +dev_dbg (dev,"Send DSP command %s" , + dsp_cmd_table[NAU8310_DSP_CMD_GET_FRAME_STATUS ]); + + kcs_setup->get_len = kcs_setup->set_len =sizeof (status); + kcs_setup->get_data = (void *)&status; + + ret =nau8310_send_dsp_command (cp,NAU8310_DSP_CMD_GET_FRAME_STATUS , kcs_setup);
[Severity: Low]
In nau8310_dsp_set_kcs_setup() in sound/soc/codecs/nau8310-dsp.c, kcs_setup_comp is declared on the stack without zero-initializing set_kcs_offset and set_kcs_data, which are then read in nau8310_send_dsp_command() and passed to nau8310_message_to_dsp(). Should kcs_setup_comp be zero-initialized (or should this call nau8310_dsp_exec_command())?
Quoted 6 lines
sound/soc/codecs/nau8310-dsp.c new file +6
-0+if (ret) { +dev_err (dev,"Send DSP command %s fail (%d)" , + dsp_cmd_table[NAU8310_DSP_CMD_GET_FRAME_STATUS ], ret); + goto err; + } +
sound/soc/codecs/nau8310-dsp.c line 619
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+if (!(status &NAU8310_DSP_ALGO_OK )) { +dev_err (dev,"Algorithm of DSP is not ready, status %x" , status); + ret = -EIO ; + goto err; + } + +dev_info (dev,"Algorithm of DSP is ready, status %x" , status); + +dev_dbg (dev,"Request firmware and wait until finish." ); + ret =request_firmware (&fw,NAU8310_DSP_FIRMWARE , dev);
[Severity: Medium]
In nau8310_read_device_properties() in sound/soc/codecs/nau8310.c, the driver reads the "firmware-name" device property and allocates nau8310->dsp_firmware. However, nau8310_dsp_set_kcs_setup() passes the hardcoded NAU8310_DSP_FIRMWARE macro to request_firmware() here. Should this pass nau8310->dsp_firmware instead?
Quoted 4 lines
sound/soc/codecs/nau8310-dsp.c new file +4
-0+if (ret) { +dev_err (dev,"Failed to load firmware (%d)" , ret); + goto err; + }
[ ... ]
Quoted 4 lines
sound/soc/codecs/nau8310-dsp.c new file +4
-0+int nau8310_dsp_init (struct snd_soc_component *cp) +{ +struct regmap *regmap = cp->regmap; +
sound/soc/codecs/nau8310-dsp.c line 663
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+nau8310_sw_reset_chip (regmap); + +/* wait for the power ready */ +msleep (120 ); + +regmap_update_bits (regmap,NAU8310_R1A_DSP_CORE_CTRL2 , +NAU8310_DSP_RUNSTALL |NAU8310_DAC_SEL_DSP_OUT ,NAU8310_DAC_SEL_DSP_OUT ); + +return nau8310_dsp_set_kcs_setup (cp); +}
[Severity: High]
Does setting NAU8310_DAC_SEL_DSP_OUT directly via regmap_update_bits() in nau8310_dsp_init() desynchronize the "DAC data from DSP" dapm switch state?
In nau8310_codec_probe(), nau8310_dsp_fw_load() schedules load_fw_work and nau8310_dsp_setup_controls() immediately registers the "DAC data from DSP" switch widget (which is backed by NAU8310_R1A_DSP_CORE_CTRL2 bit NAU8310_DAC_SEL_DSP_SFT) while load_fw_work is sleeping in msleep(120).
When snd_soc_dapm_add_routes() runs, NAU8310_DAC_SEL_DSP_OUT is still 0, so dapm initializes the switch path as disconnected. When nau8310_dsp_init() subsequently sets NAU8310_DAC_SEL_DSP_OUT in the register without going through dapm, nau8310_dsp_active() becomes true and the hardware mux selects the dsp output, but dapm still treats "DAC data from DSP" as disconnected and leaves the "DSP", "DSP Clock", and "Capture Sense" widgets unpowered during playback.
Quoted 17 lines
sound/soc/codecs/nau8310-dsp.c new file +17
-0+ +int nau8310_dsp_setup_controls (struct snd_soc_component *cp) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +struct snd_soc_dapm_context *dapm = nau8310->dapm; +int ret; + + ret =snd_soc_dapm_new_controls (dapm, nau8310_dsp_dapm_widgets, +ARRAY_SIZE (nau8310_dsp_dapm_widgets)); +if (ret) { +dev_err (cp->dev,"Add DSP widget fail (%d)" , ret); + goto err; + } + + ret =snd_soc_dapm_add_routes (dapm, nau8310_dsp_dapm_routes, +ARRAY_SIZE (nau8310_dsp_dapm_routes)); +if (ret) {
sound/soc/codecs/nau8310-dsp.c line 690
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+dev_err (cp->dev,"Add DSP route fail (%d)" , ret); + goto err; + } + +return 0 ; +err: +snd_soc_dapm_del_routes (dapm, nau8310_dsp_dapm_routes, +ARRAY_SIZE (nau8310_dsp_dapm_routes)); +return ret; +}
[Severity: Low]
In nau8310_dsp_setup_controls() in sound/soc/codecs/nau8310-dsp.c, if snd_soc_dapm_new_controls() fails, jumping to err calls snd_soc_dapm_del_routes() on routes that were never added. In addition, nau8310_dsp_dapm_routes contains a route with a control ("Switch"), which snd_soc_dapm_del_route() does not support removing. Since component probe failure already frees dapm widgets and routes via snd_soc_dapm_free(), can the snd_soc_dapm_del_routes() call in the error path be dropped?
Quoted 5 lines
sound/soc/codecs/nau8310.c new file +0
-0
[ ... ]
Quoted 6 lines
sound/soc/codecs/nau8310.c new file +6
-0+static const struct reg_default nau8310_reg_defaults[] = { + {NAU8310_R02_I2C_ADDR ,0x0000 }, + {NAU8310_R03_CLK_CTRL ,0x0000 }, + {NAU8310_R04_ENA_CTRL ,0x0000 }, + {NAU8310_R05_INTERRUPT_CTRL ,0x007f }, + {NAU8310_R06_INT_CLR_STATUS ,0x0000 },
[ ... ]
sound/soc/codecs/nau8310.c line 222
Quoted 4 lines
sound/soc/codecs/nau8310.c new file +4
-0+ { NAU8310_R9C_BIQ2_COE_9, 0x0000 }, + { NAU8310_R9D_BIQ2_COE_10, 0x0000 }, + { NAU8310_RF000_DSP_COMM, 0x0000 }, +};
[Severity: Medium]NAU8310_R06_INT_CLR_STATUS and NAU8310_RF000_DSP_COMM are marked as both volatile and writeable, and are also included in nau8310_reg_defaults[] in sound/soc/codecs/nau8310.c.
Because regcache_maple_populate() seeds the maple cache from reg_defaults[], when nau8310_suspend() calls regcache_mark_dirty() and nau8310_resume() calls regcache_sync(), regcache will write 0x0000 to NAU8310_R06_INT_CLR_STATUS and 0x00000000 to the 32-bit mailbox register NAU8310_RF000_DSP_COMM. Should volatile registers be omitted from nau8310_reg_defaults[]?
[ ... ]
Quoted 8 lines
sound/soc/codecs/nau8310.c new file +8
-0+static int nau8310_clkdet_put (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct soc_mixer_control *mc = + (struct soc_mixer_control *)kcontrol->private_value; +struct snd_soc_dapm_context *dapm =snd_soc_dapm_kcontrol_to_dapm (kcontrol); +struct snd_soc_component *component =snd_soc_dapm_to_component (dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component);
[ ... ]
Quoted 32 lines
sound/soc/codecs/nau8310.c new file +32
-0+static int nau8310_clkdet_get (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct snd_soc_dapm_context *dapm =snd_soc_dapm_kcontrol_to_dapm (kcontrol); +struct snd_soc_component *component =snd_soc_dapm_to_component (dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component); + + ucontrol->value.integer.value[0 ] = nau8310->clock_detection; + +return 0 ; +} + +static int nau8310_biq_coeff_get (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct snd_soc_dapm_context *dapm =snd_soc_dapm_kcontrol_to_dapm (kcontrol); +struct snd_soc_component *component =snd_soc_dapm_to_component (dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component); +struct soc_bytes_ext *params = (void *)kcontrol->private_value; +int i, reg, reg_val; +u16 *val; + + val = (u16 *)ucontrol->value.bytes.data; + reg =NAU8310_R80_BIQ0_COE_1 ; +for (i =0 ; i < params->max /sizeof (u16); i++) { +regmap_read (nau8310->regmap, reg + i, ®_val); +/* conversion of 16-bit integers between native CPU format +* and big endian format +*/ + reg_val =cpu_to_be16 (reg_val); + *(val + i) = reg_val; + }
sound/soc/codecs/nau8310.c line 355
Quoted 10 lines
sound/soc/codecs/nau8310.c new file +10
-0+ +return 0 ; +} + +static int nau8310_biq_coeff_put (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct snd_soc_dapm_context *dapm =snd_soc_dapm_kcontrol_to_dapm (kcontrol); +struct snd_soc_component *component =snd_soc_dapm_to_component (dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component);
[Severity: High]
Won't calling snd_soc_dapm_kcontrol_to_dapm(kcontrol) in nau8310_clkdet_put(), nau8310_clkdet_get(), nau8310_biq_coeff_get(), and nau8310_biq_coeff_put() crash when userspace accesses the "Clock Detection Switch" or "BIQ Coefficients" controls?
Both controls are registered in nau8310_snd_controls[] via soc_comp_dev_nau8310.controls, so kcontrol->private_data points to struct snd_soc_component, not struct dapm_kcontrol_data. Calling snd_soc_dapm_kcontrol_to_dapm() reinterprets the component pointer as struct dapm_kcontrol_data and dereferences data->wlist->widgets[0]->dapm. Should these four functions use snd_soc_kcontrol_component(kcontrol) instead?
Also, in nau8310_biq_coeff_get(), if regmap_read() fails, reg_val is uninitialized on the stack and gets copied into ucontrol->value.bytes.data. Should the return value of regmap_read() be checked?
[ ... ]
sound/soc/codecs/nau8310.c line 396
Quoted 6 lines
sound/soc/codecs/nau8310.c new file +6
-0+static const char *const nau8310_dac_oversampl[] = { +"64" ,"256" ,"128" ,"" ,"32" }; + +static const struct soc_enum nau8310_dac_oversampl_enum = +SOC_ENUM_SINGLE (NAU8310_R29_DAC_CTRL1 ,NAU8310_DAC_OVERSAMPLE_SFT , +ARRAY_SIZE (nau8310_dac_oversampl), nau8310_dac_oversampl);
[Severity: Medium]
In sound/soc/codecs/nau8310.c, nau8310_dac_oversampl[] includes an empty string "" at index 3 with SOC_ENUM_SINGLE, exposing reserved register value 3 to userspace. If userspace selects index 3, osr_dac_sel[3] has osr == 0 and nau8310_osr_apply() fails with -EINVAL during hw_params. Should this use SOC_VALUE_ENUM_SINGLE to skip the reserved value?
[ ... ]
Quoted 25 lines
sound/soc/codecs/nau8310.c new file +25
-0+static int nau8310_srate_clk_apply (struct nau8310 *nau8310, +const struct nau8310_srate_attr *srate_table, +int n1_sel,int mclk_mult_sel,int n2_sel,int dsp_mult_sel) +{ +struct regmap *regmap = nau8310->regmap; +int dsp_clk, mult_sel; +unsigned int ctrl_val =0 ; + +static const unsigned int mclkx_val[] = { +/* multiplier 4x, i.e. 2^2 */ + [2 ] =NAU8310_MCLK4XEN , +/* multiplier 8x, i.e. 2^3 */ + [3 ] =NAU8310_MCLK8XEN |NAU8310_MCLK4XEN , +/* multiplier 16x, i.e. 2^4 */ + [4 ] =NAU8310_MCLK16XEN |NAU8310_MCLK8XEN |NAU8310_MCLK4XEN , + }; + +if (!srate_table || srate_table->adc_div <0 || + dsp_mult_sel <0 || dsp_mult_sel >=ARRAY_SIZE (dsp_src_mult) || + n2_sel <0 || n2_sel >=ARRAY_SIZE (mclk_n2_div) || + n1_sel <0 || n1_sel >=ARRAY_SIZE (mclk_n1_div)) { +dev_err (nau8310->dev,"The fs %d isn't support in DSP." , + nau8310->fs); +return -EINVAL ; + }
[ ... ]
Quoted 12 lines
sound/soc/codecs/nau8310.c new file +12
-0+regmap_update_bits (regmap,NAU8310_R04_ENA_CTRL , +NAU8310_CLK_DSP_SRC_MASK |NAU8310_CLK_MUL_SRC_MASK | +NAU8310_MCLK_SEL_MASK , ctrl_val); + +if (mult_sel <2 || mult_sel >4 ) +return -EINVAL ; + +regmap_update_bits (regmap,NAU8310_R68_ANALOG_CONTROL_7 , +NAU8310_MCLKX_MASK , mclkx_val[mult_sel]); + +return 0 ; +}
sound/soc/codecs/nau8310.c line 746
Quoted 10 lines
sound/soc/codecs/nau8310.c new file +10
-0+ +static int nau8310_calc_dsp_clk (int mclk,int fs,int mult_param,int n1_param) +{ +int dsp_clk = (mclk << mult_param) / n1_param; + +if (dsp_clk >= (fs *DSP_CLK_MIN_MULTI_VALUE ) && dsp_clk <=DSP_CLK_MAX ) +return dsp_clk; + +return 0 ; +}
[Severity: Medium]
Can 64 kHz or 96 kHz ever succeed in nau8310_clock_config() when the dsp is bypassed, or can 1x/2x clock multipliers ever be applied?
In nau8310_calc_dsp_clk(), dsp_clk must be between fs * 2000 and 102400000. For fs = 64000 (64000 * 2000 = 128000000) and fs = 96000 (96000 * 2000 = 192000000), fs * DSP_CLK_MIN_MULTI_VALUE exceeds DSP_CLK_MAX, so nau8310_clkdsp_choose() always fails even when the dsp is bypassed. In addition, target_srate_table[] sets adc_div = -1 for 96000, which nau8310_srate_clk_apply() rejects unconditionally, and NAU8310_RATES advertises SNDRV_PCM_RATE_8000_192000 including rates (11025, 22050, 88200, 176400, 192000) not present in target_srate_table[].
Furthermore, in nau8310_srate_clk_apply(), mult_sel is an index into mclk_src_mult[] / dsp_src_mult[] where 0 and 1 represent 1x and 2x multipliers, but the check:
if (mult_sel < 2 || mult_sel > 4) return -EINVAL;
rejects mult_sel == 0 and mult_sel == 1 with -EINVAL after already updating NAU8310_R03_CLK_CTRL, NAU8310_R40_CLK_DET_CTRL, and NAU8310_R04_ENA_CTRL. Should mclkx_val[] allow 0 for 1x and 2x multipliers instead of returning -EINVAL?
[ ... ]
Quoted 5 lines
sound/soc/codecs/nau8310.c new file +5
-0+static int nau8310_clksrc_choose (struct nau8310 *nau8310, +const struct nau8310_srate_attr **srate_table, +int *n1_sel,int *mult_sel,int *n2_sel,int *dsp_mult_sel) +{ +int i, j, mclk, mclk_max, ratio, ratio_sel, n2_max, ret;
[ ... ]
sound/soc/codecs/nau8310.c line 968
Quoted 9 lines
sound/soc/codecs/nau8310.c new file +9
-0+/* Get MCLK_SRC through 1/N, Multiplier, and then 1/N2. */ + mclk_max =0 ; +for (i =0 ; i <ARRAY_SIZE (mclk_n1_div); i++) { +for (j =0 ; j <ARRAY_SIZE (mclk_src_mult); j++) { + mclk = nau8310->mclk << mclk_src_mult[j].param; + mclk = mclk / mclk_n1_div[i].param; + ratio =nau8310_clksrc_n2 (nau8310, *srate_table, mclk, n2_sel); +if (ratio !=NAU8310_MCLK_FS_RATIO_NUM && + (mclk_max < mclk || i > *n1_sel)) {
[Severity: Medium]
In nau8310_clock_config() in sound/soc/codecs/nau8310.c, n1_sel is declared uninitialized on the stack and passed to nau8310_clksrc_choose(). When the initial direct mclk check does not match, *n1_sel is never initialized before entering this loop, so evaluating i > *n1_sel when mclk_max >= mclk reads an uninitialized stack value. Should *n1_sel be initialized (e.g. to -1) before the loop?
[ ... ]
Quoted 3 lines
sound/soc/codecs/nau8310.c new file +3
-0+static int nau8310_set_tdm_slot (struct snd_soc_dai *dai,unsigned int tx_mask, +unsigned int rx_mask,int slots,int slot_width) +{
[ ... ]
sound/soc/codecs/nau8310.c line 1174
Quoted 6 lines
sound/soc/codecs/nau8310.c new file +6
-0+for (i =0 ; i <NAU8310_TDM_MAX_SLOTS ; i++) +if (tx_mask &BIT (i)) { + ctrl_val |= nau8310_tx_slot_map[i]; +dev_dbg (dev,"NAU8310_ADC_%c_SEL_SLOT%d" , + (i %2 ==0 ) ?'V' :'I' , i); + }
[Severity: Medium]
In nau8310_set_tdm_slot() in sound/soc/codecs/nau8310.c, nau8310_tx_slot_map[] contains binary-encoded slot values (0x4, 0x5, 0x6, 0x7 for even V slots and 0x4<<3, 0x5<<3, 0x6<<3, 0x7<<3 for odd I slots), not one-hot bitmasks. If tx_mask has more than one even bit or more than one odd bit set (for example, BIT(2) | BIT(4)), bitwise-ORing them (0x5 | 0x6 = 0x7) corrupts the slot selection to slot 6. Should this validate or select at most one even slot and one odd slot?
[ ... ]
Quoted 23 lines
sound/soc/codecs/nau8310.c new file +23
-0+static int nau8310_switch_internal_osc (struct snd_soc_component *cp) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +int ret; + +/* For internal Ring OSC, the default fs apply to 48kHz */ + nau8310->fs =NAU8310_DEFAULT_SAMPLE_RATE ; + ret =nau8310_set_sysclk (cp,0 ,0 , + nau8310->fs *NAU8310_SYSCLK_MULT ,SND_SOC_CLOCK_IN ); +if (ret) +return ret; + + ret =nau8310_clock_config (nau8310); +if (ret) +return ret; + +/* Switch clock source to OSC before MCLK off if DSP is enabled. */ +regmap_update_bits (nau8310->regmap,NAU8310_R04_ENA_CTRL , +NAU8310_DSP_OSC_EN |NAU8310_DSP_SEL_OSC , +NAU8310_DSP_OSC_EN |NAU8310_DSP_SEL_OSC ); + +return 0 ; +}
[ ... ]
Quoted 6 lines
sound/soc/codecs/nau8310.c new file +6
-0+static void nau8310_shutdown (struct snd_pcm_substream *substream, +struct snd_soc_dai *dai) +{ +struct snd_soc_component *cp = dai->component; +struct snd_soc_dapm_context *dapm =snd_soc_component_to_dapm (cp); +int ret;
sound/soc/codecs/nau8310.c line 1253
Quoted 10 lines
sound/soc/codecs/nau8310.c new file +10
-0+ +if (!nau8310_dsp_active (cp)) +return ; + +snd_soc_dapm_disable_pin (dapm,"Sense" ); + + ret =nau8310_switch_internal_osc (cp); +if (ret) +return ; +}
[Severity: Medium]
In sound/soc/codecs/nau8310.c, what happens in nau8310_shutdown() when both playback and capture substreams are active and one of them closes?
Because nau8310_shutdown() does not check snd_soc_component_active(cp) (or dai->active), closing one substream while the other is still running will disable the "Sense" pin and call nau8310_switch_internal_osc(cp), switching the clocking to the internal oscillator mid-stream.
Also, nau8310_switch_internal_osc() calls nau8310_set_sysclk() with 48000 * 256 (12.288 MHz), overwriting nau8310->mclk during probe, resume, and shutdown. If a machine driver sets the sysclk frequency once at initialization rather than on every hw_params, won't nau8310->mclk be clobbered?
[ ... ]
Quoted 24 lines
sound/soc/codecs/nau8310.c new file +24
-0+static int nau8310_codec_probe (struct snd_soc_component *component) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component); +struct snd_soc_dapm_context *dapm =snd_soc_component_to_dapm (component); +int ret =0 ; + + nau8310->dapm = dapm; + +/* Loading DSP firmware */ + ret =nau8310_dsp_fw_load (nau8310); +if (ret) { +dev_err (nau8310->dev,"Loading DSP firmware failed(%d)" , ret); + goto err; + } + + ret =nau8310_dsp_setup_controls (component); +if (ret) { +dev_err (nau8310->dev,"DSP setup controls failed(%d)" , ret); + goto err; + } + +err: +return ret; +}
[ ... ]
sound/soc/codecs/nau8310.c line 1378
Quoted 5 lines
sound/soc/codecs/nau8310.c new file +5
-0+static const struct snd_soc_component_driver soc_comp_dev_nau8310 = { + .probe = nau8310_codec_probe, + .set_sysclk = nau8310_set_sysclk, + .suspend = nau8310_suspend, + .resume = nau8310_resume,
[Severity: High]
In nau8310_codec_probe() in sound/soc/codecs/nau8310.c, nau8310_dsp_fw_load() schedules nau8310->load_fw_work. If nau8310_dsp_setup_controls() fails (or if card binding later fails or the component is unbound from the card), load_fw_work is not cancelled because the error path in nau8310_codec_probe() does not call cancel_work_sync(&nau8310->load_fw_work) and soc_comp_dev_nau8310 has no .remove callback.
Could nau8310_load_fw_work() continue running after component teardown or race with a subsequent component re-probe?
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
kernel test robot
build botRe: [PATCH v1 2/2] ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
In reply to Neo Changkernel test robot noticed the following build warnings:
Hi Neo,
kernel test robot noticed the following build warnings:
[auto build test WARNING on broonie-sound/for-next]
[also build test WARNING on next-20261022]
[cannot apply to linus/master v7.3]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Neo-Chang/ASoC-dt-bindings-nuvoton-nau8310-Add-NAU83G10/20261022-164036 base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next patch link: https://lore.kernel.org/r/20261022084037.259983-3-YLCHANG2%40nuvoton.com patch subject: [PATCH v1 2/2] ASoC: codecs: nau8310: Add support for NAU83G10 amplifier config: powerpc-randconfig-1000-20261023 (https://download.01.org/0day-ci/archive/20261023/[email protected]/config) compiler: powerpc-linux-gcc (GCC) 15.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261023/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In function 'nau8310_send_dsp_command',
inlined from 'nau8310_dsp_set_kcs_setup' at sound/soc/codecs/nau8310-dsp.c:603:8:
>> sound/soc/codecs/nau8310-dsp.c:419:15: warning: 'kcs_setup_comp.set_kcs_offset' is used uninitialized [-Wuninitialized]
419 | ret = nau8310_message_to_dsp(cp, cmd_info, frag_len,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
420 | kcs_setup->set_kcs_offset, kcs_setup->set_len,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
421 | kcs_setup->set_kcs_data);
| ~~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/nau8310-dsp.c: In function 'nau8310_dsp_set_kcs_setup':
sound/soc/codecs/nau8310-dsp.c:592:34: note: 'kcs_setup_comp' declared here
592 | struct nau8310_kcs_setup kcs_setup_comp, *kcs_setup = &kcs_setup_comp;
| ^~~~~~~~~~~~~~
In function 'nau8310_send_dsp_command',
inlined from 'nau8310_dsp_set_kcs_setup' at sound/soc/codecs/nau8310-dsp.c:603:8:
>> sound/soc/codecs/nau8310-dsp.c:419:15: warning: 'kcs_setup_comp.set_kcs_data' is used uninitialized [-Wuninitialized]
419 | ret = nau8310_message_to_dsp(cp, cmd_info, frag_len,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
420 | kcs_setup->set_kcs_offset, kcs_setup->set_len,
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
421 | kcs_setup->set_kcs_data);
| ~~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/nau8310-dsp.c: In function 'nau8310_dsp_set_kcs_setup':
sound/soc/codecs/nau8310-dsp.c:592:34: note: 'kcs_setup_comp' declared here
592 | struct nau8310_kcs_setup kcs_setup_comp, *kcs_setup = &kcs_setup_comp;
| ^~~~~~~~~~~~~~vim +419 sound/soc/codecs/nau8310-dsp.c
370
371 /*
372 * nau8310_send_dsp_command - Send command to DSP
373 *
374 * @cp: component to register
375 * @cmd_id: DSP supported command ID
376 * @kcs_setup: KCS setup structure
377 *
378 * The communication protocol is a Master-Slave type protocol
379 * where the host processor is the master and DSP is the slave.
380 * The Master initiates the communication and can either write or
381 * read back from the slave.
382 * Transactions from the Master are called "Messages",
383 * and read-back data from the Slave is called a "Reply".
384 *
385 * The function sends command to DSP according to the command ID.
386 * These commands include getting the information of DSP,
387 * getting or setting KCS configuration, or making DSP control.
388 */
389 static int nau8310_send_dsp_command(struct snd_soc_component *cp, int cmd_id,
390 struct nau8310_kcs_setup *kcs_setup)
391 {
392 const struct nau8310_cmd_info *cmd_info;
393 int ret = 0, frag_len = 0;
394
395 cmd_info = &nau8310_dsp_cmd_table[cmd_id];
396 if ((cmd_info->msg_param && !kcs_setup->set_len) ||
397 (cmd_info->setup_data && !kcs_setup->set_kcs_data) ||
398 (cmd_info->reply_data && !kcs_setup->get_data))
399 return -EFAULT;
400
401 /* Read up to 1kB data because the LEN field to request data is 10-bits
402 * long; and not beyond 3kB offset.
403 */
404 if (cmd_id == NAU8310_DSP_CMD_GET_KCS_SETUP &&
405 (kcs_setup->set_len > NAU8310_DSP_KCS_DAT_LEN_MAX ||
406 kcs_setup->set_kcs_offset > NAU8310_DSP_KCS_OFFSET_MAX))
407 return -ERANGE;
408
409 /* one fragment for offset and size parameters
410 * one fragment for a postamble fragment
411 */
412 if (cmd_info->msg_param)
413 frag_len += 2;
414
415 /* fragments for KCS setup writen */
416 if (cmd_info->setup_data)
417 frag_len += DIV_ROUND_UP(kcs_setup->set_len, NAU8310_DSP_DATA_BYTE);
418
> 419 ret = nau8310_message_to_dsp(cp, cmd_info, frag_len,
420 kcs_setup->set_kcs_offset, kcs_setup->set_len,
421 kcs_setup->set_kcs_data);
422 if (ret)
423 return ret;
424
425 return nau8310_reply_from_dsp(cp, cmd_info, kcs_setup->get_len,
426 kcs_setup->get_data);
427 }
428
429 static inline int nau8310_dsp_exec_command(struct snd_soc_component *cp, int cmd_id,
430 int offset, int set_len, void *set_data, int get_len, void *get_data)
431 {
432 struct nau8310_kcs_setup kcs_setup = {
433 .set_kcs_offset = offset,
434 .set_len = set_len,
435 .set_kcs_data = set_data,
436 .get_len = get_len,
437 .get_data = get_data,
438 };
439
440 return nau8310_send_dsp_command(cp, cmd_id, &kcs_setup);
441 }
442
443 /*
444 * nau8310_dsp_kcs_setup - Send KCS setup command to DSP
445 *
446 * @cp: component to register
447 * @offset: address offset relative to KCS start
448 * @size: size of data writen to KCS
449 * @data: data writen to KCS setup
450 *
451 * The function sends KCS setup command to DSP for
452 * setting KCS configuration. The maximum size that you can transfer into
453 * the DSP is 96 bytes. Therefore, the driver has to split the data into
454 * 96 bytes chucks, if the setup configuration over the threshold.
455 */
456 static int nau8310_dsp_kcs_setup(struct snd_soc_component *cp, int offset, int size,
457 const void *data)
458 {
459 u8 *data_buf = (u8 *)data;
460 unsigned int kcs_rst;
461 int retries = 0, ret, data_len, data_rem, addr_offset;
462
463 /* Limit full load of KCS_SETUP data and not beyond 3kB offset. */
464 if (!data || size > NAU8310_DSP_KCS_DAT_LEN_MAX ||
465 offset > NAU8310_DSP_KCS_OFFSET_MAX)
466 return -EINVAL;
467
468 /* sending fragments for KCS setup */
469 addr_offset = offset;
470 data_rem = size;
471
472 while (data_rem) {
473 data_len = min(data_rem, NAU8310_DSP_KCS_TX_MAX);
474
475 ret = nau8310_dsp_exec_command(cp, NAU8310_DSP_CMD_SET_KCS_SETUP,
476 addr_offset, data_len, (void *)data_buf, 0, &kcs_rst);
477 if (ret) {
478 if (retries++ < NAU8310_DSP_RETRY_MAX)
479 continue;
480 return ret;
481 }
482
483 data_buf += (u8)data_len;
484 addr_offset += data_len;
485 data_rem -= data_len;
486
487 /* checking KCS result */
488 ret = nau8310_dsp_exec_command(cp, NAU8310_DSP_CMD_GET_KCS_RSLTS,
489 0, NAU8310_DSP_DATA_BYTE, NULL, NAU8310_DSP_DATA_BYTE, &kcs_rst);
490 if (ret)
491 return ret;
492 if (kcs_rst != NAU8310_DSP_KCS_RSLTS_SUCCESS)
493 return -EINVAL;
494 }
495
496 return 0;
497 }
498
499 static int nau8310_dsp_snd_soc_dapm_put(struct snd_kcontrol *kcontrol,
500 struct snd_ctl_elem_value *ucontrol)
501 {
502 struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_to_dapm(kcontrol);
503 struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
504 struct nau8310 *nau8310 = snd_soc_component_get_drvdata(component);
505 struct soc_mixer_control *mc =
506 (struct soc_mixer_control *)kcontrol->private_value;
507 unsigned int mask = (1 << fls(mc->max)) - 1;
508 unsigned int val;
509 int ret;
510
511 if (snd_soc_dapm_get_bias_level(dapm) > SND_SOC_BIAS_STANDBY) {
512 dev_warn_ratelimited(nau8310->dev, "changing path is not allowed during playback");
513 return -EBUSY;
514 }
515
516 val = (ucontrol->value.integer.value[0] & mask);
517
518 mutex_lock(&nau8310->lock);
519 if (val == NAU8310_DAC_SRC_DSP && !nau8310->load_fw_done) {
520 dev_warn_ratelimited(nau8310->dev, "Cannot enable DSP: Firmware not ready or disabled");
521 mutex_unlock(&nau8310->lock);
522 return -EBUSY;
523 }
524
525 ret = snd_soc_dapm_put_volsw(kcontrol, ucontrol);
526
527 if (val == NAU8310_DAC_SRC_BYPASS) {
528 /* Switch the clock source of DSP to MCLK. */
529 regmap_update_bits(nau8310->regmap, NAU8310_R04_ENA_CTRL,
530 NAU8310_DSP_SEL_OSC, 0);
531 }
532 mutex_unlock(&nau8310->lock);
533
534 return ret;
535 }
536
537 #define NAU8310_DSP_SOC_DAPM_SINGLE(xname, reg, shift, mask, invert) \
538 SOC_SINGLE_EXT(xname, reg, shift, mask, invert, \
539 snd_soc_dapm_get_volsw, nau8310_dsp_snd_soc_dapm_put)
540
541 static const struct snd_kcontrol_new nau8310_dacdat_select_dsp =
542 NAU8310_DSP_SOC_DAPM_SINGLE("Switch", NAU8310_R1A_DSP_CORE_CTRL2,
543 NAU8310_DAC_SEL_DSP_SFT, 1, 0);
544
545 static int nau8310_dsp_clock_event(struct snd_soc_dapm_widget *w,
546 struct snd_kcontrol *kcontrol, int event)
547 {
548 struct snd_soc_component *cp = snd_soc_dapm_to_component(w->dapm);
549 struct nau8310 *nau8310 = snd_soc_component_get_drvdata(cp);
550 int ret = 0;
551
552 if (SND_SOC_DAPM_EVENT_ON(event)) {
553 ret = nau8310_dsp_exec_command(cp, NAU8310_DSP_CMD_CLK_RESTART,
554 0, 0, NULL, 0, NULL);
555 if (ret)
556 dev_err(cp->dev, "Send DSP command %s fail (%d)",
557 dsp_cmd_table[NAU8310_DSP_CMD_CLK_RESTART], ret);
558
559 /* Switch the clock source of DSP to MCLK. */
560 regmap_update_bits(nau8310->regmap, NAU8310_R04_ENA_CTRL,
561 NAU8310_DSP_SEL_OSC, 0);
562 } else if (SND_SOC_DAPM_EVENT_OFF(event)) {
563 ret = nau8310_dsp_exec_command(cp, NAU8310_DSP_CMD_CLK_STOP,
564 0, 0, NULL, 0, NULL);
565 if (ret)
566 dev_err(cp->dev, "Send DSP command %s fail (%d)",
567 dsp_cmd_table[NAU8310_DSP_CMD_CLK_STOP], ret);
568 }
569
570 return ret;
571 }
572
573 static const struct snd_soc_dapm_widget nau8310_dsp_dapm_widgets[] = {
574 SND_SOC_DAPM_PGA("DSP", SND_SOC_NOPM, 0, 0, NULL, 0),
575 SND_SOC_DAPM_SWITCH("DAC data from DSP", SND_SOC_NOPM,
576 0, 0, &nau8310_dacdat_select_dsp),
577 SND_SOC_DAPM_SUPPLY("DSP Clock", SND_SOC_NOPM, 0, 0,
578 nau8310_dsp_clock_event, SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
579 };
580
581 static const struct snd_soc_dapm_route nau8310_dsp_dapm_routes[] = {
582 { "DSP", NULL, "Capture Sense" },
583 { "DAC data from DSP", "Switch", "DSP" },
584 { "DAC", NULL, "DAC data from DSP" },
585 { "DSP", NULL, "DSP Clock" },
586 };
587
588 static int nau8310_dsp_set_kcs_setup(struct snd_soc_component *cp)
589 {
590 struct nau8310 *nau8310 = snd_soc_component_get_drvdata(cp);
591 struct regmap *regmap = nau8310->regmap;
> 592 struct nau8310_kcs_setup kcs_setup_comp, *kcs_setup = &kcs_setup_comp;
593 struct device *dev = cp->dev;
594 const struct firmware *fw;
595 int ret, status = 0, buf_off, buf_len;
596
597 dev_dbg(dev, "Send DSP command %s",
598 dsp_cmd_table[NAU8310_DSP_CMD_GET_FRAME_STATUS]);
599
600 kcs_setup->get_len = kcs_setup->set_len = sizeof(status);
601 kcs_setup->get_data = (void *)&status;
602
> 603 ret = nau8310_send_dsp_command(cp, NAU8310_DSP_CMD_GET_FRAME_STATUS, kcs_setup); 604 if (ret) {
605 dev_err(dev, "Send DSP command %s fail (%d)",
606 dsp_cmd_table[NAU8310_DSP_CMD_GET_FRAME_STATUS], ret);
607 goto err;
608 }
609
610 if (!(status & NAU8310_DSP_ALGO_OK)) {
611 dev_err(dev, "Algorithm of DSP is not ready, status %x", status);
612 ret = -EIO;
613 goto err;
614 }
615
616 dev_info(dev, "Algorithm of DSP is ready, status %x", status);
617
618 dev_dbg(dev, "Request firmware and wait until finish.");
619 ret = request_firmware(&fw, NAU8310_DSP_FIRMWARE, dev);
620 if (ret) {
621 dev_err(dev, "Failed to load firmware (%d)", ret);
622 goto err;
623 }
624
625 buf_off = 0;
626 buf_len = nau8310->kcs_setup_size = fw->size;
627 dev_dbg(dev, "Send DSP command %s (OFF %d, LEN %d)",
628 dsp_cmd_table[NAU8310_DSP_CMD_SET_KCS_SETUP], buf_off, buf_len);
629 ret = nau8310_dsp_kcs_setup(cp, buf_off, buf_len, fw->data);
630 if (ret) {
631 dev_err(dev, "Send DSP command %s fail (%d)",
632 dsp_cmd_table[NAU8310_DSP_CMD_SET_KCS_SETUP], ret);
633 goto err_loaded;
634 }
635
636 release_firmware(fw);
637 nau8310->load_fw_done = true;
638
639 return 0;
640
641 err_loaded:
642 regmap_update_bits(regmap, NAU8310_R13_MUTE_CTRL, NAU8310_SOFT_MUTE, 0);
643 if (fw)
644 release_firmware(fw);
645 err:
646 nau8310->load_fw_done = false;
647 return ret;
648 }
649
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Mark Brown
maintainerRe: [PATCH v1 2/2] ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
In reply to Neo Changsound/soc/codecs/Kconfig config SND_SOC_ALL_CODECS · line 177
Quoted 8 lines
Add support for the Nuvoton NAU83G10 audio amplifier. The NAU83G10 is a mono boost Class-D audio amplifier with integrated DSP and I/V-sense support.
sound/soc/codecs/Kconfig +1
-0imply SND_SOC_MT6660 imply SND_SOC_NAU8315 imply SND_SOC_NAU8325 + imply SND_SOC_NAU8310
Please keep these sorted, same in other places like Makefile.
sound/soc/codecs/nau8310-dsp.c line 186
Quoted 10 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+static void nau8310_send_data_payload (struct snd_soc_component *cp, +unsigned short dsp_addr,const void *param_data,int param_size, +int *frag_cnt,int *padding) +{ +const u8 *data = (const u8 *)param_data; +u32 payload =0 ; +int i, data_size =0 ; + +for (i =0 ; i < param_size; i++) { + payload |= data[i] << (data_size *8 );
data[i] will be promoted to int which won't DTRT for values with the top bit set, cast to u32 before shifting.
sound/soc/codecs/nau8310-dsp.c line 553
Quoted 6 lines
sound/soc/codecs/nau8310-dsp.c new file +6
-0+struct snd_soc_component *cp =snd_soc_dapm_to_component (w->dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +int ret =0 ; + +if (SND_SOC_DAPM_EVENT_ON (event)) { + ret = nau8310_dsp_exec_command(cp, NAU8310_DSP_CMD_CLK_RESTART,
This needs to take nau8310->lock to avoid races with firmware load.
sound/soc/codecs/nau8310-dsp.c line 616
Quoted 1 line
sound/soc/codecs/nau8310-dsp.c new file +1
-0+dev_info (dev,"Algorithm of DSP is ready, status %x" , status);
dev_dbg(), this will get run a lot.
sound/soc/codecs/nau8310-dsp.c line 619
Quoted 2 lines
sound/soc/codecs/nau8310-dsp.c new file +2
-0+dev_dbg (dev,"Request firmware and wait until finish." ); + ret =request_firmware (&fw,NAU8310_DSP_FIRMWARE , dev);
We have a firmware-name in the binding, shouldn't we use it?
sound/soc/codecs/nau8310-dsp.c line 642
Quoted 2 lines
sound/soc/codecs/nau8310-dsp.c new file +2
-0+err_loaded: +regmap_update_bits (regmap,NAU8310_R13_MUTE_CTRL ,NAU8310_SOFT_MUTE ,0 );
Shouldn't we leave things muted?
sound/soc/codecs/nau8310.c line 290
Quoted 6 lines
sound/soc/codecs/nau8310.c new file +6
-0+static int nau8310_clkdet_put (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct soc_mixer_control *mc = + (struct soc_mixer_control *)kcontrol->private_value; +struct snd_soc_dapm_context *dapm =snd_soc_dapm_kcontrol_to_dapm (kcontrol);
This isn't a DAPM control, we can't use this lookup.
sound/soc/codecs/nau8310.c line 1093
Quoted 8 lines
sound/soc/codecs/nau8310.c new file +8
-0+static int nau8310_set_fmt (struct snd_soc_dai *dai,unsigned int fmt) +{ +struct snd_soc_component *component = dai->component; +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component); +struct regmap *regmap = nau8310->regmap; +unsigned int ctrl1_val =0 , ctrl2_val =0 ; + +switch (fmt &SND_SOC_DAIFMT_MASTER_MASK ) {
SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK.
sound/soc/codecs/nau8310.c line 1201
Quoted 7 lines
sound/soc/codecs/nau8310.c new file +7
-0+static int nau8310_switch_internal_osc (struct snd_soc_component *cp) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +int ret; + +/* For internal Ring OSC, the default fs apply to 48kHz */ + nau8310->fs =NAU8310_DEFAULT_SAMPLE_RATE ;
This will overwrite anything set_sysclk() set, since we call this from _shutdown() and _resume() we're relying on the machine driver reconfiguring the clocks when starting streams.
Signature · 1 line
Mark Brown <[email protected]>
Re: [PATCH v1 2/2] ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
Quoted 8 lines · Mark Brown
sound/soc/codecs/Kconfig config SND_SOC_ALL_CODECS · line 177
Quoted 7 lines
Add support for the Nuvoton NAU83G10 audio amplifier. The NAU83G10 is a mono boost Class-D audio amplifier with integrated DSP and I/V-sense support.
implySND_SOC_MT6660
implySND_SOC_NAU8315
implySND_SOC_NAU8325sound/soc/codecs/Kconfig +1
-0+ imply SND_SOC_NAU8310Please keep these sorted, same in other places like
Makefile.
Got it. I will fix the sorting here and in the Makefile in v2.
Quoted 13 lines
sound/soc/codecs/nau8310-dsp.c line 186
Quoted 11 lines
sound/soc/codecs/nau8310-dsp.c new file +10
-0+static void nau8310_send_data_payload (struct snd_soc_component *cp, +unsigned short dsp_addr,const void *param_data,int param_size, +int *frag_cnt,int *padding) +{ +const u8 *data = (const u8 *)param_data; +u32 payload =0 ; +int i, data_size =0 ; + +for (i =0 ; i < param_size; i++) { + payload |= data[i] << (data_size *8 );data[i] will be promoted to int which won't DTRT for values with the top bit set, cast to u32 before shifting.
Thanks for catching this. I will cast it to u32 before shifting in v2.
Quoted 8 lines
sound/soc/codecs/nau8310-dsp.c line 553
Quoted 7 lines
sound/soc/codecs/nau8310-dsp.c new file +6
-0+struct snd_soc_component *cp =snd_soc_dapm_to_component (w->dapm); +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +int ret =0 ; + +if (SND_SOC_DAPM_EVENT_ON (event)) { + ret = nau8310_dsp_exec_command(cp, NAU8310_DSP_CMD_CLK_RESTART,This needs to take
nau8310->lockto avoid races with firmware load.
Got it. I will add the lock to protect this in v2.
Quoted 3 lines
sound/soc/codecs/nau8310-dsp.c line 616
Quoted 2 lines
sound/soc/codecs/nau8310-dsp.c new file +1
-0+dev_info (dev,"Algorithm of DSP is ready, status %x" , status);
dev_dbg(), this will get run a lot.
I will change this to dev_dbg() in v2.
Quoted 4 lines
sound/soc/codecs/nau8310-dsp.c line 619
Quoted 3 lines
sound/soc/codecs/nau8310-dsp.c new file +2
-0+dev_dbg (dev,"Request firmware and wait until finish." ); + ret =request_firmware (&fw,NAU8310_DSP_FIRMWARE , dev);We have a firmware-name in the binding, shouldn't we use it?
I will modify it to use nau8310->dsp_firmware in v2.
Quoted 4 lines
sound/soc/codecs/nau8310-dsp.c line 642
Quoted 3 lines
sound/soc/codecs/nau8310-dsp.c new file +2
-0+err_loaded: +regmap_update_bits (regmap,NAU8310_R13_MUTE_CTRL ,NAU8310_SOFT_MUTE ,0 );Shouldn't we leave things muted?
It's redundant here. I will remove it in v2.
Quoted 7 lines
sound/soc/codecs/nau8310.c line 290
Quoted 7 lines
sound/soc/codecs/nau8310.c new file +6
-0+static int nau8310_clkdet_put (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct soc_mixer_control *mc = + (struct soc_mixer_control *)kcontrol->private_value; +struct snd_soc_dapm_context *dapm =snd_soc_dapm_kcontrol_to_dapm (kcontrol);
Got it. I will remove the dapm lookup function here in v2.
Quoted 11 lines
This isn't a DAPM control, we can't use this lookup.
sound/soc/codecs/nau8310.c line 1093
Quoted 8 lines
sound/soc/codecs/nau8310.c new file +8
-0+static int nau8310_set_fmt (struct snd_soc_dai *dai,unsigned int fmt) +{ +struct snd_soc_component *component = dai->component; +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (component); +struct regmap *regmap = nau8310->regmap; +unsigned int ctrl1_val =0 , ctrl2_val =0 ; + +switch (fmt &SND_SOC_DAIFMT_MASTER_MASK ) {
SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK.
I will change it to SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK in v2.
Quoted 11 lines
sound/soc/codecs/nau8310.c line 1201
Quoted 8 lines
sound/soc/codecs/nau8310.c new file +7
-0+static int nau8310_switch_internal_osc (struct snd_soc_component *cp) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +int ret; + +/* For internal Ring OSC, the default fs apply to 48kHz */ + nau8310->fs =NAU8310_DEFAULT_SAMPLE_RATE ;This will overwrite anything
set_sysclk()set, since we call this from_shutdown()and_resume()we're relying on the machine driver reconfiguring the clocks when starting streams.
Regarding switch_internal_osc() overwriting set_sysclk() states during suspend/resume, I'm considering two fixes:
1. Add NAU8310_CLK_INTERNAL to set_sysclk(): Standardizes and unifies clock setting through set_sysclk().
It sets the 48kHz internal clock here, but would still temporarily overwrite the nau8310->fs and mclk states during _shutdown() and _resume().
2. Pass arguments to clock_config(..., mclk, fs): Configures the 48kHz hardware directly via arguments, leaving the nau8310->fs and mclk states completely untouched. Which approach would be preferred for upstream?
Quoted 1 line
Mark Brown
maintainerRe: [PATCH v1 2/2] ASoC: codecs: nau8310: Add support for NAU83G10 amplifier
Quoted 17 lines · On Thu, Oct 29, 2026 at 10:24:39AM +0800, YLCHANG2 wrote:
Quoted 11 lines · Mark Brown
sound/soc/codecs/nau8310.c line 1201
Quoted 7 lines
sound/soc/codecs/nau8310.c new file +7
-0+static int nau8310_switch_internal_osc (struct snd_soc_component *cp) +{ +struct nau8310 *nau8310 =snd_soc_component_get_drvdata (cp); +int ret; + +/* For internal Ring OSC, the default fs apply to 48kHz */ + nau8310->fs =NAU8310_DEFAULT_SAMPLE_RATE ;This will overwrite anything
set_sysclk()set, since we call this from_shutdown()and_resume()we're relying on the machine driver reconfiguring the clocks when starting streams.Regarding
switch_internal_osc()overwritingset_sysclk()states during suspend/resume, I'm considering two fixes:
Quoted 8 lines
1. Add
NAU8310_CLK_INTERNALtoset_sysclk(): Standardizes and unifies clock setting throughset_sysclk().
It sets the 48kHz internal clock here, but would still temporarily overwrite thenau8310->fsand mclk states during_shutdown()and_resume().
2. Pass arguments toclock_config(..., mclk, fs): Configures the 48kHz hardware directly via arguments, leaving thenau8310->fsand mclk states completely untouched.
Which approach would be preferred for upstream?
The second option sounds like it might be a bit safer since it won't disrupt other users, though it may also be useful to expose the internal clock via set_sysclk() if it's useful for running audio.