Re: [PATCH V2 6/8] dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing hardware context restore on system resume leads to DMA data corruption.
- [High] DMA descriptor structures lack explicit little-endian types, breaking on big-endian kernels.
--

--- Patch [6]: [PATCH V2 6/8] dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent ---
commit 85ed10a27c3d9299f4dbcb91c70a55052ad9638d
Author: Golla Nagendra <[email protected]>

dmaengine: zynqmp_dma: Use of_dma_is_coherent for dma-coherent

Use of_dma_is_coherent() instead of parsing the common dma-coherent
device-tree property directly.

> diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> index 5e81e0d950e8f..194abfd6cb25d 100644
> --- a/drivers/dma/xilinx/zynqmp_dma.c
> +++ b/drivers/dma/xilinx/zynqmp_dma.c

[ ... ]

> @@ -947,7 +948,7 @@ static int zynqmp_dma_chan_probe(struct zynqmp_dma_device *zdev,
>  	if (match_data)
>  		chan->irq_offset = match_data->offset;
>  
> -	chan->is_dmacoherent =  of_property_read_bool(node, "dma-coherent");
> +	chan->is_dmacoherent = of_dma_is_coherent(node);

[Severity: High]
This is a pre-existing issue, but does the system resume path fail to
restore the DMA hardware register context?

A system suspend-to-RAM cycle causes the PM domain to power off, which
wipes the DMA register context. Upon resume, zynqmp_dma_resume() calls
pm_runtime_force_resume():

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_resume() {
    ...
    if (!device_may_wakeup(dev))
        return pm_runtime_force_resume(dev);
    ...
}

This re-enables clocks but doesn't call zynqmp_dma_init() to restore
critical registers like ZYNQMP_DMA_DSCR_ATTR and ZYNQMP_DMA_DATA_ATTR.

Will subsequent DMA transfers proceed with non-coherent hardware defaults
and silently bypass CPU cache snooping?


[Severity: High]
This is also a pre-existing issue, but are the hardware descriptor fields
missing explicit little-endian types?

The hardware descriptor layout uses native C types instead of __le32 and
__le64:

struct zynqmp_dma_desc_ll {
    u64 addr;
    u32 size;
    u32 ctrl;
    u64 nxtdscraddr;
    u64 rsvd;
};

When the driver is built on a big-endian architecture (like ARM64 with
CPU_BIG_ENDIAN), the native assignments in zynqmp_dma_config_sg_ll_desc()
will store values in big-endian format:

drivers/dma/xilinx/zynqmp_dma.c:zynqmp_dma_config_sg_ll_desc() {
    ...
    sdesc->size = ddesc->size = len;
    sdesc->addr = src;
    ddesc->addr = dst;
    ...
}

Could the little-endian ZynqMP DMA hardware misinterpret these swapped
sizes and addresses, leading to corrupted DMA transfers and memory corruption?

>  	zdev->chan = chan;
>  	tasklet_setup(&chan->tasklet, zynqmp_dma_do_tasklet);
>  	spin_lock_init(&chan->lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6