On 28/12/2024 14:46, Ahmad Fatoum wrote:
Hi,
On 11.10.24 16:38, Neil Armstrong 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-58434ab4b...@linaro.org/
Reviewed-by: Mattijs Korpershoek <mkorpersh...@baylibre.com>
Signed-off-by: Neil Armstrong <neil.armstr...@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 8ba5fcd5312..531f0b522af 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -742,6 +742,8 @@ static void dwc3_ep0_inspect_setup(struct dwc3 *dwc,
if (!dwc->gadget_driver)
goto out;
+ dwc3_invalidate_cache((uintptr_t)ctrl, sizeof(*ctrl));
I believe this to be incorrect. ctrl here is dwc->ctrl_req, which AFAICS
is allocated in dwc3_gadget_init() using dma_alloc_coherent.
Doing cache maintenance on DMA-coherent memory doesn't make sense.
If this makes things better for you, it's probable that something else
is broken.
Additionally, this will break platforms that have DMA-coherent DWC3
and allocate normal memory instead of uncached memory. I am not sure
if there are such U-Boot platforms yet, as searching for dma-coherent DT
property yields no relevant results, but e.g. LS1046A in barebox would've
been broken by such a change.
dma_alloc_coherent() in U-boot allocates "aligned on cacheline" cached memory,
the function reuses the Linux naming but doesn't actually allocate
uncached/coherent memory.
Neil
Cheers,
Ahmad
+
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 19c3a5f5e58..e5a383407a2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2534,6 +2534,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 0ede323671b..c1ab0288142 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((unsigned long)start_addr, (unsigned long)end_addr);
}
+
+static inline void dwc3_invalidate_cache(uintptr_t addr, int length)
+{
+ uintptr_t start_addr = (uintptr_t)addr & ~(CACHELINE_SIZE - 1);
+ uintptr_t end_addr = ALIGN((uintptr_t)addr + length, CACHELINE_SIZE);
+
+ invalidate_dcache_range((unsigned long)start_addr, (unsigned
long)end_addr);
+}
#endif /* __DRIVERS_USB_DWC3_IO_H */
