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...@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...@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

Reply via email to