All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] dln2: add support for ACPI
@ 2014-12-16 16:12 Octavian Purdila
  2014-12-16 16:12 ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Octavian Purdila
                   ` (3 more replies)
  0 siblings, 4 replies; 25+ messages in thread
From: Octavian Purdila @ 2014-12-16 16:12 UTC (permalink / raw)
  To: linus.walleij, lee.jones, rjw
  Cc: johan, linux-usb, linux-kernel, linux-gpio, heikki.krogerus,
	mika.westerberg, linux-acpi, Octavian Purdila

This patch sets adds support for ACPI enumeration for devices
connected to the DLN2 bridge via a custom ACPI table.

The first two patches fix a couple of issues with the ACPI load/unload
table APIs.

The 3rd patch adds ACPI support to the MFD driver and the last patch
configures pull-up/pull-down on GPIO pins based on the APCI
configuration.

Octavian Purdila (4):
  ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id
  ACPICA: don't release ACPI_MTX_TABLES in
    acpi_tb_install_standard_table
  mfd: dln2: add support for ACPI
  gpio: dln2: add support for ACPI pin configuration

 Documentation/acpi/dln2-acpi.txt |  62 ++++++++++++++++++
 drivers/acpi/acpica/tbinstal.c   |   1 -
 drivers/acpi/acpica/tbxface.c    |   7 ++
 drivers/gpio/gpio-dln2.c         |  76 ++++++++++++++++++++++
 drivers/mfd/dln2.c               | 134 +++++++++++++++++++++++++++++++++++++++
 5 files changed, 279 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/acpi/dln2-acpi.txt

-- 
1.9.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id
  2014-12-16 16:12 [PATCH 0/4] dln2: add support for ACPI Octavian Purdila
@ 2014-12-16 16:12 ` Octavian Purdila
  2014-12-16 18:14   ` Sergei Shtylyov
                     ` (2 more replies)
  2014-12-16 16:12 ` [PATCH 2/4] ACPICA: don't release ACPI_MTX_TABLES in acpi_tb_install_standard_table Octavian Purdila
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 25+ messages in thread
From: Octavian Purdila @ 2014-12-16 16:12 UTC (permalink / raw)
  To: linus.walleij, lee.jones, rjw
  Cc: johan, linux-usb, linux-kernel, linux-gpio, heikki.krogerus,
	mika.westerberg, linux-acpi, Octavian Purdila

acpi_tb_delete_namespace_by_owner expects ACPI_MTX_INTERPRETER to be
taken. This fixes the following issue:

ACPI Error: Mutex [0x0] is not acquired, cannot release (20141107/utmutex-322)
Call Trace:
  [<ffffffff81b0bd28>] dump_stack+0x4f/0x7b
  [<ffffffff81546bfc>] acpi_ut_release_mutex+0x47/0x67
  [<ffffffff81542cf1>] acpi_tb_delete_namespace_by_owner+0x57/0x8d
  [<ffffffff81543ef1>] acpi_unload_table_id+0x3a/0x5e

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 drivers/acpi/acpica/tbxface.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
index 6482b0d..9520ae1 100644
--- a/drivers/acpi/acpica/tbxface.c
+++ b/drivers/acpi/acpica/tbxface.c
@@ -281,6 +281,11 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
 
 	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
 
+	status = acpi_ut_acquire_mutex(ACPI_MTX_INTERPRETER);
+	if (ACPI_FAILURE(status)) {
+		return_ACPI_STATUS(status);
+	}
+
 	/* Find table in the global table list */
 	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
 		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
@@ -297,6 +302,8 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
 		acpi_tb_set_table_loaded_flag(i, FALSE);
 		break;
 	}
+
+	(void)acpi_ut_release_mutex(ACPI_MTX_INTERPRETER);
 	return_ACPI_STATUS(status);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/4] ACPICA: don't release ACPI_MTX_TABLES in acpi_tb_install_standard_table
  2014-12-16 16:12 [PATCH 0/4] dln2: add support for ACPI Octavian Purdila
  2014-12-16 16:12 ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Octavian Purdila
@ 2014-12-16 16:12 ` Octavian Purdila
  2014-12-16 16:12 ` [PATCH 3/4] mfd: dln2: add support for ACPI Octavian Purdila
  2014-12-16 16:12 ` [PATCH 4/4] gpio: dln2: add support for ACPI pin configuration Octavian Purdila
  3 siblings, 0 replies; 25+ messages in thread
From: Octavian Purdila @ 2014-12-16 16:12 UTC (permalink / raw)
  To: linus.walleij, lee.jones, rjw
  Cc: johan, linux-usb, linux-kernel, linux-gpio, heikki.krogerus,
	mika.westerberg, linux-acpi, Octavian Purdila

ACPI_MTX_TABLES is taken and released by the callers of
acpi_tb_install_standard_table so releasing it in the function itself
is causing the following error if the table is reloaded:

ACPI Error: Mutex [0x2] is not acquired, cannot release (20141107/utmutex-321)
Call Trace:
  [<ffffffff81b0bd48>] dump_stack+0x4f/0x7b
  [<ffffffff81546bf5>] acpi_ut_release_mutex+0x47/0x67
  [<ffffffff81544357>] acpi_load_table+0x73/0xcb

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 drivers/acpi/acpica/tbinstal.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/acpi/acpica/tbinstal.c b/drivers/acpi/acpica/tbinstal.c
index 755b90c..c0b39f3 100644
--- a/drivers/acpi/acpica/tbinstal.c
+++ b/drivers/acpi/acpica/tbinstal.c
@@ -346,7 +346,6 @@ acpi_tb_install_standard_table(acpi_physical_address address,
 				 */
 				acpi_tb_uninstall_table(&new_table_desc);
 				*table_index = i;
-				(void)acpi_ut_release_mutex(ACPI_MTX_TABLES);
 				return_ACPI_STATUS(AE_OK);
 			}
 		}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/4] mfd: dln2: add support for ACPI
  2014-12-16 16:12 [PATCH 0/4] dln2: add support for ACPI Octavian Purdila
  2014-12-16 16:12 ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Octavian Purdila
  2014-12-16 16:12 ` [PATCH 2/4] ACPICA: don't release ACPI_MTX_TABLES in acpi_tb_install_standard_table Octavian Purdila
@ 2014-12-16 16:12 ` Octavian Purdila
  2015-01-20 15:20     ` Lee Jones
  2015-01-22  2:00   ` Rafael J. Wysocki
  2014-12-16 16:12 ` [PATCH 4/4] gpio: dln2: add support for ACPI pin configuration Octavian Purdila
  3 siblings, 2 replies; 25+ messages in thread
From: Octavian Purdila @ 2014-12-16 16:12 UTC (permalink / raw)
  To: linus.walleij, lee.jones, rjw
  Cc: johan, linux-usb, linux-kernel, linux-gpio, heikki.krogerus,
	mika.westerberg, linux-acpi, Octavian Purdila

This patch adds support to load a custom ACPI table that describes
devices connected via the DLN2 USB to I2C/SPI/GPIO bridge.

The ACPI table can be loaded either externally (from QEMU or with
CONFIG_ACPI_CUSTOM_DSDT) or it can be loaded as firmware file with the
name dln2.aml. The driver looks for an ACPI device entry with _HID set
to "DLN20000" and makes it the ACPI companion for DLN2 USB
sub-drivers.

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 Documentation/acpi/dln2-acpi.txt |  62 ++++++++++++++++++
 drivers/mfd/dln2.c               | 134 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 196 insertions(+)
 create mode 100644 Documentation/acpi/dln2-acpi.txt

diff --git a/Documentation/acpi/dln2-acpi.txt b/Documentation/acpi/dln2-acpi.txt
new file mode 100644
index 0000000..d76605f
--- /dev/null
+++ b/Documentation/acpi/dln2-acpi.txt
@@ -0,0 +1,62 @@
+Diolan DLN2 custom APCI table
+
+The Diolan DLN2 is an USB to I2C/SPI/GPIO bridge and as such it can be used to
+connect to various I2C or SPI devices. Because these busses lack an enumeration
+protocol, the driver obtains various information about the device (such as I2C
+address and GPIO pins) from either ACPI or device tree.
+
+To allow enumerating devices and their properties via ACPI, the Diolan
+driver looks for an ACPI tree with the root _HID set to "DLN20000". If
+it finds such an ACPI object it will set the ACPI companion to the
+DLN2 MFD driver and from their it will be propagated to all its
+sub-devices (I2C, GPIO, SPI).
+
+The user can either load the custom DSDT table with three methods:
+
+1. Via QEMU (see -acpitable)
+
+2. Via the CONFIG_ACPI_CUSTOM_DSDT kernel config option (see
+Documentation/acpi/dsdt-override.txt)
+
+3. By placing the custom DSDT in the firmware paths in a file name
+dln2.aml.
+
+Here is an example ACPI table that enumerates a BMC150 accelerometer
+and defines its I2C address and GPIO pin used as an interrupt source:
+
+DefinitionBlock ("ssdt.aml", "SSDT", 1, "INTEL ", "CpuDptf", 0x00000003)
+{
+	Device (DLN0)
+	{
+		Name (_ADR, Zero)
+		Name (_HID, "DLN2000")
+
+		Device (STAC)
+		{
+			Name (_ADR, Zero)
+			Name (_HID, "BMC150A")
+			Name (_CID, "INTACCL")
+			Name (_UID, One)
+
+			Method (_CRS, 0, Serialized)
+			{
+				Name (RBUF, ResourceTemplate ()
+				{
+					I2cSerialBus (0x0010, ControllerInitiated, 0x00061A80,
+						      AddressingMode7Bit, "\\DLN0",
+						      0x00, ResourceConsumer, ,)
+
+					GpioInt (Level, ActiveHigh, Exclusive, PullDown, 0x0000,
+						 "\\DLN0", 0x00, ResourceConsumer, , )
+					{ // Pin list
+						0
+					}
+				})
+				Return (RBUF)
+		       }
+		}
+	}
+}
+
+The resources defined in the devices under the DLN0 are those
+supported by the I2C, GPIO and SPI sub-systems.
diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
index f9c4a0b..93f6d1d 100644
--- a/drivers/mfd/dln2.c
+++ b/drivers/mfd/dln2.c
@@ -23,6 +23,8 @@
 #include <linux/mfd/core.h>
 #include <linux/mfd/dln2.h>
 #include <linux/rculist.h>
+#include <linux/acpi.h>
+#include <linux/firmware.h>
 
 struct dln2_header {
 	__le16 size;
@@ -714,6 +716,134 @@ static void dln2_stop(struct dln2_dev *dln2)
 
 	dln2_stop_rx_urbs(dln2);
 }
+
+#if IS_ENABLED(CONFIG_ACPI)
+
+static struct dln2_acpi_info {
+	const struct firmware *fw;
+	acpi_owner_id table_id;
+	struct acpi_device *dev;
+	int users;
+} dln2_acpi_info;
+
+static DEFINE_MUTEX(dln2_acpi_lock);
+
+static acpi_status dln2_find_acpi_handle(acpi_handle handle, u32 level,
+					 void *ctxt, void **retv)
+{
+	acpi_handle *dln2_handle = (acpi_handle *)retv;
+
+	*dln2_handle = handle;
+
+	return AE_CTRL_TERMINATE;
+}
+
+static void dln2_probe_acpi(struct dln2_dev *dln2)
+{
+	struct device *dev = &dln2->interface->dev;
+	struct dln2_acpi_info *ai = &dln2_acpi_info;
+	acpi_handle h = NULL;
+	int ret;
+	bool fw_loaded = false;
+
+	mutex_lock(&dln2_acpi_lock);
+
+	if (ai->dev)
+		goto out_success;
+
+	/*
+	 * Look for the DLN2000 HID in case the ACPI table was loaded
+	 * externally (e.g. from qemu).
+	 */
+	acpi_get_devices("DLN20000", dln2_find_acpi_handle, NULL, &h);
+	if (!h) {
+		/* Try to load the ACPI table via a firmware file */
+		ret = request_firmware(&ai->fw, "dln2.aml", NULL);
+		if (ret)
+			goto out_unlock;
+
+		ret = acpi_load_table((void *)ai->fw->data);
+		if (ret) {
+			dev_err(dev, "invalid ACPI table\n");
+			goto out_release_fw;
+		}
+
+		acpi_get_devices("DLN20000", dln2_find_acpi_handle, NULL, &h);
+		if (!h) {
+			dev_err(dev, "not a DLN2 ACPI table\n");
+			goto out_leak_fw;
+		}
+
+		ret = acpi_get_id(h, &ai->table_id);
+		if (ret) {
+			dev_err(dev, "acpi_get_id failed: %d\n", ret);
+			goto out_leak_fw;
+		}
+
+		ret = acpi_bus_scan(h);
+		if (ret) {
+			dev_err(dev, "acpi_bus_scan failed: %d\n", ret);
+			goto out_leak_fw;
+		}
+
+		fw_loaded = true;
+	}
+
+	ret = acpi_bus_get_device(h, &ai->dev);
+	if (ret) {
+		dev_err(dev, "failed to get ACPI device: %d\n", ret);
+		if (fw_loaded) {
+			acpi_unload_table_id(ai->table_id);
+			goto out_leak_fw;
+		}
+		goto out_unlock;
+	}
+
+out_success:
+	ACPI_COMPANION_SET(dev, ai->dev);
+	ai->users++;
+	mutex_unlock(&dln2_acpi_lock);
+	return;
+
+out_release_fw:
+	release_firmware(ai->fw);
+out_leak_fw:
+	/*
+	 * Once a table is loaded we can't release the firmware anymore because
+	 * acpi_unload_table does not actually unload the table but keeps it in
+	 * memory to speed up subsequent loads.
+	 */
+	ai->fw = NULL;
+out_unlock:
+	mutex_unlock(&dln2_acpi_lock);
+}
+
+static void dln2_disconnect_acpi(struct dln2_dev *dln2)
+{
+	struct dln2_acpi_info *ai = &dln2_acpi_info;
+
+	mutex_lock(&dln2_acpi_lock);
+	if (--ai->users == 0 && ai->fw) {
+		acpi_scan_lock_acquire();
+		acpi_bus_trim(ai->dev);
+		acpi_scan_lock_release();
+		acpi_unload_table_id(ai->table_id);
+		ai->dev = NULL;
+		/* we can't release firmware see comment in dln2_probe_acpi */
+		ai->fw = NULL;
+	}
+	mutex_unlock(&dln2_acpi_lock);
+}
+#else
+static void dln2_probe_acpi(struct dln2_dev *dln2)
+{
+}
+
+static void dln2_disconnect_acpi(struct dln2_dev *dln2)
+{
+}
+#endif
+
 static void dln2_disconnect(struct usb_interface *interface)
 {
 	struct dln2_dev *dln2 = usb_get_intfdata(interface);
@@ -722,6 +852,8 @@ static void dln2_disconnect(struct usb_interface *interface)
 
 	mfd_remove_devices(&interface->dev);
 
+	dln2_disconnect_acpi(dln2);
+
 	dln2_free(dln2);
 }
 
@@ -774,6 +906,8 @@ static int dln2_probe(struct usb_interface *interface,
 		goto out_stop_rx;
 	}
 
+	dln2_probe_acpi(dln2);
+
 	ret = mfd_add_hotplug_devices(dev, dln2_devs, ARRAY_SIZE(dln2_devs));
 	if (ret != 0) {
 		dev_err(dev, "failed to add mfd devices to core\n");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/4] gpio: dln2: add support for ACPI pin configuration
  2014-12-16 16:12 [PATCH 0/4] dln2: add support for ACPI Octavian Purdila
                   ` (2 preceding siblings ...)
  2014-12-16 16:12 ` [PATCH 3/4] mfd: dln2: add support for ACPI Octavian Purdila
@ 2014-12-16 16:12 ` Octavian Purdila
  3 siblings, 0 replies; 25+ messages in thread
From: Octavian Purdila @ 2014-12-16 16:12 UTC (permalink / raw)
  To: linus.walleij, lee.jones, rjw
  Cc: johan, linux-usb, linux-kernel, linux-gpio, heikki.krogerus,
	mika.westerberg, linux-acpi, Octavian Purdila

This patch configures the pull-up/pull-down properties based on the
ACPI configuration. It scans the children of the DLN2 root entry and
looks for GPIO resources and applies the pull-up/pull-down
configurations on the pins.

Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
---
 drivers/gpio/gpio-dln2.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c
index 2fdb138..98189d5 100644
--- a/drivers/gpio/gpio-dln2.c
+++ b/drivers/gpio/gpio-dln2.c
@@ -19,6 +19,7 @@
 #include <linux/gpio/driver.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/dln2.h>
+#include <linux/acpi.h>
 
 #define DLN2_GPIO_ID			0x01
 
@@ -36,6 +37,10 @@
 #define DLN2_GPIO_PIN_GET_DIRECTION	DLN2_CMD(0x14, DLN2_GPIO_ID)
 #define DLN2_GPIO_PIN_SET_EVENT_CFG	DLN2_CMD(0x1E, DLN2_GPIO_ID)
 #define DLN2_GPIO_PIN_GET_EVENT_CFG	DLN2_CMD(0x1F, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_PULLUP_ENABLE	DLN2_CMD(0x18, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_PULLUP_DISABLE	DLN2_CMD(0x19, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_PULLDOWN_ENABLE	DLN2_CMD(0x20, DLN2_GPIO_ID)
+#define CMD_GPIO_PIN_PULLDOWN_DISABLE	DLN2_CMD(0x21, DLN2_GPIO_ID)
 
 #define DLN2_GPIO_EVENT_NONE		0
 #define DLN2_GPIO_EVENT_CHANGE		1
@@ -428,6 +433,75 @@ static void dln2_gpio_event(struct platform_device *pdev, u16 echo,
 	}
 }
 
+#if IS_ENABLED(CONFIG_ACPI)
+static int dln2_gpio_do_acpi_setup(struct acpi_resource *ares, void *data)
+{
+	int i;
+	u16 cmd[2];
+	const char *info;
+	struct dln2_gpio *dln2 = data;
+	const struct acpi_resource_gpio *agpio = &ares->data.gpio;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_GPIO)
+		return 1;
+
+	switch (agpio->pin_config) {
+	case  ACPI_PIN_CONFIG_PULLUP:
+		info = "pullup";
+		cmd[0] = CMD_GPIO_PIN_PULLDOWN_DISABLE;
+		cmd[1] = CMD_GPIO_PIN_PULLUP_ENABLE;
+		break;
+	case  ACPI_PIN_CONFIG_PULLDOWN:
+		info = "pulldown";
+		cmd[0] = CMD_GPIO_PIN_PULLDOWN_ENABLE;
+		cmd[1] = CMD_GPIO_PIN_PULLUP_DISABLE;
+		break;
+	case  ACPI_PIN_CONFIG_NOPULL:
+		info = "nopull";
+		cmd[0] = CMD_GPIO_PIN_PULLDOWN_DISABLE;
+		cmd[1] = CMD_GPIO_PIN_PULLUP_DISABLE;
+		break;
+	default:
+		return 1;
+	}
+
+	for (i = 0; i < agpio->pin_table_length; i++) {
+		int pin = agpio->pin_table[i];
+
+		dev_info(dln2->gpio.dev, "setting %s on pin %d\n", info, pin);
+		dln2_gpio_pin_cmd(dln2, cmd[0], pin);
+		dln2_gpio_pin_cmd(dln2, cmd[1], pin);
+	}
+
+	return 1;
+}
+
+static void dln2_gpio_acpi_setup(struct dln2_gpio *dln2)
+{
+	struct acpi_device *adev, *child;
+	acpi_handle handle;
+
+	handle = ACPI_HANDLE(dln2->gpio.dev);
+	if (!handle || acpi_bus_get_device(handle, &adev))
+		return;
+
+	list_for_each_entry(child, &adev->children, node) {
+		LIST_HEAD(res);
+		int ret;
+
+		ret = acpi_dev_get_resources(child, &res,
+					     dln2_gpio_do_acpi_setup, dln2);
+		if (ret < 0)
+			continue;
+		acpi_dev_free_resource_list(&res);
+	}
+}
+#else
+static void dln2_gpio_acpi_setup(struct dln2_gpio *dln2)
+{
+}
+#endif
+
 static int dln2_gpio_probe(struct platform_device *pdev)
 {
 	struct dln2_gpio *dln2;
@@ -492,6 +566,8 @@ static int dln2_gpio_probe(struct platform_device *pdev)
 		goto out_gpiochip_remove;
 	}
 
+	dln2_gpio_acpi_setup(dln2);
+
 	return 0;
 
 out_gpiochip_remove:
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id
  2014-12-16 16:12 ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Octavian Purdila
@ 2014-12-16 18:14   ` Sergei Shtylyov
  2014-12-16 19:32     ` Octavian Purdila
  2015-01-22  6:37     ` Lv Zheng
       [not found]   ` <1418746353-3481-2-git-send-email-octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2 siblings, 1 reply; 25+ messages in thread
From: Sergei Shtylyov @ 2014-12-16 18:14 UTC (permalink / raw)
  To: Octavian Purdila, linus.walleij, lee.jones, rjw
  Cc: johan, linux-usb, linux-kernel, linux-gpio, heikki.krogerus,
	mika.westerberg, linux-acpi

Hello.

On 12/16/2014 07:12 PM, Octavian Purdila wrote:

> acpi_tb_delete_namespace_by_owner expects ACPI_MTX_INTERPRETER to be
> taken. This fixes the following issue:

> ACPI Error: Mutex [0x0] is not acquired, cannot release (20141107/utmutex-322)
> Call Trace:
>    [<ffffffff81b0bd28>] dump_stack+0x4f/0x7b
>    [<ffffffff81546bfc>] acpi_ut_release_mutex+0x47/0x67
>    [<ffffffff81542cf1>] acpi_tb_delete_namespace_by_owner+0x57/0x8d
>    [<ffffffff81543ef1>] acpi_unload_table_id+0x3a/0x5e

> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> ---
>   drivers/acpi/acpica/tbxface.c | 7 +++++++
>   1 file changed, 7 insertions(+)

> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
> index 6482b0d..9520ae1 100644
> --- a/drivers/acpi/acpica/tbxface.c
> +++ b/drivers/acpi/acpica/tbxface.c
> @@ -281,6 +281,11 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>
>   	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
>
> +	status = acpi_ut_acquire_mutex(ACPI_MTX_INTERPRETER);
> +	if (ACPI_FAILURE(status)) {
> +		return_ACPI_STATUS(status);
> +	}

    {} not needed here. Please run your patches thru scripts/checkpatch.pl, it 
should complain in this case.

> +
>   	/* Find table in the global table list */
>   	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
>   		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
> @@ -297,6 +302,8 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>   		acpi_tb_set_table_loaded_flag(i, FALSE);
>   		break;
>   	}
> +
> +	(void)acpi_ut_release_mutex(ACPI_MTX_INTERPRETER);

    Cast to *void*  not necessary either.

[...]

WBR, Sergei


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id
  2014-12-16 18:14   ` Sergei Shtylyov
@ 2014-12-16 19:32     ` Octavian Purdila
  0 siblings, 0 replies; 25+ messages in thread
From: Octavian Purdila @ 2014-12-16 19:32 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Linus Walleij, Lee Jones, Rafael J. Wysocki, Johan Hovold,
	linux-usb, lkml, linux-gpio, Heikki Krogerus, Mika Westerberg,
	linux-acpi

On Tue, Dec 16, 2014 at 8:14 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 12/16/2014 07:12 PM, Octavian Purdila wrote:
>
>> acpi_tb_delete_namespace_by_owner expects ACPI_MTX_INTERPRETER to be
>> taken. This fixes the following issue:
>
>
>> ACPI Error: Mutex [0x0] is not acquired, cannot release
>> (20141107/utmutex-322)
>> Call Trace:
>>    [<ffffffff81b0bd28>] dump_stack+0x4f/0x7b
>>    [<ffffffff81546bfc>] acpi_ut_release_mutex+0x47/0x67
>>    [<ffffffff81542cf1>] acpi_tb_delete_namespace_by_owner+0x57/0x8d
>>    [<ffffffff81543ef1>] acpi_unload_table_id+0x3a/0x5e
>
>
>> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
>> ---
>>   drivers/acpi/acpica/tbxface.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>
>
>> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
>> index 6482b0d..9520ae1 100644
>> --- a/drivers/acpi/acpica/tbxface.c
>> +++ b/drivers/acpi/acpica/tbxface.c
>> @@ -281,6 +281,11 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>>
>>         ACPI_FUNCTION_TRACE(acpi_unload_table_id);
>>
>> +       status = acpi_ut_acquire_mutex(ACPI_MTX_INTERPRETER);
>> +       if (ACPI_FAILURE(status)) {
>> +               return_ACPI_STATUS(status);
>> +       }
>
>
>    {} not needed here. Please run your patches thru scripts/checkpatch.pl,
> it should complain in this case.
>

I always run checkpatch. It does not complain.

>> +
>>         /* Find table in the global table list */
>>         for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i)
>> {
>>                 if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
>> @@ -297,6 +302,8 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>>                 acpi_tb_set_table_loaded_flag(i, FALSE);
>>                 break;
>>         }
>> +
>> +       (void)acpi_ut_release_mutex(ACPI_MTX_INTERPRETER);
>
>
>    Cast to *void*  not necessary either.
>

AFAICS ACPICA has a slight different coding style than the rest of the
kernel and I kept using it in these patches. Rafael, please let me
know if I am wrong.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/4] mfd: dln2: add support for ACPI
  2014-12-16 16:12 ` [PATCH 3/4] mfd: dln2: add support for ACPI Octavian Purdila
@ 2015-01-20 15:20     ` Lee Jones
  2015-01-22  2:00   ` Rafael J. Wysocki
  1 sibling, 0 replies; 25+ messages in thread
From: Lee Jones @ 2015-01-20 15:20 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: linus.walleij, rjw, johan, linux-usb, linux-kernel, linux-gpio,
	heikki.krogerus, mika.westerberg, linux-acpi

I need an ACPI Ack on this.

> This patch adds support to load a custom ACPI table that describes
> devices connected via the DLN2 USB to I2C/SPI/GPIO bridge.
> 
> The ACPI table can be loaded either externally (from QEMU or with
> CONFIG_ACPI_CUSTOM_DSDT) or it can be loaded as firmware file with the
> name dln2.aml. The driver looks for an ACPI device entry with _HID set
> to "DLN20000" and makes it the ACPI companion for DLN2 USB
> sub-drivers.
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> ---
>  Documentation/acpi/dln2-acpi.txt |  62 ++++++++++++++++++
>  drivers/mfd/dln2.c               | 134 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 196 insertions(+)
>  create mode 100644 Documentation/acpi/dln2-acpi.txt
> 
> diff --git a/Documentation/acpi/dln2-acpi.txt b/Documentation/acpi/dln2-acpi.txt
> new file mode 100644
> index 0000000..d76605f
> --- /dev/null
> +++ b/Documentation/acpi/dln2-acpi.txt
> @@ -0,0 +1,62 @@
> +Diolan DLN2 custom APCI table
> +
> +The Diolan DLN2 is an USB to I2C/SPI/GPIO bridge and as such it can be used to
> +connect to various I2C or SPI devices. Because these busses lack an enumeration
> +protocol, the driver obtains various information about the device (such as I2C
> +address and GPIO pins) from either ACPI or device tree.
> +
> +To allow enumerating devices and their properties via ACPI, the Diolan
> +driver looks for an ACPI tree with the root _HID set to "DLN20000". If
> +it finds such an ACPI object it will set the ACPI companion to the
> +DLN2 MFD driver and from their it will be propagated to all its
> +sub-devices (I2C, GPIO, SPI).
> +
> +The user can either load the custom DSDT table with three methods:
> +
> +1. Via QEMU (see -acpitable)
> +
> +2. Via the CONFIG_ACPI_CUSTOM_DSDT kernel config option (see
> +Documentation/acpi/dsdt-override.txt)
> +
> +3. By placing the custom DSDT in the firmware paths in a file name
> +dln2.aml.
> +
> +Here is an example ACPI table that enumerates a BMC150 accelerometer
> +and defines its I2C address and GPIO pin used as an interrupt source:
> +
> +DefinitionBlock ("ssdt.aml", "SSDT", 1, "INTEL ", "CpuDptf", 0x00000003)
> +{
> +	Device (DLN0)
> +	{
> +		Name (_ADR, Zero)
> +		Name (_HID, "DLN2000")
> +
> +		Device (STAC)
> +		{
> +			Name (_ADR, Zero)
> +			Name (_HID, "BMC150A")
> +			Name (_CID, "INTACCL")
> +			Name (_UID, One)
> +
> +			Method (_CRS, 0, Serialized)
> +			{
> +				Name (RBUF, ResourceTemplate ()
> +				{
> +					I2cSerialBus (0x0010, ControllerInitiated, 0x00061A80,
> +						      AddressingMode7Bit, "\\DLN0",
> +						      0x00, ResourceConsumer, ,)
> +
> +					GpioInt (Level, ActiveHigh, Exclusive, PullDown, 0x0000,
> +						 "\\DLN0", 0x00, ResourceConsumer, , )
> +					{ // Pin list
> +						0
> +					}
> +				})
> +				Return (RBUF)
> +		       }
> +		}
> +	}
> +}
> +
> +The resources defined in the devices under the DLN0 are those
> +supported by the I2C, GPIO and SPI sub-systems.
> diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
> index f9c4a0b..93f6d1d 100644
> --- a/drivers/mfd/dln2.c
> +++ b/drivers/mfd/dln2.c
> @@ -23,6 +23,8 @@
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/dln2.h>
>  #include <linux/rculist.h>
> +#include <linux/acpi.h>
> +#include <linux/firmware.h>
>  
>  struct dln2_header {
>  	__le16 size;
> @@ -714,6 +716,134 @@ static void dln2_stop(struct dln2_dev *dln2)
>  
>  	dln2_stop_rx_urbs(dln2);
>  }
> +
> +#if IS_ENABLED(CONFIG_ACPI)
> +
> +static struct dln2_acpi_info {
> +	const struct firmware *fw;
> +	acpi_owner_id table_id;
> +	struct acpi_device *dev;
> +	int users;
> +} dln2_acpi_info;
> +
> +static DEFINE_MUTEX(dln2_acpi_lock);
> +
> +static acpi_status dln2_find_acpi_handle(acpi_handle handle, u32 level,
> +					 void *ctxt, void **retv)
> +{
> +	acpi_handle *dln2_handle = (acpi_handle *)retv;
> +
> +	*dln2_handle = handle;
> +
> +	return AE_CTRL_TERMINATE;
> +}
> +
> +static void dln2_probe_acpi(struct dln2_dev *dln2)
> +{
> +	struct device *dev = &dln2->interface->dev;
> +	struct dln2_acpi_info *ai = &dln2_acpi_info;
> +	acpi_handle h = NULL;
> +	int ret;
> +	bool fw_loaded = false;
> +
> +	mutex_lock(&dln2_acpi_lock);
> +
> +	if (ai->dev)
> +		goto out_success;
> +
> +	/*
> +	 * Look for the DLN2000 HID in case the ACPI table was loaded
> +	 * externally (e.g. from qemu).
> +	 */
> +	acpi_get_devices("DLN20000", dln2_find_acpi_handle, NULL, &h);
> +	if (!h) {
> +		/* Try to load the ACPI table via a firmware file */
> +		ret = request_firmware(&ai->fw, "dln2.aml", NULL);
> +		if (ret)
> +			goto out_unlock;
> +
> +		ret = acpi_load_table((void *)ai->fw->data);
> +		if (ret) {
> +			dev_err(dev, "invalid ACPI table\n");
> +			goto out_release_fw;
> +		}
> +
> +		acpi_get_devices("DLN20000", dln2_find_acpi_handle, NULL, &h);
> +		if (!h) {
> +			dev_err(dev, "not a DLN2 ACPI table\n");
> +			goto out_leak_fw;
> +		}
> +
> +		ret = acpi_get_id(h, &ai->table_id);
> +		if (ret) {
> +			dev_err(dev, "acpi_get_id failed: %d\n", ret);
> +			goto out_leak_fw;
> +		}
> +
> +		ret = acpi_bus_scan(h);
> +		if (ret) {
> +			dev_err(dev, "acpi_bus_scan failed: %d\n", ret);
> +			goto out_leak_fw;
> +		}
> +
> +		fw_loaded = true;
> +	}
> +
> +	ret = acpi_bus_get_device(h, &ai->dev);
> +	if (ret) {
> +		dev_err(dev, "failed to get ACPI device: %d\n", ret);
> +		if (fw_loaded) {
> +			acpi_unload_table_id(ai->table_id);
> +			goto out_leak_fw;
> +		}
> +		goto out_unlock;
> +	}
> +
> +out_success:
> +	ACPI_COMPANION_SET(dev, ai->dev);
> +	ai->users++;
> +	mutex_unlock(&dln2_acpi_lock);
> +	return;
> +
> +out_release_fw:
> +	release_firmware(ai->fw);
> +out_leak_fw:
> +	/*
> +	 * Once a table is loaded we can't release the firmware anymore because
> +	 * acpi_unload_table does not actually unload the table but keeps it in
> +	 * memory to speed up subsequent loads.
> +	 */
> +	ai->fw = NULL;
> +out_unlock:
> +	mutex_unlock(&dln2_acpi_lock);
> +}
> +
> +static void dln2_disconnect_acpi(struct dln2_dev *dln2)
> +{
> +	struct dln2_acpi_info *ai = &dln2_acpi_info;
> +
> +	mutex_lock(&dln2_acpi_lock);
> +	if (--ai->users == 0 && ai->fw) {
> +		acpi_scan_lock_acquire();
> +		acpi_bus_trim(ai->dev);
> +		acpi_scan_lock_release();
> +		acpi_unload_table_id(ai->table_id);
> +		ai->dev = NULL;
> +		/* we can't release firmware see comment in dln2_probe_acpi */
> +		ai->fw = NULL;
> +	}
> +	mutex_unlock(&dln2_acpi_lock);
> +}
> +#else
> +static void dln2_probe_acpi(struct dln2_dev *dln2)
> +{
> +}
> +
> +static void dln2_disconnect_acpi(struct dln2_dev *dln2)
> +{
> +}
> +#endif
> +
>  static void dln2_disconnect(struct usb_interface *interface)
>  {
>  	struct dln2_dev *dln2 = usb_get_intfdata(interface);
> @@ -722,6 +852,8 @@ static void dln2_disconnect(struct usb_interface *interface)
>  
>  	mfd_remove_devices(&interface->dev);
>  
> +	dln2_disconnect_acpi(dln2);
> +
>  	dln2_free(dln2);
>  }
>  
> @@ -774,6 +906,8 @@ static int dln2_probe(struct usb_interface *interface,
>  		goto out_stop_rx;
>  	}
>  
> +	dln2_probe_acpi(dln2);
> +
>  	ret = mfd_add_hotplug_devices(dev, dln2_devs, ARRAY_SIZE(dln2_devs));
>  	if (ret != 0) {
>  		dev_err(dev, "failed to add mfd devices to core\n");
> -- 
> 1.9.1
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/4] mfd: dln2: add support for ACPI
@ 2015-01-20 15:20     ` Lee Jones
  0 siblings, 0 replies; 25+ messages in thread
From: Lee Jones @ 2015-01-20 15:20 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: linus.walleij, rjw, johan, linux-usb, linux-kernel, linux-gpio,
	heikki.krogerus, mika.westerberg, linux-acpi

I need an ACPI Ack on this.

> This patch adds support to load a custom ACPI table that describes
> devices connected via the DLN2 USB to I2C/SPI/GPIO bridge.
> 
> The ACPI table can be loaded either externally (from QEMU or with
> CONFIG_ACPI_CUSTOM_DSDT) or it can be loaded as firmware file with the
> name dln2.aml. The driver looks for an ACPI device entry with _HID set
> to "DLN20000" and makes it the ACPI companion for DLN2 USB
> sub-drivers.
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> ---
>  Documentation/acpi/dln2-acpi.txt |  62 ++++++++++++++++++
>  drivers/mfd/dln2.c               | 134 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 196 insertions(+)
>  create mode 100644 Documentation/acpi/dln2-acpi.txt
> 
> diff --git a/Documentation/acpi/dln2-acpi.txt b/Documentation/acpi/dln2-acpi.txt
> new file mode 100644
> index 0000000..d76605f
> --- /dev/null
> +++ b/Documentation/acpi/dln2-acpi.txt
> @@ -0,0 +1,62 @@
> +Diolan DLN2 custom APCI table
> +
> +The Diolan DLN2 is an USB to I2C/SPI/GPIO bridge and as such it can be used to
> +connect to various I2C or SPI devices. Because these busses lack an enumeration
> +protocol, the driver obtains various information about the device (such as I2C
> +address and GPIO pins) from either ACPI or device tree.
> +
> +To allow enumerating devices and their properties via ACPI, the Diolan
> +driver looks for an ACPI tree with the root _HID set to "DLN20000". If
> +it finds such an ACPI object it will set the ACPI companion to the
> +DLN2 MFD driver and from their it will be propagated to all its
> +sub-devices (I2C, GPIO, SPI).
> +
> +The user can either load the custom DSDT table with three methods:
> +
> +1. Via QEMU (see -acpitable)
> +
> +2. Via the CONFIG_ACPI_CUSTOM_DSDT kernel config option (see
> +Documentation/acpi/dsdt-override.txt)
> +
> +3. By placing the custom DSDT in the firmware paths in a file name
> +dln2.aml.
> +
> +Here is an example ACPI table that enumerates a BMC150 accelerometer
> +and defines its I2C address and GPIO pin used as an interrupt source:
> +
> +DefinitionBlock ("ssdt.aml", "SSDT", 1, "INTEL ", "CpuDptf", 0x00000003)
> +{
> +	Device (DLN0)
> +	{
> +		Name (_ADR, Zero)
> +		Name (_HID, "DLN2000")
> +
> +		Device (STAC)
> +		{
> +			Name (_ADR, Zero)
> +			Name (_HID, "BMC150A")
> +			Name (_CID, "INTACCL")
> +			Name (_UID, One)
> +
> +			Method (_CRS, 0, Serialized)
> +			{
> +				Name (RBUF, ResourceTemplate ()
> +				{
> +					I2cSerialBus (0x0010, ControllerInitiated, 0x00061A80,
> +						      AddressingMode7Bit, "\\DLN0",
> +						      0x00, ResourceConsumer, ,)
> +
> +					GpioInt (Level, ActiveHigh, Exclusive, PullDown, 0x0000,
> +						 "\\DLN0", 0x00, ResourceConsumer, , )
> +					{ // Pin list
> +						0
> +					}
> +				})
> +				Return (RBUF)
> +		       }
> +		}
> +	}
> +}
> +
> +The resources defined in the devices under the DLN0 are those
> +supported by the I2C, GPIO and SPI sub-systems.
> diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
> index f9c4a0b..93f6d1d 100644
> --- a/drivers/mfd/dln2.c
> +++ b/drivers/mfd/dln2.c
> @@ -23,6 +23,8 @@
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/dln2.h>
>  #include <linux/rculist.h>
> +#include <linux/acpi.h>
> +#include <linux/firmware.h>
>  
>  struct dln2_header {
>  	__le16 size;
> @@ -714,6 +716,134 @@ static void dln2_stop(struct dln2_dev *dln2)
>  
>  	dln2_stop_rx_urbs(dln2);
>  }
> +
> +#if IS_ENABLED(CONFIG_ACPI)
> +
> +static struct dln2_acpi_info {
> +	const struct firmware *fw;
> +	acpi_owner_id table_id;
> +	struct acpi_device *dev;
> +	int users;
> +} dln2_acpi_info;
> +
> +static DEFINE_MUTEX(dln2_acpi_lock);
> +
> +static acpi_status dln2_find_acpi_handle(acpi_handle handle, u32 level,
> +					 void *ctxt, void **retv)
> +{
> +	acpi_handle *dln2_handle = (acpi_handle *)retv;
> +
> +	*dln2_handle = handle;
> +
> +	return AE_CTRL_TERMINATE;
> +}
> +
> +static void dln2_probe_acpi(struct dln2_dev *dln2)
> +{
> +	struct device *dev = &dln2->interface->dev;
> +	struct dln2_acpi_info *ai = &dln2_acpi_info;
> +	acpi_handle h = NULL;
> +	int ret;
> +	bool fw_loaded = false;
> +
> +	mutex_lock(&dln2_acpi_lock);
> +
> +	if (ai->dev)
> +		goto out_success;
> +
> +	/*
> +	 * Look for the DLN2000 HID in case the ACPI table was loaded
> +	 * externally (e.g. from qemu).
> +	 */
> +	acpi_get_devices("DLN20000", dln2_find_acpi_handle, NULL, &h);
> +	if (!h) {
> +		/* Try to load the ACPI table via a firmware file */
> +		ret = request_firmware(&ai->fw, "dln2.aml", NULL);
> +		if (ret)
> +			goto out_unlock;
> +
> +		ret = acpi_load_table((void *)ai->fw->data);
> +		if (ret) {
> +			dev_err(dev, "invalid ACPI table\n");
> +			goto out_release_fw;
> +		}
> +
> +		acpi_get_devices("DLN20000", dln2_find_acpi_handle, NULL, &h);
> +		if (!h) {
> +			dev_err(dev, "not a DLN2 ACPI table\n");
> +			goto out_leak_fw;
> +		}
> +
> +		ret = acpi_get_id(h, &ai->table_id);
> +		if (ret) {
> +			dev_err(dev, "acpi_get_id failed: %d\n", ret);
> +			goto out_leak_fw;
> +		}
> +
> +		ret = acpi_bus_scan(h);
> +		if (ret) {
> +			dev_err(dev, "acpi_bus_scan failed: %d\n", ret);
> +			goto out_leak_fw;
> +		}
> +
> +		fw_loaded = true;
> +	}
> +
> +	ret = acpi_bus_get_device(h, &ai->dev);
> +	if (ret) {
> +		dev_err(dev, "failed to get ACPI device: %d\n", ret);
> +		if (fw_loaded) {
> +			acpi_unload_table_id(ai->table_id);
> +			goto out_leak_fw;
> +		}
> +		goto out_unlock;
> +	}
> +
> +out_success:
> +	ACPI_COMPANION_SET(dev, ai->dev);
> +	ai->users++;
> +	mutex_unlock(&dln2_acpi_lock);
> +	return;
> +
> +out_release_fw:
> +	release_firmware(ai->fw);
> +out_leak_fw:
> +	/*
> +	 * Once a table is loaded we can't release the firmware anymore because
> +	 * acpi_unload_table does not actually unload the table but keeps it in
> +	 * memory to speed up subsequent loads.
> +	 */
> +	ai->fw = NULL;
> +out_unlock:
> +	mutex_unlock(&dln2_acpi_lock);
> +}
> +
> +static void dln2_disconnect_acpi(struct dln2_dev *dln2)
> +{
> +	struct dln2_acpi_info *ai = &dln2_acpi_info;
> +
> +	mutex_lock(&dln2_acpi_lock);
> +	if (--ai->users == 0 && ai->fw) {
> +		acpi_scan_lock_acquire();
> +		acpi_bus_trim(ai->dev);
> +		acpi_scan_lock_release();
> +		acpi_unload_table_id(ai->table_id);
> +		ai->dev = NULL;
> +		/* we can't release firmware see comment in dln2_probe_acpi */
> +		ai->fw = NULL;
> +	}
> +	mutex_unlock(&dln2_acpi_lock);
> +}
> +#else
> +static void dln2_probe_acpi(struct dln2_dev *dln2)
> +{
> +}
> +
> +static void dln2_disconnect_acpi(struct dln2_dev *dln2)
> +{
> +}
> +#endif
> +
>  static void dln2_disconnect(struct usb_interface *interface)
>  {
>  	struct dln2_dev *dln2 = usb_get_intfdata(interface);
> @@ -722,6 +852,8 @@ static void dln2_disconnect(struct usb_interface *interface)
>  
>  	mfd_remove_devices(&interface->dev);
>  
> +	dln2_disconnect_acpi(dln2);
> +
>  	dln2_free(dln2);
>  }
>  
> @@ -774,6 +906,8 @@ static int dln2_probe(struct usb_interface *interface,
>  		goto out_stop_rx;
>  	}
>  
> +	dln2_probe_acpi(dln2);
> +
>  	ret = mfd_add_hotplug_devices(dev, dln2_devs, ARRAY_SIZE(dln2_devs));
>  	if (ret != 0) {
>  		dev_err(dev, "failed to add mfd devices to core\n");
> -- 
> 1.9.1
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/4] mfd: dln2: add support for ACPI
  2014-12-16 16:12 ` [PATCH 3/4] mfd: dln2: add support for ACPI Octavian Purdila
  2015-01-20 15:20     ` Lee Jones
@ 2015-01-22  2:00   ` Rafael J. Wysocki
  2015-01-22 10:13     ` Octavian Purdila
  1 sibling, 1 reply; 25+ messages in thread
From: Rafael J. Wysocki @ 2015-01-22  2:00 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: linus.walleij, lee.jones, johan, linux-usb, linux-kernel,
	linux-gpio, heikki.krogerus, mika.westerberg, linux-acpi

On Tuesday, December 16, 2014 06:12:32 PM Octavian Purdila wrote:
> This patch adds support to load a custom ACPI table that describes
> devices connected via the DLN2 USB to I2C/SPI/GPIO bridge.
> 
> The ACPI table can be loaded either externally (from QEMU or with
> CONFIG_ACPI_CUSTOM_DSDT) or it can be loaded as firmware file with the
> name dln2.aml. The driver looks for an ACPI device entry with _HID set
> to "DLN20000" and makes it the ACPI companion for DLN2 USB
> sub-drivers.
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> ---
>  Documentation/acpi/dln2-acpi.txt |  62 ++++++++++++++++++
>  drivers/mfd/dln2.c               | 134 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 196 insertions(+)
>  create mode 100644 Documentation/acpi/dln2-acpi.txt
> 
> diff --git a/Documentation/acpi/dln2-acpi.txt b/Documentation/acpi/dln2-acpi.txt
> new file mode 100644
> index 0000000..d76605f
> --- /dev/null
> +++ b/Documentation/acpi/dln2-acpi.txt
> @@ -0,0 +1,62 @@
> +Diolan DLN2 custom APCI table
> +
> +The Diolan DLN2 is an USB to I2C/SPI/GPIO bridge and as such it can be used to
> +connect to various I2C or SPI devices. Because these busses lack an enumeration
> +protocol, the driver obtains various information about the device (such as I2C
> +address and GPIO pins) from either ACPI or device tree.
> +
> +To allow enumerating devices and their properties via ACPI, the Diolan
> +driver looks for an ACPI tree with the root _HID set to "DLN20000". If
> +it finds such an ACPI object it will set the ACPI companion to the
> +DLN2 MFD driver and from their it will be propagated to all its
> +sub-devices (I2C, GPIO, SPI).
> +
> +The user can either load the custom DSDT table with three methods:

s/DSDT/ACPI/

> +
> +1. Via QEMU (see -acpitable)
> +
> +2. Via the CONFIG_ACPI_CUSTOM_DSDT kernel config option (see
> +Documentation/acpi/dsdt-override.txt)
> +
> +3. By placing the custom DSDT in the firmware paths in a file name
> +dln2.aml.

Surely SSDT?

> +
> +Here is an example ACPI table that enumerates a BMC150 accelerometer
> +and defines its I2C address and GPIO pin used as an interrupt source:
> +
> +DefinitionBlock ("ssdt.aml", "SSDT", 1, "INTEL ", "CpuDptf", 0x00000003)
> +{
> +	Device (DLN0)
> +	{
> +		Name (_ADR, Zero)
> +		Name (_HID, "DLN2000")
> +
> +		Device (STAC)
> +		{
> +			Name (_ADR, Zero)
> +			Name (_HID, "BMC150A")
> +			Name (_CID, "INTACCL")
> +			Name (_UID, One)
> +
> +			Method (_CRS, 0, Serialized)
> +			{
> +				Name (RBUF, ResourceTemplate ()
> +				{
> +					I2cSerialBus (0x0010, ControllerInitiated, 0x00061A80,
> +						      AddressingMode7Bit, "\\DLN0",
> +						      0x00, ResourceConsumer, ,)
> +
> +					GpioInt (Level, ActiveHigh, Exclusive, PullDown, 0x0000,
> +						 "\\DLN0", 0x00, ResourceConsumer, , )
> +					{ // Pin list
> +						0
> +					}
> +				})
> +				Return (RBUF)
> +		       }
> +		}
> +	}
> +}
> +
> +The resources defined in the devices under the DLN0 are those
> +supported by the I2C, GPIO and SPI sub-systems.
> diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
> index f9c4a0b..93f6d1d 100644
> --- a/drivers/mfd/dln2.c
> +++ b/drivers/mfd/dln2.c
> @@ -23,6 +23,8 @@
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/dln2.h>
>  #include <linux/rculist.h>
> +#include <linux/acpi.h>
> +#include <linux/firmware.h>
>  

OK, so correct me if I'm wrong.

When the (USB) dnl2 device is probed, it is supposed to find an ACPI companion
for itself and if it's not there, the driver will try to load a custom SSDT from
a firmware blob in the hope that the companion will be there?

So if I'm not wrong, why is this not broken?

It is not sufficient to call ACPI_COMPANION_SET(dev, ai->dev) to set the device's
companion.  acpi_bind_one() needs to be run in addition to that, but acpi_bind_one()
is not to be called from drivers.  It is called by the core automatically during
device registration and ACPI_COMPANION_SET() is to be used *before* that, not after.

So if I'm not missing anything, the design here is entirely backwards and we
need to talk about how to implement it correctly at the design level in the
first place.

And no, "Let's come up with a patch that sort of works, throw it at the maintainers
and see what happens" is not an acceptable approach, sorry.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
  2014-12-16 16:12 ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Octavian Purdila
@ 2015-01-22  6:37     ` Lv Zheng
  2015-01-22  6:37     ` Lv Zheng
       [not found]   ` <1418746353-3481-2-git-send-email-octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2 siblings, 0 replies; 25+ messages in thread
From: Lv Zheng @ 2015-01-22  6:37 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Lv Zheng, Lv Zheng, linux-acpi, Octavian Purdila,
	Prarit Bhargava, Bjorn Helgaas, linux-pci, Tony Luck, Fenghua Yu,
	linux-ia64

ACPICA has implemented acpi_unload_parent_table() which can exactly replace
the acpi_get_id()/acpi_unload_table_id() implemented in Linux kernel.  The
acpi_unload_parent_table() has been unit tested in ACPICA simulation
environment.

This patch can also help to reduce the source code differences between
Linux and ACPICA.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Cc: Octavian Purdila <octavian.purdila@intel.com>
Cc: Prarit Bhargava <prarit@sgi.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
---
 drivers/acpi/acpica/nsxfobj.c     |   44 -------------------------------------
 drivers/acpi/acpica/tbxface.c     |   39 --------------------------------
 drivers/pci/hotplug/sgi_hotplug.c |   13 ++++++-----
 include/acpi/acpixf.h             |    6 -----
 4 files changed, 7 insertions(+), 95 deletions(-)

diff --git a/drivers/acpi/acpica/nsxfobj.c b/drivers/acpi/acpica/nsxfobj.c
index dae9401..51a8329 100644
--- a/drivers/acpi/acpica/nsxfobj.c
+++ b/drivers/acpi/acpica/nsxfobj.c
@@ -53,50 +53,6 @@ ACPI_MODULE_NAME("nsxfobj")
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_get_id
- *
- * PARAMETERS:  Handle          - Handle of object whose id is desired
- *              ret_id          - Where the id will be placed
- *
- * RETURN:      Status
- *
- * DESCRIPTION: This routine returns the owner id associated with a handle
- *
- ******************************************************************************/
-acpi_status acpi_get_id(acpi_handle handle, acpi_owner_id * ret_id)
-{
-	struct acpi_namespace_node *node;
-	acpi_status status;
-
-	/* Parameter Validation */
-
-	if (!ret_id) {
-		return (AE_BAD_PARAMETER);
-	}
-
-	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
-	if (ACPI_FAILURE(status)) {
-		return (status);
-	}
-
-	/* Convert and validate the handle */
-
-	node = acpi_ns_validate_handle(handle);
-	if (!node) {
-		(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
-		return (AE_BAD_PARAMETER);
-	}
-
-	*ret_id = node->owner_id;
-
-	status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
-	return (status);
-}
-
-ACPI_EXPORT_SYMBOL(acpi_get_id)
-
-/*******************************************************************************
- *
  * FUNCTION:    acpi_get_type
  *
  * PARAMETERS:  handle          - Handle of object whose type is desired
diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
index 6482b0d..0f9dd80 100644
--- a/drivers/acpi/acpica/tbxface.c
+++ b/drivers/acpi/acpica/tbxface.c
@@ -265,45 +265,6 @@ ACPI_EXPORT_SYMBOL(acpi_get_table_header)
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_unload_table_id
- *
- * PARAMETERS:  id            - Owner ID of the table to be removed.
- *
- * RETURN:      Status
- *
- * DESCRIPTION: This routine is used to force the unload of a table (by id)
- *
- ******************************************************************************/
-acpi_status acpi_unload_table_id(acpi_owner_id id)
-{
-	int i;
-	acpi_status status = AE_NOT_EXIST;
-
-	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
-
-	/* Find table in the global table list */
-	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
-		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
-			continue;
-		}
-		/*
-		 * Delete all namespace objects owned by this table. Note that these
-		 * objects can appear anywhere in the namespace by virtue of the AML
-		 * "Scope" operator. Thus, we need to track ownership by an ID, not
-		 * simply a position within the hierarchy
-		 */
-		acpi_tb_delete_namespace_by_owner(i);
-		status = acpi_tb_release_owner_id(i);
-		acpi_tb_set_table_loaded_flag(i, FALSE);
-		break;
-	}
-	return_ACPI_STATUS(status);
-}
-
-ACPI_EXPORT_SYMBOL(acpi_unload_table_id)
-
-/*******************************************************************************
- *
  * FUNCTION:    acpi_get_table_with_size
  *
  * PARAMETERS:  signature           - ACPI signature of needed table
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index bada2099..c32fb78 100644
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -475,7 +475,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
 	struct slot *slot = bss_hotplug_slot->private;
 	struct pci_dev *dev, *temp;
 	int rc;
-	acpi_owner_id ssdt_id = 0;
+	acpi_handle ssdt_hdl = NULL;
 
 	/* Acquire update access to the bus */
 	mutex_lock(&sn_hotplug_mutex);
@@ -522,7 +522,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
 			if (ACPI_SUCCESS(ret) &&
 			    (adr>>16) == (slot->device_num + 1)) {
 				/* retain the owner id */
-				acpi_get_id(chandle, &ssdt_id);
+				ssdt_hdl = chandle;
 
 				ret = acpi_bus_get_device(chandle,
 							  &device);
@@ -547,12 +547,13 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
 	pci_unlock_rescan_remove();
 
 	/* Remove the SSDT for the slot from the ACPI namespace */
-	if (SN_ACPI_BASE_SUPPORT() && ssdt_id) {
+	if (SN_ACPI_BASE_SUPPORT() && ssdt_hdl) {
 		acpi_status ret;
-		ret = acpi_unload_table_id(ssdt_id);
+		ret = acpi_unload_parent_table(ssdt_hdl);
 		if (ACPI_FAILURE(ret)) {
-			printk(KERN_ERR "%s: acpi_unload_table_id failed (0x%x) for id %d\n",
-			       __func__, ret, ssdt_id);
+			acpi_handle_err(ssdt_hdl,
+					"%s: acpi_unload_parent_table failed (0x%x)\n",
+					__func__, ret);
 			/* try to continue on */
 		}
 	}
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 5ba7846..5dd21bc 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -891,12 +891,6 @@ ACPI_APP_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
 ACPI_GLOBAL(u8, acpi_gbl_permanent_mmap);
 
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status
-			    acpi_get_id(acpi_handle object,
-					acpi_owner_id * out_type))
-
-ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_unload_table_id(acpi_owner_id id))
-
-ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 			    acpi_get_table_with_size(acpi_string signature,
 						     u32 instance,
 						     struct acpi_table_header
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
@ 2015-01-22  6:37     ` Lv Zheng
  0 siblings, 0 replies; 25+ messages in thread
From: Lv Zheng @ 2015-01-22  6:37 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: Lv Zheng, Lv Zheng, linux-acpi, Octavian Purdila,
	Prarit Bhargava, Bjorn Helgaas, linux-pci, Tony Luck, Fenghua Yu,
	linux-ia64

ACPICA has implemented acpi_unload_parent_table() which can exactly replace
the acpi_get_id()/acpi_unload_table_id() implemented in Linux kernel.  The
acpi_unload_parent_table() has been unit tested in ACPICA simulation
environment.

This patch can also help to reduce the source code differences between
Linux and ACPICA.

Signed-off-by: Lv Zheng <lv.zheng@intel.com>
Cc: Octavian Purdila <octavian.purdila@intel.com>
Cc: Prarit Bhargava <prarit@sgi.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
---
 drivers/acpi/acpica/nsxfobj.c     |   44 -------------------------------------
 drivers/acpi/acpica/tbxface.c     |   39 --------------------------------
 drivers/pci/hotplug/sgi_hotplug.c |   13 ++++++-----
 include/acpi/acpixf.h             |    6 -----
 4 files changed, 7 insertions(+), 95 deletions(-)

diff --git a/drivers/acpi/acpica/nsxfobj.c b/drivers/acpi/acpica/nsxfobj.c
index dae9401..51a8329 100644
--- a/drivers/acpi/acpica/nsxfobj.c
+++ b/drivers/acpi/acpica/nsxfobj.c
@@ -53,50 +53,6 @@ ACPI_MODULE_NAME("nsxfobj")
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_get_id
- *
- * PARAMETERS:  Handle          - Handle of object whose id is desired
- *              ret_id          - Where the id will be placed
- *
- * RETURN:      Status
- *
- * DESCRIPTION: This routine returns the owner id associated with a handle
- *
- ******************************************************************************/
-acpi_status acpi_get_id(acpi_handle handle, acpi_owner_id * ret_id)
-{
-	struct acpi_namespace_node *node;
-	acpi_status status;
-
-	/* Parameter Validation */
-
-	if (!ret_id) {
-		return (AE_BAD_PARAMETER);
-	}
-
-	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
-	if (ACPI_FAILURE(status)) {
-		return (status);
-	}
-
-	/* Convert and validate the handle */
-
-	node = acpi_ns_validate_handle(handle);
-	if (!node) {
-		(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
-		return (AE_BAD_PARAMETER);
-	}
-
-	*ret_id = node->owner_id;
-
-	status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
-	return (status);
-}
-
-ACPI_EXPORT_SYMBOL(acpi_get_id)
-
-/*******************************************************************************
- *
  * FUNCTION:    acpi_get_type
  *
  * PARAMETERS:  handle          - Handle of object whose type is desired
diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
index 6482b0d..0f9dd80 100644
--- a/drivers/acpi/acpica/tbxface.c
+++ b/drivers/acpi/acpica/tbxface.c
@@ -265,45 +265,6 @@ ACPI_EXPORT_SYMBOL(acpi_get_table_header)
 
 /*******************************************************************************
  *
- * FUNCTION:    acpi_unload_table_id
- *
- * PARAMETERS:  id            - Owner ID of the table to be removed.
- *
- * RETURN:      Status
- *
- * DESCRIPTION: This routine is used to force the unload of a table (by id)
- *
- ******************************************************************************/
-acpi_status acpi_unload_table_id(acpi_owner_id id)
-{
-	int i;
-	acpi_status status = AE_NOT_EXIST;
-
-	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
-
-	/* Find table in the global table list */
-	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
-		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
-			continue;
-		}
-		/*
-		 * Delete all namespace objects owned by this table. Note that these
-		 * objects can appear anywhere in the namespace by virtue of the AML
-		 * "Scope" operator. Thus, we need to track ownership by an ID, not
-		 * simply a position within the hierarchy
-		 */
-		acpi_tb_delete_namespace_by_owner(i);
-		status = acpi_tb_release_owner_id(i);
-		acpi_tb_set_table_loaded_flag(i, FALSE);
-		break;
-	}
-	return_ACPI_STATUS(status);
-}
-
-ACPI_EXPORT_SYMBOL(acpi_unload_table_id)
-
-/*******************************************************************************
- *
  * FUNCTION:    acpi_get_table_with_size
  *
  * PARAMETERS:  signature           - ACPI signature of needed table
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index bada2099..c32fb78 100644
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -475,7 +475,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
 	struct slot *slot = bss_hotplug_slot->private;
 	struct pci_dev *dev, *temp;
 	int rc;
-	acpi_owner_id ssdt_id = 0;
+	acpi_handle ssdt_hdl = NULL;
 
 	/* Acquire update access to the bus */
 	mutex_lock(&sn_hotplug_mutex);
