Re: [PATCH] dma: Add Xilinx AXI Video Direct Memory Access Enginedriver support

From: Srikanth Thokala
Date: Mon Jan 20 2014 - 02:27:24 EST

Next message: Srikanth Thokala: "Re: [PATCH] dma: Add Xilinx AXI Video Direct Memory Access Enginedriver support"
Previous message: Xishi Qiu: "[PATCH V2] doc/kmemcheck: add kmemcheck to kernel-parameters"
In reply to: Levente Kurusa: "Re: [PATCH] dma: Add Xilinx AXI Video Direct Memory Access Enginedriver support"
Next in thread: Philip Balister: "Re: [PATCH] dma: Add Xilinx AXI Video Direct Memory Access Enginedriver support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Levente,

On Thu, Jan 16, 2014 at 11:57 PM, Levente Kurusa <levex@xxxxxxxxx> wrote:
> Hello,
>
> On 01/16/2014 06:53 PM, Srikanth Thokala wrote:
>> This is the driver for the AXI Video Direct Memory Access (AXI
>> VDMA) core, which is a soft Xilinx IP core that provides high-
>> bandwidth direct memory access between memory and AXI4-Stream
>> type video target peripherals. The core provides efficient two
>> dimensional DMA operations with independent asynchronous read
>> and write channel operation.
>>
>> This module works on Zynq (ARM Based SoC) and Microblaze platforms.
>>
>> Signed-off-by: Srikanth Thokala <sthokal@xxxxxxxxxx>
>> ---
>
> Just a few suggestions and fixes.
>
>> NOTE:
>> 1. Created a separate directory 'dma/xilinx' as Xilinx has two more
>> DMA IPs and we are also planning to upstream these drivers soon.
>> 2. Rebased on v3.13.0-rc8
>> ---
>> .../devicetree/bindings/dma/xilinx/xilinx_vdma.txt | 71 +
>> .../bindings/dma/xilinx/xilinx_vdma_test.txt | 39 +
>> drivers/dma/Kconfig | 23 +
>> drivers/dma/Makefile | 1 +
>> drivers/dma/xilinx/Makefile | 2 +
>> drivers/dma/xilinx/xilinx_vdma.c | 1497 ++++++++++++++++++++
>> drivers/dma/xilinx/xilinx_vdma_test.c | 629 ++++++++
>> 7 files changed, 2262 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
>> create mode 100644 Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma_test.txt
>> create mode 100644 drivers/dma/xilinx/Makefile
>> create mode 100644 drivers/dma/xilinx/xilinx_vdma.c
>> create mode 100644 drivers/dma/xilinx/xilinx_vdma_test.c
>>
>> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
>> new file mode 100644
>> index 0000000..3f5c428
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma.txt
>> @@ -0,0 +1,71 @@
>> +Xilinx AXI VDMA engine, it does transfers between memory and video devices.
>> +It can be configured to have one channel or two channels. If configured
>> +as two channels, one is to transmit to the video device and another is
>> +to receive from the video device.
>> +
>> +Required properties:
>> +- compatible: Should be "xlnx,axi-vdma-1.00.a"
>> +- #dma-cells: Should be <1>, see "dmas" property below
>> +- reg: Should contain VDMA registers location and length.
>> +- interrupts: Should contain per channel VDMA interrupts.
>> +- compatible (child node): It should be either "xlnx,axi-vdma-mm2s-channel" or
>> + "xlnx,axi-vdma-s2mm-channel". It depends on the hardware design and it
>> + can also have both channels.
>> +- xlnx,device-id: Should contain device number in each channel. It should be
>> + {0,1,2...so on} to the number of VDMA devices configured in hardware.
>> +- xlnx,num-fstores: Should be the number of framebuffers as configured in h/w.
>> +- xlnx,data-width: Should contain the stream data width, takes {32,64...so on}.
>> +- xlnx,flush-fsync: (Optional) Tells whether which channel to Flush on Fsync.
>> + It takes following values:
>> + {1}, flush both channels
>> + {2}, flush mm2s channel
>> + {3}, flush s2mm channel
>> +- xlnx,include-sg: (Optional) Tells whether configured for Scatter-mode in
>> + the hardware.
>> +- xlnx,include-dre: (Optional) Tells whether hardware is configured for Data
>> + Realignment Engine.
>> +- xlnx,genlock-mode: (Optional) Tells whether Genlock synchornisation is
>
> s/synchornisation/synchronization

