Driver Core fixes for 2.6.7-rc1

Hi,

Here are some driver core fixes for 2.6.7-rc1. This set of patches also includes the removal of the smbios driver, which the author wanted to have removed after realizing that it was not needed.

Please pull from:

	bk://kernel.bkbits.net/gregkh/linux/driver-2.6

thanks,

greg k-h

p.s. I'll send these as patches in response to this email to lkml for those who want to see them.

 drivers/firmware/smbios.c    |  248 -------------------------------------------
 drivers/firmware/smbios.h    |   53 ---------
 drivers/base/power/resume.c  |    7 -
 drivers/base/power/runtime.c |   10 +
 drivers/base/power/suspend.c |   34 +++--
 drivers/firmware/Kconfig     |    8 -
 drivers/firmware/Makefile    |    1 
 drivers/firmware/smbios.c    |    2 
 fs/sysfs/dir.c               |   12 +-
 fs/sysfs/inode.c             |    7 +
 fs/sysfs/symlink.c           |  135 ++++++++++++++++-------
 fs/sysfs/sysfs.h             |    7 -
 include/linux/pm.h           |    1 
 13 files changed, 148 insertions(+), 377 deletions(-)

-----

<tpoynor:mvista.com>:
o Fix for leave-runtime-suspended-devices-off-at-system-resume.patch
o Leave runtime suspended devices off at system resume
o Device runtime suspend/resume fixes

Greg Kroah-Hartman:
o Minor coding style fixups in resume code and added a bit of debugging help
o Report which device failed to suspend
o Remove the smbios driver as it is not needed

Maneesh Soni:
o fix-sysfs-symlinks.patch

Rene Herman:
o missing closing \n in printk

ChangeSet 1.1717.7.1, 2004/05/17 10:32:30-07:00, [email protected]

[PATCH] fix-sysfs-symlinks.patch

- Rediffed the patch for 2.6.6-mm3 to fix rejects in the latest changes in sysfs code.

o The symlinks code in sysfs doesnot point to the correct target kobject whenever target kobject is renamed and suffers from dangling symlinks if target kobject is removed.

o The following patch implements ->readlink and ->follow_link operations for sysfs instead of using the page_symlink_inode_operations. The pointer to target kobject is saved in the link dentry's d_fsdata field. The target path is generated everytime we do ->readlink and ->follow_link. This results in generating the correct target path during readlink and follow_link operations inspite of renamed target kobject.

o This also pins the target kobject during link creation and the ref. is released when the link is removed.

o Apart from being correct this patch also saves some memory by not pinning a whole page for saving the target information.

o Used a rw_semaphor sysfs_rename_sem to avoid clashing with renaming of ancestors while the target path is generated.

o Used dcache_lock in fs/sysfs/sysfs.h:sysfs_get_kobject() because of using d_drop() while removing dentries.

 fs/sysfs/dir.c     |   12 ++++
 fs/sysfs/inode.c   |    7 ++
 fs/sysfs/symlink.c |  135 ++++++++++++++++++++++++++++++++++++-----------------
 fs/sysfs/sysfs.h   |    7 +-
 4 files changed, 116 insertions(+), 45 deletions(-)

diff -Nru a/fs/sysfs/dir.c b/fs/sysfs/dir.c
@@ -10,6 +10,8 @@
 #include <linux/kobject.h>
 #include "sysfs.h"
 
+DECLARE_RWSEM(sysfs_rename_sem);
+
 static int init_dir(struct inode * inode)
 {
 	inode->i_op = &simple_dir_inode_operations;

@@ -134,8 +136,14 @@
 			/**
 			 * Unlink and unhash.
 			 */
+			__d_drop(d);
 			spin_unlock(&dcache_lock);
-			d_delete(d);
+			/* release the target kobject in case of 
+			 * a symlink
+			 */
+			if (S_ISLNK(d->d_inode->i_mode))
+				kobject_put(d->d_fsdata);
+			
 			simple_unlink(dentry->d_inode,d);
 			dput(d);
 			pr_debug(" done\n");

@@ -165,6 +173,7 @@
 	if (!kobj->parent)
 		return -EINVAL;
 
+	down_write(&sysfs_rename_sem);
 	parent = kobj->parent->dentry;
 
 	down(&parent->d_inode->i_sem);

@@ -179,6 +188,7 @@
 		dput(new_dentry);
 	}
 	up(&parent->d_inode->i_sem);	
