Message-ID: <20260128-debatable-scribe-4e55c208b31a@spud>
Date: Wed, 28 Jan 2026 20:09:25 +0000
From: Conor Dooley <conor@...nel.org>
To: Cosmin-Gabriel Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
Cc: Fabrizio Castro <fabrizio.castro.jz@...esas.com>,
	Mark Brown <broonie@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	"magnus.damm" <magnus.damm@...il.com>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] dt-bindings: spi: renesas,rzv2h-rspi: allow
 multiple DMAs

On Wed, Jan 28, 2026 at 06:51:48PM +0000, Cosmin-Gabriel Tanislav wrote:
> Hi Conor, thank you for your response.
> 
> > From: Conor Dooley <conor@...nel.org>
> > Sent: Wednesday, January 28, 2026 8:09 PM
> > 
> > On Tue, Jan 27, 2026 at 10:17:04PM +0200, Cosmin Tanislav wrote:
> > > The Renesas RZ/T2H and RZ/N2H SoCs have multiple DMA controllers that
> > > can be used with the RSPI peripheral. The current bindings only allow a
> > > single pair of RX and TX DMAs.
> > >
> > > Allow multiple DMAs by only restricting the possible names of the DMA
> > > channels.
> > >
> > 
> > > All '.*-names$' properties must conform to the string-array.yaml
> > > meta-schema, which requires both minItems and maxItems properties to be
> > > present before the items can be a schema. Otherwise, the items need to
> > > be an array.
> > 
> > Why is this in the commit message?
> > 
> 
> To provide a context for the maxItems that are needed below, even if
> there's not really a maximum. Which is why having a maxItems does not
> really make sense but it is expected by the meta-schema so we can
> constrain the names of the DMA channels.
> 
> dtschema/meta-schemas/string-array.yaml:
> 
> if:
>   not:
>     required:
>       - minItems
>       - maxItems
> then:
>   properties:
>     items:
>       type: array

Right. You can probably remove all that since I'm asking you to add
actual constraints to the property.

> > > Declare a generous maxItems of 32, which should be enough for 16 DMA
> > > controllers, so that we don't have to update this value ever again, even
> > > if currently the maximum number of DMA controllers on a Renesas SoC is
> > > 5.
> > 
> > Huh, No. The binding should constrain this to fit what the actual
> > devices do.
> > 
> 
> Should the binding for SPI be updated if a device ever comes up with
> 6 DMA controllers? It seems a bit unrelated to me. In this case, should
> we constrain the number of dmas and dma-names per SoC? Some may have 2
> DMA controllers, while others may have 5. Please let me know your
> thoughts, taking into account that I only added maxItems to satisfy the
> meta-schema.

Yes, I think you should constrain it to the correct number of providers
for each device.
Whether that's done or not, there's not all that much reason to set it
above whatever the current maximum is, since the binding will have to be
updated to add the compatible for whatever device exceeds the current max
and the limit can be increased then.

> > > Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@...esas.com>
> > > ---
> > >
> > > V2:
> > >  * new patch
> > >
> > >  .../devicetree/bindings/spi/renesas,rzv2h-rspi.yaml    | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/renesas,rzv2h-rspi.yaml
> > b/Documentation/devicetree/bindings/spi/renesas,rzv2h-rspi.yaml
> > > index a588b112e11e..383e97f0dabd 100644
> > > --- a/Documentation/devicetree/bindings/spi/renesas,rzv2h-rspi.yaml
> > > +++ b/Documentation/devicetree/bindings/spi/renesas,rzv2h-rspi.yaml
> > > @@ -57,13 +57,15 @@ properties:
> > >        - const: presetn
> > >        - const: tresetn
> > >
> > > -  dmas:
> > > -    maxItems: 2
> > > +  dmas: true
> > 
> > This should have the same constraints as dma-names. You've now allowed
> > this to have 1 and 33 dmas, because there's no requirement to have
> > dma-names when you have dmas.
> > 
> 
> I agree, I will fix it for V2 once you decide how to proceed with the
> other comments.
> 
> > >
> > >    dma-names:
> > > +    minItems: 2
> > > +    maxItems: 32
> > >      items:
> > > -      - const: rx
> > > -      - const: tx
> > > +      enum:
> > > +        - rx
> > > +        - tx
> > 
> > You've changed this to allow 32 dma-names, but they all need to be
> > called either "rx" or "tx", how is a driver meant to use dma-names to
> > get the second pair of dma channels? Shouldn't anything in excess of the
> > first two start getting numbers appended so that a driver can actually
> > request them?
> > 
> 
> The DMA core handles multiple DMA channels with the same name by checking
> their availability consecutively until finding an available one.

TIL

> I agree that this is not pretty but this pattern is already used in the
> bindings / device tree for many Renesas IPs.
> 
> There's even an exception inside dt-schema specifically for this.

Hmm, I see. Can you please put this into the commit message cos otherwise
this looks really strange!

> 
> dtschema/schemas/dma/dma.yaml:
>   dma-names:
>     anyOf:
>       - uniqueItems: true
>       - items:
>           # Hack around Renesas bindings which repeat entries to support
>           # multiple possible DMA providers
>           enum: [rx, tx]
> 
> > pw-bot: changes-requested
> > 
> > Conor.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists