On Thu, 15 Jun 2023 17:26:30 +0200 Herve Codina herve.codina@bootlin.com wrote:
Industrial I/O devices can be present in the audio path. These devices needs to be used as audio components in order to be fully integrated in the audio path.
This support allows to consider these Industrial I/O devices as auxiliary audio devices and allows one to control them using mixer controls.
Signed-off-by: Herve Codina herve.codina@bootlin.com
A few trivial things inline. With those tidied up, (for the IIO bits and general code - but I don't know the snd part well enough to review that).
Reviewed-by: Jonathan Cameron Jonathan.Cameron@huawei.com
index 000000000000..b9d72cbb85f2 --- /dev/null +++ b/sound/soc/codecs/audio-iio-aux.c @@ -0,0 +1,338 @@
...
+static int audio_iio_aux_add_controls(struct snd_soc_component *component,
struct audio_iio_aux_chan *chan)+{
- struct snd_kcontrol_new control = {};
Why not:
struct snd_kcontrol_new control = { .iface = SNDRV_CTL_ELEM_IFACE_MIXER; .name = chan->name; .info = audio_iio_aux_info_volsw; .get = audio_iio_aux_get_volsw; .put = audio_iio_aux_put_volsw; .private_value = (unsigned long)chan; };
- control.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
- control.name = chan->name;
- control.info = audio_iio_aux_info_volsw;
- control.get = audio_iio_aux_get_volsw;
- control.put = audio_iio_aux_put_volsw;
- control.private_value = (unsigned long)chan;
- return snd_soc_add_component_controls(component, &control, 1);
+}
+/*
- These data could be on stack but they are pretty big.
- As ASoC internally copy them and protect them against concurrent accesses
- (snd_soc_bind_card() protects using client_mutex), keep them in the global
- data area.
- */
+static struct snd_soc_dapm_widget widgets[3]; +static struct snd_soc_dapm_route routes[2];
+/* Be sure sizes are correct (need 3 widgets and 2 routes) */ +static_assert(ARRAY_SIZE(widgets) >= 3, "3 widgets are needed"); +static_assert(ARRAY_SIZE(routes) >= 2, "2 routes are needed");
+static int audio_iio_aux_add_dapms(struct snd_soc_component *component,
struct audio_iio_aux_chan *chan)+{
- struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
- char *output_name;
- char *input_name;
- char *pga_name;
- int ret;
- input_name = kasprintf(GFP_KERNEL, "%s IN", chan->name);
- if (!input_name)
return -ENOMEM;- output_name = kasprintf(GFP_KERNEL, "%s OUT", chan->name);
- if (!output_name) {
ret = -ENOMEM;goto out_free_input_name;- }
Trivial but a blank line here would be nice.
- pga_name = kasprintf(GFP_KERNEL, "%s PGA", chan->name);
- if (!pga_name) {
ret = -ENOMEM;goto out_free_output_name;- }
- widgets[0] = SND_SOC_DAPM_INPUT(input_name);
- widgets[1] = SND_SOC_DAPM_OUTPUT(output_name);
- widgets[2] = SND_SOC_DAPM_PGA(pga_name, SND_SOC_NOPM, 0, 0, NULL, 0);
- ret = snd_soc_dapm_new_controls(dapm, widgets, 3);
- if (ret)
goto out_free_pga_name;- routes[0].sink = pga_name;
- routes[0].control = NULL;
- routes[0].source = input_name;
- routes[1].sink = output_name;
- routes[1].control = NULL;
- routes[1].source = pga_name;
- ret = snd_soc_dapm_add_routes(dapm, routes, 2);
- /* Allocated names are no more needed (duplicated in ASoC internals) */
+out_free_pga_name:
- kfree(pga_name);
+out_free_output_name:
- kfree(output_name);
+out_free_input_name:
- kfree(input_name);
- return ret;
+}
+static int audio_iio_aux_component_probe(struct snd_soc_component *component) +{
- struct audio_iio_aux *iio_aux = snd_soc_component_get_drvdata(component);
- struct audio_iio_aux_chan *chan;
- int ret;
- int i;
- for (i = 0; i < iio_aux->num_chans; i++) {
chan = iio_aux->chans + i;ret = iio_read_max_channel_raw(chan->iio_chan, &chan->max);if (ret)return dev_err_probe(component->dev, ret,"chan[%d] %s: Cannot get max raw value\n",i, chan->name);ret = iio_read_min_channel_raw(chan->iio_chan, &chan->min);if (ret)return dev_err_probe(component->dev, ret,"chan[%d] %s: Cannot get min raw value\n",i, chan->name);if (chan->min > chan->max) {dev_dbg(component->dev, "chan[%d] %s: Swap min and max\n",i, chan->name);
Why? I'd like a comment here on what circumstances could cause this to happen.
swap(chan->min, chan->max);}/* Set initial value */ret = iio_write_channel_raw(chan->iio_chan,chan->is_invert_range ? chan->max : chan->min);if (ret)return dev_err_probe(component->dev, ret,"chan[%d] %s: Cannot set initial value\n",i, chan->name);ret = audio_iio_aux_add_controls(component, chan);if (ret)return ret;ret = audio_iio_aux_add_dapms(component, chan);if (ret)return ret;dev_dbg(component->dev, "chan[%d]: Added %s (min=%d, max=%d, invert=%s)\n",i, chan->name, chan->min, chan->max,str_on_off(chan->is_invert_range));- }
- return 0;
+}