dmaengine Archive on lore.kernel.org
 help / color / Atom feed
From: Peng Ma <peng.ma@nxp.com>
To: Vinod Koul <vkoul@kernel.org>
Cc: "dan.j.williams@intel.com" <dan.j.williams@intel.com>,
	Leo Li <leoyang.li@nxp.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>
Subject: RE: [EXT] Re: [V4 2/2] dmaengine: fsl-dpaa2-qdma: Add NXP dpaa2 qDMA controller driver for Layerscape SoCs
Date: Wed, 11 Sep 2019 02:01:50 +0000
Message-ID: <VI1PR04MB443142772665BB29B909DFF4EDB10@VI1PR04MB4431.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20190624164556.GD2962@vkoul-mobl>

Hi Vinod,

I send those series patchs(V5) on June 25, 2019. I haven't received any comments yet. Their current state
is "Not Applicable", so please let me know what I need to do next.
Thanks very much for your comments.

Patch link:
https://patchwork.kernel.org/patch/11015035/
https://patchwork.kernel.org/patch/11015033/

Best Regards,
Peng
>-----Original Message-----
>From: Vinod Koul <vkoul@kernel.org>
>Sent: 2019Äê6ÔÂ25ÈÕ 0:46
>To: Peng Ma <peng.ma@nxp.com>
>Cc: dan.j.williams@intel.com; Leo Li <leoyang.li@nxp.com>;
>linux-kernel@vger.kernel.org; dmaengine@vger.kernel.org
>Subject: [EXT] Re: [V4 2/2] dmaengine: fsl-dpaa2-qdma: Add NXP dpaa2 qDMA
>controller driver for Layerscape SoCs
>
>Caution: EXT Email
>
>On 13-06-19, 10:13, Peng Ma wrote:
>> DPPA2(Data Path Acceleration Architecture 2) qDMA supports channel
>> virtualization by allowing DMA
>
>typo virtualization
>
>> jobs to be enqueued into different frame queues.
>> Core can initiate a DMA transaction by preparing a frame
>> descriptor(FD) for each DMA job and enqueuing this job to a frame
>> queue. through a hardware portal. The qDMA
>              ^^^
>why this full stop?
>
>> +static struct dpaa2_qdma_comp *
>> +dpaa2_qdma_request_desc(struct dpaa2_qdma_chan *dpaa2_chan) {
>> +     struct dpaa2_qdma_comp *comp_temp = NULL;
>> +     unsigned long flags;
>> +
>> +     spin_lock_irqsave(&dpaa2_chan->queue_lock, flags);
>> +     if (list_empty(&dpaa2_chan->comp_free)) {
>> +             spin_unlock_irqrestore(&dpaa2_chan->queue_lock, flags);
>> +             comp_temp = kzalloc(sizeof(*comp_temp), GFP_NOWAIT);
>> +             if (!comp_temp)
>> +                     goto err;
>> +             comp_temp->fd_virt_addr =
>> +                     dma_pool_alloc(dpaa2_chan->fd_pool,
>GFP_NOWAIT,
>> +                                    &comp_temp->fd_bus_addr);
>> +             if (!comp_temp->fd_virt_addr)
>> +                     goto err_comp;
>> +
>> +             comp_temp->fl_virt_addr =
>> +                     dma_pool_alloc(dpaa2_chan->fl_pool,
>GFP_NOWAIT,
>> +                                    &comp_temp->fl_bus_addr);
>> +             if (!comp_temp->fl_virt_addr)
>> +                     goto err_fd_virt;
>> +
>> +             comp_temp->desc_virt_addr =
>> +                     dma_pool_alloc(dpaa2_chan->sdd_pool,
>GFP_NOWAIT,
>> +
>&comp_temp->desc_bus_addr);
>> +             if (!comp_temp->desc_virt_addr)
>> +                     goto err_fl_virt;
>> +
>> +             comp_temp->qchan = dpaa2_chan;
>> +             return comp_temp;
>> +     }
>> +
>> +     comp_temp = list_first_entry(&dpaa2_chan->comp_free,
>> +                                  struct dpaa2_qdma_comp, list);
>> +     list_del(&comp_temp->list);
>> +     spin_unlock_irqrestore(&dpaa2_chan->queue_lock, flags);
>> +
>> +     comp_temp->qchan = dpaa2_chan;
>> +
>> +     return comp_temp;
>> +
>> +err_fl_virt:
>
>no err logs? how will you know what went wrong?
>
>> +static enum
>> +dma_status dpaa2_qdma_tx_status(struct dma_chan *chan,
>> +                             dma_cookie_t cookie,
>> +                             struct dma_tx_state *txstate) {
>> +     return dma_cookie_status(chan, cookie, txstate);
>
>why not set dma_cookie_status as this callback?
>
>> +static int __cold dpaa2_qdma_setup(struct fsl_mc_device *ls_dev) {
>> +     struct dpaa2_qdma_priv_per_prio *ppriv;
>> +     struct device *dev = &ls_dev->dev;
>> +     struct dpaa2_qdma_priv *priv;
>> +     u8 prio_def = DPDMAI_PRIO_NUM;
>> +     int err = -EINVAL;
>> +     int i;
>> +
>> +     priv = dev_get_drvdata(dev);
>> +
>> +     priv->dev = dev;
>> +     priv->dpqdma_id = ls_dev->obj_desc.id;
>> +
>> +     /* Get the handle for the DPDMAI this interface is associate with */
>> +     err = dpdmai_open(priv->mc_io, 0, priv->dpqdma_id,
>&ls_dev->mc_handle);
>> +     if (err) {
>> +             dev_err(dev, "dpdmai_open() failed\n");
>> +             return err;
>> +     }
>> +     dev_info(dev, "Opened dpdmai object successfully\n");
>
>this is noise in kernel, consider debug level
>
>> +static int __cold dpaa2_dpdmai_bind(struct dpaa2_qdma_priv *priv) {
>> +     int err;
>> +     int i, num;
>> +     struct device *dev = priv->dev;
>> +     struct dpaa2_qdma_priv_per_prio *ppriv;
>> +     struct dpdmai_rx_queue_cfg rx_queue_cfg;
>> +     struct fsl_mc_device *ls_dev = to_fsl_mc_device(dev);
>
>the order is reverse than used in other fn, please stick to one style!
>--
>~Vinod

  reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13 10:13 [V4 1/2] dmaengine: fsl-dpaa2-qdma: Add the DPDMAI(Data Path DMA Interface) support Peng Ma
2019-06-13 10:13 ` [V4 2/2] dmaengine: fsl-dpaa2-qdma: Add NXP dpaa2 qDMA controller driver for Layerscape SoCs Peng Ma
2019-06-24 16:45   ` Vinod Koul
2019-09-11  2:01     ` Peng Ma [this message]
2019-06-14  1:36 ` [V4 1/2] dmaengine: fsl-dpaa2-qdma: Add the DPDMAI(Data Path DMA Interface) support Peng Ma
2019-06-24 12:25 ` Vinod Koul

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VI1PR04MB443142772665BB29B909DFF4EDB10@VI1PR04MB4431.eurprd04.prod.outlook.com \
    --to=peng.ma@nxp.com \
    --cc=dan.j.williams@intel.com \
    --cc=dmaengine@vger.kernel.org \
    --cc=leoyang.li@nxp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org dmaengine@archiver.kernel.org
	public-inbox-index dmaengine


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/ public-inbox