Hi,
On 03/15/2012 10:13 PM, Wim Van Sebroeck wrote:
Hi Hans,
Short selfintro for the watchdog people: I'm a FOSS developer / enthusiast
for close to 15 years now. I've written Linux kernel drivers for a ton
of webcams and various hwmon IC's.
Let me also give a short intro about myself: As a hobby I maintain the
linux-watchdog drivers since years now. I'm not paid by anyone to do that and
I'm doing this in my spare time. Apart from that I have a wive and 2 children
that also need attention. In real life I'm a freelancer (doing mostly network
and telecoms stuff) and that's how I try to survive.
I completely understand, my intend of the resend was not to complain, merely to
hopefully get some attention for it. FWIW despite my email address my kernel
contributions are mostly a hobby too.
If you then add the fact that on the 12th of February an upgrade of my home
system made sure that I lost a big part of my e-mail :-( and made sure that
I didn't had outgoing and local mail anymore :-( then you understand probably
why I have been struggling the last month... FYI: the last week I have been
busy working on my backlog of patches for the watchdog trees, before doing
anything else (like responding to e-mails).
Sorry to hear that, and I'm glad to see that you're back in business wrt you
email.
This is a resend of a series of watchdog_dev patches which I initially send
on Sep 12 2011:
http://lists.lm-sensors.org/pipermail/lm-sensors/2011-September/033707.html
And which unfortunately has seen 0 reviews / feedback since then.
I have been looking at these 5 patches last week. This is what I think of them:
patch 1: Use wddev function parameter instead of wdd global ->
was allready sent out by someone else also and thus has been incorporated
patch 2: watchdog_dev: Add support for having more then 1 watchdog ->
I am sure that we all agree that we need support for multiple watchdogs.
But I am not convinced we have the right approach for it yet.
I saw that you started looking at Alan's Code also.
Please continue the discussion about this.
I've reviewed Alan's patch and I agree that his approach is better, as soon as
he resends his patch with some (small) remarks from the review addressed. I'll
add my Reviewed-by.
The support for multiple watchdogs is not for this merge window, but for
the one after that.
Ok.
patch 3: watchdog_dev: Add support for dynamically allocated watchdog_device structs ->
Have to look in more detail on that patch, but I think this is a good thing to add.
I'm trying to get that one still in before the merge window opens (together
with the ep93xx_wdt conversion and the sp805 and mpcore patches from Viresh
(but no promisses here))
As I already said in my previous mail, please note that Viresh's patches for
mpcore run into the same issue I'm running into. To clarify the problem a bit,
it could be reproduced on a (hypothetical) mpcore arm system by:
Have an app open /dev/watchdog and keep that app running.
Then do the following as root:
cd /sys/bus/platform/drivers/mpcore_wdt
ls
You should see the name of the watchdog platform dev here
(as a symlink), something among the lines of: mpcore_wdt.XXXX
Now you can unbind the platform driver from the platform dev
doing the following:
cd /sys/bus/platform/drivers/mpcore_wdt
echo mpcore_wdt.XXXX > unbind
ls
With the ls the symlink should be gone. And /dev/watchdog
too, as the mpcore_wdt_remove function has now run! This
also means that the mpcore_wdt struct, which contains the
watchdog_device struct has been free-ed.
But the app which opened /dev/watchdog still has an open
fd to it, and when calls are made on that, watchdog_dev.c
will try to use file->private_data which now points
to free-ed memory!
I admit that triggering this is not easy and not something which
will happen accidentally, but still I believe this is something which
we ought to fix.
patch 4: watchdog_dev: Let the driver update the timeout field on set_timeout success ->
This patch is in linux-watchdog-next since yesterday (and it was in my testing tree for a week).
It's in linux-next since this morning together with fixes for the drivers that allready use the watchdog-api.
Great, thanks.
patch 5: hwmon/sch5636: Add support for the integrated watchdog ->
Depends off-course on patch 2.
It depends more on patch 3 then on 2. it will still work without patch 2,
but to test (and use it on some systems) I would need to blacklist the iTCO
module.
Also I've recently received information on how the watchdog in the sch5627
works, And it turns out to be exactly the same (*). So please drop this one
from your queue I'm working on a new version where most of the code is
shared between the sch5627 and sch5636 drivers (there already is a
sch56xx-common file used by both).
*) But it needs the BIOS to setup / poke some motherboard specific things,
so it only works on boards with BIOS support for the wd
Thanks & Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
