Date: Thu, 27 Mar 2008 17:06:30 -0600 (MDT) From: "M. Warner Losh" <imp@bsdimp.com> To: jhb@freebsd.org Cc: freebsd-arch@freebsd.org Subject: Re: AsiaBSDCon DEVSUMMIT patch Message-ID: <20080327.170630.-365729353.imp@bsdimp.com> In-Reply-To: <200803271727.51811.jhb@freebsd.org> References: <200803271105.18401.jhb@freebsd.org> <20080327.142052.-1337017421.imp@bsdimp.com> <200803271727.51811.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <200803271727.51811.jhb@freebsd.org>
John Baldwin <jhb@freebsd.org> writes:
: On Thursday 27 March 2008 04:20:52 pm M. Warner Losh wrote:
: > In message: <200803271105.18401.jhb@freebsd.org>
: > John Baldwin <jhb@freebsd.org> writes:
: > : On Thursday 27 March 2008 03:32:29 am M. Warner Losh wrote:
: > : > Greetings,
: > : >
: > : > We've been talking about the situation with suspend/resume in the
: > : > tree. Here's a quick hack to allow one to suspend/resume an
: > : > individual device. This may or may not work too well, but it is
: > : > offered up for testing and criticism.
: > : >
: > : > http://people.freebsd.org/~imp/devctl.diff
: > : >
: > : > devctl -s ath 0 suspend ath0
: > : > devctl -r ath 0 resume ath0
: >
: > Wow, you have a lot of comments for a simple test program :-)
: >
: > : Unfortunately, what you really need is to power down the device to D3 for
: > : suspend and then bring it up. Otherwise you might not lose enough state
: to
: > : notice that resume isn't restoring all of it. bge(4) doesn't survive
: resume
: > : on my laptop I think because brgphy doesn't re-patch the firmware on
: resume,
: > : and you'd need a full power down to run into that sort of thing.
: >
: > True. I was going to implement this next as a bus method to have the
: > bus to the right thing.
: >
: > : What I would actually prefer would be this:
: > :
: > : devctl ath0 power off (maps to D3 on PCI/ACPI)
: > : devctl ath0 power D1 (PCI/ACPI-specific)
: > : devctl ath0 power on (maps to D0 on PCI/APCI)
: >
: > I'm not sure I like this at all. This is about completely suspending
: > a device, or completely resuming the device for testing purposes.
: > Randomly putting the device into D1 state is a bad idea. The device
: > driver itself should do that level of detail.
:
: It is useful for doing some of what phk suggested earlier though. For
: example, putting devices in D1 and seeing if you really get an interrupt on
: link state events or USB device insertion, etc. That is, for debugging some
: of the power management stuff. I imagine that on/off will be used the vast
: majority of the time, but I can see D[12] being useful for debugging. I
: wouldn't mind being able to manually turn devices off when I'm on an airplane
: to conserve battery until such time as we get smarter drivers that
: automatically manage the power. For example, I'd like to turn off my sound
: card sometimes, or power down bge0 when iwi0 associates (since I only ever
: use 1 of them).
The problem is that turning off the device w/o the cooperation of the
device is a non-starter. You can already unload the driver from the
system and have the device be powered down, for example, by setting
hw.pci.do_power_nodriver to 1. In the short term, you'll be much
happier with this approach, as it is more general and drivers cope
with that already.
The main reason I did suspend/resume is (a) to test out suspend/resume
and (b) drivers are already supposed to handle it correctly.
I'm very leery of controlling the power of a device behind its back,
even for something as test program. The documentation for the chip
will tell you what modes it gets interrupts in, and that's a lot more
reliable than just trying it, which may or may not work for different
revisions of the same chip...
: > : If you want to do named commands (like 'power') rather than getopt args
: for
: > : everything you can use a linker set to build a table of commands (I've
: done
: > : this for RAID management utils at work) that let you do something like:
: > :
: > : struct devctl_power_request {
: > : const char device[MAXDEVNAME];
: > : const char state[32];
: > : }
: > :
: > : #define DEVIOC_POWER _IOW('d', 1, struct devctl_power_request)
: > :
: > : /* av[0] will be 'power' */
: > : static void
: > : power_command(int ac, char **av)
: > : {
: > : struct devctl_power_request req;
: > :
: > : if (ac != 3)
: > : errx(1, "Usage: devctl power <device> <state>");
: > : strlcpy(req.device, av[1], sizeof(req.device));
: > : strlcpy(req.state, av[2], sizeof(req.state));
: > : if (ioctl(fd, DEVIOC_POWER, &req) < 0)
: > : err(1, "Set power state failed");
: > : }
: > : DEVCTL_COMMAND(power);
: > :
: > : (Using a linker set makes it easier to add new commands later and have
: them
: > : all be self-contained.)
: >
: > Wow! that's a lot more complicated than I had in mind :-)
:
: But is more extensible so you can have 'devctl eject foo0' (think ACPI _EJx
: methods) or other commands that are a bit more user friendly in syntax than a
: plethora of getopt options. It's also not that hard esp. since I've
: basically sent you the implementation via private e-mail. :)
I like the idea of having different commands....
Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080327.170630.-365729353.imp>