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 v5 series mainly:
- Fixes {min,max}_array macros
Best regards,
Hervé
[1] https://lore.kernel.org/linux-kernel/[email protected]/
[2] https://lore.kernel.org/linux-kernel/[email protected]/
Changes v4 -> v5
- Patches 1, 2, 3, 4, 5, 9, 10, 11, 12, 13
No changes.
- Patch 6
Fix commit log.
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'
- Patch 7
Fix the macros to be able to use them with:
- an array defined as int *buff;
- an array defined as int buff[N];
- Rework the way to "unconstify" the temporary variable to avoid
issues due to integer promotion.
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'
- Patch 8
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'
Changes v3 -> v4
- Patches 1, 2
No changes.
- Patches 3, 4, 5
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'.
- Patch 6 (new in v4)
Fix headers inclusion order.
- Patch 7 (patch 6 in v3)
Add a comment related to __must_be_array()
Use __array[0] of *__array
- Patch 8 (patch 7 in v3)
Fix minmax.h inclusion order.
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'.
- Patch 9 (patch 8 in v3)
Add 'Suggested-by: Jonathan Cameron <[email protected]>'.
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'.
- Patch 10 (patch 9 in v3)
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'.
- Patch 11 (patch 10 in v3)
Fix a typo.
Add 'Suggested-by: Andy Shevchenko <[email protected]>'.
Add 'Reviewed-by: Andy Shevchenko <[email protected]>'.
- Patch 12 (patch 11 in v3)
Fix typos in the commit log.
Fix headers inclusion order.
Removed unneeded variable initialization.
Replace {0} by {}.
Use struct device *dev in probe().
Check an error on the snd-control-invert-range property read.
- Patch 13 (patch12 in v3)
No changes.
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 (13):
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()
iio: inkern: Fix headers inclusion order
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 literal
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 | 86 ++++-
include/linux/iio/consumer.h | 37 +-
include/linux/minmax.h | 64 ++++
include/sound/soc-dapm.h | 138 ++++---
sound/soc/codecs/Kconfig | 12 +
sound/soc/codecs/Makefile | 2 +
sound/soc/codecs/audio-iio-aux.c | 338 ++++++++++++++++++
sound/soc/generic/simple-card.c | 46 ++-
10 files changed, 769 insertions(+), 71 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
Fix the mutex.h inclusion order as it seems to be the only one
misplaces.
Signed-off-by: Herve Codina <[email protected]>
Suggested-by: Andy Shevchenko <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
---
drivers/iio/inkern.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index ce537b4ca6ca..71d0424383b6 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -5,9 +5,9 @@
*/
#include <linux/err.h>
#include <linux/export.h>
+#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/slab.h>
-#include <linux/mutex.h>
#include <linux/iio/iio.h>
#include <linux/iio/iio-opaque.h>
--
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]>
Reviewed-by: Andy Shevchenko <[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
On Thu, 15 Jun 2023 17:26:24 +0200
Herve Codina <[email protected]> wrote:
> Fix the mutex.h inclusion order as it seems to be the only one
> misplaces.
>
> Signed-off-by: Herve Codina <[email protected]>
> Suggested-by: Andy Shevchenko <[email protected]>
> Reviewed-by: Andy Shevchenko <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
> ---
> drivers/iio/inkern.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index ce537b4ca6ca..71d0424383b6 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -5,9 +5,9 @@
> */
> #include <linux/err.h>
> #include <linux/export.h>
> +#include <linux/mutex.h>
> #include <linux/property.h>
> #include <linux/slab.h>
> -#include <linux/mutex.h>
>
> #include <linux/iio/iio.h>
> #include <linux/iio/iio-opaque.h>