Re: [PATCH v2] ASoC: rt722-sdca: Add a control to support CAE firmware update

Mark Brown <[email protected]> Mon, 18 May 2026 13:07:47 +0100

Newsgroups	org.alsa-project.alsa-devel
Message-ID	<[email protected]>

On Mon, May 18, 2026 at 10:55:24AM +0800, [email protected] wrote:

> Realtek CAE requires specific tuning parameters based on
> the system vendor and SKU.
> This patch adds a kcontrol to trigger the firmware loading process.

> Reported-by: kernel test robot <[email protected]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

> - Fix the build error caught by kernel test robot. Replace the non-existent
>   snd_soc_component_get_bias_level() with snd_soc_dapm_get_bias_level().

The tags for the report only make sense to include if sending an
incremental patch, they don't make sense when sending a whole new
version since it looks like the bot asked for the new control which it
obviously didn't.

> +static int rt722_cae_load(struct rt722_sdca_priv *rt722)
> +{

> +	rt722_sdca_index_update_bits(rt722, RT722_VENDOR_REG,
> +			RT722_MISC_CTRL1, 0x8000, 0x8000);

Should the error paths disable this?  We disable it when we finish
loading but not in the error paths.

> +	regcache_cache_bypass(rt722->regmap, true);
> +	for (fw_offset = 0; fw_offset < cae_fw->size;) {
> +
> +		fw_data = (unsigned char *)&cae_fw->data[fw_offset];
> +		memcpy(tag, fw_data, 4);
> +		tag[4] = '\0';

We only validate that our current pointer is inside the firmware, we
don't validate that the record we're trying to parse fits inside the
firmware.

> +		if (strcmp(tag, xu_tag) == 0) {
> +			dev_dbg(dev, "%s: This is a XU tag", __func__);
> +			memcpy(&addr, (fw_data + 4), 4);
> +			memcpy(&size, (fw_data + 8), 4);

We don't validate the size AFAICT?

> +static int rt722_cae_update_put(struct snd_kcontrol *kcontrol,
> +				 struct snd_ctl_elem_value *ucontrol)
> +{

> +
> +	if (ucontrol->value.integer.value[0]) {
> +		if (snd_soc_dapm_get_bias_level(dapm) == SND_SOC_BIAS_OFF) {
> +			ret = rt722_cae_load(rt722);
> +			if (ret)
> +				dev_err(component->dev, "CAE load failed: %d\n", ret);

I think it'd make sense to return the error to userspace rather than
silently just reporting it as no change.

signature.asc (application/pgp-signature, 488 B)