Hello Yann,
On 03.03.21 10:33, Yann Sionneau wrote:
> On 03/03/2021 10:14, Lucas Stach wrote:
>>> Right now we aren't using any driver that require dma_alloc_coherent,
>>> but we use drivers that requires dma_alloc and dma_map_single instead.
>> I would vote for a BUILD_BUG_ON_MSG in this function, so you get a
>> compile time error and you can state what needs to be done in order to
>> get rid of the failure.
>
> If we define the function and put a BUILD_BUG_ON_MSG() inside, I am guessing
> that all builds will fail, right?
>
> But we only want the builds that actually call this function to fail.
>
> Maybe we can just define dma_alloc_coherent() as being a macro, to
> BUILD_BUG_ON_MSG.
>
> Like:
>
> #define dma_alloc_coherent(a, b) BUILD_BUG_ON_MSG(1, "dma_alloc_coherent is
> not supported yet on KVX. You would need to add MMU support to be able to map
> uncached pages")
If the macro is expanded, it will fail. Even if the code is ultimately unused
because
of linker section garbage collection. You could define dma_alloc_coherent with
following
body:
{
extern void *coherent_allocation_not_implemented_on_kvx(void);
/* You would need to add MMU support to be able to map uncached pages */
return coherent_allocation_not_implemented_on_kvx();
}
If after linker GC, a reference to dma_alloc_coherent remains, you will get a
linker
error explaining why.
Cheers,
Ahmad
>
> What do you think?
>
>>
>>>>>> +/*
>>>>>> + * The implementation of arch should follow the following rules:
>>>>>> + * map for_cpu for_device unmap
>>>>>> + * TO_DEV writeback none writeback none
>>>>>> + * FROM_DEV invalidate invalidate(*) invalidate
>>>>>> invalidate(*)
>>>>>> + * BIDIR writeback invalidate writeback invalidate
>>>>>> + *
>>>>>> + * (*) - only necessary if the CPU speculatively prefetches.
>>>>>> + *
>>>>>> + * (see https://lkml.org/lkml/2018/5/18/979)
>>>>>> + */
>>>>>> +
>>>>>> +void dma_sync_single_for_device(dma_addr_t addr, size_t size,
>>>>>> + enum dma_data_direction dir)
>>>>>> +{
>>>>>> + switch (dir) {
>>>>>> + case DMA_FROM_DEVICE:
>>>>>> + kvx_dcache_invalidate_mem_area(addr, size);
>>>> Why do you need to explicitly invalidate, but not flush? Even if the
>>>> CPU speculatively prefetches, the coherency protocol should make sure
>>>> to invalidate the speculatively loaded lines, right?
>>> Since we don't have a coherent memory, here we need to invalidate L1
>>> dcache to let the CPU see deivce's writes in memory.
>>> Also every write goes through the cache, flush is not required.
>> Ah, if all your caches are write-through that makes sense. Can you add
>> a comment somewhere stating that this implementation assumes WT caches
>> on KVX? This way we can avoid the confusion Ahamd and myself fell into
>> when glancing over the code.
>>
>> Regards,
>> Lucas
>>
>>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox
