Re: [PATCH 1/2] ASoC: codecs: nau8360: Set DAC path to bypass mode when DSP firmware load fails

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using `snd_soc_component_write` overwrites the entire `NAU8360_R12_PATH_CTRL` register, unintentionally clearing other critical audio path routing configurations.
- [Medium] Modifying a DAPM-controlled hardware mux register directly bypasses ASoC DAPM state tracking and locking.
--

commit 6a6b1e851a10df0446b789100342efda63bba458
Author: Neo Chang <YLCHANG2@nuvoton.com>
ASoC: codecs: nau8360: Set DAC path to bypass mode when DSP firmware load fails

This commit automatically routes the DAC path to bypass mode if the DSP
firmware fails to load. This is intended to keep basic audio playback
functional as a fallback mechanism.

> diff --git a/sound/soc/codecs/nau8360.c b/sound/soc/codecs/nau8360.c
> index d7af2dffb749a..b3cf207eea86d 100644
> --- a/sound/soc/codecs/nau8360.c
> +++ b/sound/soc/codecs/nau8360.c
> @@ -1702,6 +1702,7 @@ static void nau8360_load_fw_work(struct work_struct *work)
>  	if (ret) {
>  		dev_err(nau8360->dev, "Failed to initialize DSP: %d\n", ret);
>  		nau8360_dsp_enable(nau8360->regmap, false);
> +		snd_soc_component_write(cp, NAU8360_R12_PATH_CTRL, NAU8360_DAC_SEL_BYP);

[Severity: High]
Does this overwrite the entire NAU8360_R12_PATH_CTRL register?

Since NAU8360_DAC_SEL_BYP evaluates to 0, using snd_soc_component_write()
writes 0x0000 to the entire 16-bit register instead of selectively masking
the target bits. This would unintentionally clear other critical hardware
audio routing configurations (such as NAU8360_SEL_HW1_OUT,
NAU8360_AUD_SEL_SINCOUT, etc.) initialized during probe, completely breaking
audio playback rather than falling back to bypass mode.

Should this use snd_soc_component_update_bits() instead to safely update
only the DAC selection bits without clobbering the rest of the register?

[Severity: Medium]
Does modifying this hardware mux register directly circumvent ASoC DAPM state
tracking and locking?

The DAC selection bits in NAU8360_R12_PATH_CTRL are exposed to userspace
and managed by DAPM as a mux control (nau8360_dac_mux). Writing to this
register directly from a background worker introduces race conditions against
concurrent userspace ALSA control changes. Additionally, the software DAPM
graph falls out of sync with the physical hardware, which could cause DAPM
to incorrectly leave DSP widgets powered up and fail to apply power to the
bypass path widgets.

Should this hardware path change be routed through the appropriate kcontrol
or DAPM interfaces to ensure the graph and power states are correctly
re-evaluated?

>  		return;
>  	}
>  	nau8360->load_fw_done = true;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904024456.193528-1-YLCHANG2@nuvoton.com?part=1