@@ -522,7 +522,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
 			if (ACPI_SUCCESS(ret) &&
 			    (adr>>16) = (slot->device_num + 1)) {
 				/* retain the owner id */
-				acpi_get_id(chandle, &ssdt_id);
+				ssdt_hdl = chandle;
 
 				ret = acpi_bus_get_device(chandle,
 							  &device);
@@ -547,12 +547,13 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
 	pci_unlock_rescan_remove();
 
 	/* Remove the SSDT for the slot from the ACPI namespace */
-	if (SN_ACPI_BASE_SUPPORT() && ssdt_id) {
+	if (SN_ACPI_BASE_SUPPORT() && ssdt_hdl) {
 		acpi_status ret;
-		ret = acpi_unload_table_id(ssdt_id);
+		ret = acpi_unload_parent_table(ssdt_hdl);
 		if (ACPI_FAILURE(ret)) {
-			printk(KERN_ERR "%s: acpi_unload_table_id failed (0x%x) for id %d\n",
-			       __func__, ret, ssdt_id);
+			acpi_handle_err(ssdt_hdl,
+					"%s: acpi_unload_parent_table failed (0x%x)\n",
+					__func__, ret);
 			/* try to continue on */
 		}
 	}
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 5ba7846..5dd21bc 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -891,12 +891,6 @@ ACPI_APP_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
 ACPI_GLOBAL(u8, acpi_gbl_permanent_mmap);
 
 ACPI_EXTERNAL_RETURN_STATUS(acpi_status
-			    acpi_get_id(acpi_handle object,
-					acpi_owner_id * out_type))
-
-ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_unload_table_id(acpi_owner_id id))
-
-ACPI_EXTERNAL_RETURN_STATUS(acpi_status
 			    acpi_get_table_with_size(acpi_string signature,
 						     u32 instance,
 						     struct acpi_table_header
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/4] mfd: dln2: add support for ACPI
  2015-01-22  2:00   ` Rafael J. Wysocki
@ 2015-01-22 10:13     ` Octavian Purdila
  2015-01-22 22:09       ` Rafael J. Wysocki
  0 siblings, 1 reply; 25+ messages in thread
From: Octavian Purdila @ 2015-01-22 10:13 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linus Walleij, Lee Jones, Johan Hovold, linux-usb, lkml,
	linux-gpio, Heikki Krogerus, Mika Westerberg, linux-acpi

On Thu, Jan 22, 2015 at 4:00 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>

Hi Rafael,

Thanks for reviewing the patch.

> On Tuesday, December 16, 2014 06:12:32 PM Octavian Purdila wrote:
> > This patch adds support to load a custom ACPI table that describes
> > devices connected via the DLN2 USB to I2C/SPI/GPIO bridge.
> >
> > The ACPI table can be loaded either externally (from QEMU or with
> > CONFIG_ACPI_CUSTOM_DSDT) or it can be loaded as firmware file with the
> > name dln2.aml. The driver looks for an ACPI device entry with _HID set
> > to "DLN20000" and makes it the ACPI companion for DLN2 USB
> > sub-drivers.
> >
> > Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> > ---
> >  Documentation/acpi/dln2-acpi.txt |  62 ++++++++++++++++++
> >  drivers/mfd/dln2.c               | 134 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 196 insertions(+)
> >  create mode 100644 Documentation/acpi/dln2-acpi.txt
> >
> > diff --git a/Documentation/acpi/dln2-acpi.txt b/Documentation/acpi/dln2-acpi.txt
> > new file mode 100644
> > index 0000000..d76605f
> > --- /dev/null
> > +++ b/Documentation/acpi/dln2-acpi.txt
> > @@ -0,0 +1,62 @@
> > +Diolan DLN2 custom APCI table
> > +
> > +The Diolan DLN2 is an USB to I2C/SPI/GPIO bridge and as such it can be used to
> > +connect to various I2C or SPI devices. Because these busses lack an enumeration
> > +protocol, the driver obtains various information about the device (such as I2C
> > +address and GPIO pins) from either ACPI or device tree.
> > +
> > +To allow enumerating devices and their properties via ACPI, the Diolan
> > +driver looks for an ACPI tree with the root _HID set to "DLN20000". If
> > +it finds such an ACPI object it will set the ACPI companion to the
> > +DLN2 MFD driver and from their it will be propagated to all its
> > +sub-devices (I2C, GPIO, SPI).
> > +
> > +The user can either load the custom DSDT table with three methods:
>
> s/DSDT/ACPI/

OK.

> > +
> > +1. Via QEMU (see -acpitable)
> > +
> > +2. Via the CONFIG_ACPI_CUSTOM_DSDT kernel config option (see
> > +Documentation/acpi/dsdt-override.txt)
> > +
> > +3. By placing the custom DSDT in the firmware paths in a file name
> > +dln2.aml.
>
> Surely SSDT?
>

Correct.

<snip>

> > diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
> > index f9c4a0b..93f6d1d 100644
> > --- a/drivers/mfd/dln2.c
> > +++ b/drivers/mfd/dln2.c
> > @@ -23,6 +23,8 @@
> >  #include <linux/mfd/core.h>
> >  #include <linux/mfd/dln2.h>
> >  #include <linux/rculist.h>
> > +#include <linux/acpi.h>
> > +#include <linux/firmware.h>
> >
>
> OK, so correct me if I'm wrong.
>
> When the (USB) dnl2 device is probed, it is supposed to find an ACPI companion
> for itself and if it's not there, the driver will try to load a custom SSDT from
> a firmware blob in the hope that the companion will be there?
>
> So if I'm not wrong, why is this not broken?
>
> It is not sufficient to call ACPI_COMPANION_SET(dev, ai->dev) to set the device's
> companion.  acpi_bind_one() needs to be run in addition to that, but acpi_bind_one()
> is not to be called from drivers.  It is called by the core automatically during
> device registration and ACPI_COMPANION_SET() is to be used *before* that, not after.
>
> So if I'm not missing anything, the design here is entirely backwards and we
> need to talk about how to implement it correctly at the design level in the
> first place.
>

The idea here is to set the companion for the MFD sub-devices. Mika's
commit "mfd: Add ACPI support" propagates the parent's companion to
the MFD sub-devices, so it is sufficient to set the ACPI companion to
the USB device.

Then, the companion will be propagated to the sub-devices after which
acpi_bind_one() will be called for the sub-devices from
mfd_add_devices (via platform_device_add -> device_add ->
platform_notify).

It is true that the USB dev will have its ACPI companion set without
having acpi_bind_one called but I do not see any harm in that. Even
though acpi_unbind_one() is called it will not find the USB dev on the
physical node list so no put_device() imbalance is caused.

> And no, "Let's come up with a patch that sort of works, throw it at the maintainers
> and see what happens" is not an acceptable approach, sorry.

This patch is based on your feedback of the previous RFC patch set:

On Thu, Dec 11, 2014 at 11:44 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:

> it seems to me that it would be much more straightforward to check for the existence
> of the "DLN20000" device when you are about to register DLN2 USB sub-devices
> and set it directly as an ACPI companion for them if present.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/4] mfd: dln2: add support for ACPI
  2015-01-22 10:13     ` Octavian Purdila
@ 2015-01-22 22:09       ` Rafael J. Wysocki
  2015-01-23  6:47         ` Octavian Purdila
  0 siblings, 1 reply; 25+ messages in thread
From: Rafael J. Wysocki @ 2015-01-22 22:09 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: Linus Walleij, Lee Jones, Johan Hovold, linux-usb, lkml,
	linux-gpio, Heikki Krogerus, Mika Westerberg, linux-acpi

On Thursday, January 22, 2015 12:13:13 PM Octavian Purdila wrote:
> On Thu, Jan 22, 2015 at 4:00 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> >
> 
> Hi Rafael,
> 
> Thanks for reviewing the patch.
> 
> > On Tuesday, December 16, 2014 06:12:32 PM Octavian Purdila wrote:
> > > This patch adds support to load a custom ACPI table that describes
> > > devices connected via the DLN2 USB to I2C/SPI/GPIO bridge.
> > >
> > > The ACPI table can be loaded either externally (from QEMU or with
> > > CONFIG_ACPI_CUSTOM_DSDT) or it can be loaded as firmware file with the
> > > name dln2.aml. The driver looks for an ACPI device entry with _HID set
> > > to "DLN20000" and makes it the ACPI companion for DLN2 USB
> > > sub-drivers.
> > >
> > > Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> > > ---
> > >  Documentation/acpi/dln2-acpi.txt |  62 ++++++++++++++++++
> > >  drivers/mfd/dln2.c               | 134 +++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 196 insertions(+)
> > >  create mode 100644 Documentation/acpi/dln2-acpi.txt
> > >
> > > diff --git a/Documentation/acpi/dln2-acpi.txt b/Documentation/acpi/dln2-acpi.txt
> > > new file mode 100644
> > > index 0000000..d76605f
> > > --- /dev/null
> > > +++ b/Documentation/acpi/dln2-acpi.txt
> > > @@ -0,0 +1,62 @@
> > > +Diolan DLN2 custom APCI table
> > > +
> > > +The Diolan DLN2 is an USB to I2C/SPI/GPIO bridge and as such it can be used to
> > > +connect to various I2C or SPI devices. Because these busses lack an enumeration
> > > +protocol, the driver obtains various information about the device (such as I2C
> > > +address and GPIO pins) from either ACPI or device tree.
> > > +
> > > +To allow enumerating devices and their properties via ACPI, the Diolan
> > > +driver looks for an ACPI tree with the root _HID set to "DLN20000". If
> > > +it finds such an ACPI object it will set the ACPI companion to the
> > > +DLN2 MFD driver and from their it will be propagated to all its
> > > +sub-devices (I2C, GPIO, SPI).
> > > +
> > > +The user can either load the custom DSDT table with three methods:
> >
> > s/DSDT/ACPI/
> 
> OK.
> 
> > > +
> > > +1. Via QEMU (see -acpitable)
> > > +
> > > +2. Via the CONFIG_ACPI_CUSTOM_DSDT kernel config option (see
> > > +Documentation/acpi/dsdt-override.txt)
> > > +
> > > +3. By placing the custom DSDT in the firmware paths in a file name
> > > +dln2.aml.
> >
> > Surely SSDT?
> >
> 
> Correct.
> 
> <snip>
> 
> > > diff --git a/drivers/mfd/dln2.c b/drivers/mfd/dln2.c
> > > index f9c4a0b..93f6d1d 100644
> > > --- a/drivers/mfd/dln2.c
> > > +++ b/drivers/mfd/dln2.c
> > > @@ -23,6 +23,8 @@
> > >  #include <linux/mfd/core.h>
> > >  #include <linux/mfd/dln2.h>
> > >  #include <linux/rculist.h>
> > > +#include <linux/acpi.h>
> > > +#include <linux/firmware.h>
> > >
> >
> > OK, so correct me if I'm wrong.
> >
> > When the (USB) dnl2 device is probed, it is supposed to find an ACPI companion
> > for itself and if it's not there, the driver will try to load a custom SSDT from
> > a firmware blob in the hope that the companion will be there?
> >
> > So if I'm not wrong, why is this not broken?
> >
> > It is not sufficient to call ACPI_COMPANION_SET(dev, ai->dev) to set the device's
> > companion.  acpi_bind_one() needs to be run in addition to that, but acpi_bind_one()
> > is not to be called from drivers.  It is called by the core automatically during
> > device registration and ACPI_COMPANION_SET() is to be used *before* that, not after.
> >
> > So if I'm not missing anything, the design here is entirely backwards and we
> > need to talk about how to implement it correctly at the design level in the
> > first place.
> >
> 
> The idea here is to set the companion for the MFD sub-devices. Mika's
> commit "mfd: Add ACPI support" propagates the parent's companion to
> the MFD sub-devices, so it is sufficient to set the ACPI companion to
> the USB device.

For the USB device itself you'll then end up with an incomplete binding (you
can't get back to the USB device from the ACPI object), so no, it isn't
sufficient.

> Then, the companion will be propagated to the sub-devices after which
> acpi_bind_one() will be called for the sub-devices from
> mfd_add_devices (via platform_device_add -> device_add ->
> platform_notify).

In fact, your use case doesn't seem to cover any of the use cases that
the Mika's commit addressed.  Namely, your parent device doesn't have
an ACPI companion to start with and you want your MFD cells to be bound
to the "DLN2000" thing.  That's why you're setting the ACPI companion
for the USB device, isn't it?

> It is true that the USB dev will have its ACPI companion set without
> having acpi_bind_one called but I do not see any harm in that. Even
> though acpi_unbind_one() is called it will not find the USB dev on the
> physical node list so no put_device() imbalance is caused.

Well, there are places where it matters.  Some links in sysfs will be missing
for one example.  Also please see the changelog of commit 52870786ff5d (ACPI:
Use ACPI companion to match only the first physical device).

Bottom line: You really should be using acpi_bind_one() here and
acpi_unbind_one() on disconnect if you have to.

> > And no, "Let's come up with a patch that sort of works, throw it at the maintainers
> > and see what happens" is not an acceptable approach, sorry.
> 
> This patch is based on your feedback of the previous RFC patch set:

Oh, is it?  I can't recall advising you to use request_firmware() for
uploading ACPI tables or some other questionable things that the patch is
doing.

And if it still was an RFC, that wouldn't be a problem, but if you just send
non-RFC patches out, that means you want people to merge them.  This is bad
if the patches in question are not in a good enough shape and this one isn't.

Now, why is this a bad idea to load ACPI tables from a driver using
request_firmware()?  Because those tables are not device firmare.  They are
not firmware that you load into a device to make it work (which then works
with all instances of the given hardware), they are part of system configuration
information and have to match what's there in the system.  For instance, if you
ship your example SSDT with a general-purpose distro, it may just not match the
hardware configuration of systems that people will try to use it with.

So, while it is sort of OK to look up "DLN2000" and bind the USB device to
that by hand (although this looks ugly to me), it is not OK to load a random
custom SSDT if it is missing.

We need to add a generic mechanism for loading custom SSDTs not present in the
firmware image and maybe even to load them on demand, but that cannot happen in
an ad-hoc way.  So the entire table loading-unloading code in your patch can go
away for now and you need to fail probing if the "DLN2000" ACPI object is not
present.

Also I think you need to add "DLN2000" to the blacklist in drivers/acpi/acpi_platform.c
to prevent the ACPI core from creating a platform device for it automatically.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
  2015-01-22  6:37     ` Lv Zheng
@ 2015-01-22 22:43       ` Rafael J. Wysocki
  -1 siblings, 0 replies; 25+ messages in thread
From: Rafael J. Wysocki @ 2015-01-22 22:43 UTC (permalink / raw)
  To: Lv Zheng
  Cc: Rafael J. Wysocki, Len Brown, Lv Zheng, linux-acpi,
	Octavian Purdila, Prarit Bhargava, Bjorn Helgaas, linux-pci,
	Tony Luck, Fenghua Yu, linux-ia64

On Thursday, January 22, 2015 02:37:49 PM Lv Zheng wrote:
> ACPICA has implemented acpi_unload_parent_table() which can exactly replace
> the acpi_get_id()/acpi_unload_table_id() implemented in Linux kernel.  The
> acpi_unload_parent_table() has been unit tested in ACPICA simulation
> environment.
> 
> This patch can also help to reduce the source code differences between
> Linux and ACPICA.

Looks OK to me.

Do you want me to pick it up?

> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Cc: Octavian Purdila <octavian.purdila@intel.com>
> Cc: Prarit Bhargava <prarit@sgi.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: linux-ia64@vger.kernel.org
> ---
>  drivers/acpi/acpica/nsxfobj.c     |   44 -------------------------------------
>  drivers/acpi/acpica/tbxface.c     |   39 --------------------------------
>  drivers/pci/hotplug/sgi_hotplug.c |   13 ++++++-----
>  include/acpi/acpixf.h             |    6 -----
>  4 files changed, 7 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/nsxfobj.c b/drivers/acpi/acpica/nsxfobj.c
> index dae9401..51a8329 100644
> --- a/drivers/acpi/acpica/nsxfobj.c
> +++ b/drivers/acpi/acpica/nsxfobj.c
> @@ -53,50 +53,6 @@ ACPI_MODULE_NAME("nsxfobj")
>  
>  /*******************************************************************************
>   *
> - * FUNCTION:    acpi_get_id
> - *
> - * PARAMETERS:  Handle          - Handle of object whose id is desired
> - *              ret_id          - Where the id will be placed
> - *
> - * RETURN:      Status
> - *
> - * DESCRIPTION: This routine returns the owner id associated with a handle
> - *
> - ******************************************************************************/
> -acpi_status acpi_get_id(acpi_handle handle, acpi_owner_id * ret_id)
> -{
> -	struct acpi_namespace_node *node;
> -	acpi_status status;
> -
> -	/* Parameter Validation */
> -
> -	if (!ret_id) {
> -		return (AE_BAD_PARAMETER);
> -	}
> -
> -	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> -	if (ACPI_FAILURE(status)) {
> -		return (status);
> -	}
> -
> -	/* Convert and validate the handle */
> -
> -	node = acpi_ns_validate_handle(handle);
> -	if (!node) {
> -		(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> -		return (AE_BAD_PARAMETER);
> -	}
> -
> -	*ret_id = node->owner_id;
> -
> -	status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> -	return (status);
> -}
> -
> -ACPI_EXPORT_SYMBOL(acpi_get_id)
> -
> -/*******************************************************************************
> - *
>   * FUNCTION:    acpi_get_type
>   *
>   * PARAMETERS:  handle          - Handle of object whose type is desired
> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
> index 6482b0d..0f9dd80 100644
> --- a/drivers/acpi/acpica/tbxface.c
> +++ b/drivers/acpi/acpica/tbxface.c
> @@ -265,45 +265,6 @@ ACPI_EXPORT_SYMBOL(acpi_get_table_header)
>  
>  /*******************************************************************************
>   *
> - * FUNCTION:    acpi_unload_table_id
> - *
> - * PARAMETERS:  id            - Owner ID of the table to be removed.
> - *
> - * RETURN:      Status
> - *
> - * DESCRIPTION: This routine is used to force the unload of a table (by id)
> - *
> - ******************************************************************************/
> -acpi_status acpi_unload_table_id(acpi_owner_id id)
> -{
> -	int i;
> -	acpi_status status = AE_NOT_EXIST;
> -
> -	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
> -
> -	/* Find table in the global table list */
> -	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
> -		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
> -			continue;
> -		}
> -		/*
> -		 * Delete all namespace objects owned by this table. Note that these
> -		 * objects can appear anywhere in the namespace by virtue of the AML
> -		 * "Scope" operator. Thus, we need to track ownership by an ID, not
> -		 * simply a position within the hierarchy
> -		 */
> -		acpi_tb_delete_namespace_by_owner(i);
> -		status = acpi_tb_release_owner_id(i);
> -		acpi_tb_set_table_loaded_flag(i, FALSE);
> -		break;
> -	}
> -	return_ACPI_STATUS(status);
> -}
> -
> -ACPI_EXPORT_SYMBOL(acpi_unload_table_id)
> -
> -/*******************************************************************************
> - *
>   * FUNCTION:    acpi_get_table_with_size
>   *
>   * PARAMETERS:  signature           - ACPI signature of needed table
> diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
> index bada2099..c32fb78 100644
> --- a/drivers/pci/hotplug/sgi_hotplug.c
> +++ b/drivers/pci/hotplug/sgi_hotplug.c
> @@ -475,7 +475,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>  	struct slot *slot = bss_hotplug_slot->private;
>  	struct pci_dev *dev, *temp;
>  	int rc;
> -	acpi_owner_id ssdt_id = 0;
> +	acpi_handle ssdt_hdl = NULL;
>  
>  	/* Acquire update access to the bus */
>  	mutex_lock(&sn_hotplug_mutex);
> @@ -522,7 +522,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>  			if (ACPI_SUCCESS(ret) &&
>  			    (adr>>16) == (slot->device_num + 1)) {
>  				/* retain the owner id */
> -				acpi_get_id(chandle, &ssdt_id);
> +				ssdt_hdl = chandle;
>  
>  				ret = acpi_bus_get_device(chandle,
>  							  &device);
> @@ -547,12 +547,13 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>  	pci_unlock_rescan_remove();
>  
>  	/* Remove the SSDT for the slot from the ACPI namespace */
> -	if (SN_ACPI_BASE_SUPPORT() && ssdt_id) {
> +	if (SN_ACPI_BASE_SUPPORT() && ssdt_hdl) {
>  		acpi_status ret;
> -		ret = acpi_unload_table_id(ssdt_id);
> +		ret = acpi_unload_parent_table(ssdt_hdl);
>  		if (ACPI_FAILURE(ret)) {
> -			printk(KERN_ERR "%s: acpi_unload_table_id failed (0x%x) for id %d\n",
> -			       __func__, ret, ssdt_id);
> +			acpi_handle_err(ssdt_hdl,
> +					"%s: acpi_unload_parent_table failed (0x%x)\n",
> +					__func__, ret);
>  			/* try to continue on */
>  		}
>  	}
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index 5ba7846..5dd21bc 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -891,12 +891,6 @@ ACPI_APP_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
>  ACPI_GLOBAL(u8, acpi_gbl_permanent_mmap);
>  
>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> -			    acpi_get_id(acpi_handle object,
> -					acpi_owner_id * out_type))
> -
> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_unload_table_id(acpi_owner_id id))
> -
> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>  			    acpi_get_table_with_size(acpi_string signature,
>  						     u32 instance,
>  						     struct acpi_table_header
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
@ 2015-01-22 22:43       ` Rafael J. Wysocki
  0 siblings, 0 replies; 25+ messages in thread
From: Rafael J. Wysocki @ 2015-01-22 22:43 UTC (permalink / raw)
  To: Lv Zheng
  Cc: Rafael J. Wysocki, Len Brown, Lv Zheng, linux-acpi,
	Octavian Purdila, Prarit Bhargava, Bjorn Helgaas, linux-pci,
	Tony Luck, Fenghua Yu, linux-ia64

On Thursday, January 22, 2015 02:37:49 PM Lv Zheng wrote:
> ACPICA has implemented acpi_unload_parent_table() which can exactly replace
> the acpi_get_id()/acpi_unload_table_id() implemented in Linux kernel.  The
> acpi_unload_parent_table() has been unit tested in ACPICA simulation
> environment.
> 
> This patch can also help to reduce the source code differences between
> Linux and ACPICA.

Looks OK to me.

Do you want me to pick it up?

> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> Cc: Octavian Purdila <octavian.purdila@intel.com>
> Cc: Prarit Bhargava <prarit@sgi.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: linux-ia64@vger.kernel.org
> ---
>  drivers/acpi/acpica/nsxfobj.c     |   44 -------------------------------------
>  drivers/acpi/acpica/tbxface.c     |   39 --------------------------------
>  drivers/pci/hotplug/sgi_hotplug.c |   13 ++++++-----
>  include/acpi/acpixf.h             |    6 -----
>  4 files changed, 7 insertions(+), 95 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/nsxfobj.c b/drivers/acpi/acpica/nsxfobj.c
> index dae9401..51a8329 100644
> --- a/drivers/acpi/acpica/nsxfobj.c
> +++ b/drivers/acpi/acpica/nsxfobj.c
> @@ -53,50 +53,6 @@ ACPI_MODULE_NAME("nsxfobj")
>  
>  /*******************************************************************************
>   *
> - * FUNCTION:    acpi_get_id
> - *
> - * PARAMETERS:  Handle          - Handle of object whose id is desired
> - *              ret_id          - Where the id will be placed
> - *
> - * RETURN:      Status
> - *
> - * DESCRIPTION: This routine returns the owner id associated with a handle
> - *
> - ******************************************************************************/
> -acpi_status acpi_get_id(acpi_handle handle, acpi_owner_id * ret_id)
> -{
> -	struct acpi_namespace_node *node;
> -	acpi_status status;
> -
> -	/* Parameter Validation */
> -
> -	if (!ret_id) {
> -		return (AE_BAD_PARAMETER);
> -	}
> -
> -	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> -	if (ACPI_FAILURE(status)) {
> -		return (status);
> -	}
> -
> -	/* Convert and validate the handle */
> -
> -	node = acpi_ns_validate_handle(handle);
> -	if (!node) {
> -		(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> -		return (AE_BAD_PARAMETER);
> -	}
> -
> -	*ret_id = node->owner_id;
> -
> -	status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> -	return (status);
> -}
> -
> -ACPI_EXPORT_SYMBOL(acpi_get_id)
> -
> -/*******************************************************************************
> - *
>   * FUNCTION:    acpi_get_type
>   *
>   * PARAMETERS:  handle          - Handle of object whose type is desired
> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
> index 6482b0d..0f9dd80 100644
> --- a/drivers/acpi/acpica/tbxface.c
> +++ b/drivers/acpi/acpica/tbxface.c
> @@ -265,45 +265,6 @@ ACPI_EXPORT_SYMBOL(acpi_get_table_header)
>  
>  /*******************************************************************************
>   *
> - * FUNCTION:    acpi_unload_table_id
> - *
> - * PARAMETERS:  id            - Owner ID of the table to be removed.
> - *
> - * RETURN:      Status
> - *
> - * DESCRIPTION: This routine is used to force the unload of a table (by id)
> - *
> - ******************************************************************************/
> -acpi_status acpi_unload_table_id(acpi_owner_id id)
> -{
> -	int i;
> -	acpi_status status = AE_NOT_EXIST;
> -
> -	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
> -
> -	/* Find table in the global table list */
> -	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
> -		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
> -			continue;
> -		}
> -		/*
> -		 * Delete all namespace objects owned by this table. Note that these
> -		 * objects can appear anywhere in the namespace by virtue of the AML
> -		 * "Scope" operator. Thus, we need to track ownership by an ID, not
> -		 * simply a position within the hierarchy
> -		 */
> -		acpi_tb_delete_namespace_by_owner(i);
> -		status = acpi_tb_release_owner_id(i);
> -		acpi_tb_set_table_loaded_flag(i, FALSE);
> -		break;
> -	}
> -	return_ACPI_STATUS(status);
> -}
> -
> -ACPI_EXPORT_SYMBOL(acpi_unload_table_id)
> -
> -/*******************************************************************************
> - *
>   * FUNCTION:    acpi_get_table_with_size
>   *
>   * PARAMETERS:  signature           - ACPI signature of needed table
> diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
> index bada2099..c32fb78 100644
> --- a/drivers/pci/hotplug/sgi_hotplug.c
> +++ b/drivers/pci/hotplug/sgi_hotplug.c
> @@ -475,7 +475,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>  	struct slot *slot = bss_hotplug_slot->private;
>  	struct pci_dev *dev, *temp;
>  	int rc;
> -	acpi_owner_id ssdt_id = 0;
> +	acpi_handle ssdt_hdl = NULL;
>  
>  	/* Acquire update access to the bus */
>  	mutex_lock(&sn_hotplug_mutex);
> @@ -522,7 +522,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>  			if (ACPI_SUCCESS(ret) &&
>  			    (adr>>16) = (slot->device_num + 1)) {
>  				/* retain the owner id */
> -				acpi_get_id(chandle, &ssdt_id);
> +				ssdt_hdl = chandle;
>  
>  				ret = acpi_bus_get_device(chandle,
>  							  &device);
> @@ -547,12 +547,13 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>  	pci_unlock_rescan_remove();
>  
>  	/* Remove the SSDT for the slot from the ACPI namespace */
> -	if (SN_ACPI_BASE_SUPPORT() && ssdt_id) {
> +	if (SN_ACPI_BASE_SUPPORT() && ssdt_hdl) {
>  		acpi_status ret;
> -		ret = acpi_unload_table_id(ssdt_id);
> +		ret = acpi_unload_parent_table(ssdt_hdl);
>  		if (ACPI_FAILURE(ret)) {
> -			printk(KERN_ERR "%s: acpi_unload_table_id failed (0x%x) for id %d\n",
> -			       __func__, ret, ssdt_id);
> +			acpi_handle_err(ssdt_hdl,
> +					"%s: acpi_unload_parent_table failed (0x%x)\n",
> +					__func__, ret);
>  			/* try to continue on */
>  		}
>  	}
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index 5ba7846..5dd21bc 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -891,12 +891,6 @@ ACPI_APP_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
>  ACPI_GLOBAL(u8, acpi_gbl_permanent_mmap);
>  
>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> -			    acpi_get_id(acpi_handle object,
> -					acpi_owner_id * out_type))
> -
> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_unload_table_id(acpi_owner_id id))
> -
> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>  			    acpi_get_table_with_size(acpi_string signature,
>  						     u32 instance,
>  						     struct acpi_table_header
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
  2015-01-22 22:43       ` Rafael J. Wysocki
@ 2015-01-22 22:43         ` Bjorn Helgaas
  -1 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-01-22 22:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lv Zheng, Rafael J. Wysocki, Len Brown, Lv Zheng, linux-acpi,
	Octavian Purdila, Prarit Bhargava, linux-pci, Tony Luck,
	Fenghua Yu, linux-ia64

On Thu, Jan 22, 2015 at 4:43 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, January 22, 2015 02:37:49 PM Lv Zheng wrote:
>> ACPICA has implemented acpi_unload_parent_table() which can exactly replace
>> the acpi_get_id()/acpi_unload_table_id() implemented in Linux kernel.  The
>> acpi_unload_parent_table() has been unit tested in ACPICA simulation
>> environment.
>>
>> This patch can also help to reduce the source code differences between
>> Linux and ACPICA.
>
> Looks OK to me.
>
> Do you want me to pick it up?

OK by me if you take it.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> Cc: Octavian Purdila <octavian.purdila@intel.com>
>> Cc: Prarit Bhargava <prarit@sgi.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: linux-pci@vger.kernel.org
>> Cc: Tony Luck <tony.luck@intel.com>
>> Cc: Fenghua Yu <fenghua.yu@intel.com>
>> Cc: linux-ia64@vger.kernel.org
>> ---
>>  drivers/acpi/acpica/nsxfobj.c     |   44 -------------------------------------
>>  drivers/acpi/acpica/tbxface.c     |   39 --------------------------------
>>  drivers/pci/hotplug/sgi_hotplug.c |   13 ++++++-----
>>  include/acpi/acpixf.h             |    6 -----
>>  4 files changed, 7 insertions(+), 95 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/nsxfobj.c b/drivers/acpi/acpica/nsxfobj.c
>> index dae9401..51a8329 100644
>> --- a/drivers/acpi/acpica/nsxfobj.c
>> +++ b/drivers/acpi/acpica/nsxfobj.c
>> @@ -53,50 +53,6 @@ ACPI_MODULE_NAME("nsxfobj")
>>
>>  /*******************************************************************************
>>   *
>> - * FUNCTION:    acpi_get_id
>> - *
>> - * PARAMETERS:  Handle          - Handle of object whose id is desired
>> - *              ret_id          - Where the id will be placed
>> - *
>> - * RETURN:      Status
>> - *
>> - * DESCRIPTION: This routine returns the owner id associated with a handle
>> - *
>> - ******************************************************************************/
>> -acpi_status acpi_get_id(acpi_handle handle, acpi_owner_id * ret_id)
>> -{
>> -     struct acpi_namespace_node *node;
>> -     acpi_status status;
>> -
>> -     /* Parameter Validation */
>> -
>> -     if (!ret_id) {
>> -             return (AE_BAD_PARAMETER);
>> -     }
>> -
>> -     status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
>> -     if (ACPI_FAILURE(status)) {
>> -             return (status);
>> -     }
>> -
>> -     /* Convert and validate the handle */
>> -
>> -     node = acpi_ns_validate_handle(handle);
>> -     if (!node) {
>> -             (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
>> -             return (AE_BAD_PARAMETER);
>> -     }
>> -
>> -     *ret_id = node->owner_id;
>> -
>> -     status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
>> -     return (status);
>> -}
>> -
>> -ACPI_EXPORT_SYMBOL(acpi_get_id)
>> -
>> -/*******************************************************************************
>> - *
>>   * FUNCTION:    acpi_get_type
>>   *
>>   * PARAMETERS:  handle          - Handle of object whose type is desired
>> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
>> index 6482b0d..0f9dd80 100644
>> --- a/drivers/acpi/acpica/tbxface.c
>> +++ b/drivers/acpi/acpica/tbxface.c
>> @@ -265,45 +265,6 @@ ACPI_EXPORT_SYMBOL(acpi_get_table_header)
>>
>>  /*******************************************************************************
>>   *
>> - * FUNCTION:    acpi_unload_table_id
>> - *
>> - * PARAMETERS:  id            - Owner ID of the table to be removed.
>> - *
>> - * RETURN:      Status
>> - *
>> - * DESCRIPTION: This routine is used to force the unload of a table (by id)
>> - *
>> - ******************************************************************************/
>> -acpi_status acpi_unload_table_id(acpi_owner_id id)
>> -{
>> -     int i;
>> -     acpi_status status = AE_NOT_EXIST;
>> -
>> -     ACPI_FUNCTION_TRACE(acpi_unload_table_id);
>> -
>> -     /* Find table in the global table list */
>> -     for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
>> -             if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
>> -                     continue;
>> -             }
>> -             /*
>> -              * Delete all namespace objects owned by this table. Note that these
>> -              * objects can appear anywhere in the namespace by virtue of the AML
>> -              * "Scope" operator. Thus, we need to track ownership by an ID, not
>> -              * simply a position within the hierarchy
>> -              */
>> -             acpi_tb_delete_namespace_by_owner(i);
>> -             status = acpi_tb_release_owner_id(i);
>> -             acpi_tb_set_table_loaded_flag(i, FALSE);
>> -             break;
>> -     }
>> -     return_ACPI_STATUS(status);
>> -}
>> -
>> -ACPI_EXPORT_SYMBOL(acpi_unload_table_id)
>> -
>> -/*******************************************************************************
>> - *
>>   * FUNCTION:    acpi_get_table_with_size
>>   *
>>   * PARAMETERS:  signature           - ACPI signature of needed table
>> diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
>> index bada2099..c32fb78 100644
>> --- a/drivers/pci/hotplug/sgi_hotplug.c
>> +++ b/drivers/pci/hotplug/sgi_hotplug.c
>> @@ -475,7 +475,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>>       struct slot *slot = bss_hotplug_slot->private;
>>       struct pci_dev *dev, *temp;
>>       int rc;
>> -     acpi_owner_id ssdt_id = 0;
>> +     acpi_handle ssdt_hdl = NULL;
>>
>>       /* Acquire update access to the bus */
>>       mutex_lock(&sn_hotplug_mutex);
>> @@ -522,7 +522,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>>                       if (ACPI_SUCCESS(ret) &&
>>                           (adr>>16) == (slot->device_num + 1)) {
>>                               /* retain the owner id */
>> -                             acpi_get_id(chandle, &ssdt_id);
>> +                             ssdt_hdl = chandle;
>>
>>                               ret = acpi_bus_get_device(chandle,
>>                                                         &device);
>> @@ -547,12 +547,13 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>>       pci_unlock_rescan_remove();
>>
>>       /* Remove the SSDT for the slot from the ACPI namespace */
>> -     if (SN_ACPI_BASE_SUPPORT() && ssdt_id) {
>> +     if (SN_ACPI_BASE_SUPPORT() && ssdt_hdl) {
>>               acpi_status ret;
>> -             ret = acpi_unload_table_id(ssdt_id);
>> +             ret = acpi_unload_parent_table(ssdt_hdl);
>>               if (ACPI_FAILURE(ret)) {
>> -                     printk(KERN_ERR "%s: acpi_unload_table_id failed (0x%x) for id %d\n",
>> -                            __func__, ret, ssdt_id);
>> +                     acpi_handle_err(ssdt_hdl,
>> +                                     "%s: acpi_unload_parent_table failed (0x%x)\n",
>> +                                     __func__, ret);
>>                       /* try to continue on */
>>               }
>>       }
>> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
>> index 5ba7846..5dd21bc 100644
>> --- a/include/acpi/acpixf.h
>> +++ b/include/acpi/acpixf.h
>> @@ -891,12 +891,6 @@ ACPI_APP_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
>>  ACPI_GLOBAL(u8, acpi_gbl_permanent_mmap);
>>
>>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>> -                         acpi_get_id(acpi_handle object,
>> -                                     acpi_owner_id * out_type))
>> -
>> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_unload_table_id(acpi_owner_id id))
>> -
>> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>>                           acpi_get_table_with_size(acpi_string signature,
>>                                                    u32 instance,
>>                                                    struct acpi_table_header
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
@ 2015-01-22 22:43         ` Bjorn Helgaas
  0 siblings, 0 replies; 25+ messages in thread
From: Bjorn Helgaas @ 2015-01-22 22:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Lv Zheng, Rafael J. Wysocki, Len Brown, Lv Zheng, linux-acpi,
	Octavian Purdila, Prarit Bhargava, linux-pci, Tony Luck,
	Fenghua Yu, linux-ia64

On Thu, Jan 22, 2015 at 4:43 PM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, January 22, 2015 02:37:49 PM Lv Zheng wrote:
>> ACPICA has implemented acpi_unload_parent_table() which can exactly replace
>> the acpi_get_id()/acpi_unload_table_id() implemented in Linux kernel.  The
>> acpi_unload_parent_table() has been unit tested in ACPICA simulation
>> environment.
>>
>> This patch can also help to reduce the source code differences between
>> Linux and ACPICA.
>
> Looks OK to me.
>
> Do you want me to pick it up?

OK by me if you take it.

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

>> Signed-off-by: Lv Zheng <lv.zheng@intel.com>
>> Cc: Octavian Purdila <octavian.purdila@intel.com>
>> Cc: Prarit Bhargava <prarit@sgi.com>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: linux-pci@vger.kernel.org
>> Cc: Tony Luck <tony.luck@intel.com>
>> Cc: Fenghua Yu <fenghua.yu@intel.com>
>> Cc: linux-ia64@vger.kernel.org
>> ---
>>  drivers/acpi/acpica/nsxfobj.c     |   44 -------------------------------------
>>  drivers/acpi/acpica/tbxface.c     |   39 --------------------------------
>>  drivers/pci/hotplug/sgi_hotplug.c |   13 ++++++-----
>>  include/acpi/acpixf.h             |    6 -----
>>  4 files changed, 7 insertions(+), 95 deletions(-)
>>
>> diff --git a/drivers/acpi/acpica/nsxfobj.c b/drivers/acpi/acpica/nsxfobj.c
>> index dae9401..51a8329 100644
>> --- a/drivers/acpi/acpica/nsxfobj.c
>> +++ b/drivers/acpi/acpica/nsxfobj.c
>> @@ -53,50 +53,6 @@ ACPI_MODULE_NAME("nsxfobj")
>>
>>  /*******************************************************************************
>>   *
>> - * FUNCTION:    acpi_get_id
>> - *
>> - * PARAMETERS:  Handle          - Handle of object whose id is desired
>> - *              ret_id          - Where the id will be placed
>> - *
>> - * RETURN:      Status
>> - *
>> - * DESCRIPTION: This routine returns the owner id associated with a handle
>> - *
>> - ******************************************************************************/
>> -acpi_status acpi_get_id(acpi_handle handle, acpi_owner_id * ret_id)
>> -{
>> -     struct acpi_namespace_node *node;
>> -     acpi_status status;
>> -
>> -     /* Parameter Validation */
>> -
>> -     if (!ret_id) {
>> -             return (AE_BAD_PARAMETER);
>> -     }
>> -
>> -     status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
>> -     if (ACPI_FAILURE(status)) {
>> -             return (status);
>> -     }
>> -
>> -     /* Convert and validate the handle */
>> -
>> -     node = acpi_ns_validate_handle(handle);
>> -     if (!node) {
>> -             (void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
>> -             return (AE_BAD_PARAMETER);
>> -     }
>> -
>> -     *ret_id = node->owner_id;
>> -
>> -     status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
>> -     return (status);
>> -}
>> -
>> -ACPI_EXPORT_SYMBOL(acpi_get_id)
>> -
>> -/*******************************************************************************
>> - *
>>   * FUNCTION:    acpi_get_type
>>   *
>>   * PARAMETERS:  handle          - Handle of object whose type is desired
>> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
>> index 6482b0d..0f9dd80 100644
>> --- a/drivers/acpi/acpica/tbxface.c
>> +++ b/drivers/acpi/acpica/tbxface.c
>> @@ -265,45 +265,6 @@ ACPI_EXPORT_SYMBOL(acpi_get_table_header)
>>
>>  /*******************************************************************************
>>   *
>> - * FUNCTION:    acpi_unload_table_id
>> - *
>> - * PARAMETERS:  id            - Owner ID of the table to be removed.
>> - *
>> - * RETURN:      Status
>> - *
>> - * DESCRIPTION: This routine is used to force the unload of a table (by id)
>> - *
>> - ******************************************************************************/
>> -acpi_status acpi_unload_table_id(acpi_owner_id id)
>> -{
>> -     int i;
>> -     acpi_status status = AE_NOT_EXIST;
>> -
>> -     ACPI_FUNCTION_TRACE(acpi_unload_table_id);
>> -
>> -     /* Find table in the global table list */
>> -     for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
>> -             if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
>> -                     continue;
>> -             }
>> -             /*
>> -              * Delete all namespace objects owned by this table. Note that these
>> -              * objects can appear anywhere in the namespace by virtue of the AML
>> -              * "Scope" operator. Thus, we need to track ownership by an ID, not
>> -              * simply a position within the hierarchy
>> -              */
>> -             acpi_tb_delete_namespace_by_owner(i);
>> -             status = acpi_tb_release_owner_id(i);
>> -             acpi_tb_set_table_loaded_flag(i, FALSE);
>> -             break;
>> -     }
>> -     return_ACPI_STATUS(status);
>> -}
>> -
>> -ACPI_EXPORT_SYMBOL(acpi_unload_table_id)
>> -
>> -/*******************************************************************************
>> - *
>>   * FUNCTION:    acpi_get_table_with_size
>>   *
>>   * PARAMETERS:  signature           - ACPI signature of needed table
>> diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
>> index bada2099..c32fb78 100644
>> --- a/drivers/pci/hotplug/sgi_hotplug.c
>> +++ b/drivers/pci/hotplug/sgi_hotplug.c
>> @@ -475,7 +475,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>>       struct slot *slot = bss_hotplug_slot->private;
>>       struct pci_dev *dev, *temp;
>>       int rc;
>> -     acpi_owner_id ssdt_id = 0;
>> +     acpi_handle ssdt_hdl = NULL;
>>
>>       /* Acquire update access to the bus */
>>       mutex_lock(&sn_hotplug_mutex);
>> @@ -522,7 +522,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>>                       if (ACPI_SUCCESS(ret) &&
>>                           (adr>>16) = (slot->device_num + 1)) {
>>                               /* retain the owner id */
>> -                             acpi_get_id(chandle, &ssdt_id);
>> +                             ssdt_hdl = chandle;
>>
>>                               ret = acpi_bus_get_device(chandle,
>>                                                         &device);
>> @@ -547,12 +547,13 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
>>       pci_unlock_rescan_remove();
>>
>>       /* Remove the SSDT for the slot from the ACPI namespace */
>> -     if (SN_ACPI_BASE_SUPPORT() && ssdt_id) {
>> +     if (SN_ACPI_BASE_SUPPORT() && ssdt_hdl) {
>>               acpi_status ret;
>> -             ret = acpi_unload_table_id(ssdt_id);
>> +             ret = acpi_unload_parent_table(ssdt_hdl);
>>               if (ACPI_FAILURE(ret)) {
>> -                     printk(KERN_ERR "%s: acpi_unload_table_id failed (0x%x) for id %d\n",
>> -                            __func__, ret, ssdt_id);
>> +                     acpi_handle_err(ssdt_hdl,
>> +                                     "%s: acpi_unload_parent_table failed (0x%x)\n",
>> +                                     __func__, ret);
>>                       /* try to continue on */
>>               }
>>       }
>> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
>> index 5ba7846..5dd21bc 100644
>> --- a/include/acpi/acpixf.h
>> +++ b/include/acpi/acpixf.h
>> @@ -891,12 +891,6 @@ ACPI_APP_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
>>  ACPI_GLOBAL(u8, acpi_gbl_permanent_mmap);
>>
>>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>> -                         acpi_get_id(acpi_handle object,
>> -                                     acpi_owner_id * out_type))
>> -
>> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_unload_table_id(acpi_owner_id id))
>> -
>> -ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>>                           acpi_get_table_with_size(acpi_string signature,
>>                                                    u32 instance,
>>                                                    struct acpi_table_header
>>
>
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/4] mfd: dln2: add support for ACPI
  2015-01-22 22:09       ` Rafael J. Wysocki
@ 2015-01-23  6:47         ` Octavian Purdila
  2015-01-23 15:19           ` Rafael J. Wysocki
  0 siblings, 1 reply; 25+ messages in thread
From: Octavian Purdila @ 2015-01-23  6:47 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linus Walleij, Lee Jones, Johan Hovold, linux-usb, lkml,
	linux-gpio, Heikki Krogerus, Mika Westerberg, linux-acpi

On Fri, Jan 23, 2015 at 12:09 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Thursday, January 22, 2015 12:13:13 PM Octavian Purdila wrote:
>> On Thu, Jan 22, 2015 at 4:00 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
<snip>
>> The idea here is to set the companion for the MFD sub-devices. Mika's
>> commit "mfd: Add ACPI support" propagates the parent's companion to
>> the MFD sub-devices, so it is sufficient to set the ACPI companion to
>> the USB device.
>
> For the USB device itself you'll then end up with an incomplete binding (you
> can't get back to the USB device from the ACPI object), so no, it isn't
> sufficient.
>
>> Then, the companion will be propagated to the sub-devices after which
>> acpi_bind_one() will be called for the sub-devices from
>> mfd_add_devices (via platform_device_add -> device_add ->
>> platform_notify).
>
> In fact, your use case doesn't seem to cover any of the use cases that
> the Mika's commit addressed.  Namely, your parent device doesn't have
> an ACPI companion to start with and you want your MFD cells to be bound
> to the "DLN2000" thing.  That's why you're setting the ACPI companion
> for the USB device, isn't it?
>
>> It is true that the USB dev will have its ACPI companion set without
>> having acpi_bind_one called but I do not see any harm in that. Even
>> though acpi_unbind_one() is called it will not find the USB dev on the
>> physical node list so no put_device() imbalance is caused.
>
> Well, there are places where it matters.  Some links in sysfs will be missing
> for one example.  Also please see the changelog of commit 52870786ff5d (ACPI:
> Use ACPI companion to match only the first physical device).
>
> Bottom line: You really should be using acpi_bind_one() here and
> acpi_unbind_one() on disconnect if you have to.
>

OK, I understand now.

>> > And no, "Let's come up with a patch that sort of works, throw it at the maintainers
>> > and see what happens" is not an acceptable approach, sorry.
>>
>> This patch is based on your feedback of the previous RFC patch set:
>
> Oh, is it?  I can't recall advising you to use request_firmware() for
> uploading ACPI tables or some other questionable things that the patch is
> doing.
>
> And if it still was an RFC, that wouldn't be a problem, but if you just send
> non-RFC patches out, that means you want people to merge them.  This is bad
> if the patches in question are not in a good enough shape and this one isn't.
>

Yes, this should have been tagged with RFC, sorry about that.

> Now, why is this a bad idea to load ACPI tables from a driver using
> request_firmware()?  Because those tables are not device firmare.  They are
> not firmware that you load into a device to make it work (which then works
> with all instances of the given hardware), they are part of system configuration
> information and have to match what's there in the system.  For instance, if you
> ship your example SSDT with a general-purpose distro, it may just not match the
> hardware configuration of systems that people will try to use it with.
>
> So, while it is sort of OK to look up "DLN2000" and bind the USB device to
> that by hand (although this looks ugly to me), it is not OK to load a random
> custom SSDT if it is missing.
>
> We need to add a generic mechanism for loading custom SSDTs not present in the
> firmware image and maybe even to load them on demand, but that cannot happen in
> an ad-hoc way.  So the entire table loading-unloading code in your patch can go
> away for now and you need to fail probing if the "DLN2000" ACPI object is not
> present.
>

That sounds interesting, I like the idea of a generic mechanism for
loading custom SSDTs. Do you have any initial thoughts/pointers for
starting that?

Thanks for the review and feedback.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/4] mfd: dln2: add support for ACPI
  2015-01-23  6:47         ` Octavian Purdila
@ 2015-01-23 15:19           ` Rafael J. Wysocki
  0 siblings, 0 replies; 25+ messages in thread
From: Rafael J. Wysocki @ 2015-01-23 15:19 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: Linus Walleij, Lee Jones, Johan Hovold, linux-usb, lkml,
	linux-gpio, Heikki Krogerus, Mika Westerberg, linux-acpi

On Friday, January 23, 2015 08:47:58 AM Octavian Purdila wrote:
> On Fri, Jan 23, 2015 at 12:09 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> > On Thursday, January 22, 2015 12:13:13 PM Octavian Purdila wrote:
> >> On Thu, Jan 22, 2015 at 4:00 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> <snip>
> >> The idea here is to set the companion for the MFD sub-devices. Mika's
> >> commit "mfd: Add ACPI support" propagates the parent's companion to
> >> the MFD sub-devices, so it is sufficient to set the ACPI companion to
> >> the USB device.
> >
> > For the USB device itself you'll then end up with an incomplete binding (you
> > can't get back to the USB device from the ACPI object), so no, it isn't
> > sufficient.
> >
> >> Then, the companion will be propagated to the sub-devices after which
> >> acpi_bind_one() will be called for the sub-devices from
> >> mfd_add_devices (via platform_device_add -> device_add ->
> >> platform_notify).
> >
> > In fact, your use case doesn't seem to cover any of the use cases that
> > the Mika's commit addressed.  Namely, your parent device doesn't have
> > an ACPI companion to start with and you want your MFD cells to be bound
> > to the "DLN2000" thing.  That's why you're setting the ACPI companion
> > for the USB device, isn't it?
> >
> >> It is true that the USB dev will have its ACPI companion set without
> >> having acpi_bind_one called but I do not see any harm in that. Even
> >> though acpi_unbind_one() is called it will not find the USB dev on the
> >> physical node list so no put_device() imbalance is caused.
> >
> > Well, there are places where it matters.  Some links in sysfs will be missing
> > for one example.  Also please see the changelog of commit 52870786ff5d (ACPI:
> > Use ACPI companion to match only the first physical device).
> >
> > Bottom line: You really should be using acpi_bind_one() here and
> > acpi_unbind_one() on disconnect if you have to.
> >
> 
> OK, I understand now.
> 
> >> > And no, "Let's come up with a patch that sort of works, throw it at the maintainers
> >> > and see what happens" is not an acceptable approach, sorry.
> >>
> >> This patch is based on your feedback of the previous RFC patch set:
> >
> > Oh, is it?  I can't recall advising you to use request_firmware() for
> > uploading ACPI tables or some other questionable things that the patch is
> > doing.
> >
> > And if it still was an RFC, that wouldn't be a problem, but if you just send
> > non-RFC patches out, that means you want people to merge them.  This is bad
> > if the patches in question are not in a good enough shape and this one isn't.
> >
> 
> Yes, this should have been tagged with RFC, sorry about that.
> 
> > Now, why is this a bad idea to load ACPI tables from a driver using
> > request_firmware()?  Because those tables are not device firmare.  They are
> > not firmware that you load into a device to make it work (which then works
> > with all instances of the given hardware), they are part of system configuration
> > information and have to match what's there in the system.  For instance, if you
> > ship your example SSDT with a general-purpose distro, it may just not match the
> > hardware configuration of systems that people will try to use it with.
> >
> > So, while it is sort of OK to look up "DLN2000" and bind the USB device to
> > that by hand (although this looks ugly to me), it is not OK to load a random
> > custom SSDT if it is missing.
> >
> > We need to add a generic mechanism for loading custom SSDTs not present in the
> > firmware image and maybe even to load them on demand, but that cannot happen in
> > an ad-hoc way.  So the entire table loading-unloading code in your patch can go
> > away for now and you need to fail probing if the "DLN2000" ACPI object is not
> > present.
> >
> 
> That sounds interesting, I like the idea of a generic mechanism for
> loading custom SSDTs. Do you have any initial thoughts/pointers for
> starting that?

Thoughts - yes, pointers - not so much.  I'll get back to you when I'm done
with the e-mail backlog from the last few weeks.

> Thanks for the review and feedback.

No problem.

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
  2015-01-22 22:43       ` Rafael J. Wysocki
  (?)
@ 2015-01-26  1:59         ` Zheng, Lv
  -1 siblings, 0 replies; 25+ messages in thread
From: Zheng, Lv @ 2015-01-26  1:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Wysocki, Rafael J, Brown, Len, Lv Zheng, linux-acpi, Purdila,
	Octavian, Prarit Bhargava, Bjorn Helgaas, linux-pci, Luck, Tony,
	Yu, Fenghua, linux-ia64

Hi, Rafael

> From: Rafael J. Wysocki [mailto:rjw@rjwysocki.net]
> Sent: Friday, January 23, 2015 6:43 AM
> 
> On Thursday, January 22, 2015 02:37:49 PM Lv Zheng wrote:
> > ACPICA has implemented acpi_unload_parent_table() which can exactly replace
> > the acpi_get_id()/acpi_unload_table_id() implemented in Linux kernel.  The
> > acpi_unload_parent_table() has been unit tested in ACPICA simulation
> > environment.
> >
> > This patch can also help to reduce the source code differences between
> > Linux and ACPICA.
> 
> Looks OK to me.
> 
> Do you want me to pick it up?

Though sgi-hotplug.c is the only source file using the deleted functions,
I only cross-compiled an applied kernel with an ia64 tool chain,
it's not functionally tested.
Shall we wait for Octavian's confirmation?

And in linux-next branch, there is a conflict with this old patch.
Shall I rebase it?

Thanks and best regards
-Lv

> 
> > Signed-off-by: Lv Zheng <lv.zheng@intel.com>
> > Cc: Octavian Purdila <octavian.purdila@intel.com>
> > Cc: Prarit Bhargava <prarit@sgi.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org
> > Cc: Tony Luck <tony.luck@intel.com>
> > Cc: Fenghua Yu <fenghua.yu@intel.com>
> > Cc: linux-ia64@vger.kernel.org
> > ---
> >  drivers/acpi/acpica/nsxfobj.c     |   44 -------------------------------------
> >  drivers/acpi/acpica/tbxface.c     |   39 --------------------------------
> >  drivers/pci/hotplug/sgi_hotplug.c |   13 ++++++-----
> >  include/acpi/acpixf.h             |    6 -----
> >  4 files changed, 7 insertions(+), 95 deletions(-)
> >
> > diff --git a/drivers/acpi/acpica/nsxfobj.c b/drivers/acpi/acpica/nsxfobj.c
> > index dae9401..51a8329 100644
> > --- a/drivers/acpi/acpica/nsxfobj.c
> > +++ b/drivers/acpi/acpica/nsxfobj.c
> > @@ -53,50 +53,6 @@ ACPI_MODULE_NAME("nsxfobj")
> >
> >  /*******************************************************************************
> >   *
> > - * FUNCTION:    acpi_get_id
> > - *
> > - * PARAMETERS:  Handle          - Handle of object whose id is desired
> > - *              ret_id          - Where the id will be placed
> > - *
> > - * RETURN:      Status
> > - *
> > - * DESCRIPTION: This routine returns the owner id associated with a handle
> > - *
> > - ******************************************************************************/
> > -acpi_status acpi_get_id(acpi_handle handle, acpi_owner_id * ret_id)
> > -{
> > -	struct acpi_namespace_node *node;
> > -	acpi_status status;
> > -
> > -	/* Parameter Validation */
> > -
> > -	if (!ret_id) {
> > -		return (AE_BAD_PARAMETER);
> > -	}
> > -
> > -	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> > -	if (ACPI_FAILURE(status)) {
> > -		return (status);
> > -	}
> > -
> > -	/* Convert and validate the handle */
> > -
> > -	node = acpi_ns_validate_handle(handle);
> > -	if (!node) {
> > -		(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> > -		return (AE_BAD_PARAMETER);
> > -	}
> > -
> > -	*ret_id = node->owner_id;
> > -
> > -	status = acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> > -	return (status);
> > -}
> > -
> > -ACPI_EXPORT_SYMBOL(acpi_get_id)
> > -
> > -/*******************************************************************************
> > - *
> >   * FUNCTION:    acpi_get_type
> >   *
> >   * PARAMETERS:  handle          - Handle of object whose type is desired
> > diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
> > index 6482b0d..0f9dd80 100644
> > --- a/drivers/acpi/acpica/tbxface.c
> > +++ b/drivers/acpi/acpica/tbxface.c
> > @@ -265,45 +265,6 @@ ACPI_EXPORT_SYMBOL(acpi_get_table_header)
> >
> >  /*******************************************************************************
> >   *
> > - * FUNCTION:    acpi_unload_table_id
> > - *
> > - * PARAMETERS:  id            - Owner ID of the table to be removed.
> > - *
> > - * RETURN:      Status
> > - *
> > - * DESCRIPTION: This routine is used to force the unload of a table (by id)
> > - *
> > - ******************************************************************************/
> > -acpi_status acpi_unload_table_id(acpi_owner_id id)
> > -{
> > -	int i;
> > -	acpi_status status = AE_NOT_EXIST;
> > -
> > -	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
> > -
> > -	/* Find table in the global table list */
> > -	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
> > -		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
> > -			continue;
> > -		}
> > -		/*
> > -		 * Delete all namespace objects owned by this table. Note that these
> > -		 * objects can appear anywhere in the namespace by virtue of the AML
> > -		 * "Scope" operator. Thus, we need to track ownership by an ID, not
> > -		 * simply a position within the hierarchy
> > -		 */
> > -		acpi_tb_delete_namespace_by_owner(i);
> > -		status = acpi_tb_release_owner_id(i);
> > -		acpi_tb_set_table_loaded_flag(i, FALSE);
> > -		break;
> > -	}
> > -	return_ACPI_STATUS(status);
> > -}
> > -
> > -ACPI_EXPORT_SYMBOL(acpi_unload_table_id)
> > -
> > -/*******************************************************************************
> > - *
> >   * FUNCTION:    acpi_get_table_with_size
> >   *
> >   * PARAMETERS:  signature           - ACPI signature of needed table
> > diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
> > index bada2099..c32fb78 100644
> > --- a/drivers/pci/hotplug/sgi_hotplug.c
> > +++ b/drivers/pci/hotplug/sgi_hotplug.c
> > @@ -475,7 +475,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
> >  	struct slot *slot = bss_hotplug_slot->private;
> >  	struct pci_dev *dev, *temp;
> >  	int rc;
> > -	acpi_owner_id ssdt_id = 0;
> > +	acpi_handle ssdt_hdl = NULL;
> >
> >  	/* Acquire update access to the bus */
> >  	mutex_lock(&sn_hotplug_mutex);
> > @@ -522,7 +522,7 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
> >  			if (ACPI_SUCCESS(ret) &&
> >  			    (adr>>16) == (slot->device_num + 1)) {
> >  				/* retain the owner id */
> > -				acpi_get_id(chandle, &ssdt_id);
> > +				ssdt_hdl = chandle;
> >
> >  				ret = acpi_bus_get_device(chandle,
> >  							  &device);
> > @@ -547,12 +547,13 @@ static int disable_slot(struct hotplug_slot *bss_hotplug_slot)
> >  	pci_unlock_rescan_remove();
> >
> >  	/* Remove the SSDT for the slot from the ACPI namespace */
> > -	if (SN_ACPI_BASE_SUPPORT() && ssdt_id) {
> > +	if (SN_ACPI_BASE_SUPPORT() && ssdt_hdl) {
> >  		acpi_status ret;
> > -		ret = acpi_unload_table_id(ssdt_id);
> > +		ret = acpi_unload_parent_table(ssdt_hdl);
> >  		if (ACPI_FAILURE(ret)) {
> > -			printk(KERN_ERR "%s: acpi_unload_table_id failed (0x%x) for id %d\n",
> > -			       __func__, ret, ssdt_id);
> > +			acpi_handle_err(ssdt_hdl,
> > +					"%s: acpi_unload_parent_table failed (0x%x)\n",
> > +					__func__, ret);
> >  			/* try to continue on */
> >  		}
> >  	}
> > diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> > index 5ba7846..5dd21bc 100644
> > --- a/include/acpi/acpixf.h
> > +++ b/include/acpi/acpixf.h
> > @@ -891,12 +891,6 @@ ACPI_APP_DEPENDENT_RETURN_VOID(ACPI_PRINTF_LIKE(1)
> >  ACPI_GLOBAL(u8, acpi_gbl_permanent_mmap);
> >
> >  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> > -			    acpi_get_id(acpi_handle object,
> > -					acpi_owner_id * out_type))
> > -
> > -ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_unload_table_id(acpi_owner_id id))
> > -
> > -ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> >  			    acpi_get_table_with_size(acpi_string signature,
> >  						     u32 instance,
> >  						     struct acpi_table_header
> >
> 
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
@ 2015-01-26  1:59         ` Zheng, Lv
  0 siblings, 0 replies; 25+ messages in thread
From: Zheng, Lv @ 2015-01-26  1:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Wysocki, Rafael J, Brown, Len, Lv Zheng, linux-acpi, Purdila,
	Octavian, Prarit Bhargava, Bjorn Helgaas, linux-pci, Luck, Tony,
	Yu, Fenghua, linux-ia64

SGksIFJhZmFlbA0KDQo+IEZyb206IFJhZmFlbCBKLiBXeXNvY2tpIFttYWlsdG86cmp3QHJqd3lz
b2NraS5uZXRdDQo+IFNlbnQ6IEZyaWRheSwgSmFudWFyeSAyMywgMjAxNSA2OjQzIEFNDQo+IA0K
PiBPbiBUaHVyc2RheSwgSmFudWFyeSAyMiwgMjAxNSAwMjozNzo0OSBQTSBMdiBaaGVuZyB3cm90
ZToNCj4gPiBBQ1BJQ0EgaGFzIGltcGxlbWVudGVkIGFjcGlfdW5sb2FkX3BhcmVudF90YWJsZSgp
IHdoaWNoIGNhbiBleGFjdGx5IHJlcGxhY2UNCj4gPiB0aGUgYWNwaV9nZXRfaWQoKS9hY3BpX3Vu
bG9hZF90YWJsZV9pZCgpIGltcGxlbWVudGVkIGluIExpbnV4IGtlcm5lbC4gIFRoZQ0KPiA+IGFj
cGlfdW5sb2FkX3BhcmVudF90YWJsZSgpIGhhcyBiZWVuIHVuaXQgdGVzdGVkIGluIEFDUElDQSBz
aW11bGF0aW9uDQo+ID4gZW52aXJvbm1lbnQuDQo+ID4NCj4gPiBUaGlzIHBhdGNoIGNhbiBhbHNv
IGhlbHAgdG8gcmVkdWNlIHRoZSBzb3VyY2UgY29kZSBkaWZmZXJlbmNlcyBiZXR3ZWVuDQo+ID4g
TGludXggYW5kIEFDUElDQS4NCj4gDQo+IExvb2tzIE9LIHRvIG1lLg0KPiANCj4gRG8geW91IHdh
bnQgbWUgdG8gcGljayBpdCB1cD8NCg0KVGhvdWdoIHNnaS1ob3RwbHVnLmMgaXMgdGhlIG9ubHkg
c291cmNlIGZpbGUgdXNpbmcgdGhlIGRlbGV0ZWQgZnVuY3Rpb25zLA0KSSBvbmx5IGNyb3NzLWNv
bXBpbGVkIGFuIGFwcGxpZWQga2VybmVsIHdpdGggYW4gaWE2NCB0b29sIGNoYWluLA0KaXQncyBu
b3QgZnVuY3Rpb25hbGx5IHRlc3RlZC4NClNoYWxsIHdlIHdhaXQgZm9yIE9jdGF2aWFuJ3MgY29u
ZmlybWF0aW9uPw0KDQpBbmQgaW4gbGludXgtbmV4dCBicmFuY2gsIHRoZXJlIGlzIGEgY29uZmxp
Y3Qgd2l0aCB0aGlzIG9sZCBwYXRjaC4NClNoYWxsIEkgcmViYXNlIGl0Pw0KDQpUaGFua3MgYW5k
IGJlc3QgcmVnYXJkcw0KLUx2DQoNCj4gDQo+ID4gU2lnbmVkLW9mZi1ieTogTHYgWmhlbmcgPGx2
LnpoZW5nQGludGVsLmNvbT4NCj4gPiBDYzogT2N0YXZpYW4gUHVyZGlsYSA8b2N0YXZpYW4ucHVy
ZGlsYUBpbnRlbC5jb20+DQo+ID4gQ2M6IFByYXJpdCBCaGFyZ2F2YSA8cHJhcml0QHNnaS5jb20+
DQo+ID4gQ2M6IEJqb3JuIEhlbGdhYXMgPGJoZWxnYWFzQGdvb2dsZS5jb20+DQo+ID4gQ2M6IGxp
bnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmcNCj4gPiBDYzogVG9ueSBMdWNrIDx0b255Lmx1Y2tAaW50
ZWwuY29tPg0KPiA+IENjOiBGZW5naHVhIFl1IDxmZW5naHVhLnl1QGludGVsLmNvbT4NCj4gPiBD
YzogbGludXgtaWE2NEB2Z2VyLmtlcm5lbC5vcmcNCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9hY3Bp
L2FjcGljYS9uc3hmb2JqLmMgICAgIHwgICA0NCAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tDQo+ID4gIGRyaXZlcnMvYWNwaS9hY3BpY2EvdGJ4ZmFjZS5jICAgICB8ICAgMzkg
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gPiAgZHJpdmVycy9wY2kvaG90cGx1
Zy9zZ2lfaG90cGx1Zy5jIHwgICAxMyArKysrKystLS0tLQ0KPiA+ICBpbmNsdWRlL2FjcGkvYWNw
aXhmLmggICAgICAgICAgICAgfCAgICA2IC0tLS0tDQo+ID4gIDQgZmlsZXMgY2hhbmdlZCwgNyBp
bnNlcnRpb25zKCspLCA5NSBkZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9kcml2
ZXJzL2FjcGkvYWNwaWNhL25zeGZvYmouYyBiL2RyaXZlcnMvYWNwaS9hY3BpY2EvbnN4Zm9iai5j
DQo+ID4gaW5kZXggZGFlOTQwMS4uNTFhODMyOSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2Fj
cGkvYWNwaWNhL25zeGZvYmouYw0KPiA+ICsrKyBiL2RyaXZlcnMvYWNwaS9hY3BpY2EvbnN4Zm9i
ai5jDQo+ID4gQEAgLTUzLDUwICs1Myw2IEBAIEFDUElfTU9EVUxFX05BTUUoIm5zeGZvYmoiKQ0K
PiA+DQo+ID4gIC8qKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqDQo+ID4gICAqDQo+ID4gLSAqIEZVTkNU
SU9OOiAgICBhY3BpX2dldF9pZA0KPiA+IC0gKg0KPiA+IC0gKiBQQVJBTUVURVJTOiAgSGFuZGxl
ICAgICAgICAgIC0gSGFuZGxlIG9mIG9iamVjdCB3aG9zZSBpZCBpcyBkZXNpcmVkDQo+ID4gLSAq
ICAgICAgICAgICAgICByZXRfaWQgICAgICAgICAgLSBXaGVyZSB0aGUgaWQgd2lsbCBiZSBwbGFj
ZWQNCj4gPiAtICoNCj4gPiAtICogUkVUVVJOOiAgICAgIFN0YXR1cw0KPiA+IC0gKg0KPiA+IC0g
KiBERVNDUklQVElPTjogVGhpcyByb3V0aW5lIHJldHVybnMgdGhlIG93bmVyIGlkIGFzc29jaWF0
ZWQgd2l0aCBhIGhhbmRsZQ0KPiA+IC0gKg0KPiA+IC0gKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqLw0K
PiA+IC1hY3BpX3N0YXR1cyBhY3BpX2dldF9pZChhY3BpX2hhbmRsZSBoYW5kbGUsIGFjcGlfb3du
ZXJfaWQgKiByZXRfaWQpDQo+ID4gLXsNCj4gPiAtCXN0cnVjdCBhY3BpX25hbWVzcGFjZV9ub2Rl
ICpub2RlOw0KPiA+IC0JYWNwaV9zdGF0dXMgc3RhdHVzOw0KPiA+IC0NCj4gPiAtCS8qIFBhcmFt
ZXRlciBWYWxpZGF0aW9uICovDQo+ID4gLQ0KPiA+IC0JaWYgKCFyZXRfaWQpIHsNCj4gPiAtCQly
ZXR1cm4gKEFFX0JBRF9QQVJBTUVURVIpOw0KPiA+IC0JfQ0KPiA+IC0NCj4gPiAtCXN0YXR1cyA9
IGFjcGlfdXRfYWNxdWlyZV9tdXRleChBQ1BJX01UWF9OQU1FU1BBQ0UpOw0KPiA+IC0JaWYgKEFD
UElfRkFJTFVSRShzdGF0dXMpKSB7DQo+ID4gLQkJcmV0dXJuIChzdGF0dXMpOw0KPiA+IC0JfQ0K
PiA+IC0NCj4gPiAtCS8qIENvbnZlcnQgYW5kIHZhbGlkYXRlIHRoZSBoYW5kbGUgKi8NCj4gPiAt
DQo+ID4gLQlub2RlID0gYWNwaV9uc192YWxpZGF0ZV9oYW5kbGUoaGFuZGxlKTsNCj4gPiAtCWlm
ICghbm9kZSkgew0KPiA+IC0JCSh2b2lkKWFjcGlfdXRfcmVsZWFzZV9tdXRleChBQ1BJX01UWF9O
QU1FU1BBQ0UpOw0KPiA+IC0JCXJldHVybiAoQUVfQkFEX1BBUkFNRVRFUik7DQo+ID4gLQl9DQo+
ID4gLQ0KPiA+IC0JKnJldF9pZCA9IG5vZGUtPm93bmVyX2lkOw0KPiA+IC0NCj4gPiAtCXN0YXR1
cyA9IGFjcGlfdXRfcmVsZWFzZV9tdXRleChBQ1BJX01UWF9OQU1FU1BBQ0UpOw0KPiA+IC0JcmV0
dXJuIChzdGF0dXMpOw0KPiA+IC19DQo+ID4gLQ0KPiA+IC1BQ1BJX0VYUE9SVF9TWU1CT0woYWNw
aV9nZXRfaWQpDQo+ID4gLQ0KPiA+IC0vKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKg0KPiA+IC0gKg0K
PiA+ICAgKiBGVU5DVElPTjogICAgYWNwaV9nZXRfdHlwZQ0KPiA+ICAgKg0KPiA+ICAgKiBQQVJB
TUVURVJTOiAgaGFuZGxlICAgICAgICAgIC0gSGFuZGxlIG9mIG9iamVjdCB3aG9zZSB0eXBlIGlz
IGRlc2lyZWQNCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9hY3BpL2FjcGljYS90YnhmYWNlLmMg
Yi9kcml2ZXJzL2FjcGkvYWNwaWNhL3RieGZhY2UuYw0KPiA+IGluZGV4IDY0ODJiMGQuLjBmOWRk
ODAgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9hY3BpL2FjcGljYS90YnhmYWNlLmMNCj4gPiAr
KysgYi9kcml2ZXJzL2FjcGkvYWNwaWNhL3RieGZhY2UuYw0KPiA+IEBAIC0yNjUsNDUgKzI2NSw2
IEBAIEFDUElfRVhQT1JUX1NZTUJPTChhY3BpX2dldF90YWJsZV9oZWFkZXIpDQo+ID4NCj4gPiAg
LyoqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioNCj4gPiAgICoNCj4gPiAtICogRlVOQ1RJT046ICAgIGFj
cGlfdW5sb2FkX3RhYmxlX2lkDQo+ID4gLSAqDQo+ID4gLSAqIFBBUkFNRVRFUlM6ICBpZCAgICAg
ICAgICAgIC0gT3duZXIgSUQgb2YgdGhlIHRhYmxlIHRvIGJlIHJlbW92ZWQuDQo+ID4gLSAqDQo+
ID4gLSAqIFJFVFVSTjogICAgICBTdGF0dXMNCj4gPiAtICoNCj4gPiAtICogREVTQ1JJUFRJT046
IFRoaXMgcm91dGluZSBpcyB1c2VkIHRvIGZvcmNlIHRoZSB1bmxvYWQgb2YgYSB0YWJsZSAoYnkg
aWQpDQo+ID4gLSAqDQo+ID4gLSAqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKiovDQo+ID4gLWFjcGlfc3Rh
dHVzIGFjcGlfdW5sb2FkX3RhYmxlX2lkKGFjcGlfb3duZXJfaWQgaWQpDQo+ID4gLXsNCj4gPiAt
CWludCBpOw0KPiA+IC0JYWNwaV9zdGF0dXMgc3RhdHVzID0gQUVfTk9UX0VYSVNUOw0KPiA+IC0N
Cj4gPiAtCUFDUElfRlVOQ1RJT05fVFJBQ0UoYWNwaV91bmxvYWRfdGFibGVfaWQpOw0KPiA+IC0N
Cj4gPiAtCS8qIEZpbmQgdGFibGUgaW4gdGhlIGdsb2JhbCB0YWJsZSBsaXN0ICovDQo+ID4gLQlm
b3IgKGkgPSAwOyBpIDwgYWNwaV9nYmxfcm9vdF90YWJsZV9saXN0LmN1cnJlbnRfdGFibGVfY291
bnQ7ICsraSkgew0KPiA+IC0JCWlmIChpZCAhPSBhY3BpX2dibF9yb290X3RhYmxlX2xpc3QudGFi
bGVzW2ldLm93bmVyX2lkKSB7DQo+ID4gLQkJCWNvbnRpbnVlOw0KPiA+IC0JCX0NCj4gPiAtCQkv
Kg0KPiA+IC0JCSAqIERlbGV0ZSBhbGwgbmFtZXNwYWNlIG9iamVjdHMgb3duZWQgYnkgdGhpcyB0
YWJsZS4gTm90ZSB0aGF0IHRoZXNlDQo+ID4gLQkJICogb2JqZWN0cyBjYW4gYXBwZWFyIGFueXdo
ZXJlIGluIHRoZSBuYW1lc3BhY2UgYnkgdmlydHVlIG9mIHRoZSBBTUwNCj4gPiAtCQkgKiAiU2Nv
cGUiIG9wZXJhdG9yLiBUaHVzLCB3ZSBuZWVkIHRvIHRyYWNrIG93bmVyc2hpcCBieSBhbiBJRCwg
bm90DQo+ID4gLQkJICogc2ltcGx5IGEgcG9zaXRpb24gd2l0aGluIHRoZSBoaWVyYXJjaHkNCj4g
PiAtCQkgKi8NCj4gPiAtCQlhY3BpX3RiX2RlbGV0ZV9uYW1lc3BhY2VfYnlfb3duZXIoaSk7DQo+
ID4gLQkJc3RhdHVzID0gYWNwaV90Yl9yZWxlYXNlX293bmVyX2lkKGkpOw0KPiA+IC0JCWFjcGlf
dGJfc2V0X3RhYmxlX2xvYWRlZF9mbGFnKGksIEZBTFNFKTsNCj4gPiAtCQlicmVhazsNCj4gPiAt
CX0NCj4gPiAtCXJldHVybl9BQ1BJX1NUQVRVUyhzdGF0dXMpOw0KPiA+IC19DQo+ID4gLQ0KPiA+
IC1BQ1BJX0VYUE9SVF9TWU1CT0woYWNwaV91bmxvYWRfdGFibGVfaWQpDQo+ID4gLQ0KPiA+IC0v
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKg0KPiA+IC0gKg0KPiA+ICAgKiBGVU5DVElPTjogICAgYWNw
aV9nZXRfdGFibGVfd2l0aF9zaXplDQo+ID4gICAqDQo+ID4gICAqIFBBUkFNRVRFUlM6ICBzaWdu
YXR1cmUgICAgICAgICAgIC0gQUNQSSBzaWduYXR1cmUgb2YgbmVlZGVkIHRhYmxlDQo+ID4gZGlm
ZiAtLWdpdCBhL2RyaXZlcnMvcGNpL2hvdHBsdWcvc2dpX2hvdHBsdWcuYyBiL2RyaXZlcnMvcGNp
L2hvdHBsdWcvc2dpX2hvdHBsdWcuYw0KPiA+IGluZGV4IGJhZGEyMDk5Li5jMzJmYjc4IDEwMDY0
NA0KPiA+IC0tLSBhL2RyaXZlcnMvcGNpL2hvdHBsdWcvc2dpX2hvdHBsdWcuYw0KPiA+ICsrKyBi
L2RyaXZlcnMvcGNpL2hvdHBsdWcvc2dpX2hvdHBsdWcuYw0KPiA+IEBAIC00NzUsNyArNDc1LDcg
QEAgc3RhdGljIGludCBkaXNhYmxlX3Nsb3Qoc3RydWN0IGhvdHBsdWdfc2xvdCAqYnNzX2hvdHBs
dWdfc2xvdCkNCj4gPiAgCXN0cnVjdCBzbG90ICpzbG90ID0gYnNzX2hvdHBsdWdfc2xvdC0+cHJp
dmF0ZTsNCj4gPiAgCXN0cnVjdCBwY2lfZGV2ICpkZXYsICp0ZW1wOw0KPiA+ICAJaW50IHJjOw0K
PiA+IC0JYWNwaV9vd25lcl9pZCBzc2R0X2lkID0gMDsNCj4gPiArCWFjcGlfaGFuZGxlIHNzZHRf
aGRsID0gTlVMTDsNCj4gPg0KPiA+ICAJLyogQWNxdWlyZSB1cGRhdGUgYWNjZXNzIHRvIHRoZSBi
dXMgKi8NCj4gPiAgCW11dGV4X2xvY2soJnNuX2hvdHBsdWdfbXV0ZXgpOw0KPiA+IEBAIC01MjIs
NyArNTIyLDcgQEAgc3RhdGljIGludCBkaXNhYmxlX3Nsb3Qoc3RydWN0IGhvdHBsdWdfc2xvdCAq
YnNzX2hvdHBsdWdfc2xvdCkNCj4gPiAgCQkJaWYgKEFDUElfU1VDQ0VTUyhyZXQpICYmDQo+ID4g
IAkJCSAgICAoYWRyPj4xNikgPT0gKHNsb3QtPmRldmljZV9udW0gKyAxKSkgew0KPiA+ICAJCQkJ
LyogcmV0YWluIHRoZSBvd25lciBpZCAqLw0KPiA+IC0JCQkJYWNwaV9nZXRfaWQoY2hhbmRsZSwg
JnNzZHRfaWQpOw0KPiA+ICsJCQkJc3NkdF9oZGwgPSBjaGFuZGxlOw0KPiA+DQo+ID4gIAkJCQly
ZXQgPSBhY3BpX2J1c19nZXRfZGV2aWNlKGNoYW5kbGUsDQo+ID4gIAkJCQkJCQkgICZkZXZpY2Up
Ow0KPiA+IEBAIC01NDcsMTIgKzU0NywxMyBAQCBzdGF0aWMgaW50IGRpc2FibGVfc2xvdChzdHJ1
Y3QgaG90cGx1Z19zbG90ICpic3NfaG90cGx1Z19zbG90KQ0KPiA+ICAJcGNpX3VubG9ja19yZXNj
YW5fcmVtb3ZlKCk7DQo+ID4NCj4gPiAgCS8qIFJlbW92ZSB0aGUgU1NEVCBmb3IgdGhlIHNsb3Qg
ZnJvbSB0aGUgQUNQSSBuYW1lc3BhY2UgKi8NCj4gPiAtCWlmIChTTl9BQ1BJX0JBU0VfU1VQUE9S
VCgpICYmIHNzZHRfaWQpIHsNCj4gPiArCWlmIChTTl9BQ1BJX0JBU0VfU1VQUE9SVCgpICYmIHNz
ZHRfaGRsKSB7DQo+ID4gIAkJYWNwaV9zdGF0dXMgcmV0Ow0KPiA+IC0JCXJldCA9IGFjcGlfdW5s
b2FkX3RhYmxlX2lkKHNzZHRfaWQpOw0KPiA+ICsJCXJldCA9IGFjcGlfdW5sb2FkX3BhcmVudF90
YWJsZShzc2R0X2hkbCk7DQo+ID4gIAkJaWYgKEFDUElfRkFJTFVSRShyZXQpKSB7DQo+ID4gLQkJ
CXByaW50ayhLRVJOX0VSUiAiJXM6IGFjcGlfdW5sb2FkX3RhYmxlX2lkIGZhaWxlZCAoMHgleCkg
Zm9yIGlkICVkXG4iLA0KPiA+IC0JCQkgICAgICAgX19mdW5jX18sIHJldCwgc3NkdF9pZCk7DQo+
ID4gKwkJCWFjcGlfaGFuZGxlX2Vycihzc2R0X2hkbCwNCj4gPiArCQkJCQkiJXM6IGFjcGlfdW5s
b2FkX3BhcmVudF90YWJsZSBmYWlsZWQgKDB4JXgpXG4iLA0KPiA+ICsJCQkJCV9fZnVuY19fLCBy
ZXQpOw0KPiA+ICAJCQkvKiB0cnkgdG8gY29udGludWUgb24gKi8NCj4gPiAgCQl9DQo+ID4gIAl9
DQo+ID4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvYWNwaS9hY3BpeGYuaCBiL2luY2x1ZGUvYWNwaS9h
Y3BpeGYuaA0KPiA+IGluZGV4IDViYTc4NDYuLjVkZDIxYmMgMTAwNjQ0DQo+ID4gLS0tIGEvaW5j
bHVkZS9hY3BpL2FjcGl4Zi5oDQo+ID4gKysrIGIvaW5jbHVkZS9hY3BpL2FjcGl4Zi5oDQo+ID4g
QEAgLTg5MSwxMiArODkxLDYgQEAgQUNQSV9BUFBfREVQRU5ERU5UX1JFVFVSTl9WT0lEKEFDUElf
UFJJTlRGX0xJS0UoMSkNCj4gPiAgQUNQSV9HTE9CQUwodTgsIGFjcGlfZ2JsX3Blcm1hbmVudF9t
bWFwKTsNCj4gPg0KPiA+ICBBQ1BJX0VYVEVSTkFMX1JFVFVSTl9TVEFUVVMoYWNwaV9zdGF0dXMN
Cj4gPiAtCQkJICAgIGFjcGlfZ2V0X2lkKGFjcGlfaGFuZGxlIG9iamVjdCwNCj4gPiAtCQkJCQlh
Y3BpX293bmVyX2lkICogb3V0X3R5cGUpKQ0KPiA+IC0NCj4gPiAtQUNQSV9FWFRFUk5BTF9SRVRV
Uk5fU1RBVFVTKGFjcGlfc3RhdHVzIGFjcGlfdW5sb2FkX3RhYmxlX2lkKGFjcGlfb3duZXJfaWQg
aWQpKQ0KPiA+IC0NCj4gPiAtQUNQSV9FWFRFUk5BTF9SRVRVUk5fU1RBVFVTKGFjcGlfc3RhdHVz
DQo+ID4gIAkJCSAgICBhY3BpX2dldF90YWJsZV93aXRoX3NpemUoYWNwaV9zdHJpbmcgc2lnbmF0
dXJlLA0KPiA+ICAJCQkJCQkgICAgIHUzMiBpbnN0YW5jZSwNCj4gPiAgCQkJCQkJICAgICBzdHJ1
Y3QgYWNwaV90YWJsZV9oZWFkZXINCj4gPg0KPiANCj4gLS0NCj4gSSBzcGVhayBvbmx5IGZvciBt
eXNlbGYuDQo+IFJhZmFlbCBKLiBXeXNvY2tpLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5
IENlbnRlci4NCg==

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel.
@ 2015-01-26  1:59         ` Zheng, Lv
  0 siblings, 0 replies; 25+ messages in thread
From: Zheng, Lv @ 2015-01-26  1:59 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Wysocki, Rafael J, Brown, Len, Lv Zheng, linux-acpi, Purdila,
	Octavian, Prarit Bhargava, Bjorn Helgaas, linux-pci, Luck, Tony,
	Yu, Fenghua, linux-ia64

SGksIFJhZmFlbA0KDQo+IEZyb206IFJhZmFlbCBKLiBXeXNvY2tpIFttYWlsdG86cmp3QHJqd3lz
b2NraS5uZXRdDQo+IFNlbnQ6IEZyaWRheSwgSmFudWFyeSAyMywgMjAxNSA2OjQzIEFNDQo+IA0K
PiBPbiBUaHVyc2RheSwgSmFudWFyeSAyMiwgMjAxNSAwMjozNzo0OSBQTSBMdiBaaGVuZyB3cm90
ZToNCj4gPiBBQ1BJQ0EgaGFzIGltcGxlbWVudGVkIGFjcGlfdW5sb2FkX3BhcmVudF90YWJsZSgp
IHdoaWNoIGNhbiBleGFjdGx5IHJlcGxhY2UNCj4gPiB0aGUgYWNwaV9nZXRfaWQoKS9hY3BpX3Vu
bG9hZF90YWJsZV9pZCgpIGltcGxlbWVudGVkIGluIExpbnV4IGtlcm5lbC4gIFRoZQ0KPiA+IGFj
cGlfdW5sb2FkX3BhcmVudF90YWJsZSgpIGhhcyBiZWVuIHVuaXQgdGVzdGVkIGluIEFDUElDQSBz
aW11bGF0aW9uDQo+ID4gZW52aXJvbm1lbnQuDQo+ID4NCj4gPiBUaGlzIHBhdGNoIGNhbiBhbHNv
IGhlbHAgdG8gcmVkdWNlIHRoZSBzb3VyY2UgY29kZSBkaWZmZXJlbmNlcyBiZXR3ZWVuDQo+ID4g
TGludXggYW5kIEFDUElDQS4NCj4gDQo+IExvb2tzIE9LIHRvIG1lLg0KPiANCj4gRG8geW91IHdh
bnQgbWUgdG8gcGljayBpdCB1cD8NCg0KVGhvdWdoIHNnaS1ob3RwbHVnLmMgaXMgdGhlIG9ubHkg
c291cmNlIGZpbGUgdXNpbmcgdGhlIGRlbGV0ZWQgZnVuY3Rpb25zLA0KSSBvbmx5IGNyb3NzLWNv
bXBpbGVkIGFuIGFwcGxpZWQga2VybmVsIHdpdGggYW4gaWE2NCB0b29sIGNoYWluLA0KaXQncyBu
b3QgZnVuY3Rpb25hbGx5IHRlc3RlZC4NClNoYWxsIHdlIHdhaXQgZm9yIE9jdGF2aWFuJ3MgY29u
ZmlybWF0aW9uPw0KDQpBbmQgaW4gbGludXgtbmV4dCBicmFuY2gsIHRoZXJlIGlzIGEgY29uZmxp
Y3Qgd2l0aCB0aGlzIG9sZCBwYXRjaC4NClNoYWxsIEkgcmViYXNlIGl0Pw0KDQpUaGFua3MgYW5k
IGJlc3QgcmVnYXJkcw0KLUx2DQoNCj4gDQo+ID4gU2lnbmVkLW9mZi1ieTogTHYgWmhlbmcgPGx2
LnpoZW5nQGludGVsLmNvbT4NCj4gPiBDYzogT2N0YXZpYW4gUHVyZGlsYSA8b2N0YXZpYW4ucHVy
ZGlsYUBpbnRlbC5jb20+DQo+ID4gQ2M6IFByYXJpdCBCaGFyZ2F2YSA8cHJhcml0QHNnaS5jb20+
DQo+ID4gQ2M6IEJqb3JuIEhlbGdhYXMgPGJoZWxnYWFzQGdvb2dsZS5jb20+DQo+ID4gQ2M6IGxp
bnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmcNCj4gPiBDYzogVG9ueSBMdWNrIDx0b255Lmx1Y2tAaW50
ZWwuY29tPg0KPiA+IENjOiBGZW5naHVhIFl1IDxmZW5naHVhLnl1QGludGVsLmNvbT4NCj4gPiBD
YzogbGludXgtaWE2NEB2Z2VyLmtlcm5lbC5vcmcNCj4gPiAtLS0NCj4gPiAgZHJpdmVycy9hY3Bp
L2FjcGljYS9uc3hmb2JqLmMgICAgIHwgICA0NCAtLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tDQo+ID4gIGRyaXZlcnMvYWNwaS9hY3BpY2EvdGJ4ZmFjZS5jICAgICB8ICAgMzkg
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gPiAgZHJpdmVycy9wY2kvaG90cGx1
Zy9zZ2lfaG90cGx1Zy5jIHwgICAxMyArKysrKystLS0tLQ0KPiA+ICBpbmNsdWRlL2FjcGkvYWNw
aXhmLmggICAgICAgICAgICAgfCAgICA2IC0tLS0tDQo+ID4gIDQgZmlsZXMgY2hhbmdlZCwgNyBp
bnNlcnRpb25zKCspLCA5NSBkZWxldGlvbnMoLSkNCj4gPg0KPiA+IGRpZmYgLS1naXQgYS9kcml2
ZXJzL2FjcGkvYWNwaWNhL25zeGZvYmouYyBiL2RyaXZlcnMvYWNwaS9hY3BpY2EvbnN4Zm9iai5j
DQo+ID4gaW5kZXggZGFlOTQwMS4uNTFhODMyOSAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2Fj
cGkvYWNwaWNhL25zeGZvYmouYw0KPiA+ICsrKyBiL2RyaXZlcnMvYWNwaS9hY3BpY2EvbnN4Zm9i
ai5jDQo+ID4gQEAgLTUzLDUwICs1Myw2IEBAIEFDUElfTU9EVUxFX05BTUUoIm5zeGZvYmoiKQ0K
PiA+DQo+ID4gIC8qKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqDQo+ID4gICAqDQo+ID4gLSAqIEZVTkNU
SU9OOiAgICBhY3BpX2dldF9pZA0KPiA+IC0gKg0KPiA+IC0gKiBQQVJBTUVURVJTOiAgSGFuZGxl
ICAgICAgICAgIC0gSGFuZGxlIG9mIG9iamVjdCB3aG9zZSBpZCBpcyBkZXNpcmVkDQo+ID4gLSAq
ICAgICAgICAgICAgICByZXRfaWQgICAgICAgICAgLSBXaGVyZSB0aGUgaWQgd2lsbCBiZSBwbGFj
ZWQNCj4gPiAtICoNCj4gPiAtICogUkVUVVJOOiAgICAgIFN0YXR1cw0KPiA+IC0gKg0KPiA+IC0g
KiBERVNDUklQVElPTjogVGhpcyByb3V0aW5lIHJldHVybnMgdGhlIG93bmVyIGlkIGFzc29jaWF0
ZWQgd2l0aCBhIGhhbmRsZQ0KPiA+IC0gKg0KPiA+IC0gKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqLw0K
PiA+IC1hY3BpX3N0YXR1cyBhY3BpX2dldF9pZChhY3BpX2hhbmRsZSBoYW5kbGUsIGFjcGlfb3du
ZXJfaWQgKiByZXRfaWQpDQo+ID4gLXsNCj4gPiAtCXN0cnVjdCBhY3BpX25hbWVzcGFjZV9ub2Rl
ICpub2RlOw0KPiA+IC0JYWNwaV9zdGF0dXMgc3RhdHVzOw0KPiA+IC0NCj4gPiAtCS8qIFBhcmFt
ZXRlciBWYWxpZGF0aW9uICovDQo+ID4gLQ0KPiA+IC0JaWYgKCFyZXRfaWQpIHsNCj4gPiAtCQly
ZXR1cm4gKEFFX0JBRF9QQVJBTUVURVIpOw0KPiA+IC0JfQ0KPiA+IC0NCj4gPiAtCXN0YXR1cyA9
IGFjcGlfdXRfYWNxdWlyZV9tdXRleChBQ1BJX01UWF9OQU1FU1BBQ0UpOw0KPiA+IC0JaWYgKEFD
UElfRkFJTFVSRShzdGF0dXMpKSB7DQo+ID4gLQkJcmV0dXJuIChzdGF0dXMpOw0KPiA+IC0JfQ0K
PiA+IC0NCj4gPiAtCS8qIENvbnZlcnQgYW5kIHZhbGlkYXRlIHRoZSBoYW5kbGUgKi8NCj4gPiAt
DQo+ID4gLQlub2RlID0gYWNwaV9uc192YWxpZGF0ZV9oYW5kbGUoaGFuZGxlKTsNCj4gPiAtCWlm
ICghbm9kZSkgew0KPiA+IC0JCSh2b2lkKWFjcGlfdXRfcmVsZWFzZV9tdXRleChBQ1BJX01UWF9O
QU1FU1BBQ0UpOw0KPiA+IC0JCXJldHVybiAoQUVfQkFEX1BBUkFNRVRFUik7DQo+ID4gLQl9DQo+
ID4gLQ0KPiA+IC0JKnJldF9pZCA9IG5vZGUtPm93bmVyX2lkOw0KPiA+IC0NCj4gPiAtCXN0YXR1
cyA9IGFjcGlfdXRfcmVsZWFzZV9tdXRleChBQ1BJX01UWF9OQU1FU1BBQ0UpOw0KPiA+IC0JcmV0
dXJuIChzdGF0dXMpOw0KPiA+IC19DQo+ID4gLQ0KPiA+IC1BQ1BJX0VYUE9SVF9TWU1CT0woYWNw
aV9nZXRfaWQpDQo+ID4gLQ0KPiA+IC0vKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKg0KPiA+IC0gKg0K
PiA+ICAgKiBGVU5DVElPTjogICAgYWNwaV9nZXRfdHlwZQ0KPiA+ICAgKg0KPiA+ICAgKiBQQVJB
TUVURVJTOiAgaGFuZGxlICAgICAgICAgIC0gSGFuZGxlIG9mIG9iamVjdCB3aG9zZSB0eXBlIGlz
IGRlc2lyZWQNCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9hY3BpL2FjcGljYS90YnhmYWNlLmMg
Yi9kcml2ZXJzL2FjcGkvYWNwaWNhL3RieGZhY2UuYw0KPiA+IGluZGV4IDY0ODJiMGQuLjBmOWRk
ODAgMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9hY3BpL2FjcGljYS90YnhmYWNlLmMNCj4gPiAr
KysgYi9kcml2ZXJzL2FjcGkvYWNwaWNhL3RieGZhY2UuYw0KPiA+IEBAIC0yNjUsNDUgKzI2NSw2
IEBAIEFDUElfRVhQT1JUX1NZTUJPTChhY3BpX2dldF90YWJsZV9oZWFkZXIpDQo+ID4NCj4gPiAg
LyoqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioNCj4gPiAgICoNCj4gPiAtICogRlVOQ1RJT046ICAgIGFj
cGlfdW5sb2FkX3RhYmxlX2lkDQo+ID4gLSAqDQo+ID4gLSAqIFBBUkFNRVRFUlM6ICBpZCAgICAg
ICAgICAgIC0gT3duZXIgSUQgb2YgdGhlIHRhYmxlIHRvIGJlIHJlbW92ZWQuDQo+ID4gLSAqDQo+
ID4gLSAqIFJFVFVSTjogICAgICBTdGF0dXMNCj4gPiAtICoNCj4gPiAtICogREVTQ1JJUFRJT046
IFRoaXMgcm91dGluZSBpcyB1c2VkIHRvIGZvcmNlIHRoZSB1bmxvYWQgb2YgYSB0YWJsZSAoYnkg
aWQpDQo+ID4gLSAqDQo+ID4gLSAqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKiovDQo+ID4gLWFjcGlfc3Rh
dHVzIGFjcGlfdW5sb2FkX3RhYmxlX2lkKGFjcGlfb3duZXJfaWQgaWQpDQo+ID4gLXsNCj4gPiAt
CWludCBpOw0KPiA+IC0JYWNwaV9zdGF0dXMgc3RhdHVzID0gQUVfTk9UX0VYSVNUOw0KPiA+IC0N
Cj4gPiAtCUFDUElfRlVOQ1RJT05fVFJBQ0UoYWNwaV91bmxvYWRfdGFibGVfaWQpOw0KPiA+IC0N
Cj4gPiAtCS8qIEZpbmQgdGFibGUgaW4gdGhlIGdsb2JhbCB0YWJsZSBsaXN0ICovDQo+ID4gLQlm
b3IgKGkgPSAwOyBpIDwgYWNwaV9nYmxfcm9vdF90YWJsZV9saXN0LmN1cnJlbnRfdGFibGVfY291
bnQ7ICsraSkgew0KPiA+IC0JCWlmIChpZCAhPSBhY3BpX2dibF9yb290X3RhYmxlX2xpc3QudGFi
bGVzW2ldLm93bmVyX2lkKSB7DQo+ID4gLQkJCWNvbnRpbnVlOw0KPiA+IC0JCX0NCj4gPiAtCQkv
Kg0KPiA+IC0JCSAqIERlbGV0ZSBhbGwgbmFtZXNwYWNlIG9iamVjdHMgb3duZWQgYnkgdGhpcyB0
YWJsZS4gTm90ZSB0aGF0IHRoZXNlDQo+ID4gLQkJICogb2JqZWN0cyBjYW4gYXBwZWFyIGFueXdo
ZXJlIGluIHRoZSBuYW1lc3BhY2UgYnkgdmlydHVlIG9mIHRoZSBBTUwNCj4gPiAtCQkgKiAiU2Nv
cGUiIG9wZXJhdG9yLiBUaHVzLCB3ZSBuZWVkIHRvIHRyYWNrIG93bmVyc2hpcCBieSBhbiBJRCwg
bm90DQo+ID4gLQkJICogc2ltcGx5IGEgcG9zaXRpb24gd2l0aGluIHRoZSBoaWVyYXJjaHkNCj4g
PiAtCQkgKi8NCj4gPiAtCQlhY3BpX3RiX2RlbGV0ZV9uYW1lc3BhY2VfYnlfb3duZXIoaSk7DQo+
ID4gLQkJc3RhdHVzID0gYWNwaV90Yl9yZWxlYXNlX293bmVyX2lkKGkpOw0KPiA+IC0JCWFjcGlf
dGJfc2V0X3RhYmxlX2xvYWRlZF9mbGFnKGksIEZBTFNFKTsNCj4gPiAtCQlicmVhazsNCj4gPiAt
CX0NCj4gPiAtCXJldHVybl9BQ1BJX1NUQVRVUyhzdGF0dXMpOw0KPiA+IC19DQo+ID4gLQ0KPiA+
IC1BQ1BJX0VYUE9SVF9TWU1CT0woYWNwaV91bmxvYWRfdGFibGVfaWQpDQo+ID4gLQ0KPiA+IC0v
KioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioqKioq
KioqKioqKioqKioqKioqKioqKioqKg0KPiA+IC0gKg0KPiA+ICAgKiBGVU5DVElPTjogICAgYWNw
aV9nZXRfdGFibGVfd2l0aF9zaXplDQo+ID4gICAqDQo+ID4gICAqIFBBUkFNRVRFUlM6ICBzaWdu
YXR1cmUgICAgICAgICAgIC0gQUNQSSBzaWduYXR1cmUgb2YgbmVlZGVkIHRhYmxlDQo+ID4gZGlm
ZiAtLWdpdCBhL2RyaXZlcnMvcGNpL2hvdHBsdWcvc2dpX2hvdHBsdWcuYyBiL2RyaXZlcnMvcGNp
L2hvdHBsdWcvc2dpX2hvdHBsdWcuYw0KPiA+IGluZGV4IGJhZGEyMDk5Li5jMzJmYjc4IDEwMDY0
NA0KPiA+IC0tLSBhL2RyaXZlcnMvcGNpL2hvdHBsdWcvc2dpX2hvdHBsdWcuYw0KPiA+ICsrKyBi
L2RyaXZlcnMvcGNpL2hvdHBsdWcvc2dpX2hvdHBsdWcuYw0KPiA+IEBAIC00NzUsNyArNDc1LDcg
QEAgc3RhdGljIGludCBkaXNhYmxlX3Nsb3Qoc3RydWN0IGhvdHBsdWdfc2xvdCAqYnNzX2hvdHBs
dWdfc2xvdCkNCj4gPiAgCXN0cnVjdCBzbG90ICpzbG90ID0gYnNzX2hvdHBsdWdfc2xvdC0+cHJp
dmF0ZTsNCj4gPiAgCXN0cnVjdCBwY2lfZGV2ICpkZXYsICp0ZW1wOw0KPiA+ICAJaW50IHJjOw0K
PiA+IC0JYWNwaV9vd25lcl9pZCBzc2R0X2lkID0gMDsNCj4gPiArCWFjcGlfaGFuZGxlIHNzZHRf
aGRsID0gTlVMTDsNCj4gPg0KPiA+ICAJLyogQWNxdWlyZSB1cGRhdGUgYWNjZXNzIHRvIHRoZSBi
dXMgKi8NCj4gPiAgCW11dGV4X2xvY2soJnNuX2hvdHBsdWdfbXV0ZXgpOw0KPiA+IEBAIC01MjIs
NyArNTIyLDcgQEAgc3RhdGljIGludCBkaXNhYmxlX3Nsb3Qoc3RydWN0IGhvdHBsdWdfc2xvdCAq
YnNzX2hvdHBsdWdfc2xvdCkNCj4gPiAgCQkJaWYgKEFDUElfU1VDQ0VTUyhyZXQpICYmDQo+ID4g
IAkJCSAgICAoYWRyPj4xNikgPT0gKHNsb3QtPmRldmljZV9udW0gKyAxKSkgew0KPiA+ICAJCQkJ
LyogcmV0YWluIHRoZSBvd25lciBpZCAqLw0KPiA+IC0JCQkJYWNwaV9nZXRfaWQoY2hhbmRsZSwg
JnNzZHRfaWQpOw0KPiA+ICsJCQkJc3NkdF9oZGwgPSBjaGFuZGxlOw0KPiA+DQo+ID4gIAkJCQly
ZXQgPSBhY3BpX2J1c19nZXRfZGV2aWNlKGNoYW5kbGUsDQo+ID4gIAkJCQkJCQkgICZkZXZpY2Up
Ow0KPiA+IEBAIC01NDcsMTIgKzU0NywxMyBAQCBzdGF0aWMgaW50IGRpc2FibGVfc2xvdChzdHJ1
Y3QgaG90cGx1Z19zbG90ICpic3NfaG90cGx1Z19zbG90KQ0KPiA+ICAJcGNpX3VubG9ja19yZXNj
YW5fcmVtb3ZlKCk7DQo+ID4NCj4gPiAgCS8qIFJlbW92ZSB0aGUgU1NEVCBmb3IgdGhlIHNsb3Qg
ZnJvbSB0aGUgQUNQSSBuYW1lc3BhY2UgKi8NCj4gPiAtCWlmIChTTl9BQ1BJX0JBU0VfU1VQUE9S
VCgpICYmIHNzZHRfaWQpIHsNCj4gPiArCWlmIChTTl9BQ1BJX0JBU0VfU1VQUE9SVCgpICYmIHNz
ZHRfaGRsKSB7DQo+ID4gIAkJYWNwaV9zdGF0dXMgcmV0Ow0KPiA+IC0JCXJldCA9IGFjcGlfdW5s
b2FkX3RhYmxlX2lkKHNzZHRfaWQpOw0KPiA+ICsJCXJldCA9IGFjcGlfdW5sb2FkX3BhcmVudF90
YWJsZShzc2R0X2hkbCk7DQo+ID4gIAkJaWYgKEFDUElfRkFJTFVSRShyZXQpKSB7DQo+ID4gLQkJ
CXByaW50ayhLRVJOX0VSUiAiJXM6IGFjcGlfdW5sb2FkX3RhYmxlX2lkIGZhaWxlZCAoMHgleCkg
Zm9yIGlkICVkXG4iLA0KPiA+IC0JCQkgICAgICAgX19mdW5jX18sIHJldCwgc3NkdF9pZCk7DQo+
ID4gKwkJCWFjcGlfaGFuZGxlX2Vycihzc2R0X2hkbCwNCj4gPiArCQkJCQkiJXM6IGFjcGlfdW5s
b2FkX3BhcmVudF90YWJsZSBmYWlsZWQgKDB4JXgpXG4iLA0KPiA+ICsJCQkJCV9fZnVuY19fLCBy
ZXQpOw0KPiA+ICAJCQkvKiB0cnkgdG8gY29udGludWUgb24gKi8NCj4gPiAgCQl9DQo+ID4gIAl9
DQo+ID4gZGlmZiAtLWdpdCBhL2luY2x1ZGUvYWNwaS9hY3BpeGYuaCBiL2luY2x1ZGUvYWNwaS9h
Y3BpeGYuaA0KPiA+IGluZGV4IDViYTc4NDYuLjVkZDIxYmMgMTAwNjQ0DQo+ID4gLS0tIGEvaW5j
bHVkZS9hY3BpL2FjcGl4Zi5oDQo+ID4gKysrIGIvaW5jbHVkZS9hY3BpL2FjcGl4Zi5oDQo+ID4g
QEAgLTg5MSwxMiArODkxLDYgQEAgQUNQSV9BUFBfREVQRU5ERU5UX1JFVFVSTl9WT0lEKEFDUElf
UFJJTlRGX0xJS0UoMSkNCj4gPiAgQUNQSV9HTE9CQUwodTgsIGFjcGlfZ2JsX3Blcm1hbmVudF9t
bWFwKTsNCj4gPg0KPiA+ICBBQ1BJX0VYVEVSTkFMX1JFVFVSTl9TVEFUVVMoYWNwaV9zdGF0dXMN
Cj4gPiAtCQkJICAgIGFjcGlfZ2V0X2lkKGFjcGlfaGFuZGxlIG9iamVjdCwNCj4gPiAtCQkJCQlh
Y3BpX293bmVyX2lkICogb3V0X3R5cGUpKQ0KPiA+IC0NCj4gPiAtQUNQSV9FWFRFUk5BTF9SRVRV
Uk5fU1RBVFVTKGFjcGlfc3RhdHVzIGFjcGlfdW5sb2FkX3RhYmxlX2lkKGFjcGlfb3duZXJfaWQg
aWQpKQ0KPiA+IC0NCj4gPiAtQUNQSV9FWFRFUk5BTF9SRVRVUk5fU1RBVFVTKGFjcGlfc3RhdHVz
DQo+ID4gIAkJCSAgICBhY3BpX2dldF90YWJsZV93aXRoX3NpemUoYWNwaV9zdHJpbmcgc2lnbmF0
dXJlLA0KPiA+ICAJCQkJCQkgICAgIHUzMiBpbnN0YW5jZSwNCj4gPiAgCQkJCQkJICAgICBzdHJ1
Y3QgYWNwaV90YWJsZV9oZWFkZXINCj4gPg0KPiANCj4gLS0NCj4gSSBzcGVhayBvbmx5IGZvciBt
eXNlbGYuDQo+IFJhZmFlbCBKLiBXeXNvY2tpLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5
IENlbnRlci4NCg=

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id
  2014-12-16 16:12 ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Octavian Purdila
@ 2015-01-30  0:03       ` Rafael J. Wysocki
  2015-01-22  6:37     ` Lv Zheng
       [not found]   ` <1418746353-3481-2-git-send-email-octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2 siblings, 0 replies; 25+ messages in thread
From: Rafael J. Wysocki @ 2015-01-30  0:03 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A, johan-DgEjT+Ai2ygdnm+yROfE0A,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	heikki.krogerus-ral2JQCrhuEAvxtiuMwx3w,
	mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA

On Tuesday, December 16, 2014 06:12:30 PM Octavian Purdila wrote:
> acpi_tb_delete_namespace_by_owner expects ACPI_MTX_INTERPRETER to be
> taken. This fixes the following issue:
> 
> ACPI Error: Mutex [0x0] is not acquired, cannot release (20141107/utmutex-322)
> Call Trace:
>   [<ffffffff81b0bd28>] dump_stack+0x4f/0x7b
>   [<ffffffff81546bfc>] acpi_ut_release_mutex+0x47/0x67
>   [<ffffffff81542cf1>] acpi_tb_delete_namespace_by_owner+0x57/0x8d
>   [<ffffffff81543ef1>] acpi_unload_table_id+0x3a/0x5e
> 
> Signed-off-by: Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

I've queued up this one for 3.20.

> ---
>  drivers/acpi/acpica/tbxface.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
> index 6482b0d..9520ae1 100644
> --- a/drivers/acpi/acpica/tbxface.c
> +++ b/drivers/acpi/acpica/tbxface.c
> @@ -281,6 +281,11 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>  
>  	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
>  
> +	status = acpi_ut_acquire_mutex(ACPI_MTX_INTERPRETER);
> +	if (ACPI_FAILURE(status)) {
> +		return_ACPI_STATUS(status);
> +	}
> +
>  	/* Find table in the global table list */
>  	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
>  		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
> @@ -297,6 +302,8 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>  		acpi_tb_set_table_loaded_flag(i, FALSE);
>  		break;
>  	}
> +
> +	(void)acpi_ut_release_mutex(ACPI_MTX_INTERPRETER);
>  	return_ACPI_STATUS(status);
>  }
>  
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id
@ 2015-01-30  0:03       ` Rafael J. Wysocki
  0 siblings, 0 replies; 25+ messages in thread
From: Rafael J. Wysocki @ 2015-01-30  0:03 UTC (permalink / raw)
  To: Octavian Purdila
  Cc: linus.walleij, lee.jones, johan, linux-usb, linux-kernel,
	linux-gpio, heikki.krogerus, mika.westerberg, linux-acpi

On Tuesday, December 16, 2014 06:12:30 PM Octavian Purdila wrote:
> acpi_tb_delete_namespace_by_owner expects ACPI_MTX_INTERPRETER to be
> taken. This fixes the following issue:
> 
> ACPI Error: Mutex [0x0] is not acquired, cannot release (20141107/utmutex-322)
> Call Trace:
>   [<ffffffff81b0bd28>] dump_stack+0x4f/0x7b
>   [<ffffffff81546bfc>] acpi_ut_release_mutex+0x47/0x67
>   [<ffffffff81542cf1>] acpi_tb_delete_namespace_by_owner+0x57/0x8d
>   [<ffffffff81543ef1>] acpi_unload_table_id+0x3a/0x5e
> 
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>

I've queued up this one for 3.20.

> ---
>  drivers/acpi/acpica/tbxface.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/acpi/acpica/tbxface.c b/drivers/acpi/acpica/tbxface.c
> index 6482b0d..9520ae1 100644
> --- a/drivers/acpi/acpica/tbxface.c
> +++ b/drivers/acpi/acpica/tbxface.c
> @@ -281,6 +281,11 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>  
>  	ACPI_FUNCTION_TRACE(acpi_unload_table_id);
>  
> +	status = acpi_ut_acquire_mutex(ACPI_MTX_INTERPRETER);
> +	if (ACPI_FAILURE(status)) {
> +		return_ACPI_STATUS(status);
> +	}
> +
>  	/* Find table in the global table list */
>  	for (i = 0; i < acpi_gbl_root_table_list.current_table_count; ++i) {
>  		if (id != acpi_gbl_root_table_list.tables[i].owner_id) {
> @@ -297,6 +302,8 @@ acpi_status acpi_unload_table_id(acpi_owner_id id)
>  		acpi_tb_set_table_loaded_flag(i, FALSE);
>  		break;
>  	}
> +
> +	(void)acpi_ut_release_mutex(ACPI_MTX_INTERPRETER);
>  	return_ACPI_STATUS(status);
>  }
>  
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2015-01-30  0:03 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-16 16:12 [PATCH 0/4] dln2: add support for ACPI Octavian Purdila
2014-12-16 16:12 ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Octavian Purdila
2014-12-16 18:14   ` Sergei Shtylyov
2014-12-16 19:32     ` Octavian Purdila
2015-01-22  6:37   ` [RFC PATCH] ACPI: Introduce acpi_unload_parent_table() usages in Linux kernel Lv Zheng
2015-01-22  6:37     ` Lv Zheng
2015-01-22 22:43     ` Rafael J. Wysocki
2015-01-22 22:43       ` Rafael J. Wysocki
2015-01-22 22:43       ` Bjorn Helgaas
2015-01-22 22:43         ` Bjorn Helgaas
2015-01-26  1:59       ` Zheng, Lv
2015-01-26  1:59         ` Zheng, Lv
2015-01-26  1:59         ` Zheng, Lv
     [not found]   ` <1418746353-3481-2-git-send-email-octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2015-01-30  0:03     ` [PATCH 1/4] ACPICA: take ACPI_MTX_INTERPRETER in acpi_unload_table_id Rafael J. Wysocki
2015-01-30  0:03       ` Rafael J. Wysocki
2014-12-16 16:12 ` [PATCH 2/4] ACPICA: don't release ACPI_MTX_TABLES in acpi_tb_install_standard_table Octavian Purdila
2014-12-16 16:12 ` [PATCH 3/4] mfd: dln2: add support for ACPI Octavian Purdila
2015-01-20 15:20   ` Lee Jones
2015-01-20 15:20     ` Lee Jones
2015-01-22  2:00   ` Rafael J. Wysocki
2015-01-22 10:13     ` Octavian Purdila
2015-01-22 22:09       ` Rafael J. Wysocki
2015-01-23  6:47         ` Octavian Purdila
2015-01-23 15:19           ` Rafael J. Wysocki
2014-12-16 16:12 ` [PATCH 4/4] gpio: dln2: add support for ACPI pin configuration Octavian Purdila

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.