[RFC] Driver States

[RFC] Driver States

[Adam Belay]

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

Dynamic power management may require devices and drivers to transition
between various physical and logical states.  I would like to start a
discussion on how these might be defined at the bus, driver, and class
levels.

Bus Level
=========
At the bus level, there are two state attributes, power and
enable/disable.  Enable/disable may mean different things on different
buses, but they generally refer to resource decoding.  A device can only
be enabled during a non-off power state.

A possible API:

struct bus_type {
	char			* name;

	struct subsystem	subsys;
	struct kset		drivers;
	struct kset		devices;

	struct bus_attribute	* bus_attrs;
	struct device_attribute	* dev_attrs;
	struct driver_attribute	* drv_attrs;

	int		(*match)(struct device * dev, struct device_driver * drv);
	int		(*hotplug) (struct device *dev, char **envp, 
				    int num_envp, char *buffer, int buffer_size);
	int		(*suspend)(struct device * dev, pm_message_t state);
	int		(*resume)(struct device * dev);
	int		(*enable)(struct device * dev);
	int		(*disable)(struct device * dev);
};

Driver Level
============
At the driver level there are two areas of interest, physical and
logical state.  There is an additional concern of transitioning between
these states multiple times.  Because a driver acts as a bridge between
physical and logical components, I think separating these steps seems
natural.

A possible API:

struct device_driver {
	char			* name;
	struct bus_type		* bus;

	struct semaphore	unload_sem;
	struct kobject		kobj;
	struct list_head	devices;

	struct module 		* owner;

	int	(*attach)	(struct device * dev);
	int	(*start)	(struct device * dev);
	int	(*open)		(struct device * dev);
	int	(*close)	(struct device * dev);
	void 	(*stop)		(struct device * dev);
	void	(*detach)	(struct device * dev);
	void	(*shutdown)	(struct device * dev);
	int	(*suspend)	(struct device * dev, u32 state, u32 level);
	int	(*resume)	(struct device * dev, u32 level);
};

*attach - allocates data structures, creates sysfs entries, prepares driver
       to handle the hardware.
 
*start -  Sets up device resources and configures the hardware.  Loads
firmware, etc.
(physical)
 
*open -   engages the hardware, and makes it usable by the class device.
(logical and physical)
 
*close -  disengages the hardware, and stops class level access
(logical and physical)
 
*stop -   physically disables the hardware
(physical)
 
*detach - tears down the driver and releases it from the "struct device"

The idea behind *attach and *detach is to move code that would only need
to be called once out of *probe and *remove.

A table could be defined that indicates what should be called for each
power level transition.  *suspend and *resume could handle any extra
steps (ex. saving state).  As an example, *start and *stop may only be
called when power is going to be lost entirely.

Additional states are class specific and would only be used after *open
is called.

Class Level
===========
At the class level, we could have a simple start/stop mechanism.

A possible API:

struct class_device {
	struct list_head	node;

	struct kobject		kobj;
	struct class		* class;
	struct device		* dev;
	void			* class_data;

	char	class_id[BUS_ID_SIZE];

	int	(*attach)	(struct device * dev);
	int	(*start)	(struct device * dev);
	void 	(*stop)		(struct device * dev);
	void	(*detach)	(struct device * dev);
};

*attach - allocates data structures, creates sysfs entries, prepares
class to handle the device.

*start - start the logical class device, accept userspace interaction

*stop - stop the logical class device, deny userspace interaction

*detach - tear down the class driver's bindings with this class device


These are just rough ideas.  I look forward to any comments or
alternative approaches.

Thanks,
Adam



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC] Driver States

[Jordan Crouse]

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

> struct device_driver {

Its still in debate, but if we decide to do centralized policy
management (like DPM), then we will need some way to pass in attributes
such as device inactivity timeout to the device.  Personally, I would be
happy with a call that passed a pointer for flexibility,  but it seems
that recently, people are heavily favoring arguments that can be
typchecked.

[ As an aside, I think that deciding where policy management will live
will help clear up some of the nagging issues we've been having about
the role of the drivers vs. the role of the system, etc, etc. ]

> A table could be defined that indicates what should be called for each
> power level transition.  *suspend and *resume could handle any extra
> steps (ex. saving state).  As an example, *start and *stop may only be
> called when power is going to be lost entirely.

I think we discussed this during the IRC meeting.  I certainly don't
favor multiple calls which increase the level of complexity of the
calling entity.   I would need one table to tell me what power level the
device should be at, and a second table to tell me if I need to call
stop() or start().  Furthermore some devices may go clocks off during
one S-T-R transition, and then keep clocks on for wake another time. 
Our current thinking is to assume that the driver is intelligent enough
to know what to do for any given power level, and I think a single call
would suffice for that.

Jordan

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>




[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC] Driver States

[David Brownell]

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

On Monday 28 March 2005 8:29 am, Jordan Crouse wrote:
> > struct device_driver {
> 
> Its still in debate, but if we decide to do centralized policy
> management (like DPM), then we will need some way to pass in attributes
> such as device inactivity timeout to the device. 

Device inactivity is a good example of something that's _already_ done
in a well-decentralized manner.  E.g. X11/DPMS, and hdparm.  If you're
after a motivation to adopt the MontaVista DPM model, that undermines
your argument ...


> Personally, I would be 
> happy with a call that passed a pointer for flexibility,  but it seems
> that recently, people are heavily favoring arguments that can be
> typchecked.

Pointers can easily be type-checked.  :)


> [ As an aside, I think that deciding where policy management will live
> will help clear up some of the nagging issues we've been having about
> the role of the drivers vs. the role of the system, etc, etc. ]

I think system policy should live at the system level, and driver policy
should live at the driver level.  Simple.  ;)

 
> > A table could be defined that indicates what should be called for each
> > power level transition.  *suspend and *resume could handle any extra
> > steps (ex. saving state).  As an example, *start and *stop may only be
> > called when power is going to be lost entirely.
> 
> I think we discussed this during the IRC meeting.  I certainly don't
> favor multiple calls which increase the level of complexity of the
> calling entity.  

It's a tradeoff.  Needless complexity should be avoided, of course.
Device driver authors should have only a limited slate of system-wide
issues to cope with ... which argues more for keeping complexity in
the core.  On the other hand, complex core logic can be hard to maintain
and error prone too.


> I would need one table to tell me what power level the 
> device should be at, and a second table to tell me if I need to call
> stop() or start().  Furthermore some devices may go clocks off during
> one S-T-R transition, and then keep clocks on for wake another time. 

Same is true for power, not just for clocks:  both get switched on/off.


> Our current thinking is to assume that the driver is intelligent enough
> to know what to do for any given power level, and I think a single call
> would suffice for that.

There's also system configuration to think of.  A recent example:  a
device should turn off several controllers whenever it's not hooked
up to its docking station (with power supply).  Regardless of whatever
else the other system power state(s) may be.

Some of these issues are completely orthogonal to other issues about
system state.  You're right that these are examples of places where
the intelligence is best kept at the _driver_ level ... in this case,
the driver that handles the "I'm docked!" IRQ is the natural place to
know what devices a given docking configuration should activate.

- Dave


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

apm_console_blank()

[Jordan Crouse]

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

Everyone, put on your legacy helmets... :)

Attached is a diff against apm.c fixing what I think was a flaw in
the logic of the function apm_console_blank().

Previously, the code tried to power down device 0x100, and, unless that
returned no error, it tried to do the same to device 0x1FF, and unless
*that* returned no error, it tried to power down device 0x101. 
Then, it would try to engage the power management or return a
message based on only the error message of the last call.

Now, it turns out that page 41 of the APM specification indicates that
the system should be engaged, but the devices should start out
disengaged, which is what our BIOS does.  I do understand the system is
engaged at boot time if the apm_bios flags indicate APM_BIOS_DISENGAGED.
What I don't know is if our BIOS should be setting this flag or
not (normally when I get bugs like this, I question why thousands of
systems before me have somehow managed to do the right thing, and it
usually means that we're doing something boneheaded).

If it is not critical that the BIOS be engaged at boot time, then one
would expect that a device could possibly still be disengaged when the
console tries to blank the screen.  So, I went ahead and modified the
code slightly so that any set_power_state() call that returns
NOT_ENGAGED will go ahead and try to engage the device.

Comments and flames welcome.

Jordan

-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>


--- linux-2.6.11.orig/arch/i386/kernel/apm.c	2005-03-28 10:05:01.000000000 -0700
+++ linux-2.6.11/arch/i386/kernel/apm.c	2005-03-28 10:05:12.000000000 -0700
@@ -1058,25 +1058,26 @@
  *	all video devices. Typically the BIOS will do laptop backlight
and  *	monitor powerdown for us.
  */
- 
-static int apm_console_blank(int blank)
-{
-	int	error;
-	u_short	state;
+
+static int apm_console_blank(int blank) {
+	
+	int error, i;
+	u_short state;
+	u_short dev[3] = { 0x100, 0x1FF, 0x101 };
 
 	state = blank ? APM_STATE_STANDBY : APM_STATE_READY;
-	/* Blank the first display device */
-	error = set_power_state(0x100, state);
-	if ((error != APM_SUCCESS) && (error != APM_NO_ERROR)) {
-		/* try to blank them all instead */
-		error = set_power_state(0x1ff, state);
-		if ((error != APM_SUCCESS) && (error != APM_NO_ERROR))
-			/* try to blank device one instead */
-			error = set_power_state(0x101, state);
+
+	for(i = 0; i < 3; i++) {		
+		error = set_power_state(dev[i], state);
+
+		if ((error == APM_SUCCESS) || (error == APM_NO_ERROR))
+			return 1;
+
+		if (error == APM_NOT_ENGAGED)
+			break;	       
 	}
-	if ((error == APM_SUCCESS) || (error == APM_NO_ERROR))
-		return 1;
-	if (error == APM_NOT_ENGAGED) {
+
+	if (error == APM_NOT_ENGAGED && state != APM_STATE_READY) {
 		static int tried;
 		int eng_error;
 		if (tried++ == 0) {
@@ -1089,7 +1090,8 @@
 				return apm_console_blank(blank);
 		}
 	}
-	apm_error("set display", error);
+
+	apm_error("set display", error);	
 	return 0;
 }
 #endif







[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: apm_console_blank()

[Pavel Machek]

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

Hi!

> Everyone, put on your legacy helmets... :)
> 
> Attached is a diff against apm.c fixing what I think was a flaw in
> the logic of the function apm_console_blank().

I guess you need to send it to APM maintainer...

Please inline your patches, its easier to comment on them that way.

--- linux-2.6.11.orig/arch/i386/kernel/apm.c    2005-03-28 10:05:01.000000000 -0700
+++ linux-2.6.11/arch/i386/kernel/apm.c 2005-03-28 10:05:12.000000000 -0700
@@ -1058,25 +1058,26 @@
  *     all video devices. Typically the BIOS will do laptop backlight
and  *  monitor powerdown for us.
  */
-
-static int apm_console_blank(int blank)
-{
-       int     error;
-       u_short state;
+
+static int apm_console_blank(int blank) {
+
+       int error, i;
+       u_short state;

You are actually breaking formating here.

+       u_short dev[3] = { 0x100, 0x1FF, 0x101 };

        state = blank ? APM_STATE_STANDBY : APM_STATE_READY;
-       /* Blank the first display device */
-       error = set_power_state(0x100, state);
-       if ((error != APM_SUCCESS) && (error != APM_NO_ERROR)) {
-               /* try to blank them all instead */
-               error = set_power_state(0x1ff, state);
-               if ((error != APM_SUCCESS) && (error != APM_NO_ERROR))
-                       /* try to blank device one instead */
-                       error = set_power_state(0x101, state);
+
+       for(i = 0; i < 3; i++) {

Please put space between for and (.

@@ -1089,7 +1090,8 @@
                                return apm_console_blank(blank);
                }
        }
-       apm_error("set display", error);
+
+       apm_error("set display", error);
        return 0;
 }

...and don't do change formatting just because you can.

								Pavel

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC] Driver States

[Patrick Mochel]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1662 bytes --]


On Sun, 27 Mar 2005, Adam Belay wrote:

> Dynamic power management may require devices and drivers to transition
> between various physical and logical states.  I would like to start a
> discussion on how these might be defined at the bus, driver, and class
> levels.

<snip>

> Bus Level
> =========
> At the bus level, there are two state attributes, power and
> enable/disable.  Enable/disable may mean different things on different
> buses, but they generally refer to resource decoding.  A device can only
> be enabled during a non-off power state.

<...>

> Driver Level
> ============
> At the driver level there are two areas of interest, physical and
> logical state.  There is an additional concern of transitioning between
> these states multiple times.  Because a driver acts as a bridge between
> physical and logical components, I think separating these steps seems
> natural.

<...>

> *attach - allocates data structures, creates sysfs entries, prepares driver
>        to handle the hardware.
>
> *start -  Sets up device resources and configures the hardware.  Loads
> firmware, etc.
> (physical)
>
> *open -   engages the hardware, and makes it usable by the class device.
> (logical and physical)
>
> *close -  disengages the hardware, and stops class level access
> (logical and physical)
>
> *stop -   physically disables the hardware
> (physical)
>
> *detach - tears down the driver and releases it from the "struct device"
>

You have a few things here that can easily conflict, and that will be
developed at different paces. I like the direction that it's going, but
how do you intend to do it gradually. I.e. what to do first?


	Pat


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC] Driver States

[Adam Belay]

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

On Tue, 2005-03-29 at 21:57 -0800, Patrick Mochel wrote:
> On Sun, 27 Mar 2005, Adam Belay wrote:
> 
> > Dynamic power management may require devices and drivers to transition
> > between various physical and logical states.  I would like to start a
> > discussion on how these might be defined at the bus, driver, and class
> > levels.
> 
> <snip>
> 
> > Bus Level
> > =========
> > At the bus level, there are two state attributes, power and
> > enable/disable.  Enable/disable may mean different things on different
> > buses, but they generally refer to resource decoding.  A device can only
> > be enabled during a non-off power state.
> 
> <...>
> 
> > Driver Level
> > ============
> > At the driver level there are two areas of interest, physical and
> > logical state.  There is an additional concern of transitioning between
> > these states multiple times.  Because a driver acts as a bridge between
> > physical and logical components, I think separating these steps seems
> > natural.
> 
> <...>
> 
> > *attach - allocates data structures, creates sysfs entries, prepares driver
> >        to handle the hardware.
> >
> > *start -  Sets up device resources and configures the hardware.  Loads
> > firmware, etc.
> > (physical)
> >
> > *open -   engages the hardware, and makes it usable by the class device.
> > (logical and physical)
> >
> > *close -  disengages the hardware, and stops class level access
> > (logical and physical)
> >
> > *stop -   physically disables the hardware
> > (physical)
> >
> > *detach - tears down the driver and releases it from the "struct device"
> >
> 
> You have a few things here that can easily conflict, and that will be
> developed at different paces. I like the direction that it's going, but
> how do you intend to do it gradually. I.e. what to do first?

I think the first step would be for us to all agree on a design, whether
it be this one or another, so we can began planning for long term
changes.

My arguments for these changes are as follows:

     1. If a device has been powered off, powered on, and restored in
        state, it is identical to a device that the driver is
        configuring for the first time.  So calling "*start" as part of
        device resume seems like a logical course of action.
     2. Being able to start and stop a device is useful outside the
        realm of power management.  It's required for resource
        re-balancing.  Also, the user may want to disable a device, but
        the device must still be able to save state and react correctly
        during a suspend.  Allowing the user to start and stop drivers
        gives more flexibility to userspace utilities.  There may be
        sysfs configuration files that can only be changed when the
        device isn't active.  Resource configuration cannot be changed
        when the device is in use.
     3. *open and *close also might be a possibility.  When a device is
        put into a lower power state, we want to stop driver timers and
        prepare the hardware to be inactive, which is exactly the role
        of "*close".  See the existing code in most net drivers.  I
        would like to note, however, that this portion of the API is
        optional, and needs to be looked into further.  I'm considering
        dropping it in favor of having suspend and resume handle this.
     4. Having responsibilities at each driver level encourages a
        layered and object based design, reducing code duplication and
        complexity.

* "*start" and "*stop" might even be useful for device error detection.
(Ex. we currently have no notion of starting up a device over again
after a hardware failure)

I think the next step would be to look at each class subsystem, and
verify that our proposed API could work well with it.  If it's going to
change the design of many device drivers, we want to make sure we get it
right.

>From there, the bus level changes could be made, as they would affect
the least upstream code.  Things like PCI PM could also be improved
during this stage.  Greg, I know that adding *enable, and *disable to
"struct bus_type" would be nice for PCI, ACPI, etc, as it would control
whether resources are decoded and other device initialization
requirements.  What about external devices such as USB?  Would such as
interface be useful for other buses?

Next, we could make changes to "struct device_driver".  As to not break
things, (*start or *attach) and (*stop or *detach) could be temporarily
mapped to *probe and *remove until everything is fixed up.  This hack
could be made at the bus level, so the fixes could be applied one bus at
a time.

The final step would be to introduce "*open and *close", if we decide to
use them, and also class device *start and *stop.  Pat, do you have any
comments on adding *start and *stop to class devices?  It seems like an
interesting possibility to me.

Thanks,
Adam



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [RFC] Driver States

[Pavel Machek]

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

Hi!

> > You have a few things here that can easily conflict, and that will be
> > developed at different paces. I like the direction that it's going, but
> > how do you intend to do it gradually. I.e. what to do first?
> 
> I think the first step would be for us to all agree on a design, whether
> it be this one or another, so we can began planning for long term
> changes.
> 
> My arguments for these changes are as follows:

0. I do not see how to gradually roll this in.

>      4. Having responsibilities at each driver level encourages a
>         layered and object based design, reducing code duplication and
>         complexity.

Unfortunately, you'll be retrofiting this to existing drivers. AFAICS,
trying to force existing driver to "layered and object based design"
can only result in mess.
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [RFC] Driver States

[Adam Belay]

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

On Tue, 2005-04-05 at 11:24 +0200, Pavel Machek wrote:
> Hi!
> 
> > > You have a few things here that can easily conflict, and that will be
> > > developed at different paces. I like the direction that it's going, but
> > > how do you intend to do it gradually. I.e. what to do first?
> > 
> > I think the first step would be for us to all agree on a design, whether
> > it be this one or another, so we can began planning for long term
> > changes.
> > 
> > My arguments for these changes are as follows:
> 
> 0. I do not see how to gradually roll this in.
> 
> >      4. Having responsibilities at each driver level encourages a
> >         layered and object based design, reducing code duplication and
> >         complexity.
> 
> Unfortunately, you'll be retrofiting this to existing drivers. AFAICS,
> trying to force existing driver to "layered and object based design"
> can only result in mess.
> 								Pavel

Fair enough.  How does this sound?  I'd like to add "*attach" and
"*detach" to "struct device_driver".  These functions would act as one
time initializers and decontructors.  Then we could rename "*probe" to
"*start", and "*remove" to "*stop", which should be rather trivial to
fix up.  From there drivers could slowly be converted to use "*attach"
and "*detach", but will not be broken along the way.

So the basic flow would be like this:

1.) a driver is bound to a device
2.) *attach is called to allocate data structures
3.) *start when it's time to probe the device
4.) *stop when the user disables the device
5.) repeat steps 3 and 4 any number of times
6.) *detach is called when unbinding the driver

