From 82428b62aa6294ea640c7e920a9224ecaf46db65 Mon Sep 17 00:00:00 2001 From: David Brownell Date: Mon, 9 May 2005 08:07:00 -0700 Subject: [PATCH 1/2] [PATCH] Driver Core: pm diagnostics update, check for errors This patch includes various tweaks in the messaging that appears during system pm state transitions: * Warn about certain illegal calls in the device tree, like resuming child before parent or suspending parent before child. This could happen easily enough through sysfs, or in some cases when drivers use device_pm_set_parent(). * Be more consistent about dev_dbg() tracing ... do it for resume() and shutdown() too, and never if the driver doesn't have that method. * Say which type of system sleep state is being entered. Except for the warnings, these only affect debug messaging. Signed-off-by: David Brownell Acked-by: Pavel Machek Signed-off-by: Greg Kroah-Hartman --- drivers/base/power/resume.c | 11 ++++++++++- drivers/base/power/shutdown.c | 13 +++++++------ drivers/base/power/suspend.c | 17 +++++++++++++++-- kernel/power/main.c | 6 +++--- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/drivers/base/power/resume.c b/drivers/base/power/resume.c index f8f5055754d6..26468971ef5a 100644 --- a/drivers/base/power/resume.c +++ b/drivers/base/power/resume.c @@ -22,8 +22,17 @@ extern int sysdev_resume(void); int resume_device(struct device * dev) { - if (dev->bus && dev->bus->resume) + if (dev->power.pm_parent + && dev->power.pm_parent->power.power_state) { + dev_err(dev, "PM: resume from %d, parent %s still %d\n", + dev->power.power_state, + dev->power.pm_parent->bus_id, + dev->power.pm_parent->power.power_state); + } + if (dev->bus && dev->bus->resume) { + dev_dbg(dev,"resuming\n"); return dev->bus->resume(dev); + } return 0; } diff --git a/drivers/base/power/shutdown.c b/drivers/base/power/shutdown.c index d1e023fbe169..97979901c149 100644 --- a/drivers/base/power/shutdown.c +++ b/drivers/base/power/shutdown.c @@ -25,8 +25,10 @@ int device_detach_shutdown(struct device * dev) return 0; if (dev->detach_state == DEVICE_PM_OFF) { - if (dev->driver && dev->driver->shutdown) + if (dev->driver && dev->driver->shutdown) { + dev_dbg(dev, "shutdown\n"); dev->driver->shutdown(dev); + } return 0; } return dpm_runtime_suspend(dev, dev->detach_state); @@ -52,13 +54,12 @@ void device_shutdown(void) struct device * dev; down_write(&devices_subsys.rwsem); - list_for_each_entry_reverse(dev, &devices_subsys.kset.list, kobj.entry) { - pr_debug("shutting down %s: ", dev->bus_id); + list_for_each_entry_reverse(dev, &devices_subsys.kset.list, + kobj.entry) { if (dev->driver && dev->driver->shutdown) { - pr_debug("Ok\n"); + dev_dbg(dev, "shutdown\n"); dev->driver->shutdown(dev); - } else - pr_debug("Ignored.\n"); + } } up_write(&devices_subsys.rwsem); diff --git a/drivers/base/power/suspend.c b/drivers/base/power/suspend.c index a0b5cf689e63..0ec44ef840be 100644 --- a/drivers/base/power/suspend.c +++ b/drivers/base/power/suspend.c @@ -39,12 +39,25 @@ int suspend_device(struct device * dev, pm_message_t state) { int error = 0; - dev_dbg(dev, "suspending\n"); + if (dev->power.power_state) { + dev_dbg(dev, "PM: suspend %d-->%d\n", + dev->power.power_state, state); + } + if (dev->power.pm_parent + && dev->power.pm_parent->power.power_state) { + dev_err(dev, + "PM: suspend %d->%d, parent %s already %d\n", + dev->power.power_state, state, + dev->power.pm_parent->bus_id, + dev->power.pm_parent->power.power_state); + } dev->power.prev_state = dev->power.power_state; - if (dev->bus && dev->bus->suspend && !dev->power.power_state) + if (dev->bus && dev->bus->suspend && !dev->power.power_state) { + dev_dbg(dev, "suspending\n"); error = dev->bus->suspend(dev, state); + } return error; } diff --git a/kernel/power/main.c b/kernel/power/main.c index 7960ddf04a57..4cdebc972ff2 100644 --- a/kernel/power/main.c +++ b/kernel/power/main.c @@ -156,14 +156,14 @@ static int enter_state(suspend_state_t state) goto Unlock; } - pr_debug("PM: Preparing system for suspend\n"); + pr_debug("PM: Preparing system for %s sleep\n", pm_states[state]); if ((error = suspend_prepare(state))) goto Unlock; - pr_debug("PM: Entering state.\n"); + pr_debug("PM: Entering %s sleep\n", pm_states[state]); error = suspend_enter(state); - pr_debug("PM: Finishing up.\n"); + pr_debug("PM: Finishing wakeup.\n"); suspend_finish(state); Unlock: up(&pm_sem); From 0b405a0f7e4d4d18fd1fe46ddf5ff465443036ab Mon Sep 17 00:00:00 2001 From: David Brownell Date: Thu, 12 May 2005 12:06:27 -0700 Subject: [PATCH 2/2] [PATCH] Driver Core: remove driver model detach_state The driver model has a "detach_state" mechanism that: - Has never been used by any in-kernel drive; - Is superfluous, since driver remove() methods can do the same thing; - Became buggy when the suspend() parameter changed semantics and type; - Could self-deadlock when called from certain suspend contexts; - Is effectively wasted documentation, object code, and headspace. This removes that "detach_state" mechanism; net code shrink, as well as a per-device saving in the driver model and sysfs. Signed-off-by: David Brownell Signed-off-by: Greg Kroah-Hartman --- Documentation/filesystems/sysfs-pci.txt | 6 +-- Documentation/power/devices.txt | 21 ---------- Documentation/powerpc/hvcs.txt | 4 +- drivers/base/Makefile | 2 +- drivers/base/bus.c | 1 - drivers/base/core.c | 3 -- drivers/base/interface.c | 51 ------------------------- drivers/base/power/power.h | 11 ------ drivers/base/power/shutdown.c | 16 -------- include/linux/device.h | 3 -- 10 files changed, 5 insertions(+), 113 deletions(-) delete mode 100644 drivers/base/interface.c diff --git a/Documentation/filesystems/sysfs-pci.txt b/Documentation/filesystems/sysfs-pci.txt index e97d024eae77..988a62fae11f 100644 --- a/Documentation/filesystems/sysfs-pci.txt +++ b/Documentation/filesystems/sysfs-pci.txt @@ -7,7 +7,6 @@ that support it. For example, a given bus might look like this: |-- 0000:17:00.0 | |-- class | |-- config - | |-- detach_state | |-- device | |-- irq | |-- local_cpus @@ -19,7 +18,7 @@ that support it. For example, a given bus might look like this: | |-- subsystem_device | |-- subsystem_vendor | `-- vendor - `-- detach_state + `-- ... The topmost element describes the PCI domain and bus number. In this case, the domain number is 0000 and the bus number is 17 (both values are in hex). @@ -31,7 +30,6 @@ files, each with their own function. ---- -------- class PCI class (ascii, ro) config PCI config space (binary, rw) - detach_state connection status (bool, rw) device PCI device (ascii, ro) irq IRQ number (ascii, ro) local_cpus nearby CPU mask (cpumask, ro) @@ -85,4 +83,4 @@ useful return codes should be provided. Legacy resources are protected by the HAVE_PCI_LEGACY define. Platforms wishing to support legacy functionality should define it and provide -pci_legacy_read, pci_legacy_write and pci_mmap_legacy_page_range functions. \ No newline at end of file +pci_legacy_read, pci_legacy_write and pci_mmap_legacy_page_range functions. diff --git a/Documentation/power/devices.txt b/Documentation/power/devices.txt index 5d4ae9a39f1d..f987afe43e28 100644 --- a/Documentation/power/devices.txt +++ b/Documentation/power/devices.txt @@ -207,27 +207,6 @@ SYSTEM_SHUTDOWN, I do not understand this one too much. probably event #READY_AFTER_RESUME # -Driver Detach Power Management - -The kernel now supports the ability to place a device in a low-power -state when it is detached from its driver, which happens when its -module is removed. - -Each device contains a 'detach_state' file in its sysfs directory -which can be used to control this state. Reading from this file -displays what the current detach state is set to. This is 0 (On) by -default. A user may write a positive integer value to this file in the -range of 1-4 inclusive. - -A value of 1-3 will indicate the device should be placed in that -low-power state, which will cause ->suspend() to be called for that -device. A value of 4 indicates that the device should be shutdown, so -->shutdown() will be called for that device. - -The driver is responsible for reinitializing the device when the -module is re-inserted during it's ->probe() (or equivalent) method. -The driver core will not call any extra functions when binding the -device to the driver. pm_message_t meaning diff --git a/Documentation/powerpc/hvcs.txt b/Documentation/powerpc/hvcs.txt index c0a62e116e6e..dca75cbda6f8 100644 --- a/Documentation/powerpc/hvcs.txt +++ b/Documentation/powerpc/hvcs.txt @@ -347,8 +347,8 @@ address that is created by firmware. An example vty-server sysfs entry looks like the following: Pow5:/sys/bus/vio/drivers/hvcs/30000004 # ls - . current_vty devspec name partner_vtys - .. detach_state index partner_clcs vterm_state + . current_vty devspec name partner_vtys + .. index partner_clcs vterm_state Each entry is provided, by default with a "name" attribute. Reading the "name" attribute will reveal the device type as shown in the following diff --git a/drivers/base/Makefile b/drivers/base/Makefile index 6662b545e0a9..a47928a2e575 100644 --- a/drivers/base/Makefile +++ b/drivers/base/Makefile @@ -1,6 +1,6 @@ # Makefile for the Linux device tree -obj-y := core.o sys.o interface.o bus.o \ +obj-y := core.o sys.o bus.o \ driver.o class.o class_simple.o platform.o \ cpu.o firmware.o init.o map.o dmapool.o \ attribute_container.o transport_class.o diff --git a/drivers/base/bus.c b/drivers/base/bus.c index 2b3902c867da..3cb04bb04c2b 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -390,7 +390,6 @@ void device_release_driver(struct device * dev) sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj)); sysfs_remove_link(&dev->kobj, "driver"); list_del_init(&dev->driver_list); - device_detach_shutdown(dev); if (drv->remove) drv->remove(dev); dev->driver = NULL; diff --git a/drivers/base/core.c b/drivers/base/core.c index 268a9c8d168b..d21eb7744496 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -31,8 +31,6 @@ int (*platform_notify_remove)(struct device * dev) = NULL; #define to_dev(obj) container_of(obj, struct device, kobj) #define to_dev_attr(_attr) container_of(_attr, struct device_attribute, attr) -extern struct attribute * dev_default_attrs[]; - static ssize_t dev_attr_show(struct kobject * kobj, struct attribute * attr, char * buf) { @@ -89,7 +87,6 @@ static void device_release(struct kobject * kobj) static struct kobj_type ktype_device = { .release = device_release, .sysfs_ops = &dev_sysfs_ops, - .default_attrs = dev_default_attrs, }; diff --git a/drivers/base/interface.c b/drivers/base/interface.c deleted file mode 100644 index bd515843a0cb..000000000000 --- a/drivers/base/interface.c +++ /dev/null @@ -1,51 +0,0 @@ -/* - * drivers/base/interface.c - common driverfs interface that's exported to - * the world for all devices. - * - * Copyright (c) 2002-3 Patrick Mochel - * Copyright (c) 2002-3 Open Source Development Labs - * - * This file is released under the GPLv2 - * - */ - -#include -#include -#include -#include - -/** - * detach_state - control the default power state for the device. - * - * This is the state the device enters when it's driver module is - * unloaded. The value is an unsigned integer, in the range of 0-4. - * '0' indicates 'On', so no action will be taken when the driver is - * unloaded. This is the default behavior. - * '4' indicates 'Off', meaning the driver core will call the driver's - * shutdown method to quiesce the device. - * 1-3 indicate a low-power state for the device to enter via the - * driver's suspend method. - */ - -static ssize_t detach_show(struct device * dev, char * buf) -{ - return sprintf(buf, "%u\n", dev->detach_state); -} - -static ssize_t detach_store(struct device * dev, const char * buf, size_t n) -{ - u32 state; - state = simple_strtoul(buf, NULL, 10); - if (state > 4) - return -EINVAL; - dev->detach_state = state; - return n; -} - -static DEVICE_ATTR(detach_state, 0644, detach_show, detach_store); - - -struct attribute * dev_default_attrs[] = { - &dev_attr_detach_state.attr, - NULL, -}; diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h index e5eda746f2a6..2e700d795cf1 100644 --- a/drivers/base/power/power.h +++ b/drivers/base/power/power.h @@ -1,18 +1,7 @@ - - -enum { - DEVICE_PM_ON, - DEVICE_PM1, - DEVICE_PM2, - DEVICE_PM3, - DEVICE_PM_OFF, -}; - /* * shutdown.c */ -extern int device_detach_shutdown(struct device *); extern void device_shutdown(void); diff --git a/drivers/base/power/shutdown.c b/drivers/base/power/shutdown.c index 97979901c149..f50a08be424b 100644 --- a/drivers/base/power/shutdown.c +++ b/drivers/base/power/shutdown.c @@ -19,22 +19,6 @@ extern struct subsystem devices_subsys; -int device_detach_shutdown(struct device * dev) -{ - if (!dev->detach_state) - return 0; - - if (dev->detach_state == DEVICE_PM_OFF) { - if (dev->driver && dev->driver->shutdown) { - dev_dbg(dev, "shutdown\n"); - dev->driver->shutdown(dev); - } - return 0; - } - return dpm_runtime_suspend(dev, dev->detach_state); -} - - /** * We handle system devices differently - we suspend and shut them * down last and resume them first. That way, we don't do anything stupid like diff --git a/include/linux/device.h b/include/linux/device.h index cf470459fa69..df94c0de53f2 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -273,9 +273,6 @@ struct device { BIOS data relevant to device) */ struct dev_pm_info power; - u32 detach_state; /* State to enter when device is - detached from its driver. */ - u64 *dma_mask; /* dma mask (if dma'able device) */ u64 coherent_dma_mask;/* Like dma_mask, but for alloc_coherent mappings as