[PATCH v2] Add support for trigger-start/stop for simple audio card
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(+)
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 fido_max@inbox.ru --- 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,
The trigger-start/stop properties allows to specify DAI link trigger ordering method.
Signed-off-by: Maxim Kochetkov fido_max@inbox.ru --- .../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]+)?$":
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 fido_max@inbox.ru
.../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?
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 fido_max@inbox.ru --- 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)
participants (2)
-
Maxim Kochetkov
-
Rob Herring