* [PATCH 1/3] usb: dwc3: allocate setup_buf with dma_alloc_coherent()
2024-07-19 13:56 [PATCH 0/3] dwc3: gadget: properly fix cache operations Neil Armstrong
@ 2024-07-19 13:56 ` Neil Armstrong
2024-07-24 15:03 ` Mattijs Korpershoek
2024-07-19 13:56 ` [PATCH 2/3] usb: dwc3: fix dcache flush range calculation Neil Armstrong
2024-07-19 13:56 ` [PATCH 3/3] usb: dwc3: invalidate dcache on buffer used in interrupt handling Neil Armstrong
2 siblings, 1 reply; 8+ messages in thread
From: Neil Armstrong @ 2024-07-19 13:56 UTC (permalink / raw)
To: Marek Vasut, Tom Rini, Lukasz Majewski, Mattijs Korpershoek
Cc: Caleb Connolly, u-boot-qcom, u-boot, Neil Armstrong
Also allocate the setup_buf with dma_alloc_coherent() since it's
also consumed by the hardware DMA.
Signed-off-by: Neil Armstrong <neil.armstrong@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
* @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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] usb: dwc3: allocate setup_buf with dma_alloc_coherent()
2024-07-19 13:56 ` [PATCH 1/3] usb: dwc3: allocate setup_buf with dma_alloc_coherent() Neil Armstrong
@ 2024-07-24 15:03 ` Mattijs Korpershoek
2024-07-24 15:40 ` Neil Armstrong
0 siblings, 1 reply; 8+ messages in thread
From: Mattijs Korpershoek @ 2024-07-24 15:03 UTC (permalink / raw)
To: Neil Armstrong, Marek Vasut, Tom Rini, Lukasz Majewski
Cc: Caleb Connolly, u-boot-qcom, u-boot, Neil Armstrong
Hi Neil,
Thank you for the patch.
On ven., juil. 19, 2024 at 15:56, Neil Armstrong <neil.armstrong@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.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@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@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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/3] usb: dwc3: allocate setup_buf with dma_alloc_coherent()
2024-07-24 15:03 ` Mattijs Korpershoek
@ 2024-07-24 15:40 ` Neil Armstrong
0 siblings, 0 replies; 8+ messages in thread
From: Neil Armstrong @ 2024-07-24 15:40 UTC (permalink / raw)
To: Mattijs Korpershoek, Marek Vasut, Tom Rini, Lukasz Majewski
Cc: Caleb Connolly, u-boot-qcom, u-boot
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@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@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@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] usb: dwc3: fix dcache flush range calculation
2024-07-19 13:56 [PATCH 0/3] dwc3: gadget: properly fix cache operations Neil Armstrong
2024-07-19 13:56 ` [PATCH 1/3] usb: dwc3: allocate setup_buf with dma_alloc_coherent() Neil Armstrong
@ 2024-07-19 13:56 ` Neil Armstrong
2024-07-24 15:19 ` Mattijs Korpershoek
2024-07-19 13:56 ` [PATCH 3/3] usb: dwc3: invalidate dcache on buffer used in interrupt handling Neil Armstrong
2 siblings, 1 reply; 8+ messages in thread
From: Neil Armstrong @ 2024-07-19 13:56 UTC (permalink / raw)
To: Marek Vasut, Tom Rini, Lukasz Majewski, Mattijs Korpershoek
Cc: Caleb Connolly, u-boot-qcom, u-boot, Neil Armstrong
The current flush operation will omit doing a flush/invalidate on
the first and last bytes if the base address and size are not aligned
with DMA_MINALIGN.
This causes operation failures Qualcomm platforms.
Take in account the alignment and size of the buffer and also
flush the previous and last cacheline.
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/usb/dwc3/io.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
index 04791d4c9be..1aaf5413c6d 100644
--- a/drivers/usb/dwc3/io.h
+++ b/drivers/usb/dwc3/io.h
@@ -50,6 +50,9 @@ static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value)
static inline void dwc3_flush_cache(uintptr_t addr, int length)
{
- flush_dcache_range(addr, addr + ROUND(length, CACHELINE_SIZE));
+ uintptr_t start_addr = (uintptr_t)addr & ~(ARCH_DMA_MINALIGN - 1);
+ uintptr_t end_addr = ALIGN((uintptr_t)addr + length, ARCH_DMA_MINALIGN);
+
+ flush_dcache_range(start_addr, end_addr);
}
#endif /* __DRIVERS_USB_DWC3_IO_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/3] usb: dwc3: fix dcache flush range calculation
2024-07-19 13:56 ` [PATCH 2/3] usb: dwc3: fix dcache flush range calculation Neil Armstrong
@ 2024-07-24 15:19 ` Mattijs Korpershoek
0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2024-07-24 15:19 UTC (permalink / raw)
To: Neil Armstrong, Marek Vasut, Tom Rini, Lukasz Majewski
Cc: Caleb Connolly, u-boot-qcom, u-boot, Neil Armstrong
Hi Neil,
Thank you for the patch.
On ven., juil. 19, 2024 at 15:56, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> The current flush operation will omit doing a flush/invalidate on
> the first and last bytes if the base address and size are not aligned
> with DMA_MINALIGN.
>
> This causes operation failures Qualcomm platforms.
>
> Take in account the alignment and size of the buffer and also
> flush the previous and last cacheline.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> drivers/usb/dwc3/io.h | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
> index 04791d4c9be..1aaf5413c6d 100644
> --- a/drivers/usb/dwc3/io.h
> +++ b/drivers/usb/dwc3/io.h
> @@ -50,6 +50,9 @@ static inline void dwc3_writel(void __iomem *base, u32 offset, u32 value)
>
> static inline void dwc3_flush_cache(uintptr_t addr, int length)
> {
> - flush_dcache_range(addr, addr + ROUND(length, CACHELINE_SIZE));
> + uintptr_t start_addr = (uintptr_t)addr & ~(ARCH_DMA_MINALIGN - 1);
> + uintptr_t end_addr = ALIGN((uintptr_t)addr + length, ARCH_DMA_MINALIGN);
> +
> + flush_dcache_range(start_addr, end_addr);
> }
> #endif /* __DRIVERS_USB_DWC3_IO_H */
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] usb: dwc3: invalidate dcache on buffer used in interrupt handling
2024-07-19 13:56 [PATCH 0/3] dwc3: gadget: properly fix cache operations Neil Armstrong
2024-07-19 13:56 ` [PATCH 1/3] usb: dwc3: allocate setup_buf with dma_alloc_coherent() Neil Armstrong
2024-07-19 13:56 ` [PATCH 2/3] usb: dwc3: fix dcache flush range calculation Neil Armstrong
@ 2024-07-19 13:56 ` Neil Armstrong
2024-07-24 15:20 ` Mattijs Korpershoek
2 siblings, 1 reply; 8+ messages in thread
From: Neil Armstrong @ 2024-07-19 13:56 UTC (permalink / raw)
To: Marek Vasut, Tom Rini, Lukasz Majewski, Mattijs Korpershoek
Cc: Caleb Connolly, u-boot-qcom, u-boot, Neil Armstrong
On Qualcomm systems, the setup buffer and even buffers are in
a bad state at interrupt handling, so invalidate the dcache lines
for the setup_buf and event buffer to make sure we read correct
data written by the hardware.
This fixes the following error:
dwc3-generic-peripheral usb@a600000: UNKNOWN IRQ type -1
dwc3-generic-peripheral usb@a600000: UNKNOWN IRQ type 4673109
and invalid situation in dwc3_gadget_giveback() because setup_buf content
is read at 0s and leads to fatal crash fixed by [1].
[1] https://lore.kernel.org/all/20240528-topic-sm8x50-dwc3-gadget-crash-fix-v1-1-58434ab4b3d3@linaro.org/
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/usb/dwc3/ep0.c | 2 ++
drivers/usb/dwc3/gadget.c | 2 ++
drivers/usb/dwc3/io.h | 8 ++++++++
3 files changed, 12 insertions(+)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 0c7e0123368..fc1d5892106 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -743,6 +743,8 @@ static void dwc3_ep0_inspect_setup(struct dwc3 *dwc,
if (!dwc->gadget_driver)
goto out;
+ dwc3_invalidate_cache(ctrl, sizeof(*ctrl));
+
len = le16_to_cpu(ctrl->wLength);
if (!len) {
dwc->three_stage_setup = false;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index d41b590afb8..0bc9aee4daa 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2503,6 +2503,8 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3 *dwc, u32 buf)
while (left > 0) {
union dwc3_event event;
+ dwc3_invalidate_cache((uintptr_t)evt->buf, evt->length);
+
event.raw = *(u32 *) (evt->buf + evt->lpos);
dwc3_process_event_entry(dwc, &event);
diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
index 1aaf5413c6d..7cf05203b0d 100644
--- a/drivers/usb/dwc3/io.h
+++ b/drivers/usb/dwc3/io.h
@@ -55,4 +55,12 @@ static inline void dwc3_flush_cache(uintptr_t addr, int length)
flush_dcache_range(start_addr, end_addr);
}
+
+static inline void dwc3_invalidate_cache(uintptr_t addr, int length)
+{
+ uintptr_t start_addr = (uintptr_t)addr & ~(ARCH_DMA_MINALIGN - 1);
+ uintptr_t end_addr = ALIGN((uintptr_t)addr + length, ARCH_DMA_MINALIGN);
+
+ invalidate_dcache_range(start_addr, end_addr);
+}
#endif /* __DRIVERS_USB_DWC3_IO_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] usb: dwc3: invalidate dcache on buffer used in interrupt handling
2024-07-19 13:56 ` [PATCH 3/3] usb: dwc3: invalidate dcache on buffer used in interrupt handling Neil Armstrong
@ 2024-07-24 15:20 ` Mattijs Korpershoek
0 siblings, 0 replies; 8+ messages in thread
From: Mattijs Korpershoek @ 2024-07-24 15:20 UTC (permalink / raw)
To: Neil Armstrong, Marek Vasut, Tom Rini, Lukasz Majewski
Cc: Caleb Connolly, u-boot-qcom, u-boot, Neil Armstrong
Hi Neil,
Thank you for the patch.
On ven., juil. 19, 2024 at 15:56, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> On Qualcomm systems, the setup buffer and even buffers are in
> a bad state at interrupt handling, so invalidate the dcache lines
> for the setup_buf and event buffer to make sure we read correct
> data written by the hardware.
>
> This fixes the following error:
> dwc3-generic-peripheral usb@a600000: UNKNOWN IRQ type -1
> dwc3-generic-peripheral usb@a600000: UNKNOWN IRQ type 4673109
>
> and invalid situation in dwc3_gadget_giveback() because setup_buf content
> is read at 0s and leads to fatal crash fixed by [1].
>
> [1] https://lore.kernel.org/all/20240528-topic-sm8x50-dwc3-gadget-crash-fix-v1-1-58434ab4b3d3@linaro.org/
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> ---
> drivers/usb/dwc3/ep0.c | 2 ++
> drivers/usb/dwc3/gadget.c | 2 ++
> drivers/usb/dwc3/io.h | 8 ++++++++
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
> index 0c7e0123368..fc1d5892106 100644
> --- a/drivers/usb/dwc3/ep0.c
> +++ b/drivers/usb/dwc3/ep0.c
> @@ -743,6 +743,8 @@ static void dwc3_ep0_inspect_setup(struct dwc3 *dwc,
> if (!dwc->gadget_driver)
> goto out;
>
> + dwc3_invalidate_cache(ctrl, sizeof(*ctrl));
> +
> len = le16_to_cpu(ctrl->wLength);
> if (!len) {
> dwc->three_stage_setup = false;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index d41b590afb8..0bc9aee4daa 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2503,6 +2503,8 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3 *dwc, u32 buf)
> while (left > 0) {
> union dwc3_event event;
>
> + dwc3_invalidate_cache((uintptr_t)evt->buf, evt->length);
> +
> event.raw = *(u32 *) (evt->buf + evt->lpos);
>
> dwc3_process_event_entry(dwc, &event);
> diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h
> index 1aaf5413c6d..7cf05203b0d 100644
> --- a/drivers/usb/dwc3/io.h
> +++ b/drivers/usb/dwc3/io.h
> @@ -55,4 +55,12 @@ static inline void dwc3_flush_cache(uintptr_t addr, int length)
>
> flush_dcache_range(start_addr, end_addr);
> }
> +
> +static inline void dwc3_invalidate_cache(uintptr_t addr, int length)
> +{
> + uintptr_t start_addr = (uintptr_t)addr & ~(ARCH_DMA_MINALIGN - 1);
> + uintptr_t end_addr = ALIGN((uintptr_t)addr + length, ARCH_DMA_MINALIGN);
> +
> + invalidate_dcache_range(start_addr, end_addr);
> +}
> #endif /* __DRIVERS_USB_DWC3_IO_H */
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread