On 10/11/24 8:30 PM, Ritesh Harjani (IBM) wrote: > We anyway don't use any return values from fadump_cma_init(). Since > fadump_reserve_mem() from where fadump_cma_init() gets called today, > already has the required checks. > This patch makes this function return type as void. Let's also handle > extra cases like return if fadump_supported is false or dump_active, so > that in later patches we can call fadump_cma_init() separately from > setup_arch(). Usually patches to this file are posted with title format of powerpc/fadump:<> > > Acked-by: Hari Bathini <hbathini@xxxxxxxxxxxxx> > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@xxxxxxxxx> > --- > v2 -> v3: Separated the series into 2 as discussed in v2. > [v2]: https://lore.kernel.org/linuxppc-dev/cover.1728585512.git.ritesh.list@xxxxxxxxx/ > > arch/powerpc/kernel/fadump.c | 23 +++++++++-------------- > 1 file changed, 9 insertions(+), 14 deletions(-) > > diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c > index a612e7513a4f..162327d66982 100644 > --- a/arch/powerpc/kernel/fadump.c > +++ b/arch/powerpc/kernel/fadump.c > @@ -78,27 +78,23 @@ static struct cma *fadump_cma; > * But for some reason even if it fails we still have the memory reservation > * with us and we can still continue doing fadump. > */ > -static int __init fadump_cma_init(void) > +static void __init fadump_cma_init(void) > { > unsigned long long base, size; > int rc; > > - if (!fw_dump.fadump_enabled) > - return 0; > - > + if (!fw_dump.fadump_supported || !fw_dump.fadump_enabled || > + fw_dump.dump_active) > + return; Is these checks even needed here? fadump_reserve_mem() checked for all these already, also dont see any other caller for fadump_cma_init(). > /* > * Do not use CMA if user has provided fadump=nocma kernel parameter. > - * Return 1 to continue with fadump old behaviour. > */ > - if (fw_dump.nocma) > - return 1; > + if (fw_dump.nocma || !fw_dump.boot_memory_size) > + return; > > base = fw_dump.reserve_dump_area_start; > size = fw_dump.boot_memory_size; > > - if (!size) > - return 0; So this is the only place where we return 0, which in turn will make the "ret" in fadump_reserve_mem() as zero forcing to call reserve_crashkernel() in early_init_devtree(). we are removing it, becos we know "size" here will never be zero? > - > rc = cma_init_reserved_mem(base, size, 0, "fadump_cma", &fadump_cma); > if (rc) { > pr_err("Failed to init cma area for firmware-assisted dump,%d\n", rc); > @@ -108,7 +104,7 @@ static int __init fadump_cma_init(void) > * blocked from production system usage. Hence return 1, > * so that we can continue with fadump. > */ > - return 1; > + return; > } > > /* > @@ -125,10 +121,9 @@ static int __init fadump_cma_init(void) > cma_get_size(fadump_cma), > (unsigned long)cma_get_base(fadump_cma) >> 20, > fw_dump.reserve_dump_area_size); > - return 1; > } > #else > -static int __init fadump_cma_init(void) { return 1; } > +static void __init fadump_cma_init(void) { } > #endif /* CONFIG_CMA */ > > /* > @@ -638,7 +633,7 @@ int __init fadump_reserve_mem(void) > pr_info("Reserved %lldMB of memory at %#016llx (System RAM: %lldMB)\n", > (size >> 20), base, (memblock_phys_mem_size() >> 20)); > > - ret = fadump_cma_init(); > + fadump_cma_init(); > } > > return ret; > -- > 2.46.0 >
