Re: [PATCH v2 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] Directly modifying a DAPM-controlled routing register bypasses DAPM state tracking, leading to an aborted power sequence and complete audio playback failure.
- [High] Successful firmware load fails to restore the DAC path to DSP mode if it was previously set to bypass mode on failure.
--

commit 66908584f9a6b49d33fcb5f3e81314daeeff0253
Author: Neo Chang <YLCHANG2@nuvoton.com>

ASoC: codecs: nau8360: Set DAC path to bypass mode when DSP firmware load fails

Automatically route the DAC path to bypass mode if DSP firmware fails
to load, ensuring basic audio playback remains functional.

> diff --git a/sound/soc/codecs/nau8360.c b/sound/soc/codecs/nau8360.c
> index d7af2dffb749a..8b75d3b9ebd61 100644
> --- a/sound/soc/codecs/nau8360.c
> +++ b/sound/soc/codecs/nau8360.c
> @@ -1702,6 +1702,8 @@ 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_update_bits(cp, NAU8360_R12_PATH_CTRL,
> +			NAU8360_DAC_SEL_MASK, NAU8360_DAC_SEL_BYP);

[Severity: High]
Does directly modifying the routing register here bypass DAPM state tracking?

When userspace subsequently starts playback, DAPM's unmodified internal state
may still believe the path is set to DSP mode and power up the DSP Clock widget.

This would trigger nau8360_dsp_clock_event(), which sends an I2C command to the
explicitly halted DSP:

sound/soc/codecs/nau8360-dsp.c:nau8360_dsp_clock_event() {
	...
	if (SND_SOC_DAPM_EVENT_ON(event))
		ret = nau8360_send_dsp_broadcast(cp, NAU8360_DSP_CMD_CLK_RESTART);
	...
}

Could this cause the command to time out and abort the DAPM power sequence?

Additionally, does this direct register write bypass the lock and hardware state
machine updates normally handled by the mux control?

sound/soc/codecs/nau8360.c:nau8360_dac_mux_put_enum() {
	...
	mutex_lock(&nau8360->lock);

	ret = snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
	if (ret <= 0)
		goto unlock;

	nau8360_dsp_switch(component, snd_soc_enum_item_to_val(e, item[0]));

unlock:
	mutex_unlock(&nau8360->lock);
	...
}

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

[Severity: High]
If a firmware load successfully completes after a previous failure (for example,
on system resume), will the DAC path remain stuck in bypass mode?

It appears that if the hardware was forced into bypass mode during a previous
error, the success path does not restore NAU8360_R12_PATH_CTRL to match DAPM's
expected DSP mode.

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