diff mbox series

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

Message ID 20240308-asrc_8qxp-v6-3-e08f6d030e09@nxp.com (mailing list archive)
State Superseded
Headers show
Series arm64: dts: imx8qxp add asrc and sai | expand

Commit Message

Frank Li March 8, 2024, 3:30 p.m. UTC 
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 | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Conor Dooley March 8, 2024, 4:58 p.m. UTC | #1 
On Fri, Mar 08, 2024 at 10:30:52AM -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>

Please drop my ack from this, this isn't the patch I acked originally
and we were having a conversation as recently as yesterday on v4 about
this patch because Rob didn't like this approach. His suggestion is
better than the one I gave on v4 that you have used here.

Please give it a few days between resubmissions, and do not send new
versions of a series every day. It is very confusing to have different
conversions on three versions of this patch in my inbox all at the same
time.

Thanks,
Conor.

> ---
>  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> index 2456d958adeef..93e7737a49a7b 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> @@ -81,15 +81,13 @@ 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
> +    maxItems: 2
>      items:
> -      - const: rx
> -      - const: tx
> +      enum: [ rx, tx ]
>  
>    interrupts:
>      items:
> 
> -- 
> 2.34.1
>
Frank Li March 8, 2024, 7:06 p.m. UTC | #2 
On Fri, Mar 08, 2024 at 04:58:16PM +0000, Conor Dooley wrote:
> On Fri, Mar 08, 2024 at 10:30:52AM -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>
> 
> Please drop my ack from this, this isn't the patch I acked originally
> and we were having a conversation as recently as yesterday on v4 about
> this patch because Rob didn't like this approach. His suggestion is
> better than the one I gave on v4 that you have used here.

Why do you think Rob don't like this approach? He just said this is 3rd
method. And it is simple enough and match all restriction.

Frank Li

> 
> Please give it a few days between resubmissions, and do not send new
> versions of a series every day. It is very confusing to have different
> conversions on three versions of this patch in my inbox all at the same
> time.

Okay, I think it is just simple fix and almost everything already close to
settle down.

Frank

> 
> Thanks,
> Conor.
> 
> > ---
> >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > index 2456d958adeef..93e7737a49a7b 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > @@ -81,15 +81,13 @@ 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
> > +    maxItems: 2
> >      items:
> > -      - const: rx
> > -      - const: tx
> > +      enum: [ rx, tx ]
> >  
> >    interrupts:
> >      items:
> > 
> > -- 
> > 2.34.1
> >
Frank Li March 8, 2024, 8:21 p.m. UTC | #3 
On Fri, Mar 08, 2024 at 02:06:08PM -0500, Frank Li wrote:
> On Fri, Mar 08, 2024 at 04:58:16PM +0000, Conor Dooley wrote:
> > On Fri, Mar 08, 2024 at 10:30:52AM -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>
> > 
> > Please drop my ack from this, this isn't the patch I acked originally
> > and we were having a conversation as recently as yesterday on v4 about
> > this patch because Rob didn't like this approach. His suggestion is
> > better than the one I gave on v4 that you have used here.

I paste your comments here

"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"
"

Actually:

"rx", "tx" OR "tx", "rx" OR "rx" OR "tx" is exactly what we want.

"rx", "tx" OR "rx" OR "tx" is only feasible, but not perfect. Why need
limited "rx" and "tx" order? It just bring us some noise and no actual
value to do that.

Frank


> 
> Why do you think Rob don't like this approach? He just said this is 3rd
> method. And it is simple enough and match all restriction.
> 
> Frank Li
> 
> > 
> > Please give it a few days between resubmissions, and do not send new
> > versions of a series every day. It is very confusing to have different
> > conversions on three versions of this patch in my inbox all at the same
> > time.
> 
> Okay, I think it is just simple fix and almost everything already close to
> settle down.
> 
> Frank
> 
> > 
> > Thanks,
> > Conor.
> > 
> > > ---
> > >  Documentation/devicetree/bindings/sound/fsl,sai.yaml | 8 +++-----
> > >  1 file changed, 3 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > index 2456d958adeef..93e7737a49a7b 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
> > > @@ -81,15 +81,13 @@ 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
> > > +    maxItems: 2
> > >      items:
> > > -      - const: rx
> > > -      - const: tx
> > > +      enum: [ rx, tx ]
> > >  
> > >    interrupts:
> > >      items:
> > > 
> > > -- 
> > > 2.34.1
> > > 
> 
>
Rob Herring (Arm) March 8, 2024, 9:13 p.m. UTC | #4 
On Fri, Mar 08, 2024 at 03:21:14PM -0500, Frank Li wrote:
> On Fri, Mar 08, 2024 at 02:06:08PM -0500, Frank Li wrote:
> > On Fri, Mar 08, 2024 at 04:58:16PM +0000, Conor Dooley wrote:
> > > On Fri, Mar 08, 2024 at 10:30:52AM -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>
> > > 
> > > Please drop my ack from this, this isn't the patch I acked originally
> > > and we were having a conversation as recently as yesterday on v4 about
> > > this patch because Rob didn't like this approach. His suggestion is
> > > better than the one I gave on v4 that you have used here.
> 
> I paste your comments here
> 
> "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"
> "
> 
> Actually:
> 
> "rx", "tx" OR "tx", "rx" OR "rx" OR "tx" is exactly what we want.

No, it is not.

> "rx", "tx" OR "rx" OR "tx" is only feasible, but not perfect. Why need
> limited "rx" and "tx" order? 

First, that's exactly what the binding already had. Why loosen it? 
Second, defined order is just the DT way. There is less reason to 
support both ways. It is simpler for a client to read properties if it 
knows the position of entries.


> It just bring us some noise and no actual
> value to do that.
> 
> Frank
> 
> 
> > 
> > Why do you think Rob don't like this approach? He just said this is 3rd
> > method. And it is simple enough and match all restriction.

I don't like the approach. Clear enough?

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/sound/fsl,sai.yaml b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
index 2456d958adeef..93e7737a49a7b 100644
--- a/Documentation/devicetree/bindings/sound/fsl,sai.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,sai.yaml
@@ -81,15 +81,13 @@  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
+    maxItems: 2
     items:
-      - const: rx
-      - const: tx
+      enum: [ rx, tx ]
 
   interrupts:
     items: