All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Arnd Bergmann <arnd@arndb.de>, mathieu.poirier@linaro.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jslaby@suse.cz>,
	linux-serial@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-input@vger.kernel.org
Subject: Re: [PATCH] tty: remove platform_sysrq_reset_seq
Date: Fri, 22 May 2015 10:07:14 -0700	[thread overview]
Message-ID: <20150522170714.GD40101@dtor-ws> (raw)
In-Reply-To: <2397446.P9RRJqoFo7@wuerfel>

On Thu, May 21, 2015 at 11:11:28AM +0200, Arnd Bergmann wrote:
> The platform_sysrq_reset_seq code was intended as a way for an embedded
> platform to provide its own sysrq sequence at compile time. After over
> two years, nobody has started using it in an upstream kernel, and
> the platforms that were interested in it have moved on to devicetree,
> which can be used to configure the sequence without requiring kernel
> changes. The method is also incompatible with the way that most
> architectures build support for multiple platforms into a single
> kernel.
> 
> Now the code is producing warnings when built with gcc-5.1:
> 
> drivers/tty/sysrq.c: In function 'sysrq_init':
> drivers/tty/sysrq.c:959:33: warning: array subscript is above array bounds [-Warray-bounds]
>    key = platform_sysrq_reset_seq[i];
> 
> We could fix this, but it seems unlikely that it will ever be used,
> so let's just remove the code instead. We still have the option to
> pass the sequence either in DT, using the kernel command line,
> or using the /sys/module/sysrq/parameters/reset_seq file.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 154b7a489a ("Input: sysrq - allow specifying alternate reset sequence")

Mathieu, are you OK with dropping this?

> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 8ba52e56bb8b..9c5395db6a57 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -55,8 +55,7 @@
>  static int __read_mostly sysrq_enabled = CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE;
>  static bool __read_mostly sysrq_always_enabled;
>  
> -unsigned short platform_sysrq_reset_seq[] __weak = { KEY_RESERVED };
> -int sysrq_reset_downtime_ms __weak;
> +static int sysrq_reset_downtime_ms;
>  
>  static bool sysrq_on(void)
>  {
> @@ -950,23 +949,8 @@ static bool sysrq_handler_registered;
>  
>  static inline void sysrq_register_handler(void)
>  {
> -	unsigned short key;
>  	int error;
> -	int i;
> -
> -	/* First check if a __weak interface was instantiated. */
> -	for (i = 0; i < ARRAY_SIZE(sysrq_reset_seq); i++) {
> -		key = platform_sysrq_reset_seq[i];
> -		if (key == KEY_RESERVED || key > KEY_MAX)
> -			break;
>  
> -		sysrq_reset_seq[sysrq_reset_seq_len++] = key;
> -	}
> -
> -	/*
> -	 * DT configuration takes precedence over anything that would
> -	 * have been defined via the __weak interface.
> -	 */
>  	sysrq_of_get_keyreset_config();
>  
>  	error = input_register_handler(&sysrq_handler);
> 

-- 
Dmitry

WARNING: multiple messages have this Message-ID (diff)
From: dmitry.torokhov@gmail.com (Dmitry Torokhov)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] tty: remove platform_sysrq_reset_seq
Date: Fri, 22 May 2015 10:07:14 -0700	[thread overview]
Message-ID: <20150522170714.GD40101@dtor-ws> (raw)
In-Reply-To: <2397446.P9RRJqoFo7@wuerfel>

On Thu, May 21, 2015 at 11:11:28AM +0200, Arnd Bergmann wrote:
> The platform_sysrq_reset_seq code was intended as a way for an embedded
> platform to provide its own sysrq sequence at compile time. After over
> two years, nobody has started using it in an upstream kernel, and
> the platforms that were interested in it have moved on to devicetree,
> which can be used to configure the sequence without requiring kernel
> changes. The method is also incompatible with the way that most
> architectures build support for multiple platforms into a single
> kernel.
> 
> Now the code is producing warnings when built with gcc-5.1:
> 
> drivers/tty/sysrq.c: In function 'sysrq_init':
> drivers/tty/sysrq.c:959:33: warning: array subscript is above array bounds [-Warray-bounds]
>    key = platform_sysrq_reset_seq[i];
> 
> We could fix this, but it seems unlikely that it will ever be used,
> so let's just remove the code instead. We still have the option to
> pass the sequence either in DT, using the kernel command line,
> or using the /sys/module/sysrq/parameters/reset_seq file.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: 154b7a489a ("Input: sysrq - allow specifying alternate reset sequence")

Mathieu, are you OK with dropping this?

> 
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 8ba52e56bb8b..9c5395db6a57 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -55,8 +55,7 @@
>  static int __read_mostly sysrq_enabled = CONFIG_MAGIC_SYSRQ_DEFAULT_ENABLE;
>  static bool __read_mostly sysrq_always_enabled;
>  
> -unsigned short platform_sysrq_reset_seq[] __weak = { KEY_RESERVED };
> -int sysrq_reset_downtime_ms __weak;
> +static int sysrq_reset_downtime_ms;
>  
>  static bool sysrq_on(void)
>  {
> @@ -950,23 +949,8 @@ static bool sysrq_handler_registered;
>  
>  static inline void sysrq_register_handler(void)
>  {
> -	unsigned short key;
>  	int error;
> -	int i;
> -
> -	/* First check if a __weak interface was instantiated. */
> -	for (i = 0; i < ARRAY_SIZE(sysrq_reset_seq); i++) {
> -		key = platform_sysrq_reset_seq[i];
> -		if (key == KEY_RESERVED || key > KEY_MAX)
> -			break;
>  
> -		sysrq_reset_seq[sysrq_reset_seq_len++] = key;
> -	}
> -
> -	/*
> -	 * DT configuration takes precedence over anything that would
> -	 * have been defined via the __weak interface.
> -	 */
>  	sysrq_of_get_keyreset_config();
>  
>  	error = input_register_handler(&sysrq_handler);
> 

-- 
Dmitry

  reply	other threads:[~2015-05-22 17:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-21  9:11 [PATCH] tty: remove platform_sysrq_reset_seq Arnd Bergmann
2015-05-21  9:11 ` Arnd Bergmann
2015-05-22 17:07 ` Dmitry Torokhov [this message]
2015-05-22 17:07   ` Dmitry Torokhov
2015-05-22 19:58 ` Arnd Bergmann
2015-05-22 19:58   ` Arnd Bergmann

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=20150522170714.GD40101@dtor-ws \
    --to=dmitry.torokhov@gmail.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=jslaby@suse.cz \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=mathieu.poirier@linaro.org \
    /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.