Sure, will take care.

>
>> + enabled/disabled in hardware.
>> +
>> +Example:
>> +++++++++
>> +
>> +axi_vdma_0: axivdma@40030000 {
>> + compatible = "xlnx,axi-vdma-1.00.a";
>> + #dma_cells = <1>;
>> + reg = < 0x40030000 0x10000 >;
>> + xlnx,flush-fsync = <0x1>;
>> + dma-channel@40030000 {
>> + compatible = "xlnx,axi-vdma-mm2s-channel";
>> + interrupts = < 0 54 4 >;
>> + xlnx,num-fstores = <0x8>;
>> + xlnx,device-id = <0x0>;
>> + xlnx,datawidth = <0x40>;
>> + } ;
>> + dma-channel@40030030 {
>> + compatible = "xlnx,axi-vdma-s2mm-channel";
>> + interrupts = < 0 53 4 >;
>> + xlnx,num-fstores = <0x8>;
>> + xlnx,device-id = <0x0>;
>> + xlnx,datawidth = <0x40>;
>> + } ;
>> +} ;
>> +
>> +
>> +* Xilinx Video DMA client
>> +
>> +Required properties:
>> +- dmas: a list of <[Video DMA device phandle] [Channel ID]> pairs,
>> + where Channel ID is '0' for write/tx and '1' for read/rx
>> + channel.
>> +- dma-names: a list of DMA channel names, one per "dmas" entry
>> +
>> +VDMA Test Client Example:
>> ++++++++++++++++++++++++++
>> +
>> +vdmatest_0: vdmatest@0 {
>> + compatible ="xlnx,axi-vdma-test-1.00.a";
>> + dmas = <&axi_vdma_0 0
>> + &axi_vdma_0 1>;
>> + dma-names = "vdma0", "vdma1";
>> +} ;
>> diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma_test.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma_test.txt
>> new file mode 100644
>> index 0000000..5821fdc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_vdma_test.txt
>> @@ -0,0 +1,39 @@
>> +* Xilinx Video DMA Test client
>> +
>> +Required properties:
>> +- compatible: Should be "xlnx,axi-vdma-test-1.00.a"
>> +- dmas: a list of <[Video DMA device phandle] [Channel ID]> pairs,
>> + where Channel ID is '0' for write/tx and '1' for read/rx
>> + channel.
>> +- dma-names: a list of DMA channel names, one per "dmas" entry
>> +- xlnx,num-fstores: Should be the number of framebuffers as configured in
>> + VDMA device node.
>> +
>> +Example:
>> +++++++++
>> +
>> +vdmatest_0: vdmatest@0 {
>> + compatible ="xlnx,axi-vdma-test-1.00.a";
>> + dmas = <&axi_vdma_0 0
>> + &axi_vdma_0 1>;
>> + dma-names = "vdma0", "vdma1";
>> + xlnx,num-fstores = <0x8>;
>> +} ;
>> +
>> +
>> +Xilinx Video DMA Device Node Example
>> +++++++++++++++++++++++++++++++++++++
>> +axi_vdma_0: axivdma@44A40000 {
>> + compatible = "xlnx,axi-vdma-1.00.a";
>> + ...
>> + dma-channel@44A40000 {
>> + ...
>> + xlnx,num-fstores = <0x8>;
>> + ...
>> + } ;
>> + dma-channel@44A40030 {
>> + ...
>> + xlnx,num-fstores = <0x8>;
>> + ...
>> + } ;
>> +} ;
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index c823daa..675719f 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -334,6 +334,20 @@ config K3_DMA
>> Support the DMA engine for Hisilicon K3 platform
>> devices.
>>
>> +config XILINX_VDMA
>> + tristate "Xilinx AXI VDMA Engine"
>> + depends on (ARCH_ZYNQ || MICROBLAZE)
>> + select DMA_ENGINE
>> + help
>> + Enable support for Xilinx AXI VDMA Soft IP.
>> +
>> + This engine provides high-bandwidth direct memory access
>> + between memory and AXI4-Stream video type target
>> + peripherals including peripherals which support AXI4-
>> + Stream Video Protocol. It has two stream interfaces/
>> + channels, Memory Mapped to Stream (MM2S) and Stream to
>> + Memory Mapped (S2MM) for the data transfers.
>> +
>> config DMA_ENGINE
>> bool
>>
>> @@ -384,4 +398,13 @@ config DMATEST
>> config DMA_ENGINE_RAID
>> bool
>>
>> +config XILINX_VDMA_TEST
>> + tristate "DMA Test client for Xilinx AXI VDMA"
>> + depends on XILINX_VDMA
>> + help
>> + Simple VDMA driver test client. This test assumes both
>> + the stream interfaces of VDMA engine, MM2S and S2MM are
>> + connected back-to-back in the hardware design.
>> +
>> + Say N unless you're debugging a DMA Device driver.
>> endif
>> diff --git a/drivers/dma/Makefile b/drivers/dma/Makefile
>> index 0ce2da9..d84130b 100644
>> --- a/drivers/dma/Makefile
>> +++ b/drivers/dma/Makefile
>> @@ -42,3 +42,4 @@ obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o
>> obj-$(CONFIG_DMA_JZ4740) += dma-jz4740.o
>> obj-$(CONFIG_TI_CPPI41) += cppi41.o
>> obj-$(CONFIG_K3_DMA) += k3dma.o
>> +obj-y += xilinx/
>> diff --git a/drivers/dma/xilinx/Makefile b/drivers/dma/xilinx/Makefile
>> new file mode 100644
>> index 0000000..cef1e88
>> --- /dev/null
>> +++ b/drivers/dma/xilinx/Makefile
>> @@ -0,0 +1,2 @@
>> +obj-$(CONFIG_XILINX_VDMA) += xilinx_vdma.o
>> +obj-$(CONFIG_XILINX_VDMA_TEST) += xilinx_vdma_test.o
>> diff --git a/drivers/dma/xilinx/xilinx_vdma.c b/drivers/dma/xilinx/xilinx_vdma.c
>> new file mode 100644
>> index 0000000..66a12de
>> --- /dev/null
>> +++ b/drivers/dma/xilinx/xilinx_vdma.c
>> @@ -0,0 +1,1497 @@
>> +/*
>> + * DMA driver for Xilinx Video DMA Engine
>> + *
>> + * Copyright (C) 2010-2013 Xilinx, Inc. All rights reserved.
>
> Happy new year! ;-)

Ok :-)

>
>
>> + *
>> + * Based on the Freescale DMA driver.
>> + *
>> + * Description:
>> + * The AXI Video Direct Memory Access (AXI VDMA) core is a soft Xilinx IP
>> + * core that provides high-bandwidth direct memory access between memory
>> + * and AXI4-Stream type video target peripherals. The core provides efficient
>> + * two dimensional DMA operations with independent asynchronous read (S2MM)
>> + * and write (MM2S) channel operation. It can be configured to have either
>> + * one channel or two channels. If configured as two channels, one is to
>> + * transmit to the video device (MM2S) and another is to receive from the
>> + * video device (S2MM). Initialization, status, interrupt and management
>> + * registers are accessed through an AXI4-Lite slave interface.
>> + *
>> + * This program is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#include <linux/amba/xilinx_dma.h>
>> +#include <linux/bitops.h>
>> +#include <linux/dmapool.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_dma.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/slab.h>
>> +
>> +/* Register/Descriptor Offsets */
>> +#define XILINX_VDMA_MM2S_CTRL_OFFSET 0x0000
>> +#define XILINX_VDMA_S2MM_CTRL_OFFSET 0x0030
>> +#define XILINX_VDMA_MM2S_DESC_OFFSET 0x0050
>> +#define XILINX_VDMA_S2MM_DESC_OFFSET 0x00a0
>> +
>> +/* Control Registers */
>> +#define XILINX_VDMA_REG_DMACR 0x0000
>> +#define XILINX_VDMA_DMACR_DELAY_MAX 0xff
>> +#define XILINX_VDMA_DMACR_DELAY_SHIFT 24
>> +#define XILINX_VDMA_DMACR_FRAME_COUNT_MAX 0xff
>> +#define XILINX_VDMA_DMACR_FRAME_COUNT_SHIFT 16
>> +#define XILINX_VDMA_DMACR_ERR_IRQ (1 << 14)
>> +#define XILINX_VDMA_DMACR_DLY_CNT_IRQ (1 << 13)
>> +#define XILINX_VDMA_DMACR_FRM_CNT_IRQ (1 << 12)
>> +#define XILINX_VDMA_DMACR_MASTER_SHIFT 8
>> +#define XILINX_VDMA_DMACR_FSYNCSRC_SHIFT 5
>> +#define XILINX_VDMA_DMACR_FRAMECNT_EN (1 << 4)
>> +#define XILINX_VDMA_DMACR_GENLOCK_EN (1 << 3)
>> +#define XILINX_VDMA_DMACR_RESET (1 << 2)
>> +#define XILINX_VDMA_DMACR_CIRC_EN (1 << 1)
>> +#define XILINX_VDMA_DMACR_RUNSTOP (1 << 0)
>
> You could use the BIT(n) field from <linux/bitops.h>
>
>> +#define XILINX_VDMA_DMACR_DELAY_MASK \
>> + (XILINX_VDMA_DMACR_DELAY_MAX << \
>> + XILINX_VDMA_DMACR_DELAY_SHIFT)
>> +#define XILINX_VDMA_DMACR_FRAME_COUNT_MASK \
>> + (XILINX_VDMA_DMACR_FRAME_COUNT_MAX << \
>> + XILINX_VDMA_DMACR_FRAME_COUNT_SHIFT)
>> +#define XILINX_VDMA_DMACR_MASTER_MASK \
>> + (0xf << XILINX_VDMA_DMACR_MASTER_SHIFT)
>> +#define XILINX_VDMA_DMACR_FSYNCSRC_MASK \
>> + (3 << XILINX_VDMA_DMACR_FSYNCSRC_SHIFT)
>> +
>> +#define XILINX_VDMA_REG_DMASR 0x0004
>> +#define XILINX_VDMA_DMASR_DELAY_SHIFT 24
>> +#define XILINX_VDMA_DMASR_FRAME_COUNT_SHIFT 16
>> +#define XILINX_VDMA_DMASR_EOL_LATE_ERR (1 << 15)
>> +#define XILINX_VDMA_DMASR_ERR_IRQ (1 << 14)
>> +#define XILINX_VDMA_DMASR_DLY_CNT_IRQ (1 << 13)
>> +#define XILINX_VDMA_DMASR_FRM_CNT_IRQ (1 << 12)
>> +#define XILINX_VDMA_DMASR_SOF_LATE_ERR (1 << 11)
>> +#define XILINX_VDMA_DMASR_SG_DEC_ERR (1 << 10)
>> +#define XILINX_VDMA_DMASR_SG_SLV_ERR (1 << 9)
>> +#define XILINX_VDMA_DMASR_EOF_EARLY_ERR (1 << 8)
>> +#define XILINX_VDMA_DMASR_SOF_EARLY_ERR (1 << 7)
>> +#define XILINX_VDMA_DMASR_DMA_DEC_ERR (1 << 6)
>> +#define XILINX_VDMA_DMASR_DMA_SLAVE_ERR (1 << 5)
>> +#define XILINX_VDMA_DMASR_DMA_INT_ERR (1 << 4)
>> +#define XILINX_VDMA_DMASR_IDLE (1 << 1)
>> +#define XILINX_VDMA_DMASR_HALTED (1 << 0)
>
> Here as well.

