Re: [PATCH v2 4/5] dt-bindings: dma: fsl-edma: add fsl,imx8ulp-edma compatible string

From: Rob Herring
Date: Wed Mar 06 2024 - 15:40:49 EST


On Mon, Mar 4, 2024 at 5:31 PM Frank Li <Frank.li@xxxxxxx> wrote:
>
> On Mon, Mar 04, 2024 at 10:44:23AM -0600, Rob Herring wrote:
> > On Thu, Feb 29, 2024 at 03:58:10PM -0500, Frank Li wrote:
> > > From: Joy Zou <joy.zou@xxxxxxx>
> > >
> > > Introduce the compatible string 'fsl,imx8ulp-edma' to enable support for
> > > the i.MX8ULP's eDMA, alongside adjusting the clock numbering. The i.MX8ULP
> > > eDMA architecture features one clock for each DMA channel and an additional
> > > clock for the core controller. Given a maximum of 32 DMA channels, the
> > > maximum clock number consequently increases to 33.
> > >
> > > Signed-off-by: Joy Zou <joy.zou@xxxxxxx>
> > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx>
> > > ---
> > > .../devicetree/bindings/dma/fsl,edma.yaml | 26 ++++++++++++++++++++--
> > > 1 file changed, 24 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/dma/fsl,edma.yaml b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> > > index aa51d278cb67b..55cce79c759f8 100644
> > > --- a/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> > > +++ b/Documentation/devicetree/bindings/dma/fsl,edma.yaml
> > > @@ -23,6 +23,7 @@ properties:
> > > - fsl,imx7ulp-edma
> > > - fsl,imx8qm-adma
> > > - fsl,imx8qm-edma
> > > + - fsl,imx8ulp-edma
> > > - fsl,imx93-edma3
> > > - fsl,imx93-edma4
> > > - fsl,imx95-edma5
> > > @@ -53,11 +54,11 @@ properties:
> > >
> > > clocks:
> > > minItems: 1
> > > - maxItems: 2
> > > + maxItems: 33
> > >
> > > clock-names:
> > > minItems: 1
> > > - maxItems: 2
> > > + maxItems: 33
> > >
> > > big-endian:
> > > description: |
> > > @@ -108,6 +109,7 @@ allOf:
> > > properties:
> > > clocks:
> > > minItems: 2
> > > + maxItems: 2
> > > clock-names:
> > > items:
> > > - const: dmamux0
> > > @@ -136,6 +138,7 @@ allOf:
> > > properties:
> > > clock:
> > > minItems: 2
> > > + maxItems: 2
> > > clock-names:
> > > items:
> > > - const: dma
> > > @@ -151,6 +154,25 @@ allOf:
> > > dma-channels:
> > > const: 32
> > >
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: fsl,imx8ulp-edma
> > > + then:
> > > + properties:
> > > + clock:
> >
> > clocks
> >
> > > + maxItems: 33
> >
> > That is already the max. I think you want 'minItems: 33' here.
> >
> > > + clock-names:
> > > + items:
> > > + - const: dma
> > > + - pattern: "^CH[0-31]-clk$"
> >
> > '-clk' is redundant. [0-31] is not how you do a range of numbers with
> > regex.
> >
> > This doesn't cover clocks 3-33. Not a great way to express in
> > json-schema, but this should do it:
> >
> > allOf:
> > - items:
> > - const: dma
> > - items:
> > oneOf:
> > - const: dma
> > - pattern: "^ch([0-9]|[1-2][0-9]|[3[01])$"
>
> I understand pattern is wrong. But I don't understand why need 'allOf'.

The first 'items' says the 1st entry must be 'dma'. (It might need a
'maxItems: 33' too now that I look at it.) The 2nd 'items' says all
entries must be either 'dma' or the CHn pattern.

> 8ulp need clock 'dma" and "ch*". I think
>
> items:
> - const: dma
> - pattern: "^CH[0-31]-clk$"
>
> should be enough.

If it was, then I would not have said anything. If you don't believe
me see if this passes validation:

clock-names = "dma", "CH0", "foobar";

> If you means put on top allOf, other platform use clock name such as
> 'dmamux0'.

What? It's under an if/then schema.

Rob