On Wed, 2019-06-26 at 02:16 +0530, Hari Bathini wrote:
> Introduce callbacks for platform specific operations like register,
> unregister, invalidate & such, and move pseries specific code into
> platform code.
Please don't move around large blocks of code *and* change the code in
a single patch. It makes reviewing the changes extremely tedious since
the changes are mixed in with hundreds of lines of nothing.
> Signed-off-by: Hari Bathini <hbath...@linux.ibm.com>
> ---
> arch/powerpc/include/asm/fadump.h | 75 ----
> arch/powerpc/kernel/fadump-common.h | 38 ++
> arch/powerpc/kernel/fadump.c | 500 ++-----------------------
> arch/powerpc/platforms/pseries/Makefile | 1
> arch/powerpc/platforms/pseries/rtas-fadump.c | 529
> ++++++++++++++++++++++++++
> arch/powerpc/platforms/pseries/rtas-fadump.h | 96 +++++
> 6 files changed, 700 insertions(+), 539 deletions(-)
> create mode 100644 arch/powerpc/platforms/pseries/rtas-fadump.c
> create mode 100644 arch/powerpc/platforms/pseries/rtas-fadump.h
>
> +static struct fadump_ops pseries_fadump_ops = {
> + .init_fadump_mem_struct = pseries_init_fadump_mem_struct,
> + .register_fadump = pseries_register_fadump,
I realise you are just translating the existing interface, but why is
init_fadump_mem_struct() done as a seperate step and not as a part of
the registration function? The struct doesn't seem to be necessary
until the actual registration happens.
> + .unregister_fadump = pseries_unregister_fadump,
> + .invalidate_fadump = pseries_invalidate_fadump,
> + .process_fadump = pseries_process_fadump,
> + .fadump_region_show = pseries_fadump_region_show,
> + .crash_fadump = pseries_crash_fadump,
Rename this to fadump_trigger or something, it's not clear what it
does.