Sure.

>> +#define XILINX_VDMA_DMASR_DELAY_MASK \
>> + (0xff << XILINX_VDMA_DMASR_DELAY_SHIFT)
>> +#define XILINX_VDMA_DMASR_FRAME_COUNT_MASK \
>> + (0xff << XILINX_VDMA_DMASR_FRAME_COUNT_SHIFT)
>> +
>> +#define XILINX_VDMA_REG_CURDESC 0x0008
>> +#define XILINX_VDMA_REG_TAILDESC 0x0010
>> +#define XILINX_VDMA_REG_REG_INDEX 0x0014
>> +#define XILINX_VDMA_REG_FRMSTORE 0x0018
>> +#define XILINX_VDMA_REG_THRESHOLD 0x001c
>> +#define XILINX_VDMA_REG_FRMPTR_STS 0x0024
>> +#define XILINX_VDMA_REG_PARK_PTR 0x0028
>> +#define XILINX_VDMA_PARK_PTR_WR_REF_SHIFT 8
>> +#define XILINX_VDMA_PARK_PTR_RD_REF_SHIFT 0
>> +#define XILINX_VDMA_REG_VDMA_VERSION 0x002c
>> +
>> [...]
>> +
>> +/**
>> + * xilinx_vdma_slave_config - Configure VDMA channel
>> + * Run-time configuration for Axi VDMA, supports:
>> + * . halt the channel
>> + * . configure interrupt coalescing and inter-packet delay threshold
>> + * . start/stop parking
>> + * . enable genlock
>> + * . set transfer information using config struct
>> + *
>> + * @chan: Driver specific VDMA Channel pointer
>> + * @cfg: Channel configuration pointer
>> + *
>> + * Return: '0' on success and failure value on error
>> + */
>> +static int xilinx_vdma_slave_config(struct xilinx_vdma_chan *chan,
>> + struct xilinx_vdma_config *cfg)
>> +{
>> + u32 dmacr;
>> +
>> + if (cfg->reset)
>> + return xilinx_vdma_chan_reset(chan);
>> +
>> + dmacr = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR);
>> +
>> + /* If vsize is -1, it is park-related operations */
>> + if (cfg->vsize == -1) {
>> + if (cfg->park)
>> + dmacr &= ~XILINX_VDMA_DMACR_CIRC_EN;
>> + else
>> + dmacr |= XILINX_VDMA_DMACR_CIRC_EN;
>> +
>> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMACR, dmacr);
>> + return 0;
>> + }
>> +
>> + /* If hsize is -1, it is interrupt threshold settings */
>> + if (cfg->hsize == -1) {
>> + if (cfg->coalesc <= XILINX_VDMA_DMACR_FRAME_COUNT_MAX) {
>> + dmacr &= ~XILINX_VDMA_DMACR_FRAME_COUNT_MASK;
>> + dmacr |= cfg->coalesc <<
>> + XILINX_VDMA_DMACR_FRAME_COUNT_SHIFT;
>> + chan->config.coalesc = cfg->coalesc;
>> + }
>> +
>> + if (cfg->delay <= XILINX_VDMA_DMACR_DELAY_MAX) {
>> + dmacr &= ~XILINX_VDMA_DMACR_DELAY_MASK;
>> + dmacr |= cfg->delay << XILINX_VDMA_DMACR_DELAY_SHIFT;
>> + chan->config.delay = cfg->delay;
>> + }
>> +
>> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMACR, dmacr);
>> + return 0;
>> + }
>> +
>> + /* Transfer information */
>> + chan->config.vsize = cfg->vsize;
>> + chan->config.hsize = cfg->hsize;
>> + chan->config.stride = cfg->stride;
>> + chan->config.frm_dly = cfg->frm_dly;
>> + chan->config.park = cfg->park;
>
> Can't this be done with a memcpy? Not sure though.
>> +
>> + /* genlock settings */
>> + chan->config.gen_lock = cfg->gen_lock;
>> + chan->config.master = cfg->master;
>
> This as well maybe.

