On Thu, 2006-08-31 at 21:15 -0600, Bjorn Helgaas wrote:
> > On Thu, 2006-08-31 at 17:06 -0600, Bjorn Helgaas wrote:
> >> Problem 1: acpi_reserve_io_ranges() needs to return an acpi_status
> >> like AE_OK or AE_CTRL_TERMINATE, not a -EINVAL.
> >
> > Sure great sounds. I understand AE_OK is a 0 return so I can change it
> > to AE_CTRL_TERMINATE. I don't want acpi_reserve_io_ranges to return a
> > happy state when if finds a resource type is doesn't know.
>
> Except that when the motherboard driver claims a device, it really
> should claim all the resources used by the device. It currently only
> claims I/O port resources, but I think it should also claim MMIO
> resources. Otherwise, the system resource accounting is screwed up,
> and resources consumed by the motherboard device could be mistakenly
> allocated to another device.
>
> > Kame (who helped me greatly in tracking down the source my troubles)
> > thinks that the root cause is that the device (my memory_device) has
> > both a _HID and _CID. The driver for _HID is different for _CID and the
> > driver for _CID is found before _HID and I pass the wrong device up the
> > chain.
>
> Ok, this is starting to make sense. It sounds like your memory
> device has _HID of PNP0C80 and _CID of PNP0C01 (or PNP0C02).
>
> The current ACPI driver binding algorithm in acpi_bus_find_driver()
> looks at each driver, checking whether it can match either the _HID
> or the _CID of a device. Since we try the motherboard driver first,
> it matches the memory device _CID.
Ok I reverted the motherboard driver patch and cooked up the following
patch that works for my issue.
It creates the idea that acpi_match_ids has a type of request to check
against for _HID, _CID or both. See acpi_bus_match_req. I then fix up
all the needed callers to change the API to acpi_match_ids and
acpi_bus_match and have callers can say what they want to match
against.
Then in acpi_bus_find_driver I have it do 2 passes to search for _HID
first then the _CID.
Does this look like it is in the right ballpark or should we be doing
something else? Built/tested against 2.6.18-rc4-mm3.
Signed-off-by: Keith Mannthey <kmannth@xxxxxxxxxx>
diff -urN linux-2.6.17-orig/drivers/acpi/scan.c linux-2.6.17/drivers/acpi/scan.c
--- linux-2.6.17-orig/drivers/acpi/scan.c 2006-09-01 17:11:37.000000000 -0400
+++ linux-2.6.17/drivers/acpi/scan.c 2006-09-01 18:13:53.000000000 -0400
@@ -235,13 +235,13 @@
return 0;
}
-int acpi_match_ids(struct acpi_device *device, char *ids)
+int acpi_match_ids(struct acpi_device *device, char *ids, int type)
{
- if (device->flags.hardware_id)
+ if ((device->flags.hardware_id) && (type != ACPI_BUS_MATCH_CID))
if (strstr(ids, device->pnp.hardware_id))
return 0;
- if (device->flags.compatible_ids) {
+ if ((device->flags.compatible_ids) && (type != ACPI_BUS_MATCH_HID)) {
struct acpi_compatible_id_list *cid_list = device->pnp.cid_list;
int i;
@@ -329,7 +329,8 @@
device->wakeup.flags.valid = 1;
/* Power button, Lid switch always enable wakeup */
- if (!acpi_match_ids(device, "PNP0C0D,PNP0C0C,PNP0C0E"))
+ if (!acpi_match_ids(device, "PNP0C0D,PNP0C0C,PNP0C0E",
+ ACPI_BUS_MATCH_ALL))
device->wakeup.flags.run_wake = 1;
end:
@@ -471,11 +472,11 @@
* matches the specified driver's criteria.
*/
static int
-acpi_bus_match(struct acpi_device *device, struct acpi_driver *driver)
+acpi_bus_match(struct acpi_device *device, struct acpi_driver *driver, int type)
{
if (driver && driver->ops.match)
return driver->ops.match(device, driver);
- return acpi_match_ids(device, driver->ids);
+ return acpi_match_ids(device, driver->ids, type);
}
/**
@@ -549,7 +550,7 @@
continue;
spin_unlock(&acpi_device_lock);
- if (!acpi_bus_match(dev, drv)) {
+ if (!acpi_bus_match(dev, drv, ACPI_BUS_MATCH_ALL)) {
if (!acpi_bus_driver_init(dev, drv)) {
acpi_start_single_object(dev);
atomic_inc(&drv->references);
@@ -651,7 +652,22 @@
atomic_inc(&driver->references);
spin_unlock(&acpi_device_lock);
- if (!acpi_bus_match(device, driver)) {
+ if (!acpi_bus_match(device, driver, ACPI_BUS_MATCH_HID)) {
+ result = acpi_bus_driver_init(device, driver);
+ if (!result)
+ goto Done;
+ }
+ atomic_dec(&driver->references);
+ spin_lock(&acpi_device_lock);
+ }
+
+ list_for_each_safe(node, next, &acpi_bus_drivers) {
+ struct acpi_driver *driver =
+ container_of(node, struct acpi_driver, node);
+
+ atomic_inc(&driver->references);
+ spin_unlock(&acpi_device_lock);
+ if (!acpi_bus_match(device, driver, ACPI_BUS_MATCH_ALL)) {
result = acpi_bus_driver_init(device, driver);
if (!result)
goto Done;
diff -urN linux-2.6.17-orig/drivers/pnp/pnpacpi/core.c linux-2.6.17/drivers/pnp/pnpacpi/core.c
--- linux-2.6.17-orig/drivers/pnp/pnpacpi/core.c 2006-09-01 17:11:37.000000000 -0400
+++ linux-2.6.17/drivers/pnp/pnpacpi/core.c 2006-09-01 18:03:26.000000000 -0400
@@ -41,7 +41,7 @@
;
static inline int is_exclusive_device(struct acpi_device *dev)
{
- return (!acpi_match_ids(dev, excluded_id_list));
+ return (!acpi_match_ids(dev, excluded_id_list, ACPI_BUS_MATCH_ALL));
}
/*
diff -urN linux-2.6.17-orig/include/acpi/acpi_bus.h linux-2.6.17/include/acpi/acpi_bus.h
--- linux-2.6.17-orig/include/acpi/acpi_bus.h 2006-09-01 17:11:38.000000000 -0400
+++ linux-2.6.17/include/acpi/acpi_bus.h 2006-09-01 18:00:27.000000000 -0400
@@ -79,6 +79,12 @@
ACPI_BUS_DEVICE_TYPE_COUNT
};
+enum acpi_bus_match_req {
+ ACPI_BUS_MATCH_HID = 0,
+ ACPI_BUS_MATCH_CID,
+ ACPI_BUS_MATCH_ALL
+};
+
struct acpi_driver;
struct acpi_device;
@@ -335,7 +341,7 @@
int acpi_bus_trim(struct acpi_device *start, int rmdevice);
int acpi_bus_start(struct acpi_device *device);
acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle *ejd);
-int acpi_match_ids(struct acpi_device *device, char *ids);
+int acpi_match_ids(struct acpi_device *device, char *ids, int type);
int acpi_create_dir(struct acpi_device *);
void acpi_remove_dir(struct acpi_device *);
--
VGER BF report: H 0.238464
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
