public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Chen <peter.chen@kernel.org>
To: Xu Yang <xu.yang_2@nxp.com>
Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
	imx@lists.linux.dev, jun.li@nxp.com
Subject: Re: [PATCH 2/2] usb: chipidea: udc: create bounce buffer for problem sglist entries if possible
Date: Fri, 13 Sep 2024 09:34:46 +0800	[thread overview]
Message-ID: <20240913013446.GB320526@nchen-desktop> (raw)
In-Reply-To: <20240912045150.915573-2-xu.yang_2@nxp.com>

On 24-09-12 12:51:50, Xu Yang wrote:
> The chipidea controller doesn't fully support sglist, such as it can not
> transfer data spanned more dTDs to form a bus packet, so it can only work
> on very limited cases.
> 
> The limitations as below:
> 1. the end address of the first sg buffer must be 4KB aligned.
> 2. the start and end address of the middle sg buffer must be 4KB aligned.
> 3. the start address of the first sg buffer must be 4KB aligned.
> 
> However, not all the use cases violate these limitations. To make the
> controller compatible with most of the cases, this will try to bounce the
> problem sglist entries which can be found by sglist_get_invalid_entry().
> Then a bounced line buffer (the size will roundup to page size) will be
> allocated to replace the remaining problem sg entries. The data will be
> copied between problem sg entries and bounce buffer according to the
> transfer direction. The bounce buffer will be freed when the request
> completed.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.com>

> ---
>  drivers/usb/chipidea/udc.c | 147 +++++++++++++++++++++++++++++++++++++
>  drivers/usb/chipidea/udc.h |   2 +
>  2 files changed, 149 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 5d9369d01065..dc4dae2c31b4 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -10,6 +10,7 @@
>  #include <linux/delay.h>
>  #include <linux/device.h>
>  #include <linux/dmapool.h>
> +#include <linux/dma-direct.h>
>  #include <linux/err.h>
>  #include <linux/irqreturn.h>
>  #include <linux/kernel.h>
> @@ -540,6 +541,126 @@ static int prepare_td_for_sg(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  	return ret;
>  }
>  
> +/*
> + * Verify if the scatterlist is valid by iterating each sg entry.
> + * Return invalid sg entry index which is less than num_sgs.
> + */
> +static int sglist_get_invalid_entry(struct device *dma_dev, u8 dir,
> +			struct usb_request *req)
> +{
> +	int i;
> +	struct scatterlist *s = req->sg;
> +
> +	if (req->num_sgs == 1)
> +		return 1;
> +
> +	dir = dir ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
> +
> +	for (i = 0; i < req->num_sgs; i++, s = sg_next(s)) {
> +		/* Only small sg (generally last sg) may be bounced. If
> +		 * that happens. we can't ensure the addr is page-aligned
> +		 * after dma map.
> +		 */
> +		if (dma_kmalloc_needs_bounce(dma_dev, s->length, dir))
> +			break;
> +
> +		/* Make sure each sg start address (except first sg) is
> +		 * page-aligned and end address (except last sg) is also
> +		 * page-aligned.
> +		 */
> +		if (i == 0) {
> +			if (!IS_ALIGNED(s->offset + s->length,
> +						CI_HDRC_PAGE_SIZE))
> +				break;
> +		} else {
> +			if (s->offset)
> +				break;
> +			if (!sg_is_last(s) && !IS_ALIGNED(s->length,
> +						CI_HDRC_PAGE_SIZE))
> +				break;
> +		}
> +	}
> +
> +	return i;
> +}
> +
> +static int sglist_do_bounce(struct ci_hw_req *hwreq, int index,
> +			bool copy, unsigned int *bounced)
> +{
> +	void *buf;
> +	int i, ret, nents, num_sgs;
> +	unsigned int rest, rounded;
> +	struct scatterlist *sg, *src, *dst;
> +
> +	nents = index + 1;
> +	ret = sg_alloc_table(&hwreq->sgt, nents, GFP_KERNEL);
> +	if (ret)
> +		return ret;
> +
> +	sg = src = hwreq->req.sg;
> +	num_sgs = hwreq->req.num_sgs;
> +	rest = hwreq->req.length;
> +	dst = hwreq->sgt.sgl;
> +
> +	for (i = 0; i < index; i++) {
> +		memcpy(dst, src, sizeof(*src));
> +		rest -= src->length;
> +		src = sg_next(src);
> +		dst = sg_next(dst);
> +	}
> +
> +	/* create one bounce buffer */
> +	rounded = round_up(rest, CI_HDRC_PAGE_SIZE);
> +	buf = kmalloc(rounded, GFP_KERNEL);
> +	if (!buf) {
> +		sg_free_table(&hwreq->sgt);
> +		return -ENOMEM;
> +	}
> +
> +	sg_set_buf(dst, buf, rounded);
> +
> +	hwreq->req.sg = hwreq->sgt.sgl;
> +	hwreq->req.num_sgs = nents;
> +	hwreq->sgt.sgl = sg;
> +	hwreq->sgt.nents = num_sgs;
> +
> +	if (copy)
> +		sg_copy_to_buffer(src, num_sgs - index, buf, rest);
> +
> +	*bounced = rest;
> +
> +	return 0;
> +}
> +
> +static void sglist_do_debounce(struct ci_hw_req *hwreq, bool copy)
> +{
> +	void *buf;
> +	int i, nents, num_sgs;
> +	struct scatterlist *sg, *src, *dst;
> +
> +	sg = hwreq->req.sg;
> +	num_sgs = hwreq->req.num_sgs;
> +	src = sg_last(sg, num_sgs);
> +	buf = sg_virt(src);
> +
> +	if (copy) {
> +		dst = hwreq->sgt.sgl;
> +		for (i = 0; i < num_sgs - 1; i++)
> +			dst = sg_next(dst);
> +
> +		nents = hwreq->sgt.nents - num_sgs + 1;
> +		sg_copy_from_buffer(dst, nents, buf, sg_dma_len(src));
> +	}
> +
> +	hwreq->req.sg = hwreq->sgt.sgl;
> +	hwreq->req.num_sgs = hwreq->sgt.nents;
> +	hwreq->sgt.sgl = sg;
> +	hwreq->sgt.nents = num_sgs;
> +
> +	kfree(buf);
> +	sg_free_table(&hwreq->sgt);
> +}
> +
>  /**
>   * _hardware_enqueue: configures a request at hardware level
>   * @hwep:   endpoint
> @@ -552,6 +673,8 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  	struct ci_hdrc *ci = hwep->ci;
>  	int ret = 0;
>  	struct td_node *firstnode, *lastnode;
> +	unsigned int bounced_size;
> +	struct scatterlist *sg;
>  
>  	/* don't queue twice */
>  	if (hwreq->req.status == -EALREADY)
> @@ -559,11 +682,28 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  
>  	hwreq->req.status = -EALREADY;
>  
> +	if (hwreq->req.num_sgs && hwreq->req.length) {
> +		ret = sglist_get_invalid_entry(ci->dev->parent, hwep->dir,
> +					&hwreq->req);
> +		if (ret < hwreq->req.num_sgs) {
> +			ret = sglist_do_bounce(hwreq, ret, hwep->dir == TX,
> +					&bounced_size);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
>  	ret = usb_gadget_map_request_by_dev(ci->dev->parent,
>  					    &hwreq->req, hwep->dir);
>  	if (ret)
>  		return ret;
>  
> +	if (hwreq->sgt.sgl) {
> +		/* We've mapped a bigger buffer, now recover the actual size */
> +		sg = sg_last(hwreq->req.sg, hwreq->req.num_sgs);
> +		sg_dma_len(sg) = min(sg_dma_len(sg), bounced_size);
> +	}
> +
>  	if (hwreq->req.num_mapped_sgs)
>  		ret = prepare_td_for_sg(hwep, hwreq);
>  	else
> @@ -742,6 +882,10 @@ static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
>  	usb_gadget_unmap_request_by_dev(hwep->ci->dev->parent,
>  					&hwreq->req, hwep->dir);
>  
> +	/* sglist bounced */
> +	if (hwreq->sgt.sgl)
> +		sglist_do_debounce(hwreq, hwep->dir == RX);
> +
>  	hwreq->req.actual += actual;
>  
>  	if (hwreq->req.status)
> @@ -1588,6 +1732,9 @@ static int ep_dequeue(struct usb_ep *ep, struct usb_request *req)
>  
>  	usb_gadget_unmap_request(&hwep->ci->gadget, req, hwep->dir);
>  
> +	if (hwreq->sgt.sgl)
> +		sglist_do_debounce(hwreq, false);
> +
>  	req->status = -ECONNRESET;
>  
>  	if (hwreq->req.complete != NULL) {
> diff --git a/drivers/usb/chipidea/udc.h b/drivers/usb/chipidea/udc.h
> index 5193df1e18c7..c8a47389a46b 100644
> --- a/drivers/usb/chipidea/udc.h
> +++ b/drivers/usb/chipidea/udc.h
> @@ -69,11 +69,13 @@ struct td_node {
>   * @req: request structure for gadget drivers
>   * @queue: link to QH list
>   * @tds: link to TD list
> + * @sgt: hold original sglist when bounce sglist
>   */
>  struct ci_hw_req {
>  	struct usb_request	req;
>  	struct list_head	queue;
>  	struct list_head	tds;
> +	struct sg_table		sgt;
>  };
>  
>  #ifdef CONFIG_USB_CHIPIDEA_UDC
> -- 
> 2.34.1
> 

  reply	other threads:[~2024-09-13  1:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-12  4:51 [PATCH 1/2] usb: chipidea: udc: limit usb request length to max 16KB Xu Yang
2024-09-12  4:51 ` [PATCH 2/2] usb: chipidea: udc: create bounce buffer for problem sglist entries if possible Xu Yang
2024-09-13  1:34   ` Peter Chen [this message]
2024-10-09 14:47   ` Dan Carpenter
2024-09-13  1:20 ` [PATCH 1/2] usb: chipidea: udc: limit usb request length to max 16KB Peter Chen
2024-09-13  7:11   ` Xu Yang
2024-09-13  9:53     ` Peter Chen
2024-09-13 15:25       ` Xu Yang
2024-09-14  2:04         ` Peter Chen
2024-09-20  4:50           ` Xu Yang

Reply instructions:

You may reply publicly 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=20240913013446.GB320526@nchen-desktop \
    --to=peter.chen@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=imx@lists.linux.dev \
    --cc=jun.li@nxp.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=xu.yang_2@nxp.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox