Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH] watchdog device drivers:pc87413_wdt: Rewriting of pc87413_wdt driver to utilize common watchdog interface (fwd)
@ 2019-07-29 23:17 Mark Balantzyan
  2019-07-30  0:26 ` Guenter Roeck
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Balantzyan @ 2019-07-29 23:17 UTC (permalink / raw)
  To: linux; +Cc: linux-kernel, linux-watchdog, wim

[-- Attachment #1: Type: text/plain, Size: 630 bytes --]



---------- Forwarded message ----------
Hi all, sorry for the duplicate message Guenter, wanted to be sure my
message is transferred:

Thank you for your reply, Guenter! Sorry there were issues applying the
patch, I used git format-patch to produce the patch and pasted the main
contents into a plaintext email client so I thought it would work..

May I please request clarification on which functions are no longer needed?

Sorry about forgetting about that last misc_deregister(). Will do more
tests, if that’s alright with you.

In effect, may it be best to start the watchdog from the “init” function?

Thank you,
Mark

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog device drivers:pc87413_wdt: Rewriting of pc87413_wdt driver to utilize common watchdog interface (fwd)
  2019-07-29 23:17 [PATCH] watchdog device drivers:pc87413_wdt: Rewriting of pc87413_wdt driver to utilize common watchdog interface (fwd) Mark Balantzyan
@ 2019-07-30  0:26 ` Guenter Roeck
  2019-07-30  0:44   ` Mark Balantzyan
  0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2019-07-30  0:26 UTC (permalink / raw)
  To: Mark Balantzyan; +Cc: linux-kernel, linux-watchdog, wim

On 7/29/19 4:17 PM, Mark Balantzyan wrote:
> 
> 
> ---------- Forwarded message ----------
> Hi all, sorry for the duplicate message Guenter, wanted to be sure my
> message is transferred:
> 
> Thank you for your reply, Guenter! Sorry there were issues applying the
> patch, I used git format-patch to produce the patch and pasted the main
> contents into a plaintext email client so I thought it would work..
> 
> May I please request clarification on which functions are no longer needed?
> 

All functions dealing with file accesses directly. open, read, write, ioctl.
Don't you see that when you compile the code ?

> Sorry about forgetting about that last misc_deregister(). Will do more
> tests, if that’s alright with you.
> 
> In effect, may it be best to start the watchdog from the “init” function?
> 
The watchdog should only be started from its start function, when it is
opened. There are alternatives, such as informing the watchdog core that
the watchdog has been started even though it is not closed.

I am repeating myself here, since I don't recall an answer: Do you have
the hardware ? If not, it does not make sense to continue this work; it
is too risky to make all those changes without testing them on real
hardware (or, at the very least, with qemu, but qemu doesn't support
this chip).

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog device drivers:pc87413_wdt: Rewriting of pc87413_wdt driver to utilize common watchdog interface (fwd)
  2019-07-30  0:26 ` Guenter Roeck
@ 2019-07-30  0:44   ` Mark Balantzyan
  2019-07-30  1:30     ` Guenter Roeck
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Balantzyan @ 2019-07-30  0:44 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Mark Balantzyan, linux-kernel, linux-watchdog, wim

Hello all, Guenter,

I am being evaluated as a student by my organization. I appreciate your 
patience with my emails and patches.

I would like to please propose that we divide and conquer: I write the 
code for converting the driver to common watchdog interface (and I thank 
you for your guidance on it in previous email) and you test the code on 
the actual hardware you may happen to have, as I do not have it. I accept 
the fact that it is indeed risky without the hardware to ensure the driver 
works correctly, but that will be where we work in tandem, software to 
hardware, yes, if that's alright?

I think it's better if I use git send-email for the corresponding patch 
with the improvements you forecasted since it may format things better and 
may result in a non-corrupted patching.

Thank you,
Mark


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog device drivers:pc87413_wdt: Rewriting of pc87413_wdt driver to utilize common watchdog interface (fwd)
  2019-07-30  0:44   ` Mark Balantzyan
@ 2019-07-30  1:30     ` Guenter Roeck
  2019-07-30  2:21       ` Mark Balantzyan
  0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2019-07-30  1:30 UTC (permalink / raw)
  To: Mark Balantzyan; +Cc: linux-kernel, linux-watchdog, wim

Mark,

On 7/29/19 5:44 PM, Mark Balantzyan wrote:
> Hello all, Guenter,
> 
> I am being evaluated as a student by my organization. I appreciate your patience with my emails and patches.
> 
> I would like to please propose that we divide and conquer: I write the code for converting the driver to common watchdog interface (and I thank you for your guidance on it in previous email) and you test the code on the actual hardware you may happen to have, as I do not have it. I accept the fact that it is indeed risky without the hardware to ensure the driver works correctly, but that will be where we work in tandem, software to hardware, yes, if that's alright?
> 

If I had the hardware, I would say so. Then we would have no problem,
I would thank you for your work, and I would be more than happy to test
your patch on that hardware.

Unfortunately, I don't have the hardware, and I don't know anyone who (still)
has. The most recent functional change to this driver was made back in 2011.

If the task is to convert a watchdog driver, does it have to be _this_ driver ?
One that comes into mind instead would be ib700, which looks to be quite similar
and which _is_ supported by qemu. I have not tried if its qemu emulation
actually works, but it would be worth a try.

The only alternative I can see, if it has to be _this_ driver for some reason,
is for us to go through with it and then let the patch hang around until we
find someone to test it (which may be never). I am not really sure if that
would be worth our time.

> I think it's better if I use git send-email for the corresponding patch with the improvements you forecasted since it may format things better and may result in a non-corrupted patching.
> 
Most definitely, yes.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog device drivers:pc87413_wdt: Rewriting of pc87413_wdt driver to utilize common watchdog interface (fwd)
  2019-07-30  1:30     ` Guenter Roeck
@ 2019-07-30  2:21       ` Mark Balantzyan
  0 siblings, 0 replies; 5+ messages in thread
From: Mark Balantzyan @ 2019-07-30  2:21 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Mark Balantzyan, linux-kernel, linux-watchdog, wim

Hi all, Guenter,

Sure, it'd be great to work on ib700, doing both, if we may. I feel it's 
worth a shot in case somebody out there has the hardware to test the 
pc87413_wdt driver, though I'm doing my best building the individual 
module and checking for compilation errors (as best I can).

I just sent off via git send-email a quad-chain of patches for the driver.

Thanks + regards,
Mark


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-29 23:17 [PATCH] watchdog device drivers:pc87413_wdt: Rewriting of pc87413_wdt driver to utilize common watchdog interface (fwd) Mark Balantzyan
2019-07-30  0:26 ` Guenter Roeck
2019-07-30  0:44   ` Mark Balantzyan
2019-07-30  1:30     ` Guenter Roeck
2019-07-30  2:21       ` Mark Balantzyan

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org linux-watchdog@archiver.kernel.org
	public-inbox-index linux-watchdog


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/ public-inbox