All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Guenter Roeck <linux@roeck-us.net>,
	Doug Anderson <dianders@chromium.org>
Cc: Russell King <linux@arm.linux.org.uk>,
	Wim Van Sebroeck <wim@iguana.be>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Maxime Ripard <maxime.ripard@free-electrons.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org,
	Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de>,
	Heiko Stuebner <heiko@sntech.de>,
	Jonas Jensen <jonas.jensen@gmail.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Tomasz Figa <t.figa@samsung.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 04/11] arm: Support restart through restart handler call chain
Date: Fri, 22 Aug 2014 03:32:42 +0200	[thread overview]
Message-ID: <53F69DBA.4070806@suse.de> (raw)
In-Reply-To: <1408495538-27480-5-git-send-email-linux@roeck-us.net>

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);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

WARNING: multiple messages have this Message-ID (diff)
From: "Andreas Färber" <afaerber@suse.de>
To: Guenter Roeck <linux@roeck-us.net>,
	Doug Anderson <dianders@chromium.org>
Cc: Russell King <linux@arm.linux.org.uk>,
	Wim Van Sebroeck <wim@iguana.be>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Maxime Ripard <maxime.ripard@free-electrons.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org,
	linux-samsung-soc@vger.kernel.org,
	Will Deacon <will.deacon@arm.com>, Arnd Bergmann <arnd@arndb.de>,
	Heiko Stuebner <heiko@sntech.de>,
	Jonas Jensen <jonas.jensen@gmail.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Tomasz Figa <t.figa@samsung.com>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 04/11] arm: Support restart through restart handler call chain
Date: Fri, 22 Aug 2014 03:32:42 +0200	[thread overview]
Message-ID: <53F69DBA.4070806@suse.de> (raw)
In-Reply-To: <1408495538-27480-5-git-send-email-linux@roeck-us.net>

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);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: afaerber@suse.de (Andreas Färber)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v7 04/11] arm: Support restart through restart handler call chain
Date: Fri, 22 Aug 2014 03:32:42 +0200	[thread overview]
Message-ID: <53F69DBA.4070806@suse.de> (raw)
In-Reply-To: <1408495538-27480-5-git-send-email-linux@roeck-us.net>

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);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg

  parent reply	other threads:[~2014-08-22  1:32 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-20  0:45 [PATCH v7 00/11] kernel: Add support for restart handler call chain Guenter Roeck
2014-08-20  0:45 ` Guenter Roeck
2014-08-20  0:45 ` [PATCH v7 01/11] kernel: Add support for kernel " Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-21  4:08   ` Doug Anderson
2014-08-21  4:08     ` Doug Anderson
2014-08-21  4:08     ` Doug Anderson
2014-08-20  0:45 ` [PATCH v7 02/11] power/restart: Call machine_restart instead of arm_pm_restart Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-21  4:10   ` Doug Anderson
2014-08-21  4:10     ` Doug Anderson
2014-08-21  4:10     ` Doug Anderson
2014-08-21  4:42     ` Guenter Roeck
2014-08-21  4:42       ` Guenter Roeck
2014-08-21  4:42       ` Guenter Roeck
2014-08-21 19:30       ` Doug Anderson
2014-08-21 19:30         ` Doug Anderson
2014-08-21 19:30         ` Doug Anderson
2014-08-21 20:11         ` Guenter Roeck
2014-08-21 20:11           ` Guenter Roeck
2014-08-21 20:11           ` Guenter Roeck
2014-08-21 20:39   ` Sebastian Reichel
2014-08-21 20:39     ` Sebastian Reichel
2014-08-23 17:20     ` Guenter Roeck
2014-08-23 17:20       ` Guenter Roeck
2014-08-20  0:45 ` [PATCH v7 03/11] arm64: Support restart through restart handler call chain Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-21  4:13   ` Doug Anderson
2014-08-21  4:13     ` Doug Anderson
2014-08-21  4:13     ` Doug Anderson
2014-08-20  0:45 ` [PATCH v7 04/11] arm: " Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-21  4:11   ` Doug Anderson
2014-08-21  4:11     ` Doug Anderson
2014-08-21  4:11     ` Doug Anderson
2014-08-22  1:32   ` Andreas Färber [this message]
2014-08-22  1:32     ` Andreas Färber
2014-08-22  1:32     ` Andreas Färber
2014-08-22  2:19     ` Guenter Roeck
2014-08-22  2:19       ` Guenter Roeck
2014-08-23 17:11       ` Andreas Färber
2014-08-23 17:11         ` Andreas Färber
2014-08-20  0:45 ` [PATCH v7 05/11] watchdog: moxart: Register restart handler with kernel restart handler Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45 ` [PATCH v7 06/11] watchdog: alim7101: " Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45 ` [PATCH v7 07/11] watchdog: sunxi: " Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45 ` [PATCH v7 08/11] arm/arm64: Unexport restart handlers Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-21  4:12   ` Doug Anderson
2014-08-21  4:12     ` Doug Anderson
2014-08-21  4:12     ` Doug Anderson
2014-12-04 13:36   ` Geert Uytterhoeven
2014-12-04 13:36     ` Geert Uytterhoeven
2014-12-04 13:36     ` Geert Uytterhoeven
2014-12-04 14:26     ` Guenter Roeck
2014-12-04 14:26       ` Guenter Roeck
2014-12-04 14:26       ` Guenter Roeck
2014-12-04 14:26       ` Guenter Roeck
2014-12-04 14:44       ` Geert Uytterhoeven
2014-12-04 14:44         ` Geert Uytterhoeven
2014-12-04 14:44         ` Geert Uytterhoeven
2014-12-04 14:44         ` Geert Uytterhoeven
2014-12-04 14:51         ` Guenter Roeck
2014-12-04 14:51           ` Guenter Roeck
2014-12-04 14:51           ` Guenter Roeck
2014-12-04 14:51           ` Guenter Roeck
     [not found]           ` <54807505.2000406-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
2014-12-04 15:06             ` Arnd Bergmann
2014-12-04 15:06               ` Arnd Bergmann
2014-12-04 15:06               ` Arnd Bergmann
2014-12-04 16:11               ` Guenter Roeck
2014-12-04 16:11                 ` Guenter Roeck
2014-12-04 16:11                 ` Guenter Roeck
2014-08-20  0:45 ` [PATCH v7 09/11] watchdog: s3c2410: add restart handler Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45 ` [PATCH v7 10/11] clk: samsung: register restart handlers for s3c2412 and s3c2443 Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-09-03 19:05   ` Mike Turquette
2014-09-03 19:05     ` Mike Turquette
2014-09-03 19:05     ` Mike Turquette
2014-08-20  0:45 ` [PATCH v7 11/11] clk: rockchip: add restart handler Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-20  0:45   ` Guenter Roeck
2014-08-21  4:15   ` Doug Anderson
2014-08-21  4:15     ` Doug Anderson
2014-08-21  4:15     ` Doug Anderson
2014-08-21  4:15     ` Doug Anderson
2014-08-21  8:18     ` Heiko Stübner
2014-08-21  8:18       ` Heiko Stübner
2014-08-21  8:18       ` Heiko Stübner
2014-08-21  8:18       ` Heiko Stübner
2014-09-03 19:05   ` Mike Turquette
2014-09-03 19:05     ` Mike Turquette
2014-09-03 19:05     ` Mike Turquette
2014-08-23 16:35 ` [PATCH v7 00/11] kernel: Add support for restart handler call chain Guenter Roeck
2014-08-23 16:35   ` Guenter Roeck
2014-08-23 23:00   ` Heiko Stübner
2014-08-23 23:00     ` Heiko Stübner
2014-08-23 23:00     ` Heiko Stübner
2014-08-24  0:17     ` Guenter Roeck
2014-08-24  0:17       ` Guenter Roeck
2014-09-19 12:54 ` Pramod Gurav
2014-09-19 12:54   ` Pramod Gurav
2014-09-30 21:20 ` Andrew Morton
2014-09-30 21:20   ` Andrew Morton
2014-09-30 22:30   ` Guenter Roeck
2014-09-30 22:30     ` Guenter Roeck
2014-09-30 23:40     ` Stephen Rothwell
2014-09-30 23:40       ` Stephen Rothwell
2014-10-01  3:28       ` Guenter Roeck
2014-10-01  3:28         ` Guenter Roeck
2014-10-01 13:32     ` Heiko Stübner
2014-10-01 13:32       ` Heiko Stübner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53F69DBA.4070806@suse.de \
    --to=afaerber@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=catalin.marinas@arm.com \
    --cc=dbaryshkov@gmail.com \
    --cc=dianders@chromium.org \
    --cc=dwmw2@infradead.org \
    --cc=heiko@sntech.de \
    --cc=jonas.jensen@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=linux@roeck-us.net \
    --cc=maxime.ripard@free-electrons.com \
    --cc=mingo@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=t.figa@samsung.com \
    --cc=will.deacon@arm.com \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.