[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Ze5ustkOPNYpXubj@lizhi-Precision-Tower-5810>
Date: Sun, 10 Mar 2024 22:38:42 -0400
From: Frank Li <Frank.li@....com>
To: Rob Herring <robh@...nel.org>
Cc: Vinod Koul <vkoul@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Peng Fan <peng.fan@....com>,
imx@...ts.linux.dev, dmaengine@...r.kernel.org,
linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
Joy Zou <joy.zou@....com>
Subject: Re: [PATCH v2 4/5] dt-bindings: dma: fsl-edma: add fsl,imx8ulp-edma
compatible string
On Wed, Mar 06, 2024 at 02:40:23PM -0600, Rob Herring wrote:
> On Mon, Mar 4, 2024 at 5:31 PM Frank Li <Frank.li@....com> 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@....com>
> > > >
> > > > 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@....com>
> > > > Signed-off-by: Frank Li <Frank.Li@....com>
> > > > ---
> > > > .../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.
After dig into dt_scheme and json scheme, I start understand what your
means.
"clock-names": {
"minItems": 33,
"allOf": [
{
"items": [
{
"const": "dma"
}
],
"maxItems": 33,
^^^^^^^^
Here need a maxItem 33 and make sure first item is "dma" and total
array is 33.
"type": "array",
"minItems": 1
},
{
"items": {
"oneOf": [
{
"const": "dma"
},
{
"pattern": "^ch(0[0-9]|[1-2][0-9]|3[01])$"
}
]
},
"type": "array"
}
]
}
The yaml source is
allOf:
- items:
- const: dma
maxItems: 33
- items:
oneOf:
- const: dma
- pattern: "^ch(0[0-9]|[1-2][0-9]|3[01])$"
But unfortunately,
dtschema/meta-schemas/items.yaml
type: object
properties:
items:
type: array
additionalItems: false
required:
- items
- maxItems
then:
description: '"maxItems" is not needed with an "items" list'
not:
required:
- maxItem
dt_binding check will complain
'"maxItems" is not needed with an "items" list'
I am not sure how to go futher. Maybe below 'stupid' method is less impact.
items
- const: dma
- const: ch00
...
- const: ch31
Frank
>
> > 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
Powered by blists - more mailing lists