| 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
| ||
|
Message-ID: <4F196214.30608@codeaurora.org>
Date: Fri, 20 Jan 2012 18:16:12 +0530
From: Ravi Kumar V <kumarrav@...eaurora.org>
To: Vinod Koul <vinod.koul@...el.com>
CC: linux-arch@...r.kernel.org, linux@....linux.org.uk, arnd@...db.de,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
bryanh@...eaurora.org, tsoni@...lcomm.com, dwalker@...o99.com,
dan.j.williams@...el.com, davidb@...eaurora.org,
linux-arm-kernel@...ts.infradead.org, johlstei@...lcomm.com
Subject: Re: [PATCH v2 2/2] msm: DMAEngine: Add DMAEngine driver based on
old MSM DMA APIs
On 1/17/2012 8:01 PM, Vinod Koul wrote:
> On Fri, 2012-01-06 at 18:17 +0530, Ravi Kumar V wrote:
>> +static int msm_dma_alloc_chan_resources(struct dma_chan *dchan)
>> +{
>> + struct msm_dma_chan *chan = to_msm_chan(dchan);
>> +
>> + /* Has this channel already been allocated? */
>> + if (chan->desc_pool)
>> + return 1;
> that is _wrong_. This would mean you have allocated 1 descriptor.
> Please read the documentation again.
Yes you are right, i will update in next patch release.
>> +
>> +/*
>> + * Assignes cookie for each transfer descriptor passed.
>> + * Returns
>> + * Assigend cookie for success.
> typo ^^^^^^^^^
I will change
>> +
>> +
>> +/*
>> + * Prepares the transfer descriptors for BOX transaction.
>> + * Returns
>> + * address of dma_async_tx_descriptor for success.
>> + * Pointer of error value for failure.
>> + */
> pls use kernel-doc style for these...
I will update
>> +static struct dma_async_tx_descriptor *msm_dma_prep_box(
>> + struct dma_chan *dchan,
>> + struct dma_box_list *dst_box, struct dma_box_list *src_box,
>> + unsigned int num_list, unsigned long flags)
>> +{
>> +
>> +/*
>> + * Controlling the hardware channel like stopping, flushing.
>> + */
>> +static int msm_dma_chan_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>> + unsigned long arg)
>> +{
>> + int cmd_type = (int) arg;
>> +
>> + if (cmd == DMA_TERMINATE_ALL) {
>> + switch (cmd_type) {
>> + case GRACEFUL_FLUSH:
>> + msm_dmov_stop_cmd(chan->chan_id, NULL, 1);
>> + break;
>> + case FORCED_FLUSH:
>> + /*
>> + * We treate default as forced flush
>> + * so we fall through
>> + */
>> + default:
>> + msm_dmov_stop_cmd(chan->chan_id, NULL, 0);
>> + break;
>> + }
>> + }
> for other cmds you _should_ not return 0
I will update
>> + return 0;
>> +}
>> +
>> +static void msm_dma_chan_remove(struct msm_dma_chan *chan)
>> +{
>> + tasklet_kill(&chan->tasklet);
>> + list_del(&chan->channel.device_node);
>> + kfree(chan);
>> +}
>> +
>> +static __devinit int msm_dma_chan_probe(struct msm_dma_device *qdev,
>> + int channel)
>> +{
>> + struct msm_dma_chan *chan;
>> +
>> + chan = kzalloc(sizeof(*chan), GFP_KERNEL);
>> + if (!chan) {
>> + dev_err(qdev->dev, "no free memory for DMA channels!\n");
>> + return -ENOMEM;
>> + }
>> +
>> + spin_lock_init(&chan->lock);
>> + INIT_LIST_HEAD(&chan->pending_list);
>> + INIT_LIST_HEAD(&chan->active_list);
>> +
>> + chan->chan_id = channel;
>> + chan->completed_cookie = 0;
>> + chan->channel.cookie = 0;
>> + chan->max_len = MAX_TRANSFER_LENGTH;
>> + chan->err = 0;
>> + qdev->chan[channel] = chan;
>> + chan->channel.device =&qdev->common;
>> + chan->dev = qdev->dev;
>> +
>> + tasklet_init(&chan->tasklet, dma_do_tasklet, (unsigned long)chan);
>> +
>> + list_add_tail(&chan->channel.device_node,&qdev->common.channels);
>> + qdev->common.chancnt++;
>> +
>> + return 0;
>> +}
>> +
>> +static int __devinit msm_dma_probe(struct platform_device *pdev)
>> +{
>> + struct msm_dma_device *qdev;
>> + int i;
>> + int ret = 0;
>> +
>> + qdev = kzalloc(sizeof(*qdev), GFP_KERNEL);
> devm_kzalloc pls
I will update.
>> + if (!qdev) {
>> + dev_err(&pdev->dev, "Not enough memory for device\n");
>> + return -ENOMEM;
>> + }
>> +
>> + qdev->dev =&pdev->dev;
>> + INIT_LIST_HEAD(&qdev->common.channels);
>> + qdev->common.device_alloc_chan_resources =
>> + msm_dma_alloc_chan_resources;
>> + qdev->common.device_free_chan_resources =
>> + msm_dma_free_chan_resources;
>> + dma_cap_set(DMA_SG, qdev->common.cap_mask);
>> + dma_cap_set(DMA_BOX, qdev->common.cap_mask);
>> +
>> + qdev->common.device_prep_dma_sg = msm_dma_prep_sg;
>> + qdev->common.device_prep_dma_box = msm_dma_prep_box;
>> + qdev->common.device_issue_pending = msm_dma_issue_pending;
>> + qdev->common.dev =&pdev->dev;
>> + qdev->common.device_tx_status = msm_tx_status;
>> + qdev->common.device_control = msm_dma_chan_control;
>> +
>> + for (i = SD3_CHAN_START; i< MSM_DMA_MAX_CHANS_PER_DEVICE; i++) {
>> + ret = msm_dma_chan_probe(qdev, i);
>> + if (ret) {
>> + dev_err(&pdev->dev, "channel %d probe failed\n", i);
>> + goto chan_free;
>> + }
>> + }
>> +
>> + dev_info(&pdev->dev, "registering dma device\n");
>> +
>> + ret = dma_async_device_register(&qdev->common);
>> +
>> + if (ret) {
>> + dev_err(&pdev->dev, "Registering Dma device failed\n");
>> + goto chan_free;
>> + }
>> +
>> + dev_set_drvdata(&pdev->dev, qdev);
>> + return 0;
>> +chan_free:
>> + for (i = SD3_CHAN_START; i< MSM_DMA_MAX_CHANS_PER_DEVICE; i++) {
>> + if (qdev->chan[i])
>> + msm_dma_chan_remove(qdev->chan[i]);
>> + }
>> + kfree(qdev);
> you leak the chan memory allocated
I am freeing chan memory from msm_dma_chan_remove() called above.
>> + return ret;
>> +}
>> +
>> +static int __devexit msm_dma_remove(struct platform_device *pdev)
>> +{
>> + struct msm_dma_device *qdev = platform_get_drvdata(pdev);
>> + int i;
>> +
>> + dma_async_device_unregister(&qdev->common);
>> +
>> + for (i = SD3_CHAN_START; i< MSM_DMA_MAX_CHANS_PER_DEVICE; i++) {
>> + if (qdev->chan[i])
>> + msm_dma_chan_remove(qdev->chan[i]);
>> + }
>> +
>> + dev_set_drvdata(&pdev->dev, NULL);
>> + kfree(qdev);
> ditto
I am freeing chan memory from msm_dma_chan_remove() called above.
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver msm_dma_driver = {
>> + .remove = __devexit_p(msm_dma_remove),
>> + .driver = {
>> + .name = "msm_dma",
>> + .owner = THIS_MODULE,
>> + },
>> +};
>> +
>> +static __init int msm_dma_init(void)
>> +{
>> + return platform_driver_probe(&msm_dma_driver, msm_dma_probe);
>> +}
>> +subsys_initcall(msm_dma_init);
>> +
>> +static void __exit msm_dma_exit(void)
>> +{
>> + platform_driver_unregister(&msm_dma_driver);
>> +}
>> +module_exit(msm_dma_exit);
>> +
>> +MODULE_DESCRIPTION("Qualcomm DMA driver");
>> +MODULE_LICENSE("GPL v2");
> More comments, once I understand what "BOX mode" is?
> Also, if you can add basic driver without box mode, we can merge fairly
> quickly, box mode can come once we understand what we want and how...
We also implemented SG mode using device_prep_dma_sg() but we need to
pass extra parameter "command configuration" to our HW with every
descriptor.
Please can you suggest a way to transfer private param to
device_prep_dma_sg()
>
>
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
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/
Powered by blists - more mailing lists