From: Guenter Roeck <linux@roeck-us.net> To: Carlo Caione <carlo@caione.org> Cc: wim@iguana.be, linux-watchdog@vger.kernel.org, linux-arm-kernel <linux-arm-kernel@lists.infradead.org>, "robh+dt@kernel.org" <robh+dt@kernel.org>, Daniel Drake <drake@endlessm.com>, Jerry Cao <jerry.cao@amlogic.com>, Victor Wan <victor.wan@amlogic.com>, Romain Perier <romain.perier@gmail.com>, Carlo Caione <carlo@endlessm.com> Subject: Re: [PATCH v2 1/4] watchdog: meson: Enable meson SoC specific data Date: Sat, 7 Nov 2015 08:37:36 -0800 [thread overview] Message-ID: <563E28D0.3070400@roeck-us.net> (raw) In-Reply-To: <CAOQ7t2a1KBUbTvtcu7ajyX35Kx1khtYHc8pGmYdYC=872oNMQA@mail.gmail.com> On 11/07/2015 07:45 AM, Carlo Caione wrote: > On Sat, Nov 7, 2015 at 4:29 PM, Guenter Roeck <linux@roeck-us.net> wrote: >> On 11/07/2015 03:02 AM, Carlo Caione wrote: >>> >>> From: Carlo Caione <carlo@endlessm.com> >>> >>> With this patch we refactor the driver code to enable watchdog support >>> for all platforms based on Amlogic meson SoCs. >>> >>> Signed-off-by: Carlo Caione <carlo@endlessm.com> >>> --- >>> drivers/watchdog/meson_wdt.c | 56 >>> ++++++++++++++++++++++++++++++-------------- >>> 1 file changed, 39 insertions(+), 17 deletions(-) >>> >>> diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c >>> index 1f4155e..446af94 100644 >>> --- a/drivers/watchdog/meson_wdt.c >>> +++ b/drivers/watchdog/meson_wdt.c >>> @@ -19,6 +19,7 @@ >>> #include <linux/moduleparam.h> >>> #include <linux/notifier.h> >>> #include <linux/of.h> >>> +#include <linux/of_device.h> >>> #include <linux/platform_device.h> >>> #include <linux/reboot.h> >>> #include <linux/types.h> >>> @@ -27,35 +28,47 @@ >>> #define DRV_NAME "meson_wdt" >>> >>> #define MESON_WDT_TC 0x00 >>> -#define MESON_WDT_TC_EN BIT(22) >>> -#define MESON_WDT_TC_TM_MASK 0x3fffff >>> #define MESON_WDT_DC_RESET (3 << 24) >>> >>> #define MESON_WDT_RESET 0x04 >>> >>> -#define MESON_WDT_TIMEOUT 30 >>> +#define MESON_WDT_TIMEOUT 5 >> >> >> This is not a reasonable default, the change is not mentioned >> in the description, and it is unrelated to the change at hand. > > The problem is that the default is too small or simply that the change > is not mentioned? > Both. > For the meson8b the maximum timeout is ~8 sec whereas for meson6 is > ~40 sec that's why I lowered the default from 30 to 5 sec. > I can set a different default for each SoC but at least for meson8b > the value will be always < 8 sec. > Ok, but your change impacts both architectures. There is no reason to hold meson6 hostage for the meson8b limitations. An easy fix would be to set the actual timeout to min(default, max). Thanks, Guenter
WARNING: multiple messages have this Message-ID (diff)
From: linux@roeck-us.net (Guenter Roeck) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v2 1/4] watchdog: meson: Enable meson SoC specific data Date: Sat, 7 Nov 2015 08:37:36 -0800 [thread overview] Message-ID: <563E28D0.3070400@roeck-us.net> (raw) In-Reply-To: <CAOQ7t2a1KBUbTvtcu7ajyX35Kx1khtYHc8pGmYdYC=872oNMQA@mail.gmail.com> On 11/07/2015 07:45 AM, Carlo Caione wrote: > On Sat, Nov 7, 2015 at 4:29 PM, Guenter Roeck <linux@roeck-us.net> wrote: >> On 11/07/2015 03:02 AM, Carlo Caione wrote: >>> >>> From: Carlo Caione <carlo@endlessm.com> >>> >>> With this patch we refactor the driver code to enable watchdog support >>> for all platforms based on Amlogic meson SoCs. >>> >>> Signed-off-by: Carlo Caione <carlo@endlessm.com> >>> --- >>> drivers/watchdog/meson_wdt.c | 56 >>> ++++++++++++++++++++++++++++++-------------- >>> 1 file changed, 39 insertions(+), 17 deletions(-) >>> >>> diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c >>> index 1f4155e..446af94 100644 >>> --- a/drivers/watchdog/meson_wdt.c >>> +++ b/drivers/watchdog/meson_wdt.c >>> @@ -19,6 +19,7 @@ >>> #include <linux/moduleparam.h> >>> #include <linux/notifier.h> >>> #include <linux/of.h> >>> +#include <linux/of_device.h> >>> #include <linux/platform_device.h> >>> #include <linux/reboot.h> >>> #include <linux/types.h> >>> @@ -27,35 +28,47 @@ >>> #define DRV_NAME "meson_wdt" >>> >>> #define MESON_WDT_TC 0x00 >>> -#define MESON_WDT_TC_EN BIT(22) >>> -#define MESON_WDT_TC_TM_MASK 0x3fffff >>> #define MESON_WDT_DC_RESET (3 << 24) >>> >>> #define MESON_WDT_RESET 0x04 >>> >>> -#define MESON_WDT_TIMEOUT 30 >>> +#define MESON_WDT_TIMEOUT 5 >> >> >> This is not a reasonable default, the change is not mentioned >> in the description, and it is unrelated to the change at hand. > > The problem is that the default is too small or simply that the change > is not mentioned? > Both. > For the meson8b the maximum timeout is ~8 sec whereas for meson6 is > ~40 sec that's why I lowered the default from 30 to 5 sec. > I can set a different default for each SoC but at least for meson8b > the value will be always < 8 sec. > Ok, but your change impacts both architectures. There is no reason to hold meson6 hostage for the meson8b limitations. An easy fix would be to set the actual timeout to min(default, max). Thanks, Guenter
next prev parent reply other threads:[~2015-11-07 16:37 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-11-07 11:02 [PATCH v2 0/4] watchdog: meson: Add meson8b watchdog Carlo Caione 2015-11-07 11:02 ` Carlo Caione 2015-11-07 11:02 ` [PATCH v2 1/4] watchdog: meson: Enable meson SoC specific data Carlo Caione 2015-11-07 11:02 ` Carlo Caione 2015-11-07 15:29 ` Guenter Roeck 2015-11-07 15:29 ` Guenter Roeck 2015-11-07 15:45 ` Carlo Caione 2015-11-07 15:45 ` Carlo Caione 2015-11-07 16:37 ` Guenter Roeck [this message] 2015-11-07 16:37 ` Guenter Roeck 2015-11-07 11:02 ` [PATCH v2 2/4] watchdog: meson: Add meson8b " Carlo Caione 2015-11-07 11:02 ` Carlo Caione 2015-11-07 11:02 ` [PATCH v2 3/4] Documentation: watchdog: Add new bindings for meson8b Carlo Caione 2015-11-07 11:02 ` Carlo Caione 2015-11-07 11:02 ` [PATCH v2 4/4] ARM: dts: meson8b: Add watchdog node Carlo Caione 2015-11-07 11:02 ` Carlo Caione
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=563E28D0.3070400@roeck-us.net \ --to=linux@roeck-us.net \ --cc=carlo@caione.org \ --cc=carlo@endlessm.com \ --cc=drake@endlessm.com \ --cc=jerry.cao@amlogic.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-watchdog@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=romain.perier@gmail.com \ --cc=victor.wan@amlogic.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: linkBe 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.