Re: [alsa-devel] [PATCH] ASoC: wm_adsp: Expose mixer control API

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 1417 bytes --]

On Thu, Oct 31, 2019 at 02:36:43PM -0500, David Rhodes wrote:

> From: Li Xu <li.xu@cirrus.com>

> Expose mixer control API for reading and writing.
> The exposed API can be used by codec driver for
> interacting with mixer control in kernel space.
> This allows codec driver to implement more involved
> interactions with DSP firmware, such as Fast Use
> Case Switching.

It would be helpful if somewhere in the changelog you more explicitly
said that this was an in-kernel API, it isn't very clear what the API
you're adding is supposed to be.

The formatting here is also a bit weird, the lines are very short.

> +/*
> + * Find wm_coeff_ctl with input name as its subname
> + * If not found, return NULL
> + */
> +static struct wm_coeff_ctl *wm_adsp_get_ctl(struct wm_adsp *dsp,
> +                                            const char *name)

It is not clear why we only look things up by subname.  What's wrong
with the rest of the name?

> +int wm_adsp_write_ctl(struct wm_adsp *dsp, const char *name, const void *buf,
> +		      size_t len)
> +{
> +	struct wm_coeff_ctl *ctl;
> +
> +	ctl = wm_adsp_get_ctl(dsp, name);
> +	if (!ctl)
> +		return -EINVAL;
> +
> +	if (len > ctl->len)
> +		return -EINVAL;
> +
> +	return wm_coeff_write_control(ctl, buf, len);
> +}
> +EXPORT_SYMBOL_GPL(wm_adsp_write_ctl);

There should be a snd_ctl_notify() somewhere in the write path to tell
userspace that the value changed.

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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel