Re: [PATCH 3/3] powerpc: Convert fsl_rstcr_restart to a reset handler

[Andrey Smirnov]

On Tue, Jul 26, 2016 at 12:59 AM, Scott Wood <oss@xxxxxxxxxxxx> wrote:
> On Mon, 2016-07-25 at 21:25 -0700, Andrey Smirnov wrote:
>> Convert fsl_rstcr_restart into a function to be registered with
>> register_reset_handler() API and introduce fls_rstcr_restart_register()
>> function that can be added as an initcall that would do aforementioned
>> registration.
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
>
> Is there a particular motivation for this (e.g. new handlers you plan to
> register elsewhere)?

I have a MPC8548 based board that which uses, at least for time being,
SBC8548's init code(by claiming compatibility in DT) which has an
external watchdog that implements reset functionality. The driver for
watchdog is just a generic watchdog driver and having an ability to
register custom reset handlers is very handy.

I don't really have any motivation for fixing boards other than
SBC8548 and even that I can avoid doing by making a new custom board
file in my tree that would not populate .reset field. I can drop this
patch from the series if the code of those boards is in "don't touch
it unless absolutely have to" state.

>
>> diff --git a/arch/powerpc/platforms/85xx/bsc913x_qds.c
>> b/arch/powerpc/platforms/85xx/bsc913x_qds.c
>> index 07dd6ae..14ea7a0 100644
>> --- a/arch/powerpc/platforms/85xx/bsc913x_qds.c
>> +++ b/arch/powerpc/platforms/85xx/bsc913x_qds.c
>> @@ -53,6 +53,7 @@ static void __init bsc913x_qds_setup_arch(void)
>>  }
>>
>>  machine_arch_initcall(bsc9132_qds, mpc85xx_common_publish_devices);
>> +machine_arch_initcall(bsc9133_qds, fsl_rstcr_restart_register);
>
> Do we really still need to call the registration on a per-board basis, now
> that boards have a way of registering a higher-priority notifier?  Can't we
> just have setup_rstcr() do the registration when it finds the appropriate
> device tree node?

I think we could, that idea just never occurred to me. What you
describe should be a cleaner way to handle this change, I'll convert
the code to do that in v2.

>
>> +int fsl_rstcr_restart_register(void)
>> +{
>> +     static struct notifier_block restart_handler;
>> +
>> +     restart_handler.notifier_call = fsl_rstcr_restart;
>> +     restart_handler.priority = 128;
>> +
>> +     return register_restart_handler(&restart_handler);
>> +}
>> +EXPORT_SYMBOL(fsl_rstcr_restart_register);
>
> When would this ever get called from a module?

Probably never, that's just a mistake on my part. Will remove in v2.

Thanks,
Andrey