Hi Vinod,
Thank you for the patch.
On Sat, Jul 18, 2020 at 07:21:59PM +0530, Vinod Koul wrote:
> xilinx_dpdma_config() channel id is unsigned int and compares with
> ZYNQMP_DPDMA_VIDEO0 which is zero, so remove this comparison
>
> drivers/dma/xilinx/xilinx_dpdma.c:1073:15: warning: comparison of
> unsigned expression in ‘>= 0’ is always true [-Wtype-limits] if
> (chan->id >= ZYNQMP_DPDMA_VIDEO0 && chan->id <= ZYNQMP_DPDMA_VIDEO2)
I didn't see that warning, how do you produce it ?
> Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx>
> ---
> drivers/dma/xilinx/xilinx_dpdma.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
> index af88a6762ef4..8e602378f2dc 100644
> --- a/drivers/dma/xilinx/xilinx_dpdma.c
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -1070,7 +1070,7 @@ static int xilinx_dpdma_config(struct dma_chan *dchan,
> * Abuse the slave_id to indicate that the channel is part of a video
> * group.
> */
> - if (chan->id >= ZYNQMP_DPDMA_VIDEO0 && chan->id <= ZYNQMP_DPDMA_VIDEO2)
> + if (chan->id <= ZYNQMP_DPDMA_VIDEO2)
While this doesn't affect the behaviour, I'm worried about the risk of
introducing bugs in the future if the ZYNQMP_DPDMA_VIDEO0 becomes
non-zero. On the other hand, that's part of the DT ABI, so it shouldn't
change. How about
switch (chan->id) {
case ZYNQMP_DPDMA_VIDEO0:
case ZYNQMP_DPDMA_VIDEO1:
case ZYNQMP_DPDMA_VIDEO2:
chan->video_group = config->slave_id != 0;
break;
}
instead ?
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> chan->video_group = config->slave_id != 0;
>
> spin_unlock_irqrestore(&chan->lock, flags);
--
Regards,
Laurent Pinchart
