| Message ID | 20230715083046.108674-3-fido_max@inbox.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ASoC: dt-bindings: simple-card: add triggers properties | expand |
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?
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]+)?$":
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(+)