| Message ID | 1408495538-27480-5-git-send-email-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Guenter, On Tue, Aug 19, 2014 at 5:45 PM, Guenter Roeck <linux@roeck-us.net> wrote: > The kernel core now supports a restart handler call chain for system > restart functions. > > With this change, the arm_pm_restart callback is now optional, so > drop its initialization and check if it is set before calling it. > Only call the kernel restart handler if arm_pm_restart is not set. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > Acked-by: Catalin Marinas <catalin.marinas@arm.com> > Acked-by: Heiko Stuebner <heiko@sntech.de> > --- > v7: Dropped null_restart and made arm_pm_restart truly optional. > v6: No change. > v5: Renamed restart function to do_kernel_restart > v4: No change. > v3: Use wrapper function to execute notifier call chain. > v2: Only call notifier call chain if arm_pm_restart is not set. > Do not include linux/watchdog.h. > > arch/arm/kernel/process.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) Reviewed-by: Doug Anderson <dianders@chromium.org> Tested-by: Doug Anderson <dianders@chromium.org>
Hi, Am 20.08.2014 02:45, schrieb Guenter Roeck: > The kernel core now supports a restart handler call chain for system > restart functions. > > With this change, the arm_pm_restart callback is now optional, so > drop its initialization and check if it is set before calling it. > Only call the kernel restart handler if arm_pm_restart is not set. [...] > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > index 81ef686..ea279f7 100644 > --- a/arch/arm/kernel/process.c > +++ b/arch/arm/kernel/process.c > @@ -114,17 +114,13 @@ void soft_restart(unsigned long addr) > BUG(); > } > > -static void null_restart(enum reboot_mode reboot_mode, const char *cmd) > -{ > -} > - > /* > * Function pointers to optional machine specific functions > */ > void (*pm_power_off)(void); > EXPORT_SYMBOL(pm_power_off); > > -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart; > +void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); Stupid newbie question maybe, but isn't this variable uninitialized now, like any non-static variable in C99? Or does the kernel assure that all such "fields" are zero-initialized? > EXPORT_SYMBOL_GPL(arm_pm_restart); (This doesn't seem to be affecting the value of arm_pm_restart, just redeclaring it extern and adding further derived symbols.) > > /* > @@ -230,7 +226,10 @@ void machine_restart(char *cmd) > local_irq_disable(); > smp_send_stop(); > > - arm_pm_restart(reboot_mode, cmd); > + if (arm_pm_restart) Here we seem to be relying on arm_pm_restart to be NULL when not callable. I.e., wondering whether it's ruled out that the following line is triggered due to non-zero garbage in arm_pm_restart? Thanks, Andreas > + arm_pm_restart(reboot_mode, cmd); > + else > + do_kernel_restart(cmd); > > /* Give a grace period for failure to restart of 1s */ > mdelay(1000);
On Fri, Aug 22, 2014 at 03:32:42AM +0200, Andreas Färber wrote: > Hi, > > Am 20.08.2014 02:45, schrieb Guenter Roeck: > > The kernel core now supports a restart handler call chain for system > > restart functions. > > > > With this change, the arm_pm_restart callback is now optional, so > > drop its initialization and check if it is set before calling it. > > Only call the kernel restart handler if arm_pm_restart is not set. > [...] > > diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c > > index 81ef686..ea279f7 100644 > > --- a/arch/arm/kernel/process.c > > +++ b/arch/arm/kernel/process.c > > @@ -114,17 +114,13 @@ void soft_restart(unsigned long addr) > > BUG(); > > } > > > > -static void null_restart(enum reboot_mode reboot_mode, const char *cmd) > > -{ > > -} > > - > > /* > > * Function pointers to optional machine specific functions > > */ > > void (*pm_power_off)(void); > > EXPORT_SYMBOL(pm_power_off); > > > > -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart; > > +void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); > > Stupid newbie question maybe, but isn't this variable uninitialized now, > like any non-static variable in C99? Or does the kernel assure that all > such "fields" are zero-initialized? > It is initialized with NULL, like all other global and static variables in the kernel (and like pm_power_off a few lines above). Guenter
Am 22.08.2014 04:19, schrieb Guenter Roeck: > On Fri, Aug 22, 2014 at 03:32:42AM +0200, Andreas Färber wrote: >> Am 20.08.2014 02:45, schrieb Guenter Roeck: >>> The kernel core now supports a restart handler call chain for system >>> restart functions. >>> >>> With this change, the arm_pm_restart callback is now optional, so >>> drop its initialization and check if it is set before calling it. >>> Only call the kernel restart handler if arm_pm_restart is not set. >> [...] >>> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c >>> index 81ef686..ea279f7 100644 >>> --- a/arch/arm/kernel/process.c >>> +++ b/arch/arm/kernel/process.c >>> @@ -114,17 +114,13 @@ void soft_restart(unsigned long addr) >>> BUG(); >>> } >>> >>> -static void null_restart(enum reboot_mode reboot_mode, const char *cmd) >>> -{ >>> -} >>> - >>> /* >>> * Function pointers to optional machine specific functions >>> */ >>> void (*pm_power_off)(void); >>> EXPORT_SYMBOL(pm_power_off); >>> >>> -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart; >>> +void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); >> >> Stupid newbie question maybe, but isn't this variable uninitialized now, >> like any non-static variable in C99? Or does the kernel assure that all >> such "fields" are zero-initialized? >> > It is initialized with NULL, like all other global and static variables in the > kernel (and like pm_power_off a few lines above). Thanks for the explanation. In that case FWIW Reviewed-by: Andreas Färber <afaerber@suse.de> Andreas
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c index 81ef686..ea279f7 100644 --- a/arch/arm/kernel/process.c +++ b/arch/arm/kernel/process.c @@ -114,17 +114,13 @@ void soft_restart(unsigned long addr) BUG(); } -static void null_restart(enum reboot_mode reboot_mode, const char *cmd) -{ -} - /* * Function pointers to optional machine specific functions */ void (*pm_power_off)(void); EXPORT_SYMBOL(pm_power_off); -void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd) = null_restart; +void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); EXPORT_SYMBOL_GPL(arm_pm_restart); /* @@ -230,7 +226,10 @@ void machine_restart(char *cmd) local_irq_disable(); smp_send_stop(); - arm_pm_restart(reboot_mode, cmd); + if (arm_pm_restart) + arm_pm_restart(reboot_mode, cmd); + else + do_kernel_restart(cmd); /* Give a grace period for failure to restart of 1s */ mdelay(1000);