RE: [PATCH v2] Move DWC2 driver out of staging

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Dan Carpenter [mailto:dan.carpenter@xxxxxxxxxx]
> Sent: Monday, November 25, 2013 6:18 AM
> 
> I have reviewed the second half of the driver now.
> 
> drivers/staging/dwc2/hcd_ddma.c
>    616  static void dwc2_fill_host_dma_desc(struct dwc2_hsotg *hsotg,
>    617                                      struct dwc2_host_chan *chan,
>    618                                      struct dwc2_qtd *qtd, struct dwc2_qh *qh,
>    619                                      int n_desc)
>    620  {
>    621          struct dwc2_hcd_dma_desc *dma_desc = &qh->desc_list[n_desc];
>    622          int len = chan->xfer_len;
>    623
>    624          if (len > MAX_DMA_DESC_SIZE)
>    625                  len = MAX_DMA_DESC_SIZE - chan->max_packet + 1;
>                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> I don't understand what we are doing here.  Shouldn't the condition and
> the assignment match?

I'll take a closer look at this.

> drivers/staging/dwc2/hcd_intr.c
>    930          if (!len) {
>    931                  qtd->complete_split = 0;
>    932                  qtd->isoc_split_offset = 0;
>    933                  return 0;
>    934          }
>    935
>    936          frame_desc->actual_length += len;
>    937
>    938          if (chan->align_buf && len) {
>                                        ^^^
> This can be removed.

Right.

> In drivers/staging/dwc2/hcd_queue.c there are a coup nasty
> while(!done) loops.  The whole point of a while loop is the you put the
> end condition inside the condition part of the loop.  Saying
> "while (!done)" is crap because it looks useful but provides no actual
> information about when the loop is over.  If there are more than one
> done condition then use break statements.  In this case we are just
> iterating over an array and there is a C language construct called a
> "for loop" to express it.

Himangi Saraogi already submitted a patch to clean this up. Greg has
applied it to the staging-next branch of his staging tree. Can you
check that out and see if addresses all of your concerns?

-- 
Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux