From 52870786ff5d06540efacc64ca8faa74221f10dd Mon Sep 17 00:00:00 2001 From: Mika Westerberg Date: Fri, 24 Oct 2014 16:40:54 +0200 Subject: ACPI: Use ACPI companion to match only the first physical device Commit 6ab3430129e2 ("mfd: Add ACPI support") made the MFD subdevices share the parent MFD ACPI companion if no _HID/_CID is specified for the subdevice in mfd_cell description. However, since all the subdevices share the ACPI companion, the match and modalias generation logic started to use the ACPI companion as well resulting this: # cat /sys/bus/platform/devices/HID-SENSOR-200041.6.auto/modalias acpi:INT33D1:PNP0C50: instead of the expected one # cat /sys/bus/platform/devices/HID-SENSOR-200041.6.auto/modalias platform:HID-SENSOR-200041 In other words the subdevice modalias is overwritten by the one taken from ACPI companion. This causes udev not to load the driver anymore. It is useful to be able to share the ACPI companion so that MFD subdevices (and possibly other devices as well) can access the ACPI resources even if they do not have ACPI representation in the namespace themselves. An example where this is used is Minnowboard LPC driver that creates GPIO as a subdevice among other things. Without the ACPI companion gpiolib is not able to lookup the corresponding GPIO controller from ACPI GpioIo resource. To fix this, restrict the match and modalias logic to be limited to the first (primary) physical device associated with the given ACPI comapnion. The secondary devices will still be able to access the ACPI companion, but they will be matched in a different way. Fixes: 6ab3430129e2 (mfd: Add ACPI support) Reported-by: Jarkko Nikula Signed-off-by: Mika Westerberg Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 70 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 54 insertions(+), 16 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index d670158a26c5..0476e90b2091 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -141,6 +141,53 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias, return len; } +/* + * acpi_companion_match() - Can we match via ACPI companion device + * @dev: Device in question + * + * Check if the given device has an ACPI companion and if that companion has + * a valid list of PNP IDs, and if the device is the first (primary) physical + * device associated with it. + * + * If multiple physical devices are attached to a single ACPI companion, we need + * to be careful. The usage scenario for this kind of relationship is that all + * of the physical devices in question use resources provided by the ACPI + * companion. A typical case is an MFD device where all the sub-devices share + * the parent's ACPI companion. In such cases we can only allow the primary + * (first) physical device to be matched with the help of the companion's PNP + * IDs. + * + * Additional physical devices sharing the ACPI companion can still use + * resources available from it but they will be matched normally using functions + * provided by their bus types (and analogously for their modalias). + */ +static bool acpi_companion_match(const struct device *dev) +{ + struct acpi_device *adev; + bool ret; + + adev = ACPI_COMPANION(dev); + if (!adev) + return false; + + if (list_empty(&adev->pnp.ids)) + return false; + + mutex_lock(&adev->physical_node_lock); + if (list_empty(&adev->physical_node_list)) { + ret = false; + } else { + const struct acpi_device_physical_node *node; + + node = list_first_entry(&adev->physical_node_list, + struct acpi_device_physical_node, node); + ret = node->dev == dev; + } + mutex_unlock(&adev->physical_node_lock); + + return ret; +} + /* * Creates uevent modalias field for ACPI enumerated devices. * Because the other buses does not support ACPI HIDs & CIDs. @@ -149,20 +196,14 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias, */ int acpi_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env) { - struct acpi_device *acpi_dev; int len; - acpi_dev = ACPI_COMPANION(dev); - if (!acpi_dev) - return -ENODEV; - - /* Fall back to bus specific way of modalias exporting */ - if (list_empty(&acpi_dev->pnp.ids)) + if (!acpi_companion_match(dev)) return -ENODEV; if (add_uevent_var(env, "MODALIAS=")) return -ENOMEM; - len = create_modalias(acpi_dev, &env->buf[env->buflen - 1], + len = create_modalias(ACPI_COMPANION(dev), &env->buf[env->buflen - 1], sizeof(env->buf) - env->buflen); if (len <= 0) return len; @@ -179,18 +220,12 @@ EXPORT_SYMBOL_GPL(acpi_device_uevent_modalias); */ int acpi_device_modalias(struct device *dev, char *buf, int size) { - struct acpi_device *acpi_dev; int len; - acpi_dev = ACPI_COMPANION(dev); - if (!acpi_dev) + if (!acpi_companion_match(dev)) return -ENODEV; - /* Fall back to bus specific way of modalias exporting */ - if (list_empty(&acpi_dev->pnp.ids)) - return -ENODEV; - - len = create_modalias(acpi_dev, buf, size -1); + len = create_modalias(ACPI_COMPANION(dev), buf, size -1); if (len <= 0) return len; buf[len++] = '\n'; @@ -853,6 +888,9 @@ const struct acpi_device_id *acpi_match_device(const struct acpi_device_id *ids, if (!ids || !handle || acpi_bus_get_device(handle, &adev)) return NULL; + if (!acpi_companion_match(dev)) + return NULL; + return __acpi_match_device(adev, ids); } EXPORT_SYMBOL_GPL(acpi_match_device); -- cgit v1.2.3 From df9ff91801da603079018f21a9412385b62f0f8e Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 29 Oct 2014 11:33:43 +0800 Subject: Revert "ACPI / EC: Add support to disallow QR_EC to be issued before completing previous QR_EC" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is reported that the following commit breaks Samsung hardware: Commit: 558e4736f2e1b0e6323adf7a5e4df77ed6cfc1a4. Subject: ACPI / EC: Add support to disallow QR_EC to be issued before completing previous QR_EC Which means the Samsung behavior conflicts with the Acer behavior. 1. Samsung may behave like: [ +event 1 ] SCI_EVT set [ +event 2 ] SCI_EVT set write QR_EC read event [ -event 1 ] SCI_EVT clear Without the above commit, Samsung can work: [ +event 1 ] SCI_EVT set [ +event 2 ] SCI_EVT set write QR_EC CAN prepare next QR_EC as SCI_EVT=1 read event [ -event 1 ] SCI_EVT clear write QR_EC read event [ -event 2 ] SCI_EVT clear With the above commit, Samsung cannot work: [ +event 1 ] SCI_EVT set [ +event 2 ] SCI_EVT set write QR_EC read event [ -event 1 ] SCI_EVT clear CANNOT prepare next QR_EC as SCI_EVT=0 2. Acer may behave like: [ +event 1 ] SCI_EVT set [ +event 2 ] write QR_EC read event [ -event 1 ] SCI_EVT clear [ +event 2 ] SCI_EVT set Without the above commit, Acer cannot work when there is only 1 event: [ +event 1 ] SCI_EVT set write QR_EC can prepared next QR_EC as SCI_EVT=1 read event [ -event 1 ] SCI_EVT clear CANNOT write QR_EC as SCI_EVT=0 With the above commit, Acer can work: [ +event 1 ] SCI_EVT set [ +event 2 ] write QR_EC read event [ -event 1 ] SCI_EVT set can prepare next QR_EC because SCI_EVT=0 CAN write QR_EC as SCI_EVT=1 Since Acer can also work with only the following commit applied: Commit: 3afcf2ece453e1a8c2c6de19cdf06da3772a1b08 Subject: ACPI / EC: Add support to disallow QR_EC to be issued when SCI_EVT isn't set commit 558e4736f2e1b0e6323adf7a5e4df77ed6cfc1a4 can be reverted. Fixes: 558e4736f2e1 (ACPI / EC: Add support to disallow QR_EC to be issued ...) Link: https://bugzilla.kernel.org/show_bug.cgi?id=44161 Reported-and-tested-by: Ortwin Glück Signed-off-by: Lv Zheng Cc: 3.17+ # 3.17+ Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 3d304ff7f095..31b699f2d088 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -334,13 +334,13 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, pr_debug("***** Command(%s) started *****\n", acpi_ec_cmd_string(t->command)); start_transaction(ec); - spin_unlock_irqrestore(&ec->lock, tmp); - ret = ec_poll(ec); - spin_lock_irqsave(&ec->lock, tmp); if (ec->curr->command == ACPI_EC_COMMAND_QUERY) { clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); pr_debug("***** Event stopped *****\n"); } + spin_unlock_irqrestore(&ec->lock, tmp); + ret = ec_poll(ec); + spin_lock_irqsave(&ec->lock, tmp); pr_debug("***** Command(%s) stopped *****\n", acpi_ec_cmd_string(t->command)); ec->curr = NULL; -- cgit v1.2.3 From 79149001105f18bd2285ada109f9229ea24a7571 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Wed, 29 Oct 2014 11:33:49 +0800 Subject: ACPI / EC: Fix regression due to conflicting firmware behavior between Samsung and Acer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is reported that Samsung laptops that need to poll events are broken by the following commit: Commit 3afcf2ece453e1a8c2c6de19cdf06da3772a1b08 Subject: ACPI / EC: Add support to disallow QR_EC to be issued when SCI_EVT isn't set The behaviors of the 2 vendor firmwares are conflict: 1. Acer: OSPM shouldn't issue QR_EC unless SCI_EVT is set, firmware automatically sets SCI_EVT as long as there is event queued up. 2. Samsung: OSPM should issue QR_EC whatever SCI_EVT is set, firmware returns 0 when there is no event queued up. This patch is a quick fix to distinguish the behaviors to make Acer behavior only effective for Acer EC firmware so that the breakages on Samsung EC firmware can be avoided. Fixes: 3afcf2ece453 (ACPI / EC: Add support to disallow QR_EC to be issued ...) Link: https://bugzilla.kernel.org/show_bug.cgi?id=44161 Reported-and-tested-by: Ortwin Glück Signed-off-by: Lv Zheng Cc: 3.17+ # 3.17+ [ rjw : Subject ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 31b699f2d088..5f9b74b9b71f 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -126,6 +126,7 @@ static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */ static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */ static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */ static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */ +static int EC_FLAGS_QUERY_HANDSHAKE; /* Needs QR_EC issued when SCI_EVT set */ /* -------------------------------------------------------------------------- * Transaction Management @@ -236,13 +237,8 @@ static bool advance_transaction(struct acpi_ec *ec) } return wakeup; } else { - /* - * There is firmware refusing to respond QR_EC when SCI_EVT - * is not set, for which case, we complete the QR_EC - * without issuing it to the firmware. - * https://bugzilla.kernel.org/show_bug.cgi?id=86211 - */ - if (!(status & ACPI_EC_FLAG_SCI) && + if (EC_FLAGS_QUERY_HANDSHAKE && + !(status & ACPI_EC_FLAG_SCI) && (t->command == ACPI_EC_COMMAND_QUERY)) { t->flags |= ACPI_EC_COMMAND_POLL; t->rdata[t->ri++] = 0x00; @@ -1011,6 +1007,18 @@ static int ec_enlarge_storm_threshold(const struct dmi_system_id *id) return 0; } +/* + * Acer EC firmware refuses to respond QR_EC when SCI_EVT is not set, for + * which case, we complete the QR_EC without issuing it to the firmware. + * https://bugzilla.kernel.org/show_bug.cgi?id=86211 + */ +static int ec_flag_query_handshake(const struct dmi_system_id *id) +{ + pr_debug("Detected the EC firmware requiring QR_EC issued when SCI_EVT set\n"); + EC_FLAGS_QUERY_HANDSHAKE = 1; + return 0; +} + /* * On some hardware it is necessary to clear events accumulated by the EC during * sleep. These ECs stop reporting GPEs until they are manually polled, if too @@ -1085,6 +1093,9 @@ static struct dmi_system_id ec_dmi_table[] __initdata = { { ec_clear_on_resume, "Samsung hardware", { DMI_MATCH(DMI_SYS_VENDOR, "SAMSUNG ELECTRONICS CO., LTD.")}, NULL}, + { + ec_flag_query_handshake, "Acer hardware", { + DMI_MATCH(DMI_SYS_VENDOR, "Acer"), }, NULL}, {}, }; -- cgit v1.2.3