Some of the parameters have condition checks before assigning (please
see below),
so I felt this would be a better way.

>> +
>> + if (cfg->gen_lock && chan->genlock) {
>> + dmacr |= XILINX_VDMA_DMACR_GENLOCK_EN;
>> + dmacr |= cfg->master << XILINX_VDMA_DMACR_MASTER_SHIFT;
>> + }
>> +
>> + chan->config.frm_cnt_en = cfg->frm_cnt_en;
>> + if (cfg->park)
>> + chan->config.park_frm = cfg->park_frm;
>> + else
>> + chan->config.park_frm = -1;
>> +
>> + chan->config.coalesc = cfg->coalesc;
>> + chan->config.delay = cfg->delay;
>> + if (cfg->coalesc <= XILINX_VDMA_DMACR_FRAME_COUNT_MAX) {
>> + dmacr |= cfg->coalesc << XILINX_VDMA_DMACR_FRAME_COUNT_SHIFT;
>> + chan->config.coalesc = cfg->coalesc;
>> + }
>> +
>> + if (cfg->delay <= XILINX_VDMA_DMACR_DELAY_MAX) {
>> + dmacr |= cfg->delay << XILINX_VDMA_DMACR_DELAY_SHIFT;
>> + chan->config.delay = cfg->delay;
>> + }
>> +
>> + /* FSync Source selection */
>> + dmacr &= ~XILINX_VDMA_DMACR_FSYNCSRC_MASK;
>> + dmacr |= cfg->ext_fsync << XILINX_VDMA_DMACR_FSYNCSRC_SHIFT;
>> +
>> + vdma_ctrl_write(chan, XILINX_VDMA_REG_DMACR, dmacr);
>> + return 0;
>> +}
>> +
>> [...]
>> +
>> +/**
>> + * xilinx_vdma_probe - Driver probe function
>> + * @pdev: Pointer to the platform_device structure
>> + *
>> + * Return: '0' on success and failure value on error
>> + */
>> +static int xilinx_vdma_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *node = pdev->dev.of_node;
>> + struct xilinx_vdma_device *xdev;
>> + struct device_node *child;
>> + struct resource *io;
>> + u32 num_frames;
>> + int i, err;
>> +
>> + dev_info(&pdev->dev, "Probing xilinx axi vdma engine\n");
>> +
>> + /* Allocate and initialize the DMA engine structure */
>> + xdev = devm_kzalloc(&pdev->dev, sizeof(*xdev), GFP_KERNEL);
>> + if (!xdev)
>> + return -ENOMEM;
>> +
>> + xdev->dev = &pdev->dev;
>> +
>> + /* Request and map I/O memory */
>> + io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + xdev->regs = devm_ioremap_resource(&pdev->dev, io);
>> + if (IS_ERR(xdev->regs))
>> + return PTR_ERR(xdev->regs);
>> +
>> + /* Retrieve the DMA engine properties from the device tree */
>> + xdev->has_sg = of_property_read_bool(node, "xlnx,include-sg");
>> +
>> + err = of_property_read_u32(node, "xlnx,num-fstores", &num_frames);
>> + if (err < 0) {
>> + dev_err(xdev->dev, "missing xlnx,num-fstores property\n");
>> + return err;
>> + }
>> +
>> + of_property_read_u32(node, "xlnx,flush-fsync", &xdev->flush_on_fsync);
>> +
>> + /* Initialize the DMA engine */
>> + xdev->common.dev = &pdev->dev;
>> +
>> + INIT_LIST_HEAD(&xdev->common.channels);
>> + dma_cap_set(DMA_SLAVE, xdev->common.cap_mask);
>> + dma_cap_set(DMA_PRIVATE, xdev->common.cap_mask);
>> +
>> + xdev->common.device_alloc_chan_resources =
>> + xilinx_vdma_alloc_chan_resources;
>> + xdev->common.device_free_chan_resources =
>> + xilinx_vdma_free_chan_resources;
>> + xdev->common.device_prep_slave_sg = xilinx_vdma_prep_slave_sg;
>> + xdev->common.device_control = xilinx_vdma_device_control;
>> + xdev->common.device_tx_status = xilinx_vdma_tx_status;
>> + xdev->common.device_issue_pending = xilinx_vdma_issue_pending;
>> +
>> + platform_set_drvdata(pdev, xdev);
>> +
>> + /* Initialize the channels */
>> + for_each_child_of_node(node, child) {
>> + err = xilinx_vdma_chan_probe(xdev, child);
>> + if (err < 0)
>> + goto error;
>> + }
>> +
>> + for (i = 0; i < XILINX_VDMA_MAX_CHANS_PER_DEVICE; i++) {
>> + if (xdev->chan[i])
>> + xdev->chan[i]->num_frms = num_frames;
>> + }
>> +
>> + /* Register the DMA engine with the core */
>> + dma_async_device_register(&xdev->common);
>> +
>> + err = of_dma_controller_register(node, of_dma_xilinx_xlate,
>> + &xdev->common);
>> + if (err < 0)
>> + dev_err(&pdev->dev, "Unable to register DMA to DT\n");
>
> Shouldn't this return 'err'? Like when you can't register the DMA to the node,
> you might want to bail out, don't you?

