Several weeks ago, I sent a series [1] for adding a potentiometer as an
auxiliary device in ASoC. The feedback was that the potentiometer should
be directly handled in IIO (as other potentiometers) and something more
generic should be present in ASoC in order to have a binding to import
some IIO devices into sound cards.
The series related to the IIO potentiometer device is already applied.
This series introduces audio-iio-aux. Its goal is to offer the binding
between IIO and ASoC.
It exposes attached IIO devices as ASoC auxiliary devices and allows to
control them through mixer controls.
On my system, the IIO device is a potentiometer and it is present in an
amplifier design present in the audio path.
Compare to the previous iteration
https://lore.kernel.org/linux-kernel/[email protected]/
This v3 series mainly:
- Reworks/Simplifies some IIO code.
- Reworks/Simplified the audio-iio-aux driver.
- Removes an already applied patch.
Best regards,
Hervé
[1] https://lore.kernel.org/linux-kernel/[email protected]/
[2] https://lore.kernel.org/linux-kernel/[email protected]/
Changes v2 -> v3
- Patches 1, 2
No changes.
- Patch 3, 4
Add 'Acked-by: Jonathan Cameron <[email protected]>'.
- Patch 5 (new in v3)
Removed the 'unused' variable and check the null pointer when used.
- Patch 6 (new in v3)
Introduce {min,max}_array().
- Patch 7 (new in v3)
Use max_array() in iio_channel_read_max().
- Patch 8 (new in v3)
Replace a FIXME comment by a TODO one.
- Patch 9 (patch 5 in v2)
Removed the 'unused' variable and check the null pointer when used.
Use min_array().
Remplace a FIXME comment by a TODO one.
- Patch 10 (patch 6 in v2)
Convert existing macros to return a compound litteral instead of
adding a new helper.
- Patch 11 (patch 7 in v2)
Remove the file name from the C file header.
Use directly converted DAPM macros.
Replace <linux/module.h> by <linux/mod_devicetable.h>.
Add <linux/platform_device.h>.
Be sure that min <= max. Swap values if it is not the case.
Move the bool structure member after the int ones.
Remove unneeded assignements.
Use dev_err_probe() when relevant.
Use str_on_off().
Use static_assert() instead of BUILD_BUG_ON().
Remove unneeded comma and blank line.
Use device_property_*() instead of the OF API.
- patch 8 available in v2 removed as already applied
- Patch 12 (patch 9 in v2)
Use devm_add_action_or_reset().
Call simple_populate_aux() from simple_parse_of().
Changes v1 -> v2
- Patch 1
Rename simple-iio-aux to audio-iio-aux
Rename invert to snd-control-invert-range
Remove the /schemas/iio/iio-consumer.yaml reference
Remove the unneeded '|' after description
- Patch 2 (new in v2)
Introduce the simple-audio-card additional-devs subnode
- Patch 3 (new in v2)
Check err before switch() in iio_channel_read_max()
- Patch 4 (new in v2)
Fix raw reads and raw writes documentation
- Patch 5 (patch 2 in v1)
Check err before switch() in iio_channel_read_min()
Fix documentation
- Patch 6 (path 3 in v1)
No changes
- Patch 7 (patch 4 in v1)
Rename simple-iio-aux to audio-iio-aux
Rename invert to snd-control-invert-range
Remove the mask usage from audio_iio_aux_{get,put}_volsw helpers
Use directly PTR_ERR() in dev_err_probe() parameter
Remove the '!!' construction
Remove of_match_ptr()
- Patch 8 (new in v2)
Add a missing of_node_put() in the simple-card driver
- Patch 9 (new in v2)
Handle additional-devs in the simple-card driver
Herve Codina (12):
ASoC: dt-bindings: Add audio-iio-aux
ASoC: dt-bindings: simple-card: Add additional-devs subnode
iio: inkern: Check error explicitly in iio_channel_read_max()
iio: consumer.h: Fix raw values documentation notes
iio: inkern: Remove the 'unused' variable usage in
iio_channel_read_max()
minmax: Introduce {min,max}_array()
iio: inkern: Use max_array() to get the maximum value from an array
iio: inkern: Replace a FIXME comment by a TODO one
iio: inkern: Add a helper to query an available minimum raw value
ASoC: soc-dapm.h: Convert macros to return a compound litteral
ASoC: codecs: Add support for the generic IIO auxiliary devices
ASoC: simple-card: Handle additional devices
.../bindings/sound/audio-iio-aux.yaml | 64 ++++
.../bindings/sound/simple-card.yaml | 53 +++
drivers/iio/inkern.c | 84 ++++-
include/linux/iio/consumer.h | 37 +-
include/linux/minmax.h | 26 ++
include/sound/soc-dapm.h | 138 +++++---
sound/soc/codecs/Kconfig | 12 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/audio-iio-aux.c | 334 ++++++++++++++++++
sound/soc/generic/simple-card.c | 46 ++-
10 files changed, 726 insertions(+), 70 deletions(-)
create mode 100644 Documentation/devicetree/bindings/sound/audio-iio-aux.yaml
create mode 100644 sound/soc/codecs/audio-iio-aux.c
--
2.40.1
An additional-devs subnode can be present in the simple-card top node.
This subnode is used to declared some "virtual" additional devices.
Create related devices from this subnode and avoid this subnode presence
to interfere with the already supported subnodes analysis.
Signed-off-by: Herve Codina <[email protected]>
---
sound/soc/generic/simple-card.c | 46 +++++++++++++++++++++++++++++++--
1 file changed, 44 insertions(+), 2 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 6f044cc8357e..ae4a47018278 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -348,6 +348,7 @@ static int __simple_for_each_link(struct asoc_simple_priv *priv,
struct device *dev = simple_priv_to_dev(priv);
struct device_node *top = dev->of_node;
struct device_node *node;
+ struct device_node *add_devs;
uintptr_t dpcm_selectable = (uintptr_t)of_device_get_match_data(dev);
bool is_top = 0;
int ret = 0;
@@ -359,6 +360,8 @@ static int __simple_for_each_link(struct asoc_simple_priv *priv,
is_top = 1;
}
+ add_devs = of_get_child_by_name(top, PREFIX "additional-devs");
+
/* loop for all dai-link */
do {
struct asoc_simple_data adata;
@@ -367,6 +370,12 @@ static int __simple_for_each_link(struct asoc_simple_priv *priv,
struct device_node *np;
int num = of_get_child_count(node);
+ /* Skip additional-devs node */
+ if (node == add_devs) {
+ node = of_get_next_child(top, node);
+ continue;
+ }
+
/* get codec */
codec = of_get_child_by_name(node, is_top ?
PREFIX "codec" : "codec");
@@ -380,12 +389,15 @@ static int __simple_for_each_link(struct asoc_simple_priv *priv,
/* get convert-xxx property */
memset(&adata, 0, sizeof(adata));
- for_each_child_of_node(node, np)
+ for_each_child_of_node(node, np) {
+ if (np == add_devs)
+ continue;
simple_parse_convert(dev, np, &adata);
+ }
/* loop for all CPU/Codec node */
for_each_child_of_node(node, np) {
- if (plat == np)
+ if (plat == np || add_devs == np)
continue;
/*
* It is DPCM
@@ -427,6 +439,7 @@ static int __simple_for_each_link(struct asoc_simple_priv *priv,
} while (!is_top && node);
error:
+ of_node_put(add_devs);
of_node_put(node);
return ret;
}
@@ -464,6 +477,31 @@ static int simple_for_each_link(struct asoc_simple_priv *priv,
return ret;
}
+static void simple_depopulate_aux(void *data)
+{
+ struct asoc_simple_priv *priv = data;
+
+ of_platform_depopulate(simple_priv_to_dev(priv));
+}
+
+static int simple_populate_aux(struct asoc_simple_priv *priv)
+{
+ struct device *dev = simple_priv_to_dev(priv);
+ struct device_node *node;
+ int ret;
+
+ node = of_get_child_by_name(dev->of_node, PREFIX "additional-devs");
+ if (!node)
+ return 0;
+
+ ret = of_platform_populate(node, NULL, NULL, dev);
+ of_node_put(node);
+ if (ret)
+ return ret;
+
+ return devm_add_action_or_reset(dev, simple_depopulate_aux, priv);
+}
+
static int simple_parse_of(struct asoc_simple_priv *priv, struct link_info *li)
{
struct snd_soc_card *card = simple_priv_to_card(priv);
@@ -493,6 +531,10 @@ static int simple_parse_of(struct asoc_simple_priv *priv, struct link_info *li)
if (ret < 0)
return ret;
+ ret = simple_populate_aux(priv);
+ if (ret < 0)
+ return ret;
+
ret = snd_soc_of_parse_aux_devs(card, PREFIX "aux-devs");
return ret;
--
2.40.1
A helper, iio_read_max_channel_raw() exists to read the available
maximum raw value of a channel but nothing similar exists to read the
available minimum raw value.
This new helper, iio_read_min_channel_raw(), fills the hole and can be
used for reading the available minimum raw value of a channel.
It is fully based on the existing iio_read_max_channel_raw().
Signed-off-by: Herve Codina <[email protected]>
---
drivers/iio/inkern.c | 63 ++++++++++++++++++++++++++++++++++++
include/linux/iio/consumer.h | 12 +++++++
2 files changed, 75 insertions(+)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 4331e74191cf..4f383a5ef59f 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -909,6 +909,69 @@ int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
}
EXPORT_SYMBOL_GPL(iio_read_max_channel_raw);
+static int iio_channel_read_min(struct iio_channel *chan,
+ int *val, int *val2, int *type,
+ enum iio_chan_info_enum info)
+{
+ const int *vals;
+ int length;
+ int ret;
+
+ ret = iio_channel_read_avail(chan, &vals, type, &length, info);
+ if (ret < 0)
+ return ret;
+
+ switch (ret) {
+ case IIO_AVAIL_RANGE:
+ switch (*type) {
+ case IIO_VAL_INT:
+ *val = vals[0];
+ break;
+ default:
+ *val = vals[0];
+ if (val2)
+ *val2 = vals[1];
+ }
+ return 0;
+
+ case IIO_AVAIL_LIST:
+ if (length <= 0)
+ return -EINVAL;
+ switch (*type) {
+ case IIO_VAL_INT:
+ *val = min_array(vals, length);
+ break;
+ default:
+ /* TODO: learn about min for other iio values */
+ return -EINVAL;
+ }
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+int iio_read_min_channel_raw(struct iio_channel *chan, int *val)
+{
+ struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
+ int ret;
+ int type;
+
+ mutex_lock(&iio_dev_opaque->info_exist_lock);
+ if (!chan->indio_dev->info) {
+ ret = -ENODEV;
+ goto err_unlock;
+ }
+
+ ret = iio_channel_read_min(chan, val, NULL, &type, IIO_CHAN_INFO_RAW);
+err_unlock:
+ mutex_unlock(&iio_dev_opaque->info_exist_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iio_read_min_channel_raw);
+
int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
{
struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
index f536820b9cf2..e9910b41d48e 100644
--- a/include/linux/iio/consumer.h
+++ b/include/linux/iio/consumer.h
@@ -301,6 +301,18 @@ int iio_write_channel_raw(struct iio_channel *chan, int val);
*/
int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
+/**
+ * iio_read_min_channel_raw() - read minimum available raw value from a given
+ * channel, i.e. the minimum possible value.
+ * @chan: The channel being queried.
+ * @val: Value read back.
+ *
+ * Note, if standard units are required, raw reads from iio channels
+ * need the offset (default 0) and scale (default 1) to be applied
+ * as (raw + offset) * scale.
+ */
+int iio_read_min_channel_raw(struct iio_channel *chan, int *val);
+
/**
* iio_read_avail_channel_raw() - read available raw values from a given channel
* @chan: The channel being queried.
--
2.40.1
Introduce min_array() (resp max_array()) in order to get the
minimal (resp maximum) of values present in an array.
Signed-off-by: Herve Codina <[email protected]>
---
include/linux/minmax.h | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h
index 396df1121bff..37a211f22404 100644
--- a/include/linux/minmax.h
+++ b/include/linux/minmax.h
@@ -133,6 +133,32 @@
*/
#define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >)
+#define __minmax_array(op, array, len) ({ \
+ typeof(array) __array = (array); \
+ typeof(len) __len = (len); \
+ typeof(*__array + 0) __element = __array[--__len]; \
+ while (__len--) \
+ __element = op(__element, __array[__len]); \
+ __element; })
+
+/**
+ * min_array - return minimum of values present in an array
+ * @array: array
+ * @len: array length
+ *
+ * Note that @len must not be zero (empty array).
+ */
+#define min_array(array, len) __minmax_array(min, array, len)
+
+/**
+ * max_array - return maximum of values present in an array
+ * @array: array
+ * @len: array length
+ *
+ * Note that @len must not be zero (empty array).
+ */
+#define max_array(array, len) __minmax_array(max, array, len)
+
/**
* clamp_t - return a value clamped to a given range using a given type
* @type: the type of variable to use
--
2.40.1
The SND_SOC_DAPM_* helpers family are used to build widgets array in a
static way.
Convert them to return a compound litteral in order to use them in both
static and dynamic way.
With this conversion, the different SND_SOC_DAPM_* parameters can be
computed by the code and the widget can be built based on this parameter
computation.
static int create_widget(char *input_name)
{
struct snd_soc_dapm_widget widget;
char name*;
...
name = input_name;
if (!name)
name = "default";
widget = SND_SOC_DAPM_INPUT(name);
...
}
Signed-off-by: Herve Codina <[email protected]>
---
include/sound/soc-dapm.h | 138 ++++++++++++++++++++++++++-------------
1 file changed, 92 insertions(+), 46 deletions(-)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
index 87f8e1793af1..2e38dff16779 100644
--- a/include/sound/soc-dapm.h
+++ b/include/sound/soc-dapm.h
@@ -42,36 +42,45 @@ struct soc_enum;
/* codec domain */
#define SND_SOC_DAPM_VMID(wname) \
-{ .id = snd_soc_dapm_vmid, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_vmid, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0}
/* platform domain */
#define SND_SOC_DAPM_SIGGEN(wname) \
-{ .id = snd_soc_dapm_siggen, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_siggen, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM }
#define SND_SOC_DAPM_SINK(wname) \
-{ .id = snd_soc_dapm_sink, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_sink, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM }
#define SND_SOC_DAPM_INPUT(wname) \
-{ .id = snd_soc_dapm_input, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_input, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM }
#define SND_SOC_DAPM_OUTPUT(wname) \
-{ .id = snd_soc_dapm_output, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_output, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM }
#define SND_SOC_DAPM_MIC(wname, wevent) \
-{ .id = snd_soc_dapm_mic, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mic, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
.event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD}
#define SND_SOC_DAPM_HP(wname, wevent) \
-{ .id = snd_soc_dapm_hp, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_hp, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
.event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD}
#define SND_SOC_DAPM_SPK(wname, wevent) \
-{ .id = snd_soc_dapm_spk, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_spk, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
.event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD}
#define SND_SOC_DAPM_LINE(wname, wevent) \
-{ .id = snd_soc_dapm_line, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_line, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
.event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD}
@@ -82,93 +91,110 @@ struct soc_enum;
/* path domain */
#define SND_SOC_DAPM_PGA(wname, wreg, wshift, winvert,\
wcontrols, wncontrols) \
-{ .id = snd_soc_dapm_pga, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_pga, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
#define SND_SOC_DAPM_OUT_DRV(wname, wreg, wshift, winvert,\
wcontrols, wncontrols) \
-{ .id = snd_soc_dapm_out_drv, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_out_drv, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
#define SND_SOC_DAPM_MIXER(wname, wreg, wshift, winvert, \
wcontrols, wncontrols)\
-{ .id = snd_soc_dapm_mixer, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
#define SND_SOC_DAPM_MIXER_NAMED_CTL(wname, wreg, wshift, winvert, \
wcontrols, wncontrols)\
-{ .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
/* DEPRECATED: use SND_SOC_DAPM_SUPPLY */
#define SND_SOC_DAPM_MICBIAS(wname, wreg, wshift, winvert) \
-{ .id = snd_soc_dapm_micbias, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_micbias, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = NULL, .num_kcontrols = 0}
#define SND_SOC_DAPM_SWITCH(wname, wreg, wshift, winvert, wcontrols) \
-{ .id = snd_soc_dapm_switch, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_switch, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = 1}
#define SND_SOC_DAPM_MUX(wname, wreg, wshift, winvert, wcontrols) \
-{ .id = snd_soc_dapm_mux, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mux, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = 1}
#define SND_SOC_DAPM_DEMUX(wname, wreg, wshift, winvert, wcontrols) \
-{ .id = snd_soc_dapm_demux, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_demux, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = 1}
/* Simplified versions of above macros, assuming wncontrols = ARRAY_SIZE(wcontrols) */
#define SOC_PGA_ARRAY(wname, wreg, wshift, winvert,\
wcontrols) \
-{ .id = snd_soc_dapm_pga, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_pga, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols)}
#define SOC_MIXER_ARRAY(wname, wreg, wshift, winvert, \
wcontrols)\
-{ .id = snd_soc_dapm_mixer, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols)}
#define SOC_MIXER_NAMED_CTL_ARRAY(wname, wreg, wshift, winvert, \
wcontrols)\
-{ .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols)}
/* path domain with event - event handler must return 0 for success */
#define SND_SOC_DAPM_PGA_E(wname, wreg, wshift, winvert, wcontrols, \
wncontrols, wevent, wflags) \
-{ .id = snd_soc_dapm_pga, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_pga, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = wncontrols, \
.event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_OUT_DRV_E(wname, wreg, wshift, winvert, wcontrols, \
wncontrols, wevent, wflags) \
-{ .id = snd_soc_dapm_out_drv, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_out_drv, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = wncontrols, \
.event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_MIXER_E(wname, wreg, wshift, winvert, wcontrols, \
wncontrols, wevent, wflags) \
-{ .id = snd_soc_dapm_mixer, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = wncontrols, \
.event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_MIXER_NAMED_CTL_E(wname, wreg, wshift, winvert, \
wcontrols, wncontrols, wevent, wflags) \
-{ .id = snd_soc_dapm_mixer, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, \
.num_kcontrols = wncontrols, .event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_SWITCH_E(wname, wreg, wshift, winvert, wcontrols, \
wevent, wflags) \
-{ .id = snd_soc_dapm_switch, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_switch, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = 1, \
.event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_MUX_E(wname, wreg, wshift, winvert, wcontrols, \
wevent, wflags) \
-{ .id = snd_soc_dapm_mux, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mux, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = 1, \
.event = wevent, .event_flags = wflags}
@@ -176,101 +202,121 @@ struct soc_enum;
/* additional sequencing control within an event type */
#define SND_SOC_DAPM_PGA_S(wname, wsubseq, wreg, wshift, winvert, \
wevent, wflags) \
-{ .id = snd_soc_dapm_pga, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_pga, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.event = wevent, .event_flags = wflags, \
.subseq = wsubseq}
#define SND_SOC_DAPM_SUPPLY_S(wname, wsubseq, wreg, wshift, winvert, wevent, \
wflags) \
-{ .id = snd_soc_dapm_supply, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_supply, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.event = wevent, .event_flags = wflags, .subseq = wsubseq}
/* Simplified versions of above macros, assuming wncontrols = ARRAY_SIZE(wcontrols) */
#define SOC_PGA_E_ARRAY(wname, wreg, wshift, winvert, wcontrols, \
wevent, wflags) \
-{ .id = snd_soc_dapm_pga, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_pga, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols), \
.event = wevent, .event_flags = wflags}
#define SOC_MIXER_E_ARRAY(wname, wreg, wshift, winvert, wcontrols, \
wevent, wflags) \
-{ .id = snd_soc_dapm_mixer, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols), \
.event = wevent, .event_flags = wflags}
#define SOC_MIXER_NAMED_CTL_E_ARRAY(wname, wreg, wshift, winvert, \
wcontrols, wevent, wflags) \
-{ .id = snd_soc_dapm_mixer, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_mixer, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols), \
.event = wevent, .event_flags = wflags}
/* events that are pre and post DAPM */
#define SND_SOC_DAPM_PRE(wname, wevent) \
-{ .id = snd_soc_dapm_pre, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_pre, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
.event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_PRE_PMD}
#define SND_SOC_DAPM_POST(wname, wevent) \
-{ .id = snd_soc_dapm_post, .name = wname, .kcontrol_news = NULL, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_post, .name = wname, .kcontrol_news = NULL, \
.num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
.event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD}
/* stream domain */
#define SND_SOC_DAPM_AIF_IN(wname, stname, wchan, wreg, wshift, winvert) \
-{ .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
.channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), }
#define SND_SOC_DAPM_AIF_IN_E(wname, stname, wchan, wreg, wshift, winvert, \
wevent, wflags) \
-{ .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
.channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.event = wevent, .event_flags = wflags }
#define SND_SOC_DAPM_AIF_OUT(wname, stname, wchan, wreg, wshift, winvert) \
-{ .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
.channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), }
#define SND_SOC_DAPM_AIF_OUT_E(wname, stname, wchan, wreg, wshift, winvert, \
wevent, wflags) \
-{ .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
.channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.event = wevent, .event_flags = wflags }
#define SND_SOC_DAPM_DAC(wname, stname, wreg, wshift, winvert) \
-{ .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert) }
#define SND_SOC_DAPM_DAC_E(wname, stname, wreg, wshift, winvert, \
wevent, wflags) \
-{ .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_ADC(wname, stname, wreg, wshift, winvert) \
-{ .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), }
#define SND_SOC_DAPM_ADC_E(wname, stname, wreg, wshift, winvert, \
wevent, wflags) \
-{ .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_CLOCK_SUPPLY(wname) \
-{ .id = snd_soc_dapm_clock_supply, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_clock_supply, .name = wname, \
.reg = SND_SOC_NOPM, .event = dapm_clock_event, \
.event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD }
/* generic widgets */
#define SND_SOC_DAPM_REG(wid, wname, wreg, wshift, wmask, won_val, woff_val) \
-{ .id = wid, .name = wname, .kcontrol_news = NULL, .num_kcontrols = 0, \
+(struct snd_soc_dapm_widget) { \
+ .id = wid, .name = wname, .kcontrol_news = NULL, .num_kcontrols = 0, \
.reg = wreg, .shift = wshift, .mask = wmask, \
.on_val = won_val, .off_val = woff_val, }
#define SND_SOC_DAPM_SUPPLY(wname, wreg, wshift, winvert, wevent, wflags) \
-{ .id = snd_soc_dapm_supply, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_supply, .name = wname, \
SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
.event = wevent, .event_flags = wflags}
#define SND_SOC_DAPM_REGULATOR_SUPPLY(wname, wdelay, wflags) \
-{ .id = snd_soc_dapm_regulator_supply, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_regulator_supply, .name = wname, \
.reg = SND_SOC_NOPM, .shift = wdelay, .event = dapm_regulator_event, \
.event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD, \
.on_val = wflags}
#define SND_SOC_DAPM_PINCTRL(wname, active, sleep) \
-{ .id = snd_soc_dapm_pinctrl, .name = wname, \
+(struct snd_soc_dapm_widget) { \
+ .id = snd_soc_dapm_pinctrl, .name = wname, \
.priv = (&(struct snd_soc_dapm_pinctrl_priv) \
{ .active_state = active, .sleep_state = sleep,}), \
.reg = SND_SOC_NOPM, .event = dapm_pinctrl_event, \
--
2.40.1
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 <[email protected]>
---
sound/soc/codecs/Kconfig | 12 ++
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/audio-iio-aux.c | 334 +++++++++++++++++++++++++++++++
3 files changed, 348 insertions(+)
create mode 100644 sound/soc/codecs/audio-iio-aux.c
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..94fc8b6767d0
--- /dev/null
+++ b/sound/soc/codecs/audio-iio-aux.c
@@ -0,0 +1,334 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// ALSA SoC glue to use IIO devices as audio components
+//
+// Copyright 2023 CS GROUP France
+//
+// Author: Herve Codina <[email protected]>
+
+#include <linux/iio/consumer.h>
+#include <linux/minmax.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <sound/soc.h>
+#include <linux/string_helpers.h>
+#include <sound/tlv.h>
+
+struct audio_iio_aux_chan {
+ struct iio_channel *iio_chan;
+ const char *name;
+ int max;
+ int min;
+ bool is_invert_range;
+};
+
+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];
+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 *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 out;
+ }
+ output_name = kasprintf(GFP_KERNEL, "%s OUT", chan->name);
+ if (!output_name) {
+ ret = -ENOMEM;
+ goto out_free_input_name;
+ }
+ 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);
+out:
+ 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);
+ 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;
+}
+
+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 audio_iio_aux_chan *iio_aux_chan;
+ struct audio_iio_aux *iio_aux;
+ const char **names;
+ u32 *invert_ranges;
+ int count;
+ 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 = device_property_string_array_count(iio_aux->dev, "io-channel-names");
+ if (count < 0)
+ return dev_err_probe(iio_aux->dev, count, "failed to count io-channel-names\n");
+
+ iio_aux->num_chans = count;
+
+ iio_aux->chans = devm_kmalloc_array(iio_aux->dev, iio_aux->num_chans,
+ sizeof(*iio_aux->chans), GFP_KERNEL);
+ if (!iio_aux->chans)
+ return -ENOMEM;
+
+ names = kcalloc(iio_aux->num_chans, sizeof(*names), GFP_KERNEL);
+ if (!names)
+ return -ENOMEM;
+
+ invert_ranges = kcalloc(iio_aux->num_chans, sizeof(*invert_ranges), GFP_KERNEL);
+ if (!invert_ranges) {
+ ret = -ENOMEM;
+ goto out_free_names;
+ }
+
+ ret = device_property_read_string_array(iio_aux->dev, "io-channel-names",
+ names, iio_aux->num_chans);
+ if (ret < 0) {
+ dev_err_probe(iio_aux->dev, ret, "failed to read io-channel-names\n");
+ goto out_free_invert_ranges;
+ }
+
+ /*
+ * snd-control-invert-range is optional and can contain fewer items
+ * than the number of channel. Unset values default to 0.
+ */
+ count = device_property_count_u32(iio_aux->dev, "snd-control-invert-range");
+ if (count > 0) {
+ count = min_t(unsigned int, count, iio_aux->num_chans);
+ device_property_read_u32_array(iio_aux->dev, "snd-control-invert-range",
+ invert_ranges, count);
+ }
+
+ for (i = 0; i < iio_aux->num_chans; i++) {
+ iio_aux_chan = iio_aux->chans + i;
+ iio_aux_chan->name = names[i];
+ iio_aux_chan->is_invert_range = invert_ranges[i];
+
+ iio_aux_chan->iio_chan = devm_iio_channel_get(iio_aux->dev, iio_aux_chan->name);
+ if (IS_ERR(iio_aux_chan->iio_chan)) {
+ ret = PTR_ERR(iio_aux_chan->iio_chan);
+ dev_err_probe(iio_aux->dev, ret, "get IIO channel '%s' failed\n",
+ iio_aux_chan->name);
+ goto out_free_invert_ranges;
+ }
+ }
+
+ platform_set_drvdata(pdev, iio_aux);
+
+ ret = devm_snd_soc_register_component(iio_aux->dev, &audio_iio_aux_component_driver,
+ NULL, 0);
+out_free_invert_ranges:
+ kfree(invert_ranges);
+out_free_names:
+ kfree(names);
+ return ret;
+}
+
+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 <[email protected]>");
+MODULE_DESCRIPTION("IIO ALSA SoC aux driver");
+MODULE_LICENSE("GPL");
--
2.40.1
Industrial I/O devices can be present in the audio path.
These devices needs to be viewed as audio components in order to be
fully integrated in the audio path.
audio-iio-aux allows to consider these Industrial I/O devices as
auxliary audio devices.
Signed-off-by: Herve Codina <[email protected]>
---
.../bindings/sound/audio-iio-aux.yaml | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/audio-iio-aux.yaml
diff --git a/Documentation/devicetree/bindings/sound/audio-iio-aux.yaml b/Documentation/devicetree/bindings/sound/audio-iio-aux.yaml
new file mode 100644
index 000000000000..d3cc1ea4a175
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/audio-iio-aux.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/audio-iio-aux.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Audio IIO auxiliary
+
+maintainers:
+ - Herve Codina <[email protected]>
+
+description:
+ Auxiliary device based on Industrial I/O device channels
+
+allOf:
+ - $ref: dai-common.yaml#
+
+properties:
+ compatible:
+ const: audio-iio-aux
+
+ io-channels:
+ description:
+ Industrial I/O device channels used
+
+ io-channel-names:
+ description:
+ Industrial I/O channel names related to io-channels.
+ These names are used to provides sound controls, widgets and routes names.
+
+ snd-control-invert-range:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ A list of 0/1 flags defining whether or not the related channel is
+ inverted
+ items:
+ enum: [0, 1]
+ default: 0
+ description: |
+ Invert the sound control value compared to the IIO channel raw value.
+ - 1: The related sound control value is inverted meaning that the
+ minimum sound control value correspond to the maximum IIO channel
+ raw value and the maximum sound control value correspond to the
+ minimum IIO channel raw value.
+ - 0: The related sound control value is not inverted meaning that the
+ minimum (resp maximum) sound control value correspond to the
+ minimum (resp maximum) IIO channel raw value.
+
+required:
+ - compatible
+ - io-channels
+ - io-channel-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ iio-aux {
+ compatible = "audio-iio-aux";
+ io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>;
+ io-channel-names = "CH0", "CH1", "CH2", "CH3";
+ /* Invert CH1 and CH2 */
+ snd-control-invert-range = <0 1 1 0>;
+ };
--
2.40.1
Use max_array() to get the maximum value from an array instead of a
custom local loop.
Signed-off-by: Herve Codina <[email protected]>
---
drivers/iio/inkern.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index ce537b4ca6ca..ae1a41d3a559 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -8,6 +8,7 @@
#include <linux/property.h>
#include <linux/slab.h>
#include <linux/mutex.h>
+#include <linux/minmax.h>
#include <linux/iio/iio.h>
#include <linux/iio/iio-opaque.h>
@@ -875,11 +876,7 @@ static int iio_channel_read_max(struct iio_channel *chan,
return -EINVAL;
switch (*type) {
case IIO_VAL_INT:
- *val = vals[--length];
- while (length) {
- if (vals[--length] > *val)
- *val = vals[length];
- }
+ *val = max_array(vals, length);
break;
default:
/* FIXME: learn about max for other iio values */
--
2.40.1
The current implementation returns the error code as part of the
default switch case.
This can lead to returning an incorrect positive value in case of
iio_avail_type enum entries evolution.
In order to avoid this case, be more strict in error checking.
Signed-off-by: Herve Codina <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
---
drivers/iio/inkern.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 872fd5c24147..f738db9a0c04 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -858,6 +858,9 @@ static int iio_channel_read_max(struct iio_channel *chan,
val2 = &unused;
ret = iio_channel_read_avail(chan, &vals, type, &length, info);
+ if (ret < 0)
+ return ret;
+
switch (ret) {
case IIO_AVAIL_RANGE:
switch (*type) {
@@ -888,7 +891,7 @@ static int iio_channel_read_max(struct iio_channel *chan,
return 0;
default:
- return ret;
+ return -EINVAL;
}
}
--
2.40.1
The additional-devs subnode allows to declared some virtual devices
as sound card children.
These virtual devices can then be used by the sound card and so be
present in the audio path.
The first virtual device supported is the audio IIO auxiliary device
in order to support an IIO device as an audio auxiliary device.
Signed-off-by: Herve Codina <[email protected]>
---
.../bindings/sound/simple-card.yaml | 53 +++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
index b05e05c81cc4..59ac2d1d1ccf 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.yaml
+++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
@@ -148,6 +148,15 @@ definitions:
required:
- sound-dai
+ additional-devs:
+ type: object
+ description:
+ Additional devices used by the simple audio card.
+ patternProperties:
+ '^iio-aux(-.+)?$':
+ type: object
+ $ref: audio-iio-aux.yaml#
+
properties:
compatible:
contains:
@@ -187,6 +196,8 @@ properties:
$ref: "#/definitions/mclk-fs"
simple-audio-card,aux-devs:
$ref: "#/definitions/aux-devs"
+ simple-audio-card,additional-devs:
+ $ref: "#/definitions/additional-devs"
simple-audio-card,convert-rate:
$ref: "#/definitions/convert-rate"
simple-audio-card,convert-channels:
@@ -359,6 +370,48 @@ examples:
};
};
+# --------------------
+# route audio to/from a codec through an amplifier
+# designed with a potentiometer driven by IIO:
+# --------------------
+ - |
+ sound {
+ compatible = "simple-audio-card";
+
+ simple-audio-card,aux-devs = <&_in>, <&_out>;
+ simple-audio-card,routing =
+ "CODEC LEFTIN", "AMP_IN LEFT OUT",
+ "CODEC RIGHTIN", "AMP_IN RIGHT OUT",
+ "AMP_OUT LEFT IN", "CODEC LEFTOUT",
+ "AMP_OUT RIGHT IN", "CODEC RIGHTOUT";
+
+ simple-audio-card,additional-devs {
+ amp_out: iio-aux-out {
+ compatible = "audio-iio-aux";
+ io-channels = <&pot_out 0>, <&pot_out 1>;
+ io-channel-names = "LEFT", "RIGHT";
+ snd-control-invert-range = <1 1>;
+ sound-name-prefix = "AMP_OUT";
+ };
+
+ amp_in: iio_aux-in {
+ compatible = "audio-iio-aux";
+ io-channels = <&pot_in 0>, <&pot_in 1>;
+ io-channel-names = "LEFT", "RIGHT";
+ sound-name-prefix = "AMP_IN";
+ };
+ };
+
+ simple-audio-card,cpu {
+ sound-dai = <&cpu>;
+ };
+
+ simple-audio-card,codec {
+ sound-dai = <&codec>;
+ clocks = <&clocks>;
+ };
+ };
+
# --------------------
# Sampling Rate Conversion
# --------------------
--
2.40.1
The code uses a local variable to initialize a null pointer in order to
avoid accessing this null pointer later on.
Simply removed the 'unused' variable and check for the null pointer just
before accessing it.
Signed-off-by: Herve Codina <[email protected]>
---
drivers/iio/inkern.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index f738db9a0c04..ce537b4ca6ca 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -849,14 +849,10 @@ static int iio_channel_read_max(struct iio_channel *chan,
int *val, int *val2, int *type,
enum iio_chan_info_enum info)
{
- int unused;
const int *vals;
int length;
int ret;
- if (!val2)
- val2 = &unused;
-
ret = iio_channel_read_avail(chan, &vals, type, &length, info);
if (ret < 0)
return ret;
@@ -869,7 +865,8 @@ static int iio_channel_read_max(struct iio_channel *chan,
break;
default:
*val = vals[4];
- *val2 = vals[5];
+ if (val2)
+ *val2 = vals[5];
}
return 0;
--
2.40.1
On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
>
> The current implementation returns the error code as part of the
> default switch case.
> This can lead to returning an incorrect positive value in case of
> iio_avail_type enum entries evolution.
>
> In order to avoid this case, be more strict in error checking.
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Herve Codina <[email protected]>
> Acked-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/inkern.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 872fd5c24147..f738db9a0c04 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -858,6 +858,9 @@ static int iio_channel_read_max(struct iio_channel *chan,
> val2 = &unused;
>
> ret = iio_channel_read_avail(chan, &vals, type, &length, info);
> + if (ret < 0)
> + return ret;
> +
> switch (ret) {
> case IIO_AVAIL_RANGE:
> switch (*type) {
> @@ -888,7 +891,7 @@ static int iio_channel_read_max(struct iio_channel *chan,
> return 0;
>
> default:
> - return ret;
> + return -EINVAL;
> }
> }
>
> --
> 2.40.1
>
--
With Best Regards,
Andy Shevchenko
On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
>
> The code uses a local variable to initialize a null pointer in order to
> avoid accessing this null pointer later on.
>
> Simply removed the 'unused' variable and check for the null pointer just
> before accessing it.
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> drivers/iio/inkern.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index f738db9a0c04..ce537b4ca6ca 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -849,14 +849,10 @@ static int iio_channel_read_max(struct iio_channel *chan,
> int *val, int *val2, int *type,
> enum iio_chan_info_enum info)
> {
> - int unused;
> const int *vals;
> int length;
> int ret;
>
> - if (!val2)
> - val2 = &unused;
> -
> ret = iio_channel_read_avail(chan, &vals, type, &length, info);
> if (ret < 0)
> return ret;
> @@ -869,7 +865,8 @@ static int iio_channel_read_max(struct iio_channel *chan,
> break;
> default:
> *val = vals[4];
> - *val2 = vals[5];
> + if (val2)
> + *val2 = vals[5];
> }
> return 0;
>
> --
> 2.40.1
>
--
With Best Regards,
Andy Shevchenko
On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
>
> Introduce min_array() (resp max_array()) in order to get the
> minimal (resp maximum) of values present in an array.
Some comments below, after addressing them,
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> include/linux/minmax.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/include/linux/minmax.h b/include/linux/minmax.h
> index 396df1121bff..37a211f22404 100644
> --- a/include/linux/minmax.h
> +++ b/include/linux/minmax.h
> @@ -133,6 +133,32 @@
> */
> #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >)
>
> +#define __minmax_array(op, array, len) ({ \
Maybe it's my MUA, maybe the code contains spaces, can you switch to
TABs if it's the case?
> + typeof(array) __array = (array); \
We have __must_be_array()
You will need to fix the inclusions in minmax.h at the same time, it needs
linux/build_bug.h (which includes compiler.h needed for __UNIQUE_ID()
and for the above mentioned one).
> + typeof(len) __len = (len); \
> + typeof(*__array + 0) __element = __array[--__len]; \
After above, this can be written as __array[0].
> + while (__len--) \
> + __element = op(__element, __array[__len]); \
> + __element; })
> +
> +/**
> + * min_array - return minimum of values present in an array
> + * @array: array
> + * @len: array length
> + *
> + * Note that @len must not be zero (empty array).
> + */
> +#define min_array(array, len) __minmax_array(min, array, len)
> +
> +/**
> + * max_array - return maximum of values present in an array
> + * @array: array
> + * @len: array length
> + *
> + * Note that @len must not be zero (empty array).
> + */
> +#define max_array(array, len) __minmax_array(max, array, len)
> +
> /**
> * clamp_t - return a value clamped to a given range using a given type
> * @type: the type of variable to use
--
With Best Regards,
Andy Shevchenko
On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
>
> Use max_array() to get the maximum value from an array instead of a
> custom local loop.
Looks really good, thank you!
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> drivers/iio/inkern.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index ce537b4ca6ca..ae1a41d3a559 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -8,6 +8,7 @@
> #include <linux/property.h>
> #include <linux/slab.h>
> #include <linux/mutex.h>
> +#include <linux/minmax.h>
>
> #include <linux/iio/iio.h>
> #include <linux/iio/iio-opaque.h>
> @@ -875,11 +876,7 @@ static int iio_channel_read_max(struct iio_channel *chan,
> return -EINVAL;
> switch (*type) {
> case IIO_VAL_INT:
> - *val = vals[--length];
> - while (length) {
> - if (vals[--length] > *val)
> - *val = vals[length];
> - }
> + *val = max_array(vals, length);
> break;
> default:
> /* FIXME: learn about max for other iio values */
> --
> 2.40.1
>
--
With Best Regards,
Andy Shevchenko
On Mon, Jun 12, 2023 at 5:12 PM Andy Shevchenko
<[email protected]> wrote:
> On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
...
> > #include <linux/property.h>
> > #include <linux/slab.h>
> > #include <linux/mutex.h>
> > +#include <linux/minmax.h>
But can you try to preserve order (to some extent, perhaps invisible
here context is even better, I would assume mutex was added
unordered)?
> > #include <linux/iio/iio.h>
> > #include <linux/iio/iio-opaque.h>
--
With Best Regards,
Andy Shevchenko
On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
>
> The SND_SOC_DAPM_* helpers family are used to build widgets array in a
> static way.
>
> Convert them to return a compound litteral in order to use them in both
literal
> static and dynamic way.
> With this conversion, the different SND_SOC_DAPM_* parameters can be
> computed by the code and the widget can be built based on this parameter
> computation.
> static int create_widget(char *input_name)
> {
> struct snd_soc_dapm_widget widget;
> char name*;
> ...
> name = input_name;
> if (!name)
> name = "default";
>
> widget = SND_SOC_DAPM_INPUT(name);
> ...
> }
Suggested-by:?
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> include/sound/soc-dapm.h | 138 ++++++++++++++++++++++++++-------------
> 1 file changed, 92 insertions(+), 46 deletions(-)
>
> diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h
> index 87f8e1793af1..2e38dff16779 100644
> --- a/include/sound/soc-dapm.h
> +++ b/include/sound/soc-dapm.h
> @@ -42,36 +42,45 @@ struct soc_enum;
>
> /* codec domain */
> #define SND_SOC_DAPM_VMID(wname) \
> -{ .id = snd_soc_dapm_vmid, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_vmid, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0}
>
> /* platform domain */
> #define SND_SOC_DAPM_SIGGEN(wname) \
> -{ .id = snd_soc_dapm_siggen, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_siggen, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM }
> #define SND_SOC_DAPM_SINK(wname) \
> -{ .id = snd_soc_dapm_sink, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_sink, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM }
> #define SND_SOC_DAPM_INPUT(wname) \
> -{ .id = snd_soc_dapm_input, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_input, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM }
> #define SND_SOC_DAPM_OUTPUT(wname) \
> -{ .id = snd_soc_dapm_output, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_output, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM }
> #define SND_SOC_DAPM_MIC(wname, wevent) \
> -{ .id = snd_soc_dapm_mic, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mic, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
> .event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD}
> #define SND_SOC_DAPM_HP(wname, wevent) \
> -{ .id = snd_soc_dapm_hp, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_hp, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
> .event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD}
> #define SND_SOC_DAPM_SPK(wname, wevent) \
> -{ .id = snd_soc_dapm_spk, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_spk, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
> .event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD}
> #define SND_SOC_DAPM_LINE(wname, wevent) \
> -{ .id = snd_soc_dapm_line, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_line, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
> .event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD}
>
> @@ -82,93 +91,110 @@ struct soc_enum;
> /* path domain */
> #define SND_SOC_DAPM_PGA(wname, wreg, wshift, winvert,\
> wcontrols, wncontrols) \
> -{ .id = snd_soc_dapm_pga, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_pga, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
> #define SND_SOC_DAPM_OUT_DRV(wname, wreg, wshift, winvert,\
> wcontrols, wncontrols) \
> -{ .id = snd_soc_dapm_out_drv, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_out_drv, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
> #define SND_SOC_DAPM_MIXER(wname, wreg, wshift, winvert, \
> wcontrols, wncontrols)\
> -{ .id = snd_soc_dapm_mixer, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
> #define SND_SOC_DAPM_MIXER_NAMED_CTL(wname, wreg, wshift, winvert, \
> wcontrols, wncontrols)\
> -{ .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = wncontrols}
> /* DEPRECATED: use SND_SOC_DAPM_SUPPLY */
> #define SND_SOC_DAPM_MICBIAS(wname, wreg, wshift, winvert) \
> -{ .id = snd_soc_dapm_micbias, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_micbias, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = NULL, .num_kcontrols = 0}
> #define SND_SOC_DAPM_SWITCH(wname, wreg, wshift, winvert, wcontrols) \
> -{ .id = snd_soc_dapm_switch, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_switch, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = 1}
> #define SND_SOC_DAPM_MUX(wname, wreg, wshift, winvert, wcontrols) \
> -{ .id = snd_soc_dapm_mux, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mux, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = 1}
> #define SND_SOC_DAPM_DEMUX(wname, wreg, wshift, winvert, wcontrols) \
> -{ .id = snd_soc_dapm_demux, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_demux, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = 1}
>
> /* Simplified versions of above macros, assuming wncontrols = ARRAY_SIZE(wcontrols) */
> #define SOC_PGA_ARRAY(wname, wreg, wshift, winvert,\
> wcontrols) \
> -{ .id = snd_soc_dapm_pga, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_pga, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols)}
> #define SOC_MIXER_ARRAY(wname, wreg, wshift, winvert, \
> wcontrols)\
> -{ .id = snd_soc_dapm_mixer, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols)}
> #define SOC_MIXER_NAMED_CTL_ARRAY(wname, wreg, wshift, winvert, \
> wcontrols)\
> -{ .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer_named_ctl, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols)}
>
> /* path domain with event - event handler must return 0 for success */
> #define SND_SOC_DAPM_PGA_E(wname, wreg, wshift, winvert, wcontrols, \
> wncontrols, wevent, wflags) \
> -{ .id = snd_soc_dapm_pga, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_pga, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = wncontrols, \
> .event = wevent, .event_flags = wflags}
> #define SND_SOC_DAPM_OUT_DRV_E(wname, wreg, wshift, winvert, wcontrols, \
> wncontrols, wevent, wflags) \
> -{ .id = snd_soc_dapm_out_drv, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_out_drv, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = wncontrols, \
> .event = wevent, .event_flags = wflags}
> #define SND_SOC_DAPM_MIXER_E(wname, wreg, wshift, winvert, wcontrols, \
> wncontrols, wevent, wflags) \
> -{ .id = snd_soc_dapm_mixer, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = wncontrols, \
> .event = wevent, .event_flags = wflags}
> #define SND_SOC_DAPM_MIXER_NAMED_CTL_E(wname, wreg, wshift, winvert, \
> wcontrols, wncontrols, wevent, wflags) \
> -{ .id = snd_soc_dapm_mixer, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, \
> .num_kcontrols = wncontrols, .event = wevent, .event_flags = wflags}
> #define SND_SOC_DAPM_SWITCH_E(wname, wreg, wshift, winvert, wcontrols, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_switch, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_switch, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = 1, \
> .event = wevent, .event_flags = wflags}
> #define SND_SOC_DAPM_MUX_E(wname, wreg, wshift, winvert, wcontrols, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_mux, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mux, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = 1, \
> .event = wevent, .event_flags = wflags}
> @@ -176,101 +202,121 @@ struct soc_enum;
> /* additional sequencing control within an event type */
> #define SND_SOC_DAPM_PGA_S(wname, wsubseq, wreg, wshift, winvert, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_pga, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_pga, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .event = wevent, .event_flags = wflags, \
> .subseq = wsubseq}
> #define SND_SOC_DAPM_SUPPLY_S(wname, wsubseq, wreg, wshift, winvert, wevent, \
> wflags) \
> -{ .id = snd_soc_dapm_supply, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_supply, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .event = wevent, .event_flags = wflags, .subseq = wsubseq}
>
> /* Simplified versions of above macros, assuming wncontrols = ARRAY_SIZE(wcontrols) */
> #define SOC_PGA_E_ARRAY(wname, wreg, wshift, winvert, wcontrols, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_pga, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_pga, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols), \
> .event = wevent, .event_flags = wflags}
> #define SOC_MIXER_E_ARRAY(wname, wreg, wshift, winvert, wcontrols, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_mixer, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols), \
> .event = wevent, .event_flags = wflags}
> #define SOC_MIXER_NAMED_CTL_E_ARRAY(wname, wreg, wshift, winvert, \
> wcontrols, wevent, wflags) \
> -{ .id = snd_soc_dapm_mixer, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_mixer, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .kcontrol_news = wcontrols, .num_kcontrols = ARRAY_SIZE(wcontrols), \
> .event = wevent, .event_flags = wflags}
>
> /* events that are pre and post DAPM */
> #define SND_SOC_DAPM_PRE(wname, wevent) \
> -{ .id = snd_soc_dapm_pre, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_pre, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
> .event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_PRE_PMD}
> #define SND_SOC_DAPM_POST(wname, wevent) \
> -{ .id = snd_soc_dapm_post, .name = wname, .kcontrol_news = NULL, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_post, .name = wname, .kcontrol_news = NULL, \
> .num_kcontrols = 0, .reg = SND_SOC_NOPM, .event = wevent, \
> .event_flags = SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_POST_PMD}
>
> /* stream domain */
> #define SND_SOC_DAPM_AIF_IN(wname, stname, wchan, wreg, wshift, winvert) \
> -{ .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
> .channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), }
> #define SND_SOC_DAPM_AIF_IN_E(wname, stname, wchan, wreg, wshift, winvert, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_aif_in, .name = wname, .sname = stname, \
> .channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .event = wevent, .event_flags = wflags }
> #define SND_SOC_DAPM_AIF_OUT(wname, stname, wchan, wreg, wshift, winvert) \
> -{ .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
> .channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), }
> #define SND_SOC_DAPM_AIF_OUT_E(wname, stname, wchan, wreg, wshift, winvert, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_aif_out, .name = wname, .sname = stname, \
> .channel = wchan, SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .event = wevent, .event_flags = wflags }
> #define SND_SOC_DAPM_DAC(wname, stname, wreg, wshift, winvert) \
> -{ .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert) }
> #define SND_SOC_DAPM_DAC_E(wname, stname, wreg, wshift, winvert, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_dac, .name = wname, .sname = stname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .event = wevent, .event_flags = wflags}
>
> #define SND_SOC_DAPM_ADC(wname, stname, wreg, wshift, winvert) \
> -{ .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), }
> #define SND_SOC_DAPM_ADC_E(wname, stname, wreg, wshift, winvert, \
> wevent, wflags) \
> -{ .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_adc, .name = wname, .sname = stname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .event = wevent, .event_flags = wflags}
> #define SND_SOC_DAPM_CLOCK_SUPPLY(wname) \
> -{ .id = snd_soc_dapm_clock_supply, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_clock_supply, .name = wname, \
> .reg = SND_SOC_NOPM, .event = dapm_clock_event, \
> .event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD }
>
> /* generic widgets */
> #define SND_SOC_DAPM_REG(wid, wname, wreg, wshift, wmask, won_val, woff_val) \
> -{ .id = wid, .name = wname, .kcontrol_news = NULL, .num_kcontrols = 0, \
> +(struct snd_soc_dapm_widget) { \
> + .id = wid, .name = wname, .kcontrol_news = NULL, .num_kcontrols = 0, \
> .reg = wreg, .shift = wshift, .mask = wmask, \
> .on_val = won_val, .off_val = woff_val, }
> #define SND_SOC_DAPM_SUPPLY(wname, wreg, wshift, winvert, wevent, wflags) \
> -{ .id = snd_soc_dapm_supply, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_supply, .name = wname, \
> SND_SOC_DAPM_INIT_REG_VAL(wreg, wshift, winvert), \
> .event = wevent, .event_flags = wflags}
> #define SND_SOC_DAPM_REGULATOR_SUPPLY(wname, wdelay, wflags) \
> -{ .id = snd_soc_dapm_regulator_supply, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_regulator_supply, .name = wname, \
> .reg = SND_SOC_NOPM, .shift = wdelay, .event = dapm_regulator_event, \
> .event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD, \
> .on_val = wflags}
> #define SND_SOC_DAPM_PINCTRL(wname, active, sleep) \
> -{ .id = snd_soc_dapm_pinctrl, .name = wname, \
> +(struct snd_soc_dapm_widget) { \
> + .id = snd_soc_dapm_pinctrl, .name = wname, \
> .priv = (&(struct snd_soc_dapm_pinctrl_priv) \
> { .active_state = active, .sleep_state = sleep,}), \
> .reg = SND_SOC_NOPM, .event = dapm_pinctrl_event, \
> --
> 2.40.1
>
--
With Best Regards,
Andy Shevchenko
On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> 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
auxiliary
> audio devices and allows to control them using mixer controls.
allows one to control?
...
> +#include <linux/iio/consumer.h>
> +#include <linux/minmax.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <sound/soc.h>
> +#include <linux/string_helpers.h>
Perhaps a bit of order? And maybe a blank line between linux/* and sound/*?
> +#include <sound/tlv.h>
...
> + struct snd_kcontrol_new control = {0};
0 is not 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 *input_name = NULL;
> + char *output_name = NULL;
> + char *pga_name = NULL;
Now these assignments can be dropped.
> + int ret;
> +
> + input_name = kasprintf(GFP_KERNEL, "%s IN", chan->name);
> + if (!input_name) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + output_name = kasprintf(GFP_KERNEL, "%s OUT", chan->name);
> + if (!output_name) {
> + ret = -ENOMEM;
> + goto out_free_input_name;
> + }
> + 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);
> +out:
Seems redundant label, you can return directly.
> + return ret;
> +}
...
With
struct device *dev = &pdev->dev;
> + iio_aux = devm_kzalloc(&pdev->dev, sizeof(*iio_aux), GFP_KERNEL);
> + if (!iio_aux)
> + return -ENOMEM;
You can make this kind of call neater.
...
> + iio_aux->dev = &pdev->dev;
> +
> + count = device_property_string_array_count(iio_aux->dev, "io-channel-names");
It's not recommended to switch over inside one function to a new
pointer even if they are the same. With dev here as a parameters it's
much easier to understand where the property is taken from.
> + if (count < 0)
> + return dev_err_probe(iio_aux->dev, count, "failed to count io-channel-names\n");
Ditto. And for the rest.
...
> + iio_aux->chans = devm_kmalloc_array(iio_aux->dev, iio_aux->num_chans,
Esp. in this case, it will add confusion, because we have been having
the object lifetime issues with devm_*() APIs from the past and
then...
> + sizeof(*iio_aux->chans), GFP_KERNEL);
> + if (!iio_aux->chans)
> + return -ENOMEM;
...
> + /*
> + * snd-control-invert-range is optional and can contain fewer items
> + * than the number of channel. Unset values default to 0.
channels
> + */
> + count = device_property_count_u32(iio_aux->dev, "snd-control-invert-range");
> + if (count > 0) {
> + count = min_t(unsigned int, count, iio_aux->num_chans);
> + device_property_read_u32_array(iio_aux->dev, "snd-control-invert-range",
> + invert_ranges, count);
Probably good to check an error, while it might be almost a dead code.
If something happens during this we will at least know.
> + }
--
With Best Regards,
Andy Shevchenko
On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
>
> A helper, iio_read_max_channel_raw() exists to read the available
> maximum raw value of a channel but nothing similar exists to read the
> available minimum raw value.
>
> This new helper, iio_read_min_channel_raw(), fills the hole and can be
> used for reading the available minimum raw value of a channel.
> It is fully based on the existing iio_read_max_channel_raw().
Reviewed-by: Andy Shevchenko <[email protected]>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> drivers/iio/inkern.c | 63 ++++++++++++++++++++++++++++++++++++
> include/linux/iio/consumer.h | 12 +++++++
> 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 4331e74191cf..4f383a5ef59f 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -909,6 +909,69 @@ int iio_read_max_channel_raw(struct iio_channel *chan, int *val)
> }
> EXPORT_SYMBOL_GPL(iio_read_max_channel_raw);
>
> +static int iio_channel_read_min(struct iio_channel *chan,
> + int *val, int *val2, int *type,
> + enum iio_chan_info_enum info)
> +{
> + const int *vals;
> + int length;
> + int ret;
> +
> + ret = iio_channel_read_avail(chan, &vals, type, &length, info);
> + if (ret < 0)
> + return ret;
> +
> + switch (ret) {
> + case IIO_AVAIL_RANGE:
> + switch (*type) {
> + case IIO_VAL_INT:
> + *val = vals[0];
> + break;
> + default:
> + *val = vals[0];
> + if (val2)
> + *val2 = vals[1];
> + }
> + return 0;
> +
> + case IIO_AVAIL_LIST:
> + if (length <= 0)
> + return -EINVAL;
> + switch (*type) {
> + case IIO_VAL_INT:
> + *val = min_array(vals, length);
> + break;
> + default:
> + /* TODO: learn about min for other iio values */
> + return -EINVAL;
> + }
> + return 0;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +int iio_read_min_channel_raw(struct iio_channel *chan, int *val)
> +{
> + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
> + int ret;
> + int type;
> +
> + mutex_lock(&iio_dev_opaque->info_exist_lock);
> + if (!chan->indio_dev->info) {
> + ret = -ENODEV;
> + goto err_unlock;
> + }
> +
> + ret = iio_channel_read_min(chan, val, NULL, &type, IIO_CHAN_INFO_RAW);
> +err_unlock:
> + mutex_unlock(&iio_dev_opaque->info_exist_lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_min_channel_raw);
> +
> int iio_get_channel_type(struct iio_channel *chan, enum iio_chan_type *type)
> {
> struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(chan->indio_dev);
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index f536820b9cf2..e9910b41d48e 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -301,6 +301,18 @@ int iio_write_channel_raw(struct iio_channel *chan, int val);
> */
> int iio_read_max_channel_raw(struct iio_channel *chan, int *val);
>
> +/**
> + * iio_read_min_channel_raw() - read minimum available raw value from a given
> + * channel, i.e. the minimum possible value.
> + * @chan: The channel being queried.
> + * @val: Value read back.
> + *
> + * Note, if standard units are required, raw reads from iio channels
> + * need the offset (default 0) and scale (default 1) to be applied
> + * as (raw + offset) * scale.
> + */
> +int iio_read_min_channel_raw(struct iio_channel *chan, int *val);
> +
> /**
> * iio_read_avail_channel_raw() - read available raw values from a given channel
> * @chan: The channel being queried.
> --
> 2.40.1
>
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Mon, 12 Jun 2023 17:10:40 +0300
Andy Shevchenko <[email protected]> wrote:
> On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
> >
> > Introduce min_array() (resp max_array()) in order to get the
> > minimal (resp maximum) of values present in an array.
>
> Some comments below, after addressing them,
> Reviewed-by: Andy Shevchenko <[email protected]>
>
> > Signed-off-by: Herve Codina <[email protected]>
> > ---
> > include/linux/minmax.h | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/include/linux/minmax.h b/include/linux/minmax.h
> > index 396df1121bff..37a211f22404 100644
> > --- a/include/linux/minmax.h
> > +++ b/include/linux/minmax.h
> > @@ -133,6 +133,32 @@
> > */
> > #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >)
> >
> > +#define __minmax_array(op, array, len) ({ \
>
> Maybe it's my MUA, maybe the code contains spaces, can you switch to
> TABs if it's the case?
>
> > + typeof(array) __array = (array); \
>
> We have __must_be_array()
Using __must_be_array() will lead to some failure.
Indeed, we can have:
--- 8< ---
int *buff
...
min = min_array(buff, nb_item);
--- 8< ---
In this case, __must_be_array() will report that buff is not an array.
To avoid any confusion, what do you think if I renamed {min,max}_array()
to {min,max}_buffer() and replace __array by __buff and use *(__buff + xxx)
instead of array[xxx] in the macro.
This will lead to:
--- 8< ---
#define __minmax_buffer(op, buff, len) ({ \
typeof(buff) __buff = (buff); \
typeof(len) __len = (len); \
typeof(*buff + 0) __element = *(__buff + --__len); \
while (__len--) \
__element = op(__element, *(__buff + __len])); \
__element; })
#define min_buffer(buffer, len) __minmax_array(min, buffer, len)
#define max_buffer(buffer, len) __minmax_array(max, buffer, len)
--- 8< ---
Regards,
Hervé
>
> You will need to fix the inclusions in minmax.h at the same time, it needs
> linux/build_bug.h (which includes compiler.h needed for __UNIQUE_ID()
> and for the above mentioned one).
>
> > + typeof(len) __len = (len); \
> > + typeof(*__array + 0) __element = __array[--__len]; \
>
> After above, this can be written as __array[0].
>
> > + while (__len--) \
> > + __element = op(__element, __array[__len]); \
> > + __element; })
> > +
> > +/**
> > + * min_array - return minimum of values present in an array
> > + * @array: array
> > + * @len: array length
> > + *
> > + * Note that @len must not be zero (empty array).
> > + */
> > +#define min_array(array, len) __minmax_array(min, array, len)
> > +
> > +/**
> > + * max_array - return maximum of values present in an array
> > + * @array: array
> > + * @len: array length
> > + *
> > + * Note that @len must not be zero (empty array).
> > + */
> > +#define max_array(array, len) __minmax_array(max, array, len)
> > +
> > /**
> > * clamp_t - return a value clamped to a given range using a given type
> > * @type: the type of variable to use
>
Hi Andy,
On Mon, 12 Jun 2023 17:37:00 +0300
Andy Shevchenko <[email protected]> wrote:
> ...
>
> > + struct snd_kcontrol_new control = {0};
>
> 0 is not needed.
Not for this one.
The variable is in stack.
Some of the structure members will be set in the code but we need to ensure
that all others are set to 0.
The full context:
--- 8< ---
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);
}
--- 8< ---
Thanks for the review,
Hervé
On Tue, Jun 13, 2023 at 12:37 PM Herve Codina <[email protected]> wrote:
> On Mon, 12 Jun 2023 17:37:00 +0300
> Andy Shevchenko <[email protected]> wrote:
...
> > > + struct snd_kcontrol_new control = {0};
> >
> > 0 is not needed.
>
> Not for this one.
>
> The variable is in stack.
> Some of the structure members will be set in the code but we need to ensure
> that all others are set to 0.
Yes, and as I said, 0 is not needed. Compiler assumes that if you just
use plain {}.
--
With Best Regards,
Andy Shevchenko
On Tue, 13 Jun 2023 16:24:58 +0300
Andy Shevchenko <[email protected]> wrote:
> On Tue, Jun 13, 2023 at 12:37 PM Herve Codina <[email protected]> wrote:
> > On Mon, 12 Jun 2023 17:37:00 +0300
> > Andy Shevchenko <[email protected]> wrote:
>
> ...
>
> > > > + struct snd_kcontrol_new control = {0};
> > >
> > > 0 is not needed.
> >
> > Not for this one.
> >
> > The variable is in stack.
> > Some of the structure members will be set in the code but we need to ensure
> > that all others are set to 0.
>
> Yes, and as I said, 0 is not needed. Compiler assumes that if you just
> use plain {}.
>
My bad, sorry, I misunderstood.
Will be update to an empty {} in the next iteration.
Thanks,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Tue, Jun 13, 2023 at 11:00 AM Herve Codina <[email protected]> wrote:
> On Mon, 12 Jun 2023 17:10:40 +0300
> Andy Shevchenko <[email protected]> wrote:
> > On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
> > >
> > > Introduce min_array() (resp max_array()) in order to get the
> > > minimal (resp maximum) of values present in an array.
> >
> > Some comments below, after addressing them,
> > Reviewed-by: Andy Shevchenko <[email protected]>
...
> > > + typeof(array) __array = (array); \
> >
> > We have __must_be_array()
>
> Using __must_be_array() will lead to some failure.
> Indeed, we can have:
> --- 8< ---
> int *buff
> ...
> min = min_array(buff, nb_item);
> --- 8< ---
>
> In this case, __must_be_array() will report that buff is not an array.
Oh, I missed that.
> To avoid any confusion, what do you think if I renamed {min,max}_array()
> to {min,max}_buffer() and replace __array by __buff and use *(__buff + xxx)
> instead of array[xxx] in the macro.
But functionally it's still against an array.
I would stick with "array" in the name, but add a comment why
__must_be_array() is not used. If we need a stricter variant, we may
add a new wrapper with that check. That said, I think we can use
__array[0] and similar indexed accesses.
--
With Best Regards,
Andy Shevchenko
On Tue, 13 Jun 2023 20:08:08 +0300
Andy Shevchenko <[email protected]> wrote:
> On Tue, Jun 13, 2023 at 11:00 AM Herve Codina <[email protected]> wrote:
> > On Mon, 12 Jun 2023 17:10:40 +0300
> > Andy Shevchenko <[email protected]> wrote:
> > > On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <[email protected]> wrote:
> > > >
> > > > Introduce min_array() (resp max_array()) in order to get the
> > > > minimal (resp maximum) of values present in an array.
> > >
> > > Some comments below, after addressing them,
> > > Reviewed-by: Andy Shevchenko <[email protected]>
>
> ...
>
> > > > + typeof(array) __array = (array); \
> > >
> > > We have __must_be_array()
> >
> > Using __must_be_array() will lead to some failure.
> > Indeed, we can have:
> > --- 8< ---
> > int *buff
> > ...
> > min = min_array(buff, nb_item);
> > --- 8< ---
> >
> > In this case, __must_be_array() will report that buff is not an array.
>
> Oh, I missed that.
>
> > To avoid any confusion, what do you think if I renamed {min,max}_array()
> > to {min,max}_buffer() and replace __array by __buff and use *(__buff + xxx)
> > instead of array[xxx] in the macro.
>
> But functionally it's still against an array.
>
> I would stick with "array" in the name, but add a comment why
> __must_be_array() is not used. If we need a stricter variant, we may
> add a new wrapper with that check. That said, I think we can use
> __array[0] and similar indexed accesses.
>
Right, I will provide an updated version on the next iteration.
Thanks for your feedback.
Hervé