Messages in this thread | | | Date | Thu, 13 Oct 2022 06:37:39 +0100 | From | Conor Dooley <> | Subject | Re: [PATCH 1/8] riscv: move riscv nonc oherent supported() out of ZICBOM probe |
| |
On 8 October 2022 14:59:37 IST, Jisheng Zhang <jszhang@kernel.org> wrote: >On Sat, Oct 08, 2022 at 02:06:00PM +0100, Conor Dooley wrote: >> On Thu, Oct 06, 2022 at 03:08:11PM +0800, Jisheng Zhang wrote: >> > It's a bit wired to call riscv_noncoherent_supported() once when >> > insmod a module. Move the calling out of feature patch func. >> > >> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> >> > --- >> > arch/riscv/kernel/cpufeature.c | 7 +------ >> > arch/riscv/kernel/setup.c | 4 ++++ >> > 2 files changed, 5 insertions(+), 6 deletions(-) >> > >> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c >> > index 3b5583db9d80..03611b3ef45e 100644 >> > --- a/arch/riscv/kernel/cpufeature.c >> > +++ b/arch/riscv/kernel/cpufeature.c >> > @@ -272,12 +272,7 @@ static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage) >> > case RISCV_ALTERNATIVES_EARLY_BOOT: >> > return false; >> > default: >> > - if (riscv_isa_extension_available(NULL, ZICBOM)) { >> > - riscv_noncoherent_supported(); >> > - return true; >> > - } else { >> > - return false; >> > - } >> > + return riscv_isa_extension_available(NULL, ZICBOM); >> > } >> > #endif >> > >> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c >> > index 2dfc463b86bb..1a055c3f5d9d 100644 >> > --- a/arch/riscv/kernel/setup.c >> > +++ b/arch/riscv/kernel/setup.c >> > @@ -299,6 +299,10 @@ void __init setup_arch(char **cmdline_p) >> > riscv_init_cbom_blocksize(); >> > riscv_fill_hwcap(); >> > apply_boot_alternatives(); >> > +#ifdef CONFIG_RISCV_DMA_NONCOHERENT >> > + if (riscv_isa_extension_available(NULL, ZICBOM)) >> > + riscv_noncoherent_supported(); >> > +#endif >> >> I have a personal bias against ifdefs where possible, maybe @Heiko >> remembers why riscv_noncoherent_supported() was not defined as something >> like `void riscv_noncoherent_support(void){}` for when that CONFIG is >> not enabled? If it was this could become a an IS_ENABLED & we wouldn't >> have to be so careful about wrapping it's usage in ifdefs. > >Good idea. Will do in newer version.
Given this comment and the LKP report I've marked the series as changes requested in patchwork FYI.
Thanks, Conor.
> >> >> Your change in isolation makes sense to me though, so: >> Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> >> Thanks, >> Conor. >> >> > } >> > >> > static int __init topology_init(void) >> > -- >> > 2.37.2 >> >
| |