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.