Add support for IIO devices in ASoC
Subsystems (4)
cover Add support for IIO devices in ASoC
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 under review [2].
This series introduces simple-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.
Best regards,
Hervé
[1] https://lore.kernel.org/linux-kernel/[email protected]/
[2] https://lore.kernel.org/linux-kernel/[email protected]/
Herve Codina (4):
dt-bindings: sound: Add simple-iio-aux
iio: inkern: Add a helper to query an available minimum raw value
ASoC: soc-dapm.h: Add a helper to build a DAPM widget dynamically
ASoC: codecs: Add support for the generic IIO auxiliary devices
.../bindings/sound/simple-iio-aux.yaml | 65 ++++ drivers/iio/inkern.c | 67 ++++ include/linux/iio/consumer.h | 11 + include/sound/soc-dapm.h | 12 +- sound/soc/codecs/Kconfig | 12 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/simple-iio-aux.c | 307 ++++++++++++++++++ 7 files changed, 475 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/sound/simple-iio-aux.yaml create mode 100644 sound/soc/codecs/simple-iio-aux.c
Signature · 1 line
2.39.2
PATCH 1/4 dt-bindings: sound: Add simple-iio-aux
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.
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
--- .../bindings/sound/simple-iio-aux.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/simple-iio-aux.yaml
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +65 -0
@@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/simple-iio-aux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple IIO auxiliary + +maintainers: + - Herve Codina <[email protected]> + +description: | + Auxiliary device based on Industrial I/O device channels + +allOf: + - $ref: /schemas/iio/iio-consumer.yaml + - $ref: dai-common.yaml# + +properties: + compatible: + const: simple-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. + + invert: + $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: + - | + aux { + compatible = "simple-iio-aux"; + io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>; + io-channel-names = "CH0", "CH1", "CH2", "CH3"; + /* Invert CH1 and CH2 */ + invert = <0 1 1>; + };
Signature · 1 line
2.39.2
Rob Herring
maintainerRe: [PATCH 1/4] dt-bindings: sound: Add simple-iio-aux
Quoted 6 lines · Herve Codina
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.
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
What makes it simple? Any binding called simple or generic is a trigger for me. Best to avoid those terms. :)
Examples of devices would be useful here.
Quoted 15 lines
Signed-off-by Herve Codina <[email protected]>--- .../bindings/sound/simple-iio-aux.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/simple-iio-aux.yamlDocumentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +2
-0@@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 12
Quoted 10 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +10
-0+--- +$id: http://devicetree.org/schemas/sound/simple-iio-aux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple IIO auxiliary + +maintainers: + - Herve Codina <[email protected]> + +description: |
Don't need '|'
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 16
Quoted 4 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +4
-0+ Auxiliary device based on Industrial I/O device channels + +allOf: + - $ref: /schemas/iio/iio-consumer.yaml
You don't need to reference consumer schemas.
Quoted 6 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0+ - $ref: dai-common.yaml# + +properties: + compatible: + const: simple-iio-aux +
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 32
Quoted 10 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +10
-0+ 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. + + invert:
Property names should globally only have 1 type definition. This is generic enough I'd be concerned that's not the case.
Quoted 20 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +20
-0+ $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
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 62
Quoted 10 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +10
-0+ - io-channel-names + +unevaluatedProperties: false + +examples: + - | + aux { + compatible = "simple-iio-aux"; + io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>; + io-channel-names = "CH0", "CH1", "CH2", "CH3";
Not really useful names. Do you have a real example?
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 64
Quoted 2 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +2
-0+ /* Invert CH1 and CH2 */ + invert = <0 1 1>;
IMO, invert should be same length as io-channels.
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 65
Quoted 1 line
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +1
-0+ };
How do support multiple instances? Say you have 2 sound cards (or 1 sound card with multiple audio paths) each with different sets of IIO channels associated with it. You'd need a link to each 'aux' node. Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them.
Rob
Mark Brown
maintainerRe: [PATCH 1/4] dt-bindings: sound: Add simple-iio-aux
Quoted 8 lines · Rob Herring
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 62
Quoted 4 lines · Herve Codina
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +4
-0+ aux { + compatible = "simple-iio-aux"; + io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>; + io-channel-names = "CH0", "CH1", "CH2", "CH3";Not really useful names. Do you have a real example?
I fear those might be real names for channels on an IIO device...
Re: [PATCH 1/4] dt-bindings: sound: Add simple-iio-aux
In reply to Rob HerringHi Rob,
Quoted 10 lines · On Tue, 25 Apr 2023 12:30:29 -0500 Rob Herring <[email protected]> wrote:
Quoted 6 lines · Herve Codina
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.
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
What makes it simple? Any binding called simple or generic is a trigger for me. Best to avoid those terms. :)
I choose simple-iio-aux because some simple-* already exists. For instance simple-audio-amplifier or simple-audio-mux.
Do you prefer audio-iio-aux ?
Let me know if I should change.
Quoted 22 lines
Examples of devices would be useful here.
Quoted 19 lines
Signed-off-by Herve Codina <[email protected]>--- .../bindings/sound/simple-iio-aux.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/simple-iio-aux.yamlDocumentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0@@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/simple-iio-aux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# +
Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 12
Quoted 6 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0+title: Simple IIO auxiliary + +maintainers: + - Herve Codina <[email protected]> + +description: |Don't need '|'
Will be fixed.
Quoted 7 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 16
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +4
-0+ Auxiliary device based on Industrial I/O device channels + +allOf: + - $ref: /schemas/iio/iio-consumer.yamlYou don't need to reference consumer schemas.
Right, will be removed.
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +11
-0+ - $ref: dai-common.yaml# + +properties: + compatible: + const: simple-iio-aux + + io-channels: + description: + Industrial I/O device channels used + + io-channel-names:
Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 32
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +5
-0+ description: + Industrial I/O channel names related to io-channels. + These names are used to provides sound controls, widgets and routes names. + + invert:Property names should globally only have 1 type definition. This is generic enough I'd be concerned that's not the case.
What do you mean ?
Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +24
-0+ $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 +
Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 62
Quoted 6 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0+examples: + - | + aux { + compatible = "simple-iio-aux"; + io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>; + io-channel-names = "CH0", "CH1", "CH2", "CH3";Not really useful names. Do you have a real example?
As Mark said, for IIO channel, using CHx makes sense. See below, I provide a full example.
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 64
Quoted 3 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +2
-0+ /* Invert CH1 and CH2 */ + invert = <0 1 1>;IMO, invert should be same length as io-channels.
I will update.
Related to this topic, when I wrote this binding, I wanted to add some rules/constraints in order to:
- Have this property optional
- If present, force to have as many items in the invert array as the number of items present in the io-channels array.
I never succeed in writing the constraint for the invert property. It should be possible (it is done for some 'foo' 'foo-names' pair such as clocks).
Can you tell me if possible in my case and give me some pointers ?
Quoted 10 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 65
Quoted 2 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +1
-0+ };How do support multiple instances? Say you have 2 sound cards (or 1 sound card with multiple audio paths) each with different sets of IIO channels associated with it. You'd need a link to each 'aux' node. Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them.
I don't see what you mean.
I use a simple-audio-card and here is a full example using several instances:
spi {
#address-cells = <1>;
#size-cells = <0>;
/* potentiometers present in an input amplifier design */
pot_in: potentiometer@0 {
compatible = "foo,xxx";
reg = <0>;
#io-channel-cells = <1>;
};
/* potentiometers present in an output amplifier design */
pot_out: potentiometer@1 {
compatible = "foo,xxx";
reg = <1>;
#io-channel-cells = <1>;
};
/* A codec */
codec: codec@2 {
compatible = "bar,yyy";
reg = <2>;
sound-name-prefix = "CODEC";
};
};
amp_out: aux-out {
compatible = "simple-iio-aux";
io-channels = <&pot_out 0>, <&pot_out 1>,
io-channel-names = "CH0", "CH1";
invert = <1 1>;
sound-name-prefix = "AMP_OUT";
};
amp_in: aux-in {
compatible = "simple-iio-aux";
io-channels = <&pot_in 0>, <&pot_in 1>;
io-channel-names = "CH0", "CH1";
sound-name-prefix = "AMP_IN";
};
sound {
compatible = "simple-audio-card";
#address-cells = <1>;
#size-cells = <0>;
simple-audio-card,name = "My Sound Card";
simple-audio-card,aux-devs = <&_in>, <&_out>;
simple-audio-card,routing =
"CODEC IN0", "AMP_IN CH0 OUT",
"CODEC IN1", "AMP_IN CH1 OUT",
"AMP_OUT CH0 IN", "CODEC OUT0",
"AMP_OUT CH1 IN", "CODEC OUT1",
simple-audio-card,dai-link@0 {
...
};
};Best regards,
Hervé
Signature · 3 lines
Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH 1/4] dt-bindings: sound: Add simple-iio-aux
Quoted 13 lines · Herve Codina
Hi Rob,
Quoted 8 lines · On Tue, 25 Apr 2023 12:30:29 -0500 Rob Herring <[email protected]> wrote:
Quoted 6 lines · Herve Codina
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.
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
Quoted 8 lines
Quoted 2 lines
What makes it simple? Any binding called simple or generic is a trigger for me. Best to avoid those terms. :)
I choose simple-iio-aux because some simple-* already exists. For instance simple-audio-amplifier or simple-audio-mux.
Do you prefer audio-iio-aux ?
Let me know if I should change.
It means that often what people call "simple" and "generic" works only for their specific case, because it is not really simple and generic. After some time the "simple" and "generic" becomes "complicated" and "huge". Conclusion: sometimes simple and generic bindings are bad idea and you should have something specific.
Your description in the binding also does not help to match it to specific, real device. Provide the examples, as Rob asked.
...
Quoted 14 lines
Quoted 5 lines
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +5
-0+ io-channels: + description: + Industrial I/O device channels used + + io-channel-names:Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 32
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +5
-0+ description: + Industrial I/O channel names related to io-channels. + These names are used to provides sound controls, widgets and routes names. + + invert:Property names should globally only have 1 type definition. This is generic enough I'd be concerned that's not the case.
Quoted 1 line
What do you mean ?
It is quite likely this will interfere with other properties having the same name but different type/definition. If you want to keep it named generic, then please investigate how this would affect any other bindings. So easier is to make it not that generic, some more specific name.
Quoted 68 lines
Quoted 26 lines
Quoted 26 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +24
-0+ $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 +Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 62
Quoted 6 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0+examples: + - | + aux { + compatible = "simple-iio-aux"; + io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>; + io-channel-names = "CH0", "CH1", "CH2", "CH3";Not really useful names. Do you have a real example?
As Mark said, for IIO channel, using CHx makes sense. See below, I provide a full example.
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 64
Quoted 3 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +2
-0+ /* Invert CH1 and CH2 */ + invert = <0 1 1>;IMO, invert should be same length as io-channels.
I will update.
Related to this topic, when I wrote this binding, I wanted to add some rules/constraints in order to:
- Have this property optional
- If present, force to have as many items in the invert array as the number of items present in the io-channels array.I never succeed in writing the constraint for the invert property. It should be possible (it is done for some 'foo' 'foo-names' pair such as clocks).
Can you tell me if possible in my case and give me some pointers ?Quoted 10 lines
Quoted 2 lines
+ };
How do support multiple instances? Say you have 2 sound cards (or 1 sound card with multiple audio paths) each with different sets of IIO channels associated with it. You'd need a link to each 'aux' node. Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them.
Quoted 3 lines
I don't see what you mean.
I use a simple-audio-card and here is a full example using several instances:
Just like Rob said: "You'd need a link to each 'aux' node"
and you did it...
So now the rest of Rob's answer:
"Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them."
Why do you need another node if it is not really representing a real, separate device?
Best regards,
Krzysztof
Mark Brown
maintainerRe: [PATCH 1/4] dt-bindings: sound: Add simple-iio-aux
Quoted 16 lines · Krzysztof Kozlowski
Quoted 6 lines · Herve Codina
Quoted 4 lines · Rob Herring
Quoted 2 lines · Herve Codina
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
Quoted 8 lines
Quoted 2 lines
What makes it simple? Any binding called simple or generic is a trigger for me. Best to avoid those terms. :)
I choose simple-iio-aux because some simple-* already exists. For instance simple-audio-amplifier or simple-audio-mux.
Do you prefer audio-iio-aux ?
Let me know if I should change.
Quoted 8 lines
It means that often what people call "simple" and "generic" works only for their specific case, because it is not really simple and generic. After some time the "simple" and "generic" becomes "complicated" and "huge". Conclusion: sometimes simple and generic bindings are bad idea and you should have something specific.
Your description in the binding also does not help to match it to specific, real device. Provide the examples, as Rob asked.
I don't understand what you are looking for here. IIO is a subsystem which represents generic DACs and ADCs (along with other I/O things). Audio devices also have DACs and ADCs, somewhat specialised for use in audio but more limited by specs and interfaces than by anything fundamental. The goal here is to map DACs and ADCs described as IIO for use in an audio context.
ADCs are devices that convert analog signals into digital values, DACs are devices that convert digital values into analog signals.
Quoted 18 lines
Quoted 11 lines
Quoted 7 lines
How do support multiple instances? Say you have 2 sound cards (or 1 sound card with multiple audio paths) each with different sets of IIO channels associated with it. You'd need a link to each 'aux' node. Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them.
I don't see what you mean.
I use a simple-audio-card and here is a full example using several instances:Just like Rob said: "You'd need a link to each 'aux' node"
and you did it...
So now the rest of Rob's answer:
Quoted 7 lines
"Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them."
Why do you need another node if it is not really representing a real, separate device?
If nothing else I would expect it to be useful from a comprehensibility point of view to bundle multiple IIO devices into a single multi-channel audio stream, an individual IIO device is likely to only present a single channel of data but it is common to group multiple channels of audio data.
Re: [PATCH 1/4] dt-bindings: sound: Add simple-iio-aux
Hi Rob, Krzysztof, Mark,
Quoted 59 lines · On Thu, 4 May 2023 13:22:35 +0900 Mark Brown <[email protected]> wrote:
Quoted 16 lines · Krzysztof Kozlowski
Quoted 6 lines · Herve Codina
Quoted 4 lines · Rob Herring
Quoted 2 lines · Herve Codina
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
Quoted 8 lines
Quoted 2 lines
What makes it simple? Any binding called simple or generic is a trigger for me. Best to avoid those terms. :)
I choose simple-iio-aux because some simple-* already exists. For instance simple-audio-amplifier or simple-audio-mux.
Do you prefer audio-iio-aux ?
Let me know if I should change.Quoted 8 lines
It means that often what people call "simple" and "generic" works only for their specific case, because it is not really simple and generic. After some time the "simple" and "generic" becomes "complicated" and "huge". Conclusion: sometimes simple and generic bindings are bad idea and you should have something specific.
Your description in the binding also does not help to match it to specific, real device. Provide the examples, as Rob asked.
I don't understand what you are looking for here. IIO is a subsystem which represents generic DACs and ADCs (along with other I/O things). Audio devices also have DACs and ADCs, somewhat specialised for use in audio but more limited by specs and interfaces than by anything fundamental. The goal here is to map DACs and ADCs described as IIO for use in an audio context.
ADCs are devices that convert analog signals into digital values, DACs are devices that convert digital values into analog signals.
Quoted 22 lines
Quoted 11 lines
Quoted 7 lines
How do support multiple instances? Say you have 2 sound cards (or 1 sound card with multiple audio paths) each with different sets of IIO channels associated with it. You'd need a link to each 'aux' node. Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them.
I don't see what you mean.
I use a simple-audio-card and here is a full example using several instances:Just like Rob said: "You'd need a link to each 'aux' node"
and you did it...
So now the rest of Rob's answer:
"Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them."
Quoted 8 lines
Quoted 2 lines
Why do you need another node if it is not really representing a real, separate device?
If nothing else I would expect it to be useful from a comprehensibility point of view to bundle multiple IIO devices into a single multi-channel audio stream, an individual IIO device is likely to only present a single channel of data but it is common to group multiple channels of audio data.
I cannot simply add io-channels to the sound card directly. I need a node to set at least the sound-name-prefix property. Further more having a node and a related compatible string can be easier to maintain and add future evolution related to these "virtual" devices.
As some subnodes are already defined for a sound card node, I propose to group these "virtual" audio devices node in a specific bundle node. This lead to the following example:
---- 8< ----
spi {
#address-cells = <1>;
#size-cells = <0>;
/* potentiometers present in an input amplifier design */
pot_in: potentiometer@0 {
compatible = "foo,xxx";
reg = <0>;
#io-channel-cells = <1>;
};
/* potentiometers present in an output amplifier design */
pot_out: potentiometer@1 {
compatible = "foo,xxx";
reg = <1>;
#io-channel-cells = <1>;
};
/* A codec */
codec: codec@2 {
compatible = "bar,yyy";
reg = <2>;
sound-name-prefix = "CODEC";
};
};
sound {
compatible = "simple-audio-card";
#address-cells = <1>;
#size-cells = <0>;
simple-audio-card,name = "My Sound Card";
simple-audio-card,aux-devs = <&_in>, <&_out>;
simple-audio-card,routing =
"CODEC IN0", "AMP_IN CH0 OUT",
"CODEC IN1", "AMP_IN CH1 OUT",
"AMP_OUT CH0 IN", "CODEC OUT0",
"AMP_OUT CH1 IN", "CODEC OUT1";
simple-audio-card,dai-link@0 {
...
};
...
/* A bundle for the additional devices */
simple-audio-card,additional-devs {
amp_out: aux-out {
compatible = "audio-iio-aux"; /* Instead of "simple-iio-aux */
io-channels = <&pot_out 0>, <&pot_out 1>,
io-channel-names = "CH0", "CH1";
snd-control-invert-range = <1 1>; /* Old 'invert' renamed */
sound-name-prefix = "AMP_OUT";
};
amp_in: aux-in {
compatible = "audio-iio-aux";
io-channels = <&pot_in 0>, <&pot_in 1>;
io-channel-names = "CH0", "CH1";
sound-name-prefix = "AMP_IN";
};
};
};---- 8< ----
What do you think about this new binding ?
Best regards,
Hervé
Re: [PATCH 1/4] dt-bindings: sound: Add simple-iio-aux
In reply to Rob HerringHi Rob,
Quoted 10 lines · On Tue, 25 Apr 2023 12:30:29 -0500 Rob Herring <[email protected]> wrote:
Quoted 6 lines · Herve Codina
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.
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
What makes it simple? Any binding called simple or generic is a trigger for me. Best to avoid those terms. :)
I choose simple-iio-aux because some simple-* already exists. For instance simple-audio-amplifier or simple-audio-mux.
Do you prefer audio-iio-aux ?
Let me know if I should change.
Quoted 22 lines
Examples of devices would be useful here.
Quoted 19 lines
Signed-off-by Herve Codina <[email protected]>--- .../bindings/sound/simple-iio-aux.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/simple-iio-aux.yamlDocumentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0@@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/simple-iio-aux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# +
Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 12
Quoted 6 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0+title: Simple IIO auxiliary + +maintainers: + - Herve Codina <[email protected]> + +description: |Don't need '|'
Will be fixed.
Quoted 7 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 16
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +4
-0+ Auxiliary device based on Industrial I/O device channels + +allOf: + - $ref: /schemas/iio/iio-consumer.yamlYou don't need to reference consumer schemas.
Right, will be removed.
Quoted 12 lines
Quoted 12 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +11
-0+ - $ref: dai-common.yaml# + +properties: + compatible: + const: simple-iio-aux + + io-channels: + description: + Industrial I/O device channels used + + io-channel-names:
Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 32
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +5
-0+ description: + Industrial I/O channel names related to io-channels. + These names are used to provides sound controls, widgets and routes names. + + invert:Property names should globally only have 1 type definition. This is generic enough I'd be concerned that's not the case.
What do you mean ?
Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +24
-0+ $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 +
Quoted 8 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 62
Quoted 6 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +6
-0+examples: + - | + aux { + compatible = "simple-iio-aux"; + io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>; + io-channel-names = "CH0", "CH1", "CH2", "CH3";Not really useful names. Do you have a real example?
As Mark said, for IIO channel, using CHx makes sense. See below, I provide a full example.
Quoted 5 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 64
Quoted 3 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +2
-0+ /* Invert CH1 and CH2 */ + invert = <0 1 1>;IMO, invert should be same length as io-channels.
I will update.
Related to this topic, when I wrote this binding, I wanted to add some rules/constraints in order to:
- Have this property optional
- If present, force to have as many items in the invert array as the number of items present in the io-channels array.
I never succeed in writing the constraint for the invert property. It should be possible (it is done for some 'foo' 'foo-names' pair such as clocks).
Can you tell me if possible in my case and give me some pointers ?
Quoted 10 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml line 65
Quoted 2 lines
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +1
-0+ };How do support multiple instances? Say you have 2 sound cards (or 1 sound card with multiple audio paths) each with different sets of IIO channels associated with it. You'd need a link to each 'aux' node. Why not just add io-channels to the sound card nodes directly? That's already just a virtual, top-level container node grouping all the components. I don't see why we need another virtual node grouping a subset of them.
I don't see what you mean.
I use a simple-audio-card and here is a full example using several instances:
spi {
#address-cells = <1>;
#size-cells = <0>;
/* potentiometers present in an input amplifier design */
pot_in: potentiometer@0 {
compatible = "foo,xxx";
reg = <0>;
#io-channel-cells = <1>;
};
/* potentiometers present in an output amplifier design */
pot_out: potentiometer@1 {
compatible = "foo,xxx";
reg = <1>;
#io-channel-cells = <1>;
};
/* A codec */
codec: codec@2 {
compatible = "bar,yyy";
reg = <2>;
sound-name-prefix = "CODEC";
};
};
amp_out: aux-out {
compatible = "simple-iio-aux";
io-channels = <&pot_out 0>, <&pot_out 1>,
io-channel-names = "CH0", "CH1";
invert = <1 1>;
sound-name-prefix = "AMP_OUT";
};
amp_in: aux-in {
compatible = "simple-iio-aux";
io-channels = <&pot_in 0>, <&pot_in 1>;
io-channel-names = "CH0", "CH1";
sound-name-prefix = "AMP_IN";
};
sound {
compatible = "simple-audio-card";
#address-cells = <1>;
#size-cells = <0>;
simple-audio-card,name = "My Sound Card";
simple-audio-card,aux-devs = <&_in>, <&_out>;
simple-audio-card,routing =
"CODEC IN0", "AMP_IN CH0 OUT",
"CODEC IN1", "AMP_IN CH1 OUT",
"AMP_OUT CH0 IN", "CODEC OUT0",
"AMP_OUT CH1 IN", "CODEC OUT1",
simple-audio-card,dai-link@0 {
...
};
};Best regards,
Hervé
Signature · 3 lines
Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH 2/4 iio: inkern: Add a helper to query an available minimum raw value
In reply to Herve CodinaA 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().
--- drivers/iio/inkern.c | 67 ++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 11 ++++++ 2 files changed, 78 insertions(+)
drivers/iio/inkern.c +67 -0
@@ -912,6 +912,73 @@ 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) +{ +int unused; +const int *vals; +int length; +int ret; + +if (!val2) + val2 = &unused; + + ret =iio_channel_read_avail (chan, &vals, type, &length, info); +switch (ret) { +case IIO_AVAIL_RANGE : +switch (*type) { +case IIO_VAL_INT : + *val = vals[0 ]; +break ; +default : + *val = vals[0 ]; + *val2 = vals[1 ]; + } +return 0 ; + +case IIO_AVAIL_LIST : +if (length <=0 ) +return -EINVAL ; +switch (*type) { +case IIO_VAL_INT : + *val = vals[--length]; +while (length) { +if (vals[--length] < *val) + *val = vals[length]; + } +break ; +default : +/* FIXME: learn about min for other iio values */ +return -EINVAL ; + } +return 0 ; + +default : +return ret; + } +} + +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);
include/linux/iio/consumer.h +11 -0
@@ -297,6 +297,17 @@ 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 raw reads from iio channels are in adc counts and hence +* scale will need to be applied if standard units are required. +*/ +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.
Signature · 1 line
2.39.2
Jonathan Cameron
maintainerRe: [PATCH 2/4] iio: inkern: Add a helper to query an available minimum raw value
Quoted 9 lines · On Fri, 21 Apr 2023 14:41:20 +0200 Herve Codina <[email protected]>…
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 existingiio_read_max_channel_raw().Signed-off-by Herve Codina <[email protected]>
Hi Herve,
All the comments on this are really comments on the existing code. If you don't mind fixing the first one about checking the error code whilst you are here that would be great. Don't worry about the docs comment. There are lots of instances of that and the point is rather subtle and probably post dates this code being written. In a few cases raw doesn't mean ADC counts but rather something slightly modified... Long story for why!
Jonathan
Quoted 16 lines
--- drivers/iio/inkern.c | 67 ++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 11 ++++++ 2 files changed, 78 insertions(+)drivers/iio/inkern.c +3
-0@@ -912,6 +912,73 @@ 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)
drivers/iio/inkern.c int iio_read_max_channel_raw(struct iio_channel *chan, int *val) · line 927
Quoted 10 lines
drivers/iio/inkern.c +10
-0+{ +int unused; +const int *vals; +int length; +int ret; + +if (!val2) + val2 = &unused; + + ret =iio_channel_read_avail (chan, &vals, type, &length, info);
Obviously this is copied from *_read_max() but look at it here...
We should check for an error first with
if (ret < 0)
return ret;
then the switch.
Currently a different positive ret would result in that value being returned which would be odd. Not a problem today, but if we add other iio_avail_type enum entries in future and don't keep up with all the utility functions then a mess may result.
If you agree with change and wouldn't mind adding another patch to this series tidying that up for the _max case that would be great! Otherwise I'll get to fixing that at some point but not anytime soon.
Quoted 63 lines
drivers/iio/inkern.c +54
-0+switch (ret) { +case IIO_AVAIL_RANGE : +switch (*type) { +case IIO_VAL_INT : + *val = vals[0 ]; +break ; +default : + *val = vals[0 ]; + *val2 = vals[1 ]; + } +return 0 ; + +case IIO_AVAIL_LIST : +if (length <=0 ) +return -EINVAL ; +switch (*type) { +case IIO_VAL_INT : + *val = vals[--length]; +while (length) { +if (vals[--length] < *val) + *val = vals[length]; + } +break ; +default : +/* FIXME: learn about min for other iio values */ +return -EINVAL ; + } +return 0 ; + +default : +return ret; + } +} + +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);include/linux/iio/consumer.h +0
-0@@ -297,6 +297,17 @@ int iio_write_channel_raw(struct iio_channel *chan, int val);*/
include/linux/iio/consumer.h int iio_write_channel_raw(struct iio_channel *chan, int val); · line 307
Quoted 10 lines
include/linux/iio/consumer.h +8
-0int 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 raw reads from iio channels are in adc counts and hence +* scale will need to be applied if standard units are required.
Hmm. That comment is almost always true, but not quite. Not related to your patch but some cleanup of this documentation and pushing it down next to implementations should be done at some point. If anyone is really bored and wants to take this on that's fine. If not, another one for the todo list ;)
Quoted 6 lines
include/linux/iio/consumer.h +3
-0+*/ +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.
Re: [PATCH 2/4] iio: inkern: Add a helper to query an available minimum raw value
Hi Jonathan,
Quoted 13 lines · On Sat, 22 Apr 2023 17:49:16 +0100 Jonathan Cameron <[email protected]> wrote:
Quoted 9 lines · On Fri, 21 Apr 2023 14:41:20 +0200 Herve Codina <[email protected]>…
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 existingiio_read_max_channel_raw().Signed-off-by Herve Codina <[email protected]>
Quoted 8 lines
Hi Herve,
All the comments on this are really comments on the existing code. If you don't mind fixing the first one about checking the error code whilst you are here that would be great. Don't worry about the docs comment. There are lots of instances of that and the point is rather subtle and probably post dates this code being written. In a few cases raw doesn't mean ADC counts but rather something slightly modified... Long story for why!
A next iteration is already planned for this series.
I will fix the 'error checking before switch()' on the iio_channel_read_min() I introduced and add a new patch (doing the same) on the existing iio_channel_read_max().
Quoted 36 lines
Jonathan
Quoted 16 lines
--- drivers/iio/inkern.c | 67 ++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 11 ++++++ 2 files changed, 78 insertions(+)drivers/iio/inkern.c +3
-0@@ -912,6 +912,73 @@ 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)drivers/iio/inkern.c int iio_read_max_channel_raw(struct iio_channel *chan, int *val) · line 927
Quoted 10 lines
drivers/iio/inkern.c +10
-0+{ +int unused; +const int *vals; +int length; +int ret; + +if (!val2) + val2 = &unused; + + ret =iio_channel_read_avail (chan, &vals, type, &length, info);Obviously this is copied from
*_read_max()but look at it here...We should check for an error first with
if (ret < 0)return ret;then the switch.
Quoted 8 lines
Currently a different positive ret would result in that value being returned which would be odd. Not a problem today, but if we add other
iio_avail_typeenum entries in future and don't keep up with all the utility functions then a mess may result.If you agree with change and wouldn't mind adding another patch to this series tidying that up for the
_maxcase that would be great! Otherwise I'll get to fixing that at some point but not anytime soon.
I will do in the next iteration.
Quoted 66 lines
Quoted 66 lines
drivers/iio/inkern.c +54
-0+switch (ret) { +case IIO_AVAIL_RANGE : +switch (*type) { +case IIO_VAL_INT : + *val = vals[0 ]; +break ; +default : + *val = vals[0 ]; + *val2 = vals[1 ]; + } +return 0 ; + +case IIO_AVAIL_LIST : +if (length <=0 ) +return -EINVAL ; +switch (*type) { +case IIO_VAL_INT : + *val = vals[--length]; +while (length) { +if (vals[--length] < *val) + *val = vals[length]; + } +break ; +default : +/* FIXME: learn about min for other iio values */ +return -EINVAL ; + } +return 0 ; + +default : +return ret; + } +} + +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);include/linux/iio/consumer.h +0
-0@@ -297,6 +297,17 @@ int iio_write_channel_raw(struct iio_channel *chan, int val);*/ int iio_read_max_channel_raw (struct iio_channel *chan,int *val);
Quoted 14 lines
include/linux/iio/consumer.h int iio_write_channel_raw(struct iio_channel *chan, int val); · line 307
Quoted 8 lines
include/linux/iio/consumer.h +8
-0+/** +* 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 raw reads from iio channels are in adc counts and hence +* scale will need to be applied if standard units are required. Hmm. That comment is almost always true, but not quite. Not related to your patch but some cleanup of this documentation and pushing it down next to implementations should be done at some point. If anyone is really bored and wants to take this on that's fine. If not, another one for the todo list ;)
If you are ok, I can change every where in consumer.h the following:
* Note raw reads from iio channels are in adc counts and hence
* scale will need to be applied if standard units required.
by
* Note raw reads from iio channels are not in standards units and
* hence scale will need to be applied if standard units required.
Also the same for raw writes:
* Note raw writes to iio channels are in dac counts and hence
* scale will need to be applied if standard units required.
by
* Note raw writes to iio channels are not in standards units and
* hence scale will need to be applied if standard units required.
Quoted 8 lines
include/linux/iio/consumer.h int iio_write_channel_raw(struct iio_channel *chan, int val); · line 313
Quoted 7 lines
include/linux/iio/consumer.h +3
-0+*/ +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.
Thanks for the review,
Hervé
Signature · 3 lines
Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Jonathan Cameron
maintainerRe: [PATCH 2/4] iio: inkern: Add a helper to query an available minimum raw value
Quoted 23 lines
Quoted 8 lines
Quoted 8 lines
include/linux/iio/consumer.h +0
-0@@ -297,6 +297,17 @@ int iio_write_channel_raw(struct iio_channel *chan, int val);*/ int iio_read_max_channel_raw (struct iio_channel *chan,int *val);Quoted 14 lines
include/linux/iio/consumer.h int iio_write_channel_raw(struct iio_channel *chan, int val); · line 307
Quoted 8 lines
include/linux/iio/consumer.h +8
-0+/** +* 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 raw reads from iio channels are in adc counts and hence +* scale will need to be applied if standard units are required. Hmm. That comment is almost always true, but not quite. Not related to your patch but some cleanup of this documentation and pushing it down next to implementations should be done at some point. If anyone is really bored and wants to take this on that's fine. If not, another one for the todo list ;)
Quoted 6 lines
If you are ok, I can change every where in
consumer.hthe following:
* Note raw reads from iio channels are in adc counts and hence
* scale will need to be applied if standard units required.
by
* Note raw reads from iio channels are not in standards units and
* hence scale will need to be applied if standard units required.
If going to the effort, we should include offset and make it clear how they are applied.
* 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.
Quoted 7 lines
Also the same for raw writes:
* Note raw writes to iio channels are in dac counts and hence
* scale will need to be applied if standard units required.
by
* Note raw writes to iio channels are not in standards units and
* hence scale will need to be applied if standard units required.
This one is more interesting because you kind of need to apply the opposite logic. Perhaps text such as.
* Note that for raw writes to iio channels, if the value provided is
* in standard units, the affect of the scale and offset must be removed
* as (value / scale) - offset.
My slight concern is that we'll spend longer arguing about these comments than we spend on the rest of the patch set. Might be worth delaying fixing the others for a separate series after this one.
Jonathan
Quoted 13 lines
Quoted 9 lines
include/linux/iio/consumer.h int iio_write_channel_raw(struct iio_channel *chan, int val); · line 313
Quoted 8 lines
include/linux/iio/consumer.h +3
-0+*/ +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. Thanks for the review,
Hervé
Re: [PATCH 2/4] iio: inkern: Add a helper to query an available minimum raw value
In reply to Jonathan CameronHi Jonathan,
Quoted 13 lines · On Sat, 22 Apr 2023 17:49:16 +0100 Jonathan Cameron <[email protected]> wrote:
Quoted 9 lines · On Fri, 21 Apr 2023 14:41:20 +0200 Herve Codina <[email protected]>…
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 existingiio_read_max_channel_raw().Signed-off-by Herve Codina <[email protected]>
Quoted 8 lines
Hi Herve,
All the comments on this are really comments on the existing code. If you don't mind fixing the first one about checking the error code whilst you are here that would be great. Don't worry about the docs comment. There are lots of instances of that and the point is rather subtle and probably post dates this code being written. In a few cases raw doesn't mean ADC counts but rather something slightly modified... Long story for why!
A next iteration is already planned for this series.
I will fix the 'error checking before switch()' on the iio_channel_read_min() I introduced and add a new patch (doing the same) on the existing iio_channel_read_max().
Quoted 36 lines
Jonathan
Quoted 16 lines
--- drivers/iio/inkern.c | 67 ++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 11 ++++++ 2 files changed, 78 insertions(+)drivers/iio/inkern.c +3
-0@@ -912,6 +912,73 @@ 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)drivers/iio/inkern.c int iio_read_max_channel_raw(struct iio_channel *chan, int *val) · line 927
Quoted 10 lines
drivers/iio/inkern.c +10
-0+{ +int unused; +const int *vals; +int length; +int ret; + +if (!val2) + val2 = &unused; + + ret =iio_channel_read_avail (chan, &vals, type, &length, info);Obviously this is copied from
*_read_max()but look at it here...We should check for an error first with
if (ret < 0)return ret;then the switch.
Quoted 8 lines
Currently a different positive ret would result in that value being returned which would be odd. Not a problem today, but if we add other
iio_avail_typeenum entries in future and don't keep up with all the utility functions then a mess may result.If you agree with change and wouldn't mind adding another patch to this series tidying that up for the
_maxcase that would be great! Otherwise I'll get to fixing that at some point but not anytime soon.
I will do in the next iteration.
Quoted 66 lines
Quoted 66 lines
drivers/iio/inkern.c +54
-0+switch (ret) { +case IIO_AVAIL_RANGE : +switch (*type) { +case IIO_VAL_INT : + *val = vals[0 ]; +break ; +default : + *val = vals[0 ]; + *val2 = vals[1 ]; + } +return 0 ; + +case IIO_AVAIL_LIST : +if (length <=0 ) +return -EINVAL ; +switch (*type) { +case IIO_VAL_INT : + *val = vals[--length]; +while (length) { +if (vals[--length] < *val) + *val = vals[length]; + } +break ; +default : +/* FIXME: learn about min for other iio values */ +return -EINVAL ; + } +return 0 ; + +default : +return ret; + } +} + +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);include/linux/iio/consumer.h +0
-0@@ -297,6 +297,17 @@ int iio_write_channel_raw(struct iio_channel *chan, int val);*/ int iio_read_max_channel_raw (struct iio_channel *chan,int *val);
Quoted 14 lines
include/linux/iio/consumer.h int iio_write_channel_raw(struct iio_channel *chan, int val); · line 307
Quoted 8 lines
include/linux/iio/consumer.h +8
-0+/** +* 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 raw reads from iio channels are in adc counts and hence +* scale will need to be applied if standard units are required. Hmm. That comment is almost always true, but not quite. Not related to your patch but some cleanup of this documentation and pushing it down next to implementations should be done at some point. If anyone is really bored and wants to take this on that's fine. If not, another one for the todo list ;)
If you are ok, I can change every where in consumer.h the following:
* Note raw reads from iio channels are in adc counts and hence
* scale will need to be applied if standard units required.
by
* Note raw reads from iio channels are not in standards units and
* hence scale will need to be applied if standard units required.
Also the same for raw writes:
* Note raw writes to iio channels are in dac counts and hence
* scale will need to be applied if standard units required.
by
* Note raw writes to iio channels are not in standards units and
* hence scale will need to be applied if standard units required.
Quoted 8 lines
include/linux/iio/consumer.h int iio_write_channel_raw(struct iio_channel *chan, int val); · line 313
Quoted 7 lines
include/linux/iio/consumer.h +3
-0+*/ +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.
Thanks for the review,
Hervé
Signature · 3 lines
Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH 3/4 ASoC: soc-dapm.h: Add a helper to build a DAPM widget dynamically
In reply to Herve CodinaThe SND_SOC_DAPM_* helpers family are used to build widgets array in a static way.
Introduce SND_SOC_DAPM_WIDGET() in order to use the SND_SOC_DAPM_* helpers family in a dynamic way. The different SND_SOC_DAPM_* parameters can be computed by the code and the widget can be built based on this parameter computation.
For instance:
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_WIDGET(SND_SOC_DAPM_INPUT(name));
...}
--- include/sound/soc-dapm.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
include/sound/soc-dapm.h +11 -1
@@ -276,7 +276,17 @@ struct soc_enum; .reg = SND_SOC_NOPM, .event = dapm_pinctrl_event, \ .event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD } - +/* +* Helper to create a widget 'dynamically' +* This can be used with any of the SND_SOC_DAPM_* widget helper. +* For instance: +* struct snd_soc_dapm_widget widget; +* ... +* widget = SND_SOC_DAPM_WIDGET(SND_SOC_DAPM_INPUT(input_name)); +*/ +#define SND_SOC_DAPM_WIDGET (_widget)({ \ + struct snd_soc_dapm_widget _w[1] = { _widget }; \ + _w[0]; })/* dapm kcontrol types */ #define SOC_DAPM_DOUBLE (xname, reg, lshift, rshift, max, invert) \
Signature · 1 line
2.39.2
PATCH 4/4 ASoC: codecs: Add support for the generic IIO auxiliary devices
In reply to Herve CodinaIndustrial 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.
--- sound/soc/codecs/Kconfig | 12 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/simple-iio-aux.c | 307 ++++++++++++++++++++++++++++++ 3 files changed, 321 insertions(+) create mode 100644 sound/soc/codecs/simple-iio-aux.c
sound/soc/codecs/Kconfig +12 -0
@@ -211,6 +211,7 @@ config SND_SOC_ALL_CODECS imply SND_SOC_SGTL5000 imply SND_SOC_SI476X imply SND_SOC_SIMPLE_AMPLIFIER + imply SND_SOC_SIMPLE_IIO_AUX imply SND_SOC_SIMPLE_MUX imply SND_SOC_SMA1303 imply SND_SOC_SPDIF
@@ -1555,6 +1556,17 @@ config SND_SOC_SIGMADSP_REGMAP config SND_SOC_SIMPLE_AMPLIFIER tristate "Simple Audio Amplifier" +config SND_SOC_SIMPLE_IIO_AUX + tristate "Simple 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-simple-iio-aux. + config SND_SOC_SIMPLE_MUX tristate "Simple Audio Mux" depends on GPIOLIB
sound/soc/codecs/Makefile +2 -0
@@ -359,6 +359,7 @@ snd-soc-zl38060-objs := zl38060.o snd-soc-max9877-objs := max9877.o snd-soc-max98504-objs := max98504.o snd-soc-simple-amplifier-objs := simple-amplifier.o +snd-soc-simple-iio-aux-objs := simple-iio-aux.o snd-soc-tpa6130a2-objs := tpa6130a2.o snd-soc-tas2552-objs := tas2552.o snd-soc-tas2562-objs := tas2562.o
@@ -729,6 +730,7 @@ obj-$(CONFIG_SND_SOC_ZL38060) += snd-soc-zl38060.o obj-$(CONFIG_SND_SOC_MAX9877) += snd-soc-max9877.o obj-$(CONFIG_SND_SOC_MAX98504) += snd-soc-max98504.o obj-$(CONFIG_SND_SOC_SIMPLE_AMPLIFIER) += snd-soc-simple-amplifier.o +obj-$(CONFIG_SND_SOC_SIMPLE_IIO_AUX) += snd-soc-simple-iio-aux.o obj-$(CONFIG_SND_SOC_TPA6130A2) += snd-soc-tpa6130a2.o obj-$(CONFIG_SND_SOC_LPASS_MACRO_COMMON) += snd-soc-lpass-macro-common.o obj-$(CONFIG_SND_SOC_LPASS_WSA_MACRO) += snd-soc-lpass-wsa-macro.o
sound/soc/codecs/simple-iio-aux.c new file +307 -0
307 lines not on this page · raw message
Signature · 1 line
2.39.2
Jonathan Cameron
maintainerRe: [PATCH 4/4] ASoC: codecs: Add support for the generic IIO auxiliary devices
Quoted 8 lines · On Fri, 21 Apr 2023 14:41:22 +0200 Herve Codina <[email protected]>…
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]>
Hi Herve,
There are some other IIO devices that might turn up in audio paths. In theory someone might put an IIO supported amplifier in there (though current ones are far to high frequency and expensive for that to make sense). For now it probably makes sense to support potentiometers as you are doing here, though I'm guessing that in many cases they would be used with some other analog components. Does the transfer function matter at all?
Been many years since I last touched anything in ASoC so questions may be silly ;)
A few comments inline.
Jonathan
sound/soc/codecs/simple-iio-aux.c line 49
Quoted 7 lines
sound/soc/codecs/simple-iio-aux.c new file +7
-0+static int simple_iio_aux_get_volsw (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct simple_iio_aux_chan *chan = (struct simple_iio_aux_chan *)kcontrol->private_value; +int max = chan->max; +int min = chan->min; +unsigned int mask = (1 <<fls (max)) -1 ;
As below. I'm not following reason for use of mask
Quoted 12 lines
sound/soc/codecs/simple-iio-aux.c new file +12
-0+unsigned int invert = chan->is_inverted; +int ret; +int val; + + ret =iio_read_channel_raw (chan->iio_chan, &val); +if (ret <0 ) +return ret; + + ucontrol->value.integer.value[0 ] = (val & mask) - min; +if (invert) + ucontrol->value.integer.value[0 ] = max - ucontrol->value.integer.value[0 ]; +
sound/soc/codecs/simple-iio-aux.c line 71
Quoted 10 lines
sound/soc/codecs/simple-iio-aux.c new file +10
-0+return 0 ; +} + +static int simple_iio_aux_put_volsw (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct simple_iio_aux_chan *chan = (struct simple_iio_aux_chan *)kcontrol->private_value; +int max = chan->max; +int min = chan->min; +unsigned int mask = (1 <<fls (max)) -1 ;
Why is mask needed? Also seems like handling is making some strong assumptions on form of max and min. So at minimum some comments on reasoning needed.
Quoted 29 lines
sound/soc/codecs/simple-iio-aux.c new file +29
-0+unsigned int invert = chan->is_inverted; +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) & mask; +if (invert) + 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 */ +} +
...
Quoted 23 lines
sound/soc/codecs/simple-iio-aux.c new file +23
-0+static int simple_iio_aux_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct simple_iio_aux_chan *iio_aux_chan; +struct simple_iio_aux *iio_aux; +int count; +u32 tmp; +int ret; +int i; + + iio_aux =devm_kzalloc (&pdev->dev,sizeof (*iio_aux),GFP_KERNEL ); +if (!iio_aux) +return -ENOMEM ; + + iio_aux->dev = &pdev->dev; + + count =of_property_count_strings (np,"io-channel-names" ); +if (count <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names\n" , np); +return count; + } + + iio_aux->chans = devm_kmalloc_array(&pdev->dev, count,
sound/soc/codecs/simple-iio-aux.c line 261
Quoted 10 lines
sound/soc/codecs/simple-iio-aux.c new file +10
-0+ sizeof(*iio_aux->chans), GFP_KERNEL); +if (!iio_aux->chans) +return -ENOMEM ; + iio_aux->num_chans = count; + +for (i =0 ; i < iio_aux->num_chans; i++) { + iio_aux_chan = iio_aux->chans + i; + + ret =of_property_read_string_index (np,"io-channel-names" , i, + &iio_aux_chan->name);
Whilst today this will be tightly couple with of, if you can use generic firmware handling where possible (from linux/property.h) it will reduce what needs to be tidied up if anyone fills in the gaps for IIO consumer bindings in ACPI and then someone uses PRP0001 based ACPI bindings.
sound/soc/codecs/simple-iio-aux.c line 263
Quoted 2 lines
sound/soc/codecs/simple-iio-aux.c new file +2
-0+if (ret <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names[%d]\n" , np, i);
dev_err_probe() would simplify these cases a little. Not sure on ASOC view on using that for cases that won't defer. I tend to take the view it's nicer everywhere for calls in probe() functions.
sound/soc/codecs/simple-iio-aux.c line 269
Quoted 6 lines
sound/soc/codecs/simple-iio-aux.c new file +6
-0+return ret; + } + + iio_aux_chan->iio_chan =devm_iio_channel_get (iio_aux->dev, iio_aux_chan->name); +if (IS_ERR (iio_aux_chan->iio_chan)) { + ret =PTR_ERR (iio_aux_chan->iio_chan);
Put that inline instead of setting ret here.
sound/soc/codecs/simple-iio-aux.c line 277
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c new file +8
-0+return dev_err_probe (iio_aux->dev, ret, +"get IIO channel '%s' failed (%d)\n" , + iio_aux_chan->name, ret); + } + + tmp =0 ; +of_property_read_u32_index (np,"invert" , i, &tmp); + iio_aux_chan->is_inverted = !!tmp;
As it's a bool this is the same as
iio_aux_chan->is_inverted = tmp;
Quoted 11 lines
sound/soc/codecs/simple-iio-aux.c new file +11
-0+ } + +platform_set_drvdata (pdev, iio_aux); + +return devm_snd_soc_register_component (iio_aux->dev, + &simple_iio_aux_component_driver, +NULL ,0 ); +} + +#ifdef CONFIG_OF +static const struct of_device_id simple_iio_aux_ids[] = {
sound/soc/codecs/simple-iio-aux.c line 298
Quoted 10 lines
sound/soc/codecs/simple-iio-aux.c new file +10
-0+ { .compatible = "simple-iio-aux", }, + { } +}; +MODULE_DEVICE_TABLE (of, simple_iio_aux_ids); +#endif + +static struct platform_driver simple_iio_aux_driver = { + .driver = { + .name ="simple-iio-aux" , + .of_match_table =of_match_ptr (simple_iio_aux_ids),
I'd just drop the of_match_ptr() Whilst this won't work today with other firmwares, we might enable the missing parts at some stage. Also the driver is somewhat pointless without DT so I'd just assume it's always built with it. Cost is a tiny array on systems with a weird .config
Quoted 9 lines
sound/soc/codecs/simple-iio-aux.c new file +9
-0+ }, + .probe = simple_iio_aux_probe, +}; + +module_platform_driver (simple_iio_aux_driver); + +MODULE_AUTHOR ("Herve Codina <[email protected]>" ); +MODULE_DESCRIPTION ("IIO ALSA SoC aux driver" ); +MODULE_LICENSE ("GPL" );
Re: [PATCH 4/4] ASoC: codecs: Add support for the generic IIO auxiliary devices
Hi Jonathan, Mark,
Quoted 56 lines · On Sat, 22 Apr 2023 18:08:14 +0100 Jonathan Cameron <[email protected]> wrote:
Quoted 8 lines · On Fri, 21 Apr 2023 14:41:22 +0200 Herve Codina <[email protected]>…
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]>Hi Herve,
There are some other IIO devices that might turn up in audio paths. In theory someone might put an IIO supported amplifier in there (though current ones are far to high frequency and expensive for that to make sense). For now it probably makes sense to support potentiometers as you are doing here, though I'm guessing that in many cases they would be used with some other analog components. Does the transfer function matter at all?
Been many years since I last touched anything in ASoC so questions may be silly ;)
A few comments inline.
Jonathan
sound/soc/codecs/simple-iio-aux.c line 49
Quoted 7 lines
sound/soc/codecs/simple-iio-aux.c new file +7
-0+static int simple_iio_aux_get_volsw (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct simple_iio_aux_chan *chan = (struct simple_iio_aux_chan *)kcontrol->private_value; +int max = chan->max; +int min = chan->min; +unsigned int mask = (1 <<fls (max)) -1 ;As below. I'm not following reason for use of mask
Quoted 18 lines
sound/soc/codecs/simple-iio-aux.c new file +18
-0+unsigned int invert = chan->is_inverted; +int ret; +int val; + + ret =iio_read_channel_raw (chan->iio_chan, &val); +if (ret <0 ) +return ret; + + ucontrol->value.integer.value[0 ] = (val & mask) - min; +if (invert) + ucontrol->value.integer.value[0 ] = max - ucontrol->value.integer.value[0 ]; + +return 0 ; +} + +static int simple_iio_aux_put_volsw (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c line 49
Quoted 4 lines
sound/soc/codecs/simple-iio-aux.c new file +4
-0+struct simple_iio_aux_chan *chan = (struct simple_iio_aux_chan *)kcontrol->private_value; +int max = chan->max; +int min = chan->min; +unsigned int mask = (1 <<fls (max)) -1 ;Why is mask needed? Also seems like handling is making some strong assumptions on form of max and min. So at minimum some comments on reasoning needed.
This mask was present in the internal ASoC helpers used when devices can be accessed using regmap.
The IIO accesses done by simple_iio_aux_get_volsw() and simple_iio_aux_put_volsw() were based on these internal helpers. Not sure about the exact reason to this mask. Maybe Mark can answer.
For these particular use-cases using an IIO channel, the mask present in simple_iio_aux_get_volsw() and simple_iio_aux_put_volsw() can be removed.
I will remove in the next iteration except if Mark tell me to keep them.
Quoted 65 lines
Quoted 30 lines
sound/soc/codecs/simple-iio-aux.c new file +29
-0+unsigned int invert = chan->is_inverted; +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) & mask; +if (invert) + 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 */ +} +...
Quoted 30 lines
sound/soc/codecs/simple-iio-aux.c new file +30
-0+static int simple_iio_aux_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct simple_iio_aux_chan *iio_aux_chan; +struct simple_iio_aux *iio_aux; +int count; +u32 tmp; +int ret; +int i; + + iio_aux =devm_kzalloc (&pdev->dev,sizeof (*iio_aux),GFP_KERNEL ); +if (!iio_aux) +return -ENOMEM ; + + iio_aux->dev = &pdev->dev; + + count =of_property_count_strings (np,"io-channel-names" ); +if (count <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names\n" , np); +return count; + } + + iio_aux->chans =devm_kmalloc_array (&pdev->dev, count, +sizeof (*iio_aux->chans),GFP_KERNEL ); +if (!iio_aux->chans) +return -ENOMEM ; + iio_aux->num_chans = count; + +for (i =0 ; i < iio_aux->num_chans; i++) { + iio_aux_chan = iio_aux->chans + i;
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c line 261
Quoted 3 lines
sound/soc/codecs/simple-iio-aux.c new file +3
-0+ + ret =of_property_read_string_index (np,"io-channel-names" , i, + &iio_aux_chan->name);Whilst today this will be tightly couple with of, if you can use generic firmware handling where possible (from
linux/property.h) it will reduce what needs to be tidied up if anyone fills in the gaps for IIO consumer bindings in ACPI and then someone uses PRP0001 based ACPI bindings.
No device_property_read_*() function family are available to get a value from an array using an index.
I would prefer to keep the of_property_read_*() function family I use for this first IIO auxiliary device support.
Quoted 7 lines
sound/soc/codecs/simple-iio-aux.c line 263
Quoted 3 lines
sound/soc/codecs/simple-iio-aux.c new file +2
-0+if (ret <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names[%d]\n" , np, i);
dev_err_probe()would simplify these cases a little. Not sure on ASOC view on using that for cases that won't defer. I tend to take the view it's nicer everywhere for calls inprobe()functions.
I have the feeling that ASoC uses dev_err_probe() for cases that can defer. Mark, can you confirm ?
Quoted 10 lines
sound/soc/codecs/simple-iio-aux.c line 269
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c new file +6
-0+return ret; + } + + iio_aux_chan->iio_chan =devm_iio_channel_get (iio_aux->dev, iio_aux_chan->name); +if (IS_ERR (iio_aux_chan->iio_chan)) { + ret =PTR_ERR (iio_aux_chan->iio_chan);Put that inline instead of setting ret here.
Will be done in the next iteration.
Quoted 11 lines
sound/soc/codecs/simple-iio-aux.c line 277
Quoted 9 lines
sound/soc/codecs/simple-iio-aux.c new file +8
-0+return dev_err_probe (iio_aux->dev, ret, +"get IIO channel '%s' failed (%d)\n" , + iio_aux_chan->name, ret); + } + + tmp =0 ; +of_property_read_u32_index (np,"invert" , i, &tmp); + iio_aux_chan->is_inverted = !!tmp;As it's a bool this is the same as
Quoted 1 line
iio_aux_chan->is_inverted = tmp;
I will remove the '!!' construction.
Quoted 20 lines
Quoted 20 lines
sound/soc/codecs/simple-iio-aux.c new file +19
-0+ } + +platform_set_drvdata (pdev, iio_aux); + +return devm_snd_soc_register_component (iio_aux->dev, + &simple_iio_aux_component_driver, +NULL ,0 ); +} + +#ifdef CONFIG_OF +static const struct of_device_id simple_iio_aux_ids[] = { + { .compatible ="simple-iio-aux" , }, + { } +}; +MODULE_DEVICE_TABLE (of, simple_iio_aux_ids); +#endif + +static struct platform_driver simple_iio_aux_driver = { + .driver = {
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c line 298
Quoted 2 lines
sound/soc/codecs/simple-iio-aux.c new file +2
-0+ .name = "simple-iio-aux", + .of_match_table = of_match_ptr(simple_iio_aux_ids),I'd just drop the
of_match_ptr()Whilst this won't work today with other firmwares, we might enable the missing parts at some stage. Also the driver is somewhat pointless without DT so I'd just assume it's always built with it. Cost is a tiny array on systems with a weird .config
of_match_ptr will be removed (and the #ifdef CONFIG_OF also).
Quoted 11 lines
sound/soc/codecs/simple-iio-aux.c line 307
Quoted 10 lines
sound/soc/codecs/simple-iio-aux.c new file +9
-0+ }, + .probe = simple_iio_aux_probe, +}; + +module_platform_driver (simple_iio_aux_driver); + +MODULE_AUTHOR ("Herve Codina <[email protected]>" ); +MODULE_DESCRIPTION ("IIO ALSA SoC aux driver" ); +MODULE_LICENSE ("GPL" );
Thanks for the review.
Best regards,
Hervé
Jonathan Cameron
maintainerRe: [PATCH 4/4] ASoC: codecs: Add support for the generic IIO auxiliary devices
Quoted 41 lines
Quoted 32 lines
Quoted 32 lines
sound/soc/codecs/simple-iio-aux.c new file +30
-0+static int simple_iio_aux_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct simple_iio_aux_chan *iio_aux_chan; +struct simple_iio_aux *iio_aux; +int count; +u32 tmp; +int ret; +int i; + + iio_aux =devm_kzalloc (&pdev->dev,sizeof (*iio_aux),GFP_KERNEL ); +if (!iio_aux) +return -ENOMEM ; + + iio_aux->dev = &pdev->dev; + + count =of_property_count_strings (np,"io-channel-names" ); +if (count <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names\n" , np); +return count; + } + + iio_aux->chans =devm_kmalloc_array (&pdev->dev, count, +sizeof (*iio_aux->chans),GFP_KERNEL ); +if (!iio_aux->chans) +return -ENOMEM ; + iio_aux->num_chans = count; + +for (i =0 ; i < iio_aux->num_chans; i++) { + iio_aux_chan = iio_aux->chans + i;Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c line 261
Quoted 3 lines
sound/soc/codecs/simple-iio-aux.c new file +3
-0+ + ret =of_property_read_string_index (np,"io-channel-names" , i, + &iio_aux_chan->name);Whilst today this will be tightly couple with of, if you can use generic firmware handling where possible (from
linux/property.h) it will reduce what needs to be tidied up if anyone fills in the gaps for IIO consumer bindings in ACPI and then someone uses PRP0001 based ACPI bindings.
Quoted 2 lines
No device_property_read_*() function family are available to get a value from an array using an index.
That feels like it might be a feature gap in the generic property handling that should be solved. Emtirely reasonable not to do it in this series however!
Quoted 12 lines
I would prefer to keep the of_property_read_*() function family I use for this first IIO auxiliary device support.
Quoted 7 lines
sound/soc/codecs/simple-iio-aux.c line 263
Quoted 3 lines
sound/soc/codecs/simple-iio-aux.c new file +2
-0+if (ret <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names[%d]\n" , np, i);
dev_err_probe()would simplify these cases a little. Not sure on ASOC view on using that for cases that won't defer. I tend to take the view it's nicer everywhere for calls inprobe()functions.
Quoted 3 lines
I have the feeling that ASoC uses
dev_err_probe()for cases that can defer. Mark, can you confirm ?
Left as needs an answer from Mark.
Jonathan
Re: [PATCH 4/4] ASoC: codecs: Add support for the generic IIO auxiliary devices
In reply to Jonathan CameronHi Jonathan, Mark,
Quoted 56 lines · On Sat, 22 Apr 2023 18:08:14 +0100 Jonathan Cameron <[email protected]> wrote:
Quoted 8 lines · On Fri, 21 Apr 2023 14:41:22 +0200 Herve Codina <[email protected]>…
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]>Hi Herve,
There are some other IIO devices that might turn up in audio paths. In theory someone might put an IIO supported amplifier in there (though current ones are far to high frequency and expensive for that to make sense). For now it probably makes sense to support potentiometers as you are doing here, though I'm guessing that in many cases they would be used with some other analog components. Does the transfer function matter at all?
Been many years since I last touched anything in ASoC so questions may be silly ;)
A few comments inline.
Jonathan
sound/soc/codecs/simple-iio-aux.c line 49
Quoted 7 lines
sound/soc/codecs/simple-iio-aux.c new file +7
-0+static int simple_iio_aux_get_volsw (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{ +struct simple_iio_aux_chan *chan = (struct simple_iio_aux_chan *)kcontrol->private_value; +int max = chan->max; +int min = chan->min; +unsigned int mask = (1 <<fls (max)) -1 ;As below. I'm not following reason for use of mask
Quoted 18 lines
sound/soc/codecs/simple-iio-aux.c new file +18
-0+unsigned int invert = chan->is_inverted; +int ret; +int val; + + ret =iio_read_channel_raw (chan->iio_chan, &val); +if (ret <0 ) +return ret; + + ucontrol->value.integer.value[0 ] = (val & mask) - min; +if (invert) + ucontrol->value.integer.value[0 ] = max - ucontrol->value.integer.value[0 ]; + +return 0 ; +} + +static int simple_iio_aux_put_volsw (struct snd_kcontrol *kcontrol, +struct snd_ctl_elem_value *ucontrol) +{
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c line 49
Quoted 4 lines
sound/soc/codecs/simple-iio-aux.c new file +4
-0+struct simple_iio_aux_chan *chan = (struct simple_iio_aux_chan *)kcontrol->private_value; +int max = chan->max; +int min = chan->min; +unsigned int mask = (1 <<fls (max)) -1 ;Why is mask needed? Also seems like handling is making some strong assumptions on form of max and min. So at minimum some comments on reasoning needed.
This mask was present in the internal ASoC helpers used when devices can be accessed using regmap.
The IIO accesses done by simple_iio_aux_get_volsw() and simple_iio_aux_put_volsw() were based on these internal helpers. Not sure about the exact reason to this mask. Maybe Mark can answer.
For these particular use-cases using an IIO channel, the mask present in simple_iio_aux_get_volsw() and simple_iio_aux_put_volsw() can be removed.
I will remove in the next iteration except if Mark tell me to keep them.
Quoted 65 lines
Quoted 30 lines
sound/soc/codecs/simple-iio-aux.c new file +29
-0+unsigned int invert = chan->is_inverted; +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) & mask; +if (invert) + 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 */ +} +...
Quoted 30 lines
sound/soc/codecs/simple-iio-aux.c new file +30
-0+static int simple_iio_aux_probe (struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct simple_iio_aux_chan *iio_aux_chan; +struct simple_iio_aux *iio_aux; +int count; +u32 tmp; +int ret; +int i; + + iio_aux =devm_kzalloc (&pdev->dev,sizeof (*iio_aux),GFP_KERNEL ); +if (!iio_aux) +return -ENOMEM ; + + iio_aux->dev = &pdev->dev; + + count =of_property_count_strings (np,"io-channel-names" ); +if (count <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names\n" , np); +return count; + } + + iio_aux->chans =devm_kmalloc_array (&pdev->dev, count, +sizeof (*iio_aux->chans),GFP_KERNEL ); +if (!iio_aux->chans) +return -ENOMEM ; + iio_aux->num_chans = count; + +for (i =0 ; i < iio_aux->num_chans; i++) { + iio_aux_chan = iio_aux->chans + i;
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c line 261
Quoted 3 lines
sound/soc/codecs/simple-iio-aux.c new file +3
-0+ + ret =of_property_read_string_index (np,"io-channel-names" , i, + &iio_aux_chan->name);Whilst today this will be tightly couple with of, if you can use generic firmware handling where possible (from
linux/property.h) it will reduce what needs to be tidied up if anyone fills in the gaps for IIO consumer bindings in ACPI and then someone uses PRP0001 based ACPI bindings.
No device_property_read_*() function family are available to get a value from an array using an index.
I would prefer to keep the of_property_read_*() function family I use for this first IIO auxiliary device support.
Quoted 7 lines
sound/soc/codecs/simple-iio-aux.c line 263
Quoted 3 lines
sound/soc/codecs/simple-iio-aux.c new file +2
-0+if (ret <0 ) { +dev_err (iio_aux->dev,"%pOF: failed to read io-channel-names[%d]\n" , np, i);
dev_err_probe()would simplify these cases a little. Not sure on ASOC view on using that for cases that won't defer. I tend to take the view it's nicer everywhere for calls inprobe()functions.
I have the feeling that ASoC uses dev_err_probe() for cases that can defer. Mark, can you confirm ?
Quoted 10 lines
sound/soc/codecs/simple-iio-aux.c line 269
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c new file +6
-0+return ret; + } + + iio_aux_chan->iio_chan =devm_iio_channel_get (iio_aux->dev, iio_aux_chan->name); +if (IS_ERR (iio_aux_chan->iio_chan)) { + ret =PTR_ERR (iio_aux_chan->iio_chan);Put that inline instead of setting ret here.
Will be done in the next iteration.
Quoted 11 lines
sound/soc/codecs/simple-iio-aux.c line 277
Quoted 9 lines
sound/soc/codecs/simple-iio-aux.c new file +8
-0+return dev_err_probe (iio_aux->dev, ret, +"get IIO channel '%s' failed (%d)\n" , + iio_aux_chan->name, ret); + } + + tmp =0 ; +of_property_read_u32_index (np,"invert" , i, &tmp); + iio_aux_chan->is_inverted = !!tmp;As it's a bool this is the same as
Quoted 1 line
iio_aux_chan->is_inverted = tmp;
I will remove the '!!' construction.
Quoted 20 lines
Quoted 20 lines
sound/soc/codecs/simple-iio-aux.c new file +19
-0+ } + +platform_set_drvdata (pdev, iio_aux); + +return devm_snd_soc_register_component (iio_aux->dev, + &simple_iio_aux_component_driver, +NULL ,0 ); +} + +#ifdef CONFIG_OF +static const struct of_device_id simple_iio_aux_ids[] = { + { .compatible ="simple-iio-aux" , }, + { } +}; +MODULE_DEVICE_TABLE (of, simple_iio_aux_ids); +#endif + +static struct platform_driver simple_iio_aux_driver = { + .driver = {
Quoted 8 lines
sound/soc/codecs/simple-iio-aux.c line 298
Quoted 2 lines
sound/soc/codecs/simple-iio-aux.c new file +2
-0+ .name = "simple-iio-aux", + .of_match_table = of_match_ptr(simple_iio_aux_ids),I'd just drop the
of_match_ptr()Whilst this won't work today with other firmwares, we might enable the missing parts at some stage. Also the driver is somewhat pointless without DT so I'd just assume it's always built with it. Cost is a tiny array on systems with a weird .config
of_match_ptr will be removed (and the #ifdef CONFIG_OF also).
Quoted 11 lines
sound/soc/codecs/simple-iio-aux.c line 307
Quoted 10 lines
sound/soc/codecs/simple-iio-aux.c new file +9
-0+ }, + .probe = simple_iio_aux_probe, +}; + +module_platform_driver (simple_iio_aux_driver); + +MODULE_AUTHOR ("Herve Codina <[email protected]>" ); +MODULE_DESCRIPTION ("IIO ALSA SoC aux driver" ); +MODULE_LICENSE ("GPL" );
Thanks for the review.
Best regards,
Hervé
PATCH 1/4 dt-bindings: sound: Add simple-iio-aux
In reply to Herve CodinaIndustrial 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.
simple-iio-aux allows to consider these Industrial I/O devices as auxliary audio devices.
--- .../bindings/sound/simple-iio-aux.yaml | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/simple-iio-aux.yaml
Documentation/devicetree/bindings/sound/simple-iio-aux.yaml new file +65 -0
@@ -0,0 +1,65 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/sound/simple-iio-aux.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple IIO auxiliary + +maintainers: + - Herve Codina <[email protected]> + +description: | + Auxiliary device based on Industrial I/O device channels + +allOf: + - $ref: /schemas/iio/iio-consumer.yaml + - $ref: dai-common.yaml# + +properties: + compatible: + const: simple-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. + + invert: + $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: + - | + aux { + compatible = "simple-iio-aux"; + io-channels = <&iio 0>, <&iio 1>, <&iio 2>, <&iio 3>; + io-channel-names = "CH0", "CH1", "CH2", "CH3"; + /* Invert CH1 and CH2 */ + invert = <0 1 1>; + };
Signature · 1 line
2.39.2
PATCH 2/4 iio: inkern: Add a helper to query an available minimum raw value
In reply to Herve CodinaA 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().
--- drivers/iio/inkern.c | 67 ++++++++++++++++++++++++++++++++++++ include/linux/iio/consumer.h | 11 ++++++ 2 files changed, 78 insertions(+)
drivers/iio/inkern.c +67 -0
@@ -912,6 +912,73 @@ 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) +{ +int unused; +const int *vals; +int length; +int ret; + +if (!val2) + val2 = &unused; + + ret =iio_channel_read_avail (chan, &vals, type, &length, info); +switch (ret) { +case IIO_AVAIL_RANGE : +switch (*type) { +case IIO_VAL_INT : + *val = vals[0 ]; +break ; +default : + *val = vals[0 ]; + *val2 = vals[1 ]; + } +return 0 ; + +case IIO_AVAIL_LIST : +if (length <=0 ) +return -EINVAL ; +switch (*type) { +case IIO_VAL_INT : + *val = vals[--length]; +while (length) { +if (vals[--length] < *val) + *val = vals[length]; + } +break ; +default : +/* FIXME: learn about min for other iio values */ +return -EINVAL ; + } +return 0 ; + +default : +return ret; + } +} + +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);
include/linux/iio/consumer.h +11 -0
@@ -297,6 +297,17 @@ 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 raw reads from iio channels are in adc counts and hence +* scale will need to be applied if standard units are required. +*/ +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.
Signature · 1 line
2.39.2
PATCH 3/4 ASoC: soc-dapm.h: Add a helper to build a DAPM widget dynamically
In reply to Herve CodinaThe SND_SOC_DAPM_* helpers family are used to build widgets array in a static way.
Introduce SND_SOC_DAPM_WIDGET() in order to use the SND_SOC_DAPM_* helpers family in a dynamic way. The different SND_SOC_DAPM_* parameters can be computed by the code and the widget can be built based on this parameter computation.
For instance:
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_WIDGET(SND_SOC_DAPM_INPUT(name));
...}
--- include/sound/soc-dapm.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
include/sound/soc-dapm.h +11 -1
@@ -276,7 +276,17 @@ struct soc_enum; .reg = SND_SOC_NOPM, .event = dapm_pinctrl_event, \ .event_flags = SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD } - +/* +* Helper to create a widget 'dynamically' +* This can be used with any of the SND_SOC_DAPM_* widget helper. +* For instance: +* struct snd_soc_dapm_widget widget; +* ... +* widget = SND_SOC_DAPM_WIDGET(SND_SOC_DAPM_INPUT(input_name)); +*/ +#define SND_SOC_DAPM_WIDGET (_widget)({ \ + struct snd_soc_dapm_widget _w[1] = { _widget }; \ + _w[0]; })/* dapm kcontrol types */ #define SOC_DAPM_DOUBLE (xname, reg, lshift, rshift, max, invert) \
Signature · 1 line
2.39.2
PATCH 4/4 ASoC: codecs: Add support for the generic IIO auxiliary devices
In reply to Herve CodinaIndustrial 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.
--- sound/soc/codecs/Kconfig | 12 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/simple-iio-aux.c | 307 ++++++++++++++++++++++++++++++ 3 files changed, 321 insertions(+) create mode 100644 sound/soc/codecs/simple-iio-aux.c
sound/soc/codecs/Kconfig +12 -0
@@ -211,6 +211,7 @@ config SND_SOC_ALL_CODECS imply SND_SOC_SGTL5000 imply SND_SOC_SI476X imply SND_SOC_SIMPLE_AMPLIFIER + imply SND_SOC_SIMPLE_IIO_AUX imply SND_SOC_SIMPLE_MUX imply SND_SOC_SMA1303 imply SND_SOC_SPDIF
@@ -1555,6 +1556,17 @@ config SND_SOC_SIGMADSP_REGMAP config SND_SOC_SIMPLE_AMPLIFIER tristate "Simple Audio Amplifier" +config SND_SOC_SIMPLE_IIO_AUX + tristate "Simple 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-simple-iio-aux. + config SND_SOC_SIMPLE_MUX tristate "Simple Audio Mux" depends on GPIOLIB
sound/soc/codecs/Makefile +2 -0
@@ -359,6 +359,7 @@ snd-soc-zl38060-objs := zl38060.o snd-soc-max9877-objs := max9877.o snd-soc-max98504-objs := max98504.o snd-soc-simple-amplifier-objs := simple-amplifier.o +snd-soc-simple-iio-aux-objs := simple-iio-aux.o snd-soc-tpa6130a2-objs := tpa6130a2.o snd-soc-tas2552-objs := tas2552.o snd-soc-tas2562-objs := tas2562.o
@@ -729,6 +730,7 @@ obj-$(CONFIG_SND_SOC_ZL38060) += snd-soc-zl38060.o obj-$(CONFIG_SND_SOC_MAX9877) += snd-soc-max9877.o obj-$(CONFIG_SND_SOC_MAX98504) += snd-soc-max98504.o obj-$(CONFIG_SND_SOC_SIMPLE_AMPLIFIER) += snd-soc-simple-amplifier.o +obj-$(CONFIG_SND_SOC_SIMPLE_IIO_AUX) += snd-soc-simple-iio-aux.o obj-$(CONFIG_SND_SOC_TPA6130A2) += snd-soc-tpa6130a2.o obj-$(CONFIG_SND_SOC_LPASS_MACRO_COMMON) += snd-soc-lpass-macro-common.o obj-$(CONFIG_SND_SOC_LPASS_WSA_MACRO) += snd-soc-lpass-wsa-macro.o
sound/soc/codecs/simple-iio-aux.c new file +307 -0
307 lines not on this page · raw message
Signature · 1 line
2.39.2