[PATCH 1/3] usb: dwc3: allocate setup_buf with dma_alloc_coherent()

[Neil Armstrong]

On 24/07/2024 17:03, Mattijs Korpershoek wrote:
> Hi Neil,
> 
> Thank you for the patch.
> 
> On ven., juil. 19, 2024 at 15:56, Neil Armstrong <neil.armstrong at linaro.org> wrote:
> 
>> Also allocate the setup_buf with dma_alloc_coherent() since it's
> 
> The subject of the patch says:
> 
> "usb: dwc3: allocate setup_buf with dma_alloc_coherent()"
> 
> Isn't this line just repeating the title?
> 
>> also consumed by the hardware DMA.

Yeah it's a verbose rewrite of the subject, I'll rewrite it to be less bad!

thanks
Neil

>>
>> Signed-off-by: Neil Armstrong <neil.armstrong at linaro.org>
>> ---
>>   drivers/usb/dwc3/core.h   | 2 ++
>>   drivers/usb/dwc3/ep0.c    | 4 ++--
>>   drivers/usb/dwc3/gadget.c | 8 ++++----
>>   3 files changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 7374ce950da..ce35460c405 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -670,6 +670,7 @@ struct dwc3_scratchpad_array {
>>    * @ep0_trb: dma address of ep0_trb
>>    * @ep0_usb_req: dummy req used while handling STD USB requests
>>    * @ep0_bounce_addr: dma address of ep0_bounce
>> + * @setup_buf_addr: dma address if setup_buf
> 
> if -> of
> 
> Both remarks being minor, please add:
> 
> Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
> 
>>    * @scratch_addr: dma address of scratchbuf
>>    * @lock: for synchronizing
>>    * @dev: pointer to our struct device
>> @@ -757,6 +758,7 @@ struct dwc3 {
>>   	dma_addr_t		ep0_trb_addr;
>>   	dma_addr_t		ep0_bounce_addr;
>>   	dma_addr_t		scratch_addr;
>> +	dma_addr_t		setup_buf_addr;
>>   	struct dwc3_request	ep0_usb_req;
>>   
>>   	/* device lock */
>> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
>> index 16b11ce3d9f..0c7e0123368 100644
>> --- a/drivers/usb/dwc3/ep0.c
>> +++ b/drivers/usb/dwc3/ep0.c
>> @@ -381,7 +381,7 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
>>   	dep = dwc->eps[0];
>>   	dwc->ep0_usb_req.dep = dep;
>>   	dwc->ep0_usb_req.request.length = sizeof(*response_pkt);
>> -	dwc->ep0_usb_req.request.buf = dwc->setup_buf;
>> +	dwc->ep0_usb_req.request.buf = (void *)dwc->setup_buf_addr;
>>   	dwc->ep0_usb_req.request.complete = dwc3_ep0_status_cmpl;
>>   
>>   	return __dwc3_gadget_ep0_queue(dep, &dwc->ep0_usb_req);
>> @@ -663,7 +663,7 @@ static int dwc3_ep0_set_sel(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
>>   	dep = dwc->eps[0];
>>   	dwc->ep0_usb_req.dep = dep;
>>   	dwc->ep0_usb_req.request.length = dep->endpoint.maxpacket;
>> -	dwc->ep0_usb_req.request.buf = dwc->setup_buf;
>> +	dwc->ep0_usb_req.request.buf = (void *)dwc->setup_buf_addr;
>>   	dwc->ep0_usb_req.request.complete = dwc3_ep0_set_sel_cmpl;
>>   
>>   	return __dwc3_gadget_ep0_queue(dep, &dwc->ep0_usb_req);
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 7d6bcc2627f..d41b590afb8 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -2622,8 +2622,8 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>>   		goto err1;
>>   	}
>>   
>> -	dwc->setup_buf = memalign(CONFIG_SYS_CACHELINE_SIZE,
>> -				  DWC3_EP0_BOUNCE_SIZE);
>> +	dwc->setup_buf = dma_alloc_coherent(DWC3_EP0_BOUNCE_SIZE,
>> +					(unsigned long *)&dwc->setup_buf_addr);
>>   	if (!dwc->setup_buf) {
>>   		ret = -ENOMEM;
>>   		goto err2;
>> @@ -2670,7 +2670,7 @@ err4:
>>   	dma_free_coherent(dwc->ep0_bounce);
>>   
>>   err3:
>> -	kfree(dwc->setup_buf);
>> +	dma_free_coherent(dwc->setup_buf);
>>   
>>   err2:
>>   	dma_free_coherent(dwc->ep0_trb);
>> @@ -2692,7 +2692,7 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
>>   
>>   	dma_free_coherent(dwc->ep0_bounce);
>>   
>> -	kfree(dwc->setup_buf);
>> +	dma_free_coherent(dwc->setup_buf);
>>   
>>   	dma_free_coherent(dwc->ep0_trb);
>>   
>>
>> -- 
>> 2.34.1