+	up_write(&sysfs_rename_sem);
 
 	return error;
 }

diff -Nru a/fs/sysfs/inode.c b/fs/sysfs/inode.c
@@ -96,7 +96,12 @@
 			pr_debug("sysfs: Removing %s (%d)\n", victim->d_name.name,
 				 atomic_read(&victim->d_count));
 
-			d_delete(victim);
+			d_drop(victim);
+			/* release the target kobject in case of 
+			 * a symlink
+			 */
+			if (S_ISLNK(victim->d_inode->i_mode))
+				kobject_put(victim->d_fsdata);
 			simple_unlink(dir->d_inode,victim);
 		}
 		/*

diff -Nru a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
@@ -12,15 +12,18 @@
 extern int sysfs_create_subdir(struct kobject *, const char *, struct dentry **);
 extern void sysfs_remove_subdir(struct dentry *);
 
+extern int sysfs_readlink(struct dentry *, char __user *, int );
+extern int sysfs_follow_link(struct dentry *, struct nameidata *);
+extern struct rw_semaphore sysfs_rename_sem;
 
 static inline struct kobject *sysfs_get_kobject(struct dentry *dentry)
 {
 	struct kobject * kobj = NULL;
 
-	spin_lock(&dentry->d_lock);
+	spin_lock(&dcache_lock);
 	if (!d_unhashed(dentry))
 		kobj = kobject_get(dentry->d_fsdata);
-	spin_unlock(&dentry->d_lock);
+	spin_unlock(&dcache_lock);
 
 	return kobj;
 }

ChangeSet 1.1717.7.2, 2004/05/17 17:28:58-07:00, [email protected]

[PATCH] missing closing \n in printk

 drivers/firmware/smbios.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -Nru a/drivers/firmware/smbios.c b/drivers/firmware/smbios.c
@@ -126,7 +126,7 @@
 	if(keep_going != 0)
 		printk(KERN_INFO "Warning: SMBIOS table does not end with a"
 				" structure type 127. This may indicate a"
-				" truncated table.");
+				" truncated table.\n");
 
 	if(sdev->smbios_table_real_length != max_length)
 		printk(KERN_INFO "Warning: BIOS specified SMBIOS table length"

ChangeSet 1.1717.7.3, 2004/05/27 15:48:10-07:00, [email protected]

Remove the smbios driver as it is not needed.

You can do the same from userspace, and the author requests that the driver be deleted from the kernel tree before people start using it.

 drivers/firmware/smbios.c |  248 ----------------------------------------------
 drivers/firmware/smbios.h |   53 ---------
 drivers/firmware/Kconfig  |    8 -
 drivers/firmware/Makefile |    1 
 4 files changed, 310 deletions(-)

diff -Nru a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
@@ -34,12 +34,4 @@
 	  Subsequent efibootmgr releases may be found at:
 	  http://linux.dell.com/efibootmgr
 
-config SMBIOS
-	tristate "BIOS SMBIOS table access driver."
-	help
-	  Say Y or M here if you want to enable access to the SMBIOS table
-	  via driverfs. It exposes /sys/firmware/smbios/ subdirectory tree
-	  containing a binary dump of the SMBIOS table header as well as the SMBIOS
-	  table.
-
 endmenu

diff -Nru a/drivers/firmware/Makefile b/drivers/firmware/Makefile
@@ -3,4 +3,3 @@
 #
 obj-$(CONFIG_EDD)             	+= edd.o
 obj-$(CONFIG_EFI_VARS)		+= efivars.o
-obj-$(CONFIG_SMBIOS)            += smbios.o

ChangeSet 1.1717.7.4, 2004/05/28 10:04:18-07:00, [email protected]

[PATCH] Device runtime suspend/resume fixes

(1) Set device power state at runtime resume (as is done for runtime suspend) so that a later suspend does not think the device is still suspended (refusing to suspend it again).

(2) Move devices from the active list to the off list only when suspending all devices as part of a system suspend, not for runtime suspend. This matches the resume code, which only moves devices from off to active during system resume, such that runtime resume currently doesn't move the suspended device back to the active list. (This also avoids reordering the device list for runtime suspends; the list is in order of registration and suspend/resume works best that way -- granted, more sweeping improvements in how device dependencies are accounted for in the suspend/resume order are also needed someday.)

Runtime device suspend/resume is in some cases used frequently on battery-powered embedded devices, to save additional power and to handle device power state interactions with overall system power state on certain platforms.

 drivers/base/power/runtime.c |    5 ++++-
 drivers/base/power/suspend.c |   23 ++++++++++-------------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff -Nru a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
@@ -14,7 +14,10 @@
 {
 	if (!dev->power.power_state)
 		return;
-	resume_device(dev);
+	if (! resume_device(dev))
+		dev->power.power_state = 0;
+
+	return;
 }
 
 

diff -Nru a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c
@@ -42,13 +42,6 @@
 	if (dev->bus && dev->bus->suspend)
 		error = dev->bus->suspend(dev,state);
 
-	if (!error) {
-		list_del(&dev->power.entry);
-		list_add(&dev->power.entry,&dpm_off);
-	} else if (error == -EAGAIN) {
-		list_del(&dev->power.entry);
-		list_add(&dev->power.entry,&dpm_off_irq);
-	}
 	return error;
 }
 

@@ -81,12 +74,16 @@
 	while(!list_empty(&dpm_active)) {
 		struct list_head * entry = dpm_active.prev;
 		struct device * dev = to_device(entry);
-		if ((error = suspend_device(dev,state))) {
-			if (error != -EAGAIN)
-				goto Error;
-			else
-				error = 0;
-		}
+		error = suspend_device(dev,state);
+
+		if (!error) {
+			list_del(&dev->power.entry);
+			list_add(&dev->power.entry,&dpm_off);
+		} else if (error == -EAGAIN) {
+			list_del(&dev->power.entry);
+			list_add(&dev->power.entry,&dpm_off_irq);
+		} else
+			goto Error;
 	}
  Done:
 	up(&dpm_sem);

ChangeSet 1.1717.7.5, 2004/05/28 10:04:44-07:00, [email protected]

[PATCH] Leave runtime suspended devices off at system resume

Currently all devices are resumed at system resume time, including any that were individually powered off ("at runtime") prior to the system suspend. In certain cases it can be nice to force back on individually suspended devices, such as the display, but hopefully this policy can be left up to userspace power managers; the kernel should probably honor the settings previously made by userspace/drivers. This seems preferable to requiring a power-conscious system to re-suspend devices after a system resume; furthermore, for certain platforms (such as XScale PXA27X) there can be disastrous consequences of powering up devices when the system is in a state incompatible with operation of the device.

Suggested patch does this:

(1) At system resume, checks power_state to see if the device was suspended prior to system suspend, and skips powering on the device if so.

(2) Does not re-suspend an already-suspended device at system suspend (using a different method than is currently employed, which reorders the list, see #3).

(3) Preserves the active/off device list order despite the above changes to suspend/resume behavior, to avoid dependency problems that tend to occur when the list is reordered.

 drivers/base/power/resume.c  |    5 ++++-
 drivers/base/power/suspend.c |    2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff -Nru a/drivers/base/power/resume.c b/drivers/base/power/resume.c
@@ -35,7 +35,10 @@
 		struct list_head * entry = dpm_off.next;
 		struct device * dev = to_device(entry);
 		list_del_init(entry);
-		resume_device(dev);
+
+		if (!dev->power.power_state)
+			resume_device(dev);
+
 		list_add_tail(entry,&dpm_active);
 	}
 }

diff -Nru a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c
@@ -39,7 +39,7 @@
 {
 	int error = 0;
 
-	if (dev->bus && dev->bus->suspend)
+	if (dev->bus && dev->bus->suspend && !dev->power.power_state)
 		error = dev->bus->suspend(dev,state);
 
 	return error;

ChangeSet 1.1717.7.6, 2004/05/28 10:05:31-07:00, [email protected]

[PATCH] Report which device failed to suspend

Based on a patch from Nickolai Zeldovich <[email protected]> but put into the proper place by me.

 drivers/base/power/suspend.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletion(-)

diff -Nru a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c
@@ -39,6 +39,8 @@
 {
 	int error = 0;
 
+	dev_dbg(dev, "suspending\n");
+
 	if (dev->bus && dev->bus->suspend && !dev->power.power_state)
 		error = dev->bus->suspend(dev,state);
 

@@ -82,8 +84,11 @@
 		} else if (error == -EAGAIN) {
 			list_del(&dev->power.entry);
 			list_add(&dev->power.entry,&dpm_off_irq);
-		} else
+		} else {
+			printk(KERN_ERR "Could not suspend device %s: "
+				"error %d\n", kobject_name(&dev->kobj), error);
 			goto Error;
+		}
 	}
  Done:
 	up(&dpm_sem);

ChangeSet 1.1717.7.7, 2004/05/28 10:31:37-07:00, [email protected]

Minor coding style fixups in resume code and added a bit of debugging help.

 drivers/base/power/runtime.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff -Nru a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
@@ -12,12 +12,11 @@
 
 static void runtime_resume(struct device * dev)
 {
+	dev_dbg(dev, "resuming\n");
 	if (!dev->power.power_state)
 		return;
-	if (! resume_device(dev))
+	if (!resume_device(dev))
 		dev->power.power_state = 0;
-
-	return;
 }
 
 

ChangeSet 1.1758, 2004/05/28 13:13:30-07:00, [email protected]

[PATCH] Fix for leave-runtime-suspended-devices-off-at-system-resume.patch

A patch to fix my previous leave-runtime-suspended-devices-off-at-system-resume patch; the new changes save a copy of power.power_state in order to know whether to resume a device, independently of mods to that field by a driver suspend routine. This fixes 2.6.7-rc1-mm1 in the same fashion as the updated 2.6.6 patch sent previously.

 drivers/base/power/resume.c  |    2 +-
 drivers/base/power/suspend.c |    2 ++
 include/linux/pm.h           |    1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff -Nru a/drivers/base/power/resume.c b/drivers/base/power/resume.c
@@ -36,7 +36,7 @@
 		struct device * dev = to_device(entry);
 		list_del_init(entry);
 
-		if (!dev->power.power_state)
+		if (!dev->power.prev_state)
 			resume_device(dev);
 
 		list_add_tail(entry,&dpm_active);

diff -Nru a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c
@@ -41,6 +41,8 @@
 
 	dev_dbg(dev, "suspending\n");
 
+	dev->power.prev_state = dev->power.power_state;
+
 	if (dev->bus && dev->bus->suspend && !dev->power.power_state)
 		error = dev->bus->suspend(dev,state);
 

diff -Nru a/include/linux/pm.h b/include/linux/pm.h
@@ -231,6 +231,7 @@
 struct dev_pm_info {
 	u32			power_state;
 #ifdef	CONFIG_PM
+	u32			prev_state;
 	u8			* saved_state;
 	atomic_t		pm_users;
 	struct device		* pm_parent;

Hi!

Seems good.

I'm seeing lots of problems with drivers & swsusp these days. Perhaps even printing names of devices as they are suspended is good idea?

Pavel

934a471f20d6580d5aad759bf0d97ddc

You mean like the current kernel tree does if you enable CONFIG_DEBUG_DRIVER? :)

greg k-h

Hi!

Well, something little less verbose but enabled by default would do the trick.
Pavel

934a471f20d6580d5aad759bf0d97ddc

I don't think the current one is all that verbose, it just prints out one line for every device being suspended. And if a user is having problems, it's quite easy to ask them to enable that option and send the output from a failed suspend.

But if you have a patch to change it, please send it.

thanks,

greg k-h