Thread (34 messages) 34 messages, 7 authors, 2015-05-13

RE: [PATCH V1 5/6] watchdog: da9062: DA9062 watchdog driver

From: Opensource [Steve Twiss] <hidden>
Date: 2015-05-06 16:34:14
Also in: linux-devicetree, linux-rtc, linux-watchdog, lkml

On 06 May 2015 17:02 Guenter Roeck wrote:
On Wed, May 06, 2015 at 02:54:37PM +0000, Opensource [Steve Twiss] wrote:
quoted
On 18 April 2015 16:53 Guenter Roeck wrote:

Hi Guenter,

Thanks for your comments.
quoted
On 04/17/2015 07:23 AM, S Twiss wrote:
quoted
From: S Twiss <redacted>

Add watchdog driver support for DA9062

Signed-off-by: Steve Twiss <redacted>
Hi Steve,

Key question here is if the da9062 is really so much different to the da9062
that you can not use the same driver.
The DA9062 watchdog driver does have some similarities with the DA9063 watchdog
base functionality -- however the watchdog component in the DA9062 chip has more
features yet to be added in software. I do intend to add these other features ...
however, if "not adding them here" is a problem I can drop the DA9062 watchdog
driver from this patch-set until I have time to write in the newer changes.
quoted
I am especially concerned about the added da9062_reset_watchdog_timer(),
given the delay it introduces.
After giving this some thought, I am going to remove this 300ms delay from the
reset_watchdog_timer() function for my next submission attempt. However
I am adding a 300ms delay into the stop() and update_timeout_register() functions
instead.

The DA9062 watchdog ping (register CONTROL_F) is "windowed" for protection
against spurious writes -- i.e. the ping function cannot be called within a 250ms
time limit or the PMIC will reset. This windowing protection also extends to altering
the timeout scale in the CONTROL_D register -- in which case if the timeout
register is altered and the ping() function is called within the 250ms limit, the
PMIC will reset. The delay is there to stop that from happening.

I realised my previous patch was over-sanitised: by putting the time delay into the
ping() function I was protecting CONTROL_D in stop() and update_timeout_register(),
but I was being too over-protective of the ping() function. Therefore if there was an
"incorrect trigger signal", the watchdog would not be allowed to fail because the
driver would have filtered out the errors.
Hi Steve,

From your description, it sounds like the protection is only necessary if there
was a previous write to the same register(s). If so, it might make sense to
record the time of such writes, and only add the delay if necessary, and only
for the remainder of the time.

Would this be possible ?
Hi Guenter,

I think so -- sounds like it should be possible.
Internally, there are several places where the two registers are written in succession.
Also, I'll have to re-write my tests in several places.

Probably the best solution would be to defer this watchdog driver for now, and I'll
re-submit it at a later date once the other parts of the DA9062 driver are [hopefully :)]
accepted. That way I can concentrate a solid block of time on the re-testing ... this
is the most time consuming.

Is that acceptable to you? -- I don't want to lose your existing comments from your
previous posts, so I will keep track of those changes you have already requested.

Regards,
Steve
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help