| Message ID | 20230523151223.109551-8-herve.codina@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for IIO devices in ASoC | expand |
On Tue, 23 May 2023 17:12:21 +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 auxliary > audio devices and allows to control them using mixer controls. > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > --- > diff --git a/sound/soc/codecs/audio-iio-aux.c b/sound/soc/codecs/audio-iio-aux.c > new file mode 100644 > index 000000000000..21575c4b35fd > --- /dev/null > +++ b/sound/soc/codecs/audio-iio-aux.c > @@ -0,0 +1,302 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +// > +// audio-iio-aux.c -- ALSA SoC glue to use IIO devices as audio components > +// > +// Copyright 2023 CS GROUP France > +// > +// Author: Herve Codina <herve.codina@bootlin.com> > + > +#include <linux/iio/consumer.h> > +#include <linux/module.h> #include <linux/mod_devicetable.h> ideally to pick up the of_device_id definition without bouncing through some non obvious header path. > +#include <linux/slab.h> > +#include <sound/soc.h> > +#include <sound/tlv.h> Otherwise, the IIO elements of this look good. So for those at least Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> I don't have enough knowledge of the snd stuff to review those parts. Jonathan
Tue, May 23, 2023 at 05:12:21PM +0200, Herve Codina kirjoitti: > 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 auxliary > audio devices and allows to control them using mixer controls. ... > +// audio-iio-aux.c -- ALSA SoC glue to use IIO devices as audio components Putting file name into file is not a good idea in case the file will be renamed in the future. ... > +struct audio_iio_aux_chan { > + struct iio_channel *iio_chan; > + const char *name; > + bool is_invert_range; If you put bool after int:s it may save a few bytes in some cases. > + int max; > + int min; Wondering if there is already a data type for the ranges (like linear_range.h, but not sure it's applicable here). > +}; ... > + if (val < 0) > + return -EINVAL; > + if (val > max - min) Btw, who will validate that max > min? > + return -EINVAL; ... > + return 1; /* The value changed */ Perhaps this 1 needs a definition? ... > +static struct snd_soc_dapm_widget widgets[3] = {0}; > +static struct snd_soc_dapm_route routes[2] = {0}; 0:s are not needed. Moreover, the entire assingments are redundant as this is guaranteed by the C standard. ... > + char *input_name = NULL; > + char *output_name = NULL; > + char *pga_name = NULL; Redundant assignments if you properly label the freeing. ... > + BUILD_BUG_ON(ARRAY_SIZE(widgets) < 3); Use static_assert() at the place where the array is defined. ... > + BUILD_BUG_ON(ARRAY_SIZE(routes) < 2); Ditto. ... > +end: out_free: > + /* Allocated names are no more needed (duplicated in ASoC internals) */ > + kfree(pga_name); > + kfree(output_name); > + kfree(input_name); > + > + return ret; ... > + 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) { > + dev_err(component->dev, "chan[%d] %s: Cannot get max raw value (%d)\n", > + i, chan->name, ret); > + return ret; It sounds like a part of ->probe() flow, correct? Can dev_err_probe() be used here? > + } > + > + ret = iio_read_min_channel_raw(chan->iio_chan, &chan->min); > + if (ret) { > + dev_err(component->dev, "chan[%d] %s: Cannot get min raw value (%d)\n", > + i, chan->name, ret); > + return ret; Ditto. > + } > + > + /* Set initial value */ > + ret = iio_write_channel_raw(chan->iio_chan, > + chan->is_invert_range ? chan->max : chan->min); > + if (ret) { > + dev_err(component->dev, "chan[%d] %s: Cannot set initial value (%d)\n", > + i, chan->name, ret); > + return ret; Ditto. > + } ... > + dev_dbg(component->dev, "chan[%d]: Added %s (min=%d, max=%d, invert=%s)\n", > + i, chan->name, chan->min, chan->max, > + chan->is_invert_range ? "on" : "off"); str_on_off() > + } ... > + count = of_property_count_strings(np, "io-channel-names"); > + if (count < 0) { > + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names\n", np); > + return count; return dev_err_probe(); > + } ... > + for (i = 0; i < iio_aux->num_chans; i++) { > + iio_aux_chan = iio_aux->chans + i; > + > + ret = of_property_read_string_index(np, "io-channel-names", i, > + &iio_aux_chan->name); > + if (ret < 0) { > + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names[%d]\n", np, i); > + return ret; Ditto. > + } > + tmp = 0; > + of_property_read_u32_index(np, "snd-control-invert-range", i, &tmp); > + iio_aux_chan->is_invert_range = tmp; You can use this variable directly. > + } Btw, can you avoid using OF APIs? It's better to have device property/fwnode API to be used from day 1. ... > + platform_set_drvdata(pdev, iio_aux); Which callback is using this driver data? ... > +static const struct of_device_id audio_iio_aux_ids[] = { > + { .compatible = "audio-iio-aux", }, Inner comma is not needed. > + { } > +}; ... > +static struct platform_driver audio_iio_aux_driver = { > + .driver = { > + .name = "audio-iio-aux", > + .of_match_table = audio_iio_aux_ids, > + }, > + .probe = audio_iio_aux_probe, > +}; > + Redundant blank line > +module_platform_driver(audio_iio_aux_driver);
Hi Jonathan, On Sun, 28 May 2023 18:38:55 +0100 Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 23 May 2023 17:12:21 +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 auxliary > > audio devices and allows to control them using mixer controls. > > > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > > --- > > > diff --git a/sound/soc/codecs/audio-iio-aux.c b/sound/soc/codecs/audio-iio-aux.c > > new file mode 100644 > > index 000000000000..21575c4b35fd > > --- /dev/null > > +++ b/sound/soc/codecs/audio-iio-aux.c > > @@ -0,0 +1,302 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +// > > +// audio-iio-aux.c -- ALSA SoC glue to use IIO devices as audio components > > +// > > +// Copyright 2023 CS GROUP France > > +// > > +// Author: Herve Codina <herve.codina@bootlin.com> > > + > > +#include <linux/iio/consumer.h> > > +#include <linux/module.h> > > #include <linux/mod_devicetable.h> ideally to pick up > the of_device_id definition without bouncing through some non > obvious header path. Right, <linux/module.h> will be replaced by <linux/mod_devicetable.h> in the next iteration. Thanks for the review, Hervé > > > > +#include <linux/slab.h> > > +#include <sound/soc.h> > > +#include <sound/tlv.h> > > Otherwise, the IIO elements of this look good. So for those at least > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > > I don't have enough knowledge of the snd stuff to review those > parts. > > Jonathan > >
Hi Andy, On Sat, 3 Jun 2023 21:26:19 +0300 andy.shevchenko@gmail.com wrote: > Tue, May 23, 2023 at 05:12:21PM +0200, Herve Codina kirjoitti: > > 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 auxliary > > audio devices and allows to control them using mixer controls. > > ... > > > +// audio-iio-aux.c -- ALSA SoC glue to use IIO devices as audio components > > Putting file name into file is not a good idea in case the file will be renamed > in the future. Indeed, the file name will be removed in the nest iteration. > > ... > > > +struct audio_iio_aux_chan { > > + struct iio_channel *iio_chan; > > + const char *name; > > + bool is_invert_range; > > If you put bool after int:s it may save a few bytes in some cases. I will mode is_invert_range after the int members. > > > + int max; > > + int min; > > Wondering if there is already a data type for the ranges (like linear_range.h, > but not sure it's applicable here). Seems not applicable here. - IIO does not use linear_range or something similar. It just uses simple int. - ASoC does not use linear_range or something similar. It just uses simple long. So, I keep the simple int min and max. > > > +}; > > ... > > > + if (val < 0) > > + return -EINVAL; > > + if (val > max - min) > > Btw, who will validate that max > min? By construction, min = 0 max = iio_read_max_channel_raw() - iio_read_min_channel_raw() and iio_read_max_channel_raw() returns a value greater or equal to iio_read_min_channel_raw(). But to be sure, I will check the last asumption at probe() and swap the minimum and maximum values if needed. > > > + return -EINVAL; > > ... > > > + return 1; /* The value changed */ > > Perhaps this 1 needs a definition? Yes but to be coherent, in ASoC code, many places need to be changed too in order to use the newly defined value. I don't think these modifications should be part of this series. > > ... > > > +static struct snd_soc_dapm_widget widgets[3] = {0}; > > +static struct snd_soc_dapm_route routes[2] = {0}; > > 0:s are not needed. Moreover, the entire assingments are redundant > as this is guaranteed by the C standard. Indeed, the 0 assignment will be removed in the next iteration. > > ... > > > + char *input_name = NULL; > > + char *output_name = NULL; > > + char *pga_name = NULL; > > Redundant assignments if you properly label the freeing. I will rework the error paths (gotos) to avoid these assignement. > > ... > > > + BUILD_BUG_ON(ARRAY_SIZE(widgets) < 3); > > Use static_assert() at the place where the array is defined. Will be done in next iteration. > > ... > > > + BUILD_BUG_ON(ARRAY_SIZE(routes) < 2); > > Ditto. Will be done in next iteration. > > ... > > > +end: > > out_free: > > > + /* Allocated names are no more needed (duplicated in ASoC internals) */ > > + kfree(pga_name); > > + kfree(output_name); > > + kfree(input_name); > > + > > + return ret; > > ... > > > + 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) { > > + dev_err(component->dev, "chan[%d] %s: Cannot get max raw value (%d)\n", > > + i, chan->name, ret); > > + return ret; > > It sounds like a part of ->probe() flow, correct? > Can dev_err_probe() be used here? Will be changed in the next iteration. > > > + } > > + > > + ret = iio_read_min_channel_raw(chan->iio_chan, &chan->min); > > + if (ret) { > > + dev_err(component->dev, "chan[%d] %s: Cannot get min raw value (%d)\n", > > + i, chan->name, ret); > > + return ret; > > Ditto. Will be changed in the next iteration. > > > + } > > + > > + /* Set initial value */ > > + ret = iio_write_channel_raw(chan->iio_chan, > > + chan->is_invert_range ? chan->max : chan->min); > > + if (ret) { > > + dev_err(component->dev, "chan[%d] %s: Cannot set initial value (%d)\n", > > + i, chan->name, ret); > > + return ret; > > Ditto. Will be changed in the next iteration. > > > + } > > ... > > > + dev_dbg(component->dev, "chan[%d]: Added %s (min=%d, max=%d, invert=%s)\n", > > + i, chan->name, chan->min, chan->max, > > + chan->is_invert_range ? "on" : "off"); > > str_on_off() Indeed, I didn't know str_on_off(). Thanks for pointing. Will be use in next iteration. > > > + } > > ... > > > + count = of_property_count_strings(np, "io-channel-names"); > > + if (count < 0) { > > > + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names\n", np); > > + return count; > > return dev_err_probe(); Will be changed in next iteration. > > > + } > > ... > > > + for (i = 0; i < iio_aux->num_chans; i++) { > > + iio_aux_chan = iio_aux->chans + i; > > + > > + ret = of_property_read_string_index(np, "io-channel-names", i, > > + &iio_aux_chan->name); > > + if (ret < 0) { > > + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names[%d]\n", np, i); > > + return ret; > > Ditto. Will be changed in next iteration. > > > + } > > > + tmp = 0; > > + of_property_read_u32_index(np, "snd-control-invert-range", i, &tmp); > > > + iio_aux_chan->is_invert_range = tmp; > > You can use this variable directly. Not sure, is_invert_range is a bool and tmp is a u32. In previous iteration, I wrote iio_aux_chan->is_invert_range = !!tmp; > > > + } > > Btw, can you avoid using OF APIs? It's better to have device property/fwnode > API to be used from day 1. Hum, this comment was raised in the previous iteration https://lore.kernel.org/linux-kernel/20230501162456.3448c494@jic23-huawei/ I didn't find any equivalent to of_property_read_u32_index() in the device_property_read_*() function family. I mean I did find anything available to get a value from an array using an index. In the previous iteration it was concluded that keeping OF APIs in this series seemed "reasonable". > > ... > > > + platform_set_drvdata(pdev, iio_aux); > > Which callback is using this driver data? None -> I will remove platform_set_drvdata(). > > ... > > > +static const struct of_device_id audio_iio_aux_ids[] = { > > + { .compatible = "audio-iio-aux", }, > > Inner comma is not needed. Will be fixed. > > > + { } > > +}; > > ... > > > +static struct platform_driver audio_iio_aux_driver = { > > + .driver = { > > + .name = "audio-iio-aux", > > + .of_match_table = audio_iio_aux_ids, > > + }, > > + .probe = audio_iio_aux_probe, > > +}; > > > + > > Redundant blank line Will be fixed. > > > +module_platform_driver(audio_iio_aux_driver); >
On Tue, Jun 6, 2023 at 4:54 PM Herve Codina <herve.codina@bootlin.com> wrote: > On Sat, 3 Jun 2023 21:26:19 +0300 > andy.shevchenko@gmail.com wrote: > > Tue, May 23, 2023 at 05:12:21PM +0200, Herve Codina kirjoitti: ... > > > + int max; > > > + int min; > > > > Wondering if there is already a data type for the ranges (like linear_range.h, > > but not sure it's applicable here). > > Seems not applicable here. > - IIO does not use linear_range or something similar. It just uses simple int. > - ASoC does not use linear_range or something similar. It just uses simple long. > > So, I keep the simple int min and max. Sure. ... > > > + return 1; /* The value changed */ > > > > Perhaps this 1 needs a definition? > > Yes but to be coherent, in ASoC code, many places need to be changed too > in order to use the newly defined value. > I don't think these modifications should be part of this series. Yes, we are all for consistency. ... > > > + for (i = 0; i < iio_aux->num_chans; i++) { > > > + iio_aux_chan = iio_aux->chans + i; > > > + > > > + ret = of_property_read_string_index(np, "io-channel-names", i, > > > + &iio_aux_chan->name); > > > + if (ret < 0) { > > > + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names[%d]\n", np, i); > > > + return ret; > > > > Ditto. > Will be changed in next iteration. > > > > > + } > > > > > + tmp = 0; > > > + of_property_read_u32_index(np, "snd-control-invert-range", i, &tmp); > > > > > + iio_aux_chan->is_invert_range = tmp; > > > > You can use this variable directly. > > Not sure, is_invert_range is a bool and tmp is a u32. Ah, I see. > In previous iteration, I wrote > iio_aux_chan->is_invert_range = !!tmp; > > > > + } > > > > Btw, can you avoid using OF APIs? It's better to have device property/fwnode > > API to be used from day 1. > > Hum, this comment was raised in the previous iteration > https://lore.kernel.org/linux-kernel/20230501162456.3448c494@jic23-huawei/ > > I didn't find any equivalent to of_property_read_u32_index() in the > device_property_read_*() function family. > I mean I did find anything available to get a value from an array using an index. This is done by reading the entire array at once and then parsing as you wish in the code, device_property_read_u32_array() is for that. > In the previous iteration it was concluded that keeping OF APIs in this series > seemed "reasonable". Maybe, but consider the above. -- With Best Regards, Andy Shevchenko
Hi Andy, On Tue, 6 Jun 2023 15:54:04 +0200 Herve Codina <herve.codina@bootlin.com> wrote: ... > > > > > + platform_set_drvdata(pdev, iio_aux); > > > > Which callback is using this driver data? > > None -> I will remove platform_set_drvdata(). > My previous answer was not correct. The platform_set_drvdata() call is needed. In fact, the driver uses snd_soc_component_get_drvdata() https://elixir.bootlin.com/linux/v6.4-rc5/source/include/sound/soc-component.h#L425 and this snd_soc_component_get_drvdata() get the driver data set by the platform_set_drvdata() call. I cannot use snd_soc_component_set_drvdata() to set the driver data because I haven't got the struct snd_soc_component instance when I need to set the driver data. So, I will not remove the platform_set_drvdata() call. The sequence is: --- 8< --- static int audio_iio_aux_probe(struct platform_device *pdev) { struct audio_iio_aux *iio_aux; iio_aux = devm_kzalloc(&pdev->dev, sizeof(*iio_aux), GFP_KERNEL); if (!iio_aux) return -ENOMEM; ... platform_set_drvdata(pdev, iio_aux); return devm_snd_soc_register_component(iio_aux->dev, &audio_iio_aux_component_driver, NULL, 0); } --- 8< --- The struct snd_soc_component instance will be create during the devm_snd_soc_register_component() call. Regards, Hervé
Hi Andy, On Tue, 6 Jun 2023 17:34:22 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: ... > > > > > > Btw, can you avoid using OF APIs? It's better to have device property/fwnode > > > API to be used from day 1. > > > > Hum, this comment was raised in the previous iteration > > https://lore.kernel.org/linux-kernel/20230501162456.3448c494@jic23-huawei/ > > > > I didn't find any equivalent to of_property_read_u32_index() in the > > device_property_read_*() function family. > > I mean I did find anything available to get a value from an array using an index. > > This is done by reading the entire array at once and then parsing as > you wish in the code, device_property_read_u32_array() is for that. > > > In the previous iteration it was concluded that keeping OF APIs in this series > > seemed "reasonable". > > Maybe, but consider the above. I see. Will switch to device_property_*() family in the next iteration. Thanks, Hervé
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 44806bfe8ee5..92b7c417f1b2 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -53,6 +53,7 @@ config SND_SOC_ALL_CODECS imply SND_SOC_AK5558 imply SND_SOC_ALC5623 imply SND_SOC_ALC5632 + imply SND_SOC_AUDIO_IIO_AUX imply SND_SOC_AW8738 imply SND_SOC_AW88395 imply SND_SOC_BT_SCO @@ -608,6 +609,17 @@ config SND_SOC_ALC5632 tristate depends on I2C +config SND_SOC_AUDIO_IIO_AUX + tristate "Audio IIO Auxiliary device" + depends on IIO + help + Enable support for Industrial I/O devices as audio auxiliary devices. + This allows to have an IIO device present in the audio path and + controlled using mixer controls. + + To compile this driver as a module, choose M here: the module + will be called snd-soc-audio-iio-aux. + config SND_SOC_AW8738 tristate "Awinic AW8738 Audio Amplifier" select GPIOLIB diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 2c45c2f97e4e..f2828d3616c5 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -45,6 +45,7 @@ snd-soc-ak4671-objs := ak4671.o snd-soc-ak5386-objs := ak5386.o snd-soc-ak5558-objs := ak5558.o snd-soc-arizona-objs := arizona.o arizona-jack.o +snd-soc-audio-iio-aux-objs := audio-iio-aux.o snd-soc-aw8738-objs := aw8738.o snd-soc-aw88395-lib-objs := aw88395/aw88395_lib.o snd-soc-aw88395-objs := aw88395/aw88395.o \ @@ -421,6 +422,7 @@ obj-$(CONFIG_SND_SOC_AK5558) += snd-soc-ak5558.o obj-$(CONFIG_SND_SOC_ALC5623) += snd-soc-alc5623.o obj-$(CONFIG_SND_SOC_ALC5632) += snd-soc-alc5632.o obj-$(CONFIG_SND_SOC_ARIZONA) += snd-soc-arizona.o +obj-$(CONFIG_SND_SOC_AUDIO_IIO_AUX) += snd-soc-audio-iio-aux.o obj-$(CONFIG_SND_SOC_AW8738) += snd-soc-aw8738.o obj-$(CONFIG_SND_SOC_AW88395_LIB) += snd-soc-aw88395-lib.o obj-$(CONFIG_SND_SOC_AW88395) +=snd-soc-aw88395.o diff --git a/sound/soc/codecs/audio-iio-aux.c b/sound/soc/codecs/audio-iio-aux.c new file mode 100644 index 000000000000..21575c4b35fd --- /dev/null +++ b/sound/soc/codecs/audio-iio-aux.c @@ -0,0 +1,302 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// audio-iio-aux.c -- ALSA SoC glue to use IIO devices as audio components +// +// Copyright 2023 CS GROUP France +// +// Author: Herve Codina <herve.codina@bootlin.com> + +#include <linux/iio/consumer.h> +#include <linux/module.h> +#include <linux/slab.h> +#include <sound/soc.h> +#include <sound/tlv.h> + +struct audio_iio_aux_chan { + struct iio_channel *iio_chan; + const char *name; + bool is_invert_range; + int max; + int min; +}; + +struct audio_iio_aux { + struct device *dev; + struct audio_iio_aux_chan *chans; + unsigned int num_chans; +}; + +static int audio_iio_aux_info_volsw(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + struct audio_iio_aux_chan *chan = (struct audio_iio_aux_chan *)kcontrol->private_value; + + uinfo->count = 1; + uinfo->value.integer.min = 0; + uinfo->value.integer.max = chan->max - chan->min; + uinfo->type = (uinfo->value.integer.max == 1) ? + SNDRV_CTL_ELEM_TYPE_BOOLEAN : SNDRV_CTL_ELEM_TYPE_INTEGER; + return 0; +} + +static int audio_iio_aux_get_volsw(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct audio_iio_aux_chan *chan = (struct audio_iio_aux_chan *)kcontrol->private_value; + int max = chan->max; + int min = chan->min; + bool invert_range = chan->is_invert_range; + int ret; + int val; + + ret = iio_read_channel_raw(chan->iio_chan, &val); + if (ret < 0) + return ret; + + ucontrol->value.integer.value[0] = val - min; + if (invert_range) + ucontrol->value.integer.value[0] = max - ucontrol->value.integer.value[0]; + + return 0; +} + +static int audio_iio_aux_put_volsw(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct audio_iio_aux_chan *chan = (struct audio_iio_aux_chan *)kcontrol->private_value; + int max = chan->max; + int min = chan->min; + bool invert_range = chan->is_invert_range; + int val; + int ret; + int tmp; + + val = ucontrol->value.integer.value[0]; + if (val < 0) + return -EINVAL; + if (val > max - min) + return -EINVAL; + + val = val + min; + if (invert_range) + val = max - val; + + ret = iio_read_channel_raw(chan->iio_chan, &tmp); + if (ret < 0) + return ret; + + if (tmp == val) + return 0; + + ret = iio_write_channel_raw(chan->iio_chan, val); + if (ret) + return ret; + + return 1; /* The value changed */ +} + +static int audio_iio_aux_add_controls(struct snd_soc_component *component, + struct audio_iio_aux_chan *chan) +{ + struct snd_kcontrol_new control = {0}; + + 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] = {0}; +static struct snd_soc_dapm_route routes[2] = {0}; + +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 *input_name = NULL; + char *output_name = NULL; + char *pga_name = NULL; + int ret; + + input_name = kasprintf(GFP_KERNEL, "%s IN", chan->name); + if (!input_name) { + ret = -ENOMEM; + goto end; + } + output_name = kasprintf(GFP_KERNEL, "%s OUT", chan->name); + if (!output_name) { + ret = -ENOMEM; + goto end; + } + pga_name = kasprintf(GFP_KERNEL, "%s PGA", chan->name); + if (!pga_name) { + ret = -ENOMEM; + goto end; + } + + BUILD_BUG_ON(ARRAY_SIZE(widgets) < 3); + widgets[0] = SND_SOC_DAPM_WIDGET(SND_SOC_DAPM_INPUT(input_name)); + widgets[1] = SND_SOC_DAPM_WIDGET(SND_SOC_DAPM_OUTPUT(output_name)); + widgets[2] = SND_SOC_DAPM_WIDGET(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 end; + + BUILD_BUG_ON(ARRAY_SIZE(routes) < 2); + 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); + +end: + /* Allocated names are no more needed (duplicated in ASoC internals) */ + kfree(pga_name); + kfree(output_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) { + dev_err(component->dev, "chan[%d] %s: Cannot get max raw value (%d)\n", + i, chan->name, ret); + return ret; + } + + ret = iio_read_min_channel_raw(chan->iio_chan, &chan->min); + if (ret) { + dev_err(component->dev, "chan[%d] %s: Cannot get min raw value (%d)\n", + i, chan->name, ret); + return ret; + } + + /* Set initial value */ + ret = iio_write_channel_raw(chan->iio_chan, + chan->is_invert_range ? chan->max : chan->min); + if (ret) { + dev_err(component->dev, "chan[%d] %s: Cannot set initial value (%d)\n", + i, chan->name, ret); + return ret; + } + + 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, + chan->is_invert_range ? "on" : "off"); + } + + return 0; +} + +static const struct snd_soc_component_driver audio_iio_aux_component_driver = { + .probe = audio_iio_aux_component_probe, +}; + +static int audio_iio_aux_probe(struct platform_device *pdev) +{ + struct device_node *np = pdev->dev.of_node; + struct audio_iio_aux_chan *iio_aux_chan; + struct audio_iio_aux *iio_aux; + int count; + u32 tmp; + int ret; + int i; + + iio_aux = devm_kzalloc(&pdev->dev, sizeof(*iio_aux), GFP_KERNEL); + if (!iio_aux) + return -ENOMEM; + + iio_aux->dev = &pdev->dev; + + count = of_property_count_strings(np, "io-channel-names"); + if (count < 0) { + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names\n", np); + return count; + } + + iio_aux->chans = devm_kmalloc_array(&pdev->dev, count, + sizeof(*iio_aux->chans), GFP_KERNEL); + if (!iio_aux->chans) + return -ENOMEM; + iio_aux->num_chans = count; + + for (i = 0; i < iio_aux->num_chans; i++) { + iio_aux_chan = iio_aux->chans + i; + + ret = of_property_read_string_index(np, "io-channel-names", i, + &iio_aux_chan->name); + if (ret < 0) { + dev_err(iio_aux->dev, "%pOF: failed to read io-channel-names[%d]\n", np, i); + return ret; + } + + iio_aux_chan->iio_chan = devm_iio_channel_get(iio_aux->dev, iio_aux_chan->name); + if (IS_ERR(iio_aux_chan->iio_chan)) { + return dev_err_probe(iio_aux->dev, + PTR_ERR(iio_aux_chan->iio_chan), + "get IIO channel '%s' failed\n", + iio_aux_chan->name); + } + + tmp = 0; + of_property_read_u32_index(np, "snd-control-invert-range", i, &tmp); + iio_aux_chan->is_invert_range = tmp; + } + + platform_set_drvdata(pdev, iio_aux); + + return devm_snd_soc_register_component(iio_aux->dev, + &audio_iio_aux_component_driver, + NULL, 0); +} + +static const struct of_device_id audio_iio_aux_ids[] = { + { .compatible = "audio-iio-aux", }, + { } +}; +MODULE_DEVICE_TABLE(of, audio_iio_aux_ids); + +static struct platform_driver audio_iio_aux_driver = { + .driver = { + .name = "audio-iio-aux", + .of_match_table = audio_iio_aux_ids, + }, + .probe = audio_iio_aux_probe, +}; + +module_platform_driver(audio_iio_aux_driver); + +MODULE_AUTHOR("Herve Codina <herve.codina@bootlin.com>"); +MODULE_DESCRIPTION("IIO ALSA SoC aux driver"); +MODULE_LICENSE("GPL");
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 auxliary audio devices and allows to control them using mixer controls. Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- sound/soc/codecs/Kconfig | 12 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/audio-iio-aux.c | 302 +++++++++++++++++++++++++++++++ 3 files changed, 316 insertions(+) create mode 100644 sound/soc/codecs/audio-iio-aux.c