linux-kernel - Re: [PATCH v6 2/2] dma: Add Xilinx AXI Video Direct Memory Access Engine driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABb+yY1U_XFQndHq4aumZ+BNbCu3Frrk+x9fM9Dg5nxDT=HfoQ@mail.gmail.com>
Date:	Mon, 24 Mar 2014 20:53:44 +0530
From:	Jassi Brar <jassisinghbrar@...il.com>
To:	Srikanth Thokala <sthokal@...inx.com>
Cc:	Jassi Brar <jaswinder.singh@...aro.org>,
	Dan Williams <dan.j.williams@...el.com>,
	Vinod Koul <vinod.koul@...el.com>,
	Michal Simek <michal.simek@...inx.com>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	Levente Kurusa <levex@...ux.com>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	dmaengine@...r.kernel.org,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v6 2/2] dma: Add Xilinx AXI Video Direct Memory Access
 Engine driver support

On Mon, Mar 24, 2014 at 8:44 PM, Srikanth Thokala <sthokal@...inx.com> wrote:
> On Mon, Mar 24, 2014 at 4:21 PM, Jassi Brar <jaswinder.singh@...aro.org> wrote:
>> On 24 March 2014 14:30, Srikanth Thokala <sthokal@...inx.com> wrote:
>>> Hi Jassi,
>>>
>>> Thanks for the Acked-by.
>>>
>>> On Mon, Mar 24, 2014 at 11:50 AM, Jassi Brar <jassisinghbrar@...il.com> wrote:
>>>> On Tue, Mar 18, 2014 at 12:36 AM, Srikanth Thokala <sthokal@...inx.com> wrote:
>>>>
>>>>> +
>>>>> +/**
>>>>> + * xilinx_vdma_prep_slave_sg - prepare a descriptor for a DMA_SLAVE transaction
>>>>
>>>> Should be xilinx_vdma_dma_prep_interleaved here.
>>>
>>> Ok, I will correct it.  Thanks.
>>>
>>>>
>>>>> +/**
>>>>> + * xilinx_vdma_channel_set_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
>>>>> + *
>>>>> + * @dchan: DMA channel
>>>>> + * @cfg: VDMA device configuration pointer
>>>>> + *
>>>>> + * Return: '0' on success and failure value on error
>>>>> + */
>>>>> +int xilinx_vdma_channel_set_config(struct dma_chan *dchan,
>>>>> +                                       struct xilinx_vdma_config *cfg)
>>>>> +{
>>>>> +       struct xilinx_vdma_chan *chan = to_xilinx_chan(dchan);
>>>>> +       u32 dmacr;
>>>>> +
>>>>> +       if (cfg->reset)
>>>>> +               return xilinx_vdma_chan_reset(chan);
>>>>> +
>>>>> +       dmacr = vdma_ctrl_read(chan, XILINX_VDMA_REG_DMACR);
>>>>> +
>>>>> +       chan->config.frm_dly = cfg->frm_dly;
>>>>> +       chan->config.park = cfg->park;
>>>>> +
>>>>> +       /* genlock settings */
>>>>> +       chan->config.gen_lock = cfg->gen_lock;
>>>>> +       chan->config.master = cfg->master;
>>>>> +
>>>>> +       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;
>>>>> +}
>>>>> +EXPORT_SYMBOL(xilinx_vdma_channel_set_config);
>>>>> +
>>>> Bypassing the DMAEngine api doesn't seem very neat.
>>>> Is there currently any client driver that /changes/ this configuration
>>>> between channel requests? If no, then simply get the configuration
>>>> from DT node and avoid this api.
>>>
>>> This change is recently suggested by Lars in the following patch series,
>>> "dma: Remove comment about embedding dma_slave_config into custom structs"
>>>
>> If some user does indeed need to /change/ the configuration across
>> channel requests, we can keep function. Is there currently any such
>> user?
>
> Yes, we are planning to push these changes soon to drivers/media/.
>
This is not what I asked though. I asked if some client driver changes
the configuration options based on some runtime decision? If yes, then
the api is warranted. Otherwise, you might consider getting the static
configuration from DT. That's all I suggest.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/