Support for trigger-start/stop for simple audio card.
Maxim Kochetkov (3):
ASoC: simple-card-utils: introduce asoc_simple_parse_triggers()
ASoC: dt-bindings: simple-card: add triggers properties
ASoC: simple-card: add triggers parsing from DT node support
.../bindings/sound/simple-card.yaml | 31 +++++++++++++
include/sound/simple_card_utils.h | 3 ++
sound/soc/generic/simple-card-utils.c | 45 +++++++++++++++++++
sound/soc/generic/simple-card.c | 4 ++
4 files changed, 83 insertions(+)
--
2.40.1
It may be useful to specify trigger-start/stop for some DMA-based
simple audio card. So add this "trigger-start/stop" device tree
entry parser.
Signed-off-by: Maxim Kochetkov <[email protected]>
---
sound/soc/generic/simple-card.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index a78babf44f38..f934534d5c94 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -181,6 +181,10 @@ static int simple_link_init(struct asoc_simple_priv *priv,
struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
int ret;
+ ret = asoc_simple_parse_triggers(node, prefix, dai_link);
+ if (ret)
+ return ret;
+
ret = asoc_simple_parse_daifmt(dev, node, codec,
prefix, &dai_link->dai_fmt);
if (ret < 0)
--
2.40.1
This function parses DT DAI link triggers params and assigns
the appropriate fields in struct snd_soc_dai_link.
It supports two triggers trigger-stop and trigger-start.
Signed-off-by: Maxim Kochetkov <[email protected]>
---
include/sound/simple_card_utils.h | 3 ++
sound/soc/generic/simple-card-utils.c | 45 +++++++++++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index b450d5873227..ea8c17f7b280 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -180,6 +180,9 @@ int asoc_simple_parse_widgets(struct snd_soc_card *card,
char *prefix);
int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
char *prefix);
+int asoc_simple_parse_triggers(struct device_node *node,
+ char *prefix,
+ struct snd_soc_dai_link *dai_link);
int asoc_simple_init_jack(struct snd_soc_card *card,
struct asoc_simple_jack *sjack,
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 3019626b0592..58f5c672184a 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -737,6 +737,51 @@ int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
}
EXPORT_SYMBOL_GPL(asoc_simple_parse_pin_switches);
+static enum snd_soc_trigger_order asoc_simple_parse_trigger(
+ struct device_node *node,
+ char *prefix,
+ char *trigger_name)
+{
+ enum snd_soc_trigger_order trigger = SND_SOC_TRIGGER_ORDER_DEFAULT;
+ struct {
+ char *name;
+ unsigned int val;
+ } of_trigger_table[] = {
+ { "default", SND_SOC_TRIGGER_ORDER_DEFAULT },
+ { "ldc", SND_SOC_TRIGGER_ORDER_LDC },
+ };
+ const char *str;
+ char prop[128];
+ int ret;
+
+ if (!prefix)
+ prefix = "";
+
+ snprintf(prop, sizeof(prop), "%s%s", prefix, trigger_name);
+
+ ret = of_property_read_string(node, prop, &str);
+ if (ret == 0) {
+ for (int i = 0; i < ARRAY_SIZE(of_trigger_table); i++) {
+ if (strcmp(str, of_trigger_table[i].name) == 0) {
+ trigger = of_trigger_table[i].val;
+ break;
+ }
+ }
+ }
+
+ return trigger;
+}
+
+int asoc_simple_parse_triggers(struct device_node *node,
+ char *prefix,
+ struct snd_soc_dai_link *dai_link)
+{
+ dai_link->trigger_stop = asoc_simple_parse_trigger(node, prefix, "trigger-stop");
+ dai_link->trigger_start = asoc_simple_parse_trigger(node, prefix, "trigger-start");
+ return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_parse_triggers);
+
int asoc_simple_init_jack(struct snd_soc_card *card,
struct asoc_simple_jack *sjack,
int is_hp, char *prefix,
--
2.40.1
The trigger-start/stop properties allows to specify DAI link
trigger ordering method.
Signed-off-by: Maxim Kochetkov <[email protected]>
---
.../bindings/sound/simple-card.yaml | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
index 59ac2d1d1ccf..f1878d470d83 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.yaml
+++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
@@ -99,6 +99,28 @@ definitions:
description: the widget names for which pin switches must be created.
$ref: /schemas/types.yaml#/definitions/string-array
+ trigger-start:
+ description: |-
+ Start trigger ordering method:
+ default: Link->Component->DAI
+ ldc: Link->DAI->Component
+ $ref: /schemas/types.yaml#/definitions/string
+ items:
+ enum:
+ - default
+ - ldc
+
+ trigger-stop:
+ description: |-
+ Stop trigger ordering method:
+ default: DAI->Component->Link
+ ldc: Component->DAI->Link
+ $ref: /schemas/types.yaml#/definitions/string
+ items:
+ enum:
+ - default
+ - ldc
+
format:
description: audio format.
items:
@@ -210,6 +232,10 @@ properties:
maxItems: 1
simple-audio-card,mic-det-gpio:
maxItems: 1
+ simple-audio-card,trigger-start:
+ $ref: "#/definitions/trigger-start"
+ simple-audio-card,trigger-stop:
+ $ref: "#/definitions/trigger-stop"
patternProperties:
"^simple-audio-card,cpu(@[0-9a-f]+)?$":
@@ -259,6 +285,11 @@ patternProperties:
maxItems: 1
mic-det-gpio:
maxItems: 1
+ trigger-start:
+ $ref: "#/definitions/trigger-start"
+ trigger-stop:
+ $ref: "#/definitions/trigger-stop"
+
patternProperties:
"^cpu(-[0-9]+)?$":
--
2.40.1
On Sat, Jul 15, 2023 at 11:30:43AM +0300, Maxim Kochetkov wrote:
> The trigger-start/stop properties allows to specify DAI link
> trigger ordering method.
Obviously. Why do you need these? What problem does it solve?
I don't think these belong in simple-card. What's next? What if you need
delays between each step? This is the problem with 'simple' or 'generic'
bindings. It's a never ending addition of properties which are not well
thought out.
>
> Signed-off-by: Maxim Kochetkov <[email protected]>
> ---
> .../bindings/sound/simple-card.yaml | 31 +++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
> index 59ac2d1d1ccf..f1878d470d83 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
> @@ -99,6 +99,28 @@ definitions:
> description: the widget names for which pin switches must be created.
> $ref: /schemas/types.yaml#/definitions/string-array
>
> + trigger-start:
> + description: |-
> + Start trigger ordering method:
> + default: Link->Component->DAI
> + ldc: Link->DAI->Component
> + $ref: /schemas/types.yaml#/definitions/string
> + items:
> + enum:
> + - default
Why do you need a value of 'default'? What's the default when the
property is not present?
> + - ldc
> +
> + trigger-stop:
> + description: |-
> + Stop trigger ordering method:
> + default: DAI->Component->Link
> + ldc: Component->DAI->Link
> + $ref: /schemas/types.yaml#/definitions/string
> + items:
> + enum:
> + - default
> + - ldc
> +
> format:
> description: audio format.
> items:
> @@ -210,6 +232,10 @@ properties:
> maxItems: 1
> simple-audio-card,mic-det-gpio:
> maxItems: 1
> + simple-audio-card,trigger-start:
> + $ref: "#/definitions/trigger-start"
> + simple-audio-card,trigger-stop:
> + $ref: "#/definitions/trigger-stop"
Don't continue this 'simple-audio-card,' prefix pattern. With it, no
other binding can use these properties.
>
> patternProperties:
> "^simple-audio-card,cpu(@[0-9a-f]+)?$":
> @@ -259,6 +285,11 @@ patternProperties:
> maxItems: 1
> mic-det-gpio:
> maxItems: 1
> + trigger-start:
> + $ref: "#/definitions/trigger-start"
> + trigger-stop:
> + $ref: "#/definitions/trigger-stop"
> +
>
> patternProperties:
> "^cpu(-[0-9]+)?$":
> --
> 2.40.1
>
On 19.07.2023 01:08, Rob Herring wrote:
> On Sat, Jul 15, 2023 at 11:30:43AM +0300, Maxim Kochetkov wrote:
>> The trigger-start/stop properties allows to specify DAI link
>> trigger ordering method.
>
> Obviously. Why do you need these? What problem does it solve?
It allows using simple card for some DMA-based CPU component which
requires different start/stop sequence (stop DMA before CPU component,
start DMA after CPU component).
There are a lot of boards which have no audio codec on board. It has
only I2S/TDM/etc... and you can attach any external codec on its pins.
It looks like simple audio card is enough for this cases. It is much
better than to copy-paste simple audio card code to the new custom
driver with new combination of CPU/codec.
>
> I don't think these belong in simple-card. What's next? What if you need
> delays between each step? This is the problem with 'simple' or 'generic'
> bindings. It's a never ending addition of properties which are not well
> thought out.
Can you please suggest the better way to specify start/stop trigger
order via DT?
>> + trigger-start:
>> + description: |-
>> + Start trigger ordering method:
>> + default: Link->Component->DAI
>> + ldc: Link->DAI->Component
>> + $ref: /schemas/types.yaml#/definitions/string
>> + items:
>> + enum:
>> + - default
>
> Why do you need a value of 'default'? What's the default when the
> property is not present?
It comes from
enum snd_soc_trigger_order {
SND_SOC_TRIGGER_ORDER_DEFAULT = 0,
SND_SOC_TRIGGER_ORDER_LDC,
SND_SOC_TRIGGER_ORDER_MAX,
};
default value is 0 (SND_SOC_TRIGGER_ORDER_DEFAULT)
>> format:
>> description: audio format.
>> items:
>> @@ -210,6 +232,10 @@ properties:
>> maxItems: 1
>> simple-audio-card,mic-det-gpio:
>> maxItems: 1
>> + simple-audio-card,trigger-start:
>> + $ref: "#/definitions/trigger-start"
>> + simple-audio-card,trigger-stop:
>> + $ref: "#/definitions/trigger-stop"
>
> Don't continue this 'simple-audio-card,' prefix pattern. With it, no
> other binding can use these properties.
Ok.
>
>>
>> patternProperties:
>> "^simple-audio-card,cpu(@[0-9a-f]+)?$":
>> @@ -259,6 +285,11 @@ patternProperties:
>> maxItems: 1
>> mic-det-gpio:
>> maxItems: 1
>> + trigger-start:
>> + $ref: "#/definitions/trigger-start"
>> + trigger-stop:
>> + $ref: "#/definitions/trigger-stop"
>> +
Should I keep only this section?