Re: power management and pseudo-devices
On Mon, 19 Jul 2010, Antti Kantee wrote:
Include "ioconf.h" I think.
Tried that. It works for compiling the kernel. Unfortunately,
swwdog is included in rump, and there doesn't seem to be an ioconf.h
available for the librump build.
Well, whatever. I don't think I want to look at that.
:)
Actually, I still think this is wrong. It might make librump compile
and even link, that doesn't mean it will be usable if nothing ever
creates that extern symbol. You'll have to check with pooka or explore
the code, but there have to be some components of rump that have partial
configuration files. After all, he created the "ioconf" directive for
that purpose.
First of all, let's take a step up from the trenches and try to understand
the problem we're dealing with instead of trying to arbitrarily guess how
to fix the build. We have two ways of building kernel code: monolithic
(./build.sh kernel=CONF) and modular (kernel modules, rump components).
The latter ones currently always do config stuff on-demand, so changes
cause breakage. Should a swwdog kernel module exist (why isn't there
one?), it would run into the same problem.
Now, the ioconf keyword for config(1) is meant to help build modular
kernel code by allowing to specify a partial config file. Currently,
as the name implies, it takes care of creating ioconf.[hc], namely in
this case struct cfdriver swwdog_cd.
Hmmm, I don't seem to see any mention of the ioconf keyword in the current
config(1) or config(5) man pages.
Adding a SYSMON.ioconf will solve the problem:
=== snip ===
ioconf sysmon
include "conf/files"
pseudo-device swwdog
=== snip ===
The good news is that if some day a sysmon kernel module is added,
the exact same ioconf can be used without having to once again run
into trouble.
Yay - it appears to work! Revised diffs are attached to this E-mail.
Then let's view the broader scale. I think acpibat is currently the
only kernel module using ioconf, and I haven't bothered converting others
since I have realized that the scope of ioconf was a little too narrow.
I'm planning to change the keyword to "module" and add support for source
files. This way the default build for every modular kernel component
goes through config, and we can avoid issues due to config changes.
IIRC I have the config(1) part for this done already, but being able to
use an autogenerated SRCS list requires some bsd.subdir.mk style advanced
make hackery which I haven't been in the mood for. Plus, there's of
course the mess with "file some.c foo | bar & (baz ^ xyzzy) & !!!frobnitz".
Finally, on the eternal "someone should" astral plane, someone should
fix the kernel build to consist of building a set of modules and linking
them together, so we don't have more than one way to skin a kernel.
I'm pretty sure "someone" != "me" :)
-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-Index: sys/rump/dev/lib/libsysmon/SYSMON.ioconf
===
RCS file: sys/rump/dev/lib/libsysmon/SYSMON.ioconf
diff -N sys/rump/dev/lib/libsysmon/SYSMON.ioconf
--- /dev/null 1 Jan 1970 00:00:00 -
+++ sys/rump/dev/lib/libsysmon/SYSMON.ioconf19 Jul 2010 12:50:54 -
@@ -0,0 +1,8 @@
+# $NetBSD$
+#
+
+ioconf swwdog
+
+include "conf/files"
+
+pseudo-device swwdog
Index: sys/rump/dev/lib/libsysmon/Makefile
===
RCS file: /cvsroot/src/sys/rump/dev/lib/libsysmon/Makefile,v
retrieving revision 1.2
diff -u -p -r1.2 Makefile
--- sys/rump/dev/lib/libsysmon/Makefile 16 Feb 2010 20:42:45 - 1.2
+++ sys/rump/dev/lib/libsysmon/Makefile 19 Jul 2010 12:50:54 -
@@ -4,6 +4,7 @@
.PATH: ${.CURDIR}/../../../../dev/sysmon
LIB= rumpdev_sysmon
+IOCONF=SYSMON.ioconf
SRCS= sysmon_taskq.c sysmon_power.c sysmon_envsys.c sysmon_envsys_events.c \
sysmon_envsys_tables.c sysmon_envsys_util.c sysmon_wdog.c sysmon.c \
Index: sys/dev/sysmon/files.sysmon
===
RCS file: /cvsroot/src/sys/dev/sysmon/files.sysmon,v
retrieving revision 1.11
diff -u -p -r1.11 files.sysmon
--- sys/dev/sysmon/files.sysmon 30 Jan 2010 21:55:28 - 1.11
+++ sys/dev/sysmon/files.sysmon 19 Jul 2010 12:50:54 -
@@ -18,5 +18,5 @@ file dev/sysmon/sysmon_wdog.csysmon_wdo
file dev/sysmon/sysmon.c sysmon_envsys | sysmon_wdog |
sysmon_power
-defpseudo swwdog: sysmon_wdog
+defpseudodev swwdog: sysmon_wdog
filedev/sysmon/swwdog.cswwdog
I
Re: power management and pseudo-devices
On Mon Jul 19 2010 at 01:28:42 +, Quentin Garnier wrote: > On Sun, Jul 18, 2010 at 05:31:42PM -0700, Paul Goyette wrote: > > On Mon, 19 Jul 2010, Quentin Garnier wrote: > > > > >>>Include "ioconf.h" I think. > > >> > > >>Tried that. It works for compiling the kernel. Unfortunately, > > >>swwdog is included in rump, and there doesn't seem to be an ioconf.h > > >>available for the librump build. > > > > > >Well, whatever. I don't think I want to look at that. > > > > :) > > Actually, I still think this is wrong. It might make librump compile > and even link, that doesn't mean it will be usable if nothing ever > creates that extern symbol. You'll have to check with pooka or explore > the code, but there have to be some components of rump that have partial > configuration files. After all, he created the "ioconf" directive for > that purpose. First of all, let's take a step up from the trenches and try to understand the problem we're dealing with instead of trying to arbitrarily guess how to fix the build. We have two ways of building kernel code: monolithic (./build.sh kernel=CONF) and modular (kernel modules, rump components). The latter ones currently always do config stuff on-demand, so changes cause breakage. Should a swwdog kernel module exist (why isn't there one?), it would run into the same problem. Now, the ioconf keyword for config(1) is meant to help build modular kernel code by allowing to specify a partial config file. Currently, as the name implies, it takes care of creating ioconf.[hc], namely in this case struct cfdriver swwdog_cd. Adding a SYSMON.ioconf will solve the problem: === snip === ioconf sysmon include "conf/files" pseudo-device swwdog === snip === The good news is that if some day a sysmon kernel module is added, the exact same ioconf can be used without having to once again run into trouble. Then let's view the broader scale. I think acpibat is currently the only kernel module using ioconf, and I haven't bothered converting others since I have realized that the scope of ioconf was a little too narrow. I'm planning to change the keyword to "module" and add support for source files. This way the default build for every modular kernel component goes through config, and we can avoid issues due to config changes. IIRC I have the config(1) part for this done already, but being able to use an autogenerated SRCS list requires some bsd.subdir.mk style advanced make hackery which I haven't been in the mood for. Plus, there's of course the mess with "file some.c foo | bar & (baz ^ xyzzy) & !!!frobnitz". Finally, on the eternal "someone should" astral plane, someone should fix the kernel build to consist of building a set of modules and linking them together, so we don't have more than one way to skin a kernel.
Re: power management and pseudo-devices
On Sun, Jul 18, 2010 at 05:31:42PM -0700, Paul Goyette wrote: > On Mon, 19 Jul 2010, Quentin Garnier wrote: > > >>>Include "ioconf.h" I think. > >> > >>Tried that. It works for compiling the kernel. Unfortunately, > >>swwdog is included in rump, and there doesn't seem to be an ioconf.h > >>available for the librump build. > > > >Well, whatever. I don't think I want to look at that. > > :) Actually, I still think this is wrong. It might make librump compile and even link, that doesn't mean it will be usable if nothing ever creates that extern symbol. You'll have to check with pooka or explore the code, but there have to be some components of rump that have partial configuration files. After all, he created the "ioconf" directive for that purpose. -- Quentin Garnier - c...@cubidou.net - c...@netbsd.org "See the look on my face from staying too long in one place [...] every time the morning breaks I know I'm closer to falling" KT Tunstall, Saving My Face, Drastic Fantastic, 2007. pgpm4e6nfBlwc.pgp Description: PGP signature
Re: power management and pseudo-devices
On Mon, 19 Jul 2010, Quentin Garnier wrote: Include "ioconf.h" I think. Tried that. It works for compiling the kernel. Unfortunately, swwdog is included in rump, and there doesn't seem to be an ioconf.h available for the librump build. Well, whatever. I don't think I want to look at that. :) While you're there, I guess you could make swwdog_reboot a sysctl node. Good point. Would this belong in the kern or machdep sysctl tree? I would set hw.swwdog.reboot. And I don't understand why you bother setting that up in swwdog_attach(), when we've just discussed there could only be one anyway (and assuming that there could be more than one, it seems to me that it would be plain overdesign to allow the setting to be set per-device, but that's just me). I figured we might relax the one-instance requirement in the future, and it doesn't seem to cost anything to plan ahead. But that's just me! :) -int swwdog_reboot = 0; /* set for panic instead of reboot */ +bool swwdog_reboot = 0;/* set for panic instead of reboot */ If you change its type to bool, you might as well change the value to false. Duh! Of course! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: power management and pseudo-devices
On Sun, Jul 18, 2010 at 04:52:58PM -0700, Paul Goyette wrote: [...] > >>+ > >>+extern struct cfdriver swwdog_cd; > > > >Include "ioconf.h" I think. > > Tried that. It works for compiling the kernel. Unfortunately, > swwdog is included in rump, and there doesn't seem to be an ioconf.h > available for the librump build. Well, whatever. I don't think I want to look at that. [...] > >Ok, so you should be aware here that you're changing the logic of that > >function, and you're only allowing for the creation of one swwdog(4). > > The limitation of only one swwdog(4) was deliberate, especially > since the original code had a "#define NSWWDOG 1" anyway! I missed that in the initial code. [...] > >While you're there, I guess you could make swwdog_reboot a sysctl node. > > Good point. Would this belong in the kern or machdep sysctl tree? I would set hw.swwdog.reboot. And I don't understand why you bother setting that up in swwdog_attach(), when we've just discussed there could only be one anyway (and assuming that there could be more than one, it seems to me that it would be plain overdesign to allow the setting to be set per-device, but that's just me). > -int swwdog_reboot = 0; /* set for panic instead of reboot */ > +bool swwdog_reboot = 0; /* set for panic instead of reboot */ If you change its type to bool, you might as well change the value to false. > Index: src/sys/dev/sysmon/files.sysmon > === > RCS file: /cvsroot/src/sys/dev/sysmon/files.sysmon,v > retrieving revision 1.11 > diff -u -p -r1.11 files.sysmon > --- src/sys/dev/sysmon/files.sysmon 30 Jan 2010 21:55:28 - 1.11 > +++ src/sys/dev/sysmon/files.sysmon 18 Jul 2010 23:21:06 - > @@ -18,5 +18,5 @@ filedev/sysmon/sysmon_wdog.csysmon_wdo > file dev/sysmon/sysmon.c sysmon_envsys | sysmon_wdog | > sysmon_power > > -defpseudo swwdog: sysmon_wdog > +defpseudodev swwdog: sysmon_wdog > filedev/sysmon/swwdog.cswwdog You know, I was looking at -5 sources, which have the needs-count flag there and therefore don't have the #define for NSWWDOG. Must be pooka's doing I guess; it's certainly better that way. -- Quentin Garnier - c...@cubidou.net - c...@netbsd.org "See the look on my face from staying too long in one place [...] every time the morning breaks I know I'm closer to falling" KT Tunstall, Saving My Face, Drastic Fantastic, 2007. pgp7q6VjWeClX.pgp Description: PGP signature
Re: power management and pseudo-devices
On Sun, 18 Jul 2010, Quentin Garnier wrote:
Not really objections, just a few comments.
+struct swwdog_softc *sc_swwdog = NULL;
That can't possibly be needed (and, if it was, you'd have to mutexify it
wouldn't you?).
The only purpose of this was to make sure we didn't "magically" get a
request to configure a second swwdog(4).
+
+extern struct cfdriver swwdog_cd;
Include "ioconf.h" I think.
Tried that. It works for compiling the kernel. Unfortunately, swwdog
is included in rump, and there doesn't seem to be an ioconf.h available
for the librump build.
+swwdogattach(int n)
{
- int i;
+ int err;
+ cfdata_t cf;
+
+ if (n > 1)
+ aprint_verbose("%s: ignoring count of %d\n",
+ swwdog_cd.cd_name, n);
+
+ err = config_cfattach_attach(swwdog_cd.cd_name, &swwdog_ca);
+ if (err) {
+ aprint_error("%s: couldn't register cfattach: %d\n",
+ swwdog_cd.cd_name, err);
+ config_cfdriver_detach(&swwdog_cd);
+ return;
+ }
+
+ cf = kmem_alloc(sizeof(struct cfdata), KM_NOSLEEP);
+ if (cf == NULL)
+ aprint_error("%s: couldn't allocate cfdata\n",
+ swwdog_cd.cd_name);
+ else {
+ cf->cf_name = swwdog_cd.cd_name;
+ cf->cf_atname = swwdog_cd.cd_name;
+ cf->cf_unit = 0;
+ cf->cf_fstate = FSTATE_STAR;
- for (i = 0; i < NSWWDOG; i++) {
- struct swwdog_softc *sc = &sc_wdog[i];
+ (void)config_attach_pseudo(cf);
Ok, so you should be aware here that you're changing the logic of that
function, and you're only allowing for the creation of one swwdog(4).
The limitation of only one swwdog(4) was deliberate, especially since
the original code had a "#define NSWWDOG 1" anyway! Furthermore, even
though you might have more than one watchdog provider, the whole code
was written to permit only one to be armed at any given time. So it
really doesn't make much sense to permit multiple swwdog(4)s.
That feature of the old code--implemented with needs-count no less,
brrr--is highly debatable, and the manual page actually says it's
pointless.
So yeah, go ahead and kill it (you can keep the __unused on int count by
the way), but change the manual page.
I will update the man page accordingly, and silently ignore the count.
Also, you don't have to allocate a struct cfdata, you can use a static
one (indepedently of the fact that you only use it once, by the way, all
the other pseudo-devices could be changed in that respect).
Yes, I "borrowed" that part from another driver - dev/pad/pad.c I will
make it a static item.
+static int
+swwdog_match(device_t parent, cfdata_t data, void *aux)
+{
+ /* Match unless we already have a swwdog */
+ if (sc_swwdog == NULL)
+ return 1;
+
+ return 0;
+}
What could possibly trigger another config_attach_pseudo? I know that
config(9) is special, but not magic :-)
Sometimes it feels like magic! :)
+ if (!pmf_device_register(self, NULL, NULL))
+ aprint_error_dev(self, "couldn't establish power "
+ "handler\n");
So what was your initial point about swwdog(4) and pmf(9), anyway?
HeHeHe - yeah, I actually forgot to put that little detail in in the
first round! The swwdog_suspend() routine looks like this in the
current code:
if (!pmf_device_register(self, swwdog_suspend, NULL))
aprint_error_dev(self, "couldn't establish power "
"handler\n");
And the referenced routine is as follows:
static bool
swwdog_suspend(device_t dev, const pmf_qual_t *qual)
{
struct swwdog_softc *sc = device_private(dev);
/* Don't allow suspend if watchdog is armed */
if ((sc->sc_smw.smw_mode & WDOG_MODE_MASK) != WDOG_MODE_DISARMED)
return false;
return true;
}
While you're there, I guess you could make swwdog_reboot a sysctl node.
Good point. Would this belong in the kern or machdep sysctl tree? I am
assuming it would be machdep.swwdog0.reboot for now, but it is easily
changed.
Revised diffs (including updated man page) attached.
-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-Index: src/share/man/man4/swwdog.4
===
RCS file: /cvsroot/src/share/man/man4/swwdog.4,v
retrieving revision 1.4
diff -u -p -r1.4 swwdog.4
--- src/share/man/man4/swwdog.4 30 Jan 2
Re: power management and pseudo-devices
On Sun, Jul 18, 2010 at 12:35:31PM -0700, Paul Goyette wrote:
> (Resent, this time with attachment!)
>
> On Sun, 18 Jul 2010, Quentin Garnier wrote:
>
> >On Sun, Jul 18, 2010 at 07:14:57AM -0700, Paul Goyette wrote:
> >>Currently, pseudo-devices are silently attached to the device tree
> >>without giving the driver any access to the associated device_t
> >>structure. As a result, the pseudo-device driver is unable to
> >>access much of the pmf framework. In particular, the pseudo-device
> >>cannot register a suspend/resume/shutdown handler, nor can it
> >>register to receive pmf event notifications.
> >>
> >>It seems to me that it would be reasonably useful for pseudo-devices
> >>to be capabile of participating in pmf. One particular example I've
> >>run into recently is the swwdog pseudo-device. Other watchdog
> >>drivers can prevent a suspend if their timer is armed, but since
> >>swwdog is only a pseudo-device it cannot register a pmf handler.
> >
> >Convert it to a defpseudodev then.
>
> I've done this, and it works! Diffs attached. Any objections to
> committing them?
Not really objections, just a few comments.
> +struct swwdog_softc *sc_swwdog = NULL;
That can't possibly be needed (and, if it was, you'd have to mutexify it
wouldn't you?).
> +
> +extern struct cfdriver swwdog_cd;
Include "ioconf.h" I think.
> +swwdogattach(int n)
> {
> - int i;
> + int err;
> + cfdata_t cf;
> +
> + if (n > 1)
> + aprint_verbose("%s: ignoring count of %d\n",
> + swwdog_cd.cd_name, n);
> +
> + err = config_cfattach_attach(swwdog_cd.cd_name, &swwdog_ca);
> + if (err) {
> + aprint_error("%s: couldn't register cfattach: %d\n",
> + swwdog_cd.cd_name, err);
> + config_cfdriver_detach(&swwdog_cd);
> + return;
> + }
> +
> + cf = kmem_alloc(sizeof(struct cfdata), KM_NOSLEEP);
> + if (cf == NULL)
> + aprint_error("%s: couldn't allocate cfdata\n",
> + swwdog_cd.cd_name);
> + else {
> + cf->cf_name = swwdog_cd.cd_name;
> + cf->cf_atname = swwdog_cd.cd_name;
> + cf->cf_unit = 0;
> + cf->cf_fstate = FSTATE_STAR;
>
> - for (i = 0; i < NSWWDOG; i++) {
> - struct swwdog_softc *sc = &sc_wdog[i];
> + (void)config_attach_pseudo(cf);
Ok, so you should be aware here that you're changing the logic of that
function, and you're only allowing for the creation of one swwdog(4).
That feature of the old code--implemented with needs-count no less,
brrr--is highly debatable, and the manual page actually says it's
pointless.
So yeah, go ahead and kill it (you can keep the __unused on int count by
the way), but change the manual page.
Also, you don't have to allocate a struct cfdata, you can use a static
one (indepedently of the fact that you only use it once, by the way, all
the other pseudo-devices could be changed in that respect).
> +static int
> +swwdog_match(device_t parent, cfdata_t data, void *aux)
> +{
> + /* Match unless we already have a swwdog */
> + if (sc_swwdog == NULL)
> + return 1;
> +
> + return 0;
> +}
What could possibly trigger another config_attach_pseudo? I know that
config(9) is special, but not magic :-)
> + if (!pmf_device_register(self, NULL, NULL))
> + aprint_error_dev(self, "couldn't establish power "
> + "handler\n");
So what was your initial point about swwdog(4) and pmf(9), anyway?
While you're there, I guess you could make swwdog_reboot a sysctl node.
--
Quentin Garnier - c...@cubidou.net - c...@netbsd.org
"See the look on my face from staying too long in one place
[...] every time the morning breaks I know I'm closer to falling"
KT Tunstall, Saving My Face, Drastic Fantastic, 2007.
pgpfGEuqltjV7.pgp
Description: PGP signature
Re: power management and pseudo-devices
(Resent, this time with attachment!)
On Sun, 18 Jul 2010, Quentin Garnier wrote:
On Sun, Jul 18, 2010 at 07:14:57AM -0700, Paul Goyette wrote:
Currently, pseudo-devices are silently attached to the device tree
without giving the driver any access to the associated device_t
structure. As a result, the pseudo-device driver is unable to
access much of the pmf framework. In particular, the pseudo-device
cannot register a suspend/resume/shutdown handler, nor can it
register to receive pmf event notifications.
It seems to me that it would be reasonably useful for pseudo-devices
to be capabile of participating in pmf. One particular example I've
run into recently is the swwdog pseudo-device. Other watchdog
drivers can prevent a suspend if their timer is armed, but since
swwdog is only a pseudo-device it cannot register a pmf handler.
Convert it to a defpseudodev then.
I've done this, and it works! Diffs attached. Any objections to
committing them?
-
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com|
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-Index: files.sysmon
===
RCS file: /cvsroot/src/sys/dev/sysmon/files.sysmon,v
retrieving revision 1.11
diff -u -p -r1.11 files.sysmon
--- files.sysmon30 Jan 2010 21:55:28 - 1.11
+++ files.sysmon18 Jul 2010 19:30:23 -
@@ -18,5 +18,5 @@ file dev/sysmon/sysmon_wdog.csysmon_wdo
file dev/sysmon/sysmon.c sysmon_envsys | sysmon_wdog |
sysmon_power
-defpseudo swwdog: sysmon_wdog
+defpseudodev swwdog: sysmon_wdog
filedev/sysmon/swwdog.cswwdog
Index: swwdog.c
===
RCS file: /cvsroot/src/sys/dev/sysmon/swwdog.c,v
retrieving revision 1.9
diff -u -p -r1.9 swwdog.c
--- swwdog.c31 Jan 2010 02:54:56 - 1.9
+++ swwdog.c18 Jul 2010 19:30:23 -
@@ -44,20 +44,28 @@ __KERNEL_RCSID(0, "$NetBSD: swwdog.c,v 1
#include
#include
#include
+#include
#include
#include
#include
#include
-#define NSWWDOG 1
struct swwdog_softc {
+ device_t sc_dev;
struct sysmon_wdog sc_smw;
struct callout sc_c;
- char sc_name[20];
int sc_wdog_armed;
-} sc_wdog[NSWWDOG];
+};
-void swwdogattach(int);
+struct swwdog_softc *sc_swwdog = NULL;
+
+extern struct cfdriver swwdog_cd;
+
+void swwdogattach(int);
+
+static int swwdog_match(device_t, cfdata_t, void *);
+static voidswwdog_attach(device_t, device_t, void *);
+static int swwdog_detach(device_t, int);
static int swwdog_setmode(struct sysmon_wdog *);
static int swwdog_tickle(struct sysmon_wdog *);
@@ -71,17 +79,63 @@ int swwdog_reboot = 0; /* set for panic
#defineSWDOG_DEFAULT 60 /* 60-second default period */
+CFATTACH_DECL_NEW(swwdog, sizeof(struct swwdog_softc),
+ swwdog_match, swwdog_attach, swwdog_detach, NULL);
+
void
-swwdogattach(int count __unused)
+swwdogattach(int n)
{
- int i;
+ int err;
+ cfdata_t cf;
+
+ if (n > 1)
+ aprint_verbose("%s: ignoring count of %d\n",
+ swwdog_cd.cd_name, n);
+
+ err = config_cfattach_attach(swwdog_cd.cd_name, &swwdog_ca);
+ if (err) {
+ aprint_error("%s: couldn't register cfattach: %d\n",
+ swwdog_cd.cd_name, err);
+ config_cfdriver_detach(&swwdog_cd);
+ return;
+ }
+
+ cf = kmem_alloc(sizeof(struct cfdata), KM_NOSLEEP);
+ if (cf == NULL)
+ aprint_error("%s: couldn't allocate cfdata\n",
+ swwdog_cd.cd_name);
+ else {
+ cf->cf_name = swwdog_cd.cd_name;
+ cf->cf_atname = swwdog_cd.cd_name;
+ cf->cf_unit = 0;
+ cf->cf_fstate = FSTATE_STAR;
- for (i = 0; i < NSWWDOG; i++) {
- struct swwdog_softc *sc = &sc_wdog[i];
+ (void)config_attach_pseudo(cf);
+ }
+
+ return;
+}
- snprintf(sc->sc_name, sizeof sc->sc_name, "swwdog%d", i);
- printf("%s: ", sc->sc_name);
- sc->sc_smw.smw_name = sc->sc_name;
+static int
+swwdog_match(device_t parent, cfdata_t data, void *aux)
+{
+ /* Match unless we already have a swwdog */
+ if (sc_swwdog == NULL)
+ return 1;
+
+ return 0;
+}
+
+static void
+swwdog_attach(device_t parent, device_t self, void *aux)
+{
+ struct swwdog_softc *sc = device_private(self);
+
+ if (sc_swwdog == NULL) {
+
Re: power management and pseudo-devices
On Sun, 18 Jul 2010, Quentin Garnier wrote: On Sun, Jul 18, 2010 at 07:14:57AM -0700, Paul Goyette wrote: Currently, pseudo-devices are silently attached to the device tree without giving the driver any access to the associated device_t structure. As a result, the pseudo-device driver is unable to access much of the pmf framework. In particular, the pseudo-device cannot register a suspend/resume/shutdown handler, nor can it register to receive pmf event notifications. It seems to me that it would be reasonably useful for pseudo-devices to be capabile of participating in pmf. One particular example I've run into recently is the swwdog pseudo-device. Other watchdog drivers can prevent a suspend if their timer is armed, but since swwdog is only a pseudo-device it cannot register a pmf handler. Convert it to a defpseudodev then. I've done this, and it works. Diffs are attached. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: power management and pseudo-devices
On Sun, Jul 18, 2010 at 07:14:57AM -0700, Paul Goyette wrote: > Currently, pseudo-devices are silently attached to the device tree > without giving the driver any access to the associated device_t > structure. As a result, the pseudo-device driver is unable to > access much of the pmf framework. In particular, the pseudo-device > cannot register a suspend/resume/shutdown handler, nor can it > register to receive pmf event notifications. > > It seems to me that it would be reasonably useful for pseudo-devices > to be capabile of participating in pmf. One particular example I've > run into recently is the swwdog pseudo-device. Other watchdog > drivers can prevent a suspend if their timer is armed, but since > swwdog is only a pseudo-device it cannot register a pmf handler. Convert it to a defpseudodev then. -- Quentin Garnier - c...@cubidou.net - c...@netbsd.org "See the look on my face from staying too long in one place [...] every time the morning breaks I know I'm closer to falling" KT Tunstall, Saving My Face, Drastic Fantastic, 2007. pgpSKG5AwFG9b.pgp Description: PGP signature
power management and pseudo-devices
Currently, pseudo-devices are silently attached to the device tree without giving the driver any access to the associated device_t structure. As a result, the pseudo-device driver is unable to access much of the pmf framework. In particular, the pseudo-device cannot register a suspend/resume/shutdown handler, nor can it register to receive pmf event notifications. It seems to me that it would be reasonably useful for pseudo-devices to be capabile of participating in pmf. One particular example I've run into recently is the swwdog pseudo-device. Other watchdog drivers can prevent a suspend if their timer is armed, but since swwdog is only a pseudo-device it cannot register a pmf handler. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
