Re: [PATCH 1/1] usb: chipidea: udc: delete td from req's td list at ep_dequeue

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

 



On Mon, Jun 30, 2014 at 09:11:11AM +0200, Andrzej Pietrasiewicz wrote:
> Hello Peter,
> 
> W dniu 30.06.2014 06:15, Peter Chen pisze:
> >We need to delete un-finished td from current request's td list
> >at ep_dequeue API, otherwise, this non-user td will be remained
> >at td list before this request is freed. So if we do ep_queue->
> >ep_dequeue->ep_queue sequence, when the complete interrupt for
> >the second ep_queue comes, we search td list for this request,
> >the first td (added by the first ep_queue) will be handled, and
> >its status is still active, so we will consider the this transfer
> >still not be completed, but in fact, it has completed. It causes
> >the peripheral side considers it never receives current data for
> >this transfer.
> >
> 
> <snip>
> 
> >+	struct td_node *node, *tmpnode;
> >
> >  	if (ep == NULL || req == NULL || hwreq->req.status != -EALREADY ||
> >  		hwep->ep.desc == NULL || list_empty(&hwreq->queue) ||
> >@@ -1331,6 +1332,13 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
> >
> >  	hw_ep_flush(hwep->ci, hwep->num, hwep->dir);
> >
> >+	list_for_each_entry_safe(node, tmpnode, &hwreq->tds, td) {
> >+		dma_pool_free(hwep->td_pool, node->ptr, node->dma);
> >+		list_del_init(&node->td);
> >+		node->ptr = NULL;
> 
> Why bother initializing node->td's prev and next members and assigning NULL
> to ptrif...
> 
> >+		kfree(node);
> 
> ... in the very next line the whole structure is freed?
> 
> I would suggest
> 
> +	list_for_each_entry_safe(node, tmpnode, &hwreq->tds, td) {
> +		dma_pool_free(hwep->td_pool, node->ptr, node->dma);
> +		list_del(&node->td);
> +		kfree(node);
> +	}

Thanks, AP.

Yes, it seems it is enough, I just copy it from the ep_free_request.
I don't know why Michael did it before, Michael, can you recall it?
Besides, I see some code use list_del_init, why we need to re-init
it after we delete it from the list.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]