Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 04 Jan 2023 11:19:44 +0100 | From | "Arnd Bergmann" <> | Subject | Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability |
| |
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
| ![\](/images/icornerr.gif) |