Hi Prabhakar, Thanks for your patch! On Thu, Mar 30, 2023 at 10:42 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > > Currently, selecting which CMOs to use on a given platform is done using > and ALTERNATIVE_X() macro. This was manageable when there were just two the ALTERNATIVE_X() > CMO implementations, but now that there are more and more platforms coming > needing custom CMOs, the use of the ALTERNATIVE_X() macro is unmanageable. > > To avoid such issues this patch switches to use of function pointers "the use" or "using" > instead of ALTERNATIVE_X() macro for cache management (the only drawback the ALTERNATIVE_X() > being performance over the previous approach). > > void (*clean_range)(unsigned long addr, unsigned long size); > void (*inv_range)(unsigned long addr, unsigned long size); > void (*flush_range)(unsigned long addr, unsigned long size); > > The above function pointers are provided to be overridden for platforms > needing CMO. > > Convert ZICBOM and T-HEAD CMO to use function pointers. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > --- a/arch/riscv/errata/thead/errata.c > +++ b/arch/riscv/errata/thead/errata.c > +#ifdef CONFIG_ERRATA_THEAD_CMO > +static void thead_register_cmo_ops(void) > +{ > + riscv_noncoherent_register_cache_ops(&thead_cmo_ops); > +} > +#else > +static void thead_register_cmo_ops(void) {} > +#endif > --- a/arch/riscv/mm/dma-noncoherent.c > +++ b/arch/riscv/mm/dma-noncoherent.c > @@ -75,3 +83,12 @@ void riscv_noncoherent_supported(void) > "Non-coherent DMA support enabled without a block size\n"); > noncoherent_supported = true; > } > + > +void riscv_noncoherent_register_cache_ops(const struct riscv_cache_ops *ops) > +{ > + if (!ops) > + return; This is never true. I guess originally you wanted to call riscv_noncoherent_register_cache_ops() unconditionally from common code, instead of the various *register_cmo_ops()? But that would have required something like #ifdef CONFIG_ERRATA_THEAD_CMO #define THEAD_CMO_OPS_PTR (&thead_cmo_ops) #else #define THEAD_CMO_OPS_PTR NULL #endif Or can we come up with some macro like pm_ptr(), but that also takes care of the "&", so we can do "#define thead_cmo_ops NULL"? > + > + noncoherent_cache_ops = *ops; > +} > +EXPORT_SYMBOL_GPL(riscv_noncoherent_register_cache_ops); Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds