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.