On Wed, Jan 4, 2023, at 10:23, Conor Dooley wrote:
>>Right, no need to touch the existing file as part of this series,
>>it probably just gets in the way of defining a good interface here.
>
> Sure. Can leave it where it was & I'll sort it out later when it's
> errata etc get added.
>
> Btw, would you mind pointing out where you wanted to have that if/else
> you mentioned on IRC?
I meant replacing both of the runtime patching indirections in
arch_sync_dma_for_device(). At the moment, this function calls
ALT_CMO_OP(), which is patched to either call the ZICBOM or the
THEAD variant, and if I read this right you add a third case
there with another level of indirection using static_branch.
I would try to replace both of these indirections and instead
handle it all from C code in arch_sync_dma_for_device() directly,
for the purpose of readability and maintainability.
static inline void dma_cache_clean(void *vaddr, size_t size)
{
if (!cache_maint_ops.clean)
zicbom_cache_clean(vaddr, size, riscv_cbom_block_size);
else
cache_maint_ops.clean(vaddr, size, riscv_cbom_block_size);
}
void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
enum dma_data_direction dir)
{
void *vaddr = phys_to_virt(paddr);
switch (dir) {
case DMA_TO_DEVICE:
case DMA_FROM_DEVICE:
dma_cache_clean(vaddr, size);
break;
case DMA_BIDIRECTIONAL:
dma_cache_flush(vaddr, size);
break;
default:
break;
}
}
which then makes it very clear what the actual code path
is, while leaving the zicbom case free of indirect function
calls. You can still use a static_branch() to optimize the
conditional, but I would try to avoid any extra indirection
levels or errata checks.
Arnd
