Re: [PATCH v2 3/3] usb: chipidea: udc: create bounce buffer for problem sglist entries if possible

Kees Bakker
2024-10-08T20:27:18Z

Op 23-09-2024 om 10:12 schreef Xu Yang:
> 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.
>
> Acked-by: Peter Chen <peter.chen@kernel.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v2:
> - judge ci->has_short_pkt_limit
> - add Ack-by tag
> ---
> drivers/usb/chipidea/udc.c | 148 +++++++++++++++++++++++++++++++++++++
> drivers/usb/chipidea/udc.h | 2 +
> 2 files changed, 150 insertions(+)
> [...]
> @@ -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,29 @@ 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 &&
> + ci->has_short_pkt_limit) {
> + ret = sglist_get_invalid_entry(ci->dev->parent, hwep->dir,
> + &hwreq->req);
> + if (ret < hwreq->req.num_sgs) {
bounced_size is only initialized here
> + 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);
bounced_size can be uninitialized at this point, if the earlier if
condition is false.
> + sg_dma_len(sg) = min(sg_dma_len(sg), bounced_size);
> + }
> +
>
--
Kees