Re: [PATCH v9 2/2] ASoC: codecs: nau8360: Add support for NAU83G60 amplifier

On Thu, Aug 20, 2026 at 10:29:25AM +0800, Neo Chang wrote:
> Add support for the Nuvoton NAU83G60 audio codec. The NAU83G60 is a
> stereo 30W+30W smart amplifier with an integrated low-latency
> Advanced Audio DSP.

A couple of issues here, mostly this looks fine.

> +static int nau8360_peq_coeff_put(struct snd_kcontrol *kcontrol,
> +	struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *cp = snd_kcontrol_chip(kcontrol);
> +	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(cp);
> +	struct nau8360 *nau8360 = snd_soc_component_get_drvdata(cp);
> +	struct soc_bytes_ext *params = (void *)kcontrol->private_value;
> +	int i, reg, ret = 0;
> +	__be16 *data = NULL;
> +	bool changed = false;
> +
> +	/* Use the DAPM lock to prevent race conditions during DAPM power-up
> +	 * state transitions, and check component active status to prohibit
> +	 * PEQ access during active audio streams (playback and capture).
> +	 */
> +	snd_soc_dapm_mutex_lock(dapm);
> +	if (snd_soc_component_active(cp)) {
> +		dev_dbg(nau8360->dev,
> +			"PEQ coefficient access is ignored during audio is active");
> +		goto unlock_dapm;

This should probably be -EBUSY or something, though that will upset
mixer-test.  As things stand since the values change when a stream
starts and stops there should be an event when that happens, and since
we report success some value ought to be filled in for the read.

> +static void nau8360_dsp_fw_load(struct nau8360 *nau8360)
> +{
> +	nau8360->load_fw_done = false;
> +	schedule_work(&nau8360->load_fw_work);
> +}

> +static int __maybe_unused nau8360_resume(struct snd_soc_component *component)
> +{
> +	struct nau8360 *nau8360 = snd_soc_component_get_drvdata(component);
> +	struct regmap *regmap = nau8360->regmap;

> +	nau8360_dsp_fw_load(nau8360);

This only kicks off the firmware load, it doesn't wait for it to
complete.  That means that if a stream restarts quickly after resume we
will run into issues - I see that the normal playback path just errors
if we try to start without the firmware.  We should wait, either here or
in the startup path.

> +	if (NAU8360_IS_DSP_REG(reg))
> +		*value = nau8360->i2c_read_buf[0] |
> +			(nau8360->i2c_read_buf[1] << 8) |
> +			(nau8360->i2c_read_buf[2] << 16) |
> +			(nau8360->i2c_read_buf[3] << 24);

get_unaligned_le32()?

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmqIbbMACgkQJNaLcl1U
h9ARoQf/Vm6uj3Mw8pW8Ni5unT/J30hjetyZ2zxoIepaS5lXsLB5W34wxwhZhTQq
D3wbdzK1JJnvXXivIoyoJjY8EZZlXCp7si6f89XtOokkQ8Mvw21EgUetKaDBrNfw
j91Nw8GBQnY5B+jM4qSvgmHi0nE3lujSpcKm6WQyUrGO2iMJpanztxuv8Py6DCDI
ojPbtfNA8mCWtiA3gLJC4R3O1DbJZRuKJLpgCyp35NMnttJDaajjdAhIgPAfK/jV
thzBU72FrQpI4JsDNgT7j9aXXFQBT1k62EKBOEfAB+CFvAz2sgdkd5l/70FCEnRV
65SCV405IUFuSECXuVpFIzRAJ57p6w==
=NzXF
-----END PGP SIGNATURE-----