Re: [PATCH 5/9] dt-bindings: dma: Convert fsl,elo*-dma bindings to YAML

J. Neuschäfer
2025-02-04T21:47:47Z

On Wed, Jan 29, 2025 at 05:52:31PM -0500, Frank Li wrote:
> On Sun, Jan 26, 2025 at 07:59:00PM +0100, J. Neuschäfer wrote:
> > The devicetree bindings for Freescale DMA engines have so far existed as
> > a text file. This patch converts them to YAML, and specifies all the
> > compatible strings currently in use in arch/powerpc/boot/dts.
> >
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
> > .../devicetree/bindings/dma/fsl,elo-dma.yaml | 129 +++++++++++++
> > .../devicetree/bindings/dma/fsl,elo3-dma.yaml | 105 +++++++++++
> > .../devicetree/bindings/dma/fsl,eloplus-dma.yaml | 120 ++++++++++++
> > .../devicetree/bindings/powerpc/fsl/dma.txt | 204 ---------------------
> > 4 files changed, 354 insertions(+), 204 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml b/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..d1f4978a672c1217c322c27f243470b2de8c99d4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/dma/fsl,elo-dma.yaml
> > @@ -0,0 +1,129 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/dma/fsl,elo-dma.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Freescale Elo DMA Controller
> > +
> > +maintainers:
> > + - J. Neuschäfer <j.ne@posteo.net>
> > +
> > +description: |
>
> needn't | here

Will remove.

>
> > + This is a little-endian 4-channel DMA controller, used in Freescale mpc83xx
> > + series chips such as mpc8315, mpc8349, mpc8379 etc.
> > +
> > + Note on DMA channel compatible properties: The compatible property must say
> > + "fsl,elo-dma-channel" or "fsl,eloplus-dma-channel" to be used by the Elo DMA
>
> There are not 'fsl,eloplus-dma-channel' under "^dma-channel@.*$". I suggest
> remove this because 'compatible': items already show such information.

Good point, I'll trim this text down.

> > + driver (fsldma). Any DMA channel used by fsldma cannot be used by another
> > + DMA driver, such as the SSI sound drivers for the MPC8610. Therefore, any
> > + DMA channel that should be used for another driver should not use
> > + "fsl,elo-dma-channel" or "fsl,eloplus-dma-channel". For the SSI drivers, for
> > + example, the compatible property should be "fsl,ssi-dma-channel". See
> > + ssi.txt for more information.

I noticed fsl,ssi.txt has been converted to YAML since this text was
originally written, so I'll make reference to that.

[...]
> > +examples:
> > + - |
> > + dma@82a8 {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + compatible = "fsl,mpc8349-dma", "fsl,elo-dma";
> > + reg = <0x82a8 4>;
>
> compatible and reg should be first two property.

Will fix.

>
> > + ranges = <0 0x8100 0x1a4>;
> > + interrupt-parent = <&ipic>;
> > + interrupts = <71 8>;
> > + cell-index = <0>;
>
> need space line here.

Will fix.

> > + dma-channel@0 {
> > + compatible = "fsl,mpc8349-dma-channel", "fsl,elo-dma-channel";
> > + cell-index = <0>;
> > + reg = <0 0x80>;
> > + interrupt-parent = <&ipic>;
> > + interrupts = <71 8>;
> > + };
>
> need space line here. check other's example dts

Will fix in all files.

[...]
> > +patternProperties:
> > + "^dma-channel@.*$":
> > + type: object
> > +
> > + properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - fsl,mpc8540-dma-channel
> > + - fsl,mpc8541-dma-channel
> > + - fsl,mpc8548-dma-channel
> > + - fsl,mpc8555-dma-channel
> > + - fsl,mpc8560-dma-channel
> > + - fsl,mpc8572-dma-channel
> > + - const: fsl,eloplus-dma-channel
>
> I think you can merge this fsl,mpc83xx-dma yaml file
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: fsl,elo-dma
> + then:
> + patternProperties:
> + "^dma-channel@.*$":
> + properties:
> + compatible:
> + items:
> + - enum:
> ....
> + else
> + patternProperties:
> + "^dma-channel@.*$":
> + properties:
> + compatible:
> + items:
> + - enum:
> ....
> + - const: fsl,eloplus-dma-channel

I suppose that works, but I'm not entirely convinced it would help with
readability, compared to leaving the three variants separate.

Thank you for your review!

Best regards,
J. Neuschäfer