Re: [PATCH] usb: dwc2: gadget: add scatter-gather mode

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

 



Hi Andrzej,

On 3/29/2019 6:05 PM, Andrzej Pietrasiewicz wrote:
> From: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> 
> NOT FOR MERGING
> 
> @Minas: can you please test with this patch instead of the original one?
> 
Tested new version. No issue seen by running USB CV (Ch9, MSC) tests.
Could you please elaborate which else tests you performed?

> @Marek: can you please verify if null pointer bug exists in this version?
> 
> This patch adds support for transferring requests, which are
> non-contiguous in physical memory, i.e. the data buffer is described by
> a scatter-list. This allows transferring large requests without relying
> on error-prone contiguous buffer allocations. This way of allocating
> requests is already implemented in functionfs and TCM USB functions and
> automatically used if UDC driver advertises scatter-gather suppport.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxx>
> [mszyprow: fixed null pointer issue, rewrote commit message]
> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> [andrzej.p: improved handling zlps]
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx>
> ---
>   drivers/usb/dwc2/gadget.c | 107 +++++++++++++++++++++++++++-----------
>   1 file changed, 76 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index e15d8a462085..e76b2e040b4c 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -768,22 +768,13 @@ static u32 dwc2_gadget_get_desc_params(struct dwc2_hsotg_ep *hs_ep, u32 *mask)
>   	return desc_size;
>   }
>   
> -/*
> - * dwc2_gadget_config_nonisoc_xfer_ddma - prepare non ISOC DMA desc chain.
> - * @hs_ep: The endpoint
> - * @dma_buff: DMA address to use
> - * @len: Length of the transfer
> - *
> - * This function will iterate over descriptor chain and fill its entries
> - * with corresponding information based on transfer data.
> - */
> -static void dwc2_gadget_config_nonisoc_xfer_ddma(struct dwc2_hsotg_ep *hs_ep,
> +static void dwc2_gadget_fill_nonisoc_xfer_ddma_one(struct dwc2_hsotg_ep *hs_ep,
> +						 struct dwc2_dma_desc **desc,
>   						 dma_addr_t dma_buff,
> -						 unsigned int len)
> +						 unsigned int len,
> +						 bool true_last)
>   {
> -	struct dwc2_hsotg *hsotg = hs_ep->parent;
>   	int dir_in = hs_ep->dir_in;
> -	struct dwc2_dma_desc *desc = hs_ep->desc_list;
>   	u32 mps = hs_ep->ep.maxpacket;
>   	u32 maxsize = 0;
>   	u32 offset = 0;
> @@ -798,41 +789,82 @@ static void dwc2_gadget_config_nonisoc_xfer_ddma(struct dwc2_hsotg_ep *hs_ep,
>   		hs_ep->desc_count = 1;
>   
>   	for (i = 0; i < hs_ep->desc_count; ++i) {
> -		desc->status = 0;
> -		desc->status |= (DEV_DMA_BUFF_STS_HBUSY
> +		(*desc)->status = 0;
> +		(*desc)->status |= (DEV_DMA_BUFF_STS_HBUSY
>   				 << DEV_DMA_BUFF_STS_SHIFT);
>   
>   		if (len > maxsize) {
>   			if (!hs_ep->index && !dir_in)
> -				desc->status |= (DEV_DMA_L | DEV_DMA_IOC);
> +				(*desc)->status |= (DEV_DMA_L | DEV_DMA_IOC);
>   
> -			desc->status |= (maxsize <<
> -						DEV_DMA_NBYTES_SHIFT & mask);
> -			desc->buf = dma_buff + offset;
> +			(*desc)->status |=
> +				maxsize << DEV_DMA_NBYTES_SHIFT & mask;
> +			(*desc)->buf = dma_buff + offset;
>   
>   			len -= maxsize;
>   			offset += maxsize;
>   		} else {
> -			desc->status |= (DEV_DMA_L | DEV_DMA_IOC);
> +			if (true_last)
> +				(*desc)->status |= (DEV_DMA_L | DEV_DMA_IOC);
>   
>   			if (dir_in)
> -				desc->status |= (len % mps) ? DEV_DMA_SHORT :
> -					((hs_ep->send_zlp) ? DEV_DMA_SHORT : 0);
> -			if (len > maxsize)
> -				dev_err(hsotg->dev, "wrong len %d\n", len);
> +				(*desc)->status |= (len % mps) ? DEV_DMA_SHORT :
> +					((hs_ep->send_zlp && true_last) ?
> +					DEV_DMA_SHORT : 0);
>   
> -			desc->status |=
> +			(*desc)->status |=
>   				len << DEV_DMA_NBYTES_SHIFT & mask;
> -			desc->buf = dma_buff + offset;
> +			(*desc)->buf = dma_buff + offset;
>   		}
>   
> -		desc->status &= ~DEV_DMA_BUFF_STS_MASK;
> -		desc->status |= (DEV_DMA_BUFF_STS_HREADY
> +		(*desc)->status &= ~DEV_DMA_BUFF_STS_MASK;
> +		(*desc)->status |= (DEV_DMA_BUFF_STS_HREADY
>   				 << DEV_DMA_BUFF_STS_SHIFT);
> -		desc++;
> +		(*desc)++;
>   	}
>   }
>   
> +/*
> + * dwc2_gadget_config_nonisoc_xfer_ddma - prepare non ISOC DMA desc chain.
> + * @hs_ep: The endpoint
> + * @ureq: Request to transfer
> + * @offset: offset in bytes
> + * @len: Length of the transfer
> + *
> + * This function will iterate over descriptor chain and fill its entries
> + * with corresponding information based on transfer data.
> + */
> +static void dwc2_gadget_config_nonisoc_xfer_ddma(struct dwc2_hsotg_ep *hs_ep,
> +						 dma_addr_t dma_buff,
> +						 unsigned int len)
> +{
> +	struct usb_request *ureq = NULL;
> +	struct dwc2_dma_desc *desc = hs_ep->desc_list;
> +	struct scatterlist *sg;
> +	int i;
> +	u8 desc_count = 0;
> +
> +	if (hs_ep->req)
> +		ureq = &hs_ep->req->req;
> +
> +	/* non-DMA sg buffer */
> +	if (!ureq || !ureq->num_sgs) {
> +		dwc2_gadget_fill_nonisoc_xfer_ddma_one(hs_ep, &desc,
> +			dma_buff, len, true);
> +		return;
> +	}
> +
> +	/* DMA sg buffer */
> +	for_each_sg(ureq->sg, sg, ureq->num_sgs, i) {
> +		dwc2_gadget_fill_nonisoc_xfer_ddma_one(hs_ep, &desc,
> +			sg_dma_address(sg) + sg->offset, sg_dma_len(sg),
> +			sg_is_last(sg));
> +		desc_count += hs_ep->desc_count;
> +	}
> +
> +	hs_ep->desc_count = desc_count;
> +}
> +
>   /*
>    * dwc2_gadget_fill_isoc_desc - fills next isochronous descriptor in chain.
>    * @hs_ep: The isochronous endpoint.
> @@ -944,7 +976,13 @@ static void dwc2_gadget_start_isoc_ddma(struct dwc2_hsotg_ep *hs_ep)
>   
>   	hs_ep->next_desc = 0;
>   	list_for_each_entry_safe(hs_req, treq, &hs_ep->queue, queue) {
> -		ret = dwc2_gadget_fill_isoc_desc(hs_ep, hs_req->req.dma,
> +		dma_addr_t dma_addr = hs_req->req.dma;
> +
> +		if (hs_req->req.num_sgs) {
> +			WARN_ON(hs_req->req.num_sgs > 1);
> +			dma_addr = sg_dma_address(hs_req->req.sg);
> +		}
> +		ret = dwc2_gadget_fill_isoc_desc(hs_ep, dma_addr,
>   						 hs_req->req.length);
>   		if (ret)
>   			break;
> @@ -1399,7 +1437,13 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct usb_request *req,
>   	 */
>   	if (using_desc_dma(hs) && hs_ep->isochronous) {
>   		if (hs_ep->target_frame != TARGET_FRAME_INITIAL) {
> -			dwc2_gadget_fill_isoc_desc(hs_ep, hs_req->req.dma,
> +			dma_addr_t dma_addr = hs_req->req.dma;
> +
> +			if (hs_req->req.num_sgs) {
> +				WARN_ON(hs_req->req.num_sgs > 1);
> +				dma_addr = sg_dma_address(hs_req->req.sg);
> +			}
> +			dwc2_gadget_fill_isoc_desc(hs_ep, dma_addr,
>   						   hs_req->req.length);
>   		}
>   		return 0;
> @@ -4386,6 +4430,7 @@ static int dwc2_hsotg_udc_start(struct usb_gadget *gadget,
>   	hsotg->enabled = 0;
>   	spin_unlock_irqrestore(&hsotg->lock, flags);
>   
> +	gadget->sg_supported = using_desc_dma(hsotg);
>   	dev_info(hsotg->dev, "bound driver %s\n", driver->driver.name);
>   
>   	return 0;
> 





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

  Powered by Linux