On Tue, Dec 19, 2006 at 07:19:36PM -0800, David Brownell wrote:
> On Tuesday 19 December 2006 4:09 pm, Matthew Garrett wrote:
> > I'm sorry, which bit of "Don't break userspace API without adequate
> > prior warning and with a workable replacement" is difficult to
> > understand?
>
> What part of "it was already broken" do YOU not understand? The
> whole notion is unsustainable. It doesn't work cross-platform, or
> for multiple bus types. It confuses system-wide suspend mechanisms
> with runtime mechanisms. It breaks guaranteed parent/child ordering
> of suspend/resume calls. (And more...)
Linux is utterly riddled with broken APIs. It's possible to see that as
a downside of the "Release early, release often" model, but the
advantage is that we get the opportunity to determine how these
interfaces are broken. Based on that, we can either improve the existing
interface or decide that it's broken beyond repair and design a new one.
What we don't do is decide that an interface is broken, deprecate it
and in the same release break it even for the cases where it
previously worked. That's just insane.
> Let us know when you get tired of whining and want to move on to
> getting a real solution to the set of problems here. I've pointed
> out that reverting Linus' patch would be one option to get your
> short term issue rsolved ... that would remove a capability from
> PCI drivers, but you could then use that deprecated mechanism.
> I've also pointed out that you could start working towards a real
> long term solution.
I could, and in the long run I intend to. On the other hand, I don't
expect to have enough time to fix every single in-tree network driver
before 2.6.20, so...
> Do you have an alternate solution?
How about something like this? Entirely untested, but I think it shows
the basic idea.
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index f9c903b..4865918 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -597,6 +597,17 @@ static int platform_resume(struct device * dev)
return ret;
}
+static int platform_requires_disabled_interrupts(struct device * dev)
+{
+ int ret = 0;
+
+ if (dev->driver && (dev->driver->resume_early
+ || dev->driver->suspend_late))
+ ret = 1;
+
+ return ret;
+}
+
struct bus_type platform_bus_type = {
.name = "platform",
.dev_attrs = platform_dev_attrs,
@@ -604,8 +615,9 @@ struct bus_type platform_bus_type = {
.uevent = platform_uevent,
.suspend = platform_suspend,
.suspend_late = platform_suspend_late,
- .resume_early = platform_resume_early,
+ .resume_early = platform_resume_early,
.resume = platform_resume,
+ .requires_disabled_interrupts = platform_requires_disabled_interrupts,
};
EXPORT_SYMBOL_GPL(platform_bus_type);
diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
index 2d47517..97c6d65 100644
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -46,7 +46,8 @@ static ssize_t state_store(struct device * dev, struct device_attribute *attr, c
int error = -EINVAL;
/* disallow incomplete suspend sequences */
- if (dev->bus && (dev->bus->suspend_late || dev->bus->resume_early))
+ if (dev->bus && dev->bus->requires_disabled_interrupts
+ && dev->bus->requries_disabled_interrupts())
return error;
state.event = PM_EVENT_SUSPEND;
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e5ae3a0..9808d42 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -351,6 +351,18 @@ static int pci_device_resume(struct device * dev)
return error;
}
+static int pci_device_requires_disabled_interrupts(struct device * dev)
+{
+ int error = 0;
+ struct pci_dev * pci_dev = to_pci_dev(dev);
+ struct pci_driver * drv = pci_dev->driver;
+
+ if (drv && (drv->resume_early || drv_suspend_late))
+ error = 1;
+
+ return error;
+}
+
static int pci_device_resume_early(struct device * dev)
{
int error = 0;
@@ -569,6 +581,7 @@ struct bus_type pci_bus_type = {
.suspend_late = pci_device_suspend_late,
.resume_early = pci_device_resume_early,
.resume = pci_device_resume,
+ .requires_disabled_interrupts = pci_requires_disabled_interrupts,
.shutdown = pci_device_shutdown,
.dev_attrs = pci_dev_attrs,
};
diff --git a/include/linux/device.h b/include/linux/device.h
index 49ab53c..0686234 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -59,6 +59,7 @@ struct bus_type {
int (*suspend)(struct device * dev, pm_message_t state);
int (*suspend_late)(struct device * dev, pm_message_t state);
int (*resume_early)(struct device * dev);
+ int (*requires_disabled_interrupts)(struct device * dev);
int (*resume)(struct device * dev);
};
--
Matthew Garrett | [email protected]
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
