Hari Bathini <hbath...@linux.ibm.com> writes:
> Make way for refactoring platform specific FADump code by moving code
> that could be referenced from multiple places to fadump-common.c file.
>
> Signed-off-by: Hari Bathini <hbath...@linux.ibm.com>
> ---
> arch/powerpc/kernel/Makefile | 2
> arch/powerpc/kernel/fadump-common.c | 140 ++++++++++++++++++++++++++++++++++
> arch/powerpc/kernel/fadump-common.h | 8 ++
> arch/powerpc/kernel/fadump.c | 146
> ++---------------------------------
> 4 files changed, 158 insertions(+), 138 deletions(-)
> create mode 100644 arch/powerpc/kernel/fadump-common.c
I don't understand why we need fadump.c and fadump-common.c? They're
both common/shared across pseries & powernv aren't they?
By the end of the series we end up with 149 lines in fadump-common.c
which seems like a waste of time. Just put it all in fadump.c.
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 56dfa7a..439d548 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -78,7 +78,7 @@ obj-$(CONFIG_EEH) += eeh.o eeh_pe.o eeh_dev.o
> eeh_cache.o \
> eeh_driver.o eeh_event.o eeh_sysfs.o
> obj-$(CONFIG_GENERIC_TBSYNC) += smp-tbsync.o
> obj-$(CONFIG_CRASH_DUMP) += crash_dump.o
> -obj-$(CONFIG_FA_DUMP) += fadump.o
> +obj-$(CONFIG_FA_DUMP) += fadump.o fadump-common.o
> ifdef CONFIG_PPC32
> obj-$(CONFIG_E500) += idle_e500.o
> endif
> diff --git a/arch/powerpc/kernel/fadump-common.c
> b/arch/powerpc/kernel/fadump-common.c
> new file mode 100644
> index 0000000..7f39e4f
> --- /dev/null
> +++ b/arch/powerpc/kernel/fadump-common.c
> @@ -0,0 +1,140 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Firmware-Assisted Dump internal code.
> + *
> + * Copyright 2011, IBM Corporation
> + * Author: Mahesh Salgaonkar <mah...@linux.ibm.com>
Can we not put emails in C files anymore please, they just bitrot, just
the names is fine.
> + * Copyright 2019, IBM Corp.
> + * Author: Hari Bathini <hbath...@linux.ibm.com>
These can just be:
* Copyright 2011, Mahesh Salgaonkar, IBM Corporation.
* Copyright 2019, Hari Bathini, IBM Corporation.
> + */
> +
> +#undef DEBUG
Don't undef DEBUG please.
> +#define pr_fmt(fmt) "fadump: " fmt
> +
> +#include <linux/memblock.h>
> +#include <linux/elf.h>
> +#include <linux/mm.h>
> +#include <linux/crash_core.h>
> +
> +#include "fadump-common.h"
> +
> +void *fadump_cpu_notes_buf_alloc(unsigned long size)
> +{
> + void *vaddr;
> + struct page *page;
> + unsigned long order, count, i;
> +
> + order = get_order(size);
> + vaddr = (void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO, order);
> + if (!vaddr)
> + return NULL;
> +
> + count = 1 << order;
> + page = virt_to_page(vaddr);
> + for (i = 0; i < count; i++)
> + SetPageReserved(page + i);
> + return vaddr;
> +}
I realise you're just moving this code, but why do we need all this hand
rolled allocation stuff?
cheers