The driver layering stuff could come later, but just implementing these
specific components would have immediate benefits.

In this early stage in development, I'd like to at least be able to
start and stop drivers for reasons outside of power management (ex. user
preference or resource re-balancing).  If a "*resume" function can also
utilize this functionality, then all the better.

Thanks,
Adam



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Re: [RFC] Driver States

[Pavel Machek]

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

Hi!

> > > > You have a few things here that can easily conflict, and that will be
> > > > developed at different paces. I like the direction that it's going, but
> > > > how do you intend to do it gradually. I.e. what to do first?
> > > 
> > > I think the first step would be for us to all agree on a design, whether
> > > it be this one or another, so we can began planning for long term
> > > changes.
> > > 
> > > My arguments for these changes are as follows:
> > 
> > 0. I do not see how to gradually roll this in.
> > 
> > >      4. Having responsibilities at each driver level encourages a
> > >         layered and object based design, reducing code duplication and
> > >         complexity.
> > 
> > Unfortunately, you'll be retrofiting this to existing drivers. AFAICS,
> > trying to force existing driver to "layered and object based design"
> > can only result in mess.
> > 								Pavel
> 
> Fair enough.  How does this sound?  I'd like to add "*attach" and
> "*detach" to "struct device_driver".  These functions would act as one
> time initializers and decontructors.  Then we could rename "*probe" to
> "*start", and "*remove" to "*stop", which should be rather trivial to

I do not think you'll find rename across all the drivers easy. You
could get away with "I create start, and if it does not exist, probe
is called instead", but you need pretty good justification for that, too.

								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]