Re: [PATCH v2] ASoC: dt-bindings: simple-card: add triggers properties

[Maxim Kochetkov]

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?