| Message ID | 20241011-u-boot-dwc3-gadget-dcache-fixup-v4-2-5f3498d8035b@linaro.org |
|---|---|
| State | Accepted |
| Commit | 502a50ab1f7e32e3e90056597e8ce6a0931789ba |
| Headers | show |
| Series | dwc3: gadget: properly fix cache operations | expand |
On 10/11/24 4:38 PM, Neil Armstrong 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 CACHELINE_SIZE. > > This causes operation failures Qualcomm platforms. > > Take in account the alignment and size of the buffer and also > flush the previous and last cacheline. > > Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> > 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..0ede323671b 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 & ~(CACHELINE_SIZE - 1); > + uintptr_t end_addr = ALIGN((uintptr_t)addr + length, CACHELINE_SIZE); > + > + flush_dcache_range((unsigned long)start_addr, (unsigned long)end_addr); > } > #endif /* __DRIVERS_USB_DWC3_IO_H */ You likely want a max(CONFIG_SYS_CACHELINE_SIZE, dwc3-buffer-alignment-requirement) to really correctly align the buffer.
Hi, Le 12/10/2024 à 05:37, Marek Vasut a écrit : > On 10/11/24 4:38 PM, Neil Armstrong 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 CACHELINE_SIZE. >> >> This causes operation failures Qualcomm platforms. >> >> Take in account the alignment and size of the buffer and also >> flush the previous and last cacheline. >> >> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >> 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..0ede323671b 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 & ~(CACHELINE_SIZE - 1); >> + uintptr_t end_addr = ALIGN((uintptr_t)addr + length, CACHELINE_SIZE); >> + >> + flush_dcache_range((unsigned long)start_addr, (unsigned long)end_addr); >> } >> #endif /* __DRIVERS_USB_DWC3_IO_H */ > > You likely want a max(CONFIG_SYS_CACHELINE_SIZE, dwc3-buffer-alignment-requirement) to really correctly align the buffer. No this change is related to the system cacheline to properly flush/invalidate the data to the system memory, the dwc3 buffer alignment (if there's one) should be handled when allocating the memory, but here we're using dma_alloc_coherent() which use DMA_MINALIGN. So it's another unrelated story. Neil
On 10/13/24 6:35 PM, Neil Armstrong wrote: > Hi, > > Le 12/10/2024 à 05:37, Marek Vasut a écrit : >> On 10/11/24 4:38 PM, Neil Armstrong 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 CACHELINE_SIZE. >>> >>> This causes operation failures Qualcomm platforms. >>> >>> Take in account the alignment and size of the buffer and also >>> flush the previous and last cacheline. >>> >>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> >>> 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..0ede323671b 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 & ~(CACHELINE_SIZE - 1); >>> + uintptr_t end_addr = ALIGN((uintptr_t)addr + length, >>> CACHELINE_SIZE); >>> + >>> + flush_dcache_range((unsigned long)start_addr, (unsigned >>> long)end_addr); >>> } >>> #endif /* __DRIVERS_USB_DWC3_IO_H */ >> >> You likely want a max(CONFIG_SYS_CACHELINE_SIZE, dwc3-buffer- >> alignment-requirement) to really correctly align the buffer. > > No this change is related to the system cacheline to properly flush/ > invalidate the data > to the system memory, the dwc3 buffer alignment (if there's one) should > be handled > when allocating the memory, but here we're using dma_alloc_coherent() which > use DMA_MINALIGN. OK Reviewed-by: Marek Vasut <marex@denx.de>
diff --git a/drivers/usb/dwc3/io.h b/drivers/usb/dwc3/io.h index 04791d4c9be..0ede323671b 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 & ~(CACHELINE_SIZE - 1); + uintptr_t end_addr = ALIGN((uintptr_t)addr + length, CACHELINE_SIZE); + + flush_dcache_range((unsigned long)start_addr, (unsigned long)end_addr); } #endif /* __DRIVERS_USB_DWC3_IO_H */