[PATCH v2 2/3] usb: dwc3: fix dcache flush range calculation

[neil.armstrong at linaro.org]

On 01/10/2024 17:47, Marek Vasut wrote:
> On 10/1/24 5:32 PM, Neil Armstrong wrote:
>> On 01/10/2024 17:21, Marek Vasut wrote:
>>> On 7/24/24 5:48 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 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.
>>>>
>>>> Reviewed-by: Mattijs Korpershoek <mkorpershoek at baylibre.com>
>>>> Signed-off-by: Neil Armstrong <neil.armstrong at 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);
>>> include/cpu_func.h:void flush_dcache_range(unsigned long start, unsigned long stop);
>>>
>>> So you shouldn't be feeding this function uintptr_t , right ?
>>
>> uintptr_t is the right type to perform pointer calculation,
>> I should probably cast it to unsigned long for flush_dcache_range.
> 
> Better fix flush_dcache_range(), using unsigned long there is not great, but that's for separate series from this bugfix.

Since uintptr_t is a typedef to unsigned long, it won't change anything.

> 
>> I'll do this in v2.
>>
>>>
>>> Also, why change CACHELINE_SIZE to ARCH_DMA_MINALIGN ?
>>
>> Because all the memory allocated for DMA was done with dma_alloc_coherent()
>> which uses ARCH_DMA_MINALIGN directly or indirectly.
>>
>> I should probably remove CACHELINE_SIZE in the same patch.
> And document this in the commit message, that's useful information.

Done, thx

> 
> Thanks