DMA Client devices, that do not have dt nodes, can still get the channel
through dma_request_channel() even if this call failed to register. So, doing
this way.

>
>> +
>> + return 0;
>> +
>> +error:
>> + for (i = 0; i < XILINX_VDMA_MAX_CHANS_PER_DEVICE; i++) {
>> + if (xdev->chan[i])
>> + xilinx_vdma_chan_remove(xdev->chan[i]);
>> + }
>> +
>> + return err;
>> +}
>> [...]
>> +static int xilinx_vdmatest_add_slave_channels(struct dma_chan *tx_chan,
>> + struct dma_chan *rx_chan)
>> +{
>> + struct xilinx_vdmatest_chan *tx_dtc, *rx_dtc;
>> + unsigned int thread_count = 0;
>> +
>> + tx_dtc = kmalloc(sizeof(struct xilinx_vdmatest_chan), GFP_KERNEL);
>> + if (!tx_dtc)
>> + return -ENOMEM;
>> +
>> + rx_dtc = kmalloc(sizeof(struct xilinx_vdmatest_chan), GFP_KERNEL);
>> + if (!rx_dtc)
>> + return -ENOMEM;
>
> That's a memory leak. You gotta kfree tx_dtc.

Will take care in the v2 patch.

Thanks
Srikanth

>> [...]
>>
>
>
> --
> Regards,
> Levente Kurusa
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Srikanth Thokala: "Re: [PATCH] dma: Add Xilinx AXI Video Direct Memory Access Enginedriver support"
Previous message: Xishi Qiu: "[PATCH V2] doc/kmemcheck: add kmemcheck to kernel-parameters"
In reply to: Levente Kurusa: "Re: [PATCH] dma: Add Xilinx AXI Video Direct Memory Access Enginedriver support"
Next in thread: Philip Balister: "Re: [PATCH] dma: Add Xilinx AXI Video Direct Memory Access Enginedriver support"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]