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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3653 bytes --]

On Fri, Apr 24, 2026 at 11:39:53AM +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.

> +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 soc_bytes_ext *params = (void *)kcontrol->private_value;
> +	int i, ret, reg = nau8360_peq_regaddr(kcontrol->id.name);
> +	__be16 *data;

> +	snd_soc_component_update_bits(cp, NAU8360_R9D_PEQ_CTL, NAU8360_HW1_MEM_TEST,
> +		NAU8360_HW1_MEM_TEST);
> +	for (i = 0; i < params->max / sizeof(u16); i++)
> +		snd_soc_component_write(cp, reg + i, be16_to_cpu(*(data + i)));
> +	snd_soc_component_update_bits(cp, NAU8360_R9D_PEQ_CTL, NAU8360_HW1_MEM_TEST, 0);
> +
> +	kfree(data);
> +
> +	return 0;

This should return 1 if the value was changed to generate notifications.

> +/**
> + * nau8360_set_tdm_slot - configure DAI TDM.
> + * @tx_mask: 4-bits value representing each active TX slots. Range: 0 (skip), 1~8. Ex.
> + *	bit 0-3 for left AEC output channel selection
> + *	bit 4-7 for right AEC output channel selection
> + *	bit 8-11 for left Isense output channel selection
> + *	bit 12-15 for right Isense output channel selection
> + *	bit 16-19 for left Vsense output channel selection
> + *	bit 20-23 for right Vsense output channel selection
> + *	bit 24-27 for Junction Temperature (Tj) data output channel selection
> + *	bit 28-31 for VBAT measured data output channel selection
> + * @rx_mask: Bitmask representing active RX slots. Ex.
> + *	bit 0-7 for left DAC channel source selection
> + *	bit 8-15 for right DAC channel source selection
> + *	bit 16-23 for left ANC channel source selection
> + *	bit 24-31 for right ANC channel source selection
> + *
> + * Configures a DAI for TDM operation. Only support 8 slots TDM.
> + */
> +static int nau8360_set_tdm_slot(struct snd_soc_dai *dai, unsigned int tx_mask,
> +	unsigned int rx_mask, int slots, int slot_width)

That's not the way the API is supposed to work, the mask should be which
slots are active as a bitmask.  It looks like what you want here is a
series of muxes which control the routing to some AIF widgets
representing the TDM slots.

> +static const struct regmap_config nau8360_regmap_config = {
> +	.reg_bits = NAU8360_REG_ADDR_LEN,
> +	.val_bits = NAU8360_REG_DATA_LEN,
> +
> +	.max_register = NAU8360_REG_MAX,
> +	.readable_reg = nau8360_readable_reg,
> +	.writeable_reg = nau8360_writeable_reg,
> +	.volatile_reg = nau8360_volatile_reg,
> +	.reg_read = nau8360_reg_read,
> +	.reg_write = nau8360_reg_write,
> +
> +	.cache_type = REGCACHE_RBTREE,

Use REGCACHE_MAPLE unless you've got a particular reason to use
something else, it's a more modern data structure than _RBTREE and makes
choices more suited to current hardware.

> +	/* DAC gain setting 0dB by changing current cell current. */
> +	regmap_update_bits(regmap, NAU8360_R6E_DAC_CFG0, NAU8360_DAC_CUR_MASK,
> +		NAU8360_DAC_CUR_0DB);

Things like gains should normally be user visible and left at the chip
defaults, that way we're not making use case specific decisions.

> +static void nau8360_read_device_properties(struct nau8360 *nau8360)
> +{

> +	nau8360->pbtl_enable = device_property_read_bool(dev, "nuvoton,pbtl-enable");

This is missing from the binding documentation.

> +static int nau8360_i2c_probe(struct i2c_client *i2c)
> +{

> +	return snd_soc_register_component(dev, &soc_comp_dev_nau8360, &nau8360_dai, 1);
> +}

Nothing unregisters this, you should use devm_snd_soc_register_component()

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]