Re: [PATCH 1/8] dt-bindings: dma: marvell,mmp-dma: Add SpacemiT PDMA compatibility

Guodong Xu
2025-06-12T01:44:39Z

On Thu, Jun 12, 2025 at 12:27 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Wed, Jun 11, 2025 at 08:57:16PM +0800, Guodong Xu wrote:
> > Add "spacemit,pdma-1.0" compatible string to support SpacemiT PDMA
> > controller in the Marvell MMP DMA device tree bindings. This enables:
> >
> > - Support for SpacemiT PDMA controller configuration
> > - New optional properties for platform-specific integration:
> > * clocks: Clock controller for the DMA
> > * resets: Reset controller for the DMA
> >
> > Also, add explicit #dma-cells property definition to avoid
> > "make dtbs_check W=3" warnings about unevaluated properties.
> >
> > The #dma-cells property is defined as 2 cells to maintain compatibility
> > with existing ARM device trees. The first cell specifies the DMA request
> > line number, while the second cell is currently unused by the driver but
> > required for backward compatibility with PXA device tree files.
> >
> > Signed-off-by: Guodong Xu <guodong@riscstar.com>
> > ---
> > .../bindings/dma/marvell,mmp-dma.yaml | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> > index d447d5207be0..e117a81414bd 100644
> > --- a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> > +++ b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
> > @@ -18,6 +18,7 @@ properties:
> > - marvell,pdma-1.0
> > - marvell,adma-1.0
> > - marvell,pxa910-squ
> > + - spacemit,pdma-1.0
>
> You need a soc-specific compatible here.
>
> >
> > reg:
> > maxItems: 1
> > @@ -32,6 +33,21 @@ properties:
> > A phandle to the SRAM pool
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > + clocks:
> > + description: Clock for the controller
> > + maxItems: 1
> > +
> > + resets:
> > + description: Reset controller for the DMA controller
> > + maxItems: 1
> > +
> > + '#dma-cells':
> > + const: 2
> > + description:
> > + The first cell contains the DMA request number for the peripheral
> > + device. The second cell is currently unused but must be present for
> > + backward compatibility.
>
> These properties are only valid for your new device, right?
> If so, please restrict them to only the spacemit platform.
>

For clocks and resets, yes that's correct, only for spacemit k1. I'll add
conditional constraints to restrict them to the spacemit platform.

Regarding #dma-cells, it's used by other platforms too. You are right that
it requires clarification. I should have been more careful in my initial
analysis.

After reviewing the in-tree device trees and driver code, here's what I found:

For PDMA controllers:
Both "marvell,pdma-1.0" and the spacemit pdma use #dma-cells = <2>;
Handled by drivers/dma/mmp_pdma.c with _xlate() callback

For ADMA/audio controllers:
"marvell,adma-1.0" uses #dma-cells = <1>

"marvell,pxa910-squ" is not found in any in-tree device trees, but based
on the original TXT binding [1], both adma and pxa910-squ are "Marvell
Two Channel DMA Controller used specifically for audio", and the shared
driver code (drivers/dma/mmp_tdma.c) also shows it should use
#dma-cells = <1>.

So, I'll add conditional constraints in the allOf section.
For "marvell,pdma-1.0" and spacemit pdma, #dma-cells must be 2;
For "marvell,adma-1.0" and "marvell,pxa910-squ", #dma-cells must be 1.
Also, as said, I will restrict clocks and resets to spacemit only.

Thank you again for catching this. Let me know what you think.

Link: https://lore.kernel.org/all/20240131-pxa-dma-yaml-v2-2-9611d0af0edc@skole.hr/
[1]

-Guodong

> > +
> > '#dma-channels':
> > deprecated: true
> >
> > @@ -52,6 +68,7 @@ allOf:
> > contains:
> > enum:
> > - marvell,pdma-1.0
> > + - spacemit,pdma-1.0
> > then:
> > properties:
> > asram: false
> > --
> > 2.43.0
> >