Thread (1 message) 1 message, 1 author, 2010-03-21
DORMANTno replies

[PATCH 1/3] watchdog: Add support for the Freescale MXC watchdog

From: Vladimir Zapolskiy <hidden>
Date: 2010-03-21 18:27:08

Wolfram Sang [off-list ref] writes:

Hi,
Hi Vladimir,

On Sat, Mar 20, 2010 at 09:40:07PM +0300, Vladimir Zapolskiy wrote:
quoted
The driver is extremely simple, so from my biased position only minor
benefits can be found in my version:
* introduced spinlock to protect concurrent write to registers
* SETTIMEOUT option is present and it works well on imx31
* correct zero byte write()
* clock enabled only when watchdog node is opened
* dynamic wdt structure, which potentially simplifies future support of
  several watchdogs found on imx51 and imx37 IIRC
* no critical message on close with unset NOWAYOUT on non-imx1 SoCs

Your pretty good version supports imx1, and I cann't test my version on
imx1, because I don't have such hardware on hand.

Obviously better to update your reviewed one, and I hope some comments or even
updates from my side could be accepted by you :)
Thanks for agreeing on the procedure and pointing out the benefits of your
driver. I will surely have a look at them and don't be surprised if you will
find this or that incorporated ;) I haven't really started yet, so I might be
missing something: What races do you want to protect against with the spinlock?
The ping?
locking was added in analogy with other watchdog drivers. Just checked
imx1, imx3 and imx5 reference manuals, for all chips "any number of
instructions can be executed between two writes" to WSR. I understand
this that only ping/ping races can influence the watchdog, and that kind
of race does not look harmful.

So the locking could be omitted.

With best wishes,
Vladimir
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help