Re: [PATCH V3 4/6] dt-bindings: dma: xlnx,zynqmp-dma: Add Versal Net compatible support
"Golla, Nagendra" <[email protected]>
| Newsgroups | org.kernel.vger.linux-pm,org.infradead.lists.linux-arm-kernel,org.kernel.vger.dmaengine,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Krzysztof On 8/12/2026 4:13 PM, Krzysztof Kozlowski wrote: > On Mon, Aug 10, 2026 at 03:34:50PM +0530, Golla Nagendra wrote: >> From: Jay Buddhabhatti <[email protected]> >> >> Introduce a new compatible string amd,versal-net-dma-1.0 for >> Versal Net ZDMA.The new compatible requires xlnx,zynqmp-dma-1.0 as a >> fallback. >> >> Add a reset property to describe the per-channel reset line exposed >> on Versal Gen2 and Versal Net. Mark the resets property as required >> for both Versal Gen2 and Versal Net, and not applicable for other >> platforms. > > I cannot find previous discussion in my inbox and cover letter has no > links to previous versions (when finally will you start using b4?), so > you might get the same review over and over. Thanks for the review Please fin the previous version lore links below https://lore.kernel.org/all/[email protected]/ https://lore.kernel.org/all/[email protected]/ > >> >> Note that making resets required for Versal Gen2 results in an ABI >> break but this is unavoidable. When DMA is used by two software components >> back to back (e.g., firmware before Linux boot), DMA functionality fails >> due to configuration differences or the DMA being left in a bad state by >> the previous user. The channel can only be cleanly re-configured after >> a DMA channel reset,making it essential. > > So this was never working? What exactly is unavoidable? It does work in many scenarios when there was no other FW using the DMA prior to Linux boot. But there's no guarantee that the channel is left in a clean state and we have observed error scenarios where a reset is necessary before Linux can reconfigure the IP. We added the reset to fix this and hence making it a required property is unavoidable. We are only doing this for Versal Gen2 and Versal Net where this provision is available. We are unable to enforce this for older devices because there is no per-channel reset there. Thanks, Nagendra > >> >> Signed-off-by: Jay Buddhabhatti <[email protected]> >> Co-developed-by: Golla Nagendra <[email protected]> >> Signed-off-by: Golla Nagendra <[email protected]> >> --- >> Changes in V3: >> - Use allOf:if:then: to restrict the resets property per-compatible, >> making it required for amd,versal2-dma-1.0 and amd,versal-net-dma-1.0 >> and explicitly setting resets: false for all other platforms >> - Add amd,versal-net-dma-1.0 compatible string with xlnx,zynqmp-dma-1.0 >> as fallback >> - Switch compatible from enum to oneOf to accommodate the new multi-item >> entry for Versal Net >> - Add ABI break justification >> --- >> .../dma/xilinx/xlnx,zynqmp-dma-1.0.yaml | 25 ++++++++++++++++--- >> 1 file changed, 22 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml >> index 2da86037ad79..87c65aef5c7d 100644 >> --- a/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml >> +++ b/Documentation/devicetree/bindings/dma/xilinx/xlnx,zynqmp-dma-1.0.yaml >> @@ -18,15 +18,31 @@ maintainers: >> >> allOf: >> - $ref: ../dma-controller.yaml# >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - amd,versal2-dma-1.0 >> + - amd,versal-net-dma-1.0 > >> + then: >> + required: >> + - resets >> + else: >> + properties: >> + resets: false > > Entire allOf goes after "required:" block, by convention, see > example-schema. > > Best regards, > Krzysztof >