On 6/22/2022 9:46 AM, Vitaly Rodionov wrote:
From: Stefan Binding sbinding@opensource.cirrus.com
The cs35l41 part contains a DSP which is able to run firmware. The cs_dsp library can be used to control the DSP. These controls can be exposed to userspace using ALSA controls. This library adds apis to be able to interface between cs_dsp and hda drivers and expose the relevant controls as ALSA controls.
Signed-off-by: Stefan Binding sbinding@opensource.cirrus.com Signed-off-by: Vitaly Rodionov vitalyr@opensource.cirrus.com
MAINTAINERS | 1 + sound/pci/hda/Kconfig | 4 + sound/pci/hda/Makefile | 2 + sound/pci/hda/hda_cs_dsp_ctl.c | 207 +++++++++++++++++++++++++++++++++ sound/pci/hda/hda_cs_dsp_ctl.h | 33 ++++++ 5 files changed, 247 insertions(+) create mode 100644 sound/pci/hda/hda_cs_dsp_ctl.c create mode 100644 sound/pci/hda/hda_cs_dsp_ctl.h
...
+static unsigned int wmfw_convert_flags(unsigned int in) +{
- unsigned int out, rd, wr, vol;
- rd = SNDRV_CTL_ELEM_ACCESS_READ;
- wr = SNDRV_CTL_ELEM_ACCESS_WRITE;
- vol = SNDRV_CTL_ELEM_ACCESS_VOLATILE;
- out = 0;
- if (in) {
out |= rd;if (in & WMFW_CTL_FLAG_WRITEABLE)out |= wr;if (in & WMFW_CTL_FLAG_VOLATILE)out |= vol;- } else {
out |= rd | wr | vol;- }
- return out;
+}
This is more question of preference, so you can leave above function as is, but you could also do something like the following, which is bit shorter: static unsigned int wmfw_convert_flags(unsigned int in) { unsigned int out = SNDRV_CTL_ELEM_ACCESS_READ;
if (!in) return SNDRV_CTL_ELEM_ACCESS_READWRITE | SNDRV_CTL_ELEM_ACCESS_VOLATILE;
if (in & WMFW_CTL_FLAG_WRITEABLE) out |= SNDRV_CTL_ELEM_ACCESS_WRITE; if (in & WMFW_CTL_FLAG_VOLATILE) out |= SNDRV_CTL_ELEM_ACCESS_VOLATILE;
return out; }
+static int hda_cs_dsp_add_kcontrol(struct hda_cs_dsp_coeff_ctl *ctl) +{
- struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
- struct snd_kcontrol_new *kcontrol;
- struct snd_kcontrol *kctl;
- int ret = 0;
- if (cs_ctl->len > ADSP_MAX_STD_CTRL_SIZE) {
dev_err(cs_ctl->dsp->dev, "Control %s: length %zu exceeds maximum %d\n", ctl->name,cs_ctl->len, ADSP_MAX_STD_CTRL_SIZE);return -EINVAL;- }
- kcontrol = kzalloc(sizeof(*kcontrol), GFP_KERNEL);
- if (!kcontrol)
return -ENOMEM;- kcontrol->name = ctl->name;
- kcontrol->info = hda_cs_dsp_coeff_info;
- kcontrol->iface = SNDRV_CTL_ELEM_IFACE_MIXER;
- kcontrol->private_value = (unsigned long)ctl;
- kcontrol->access = wmfw_convert_flags(cs_ctl->flags);
- kcontrol->get = hda_cs_dsp_coeff_get;
- kcontrol->put = hda_cs_dsp_coeff_put;
- kctl = snd_ctl_new1(kcontrol, NULL);
Wouldn't kctl = snd_ctl_new1(kcontrol, ctl); work instead of kcontrol->private_value = (unsigned long)ctl; ... kctl = snd_ctl_new1(kcontrol, NULL); ?
You can then get the value using snd_kcontrol_chip() macro, so instead of doing quite long lines with casts like: struct hda_cs_dsp_coeff_ctl *ctl = (struct hda_cs_dsp_coeff_ctl *)kctl->private_value; you could do struct hda_cs_dsp_coeff_ctl *ctl = snd_kcontrol_chip(kctl);