All of lore.kernel.org
 help / color / mirror / Atom feed
From: Timur Tabi <timur@freescale.com>
To: Josh Boyer <jwboyer@linux.vnet.ibm.com>
Cc: <linux-watchdog@vger.kernel.org>, <kumar.gala@freescale.com>,
	<linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH] watchdog: add CONFIG_WATCHDOG_NOWAYOUT support to PowerPC Book-E watchdog driver
Date: Fri, 3 Dec 2010 12:22:30 -0600	[thread overview]
Message-ID: <4CF93566.1050601@freescale.com> (raw)
In-Reply-To: <20101203180751.GA1903@zod.rchland.ibm.com>

Josh Boyer wrote:
> I'm confused why you can't use booke_wdt_enabled for the purposes of the
> device having been opened.  It seems the use of the wdt_is_active
> basically duplicates this functionalit (and oddly with the bit
> manipulation instead of just atomic_inc/dec).

Because the watchdog can be enabled even when the driver is not open.
booke_wdt_enabled is also initialized in setup_32.c.  So booke_wdt_enabled
represents the watchdog hardwre, whereas wdt_is_active represents the open
condition of /dev/watchdog.

However, now that I think about it, maybe that just causes confusion.  If the
watchdog is already running because of a command-line parameter, should we
prevent /dev/watchdog from ever being opened?  If you're okay with that, then I
can combine the two variables.

> If you were to keep this variable instead of just using
> booke_wdt_enabled, wouldn't it be more correct to have the clear_bit
> only done inside the #ifndef?  The timer is very much still active if
> NOWAYOUT is set...

In this case, yes.

-- 
Timur Tabi
Linux kernel developer at Freescale


WARNING: multiple messages have this Message-ID (diff)
From: Timur Tabi <timur@freescale.com>
To: Josh Boyer <jwboyer@linux.vnet.ibm.com>
Cc: linuxppc-dev@ozlabs.org, kumar.gala@freescale.com,
	linux-watchdog@vger.kernel.org
Subject: Re: [PATCH] watchdog: add CONFIG_WATCHDOG_NOWAYOUT support to PowerPC Book-E watchdog driver
Date: Fri, 3 Dec 2010 12:22:30 -0600	[thread overview]
Message-ID: <4CF93566.1050601@freescale.com> (raw)
In-Reply-To: <20101203180751.GA1903@zod.rchland.ibm.com>

Josh Boyer wrote:
> I'm confused why you can't use booke_wdt_enabled for the purposes of the
> device having been opened.  It seems the use of the wdt_is_active
> basically duplicates this functionalit (and oddly with the bit
> manipulation instead of just atomic_inc/dec).

Because the watchdog can be enabled even when the driver is not open.
booke_wdt_enabled is also initialized in setup_32.c.  So booke_wdt_enabled
represents the watchdog hardwre, whereas wdt_is_active represents the open
condition of /dev/watchdog.

However, now that I think about it, maybe that just causes confusion.  If the
watchdog is already running because of a command-line parameter, should we
prevent /dev/watchdog from ever being opened?  If you're okay with that, then I
can combine the two variables.

> If you were to keep this variable instead of just using
> booke_wdt_enabled, wouldn't it be more correct to have the clear_bit
> only done inside the #ifndef?  The timer is very much still active if
> NOWAYOUT is set...

In this case, yes.

-- 
Timur Tabi
Linux kernel developer at Freescale

  reply	other threads:[~2010-12-03 18:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-03 16:51 [PATCH] watchdog: add CONFIG_WATCHDOG_NOWAYOUT support to PowerPC Book-E watchdog driver Timur Tabi
2010-12-03 18:07 ` Josh Boyer
2010-12-03 18:07   ` Josh Boyer
2010-12-03 18:22   ` Timur Tabi [this message]
2010-12-03 18:22     ` Timur Tabi
2010-12-03 19:05     ` Josh Boyer
2010-12-03 19:05       ` Josh Boyer
2010-12-03 19:10       ` Timur Tabi
2010-12-03 19:10         ` Timur Tabi
2010-12-03 19:39         ` Josh Boyer
2010-12-03 19:39           ` Josh Boyer
2010-12-03 19:43           ` Timur Tabi
2010-12-03 19:43             ` Timur Tabi
2010-12-03 19:50             ` Josh Boyer
2010-12-03 19:50               ` Josh Boyer

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=4CF93566.1050601@freescale.com \
    --to=timur@freescale.com \
    --cc=jwboyer@linux.vnet.ibm.com \
    --cc=kumar.gala@freescale.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.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.