| 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: <1326810701.1540.161.camel@vkoul-udesk3>
Date: Tue, 17 Jan 2012 20:01:41 +0530
From: Vinod Koul <vinod.koul@...el.com>
To: Ravi Kumar V <kumarrav@...eaurora.org>
Cc: dan.j.williams@...el.com, arnd@...db.de,
linux-arch@...r.kernel.org, davidb@...eaurora.org,
dwalker@...o99.com, bryanh@...eaurora.org, linux@....linux.org.uk,
tsoni@...lcomm.com, johlstei@...lcomm.com,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 2/2] msm: DMAEngine: Add DMAEngine driver based on
old MSM DMA APIs
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.
> +
> +/*
> + * Assignes cookie for each transfer descriptor passed.
> + * Returns
> + * Assigend cookie for success.
typo ^^^^^^^^^
> +
> +
> +/*
> + * 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...
> +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)
> +{
> + struct msm_dma_chan *chan;
> + struct msm_dma_desc_sw *new;
> + int cmd_cnt = 0;
> + int first = 0;
> + int i;
> + struct adm_box_cmd_t *box_cmd_vaddr;
> +
> + if (!dchan)
> + return ERR_PTR(-EINVAL);
> +
> + if (num_list == 0)
> + return ERR_PTR(-EINVAL);
> + if (!dst_box || !src_box)
> + return ERR_PTR(-EINVAL);
> +
> + chan = to_msm_chan(dchan);
> +
> + cmd_cnt = num_list;
> +
> + new = msm_dma_alloc_descriptor(chan, cmd_cnt, DMA_BOX);
> +
> + if (!new) {
> + dev_err(chan->dev,
> + "No free memory for link descriptor\n");
> + return ERR_PTR(-ENOMEM);
> + }
> + box_cmd_vaddr = new->vaddr_box_cmd;
> +
> + for (i = 0; i < num_list; i++) {
> +
> + box_cmd_vaddr->src_row_addr =
> + box_dma_row_address(src_box + i);
> + box_cmd_vaddr->src_dst_len =
> + (box_dma_row_len(src_box + i) &
> + MSM_BOX_SRC_RLEN_MASK) <<
> + MSM_BOX_SRC_RLEN_SHIFT;
> + box_cmd_vaddr->cmd_cntrl =
> + (box_dma_private_data(src_box + i) &
> + MSM_DMA_CMD_MASK) | MSM_BOX_MODE_CMD;
> +
> + box_cmd_vaddr->num_rows = (box_dma_row_num(src_box + i) &
> + MSM_BOX_SRC_RNUM_MASK) <<
> + MSM_BOX_SRC_RNUM_SHIFT;
> +
> + box_cmd_vaddr->row_offset = (box_dma_row_offset(src_box + i) &
> + MSM_BOX_SRC_ROFFSET_MASK) <<
> + MSM_BOX_SRC_ROFFSET_SHIFT;
> +
> + if (first == 0) {
> + if (cmd_cnt == 1)
> + box_cmd_vaddr->cmd_cntrl |=
> + CMD_LC | CMD_OCB | CMD_OCU;
> + else
> + box_cmd_vaddr->cmd_cntrl |=
> + CMD_OCB;
> + first = 1;
> + }
> + box_cmd_vaddr++;
> + }
> +
> + if (cmd_cnt > 1) {
> + box_cmd_vaddr--;
> + box_cmd_vaddr->cmd_cntrl |= CMD_LC | CMD_OCU;
> + }
> +
> + box_cmd_vaddr = new->vaddr_box_cmd;
> +
> + for (i = 0; i < num_list; i++) {
> +
> + box_cmd_vaddr->dst_row_addr = box_dma_row_address(dst_box + i);
> + box_cmd_vaddr->src_dst_len |=
> + (box_dma_row_len(dst_box + i) & MSM_BOX_DST_RLEN_MASK);
> + box_cmd_vaddr->num_rows |=
> + (box_dma_row_num(dst_box + i) &
> + MSM_BOX_DST_RNUM_MASK);
> +
> + box_cmd_vaddr->row_offset |=
> + (box_dma_row_offset(dst_box + i) &
> + MSM_BOX_DST_ROFFSET_MASK);
> + box_cmd_vaddr++;
> + }
> +#ifdef DEBUG
> + i = 0;
> + box_cmd_vaddr = new->vaddr_box_cmd;
> + do {
> + dev_dbg(chan->dev, "cmd %d src 0x%x dst 0x%x len 0x%x "
> + "cntrl 0x%x row_offset 0x%x num_rows 0x%x\n",
> + i, box_cmd_vaddr->src_row_addr,
> + box_cmd_vaddr->dst_row_addr,
> + box_cmd_vaddr->src_dst_len,
> + box_cmd_vaddr->cmd_cntrl,
> + box_cmd_vaddr->row_offset,
> + box_cmd_vaddr->num_rows);
> + box_cmd_vaddr++;
> + i++;
> + } while (!((box_cmd_vaddr-1)->cmd_cntrl & CMD_LC));
> +#endif
> + new->async_tx.flags = flags;
> + new->async_tx.cookie = -EBUSY;
> +
> + return &new->async_tx;
> +}
> +
> +/*
> + * 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
> + 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
> + 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
> + 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
> +
> + 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...
--
~Vinod
--
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