[PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names

Frank Li posted 4 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Frank Li 1 year, 9 months ago
Some sai only connect one direction dma (rx/tx) in SOC. For example:
imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
for dma-names.

Remove description under dmas because no user use index to get dma channel.
All user use 'dma-names' to get correct dma channel. dma-names already in
'required' list.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
index 2456d958adeef..6f551c68d33db 100644
--- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
@@ -81,15 +81,14 @@ properties:
 
   dmas:
     minItems: 1
-    items:
-      - description: DMA controller phandle and request line for RX
-      - description: DMA controller phandle and request line for TX
+    maxItems: 2
 
   dma-names:
-    minItems: 1
-    items:
-      - const: rx
-      - const: tx
+    oneOf:
+      - items:
+          - const: rx
+          - const: tx
+      - enum: [ rx, tx ]
 
   interrupts:
     items:

-- 
2.34.1
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Conor Dooley 1 year, 9 months ago
On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> Some sai only connect one direction dma (rx/tx) in SOC. For example:
> imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> for dma-names.
> 
> Remove description under dmas because no user use index to get dma channel.
> All user use 'dma-names' to get correct dma channel. dma-names already in
> 'required' list.

Acked-by: Conor Dooley <conor.dooley@microchip.com>

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> index 2456d958adeef..6f551c68d33db 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> @@ -81,15 +81,14 @@ properties:
>  
>    dmas:
>      minItems: 1
> -    items:
> -      - description: DMA controller phandle and request line for RX
> -      - description: DMA controller phandle and request line for TX
> +    maxItems: 2
>  
>    dma-names:
> -    minItems: 1
> -    items:
> -      - const: rx
> -      - const: tx
> +    oneOf:
> +      - items:
> +          - const: rx
> +          - const: tx
> +      - enum: [ rx, tx ]

I'm not entirely sure if this was Rob's suggestion, I got the impression
he was suggesting that in the two items case we'd not care about the
order. But while I think this is different to that suggestion it's also
not wrong.

Thanks,
Conor.
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Frank Li 1 year, 9 months ago
On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > for dma-names.
> > 
> > Remove description under dmas because no user use index to get dma channel.
> > All user use 'dma-names' to get correct dma channel. dma-names already in
> > 'required' list.
> 
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
> 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > index 2456d958adeef..6f551c68d33db 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > @@ -81,15 +81,14 @@ properties:
> >  
> >    dmas:
> >      minItems: 1
> > -    items:
> > -      - description: DMA controller phandle and request line for RX
> > -      - description: DMA controller phandle and request line for TX
> > +    maxItems: 2
> >  
> >    dma-names:
> > -    minItems: 1
> > -    items:
> > -      - const: rx
> > -      - const: tx
> > +    oneOf:
> > +      - items:
> > +          - const: rx
> > +          - const: tx
> > +      - enum: [ rx, tx ]
> 
> I'm not entirely sure if this was Rob's suggestion, I got the impression
> he was suggesting that in the two items case we'd not care about the
> order. But while I think this is different to that suggestion it's also
> not wrong.

I log this at cover-letter. b4 can't support write change log at every
patch yet. Rob's suggest was not work. dt-binding check complain too long
if there are two dma-names = "rx", "tx". 

Frank

> 
> Thanks,
> Conor.
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Rob Herring 1 year, 9 months ago
On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > for dma-names.
> > > 
> > > Remove description under dmas because no user use index to get dma channel.
> > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > 'required' list.
> > 
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > index 2456d958adeef..6f551c68d33db 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > @@ -81,15 +81,14 @@ properties:
> > >  
> > >    dmas:
> > >      minItems: 1
> > > -    items:
> > > -      - description: DMA controller phandle and request line for RX
> > > -      - description: DMA controller phandle and request line for TX
> > > +    maxItems: 2
> > >  
> > >    dma-names:
> > > -    minItems: 1
> > > -    items:
> > > -      - const: rx
> > > -      - const: tx
> > > +    oneOf:
> > > +      - items:
> > > +          - const: rx
> > > +          - const: tx
> > > +      - enum: [ rx, tx ]
> > 
> > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > he was suggesting that in the two items case we'd not care about the
> > order. But while I think this is different to that suggestion it's also
> > not wrong.
> 
> I log this at cover-letter. b4 can't support write change log at every
> patch yet.

It never will (probably). That's because it doesn't need to. You can 
just do it with git. When you edit the commit message, then after the 
tags, Add '---' and put whatever you want after. That works as long as 
the commit is applied from a patch as 'git am' will drop it. 

> Rob's suggest was not work. dt-binding check complain too long
> if there are two dma-names = "rx", "tx". 

So I'm wrong or you didn't have it correct? No way to tell with your 
explanation. Let me give you the exact schema:

dma-names:
  minItems: 1
  items:
    - enum: [ rx, tx ]
    - const: tx

This says we can have 1 or 2 entries. The first entry can be either rx 
or tx. The 2nd entry must be tx. That's what you want. However, '"tx", 
"tx"' is allowed with the above, but we enforce items to be unique 
elsewhere. Or I thought we did, but we relaxed '.*-names$' at some 
point. I'm going to fix that now.

Rob
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Frank Li 1 year, 9 months ago
On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > for dma-names.
> > > > 
> > > > Remove description under dmas because no user use index to get dma channel.
> > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > 'required' list.
> > > 
> > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > 
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > index 2456d958adeef..6f551c68d33db 100644
> > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > @@ -81,15 +81,14 @@ properties:
> > > >  
> > > >    dmas:
> > > >      minItems: 1
> > > > -    items:
> > > > -      - description: DMA controller phandle and request line for RX
> > > > -      - description: DMA controller phandle and request line for TX
> > > > +    maxItems: 2
> > > >  
> > > >    dma-names:
> > > > -    minItems: 1
> > > > -    items:
> > > > -      - const: rx
> > > > -      - const: tx
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - const: rx
> > > > +          - const: tx
> > > > +      - enum: [ rx, tx ]
> > > 
> > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > he was suggesting that in the two items case we'd not care about the
> > > order. But while I think this is different to that suggestion it's also
> > > not wrong.
> > 
> > I log this at cover-letter. b4 can't support write change log at every
> > patch yet.
> 
> It never will (probably). That's because it doesn't need to. You can 
> just do it with git. When you edit the commit message, then after the 
> tags, Add '---' and put whatever you want after. That works as long as 
> the commit is applied from a patch as 'git am' will drop it. 

The key problem is that I don't want to lost notes when respin patches. It
is easy to make mistake when I copy old serise change logs.

Previously I use git notes + git-rebase, it work fine. Notes can be kept
when I do rebase and git commit --amend. But one thing is not good. 
git send-email --to-cmd=./script/get_maintainer.sh *.patch. It can't
combine all patches's maitainer to a list. It looks like difference patch
will be difference --to list.

b4 can help some case. But can't keep git-notes information when rebase.
It should be git bugs or feature missed.

> 
> > Rob's suggest was not work. dt-binding check complain too long
> > if there are two dma-names = "rx", "tx". 
> 
> So I'm wrong or you didn't have it correct? No way to tell with your 
> explanation. Let me give you the exact schema:
> 
> dma-names:
>   minItems: 1
>   items:
>     - enum: [ rx, tx ]
>     - const: tx
> 
> This says we can have 1 or 2 entries. The first entry can be either rx 
> or tx. The 2nd entry must be tx. That's what you want. However, '"tx", 
> "tx"' is allowed with the above, but we enforce items to be unique 
> elsewhere. Or I thought we did, but we relaxed '.*-names$' at some 
> point. I'm going to fix that now.

Conor find out my problem. The below code works. I missed maxItems.

  dma-names:
     minItems: 1
     maxItems: 2
     items
       enum: [ rx, tx ]

Frank

> 
> Rob
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Rob Herring 1 year, 9 months ago
On Wed, Mar 6, 2024 at 3:19 PM Frank Li <Frank.li@nxp.com> wrote:
>
> On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> > On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > > for dma-names.
> > > > >
> > > > > Remove description under dmas because no user use index to get dma channel.
> > > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > > 'required' list.
> > > >
> > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > >
> > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > index 2456d958adeef..6f551c68d33db 100644
> > > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > @@ -81,15 +81,14 @@ properties:
> > > > >
> > > > >    dmas:
> > > > >      minItems: 1
> > > > > -    items:
> > > > > -      - description: DMA controller phandle and request line for RX
> > > > > -      - description: DMA controller phandle and request line for TX
> > > > > +    maxItems: 2
> > > > >
> > > > >    dma-names:
> > > > > -    minItems: 1
> > > > > -    items:
> > > > > -      - const: rx
> > > > > -      - const: tx
> > > > > +    oneOf:
> > > > > +      - items:
> > > > > +          - const: rx
> > > > > +          - const: tx
> > > > > +      - enum: [ rx, tx ]
> > > >
> > > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > > he was suggesting that in the two items case we'd not care about the
> > > > order. But while I think this is different to that suggestion it's also
> > > > not wrong.
> > >
> > > I log this at cover-letter. b4 can't support write change log at every
> > > patch yet.
> >
> > It never will (probably). That's because it doesn't need to. You can
> > just do it with git. When you edit the commit message, then after the
> > tags, Add '---' and put whatever you want after. That works as long as
> > the commit is applied from a patch as 'git am' will drop it.
>
> The key problem is that I don't want to lost notes when respin patches. It
> is easy to make mistake when I copy old serise change logs.

You don't. It is all saved in the commit. When you rebase for the next
version, you just add to the changes history.

> Previously I use git notes + git-rebase, it work fine. Notes can be kept
> when I do rebase and git commit --amend. But one thing is not good.
> git send-email --to-cmd=./script/get_maintainer.sh *.patch. It can't
> combine all patches's maitainer to a list. It looks like difference patch
> will be difference --to list.
>
> b4 can help some case. But can't keep git-notes information when rebase.
> It should be git bugs or feature missed.

git-notes always seemed clunky and to be extra work to me. Just having
the changelog in the commit msg is a lot easier.

> > > Rob's suggest was not work. dt-binding check complain too long
> > > if there are two dma-names = "rx", "tx".
> >
> > So I'm wrong or you didn't have it correct? No way to tell with your
> > explanation. Let me give you the exact schema:
> >
> > dma-names:
> >   minItems: 1
> >   items:
> >     - enum: [ rx, tx ]
> >     - const: tx
> >
> > This says we can have 1 or 2 entries. The first entry can be either rx
> > or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> > "tx"' is allowed with the above, but we enforce items to be unique
> > elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> > point. I'm going to fix that now.
>
> Conor find out my problem. The below code works. I missed maxItems.
>
>   dma-names:
>      minItems: 1
>      maxItems: 2
>      items

Missing ':'

>        enum: [ rx, tx ]

That is not my suggestion. This would be my 3rd choice after what I
proposed or what you had already. Please plug in exactly what I told
you and report back what doesn't work.

Rob
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Frank Li 1 year, 9 months ago
On Wed, Mar 06, 2024 at 04:58:22PM -0600, Rob Herring wrote:
> On Wed, Mar 6, 2024 at 3:19 PM Frank Li <Frank.li@nxp.com> wrote:
> >
> > On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> > > On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > > > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > > > for dma-names.
> > > > > >
> > > > > > Remove description under dmas because no user use index to get dma channel.
> > > > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > > > 'required' list.
> > > > >
> > > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > >
> > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > ---
> > > > > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > index 2456d958adeef..6f551c68d33db 100644
> > > > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > @@ -81,15 +81,14 @@ properties:
> > > > > >
> > > > > >    dmas:
> > > > > >      minItems: 1
> > > > > > -    items:
> > > > > > -      - description: DMA controller phandle and request line for RX
> > > > > > -      - description: DMA controller phandle and request line for TX
> > > > > > +    maxItems: 2
> > > > > >
> > > > > >    dma-names:
> > > > > > -    minItems: 1
> > > > > > -    items:
> > > > > > -      - const: rx
> > > > > > -      - const: tx
> > > > > > +    oneOf:
> > > > > > +      - items:
> > > > > > +          - const: rx
> > > > > > +          - const: tx
> > > > > > +      - enum: [ rx, tx ]
> > > > >
> > > > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > > > he was suggesting that in the two items case we'd not care about the
> > > > > order. But while I think this is different to that suggestion it's also
> > > > > not wrong.
> > > >
> > > > I log this at cover-letter. b4 can't support write change log at every
> > > > patch yet.
> > >
> > > It never will (probably). That's because it doesn't need to. You can
> > > just do it with git. When you edit the commit message, then after the
> > > tags, Add '---' and put whatever you want after. That works as long as
> > > the commit is applied from a patch as 'git am' will drop it.
> >
> > The key problem is that I don't want to lost notes when respin patches. It
> > is easy to make mistake when I copy old serise change logs.
> 
> You don't. It is all saved in the commit. When you rebase for the next
> version, you just add to the changes history.
> 
> > Previously I use git notes + git-rebase, it work fine. Notes can be kept
> > when I do rebase and git commit --amend. But one thing is not good.
> > git send-email --to-cmd=./script/get_maintainer.sh *.patch. It can't
> > combine all patches's maitainer to a list. It looks like difference patch
> > will be difference --to list.
> >
> > b4 can help some case. But can't keep git-notes information when rebase.
> > It should be git bugs or feature missed.
> 
> git-notes always seemed clunky and to be extra work to me. Just having
> the changelog in the commit msg is a lot easier.
> 
> > > > Rob's suggest was not work. dt-binding check complain too long
> > > > if there are two dma-names = "rx", "tx".
> > >
> > > So I'm wrong or you didn't have it correct? No way to tell with your
> > > explanation. Let me give you the exact schema:
> > >
> > > dma-names:
> > >   minItems: 1
> > >   items:
> > >     - enum: [ rx, tx ]
> > >     - const: tx
> > >
> > > This says we can have 1 or 2 entries. The first entry can be either rx
> > > or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> > > "tx"' is allowed with the above, but we enforce items to be unique
> > > elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> > > point. I'm going to fix that now.
> >
> > Conor find out my problem. The below code works. I missed maxItems.
> >
> >   dma-names:
> >      minItems: 1
> >      maxItems: 2
> >      items
> 
> Missing ':'
> 
> >        enum: [ rx, tx ]
> 
> That is not my suggestion. This would be my 3rd choice after what I
> proposed or what you had already. Please plug in exactly what I told
> you and report back what doesn't work.

It also works. look likes conor's method is a little bit easy to be
understood.

Frank
> 
> Rob
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Conor Dooley 1 year, 9 months ago
On Wed, Mar 06, 2024 at 09:02:38PM -0500, Frank Li wrote:
> On Wed, Mar 06, 2024 at 04:58:22PM -0600, Rob Herring wrote:
> > On Wed, Mar 6, 2024 at 3:19 PM Frank Li <Frank.li@nxp.com> wrote:
> > >
> > > On Wed, Mar 06, 2024 at 02:55:24PM -0600, Rob Herring wrote:
> > > > On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > > > > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > > > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > > > > for dma-names.
> > > > > > >
> > > > > > > Remove description under dmas because no user use index to get dma channel.
> > > > > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > > > > 'required' list.
> > > > > >
> > > > > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > >
> > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > > > > ---
> > > > > > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > > > > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > > > > >
> > > > > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > > index 2456d958adeef..6f551c68d33db 100644
> > > > > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > > > > @@ -81,15 +81,14 @@ properties:
> > > > > > >
> > > > > > >    dmas:
> > > > > > >      minItems: 1
> > > > > > > -    items:
> > > > > > > -      - description: DMA controller phandle and request line for RX
> > > > > > > -      - description: DMA controller phandle and request line for TX
> > > > > > > +    maxItems: 2
> > > > > > >
> > > > > > >    dma-names:
> > > > > > > -    minItems: 1
> > > > > > > -    items:
> > > > > > > -      - const: rx
> > > > > > > -      - const: tx
> > > > > > > +    oneOf:
> > > > > > > +      - items:
> > > > > > > +          - const: rx
> > > > > > > +          - const: tx
> > > > > > > +      - enum: [ rx, tx ]
> > > > > >
> > > > > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > > > > he was suggesting that in the two items case we'd not care about the
> > > > > > order. But while I think this is different to that suggestion it's also
> > > > > > not wrong.

> > > > > Rob's suggest was not work. dt-binding check complain too long
> > > > > if there are two dma-names = "rx", "tx".
> > > >
> > > > So I'm wrong or you didn't have it correct? No way to tell with your
> > > > explanation. Let me give you the exact schema:
> > > >
> > > > dma-names:
> > > >   minItems: 1
> > > >   items:
> > > >     - enum: [ rx, tx ]
> > > >     - const: tx
> > > >
> > > > This says we can have 1 or 2 entries. The first entry can be either rx
> > > > or tx. The 2nd entry must be tx. That's what you want. However, '"tx",
> > > > "tx"' is allowed with the above, but we enforce items to be unique
> > > > elsewhere. Or I thought we did, but we relaxed '.*-names$' at some
> > > > point. I'm going to fix that now.
> > >
> > > Conor find out my problem. The below code works. I missed maxItems.
> > >
> > >   dma-names:
> > >      minItems: 1
> > >      maxItems: 2
> > >      items
> > 
> > Missing ':'
> > 
> > >        enum: [ rx, tx ]
> > 
> > That is not my suggestion. This would be my 3rd choice after what I
> > proposed or what you had already. Please plug in exactly what I told
> > you and report back what doesn't work.
> 
> It also works. look likes conor's method is a little bit easy to be
> understood.

What I suggested is different, it is more permissive than what you have
or what Rob suggested. Your original one allows
"rx", "tx" OR "rx" OR "tx"
Rob's allows the same but with a nicer syntax. What that stm binding I
mentioned allows is
"rx", "tx" OR "tx", "rx" OR "rx" OR "tx"
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Conor Dooley 1 year, 9 months ago
On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > for dma-names.
> > > 
> > > Remove description under dmas because no user use index to get dma channel.
> > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > 'required' list.
> > 
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > index 2456d958adeef..6f551c68d33db 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > @@ -81,15 +81,14 @@ properties:
> > >  
> > >    dmas:
> > >      minItems: 1
> > > -    items:
> > > -      - description: DMA controller phandle and request line for RX
> > > -      - description: DMA controller phandle and request line for TX
> > > +    maxItems: 2
> > >  
> > >    dma-names:
> > > -    minItems: 1
> > > -    items:
> > > -      - const: rx
> > > -      - const: tx
> > > +    oneOf:
> > > +      - items:
> > > +          - const: rx
> > > +          - const: tx
> > > +      - enum: [ rx, tx ]
> > 
> > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > he was suggesting that in the two items case we'd not care about the
> > order. But while I think this is different to that suggestion it's also
> > not wrong.
> 
> I log this at cover-letter. b4 can't support write change log at every
> patch yet. Rob's suggest was not work. dt-binding check complain too long
> if there are two dma-names = "rx", "tx". 

Yeh, that is what happens if you just have
dma-names:
  enum: [ rx, tx ]
since that only allows one entry. Take a look at st,stm32-uart.yaml
instead, for example.
Re: [PATCH v4 3/4] ASoC: dt-bindings: fsl-sai: allow only one dma-names
Posted by Frank Li 1 year, 9 months ago
On Wed, Mar 06, 2024 at 07:45:34PM +0000, Conor Dooley wrote:
> On Wed, Mar 06, 2024 at 02:25:53PM -0500, Frank Li wrote:
> > On Wed, Mar 06, 2024 at 06:45:13PM +0000, Conor Dooley wrote:
> > > On Tue, Mar 05, 2024 at 12:33:04PM -0500, Frank Li wrote:
> > > > Some sai only connect one direction dma (rx/tx) in SOC. For example:
> > > > imx8qxp sai5 only connect tx dma channel. So allow only one "rx" or "tx"
> > > > for dma-names.
> > > > 
> > > > Remove description under dmas because no user use index to get dma channel.
> > > > All user use 'dma-names' to get correct dma channel. dma-names already in
> > > > 'required' list.
> > > 
> > > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > > 
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 13 ++++++-------
> > > >  1 file changed, 6 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > index 2456d958adeef..6f551c68d33db 100644
> > > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > > @@ -81,15 +81,14 @@ properties:
> > > >  
> > > >    dmas:
> > > >      minItems: 1
> > > > -    items:
> > > > -      - description: DMA controller phandle and request line for RX
> > > > -      - description: DMA controller phandle and request line for TX
> > > > +    maxItems: 2
> > > >  
> > > >    dma-names:
> > > > -    minItems: 1
> > > > -    items:
> > > > -      - const: rx
> > > > -      - const: tx
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - const: rx
> > > > +          - const: tx
> > > > +      - enum: [ rx, tx ]
> > > 
> > > I'm not entirely sure if this was Rob's suggestion, I got the impression
> > > he was suggesting that in the two items case we'd not care about the
> > > order. But while I think this is different to that suggestion it's also
> > > not wrong.
> > 
> > I log this at cover-letter. b4 can't support write change log at every
> > patch yet. Rob's suggest was not work. dt-binding check complain too long
> > if there are two dma-names = "rx", "tx". 
> 
> Yeh, that is what happens if you just have
> dma-names:
>   enum: [ rx, tx ]
> since that only allows one entry. Take a look at st,stm32-uart.yaml
> instead, for example.

Yes, it works. Thank you very much. I will update it at next version.

Frank