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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux