From: Guenter Roeck <linux@roeck-us.net>
To: Jagdish Tirumala <t.jag587@gmail.com>
Cc: wim@linux-watchdog.org, yamada.masahiro@socionext.com,
linux-watchdog@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] WATCHDOG: uniphier_wdt.c: Fixed error do not initialise statics to 0
Date: Wed, 12 Sep 2018 09:02:29 -0700 [thread overview]
Message-ID: <20180912160229.GA3300@roeck-us.net> (raw)
In-Reply-To: <20180912153354.4658-1-t.jag587@gmail.com>
On Wed, Sep 12, 2018 at 08:33:54AM -0700, Jagdish Tirumala wrote:
> Fixed error do not initialise statics to 0
> in file drivers/watchdog/uniphier_wdt.c
>
I don't mind the patch, but I do mind both subject and description
as misleading. This is not an error, except for checkpatch.
A subject such as "watchdog: uniphier_wdt: Do not initialize static
variables to 0" and a description along the line of
Fix the following checkpatch error:
ERROR: do not initialise statics to 0
#44: FILE: drivers/watchdog/uniphier_wdt.c:44:
+static unsigned int timeout = 0;
would be more appropriate.
Please also note that the watchdog subsystem does not use capital
letters to describe the subsystem name, and the driver tag should not
include ".c". As a general note, I would suggest to do a quick browse
through commits in a subsystem to get an idea about its conventions.
Thanks,
Guenter
> Signed-off-by: Jagdish Tirumala <t.jag587@gmail.com>
> ---
> drivers/watchdog/uniphier_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
> index e20a7a459d69..2c62ad7250fb 100644
> --- a/drivers/watchdog/uniphier_wdt.c
> +++ b/drivers/watchdog/uniphier_wdt.c
> @@ -41,7 +41,7 @@
> #define WDT_PERIOD_MIN 1
> #define WDT_PERIOD_MAX 128
>
> -static unsigned int timeout = 0;
> +static unsigned int timeout;
> static bool nowayout = WATCHDOG_NOWAYOUT;
>
> struct uniphier_wdt_dev {
> --
> 2.17.1
>
WARNING: multiple messages have this Message-ID (diff)
From: linux@roeck-us.net (Guenter Roeck)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] WATCHDOG: uniphier_wdt.c: Fixed error do not initialise statics to 0
Date: Wed, 12 Sep 2018 09:02:29 -0700 [thread overview]
Message-ID: <20180912160229.GA3300@roeck-us.net> (raw)
In-Reply-To: <20180912153354.4658-1-t.jag587@gmail.com>
On Wed, Sep 12, 2018 at 08:33:54AM -0700, Jagdish Tirumala wrote:
> Fixed error do not initialise statics to 0
> in file drivers/watchdog/uniphier_wdt.c
>
I don't mind the patch, but I do mind both subject and description
as misleading. This is not an error, except for checkpatch.
A subject such as "watchdog: uniphier_wdt: Do not initialize static
variables to 0" and a description along the line of
Fix the following checkpatch error:
ERROR: do not initialise statics to 0
#44: FILE: drivers/watchdog/uniphier_wdt.c:44:
+static unsigned int timeout = 0;
would be more appropriate.
Please also note that the watchdog subsystem does not use capital
letters to describe the subsystem name, and the driver tag should not
include ".c". As a general note, I would suggest to do a quick browse
through commits in a subsystem to get an idea about its conventions.
Thanks,
Guenter
> Signed-off-by: Jagdish Tirumala <t.jag587@gmail.com>
> ---
> drivers/watchdog/uniphier_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/uniphier_wdt.c b/drivers/watchdog/uniphier_wdt.c
> index e20a7a459d69..2c62ad7250fb 100644
> --- a/drivers/watchdog/uniphier_wdt.c
> +++ b/drivers/watchdog/uniphier_wdt.c
> @@ -41,7 +41,7 @@
> #define WDT_PERIOD_MIN 1
> #define WDT_PERIOD_MAX 128
>
> -static unsigned int timeout = 0;
> +static unsigned int timeout;
> static bool nowayout = WATCHDOG_NOWAYOUT;
>
> struct uniphier_wdt_dev {
> --
> 2.17.1
>
next prev parent reply other threads:[~2018-09-12 21:07 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-12 15:33 [PATCH] WATCHDOG: uniphier_wdt.c: Fixed error do not initialise statics to 0 Jagdish Tirumala
2018-09-12 15:33 ` Jagdish Tirumala
2018-09-12 16:02 ` Guenter Roeck [this message]
2018-09-12 16:02 ` Guenter Roeck
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=20180912160229.GA3300@roeck-us.net \
--to=linux@roeck-us.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=t.jag587@gmail.com \
--cc=wim@linux-watchdog.org \
--cc=yamada.masahiro@socionext.com \
/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.