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 via email to