Re: [PATCH 1/1] booke/watchdog: refine and clean up the codes
Hi Tang, > From: Tang Yuantian > > Basically, this patch does the following: > 1. Move the codes of parsing boot parameters from setup-common.c >to driver. In this way, code reader can know directly that >there are boot parameters that can change the timeout. > 2. Make boot parameter 'booke_wdt_period' effective. >currently, when driver is loaded, default timeout is always >being used in stead of booke_wdt_period. > 3. Wrap up the watchdog timeout in device struct and clean up >unnecessary codes. > > Signed-off-by: Tang Yuantian > Acked-by: Scott Wood > --- > resend to watchdog maintainer > > arch/powerpc/kernel/setup-common.c | 27 > drivers/watchdog/booke_wdt.c | 51 > -- > 2 files changed, 33 insertions(+), 45 deletions(-) Patch has been added to linux-watchdog-next. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 10/10] Revert "powerpc/watchdog: Don't enable interrupt on PPC64 BookE"
Hi Scott, > On Sat, 2014-03-15 at 20:51 +0100, Wim Van Sebroeck wrote: > > Hi Scott, > > > > > This reverts commit 3978bdb4ed653342b0be66c031bf61b72cc55d60, now that > > > critical interrupts are properly supported on ppc64 booke. > > > > > > Signed-off-by: Scott Wood > > > Cc: Laurentiu Tudor > > > Cc: Wim Van Sebroeck > > > --- > > > drivers/watchdog/booke_wdt.c | 8 > > > 1 file changed, 8 deletions(-) > > > > > > diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c > > > index f1b8d55..a8dbceb3 100644 > > > --- a/drivers/watchdog/booke_wdt.c > > > +++ b/drivers/watchdog/booke_wdt.c > > > @@ -138,14 +138,6 @@ static void __booke_wdt_enable(void *data) > > > val &= ~WDTP_MASK; > > > val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(booke_wdt_period)); > > > > > > -#ifdef CONFIG_PPC_BOOK3E_64 > > > - /* > > > - * Crit ints are currently broken on PPC64 Book-E, so > > > - * just disable them for now. > > > - */ > > > - val &= ~TCR_WIE; > > > -#endif > > > - > > > mtspr(SPRN_TCR, val); > > > } > > > > > > > Patch has been added to linux-watchdog-next. > > Please unapply it. It is patch 10/10 and depends on the previous parts > of the patchset to make critical interrupts work properly. Unapplied. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 10/10] Revert "powerpc/watchdog: Don't enable interrupt on PPC64 BookE"
Hi Scott, > This reverts commit 3978bdb4ed653342b0be66c031bf61b72cc55d60, now that > critical interrupts are properly supported on ppc64 booke. > > Signed-off-by: Scott Wood > Cc: Laurentiu Tudor > Cc: Wim Van Sebroeck > --- > drivers/watchdog/booke_wdt.c | 8 > 1 file changed, 8 deletions(-) > > diff --git a/drivers/watchdog/booke_wdt.c b/drivers/watchdog/booke_wdt.c > index f1b8d55..a8dbceb3 100644 > --- a/drivers/watchdog/booke_wdt.c > +++ b/drivers/watchdog/booke_wdt.c > @@ -138,14 +138,6 @@ static void __booke_wdt_enable(void *data) > val &= ~WDTP_MASK; > val |= (TCR_WIE|TCR_WRC(WRC_CHIP)|WDTP(booke_wdt_period)); > > -#ifdef CONFIG_PPC_BOOK3E_64 > - /* > - * Crit ints are currently broken on PPC64 Book-E, so > - * just disable them for now. > - */ > - val &= ~TCR_WIE; > -#endif > - > mtspr(SPRN_TCR, val); > } > Patch has been added to linux-watchdog-next. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] watchdog: mpc8xxx_wdt: MPC8xx is HW enabled
Hi Christophe,
> MPC8xx watchdog is enabled at startup by HW.
> If the bootloader disables it, it cannot be reenabled.
>
> Signed-off-by: Christophe Leroy
>
> diff -ur a/drivers/watchdog/mpc8xxx_wdt.c b/drivers/watchdog/mpc8xxx_wdt.c
> --- a/drivers/watchdog/mpc8xxx_wdt.c 2013-05-11 22:57:46.0 +0200
> +++ b/drivers/watchdog/mpc8xxx_wdt.c 2013-08-08 02:12:15.0 +0200
> @@ -273,6 +310,7 @@
> .compatible = "fsl,mpc823-wdt",
> .data = &(struct mpc8xxx_wdt_type) {
> .prescaler = 0x800,
> + .hw_enabled = true,
> },
> },
> {},
>
> ---
> Ce courrier électronique ne contient aucun virus ou logiciel malveillant
> parce que la protection avast! Antivirus est active.
> http://www.avast.com
>
This patch has been added to linux-watchdog-next.
Kind regards,
Wim.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3] watchdog: mpc8xxx_wdt convert to watchdog core
Hi Christophe, > Convert mpc8xxx_wdt.c to the new watchdog API. > > Signed-off-by: Christophe Leroy This patch has been added to linux-watchdog-next. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Enhanced support for MPC8xx/8xxx watchdog
Hi Christophe,
> This patch modifies the behaviour of the MPC8xx/8xxx watchdog. On the MPC8xx,
> at 133Mhz, the maximum timeout of the watchdog timer is 1s, which means it
> must
> be pinged twice a second. This is not in line with the Linux watchdog concept
> which is based on a default watchdog timeout around 60s.
> This patch introduces an intermediate layer between the CPU and the userspace.
> The kernel pings the watchdog at the required frequency at the condition that
> userspace tools refresh it regularly.
> The new parameter 'heartbeat' allows to set up the userspace timeout.
> The driver also implements the WDIOC_SETTIMEOUT ioctl.
No, no, no... we should standardise on naming. heartbeat should be the thing
that
actually keeps the dog alive, whereas timeout is the userspace timeout that the
user
can play with...
> Signed-off-by: Christophe Leroy
>
> diff -ur linux-3.7.7/drivers/watchdog/mpc8xxx_wdt.c
> linux/drivers/watchdog/mpc8xxx_wdt.c
> --- linux-3.7.7/drivers/watchdog/mpc8xxx_wdt.c2013-02-11
> 18:05:09.0 +0100
> +++ linux/drivers/watchdog/mpc8xxx_wdt.c 2013-02-13 15:55:22.0
> +0100
> @@ -52,10 +52,17 @@
> static struct mpc8xxx_wdt __iomem *wd_base;
> static int mpc8xxx_wdt_init_late(void);
>
> +#define WD_TIMO 10 /* Default heartbeat = 10 seconds */
> +
> +static int heartbeat = WD_TIMO;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat,
> + "Watchdog SW heartbeat in seconds. (0 < heartbeat < 65536s, default="
> + __MODULE_STRING(WD_TIMO) "s)");
and the userspace timeout should be an unsigned int to be honoust.
> static u16 timeout = 0x;
> module_param(timeout, ushort, 0);
> MODULE_PARM_DESC(timeout,
> - "Watchdog timeout in ticks. (0 + "Watchdog HW timeout in ticks. (0
> static bool reset = 1;
> module_param(reset, bool, 0);
> @@ -74,8 +81,10 @@
> static int prescale = 1;
> static unsigned int timeout_sec;
>
> +static int wdt_auto = 1;
> static unsigned long wdt_is_open;
> static DEFINE_SPINLOCK(wdt_spinlock);
> +static unsigned long wdt_last_ping;
>
> static void mpc8xxx_wdt_keepalive(void)
> {
> @@ -91,9 +100,20 @@
>
> static void mpc8xxx_wdt_timer_ping(unsigned long arg)
> {
> - mpc8xxx_wdt_keepalive();
> - /* We're pinging it twice faster than needed, just to be sure. */
> - mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
> + if (wdt_auto)
> + wdt_last_ping = jiffies;
> +
> + if (jiffies - wdt_last_ping <= heartbeat * HZ) {
> + mpc8xxx_wdt_keepalive();
> + /* We're pinging it twice faster than needed, to be sure. */
> + mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
> + }
> +}
> +
> +static void mpc8xxx_wdt_sw_keepalive(void)
> +{
> + wdt_last_ping = jiffies;
> + mpc8xxx_wdt_timer_ping(0);
> }
>
> static void mpc8xxx_wdt_pr_warn(const char *msg)
> @@ -106,7 +126,7 @@
>size_t count, loff_t *ppos)
> {
> if (count)
> - mpc8xxx_wdt_keepalive();
> + mpc8xxx_wdt_sw_keepalive();
> return count;
> }
>
> @@ -130,7 +150,7 @@
>
> out_be32(&wd_base->swcrr, tmp);
>
> - del_timer_sync(&wdt_timer);
> + wdt_auto = 0;
>
> return nonseekable_open(inode, file);
> }
> @@ -138,7 +158,8 @@
> static int mpc8xxx_wdt_release(struct inode *inode, struct file *file)
> {
> if (!nowayout)
> - mpc8xxx_wdt_timer_ping(0);
> + wdt_auto = 1;
> +
> else
> mpc8xxx_wdt_pr_warn("watchdog closed");
> clear_bit(0, &wdt_is_open);
> @@ -163,10 +184,12 @@
> case WDIOC_GETBOOTSTATUS:
> return put_user(0, p);
> case WDIOC_KEEPALIVE:
> - mpc8xxx_wdt_keepalive();
> + mpc8xxx_wdt_sw_keepalive();
> return 0;
> case WDIOC_GETTIMEOUT:
> - return put_user(timeout_sec, p);
> + return put_user(heartbeat, p);
> + case WDIOC_SETTIMEOUT:
> + return get_user(heartbeat, p);
> default:
> return -ENOTTY;
> }
> @@ -215,6 +238,8 @@
> ret = -ENOSYS;
> goto err_unmap;
> }
> + if (enabled)
> + timeout = in_be32(&wd_base->swcrr) >> 16;
>
> /* Calculate the timeout in seconds */
> if (prescale)
> @@ -273,6 +298,7 @@
> .compatible = "fsl,mpc823-wdt",
> .data = &(struct mpc8xxx_wdt_type) {
> .prescaler = 0x800,
> + .hw_enabled = true,
> },
> },
> {},
The rest of the code is OK and when above comments are corrected,
I will add the patch to improve the userspace experience.
Kind regards,
Wim.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 17/25] watchdog/mpc8xxx: add a const qualifier
Hi Uwe, > From: Arnd Bergmann > > This prepares *of_device_id.data becoming const. Without this change > the following warning would occur: > > drivers/watchdog/mpc8xxx_wdt.c: In function 'mpc8xxx_wdt_probe': > drivers/watchdog/mpc8xxx_wdt.c:203:11: warning: assignment discards > 'const' qualifier from pointer target type [enabled by default] > > Signed-off-by: Arnd Bergmann > [ukl: split Arnd's patch by driver and add changelog] > Signed-off-by: Uwe Kleine-König Acked-by: Wim Van Sebroeck Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] [v2] watchdog: booke_wdt: clean up status messages
Hi Kumar, I allready had it in the watchdog tree for a couple of weeks (and thus also in linux-next). Are you going to sent it of to Linus or will I sent it over via the watchdog tree? Thanks in advance, Wim. > > On Feb 8, 2011, at 5:39 PM, Timur Tabi wrote: > > > Improve the status messages that are displayed during some operations of the > > PowerPC watchdog timer driver. When the watchdog is enabled, the timeout is > > displayed as a number of seconds, instead of an obscure "period". The > > "period" > > is the position of a bit in a 64-bit timer register. The higher the value, > > the quicker the watchdog timeout occurs. Some people chose a high "period" > > value for the timer and get confused as to why the board resets within a > > few seconds. > > > > Messages displayed during open and close are now debug messages, so that > > they > > don't clutter the console by default. Finally, printk() is replaced with > > the > > pr_xxx() equivalent. > > > > Signed-off-by: Timur Tabi > > --- > > drivers/watchdog/booke_wdt.c | 19 +-- > > 1 files changed, 9 insertions(+), 10 deletions(-) > > applied > > - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] booke_wdt: Fix build (unconstify watchdog_info)
Hi Ben,
> > commit 42747d712de56cf2087b702d2ad90af114c53138 ("[WATCHDOG]
> > watchdog_info constify") introduced the following build failure:
> >
> >CC booke_wdt.o
> > booke_wdt.c: In function 'booke_wdt_init':
> > booke_wdt.c:220: error: assignment of read-only variable 'ident'
> >
> > Fix this by removing 'const' qualifier from watchdog_info struct.
>
> Should this go via powerpc.git ? If yes, I'll send to Linus asap.
I'm sending watchdog related patches to Linus today. This one will be included.
Kind regards,
Wim.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC] [PATCH] watchdog_info separation and constify
Hi Alan, > > please comment on following patch. > > Why move them out - why not just make them const ? There's 2 options: 1) we only make them const now. And we move them out later when we do the conversion to the generic watchdog api (which means that we will rip out the code for the open, release, write and ioctl handling of /dev/watchdog) or 2) we make them const and move them out of ioctl and then the ripping out of the code is much easier. I opted for the second option. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC] [PATCH] watchdog_info separation and constify
Hi All,
please comment on following patch.
Kind regards,
Wim.
commit 88d0b1a9c071d26e7b4831320067c84b04ea04a8
Author: Wim Van Sebroeck
Date: Sat Dec 26 18:55:22 2009 +
[WATCHDOG] watchdog_info separation and constify
make sure that the watchdog_info struct is seperated from the ioctl code.
Also make the struct const where possible.
Signed-off-by: Wim Van Sebroeck
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index 6f8ebe1..072b948 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -553,7 +553,7 @@ static ssize_t mpc52xx_wdt_write(struct file *file, const
char __user *data,
return 0;
}
-static struct watchdog_info mpc5200_wdt_info = {
+static const struct watchdog_info mpc5200_wdt_info = {
.options= WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
.identity = WDT_IDENTITY,
};
diff --git a/drivers/hwmon/fschmd.c b/drivers/hwmon/fschmd.c
index bd0fc67..878284b 100644
--- a/drivers/hwmon/fschmd.c
+++ b/drivers/hwmon/fschmd.c
@@ -845,14 +845,15 @@ static ssize_t watchdog_write(struct file *filp, const
char __user *buf,
return count;
}
+static struct watchdog_info ident = {
+ .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
+ WDIOF_CARDRESET,
+ .identity = "FSC watchdog"
+};
+
static int watchdog_ioctl(struct inode *inode, struct file *filp,
unsigned int cmd, unsigned long arg)
{
- static struct watchdog_info ident = {
- .options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
- WDIOF_CARDRESET,
- .identity = "FSC watchdog"
- };
int i, ret = 0;
struct fschmd_data *data = filp->private_data;
diff --git a/drivers/watchdog/acquirewdt.c b/drivers/watchdog/acquirewdt.c
index 4d18c87..f0c956f 100644
--- a/drivers/watchdog/acquirewdt.c
+++ b/drivers/watchdog/acquirewdt.c
@@ -145,16 +145,17 @@ static ssize_t acq_write(struct file *file, const char
__user *buf,
return count;
}
+static const struct watchdog_info ident = {
+ .options = WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
+ .firmware_version = 1,
+ .identity = WATCHDOG_NAME,
+};
+
static long acq_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
int options, retval = -EINVAL;
void __user *argp = (void __user *)arg;
int __user *p = argp;
- static struct watchdog_info ident = {
- .options = WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
- .firmware_version = 1,
- .identity = WATCHDOG_NAME,
- };
switch (cmd) {
case WDIOC_GETSUPPORT:
diff --git a/drivers/watchdog/advantechwdt.c b/drivers/watchdog/advantechwdt.c
index 824d076..3a9f1df 100644
--- a/drivers/watchdog/advantechwdt.c
+++ b/drivers/watchdog/advantechwdt.c
@@ -132,18 +132,19 @@ static ssize_t advwdt_write(struct file *file, const char
__user *buf,
return count;
}
+static const struct watchdog_info ident = {
+ .options = WDIOF_KEEPALIVEPING |
+ WDIOF_SETTIMEOUT |
+ WDIOF_MAGICCLOSE,
+ .firmware_version = 1,
+ .identity = WATCHDOG_NAME,
+};
+
static long advwdt_ioctl(struct file *file, unsigned int cmd, unsigned long
arg)
{
int new_timeout;
void __user *argp = (void __user *)arg;
int __user *p = argp;
- static struct watchdog_info ident = {
- .options = WDIOF_KEEPALIVEPING |
- WDIOF_SETTIMEOUT |
- WDIOF_MAGICCLOSE,
- .firmware_version = 1,
- .identity = WATCHDOG_NAME,
- };
switch (cmd) {
case WDIOC_GETSUPPORT:
diff --git a/drivers/watchdog/adx_wdt.c b/drivers/watchdog/adx_wdt.c
index 9d7d155..a5ca7a6 100644
--- a/drivers/watchdog/adx_wdt.c
+++ b/drivers/watchdog/adx_wdt.c
@@ -37,7 +37,7 @@ struct adx_wdt {
spinlock_t lock;
};
-static struct watchdog_info adx_wdt_info = {
+static const struct watchdog_info adx_wdt_info = {
.identity = "Avionic Design Xanthos Watchdog",
.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING,
};
diff --git a/drivers/watchdog/alim1535_wdt.c b/drivers/watchdog/alim1535_wdt.c
index 937a80f..d5225ad 100644
--- a/drivers/watchdog/alim1535_wdt.c
+++ b/drivers/watchdog/alim1535_wdt.c
@@ -176,17 +176,18 @@ static ssize_t ali_write(struct file *file, const char
__user *data,
* we want an extension to enable irq ack monitoring and the like
*/
+static const struct watchdog_info ident = {
+ .options = WDIOF_KEEPALIVEPING |
+ WDIOF_SETTIMEOUT |
+ WDIOF_MAGICCLOSE,
+ .firmware_version = 0,
+ .identity = "ALi M1535 WatchDog Timer",
+};
+
static long al
Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
Hi Grant, Sorry for the delay, I need to deliver a project in a weeks time... > In actual fact, it is a single device with multiple functions, some of > which can be used at the same time. The driver for the device > determines what the device instance supports and registers the > appropriate interfaces. There is a GPIO controller, a PWM controller, > a general purpose timer, and the watchdog. Because of the > multifunction nature of the device, there are subtle interactions > between the functions that the driver needs to maintain. I don't want > to export functions from the driver which will only be used by a > watchdog instance. I also don't want the added code and complexity of > a secondary probe path. It is simpler and less code to roll all the > behaviour up into the one driver single driver that gets probed once. > > >From the maintenance perspective, having the main driver in > arch/powerpc and the watchdog bit in drivers/watchdog doesn't really > help much anyway because anything that changes the internal driver API > (between the core and watchdog bits) will require cross-maintainer > changes. ie. do changes go through my tree because they touch > arch/powerpc, or do they go through yours because they touch > drivers/watchdog? I'd much rather all the internal details be > contained within a single driver. Your argument about maintenance is the same one as I have: If all watchdog driver pieces are under drivers/watchdog then it's easier for me to maintain them. (Definitely if we are doing clean-up work and API changes). > Besides, there is already precedence for very arch-specific drivers > living under arch/*/. find ./arch -name *gpio* But in this case: I know where to find them and I will keep a mental note about this one. And yes indeed some very arch specific drivers can reside under arch/*/* . So please go ahead with pulling this one in a single driver. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] mpc52xx/wdt: WDT uses GPT api
Hi All, >> Can the WDT functionality just be merged entirely into >> arch/powerpc/platforms/52xx/mpc52xx_gpt.c, eliminating the need for >> this file entirely? I think I'd rather have all the GPT "built in" >> behaviour handled by a single driver. > > I also thought about it, as it has IMHO the cleaner code, and it would have > the extra benefit that the gpt-wdt api doesn't need to be public. > > However, the reasons I hesitated to do so are: > - I don't want to remove a file someone else wrote (even it doesn't work); > - WDT code is shifted from drivers/watchdog to arch/powerpc/platforms/52xx > which might not be the "logical" place from the directory layout's pov; > - a file living in arch/powerpc/platforms/52xx depends upon config options > set from drivers/watchdog/Kconfig which may be confusing. > > You see these are more political/cosmetical questions, so I would prefer to > leave the decision to the maintainers (i.e. you and Wim). Preparing a fully > merged driver is actually a matter of minutes! My opinion: it is harder to maintain the watchdog code if it is being moved away from drivers/watchdog. I need to check the code before I comment any further on this, but my first thought is: why don't you do it with platform resources like other devices are doing? This way you can keep the platform code under arch and the watchdog itself under drivers/watchdog/. You can look at the following drivers as an example: adx_wdt.c ar7_wdt.c at32ap700x_wdt.c coh901327_wdt.c davinci_wdt.c mpcore_wdt.c mv64x60_wdt.c nuc900_wdt.c omap_wdt.c pnx4008_wdt.c rc32434_wdt.c s3c2410_wdt.c txx9wdt.c . Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fix book E watchdog to take WDIOC_SETTIMEOUT arg in seconds
Hi Josh, > >Now we only need someone that can look at the CONFIG_4xx cases still :-) > > It seems the FSL watchdog is much more flexible than the one found in 4xx > cores. On 4xx, you basically have 4 static choices that represent specific > times determined by the clock frequency. I'm concerned that the lack of > granularity here will result in less than desirable behavior. > > For example, with a 400MHz clock you would get choices of roughly: > > 5.2 ms > 83.9 ms > 1.34 s > 21.47 s > > Personally, I consider the first two options basically unusable. Considering > the second two, if a user were to say "Set the timeout for 2 seconds" they > would then get a timeout of 21 seconds with the framework Chris' patch has > set up. That doesn't really seem to be ideal to me. Hmm, my opinion: in that case we should use a timer that triggers the watchdog until userspaces times out (like we do for other watchdogs allready). Maybe we should split this driver. I have the same issue with the Freescale i.mx driver that is under review also. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] fix book E watchdog to take WDIOC_SETTIMEOUT arg in seconds
Hi Chris, > The WDIOC_SETTIMEOUT argument is supposed to be a "seconds" value. > However, the book E wdt currently treats it as a "period" which is > interpreted in a board-specific way. > > This patch allows the user to pass in a "seconds" value and the driver > will set the smallest timeout that is at least as large as specified > by the user. It's been tested on e500 hardware and works as > expected. > > The patch only modifies the CONFIG_FSL_BOOKE case, the CONFIG_4xx case > is left unmodified as I don't have any hardware to test it on. > > Signed-off-by: Chris Friesen Added with some small changes to keep checkpatch happy. (removed trailing spaces + changed sizeof(struct watchdog_info) to sizeof(ident) and changed some spaces in tabs). Now we only need someone that can look at the CONFIG_4xx cases still :-) Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 1/2] powerpc/86xx: Board support for GE Fanuc's PPC9A
Hi Kumar, >>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig >>> index c7352f7..ecf52e4 100644 >>> --- a/drivers/watchdog/Kconfig >>> +++ b/drivers/watchdog/Kconfig >>> @@ -772,7 +772,7 @@ config TXX9_WDT >>> >>> config GEF_WDT >>> tristate "GE Fanuc Watchdog Timer" >>> - depends on GEF_SBC610 || GEF_SBC310 >>> + depends on GEF_SBC610 || GEF_SBC310 || GEF_PPC9A >>> ---help--- >>> Watchdog timer found in a number of GE Fanuc single board >>> computers. >> >> This part is Signed-off-by me. The rest I leave up to the powerpc >> maintainers. >> >> Kind regards, >> Wim. > > Ok w/me adding an actually signed-of-by for you when I apply this? Yes, please add a signed-off-by for me when you apply this patch. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v3 1/2] powerpc/86xx: Board support for GE Fanuc's PPC9A
Hi Martyn, > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index c7352f7..ecf52e4 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -772,7 +772,7 @@ config TXX9_WDT > > config GEF_WDT > tristate "GE Fanuc Watchdog Timer" > - depends on GEF_SBC610 || GEF_SBC310 > + depends on GEF_SBC610 || GEF_SBC310 || GEF_PPC9A > ---help--- > Watchdog timer found in a number of GE Fanuc single board computers. This part is Signed-off-by me. The rest I leave up to the powerpc maintainers. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] watchdog: Basic support for GE Fanuc's FPGA based watchdog timer
Hi Kumar, > >the second patch is also in. Do we keep in in the watchdog tree > >since it almost doesn't > >touch the powerpc arch or do you want it in your tree? > > If you have it in your tree to pull go ahead and send it via > watchdog. (I assume this will be for 2.6.29) Ok. Will do it like that. And yes it is for 2.6.29. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] watchdog: Basic support for GE Fanuc's FPGA based watchdog timer
Hi All, > Any status of acceptance of this? I changed the ioctl to an unlocked ioctl and removed the temperature ioctl call. It's now in my linux-2.6-watchdog-next tree. Can someone verify that it still works? (http://git.kernel.org/?p=linux/kernel/git/wim/linux-2.6-watchdog-next.git;a=summary) Kumar, the second patch is also in. Do we keep in in the watchdog tree since it almost doesn't touch the powerpc arch or do you want it in your tree? Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] watchdog: Basic support for GE Fanuc's FPGA based watchdog timer
Hi Kumar, > >drivers/watchdog/Kconfig |6 + > >drivers/watchdog/Makefile |1 > >drivers/watchdog/gef_wdt.c | 333 +++ > >+ > >3 files changed, 340 insertions(+), 0 deletions(-) > > Wim, > > Any status of acceptance of this? I started reviewing it on sunday. It looks OK so you can expect inclusion in my -next tree this week. A couple of days later I would like to forward it to Linus for mainline inclusion. It would like to have it in 2.6.29. Kind regards, Wim. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
