Add "spacemit,k1-pdma" compatible string to support SpacemiT K1 PDMA
controller. This variant requires:
- clocks: Clock controller for the DMA
- resets: Reset controller for the DMA
Also add explicit #dma-cells property definition with proper constraints:
- 2 cells for marvell,pdma-1.0 and spacemit,k1-pdma
- (request number + unused)
- 1 cell for other variants
- (request number only)
This fixes "make dtbs_check W=3" warnings about unevaluated properties.
Signed-off-by: Guodong Xu <guodong@riscstar.com>
---
v2:
- Used more specific compatible string "spacemit,k1-pdma"
- Enhanced DT bindings with conditional constraints:
- clocks/resets properties only required for SpacemiT K1
- #dma-cells set to 2 for marvell,pdma-1.0 and spacemit,k1-pdma
- #dma-cells set to 1 for other variants, ie.
marvell,adma-1.0 and marvell,pxa910-squ
---
.../devicetree/bindings/dma/marvell,mmp-dma.yaml | 49 ++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml
index d447d5207be0436bc7fb648dffe31f8b780b491d..7b5f7ccfc9dbb69bfef250146cba5434548f3702 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,k1-pdma
reg:
maxItems: 1
@@ -32,6 +33,19 @@ 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':
+ description:
+ DMA specifier, consisting of a phandle to DMA controller plus the
+ following integer cells
+
'#dma-channels':
deprecated: true
@@ -52,12 +66,47 @@ allOf:
contains:
enum:
- marvell,pdma-1.0
+ - spacemit,k1-pdma
then:
properties:
asram: false
else:
required:
- asram
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: spacemit,k1-pdma
+ then:
+ required:
+ - clocks
+ - resets
+ else:
+ properties:
+ clocks: false
+ resets: false
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - marvell,pdma-1.0
+ - spacemit,k1-pdma
+ then:
+ properties:
+ '#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.
+ else:
+ properties:
+ '#dma-cells':
+ const: 1
+ description:
+ The cell contains the DMA request number for the peripheral device.
unevaluatedProperties: false
--
2.43.0On 01/07/2025 07:36, Guodong Xu wrote: > Add "spacemit,k1-pdma" compatible string to support SpacemiT K1 PDMA > controller. This variant requires: Why is this marvell? This should be explained here, it's really unexpected. > - clocks: Clock controller for the DMA > - resets: Reset controller for the DMA > > Also add explicit #dma-cells property definition with proper constraints: > - 2 cells for marvell,pdma-1.0 and spacemit,k1-pdma > - (request number + unused) > - 1 cell for other variants > - (request number only) > > This fixes "make dtbs_check W=3" warnings about unevaluated properties. How can I reproduce these warnings? Looks like this is not relevant here. Each patch is applied one after another and commit msg must be correct at this point. > > Signed-off-by: Guodong Xu <guodong@riscstar.com> > --- > v2: > - Used more specific compatible string "spacemit,k1-pdma" > - Enhanced DT bindings with conditional constraints: > - clocks/resets properties only required for SpacemiT K1 > - #dma-cells set to 2 for marvell,pdma-1.0 and spacemit,k1-pdma > - #dma-cells set to 1 for other variants, ie. > marvell,adma-1.0 and marvell,pxa910-squ > --- > .../devicetree/bindings/dma/marvell,mmp-dma.yaml | 49 ++++++++++++++++++++++ > 1 file changed, 49 insertions(+) > > diff --git a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml > index d447d5207be0436bc7fb648dffe31f8b780b491d..7b5f7ccfc9dbb69bfef250146cba5434548f3702 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,k1-pdma > > reg: > maxItems: 1 > @@ -32,6 +33,19 @@ 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': > + description: > + DMA specifier, consisting of a phandle to DMA controller plus the > + following integer cells Why do you need it here? Isn't this coming from dma common schemas? > + > '#dma-channels': > deprecated: true > > @@ -52,12 +66,47 @@ allOf: > contains: > enum: > - marvell,pdma-1.0 > + - spacemit,k1-pdma > then: > properties: > asram: false > else: > required: > - asram > + - if: > + properties: > + compatible: > + contains: > + const: spacemit,k1-pdma > + then: > + required: > + - clocks > + - resets > + else: > + properties: > + clocks: false > + resets: false > + - if: > + properties: > + compatible: > + contains: > + enum: > + - marvell,pdma-1.0 > + - spacemit,k1-pdma > + then: > + properties: > + '#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. > + else: > + properties: > + '#dma-cells': > + const: 1 > + description: > + The cell contains the DMA request number for the peripheral device. It's getting complicated. I suggest to make your own schema. Then you would also switch to preferred 'sram' property instead of that legacy 'asram'. Really, ancient schemas should not be grown for new, completely different platforms. Best regards, Krzysztof
Hi, Krzysztof On Tue, Jul 1, 2025 at 3:35 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 01/07/2025 07:36, Guodong Xu wrote: > > Add "spacemit,k1-pdma" compatible string to support SpacemiT K1 PDMA > > controller. This variant requires: > > Why is this marvell? This should be explained here, it's really unexpected. > SpacemiT K1 SoC uses the same DMA controller as Marvell MMP. They share most of the registers (and address offsets) and only enhanced in addressing space capability (from 32bit to 64bit). Also, spacemit,k1-pdma and marvell,pdma-1.0 use the same driver (mmp_pdma.c), that's the reason why I chose keeping them in the same binding file. > > - clocks: Clock controller for the DMA > > - resets: Reset controller for the DMA > > > > Also add explicit #dma-cells property definition with proper constraints: > > - 2 cells for marvell,pdma-1.0 and spacemit,k1-pdma > > - (request number + unused) > > - 1 cell for other variants > > - (request number only) > > > > This fixes "make dtbs_check W=3" warnings about unevaluated properties. > > How can I reproduce these warnings? Looks like this is not relevant > here. Each patch is applied one after another and commit msg must be > correct at this point. > You're absolutely right about the commit message needing to be accurate at this point. The dtbs_check warnings only occur when compiling board DTS files that contain nodes with marvell,pdma-1.0 or spacemit,k1-pdma compatible strings - specifically when PATCH 7 of this series. I'll revise the commit message to better reflect that this patch enhances things, rather than claiming it "fixes" warnings at this stage. > > > > > Signed-off-by: Guodong Xu <guodong@riscstar.com> > > --- > > v2: > > - Used more specific compatible string "spacemit,k1-pdma" > > - Enhanced DT bindings with conditional constraints: > > - clocks/resets properties only required for SpacemiT K1 > > - #dma-cells set to 2 for marvell,pdma-1.0 and spacemit,k1-pdma > > - #dma-cells set to 1 for other variants, ie. > > marvell,adma-1.0 and marvell,pxa910-squ > > --- > > .../devicetree/bindings/dma/marvell,mmp-dma.yaml | 49 ++++++++++++++++++++++ > > 1 file changed, 49 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml b/Documentation/devicetree/bindings/dma/marvell,mmp-dma.yaml > > index d447d5207be0436bc7fb648dffe31f8b780b491d..7b5f7ccfc9dbb69bfef250146cba5434548f3702 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,k1-pdma > > > > reg: > > maxItems: 1 > > @@ -32,6 +33,19 @@ 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': > > + description: > > + DMA specifier, consisting of a phandle to DMA controller plus the > > + following integer cells > > Why do you need it here? Isn't this coming from dma common schemas? Thanks for pointing out this. It should be removed. I will fix it. > > + > > '#dma-channels': > > deprecated: true > > > > @@ -52,12 +66,47 @@ allOf: > > contains: > > enum: > > - marvell,pdma-1.0 > > + - spacemit,k1-pdma > > then: > > properties: > > asram: false > > else: > > required: > > - asram > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: spacemit,k1-pdma > > + then: > > + required: > > + - clocks > > + - resets > > + else: > > + properties: > > + clocks: false > > + resets: false > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - marvell,pdma-1.0 > > + - spacemit,k1-pdma > > + then: > > + properties: > > + '#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. > > + else: > > + properties: > > + '#dma-cells': > > + const: 1 > > + description: > > + The cell contains the DMA request number for the peripheral device. > > > It's getting complicated. I suggest to make your own schema. Then you > would also switch to preferred 'sram' property instead of that legacy > 'asram'. > > Really, ancient schemas should not be grown for new, completely The reason that they share the same device driver may not be strong enough compared to what you said here. > different platforms. > Complexity wise, I agree with you. I should admit that I haven't dug into the history enough. Anyway, unless other people have strong opinion, I will create a new schema namely spacemit,k1-pdma.yaml Thanks Krzysztof. BR, Guodong > > Best regards, > Krzysztof
On 01/07/2025 11:52, Guodong Xu wrote: > Hi, Krzysztof > > On Tue, Jul 1, 2025 at 3:35 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >> On 01/07/2025 07:36, Guodong Xu wrote: >>> Add "spacemit,k1-pdma" compatible string to support SpacemiT K1 PDMA >>> controller. This variant requires: >> >> Why is this marvell? This should be explained here, it's really unexpected. >> > > SpacemiT K1 SoC uses the same DMA controller as Marvell MMP. They share most > of the registers (and address offsets) and only enhanced in addressing space > capability (from 32bit to 64bit). I hope you got the last comment... > > Also, spacemit,k1-pdma and marvell,pdma-1.0 use the same driver (mmp_pdma.c), > that's the reason why I chose keeping them in the same binding file. That's moderate reason. I explained here further - don't grow old bindings with completely new devices, because you keep growing old, poorer patterns. You have a new device, you can make it right. ... >>> + properties: >>> + compatible: >>> + contains: >>> + const: spacemit,k1-pdma >>> + then: >>> + required: >>> + - clocks >>> + - resets >>> + else: >>> + properties: >>> + clocks: false >>> + resets: false >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - marvell,pdma-1.0 >>> + - spacemit,k1-pdma >>> + then: >>> + properties: >>> + '#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. >>> + else: >>> + properties: >>> + '#dma-cells': >>> + const: 1 >>> + description: >>> + The cell contains the DMA request number for the peripheral device. >> >> >> It's getting complicated. I suggest to make your own schema. Then you >> would also switch to preferred 'sram' property instead of that legacy >> 'asram'. >> >> Really, ancient schemas should not be grown for new, completely > > The reason that they share the same device driver may not be strong enough > compared to what you said here. If DMA maintainer(s) reject complexity in driver, then it is fine. But till that happens, you should rather come with a clean new binding and don't grow legacy. Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.