On 28-05-20, 05:52, Laurent Pinchart wrote:
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -0,0 +1,1554 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Xilinx ZynqMP DPDMA Engine driver
> + *
> + * Copyright (C) 2015 - 2019 Xilinx, Inc.
2020 please
> +static struct xilinx_dpdma_tx_desc *
> +xilinx_dpdma_chan_alloc_tx_desc(struct xilinx_dpdma_chan *chan)
> +{
> + struct xilinx_dpdma_tx_desc *tx_desc;
> +
> + tx_desc = kzalloc(sizeof(*tx_desc), GFP_KERNEL);
GFP_NOWAIT please, this is called from a prep call so needs to be atomic
context
> +static int xilinx_dpdma_config(struct dma_chan *dchan,
> + struct dma_slave_config *config)
> +{
> + struct xilinx_dpdma_chan *chan = to_xilinx_chan(dchan);
> + unsigned long flags;
> + int ret = 0;
> +
> + if (config->direction != DMA_MEM_TO_DEV)
> + return -EINVAL;
sorry but direction is deprecated and supposed to be remove, can you
please remove this
> +
> + /*
> + * The destination address doesn't need to be specified as the DPDMA is
> + * hardwired to the destination (the DP controller). The transfer
> + * width, burst size and port window size are thus meaningless, they're
> + * fixed both on the DPDMA side and on the DP controller side.
> + */
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + /* Can't reconfigure a running channel. */
> + if (chan->running) {
> + ret = -EBUSY;
> + goto unlock;
> + }
why does this part matter? The configuration is passed here and should
be applied to next descriptor submitted, channel can be busy.
> +
> + /*
> + * Abuse the slave_id to indicate that the channel is part of a video
> + * group.
> + */
Of course, what does video grp mean here?
> + if (chan->id >= ZYNQMP_DPDMA_VIDEO0 && chan->id <= ZYNQMP_DPDMA_VIDEO2)
> + chan->video_group = config->slave_id != 0;
so only thing we care here is slave_id? What about dma burst parameters?
--
~Vinod
