linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ACPI:  Enable SCI_EMULATE to manually simulate physical hotplug testing.
@ 2012-09-03 21:27 Yinghai Lu
  2012-09-04 16:27 ` Toshi Kani
  0 siblings, 1 reply; 9+ messages in thread
From: Yinghai Lu @ 2012-09-03 21:27 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-acpi, linux-kernel, Ashok Raj, Yinghai Lu

From: Ashok Raj <ashok.raj@intel.com>

Emulate an ACPI SCI interrupt to emulate a hot-plug event. Useful
for testing ACPI based hot-plug on systems that don't have the
necessary firmware support.

Enable CONFIG_ACPI_SCI_EMULATE on kernel compile.

Now you will notice /sys/kernel/debug/acpi/sci_notify when new kernel is
booted.

echo "\_SB.PCIB 1" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-add
of root bus that is corresponding to PCIB.

echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-remove
of root bus that is corresponding to PCIB.

-v2: Update to current upstream, and remove not related stuff.
-v3: According to Len's request, update it to use debugfs.  - Yinghai Lu

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: linux-acpi@vger.kernel.org

===================================================================
---
 drivers/acpi/Kconfig   |   10 +++
 drivers/acpi/Makefile  |    1 
 drivers/acpi/sci_emu.c |  145 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 156 insertions(+)

Index: linux-2.6/drivers/acpi/Kconfig
===================================================================
--- linux-2.6.orig/drivers/acpi/Kconfig
+++ linux-2.6/drivers/acpi/Kconfig
@@ -272,6 +272,16 @@ config ACPI_BLACKLIST_YEAR
 	  Enter 0 to disable this mechanism and allow ACPI to
 	  run by default no matter what the year.  (default)
 
+config ACPI_SCI_EMULATE
+        bool "ACPI SCI Event Emulation Support"
+        depends on DEBUG_FS
+	default n
+	help
+	  This will enable your system to emulate sci hotplug event
+	  notification through proc file system. For example user needs to
+	  echo "XXX 0" > /sys/kernel/debug/acpi/sci_notify (where, XXX is
+	  a target ACPI device object name present under \_SB scope).
+
 config ACPI_DEBUG
 	bool "Debug Statements"
 	default n
Index: linux-2.6/drivers/acpi/sci_emu.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/acpi/sci_emu.c
@@ -0,0 +1,145 @@
+/*
+ *  Code to emulate SCI interrupt for Hotplug node insertion/removal
+ */
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/uaccess.h>
+#include <linux/debugfs.h>
+#include <acpi/acpi_drivers.h>
+
+#include "internal.h"
+
+#include "acpica/accommon.h"
+#include "acpica/acnamesp.h"
+#include "acpica/acevents.h"
+
+#define _COMPONENT		ACPI_SYSTEM_COMPONENT
+ACPI_MODULE_NAME("sci_emu");
+MODULE_LICENSE("GPL");
+
+static struct dentry *sci_notify_dentry;
+
+static void sci_notify_client(char *acpi_name, u32 event)
+{
+	struct acpi_namespace_node *node;
+	acpi_status status, status1;
+	acpi_handle hlsb, hsb;
+	union acpi_operand_object *obj_desc;
+
+	status = acpi_get_handle(NULL, "\\_SB", &hsb);
+	status1 = acpi_get_handle(hsb, acpi_name, &hlsb);
+	if (ACPI_FAILURE(status) || ACPI_FAILURE(status1)) {
+		pr_err(PREFIX
+	"acpi getting handle to <\\_SB.%s> failed inside notify_client\n",
+			acpi_name);
+		return;
+	}
+
+	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
+	if (ACPI_FAILURE(status)) {
+		pr_err(PREFIX "Acquiring acpi namespace mutext failed\n");
+		return;
+	}
+
+	node = acpi_ns_validate_handle(hlsb);
+	if (!node) {
+		acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+		pr_err(PREFIX "Mapping handle to node failed\n");
+		return;
+	}
+
+	/*
+	 * Check for internal object and make sure there is a handler
+	 * registered for this object
+	 */
+	obj_desc = acpi_ns_get_attached_object(node);
+	if (obj_desc) {
+		if (obj_desc->common_notify.notify_list[0]) {
+			/*
+			 * Release the lock and queue the item for later
+			 * exectuion
+			 */
+			acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+			status = acpi_ev_queue_notify_request(node, event);
+			if (ACPI_FAILURE(status))
+				pr_err(PREFIX "acpi_ev_queue_notify_request failed\n");
+			else
+				pr_info(PREFIX "Notify event is queued\n");
+			return;
+		}
+	} else {
+		pr_info(PREFIX "Notify handler not registered for this device\n");
+	}
+
+	acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+	return;
+}
+
+static ssize_t sci_notify_write(struct file *file, const char __user *user_buf,
+				 size_t count, loff_t *ppos)
+{
+	u32 event;
+	char *name1 = NULL;
+	char *name2 = NULL;
+	const char *delim = " ";
+	char *temp_buf = NULL;
+	char *temp_buf_addr = NULL;
+
+	temp_buf = kmalloc(count+1, GFP_ATOMIC);
+	if (!temp_buf) {
+		pr_warn(PREFIX "sci_notify_wire: Memory allocation failed\n");
+		return count;
+	}
+	temp_buf[count] = '\0';
+	temp_buf_addr = temp_buf;
+	if (copy_from_user(temp_buf, user_buf, count))
+		goto out;
+
+	name1 = strsep(&temp_buf, delim);
+	name2 = strsep(&temp_buf, delim);
+
+	if (name1 && name2) {
+		ssize_t ret;
+		unsigned long val;
+
+		ret = kstrtoul(name2, 10, &val);
+		if (ret) {
+			pr_warn(PREFIX "unknown event\n");
+			goto out;
+		}
+
+		event = (u32)val;
+	} else {
+		pr_warn(PREFIX "unknown device\n");
+		goto out;
+	}
+
+	pr_info(PREFIX "ACPI device name is <%s>, event code is <%d>\n",
+		name1, event);
+
+	sci_notify_client(name1, event);
+
+out:
+	kfree(temp_buf_addr);
+	return count;
+}
+
+static const struct file_operations sci_notify_fops = {
+	.write = sci_notify_write,
+};
+
+static int __init acpi_sci_notify_init(void)
+{
+	if (acpi_debugfs_dir == NULL)
+		return -ENOENT;
+
+	sci_notify_dentry = debugfs_create_file("sci_notify", S_IWUSR,
+				acpi_debugfs_dir, NULL, &sci_notify_fops);
+	if (sci_notify_dentry == NULL)
+		return -ENODEV;
+
+	return 0;
+}
+
+device_initcall(acpi_sci_notify_init);
Index: linux-2.6/drivers/acpi/Makefile
===================================================================
--- linux-2.6.orig/drivers/acpi/Makefile
+++ linux-2.6/drivers/acpi/Makefile
@@ -31,6 +31,7 @@ acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o
 # ACPI Bus and Device Drivers
 #
 acpi-y				+= bus.o glue.o
+acpi-$(CONFIG_ACPI_SCI_EMULATE)	+= sci_emu.o
 acpi-y				+= scan.o
 acpi-y				+= processor_core.o
 acpi-y				+= ec.o

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

* Re: [PATCH] ACPI:  Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-03 21:27 [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing Yinghai Lu
@ 2012-09-04 16:27 ` Toshi Kani
  2012-09-04 19:17   ` Yinghai Lu
  0 siblings, 1 reply; 9+ messages in thread
From: Toshi Kani @ 2012-09-04 16:27 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

On Mon, 2012-09-03 at 14:27 -0700, Yinghai Lu wrote:
> From: Ashok Raj <ashok.raj@intel.com>
> 
> Emulate an ACPI SCI interrupt to emulate a hot-plug event. Useful
> for testing ACPI based hot-plug on systems that don't have the
> necessary firmware support.
> 
> Enable CONFIG_ACPI_SCI_EMULATE on kernel compile.
> 
> Now you will notice /sys/kernel/debug/acpi/sci_notify when new kernel is
> booted.
> 
> echo "\_SB.PCIB 1" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-add
> of root bus that is corresponding to PCIB.
> 
> echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-remove
> of root bus that is corresponding to PCIB.

Hi Yinghai,

This feature has been very useful.  Thanks for working on this change.
I have a few comments below.


> -v2: Update to current upstream, and remove not related stuff.
> -v3: According to Len's request, update it to use debugfs.  - Yinghai Lu
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Len Brown <lenb@kernel.org>
> Cc: linux-acpi@vger.kernel.org
> 
> ===================================================================
> ---
>  drivers/acpi/Kconfig   |   10 +++
>  drivers/acpi/Makefile  |    1 
>  drivers/acpi/sci_emu.c |  145 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 156 insertions(+)
> 
> Index: linux-2.6/drivers/acpi/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/Kconfig
> +++ linux-2.6/drivers/acpi/Kconfig
> @@ -272,6 +272,16 @@ config ACPI_BLACKLIST_YEAR
>  	  Enter 0 to disable this mechanism and allow ACPI to
>  	  run by default no matter what the year.  (default)
>  
> +config ACPI_SCI_EMULATE
> +        bool "ACPI SCI Event Emulation Support"
> +        depends on DEBUG_FS
> +	default n
> +	help
> +	  This will enable your system to emulate sci hotplug event
> +	  notification through proc file system. For example user needs to
> +	  echo "XXX 0" > /sys/kernel/debug/acpi/sci_notify (where, XXX is
> +	  a target ACPI device object name present under \_SB scope).
> +
>  config ACPI_DEBUG
>  	bool "Debug Statements"
>  	default n
> Index: linux-2.6/drivers/acpi/sci_emu.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/acpi/sci_emu.c
> @@ -0,0 +1,145 @@
> +/*
> + *  Code to emulate SCI interrupt for Hotplug node insertion/removal
> + */
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/uaccess.h>
> +#include <linux/debugfs.h>
> +#include <acpi/acpi_drivers.h>
> +
> +#include "internal.h"
> +
> +#include "acpica/accommon.h"
> +#include "acpica/acnamesp.h"
> +#include "acpica/acevents.h"
> +
> +#define _COMPONENT		ACPI_SYSTEM_COMPONENT
> +ACPI_MODULE_NAME("sci_emu");
> +MODULE_LICENSE("GPL");
> +
> +static struct dentry *sci_notify_dentry;
> +
> +static void sci_notify_client(char *acpi_name, u32 event)
> +{
> +	struct acpi_namespace_node *node;
> +	acpi_status status, status1;
> +	acpi_handle hlsb, hsb;
> +	union acpi_operand_object *obj_desc;
> +
> +	status = acpi_get_handle(NULL, "\\_SB", &hsb);
> +	status1 = acpi_get_handle(hsb, acpi_name, &hlsb);

Why do you obtain hsb for \_SB when acpi_name is supposed to be a full
path name?  Can you simply specify a NULL like this?
  status = acpi_get_handle(NULL, acpi_name, &hlsb);


> +	if (ACPI_FAILURE(status) || ACPI_FAILURE(status1)) {
> +		pr_err(PREFIX
> +	"acpi getting handle to <\\_SB.%s> failed inside notify_client\n",
> +			acpi_name);
> +		return;
> +	}
> +
> +	status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> +	if (ACPI_FAILURE(status)) {
> +		pr_err(PREFIX "Acquiring acpi namespace mutext failed\n");
> +		return;
> +	}
> +
> +	node = acpi_ns_validate_handle(hlsb);
> +	if (!node) {
> +		acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> +		pr_err(PREFIX "Mapping handle to node failed\n");
> +		return;
> +	}
> +
> +	/*
> +	 * Check for internal object and make sure there is a handler
> +	 * registered for this object
> +	 */
> +	obj_desc = acpi_ns_get_attached_object(node);
> +	if (obj_desc) {
> +		if (obj_desc->common_notify.notify_list[0]) {

Is the above check necessary?  acpi_ev_queue_notify_request() sets up to
call the global handler, acpi_gbl_global_notify[0], even if the object
does not have a local handler registered.

Thanks,
-Toshi


> +			/*
> +			 * Release the lock and queue the item for later
> +			 * exectuion
> +			 */
> +			acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> +			status = acpi_ev_queue_notify_request(node, event);
> +			if (ACPI_FAILURE(status))
> +				pr_err(PREFIX "acpi_ev_queue_notify_request failed\n");
> +			else
> +				pr_info(PREFIX "Notify event is queued\n");
> +			return;
> +		}
> +	} else {
> +		pr_info(PREFIX "Notify handler not registered for this device\n");
> +	}
> +
> +	acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> +	return;
> +}
> +
> +static ssize_t sci_notify_write(struct file *file, const char __user *user_buf,
> +				 size_t count, loff_t *ppos)
> +{
> +	u32 event;
> +	char *name1 = NULL;
> +	char *name2 = NULL;
> +	const char *delim = " ";
> +	char *temp_buf = NULL;
> +	char *temp_buf_addr = NULL;
> +
> +	temp_buf = kmalloc(count+1, GFP_ATOMIC);
> +	if (!temp_buf) {
> +		pr_warn(PREFIX "sci_notify_wire: Memory allocation failed\n");
> +		return count;
> +	}
> +	temp_buf[count] = '\0';
> +	temp_buf_addr = temp_buf;
> +	if (copy_from_user(temp_buf, user_buf, count))
> +		goto out;
> +
> +	name1 = strsep(&temp_buf, delim);
> +	name2 = strsep(&temp_buf, delim);
> +
> +	if (name1 && name2) {
> +		ssize_t ret;
> +		unsigned long val;
> +
> +		ret = kstrtoul(name2, 10, &val);
> +		if (ret) {
> +			pr_warn(PREFIX "unknown event\n");
> +			goto out;
> +		}
> +
> +		event = (u32)val;
> +	} else {
> +		pr_warn(PREFIX "unknown device\n");
> +		goto out;
> +	}
> +
> +	pr_info(PREFIX "ACPI device name is <%s>, event code is <%d>\n",
> +		name1, event);
> +
> +	sci_notify_client(name1, event);
> +
> +out:
> +	kfree(temp_buf_addr);
> +	return count;
> +}
> +
> +static const struct file_operations sci_notify_fops = {
> +	.write = sci_notify_write,
> +};
> +
> +static int __init acpi_sci_notify_init(void)
> +{
> +	if (acpi_debugfs_dir == NULL)
> +		return -ENOENT;
> +
> +	sci_notify_dentry = debugfs_create_file("sci_notify", S_IWUSR,
> +				acpi_debugfs_dir, NULL, &sci_notify_fops);
> +	if (sci_notify_dentry == NULL)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +device_initcall(acpi_sci_notify_init);
> Index: linux-2.6/drivers/acpi/Makefile
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/Makefile
> +++ linux-2.6/drivers/acpi/Makefile
> @@ -31,6 +31,7 @@ acpi-$(CONFIG_ACPI_SLEEP)	+= proc.o
>  # ACPI Bus and Device Drivers
>  #
>  acpi-y				+= bus.o glue.o
> +acpi-$(CONFIG_ACPI_SCI_EMULATE)	+= sci_emu.o
>  acpi-y				+= scan.o
>  acpi-y				+= processor_core.o
>  acpi-y				+= ec.o
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



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

* Re: [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-04 16:27 ` Toshi Kani
@ 2012-09-04 19:17   ` Yinghai Lu
  2012-09-04 21:34     ` Toshi Kani
  0 siblings, 1 reply; 9+ messages in thread
From: Yinghai Lu @ 2012-09-04 19:17 UTC (permalink / raw)
  To: Toshi Kani; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

On Tue, Sep 4, 2012 at 9:27 AM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Mon, 2012-09-03 at 14:27 -0700, Yinghai Lu wrote:
>> From: Ashok Raj <ashok.raj@intel.com>
>>
>> Emulate an ACPI SCI interrupt to emulate a hot-plug event. Useful
>> for testing ACPI based hot-plug on systems that don't have the
>> necessary firmware support.
>>
>> Enable CONFIG_ACPI_SCI_EMULATE on kernel compile.
>>
>> Now you will notice /sys/kernel/debug/acpi/sci_notify when new kernel is
>> booted.
>>
>> echo "\_SB.PCIB 1" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-add
>> of root bus that is corresponding to PCIB.
>>
>> echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-remove
>> of root bus that is corresponding to PCIB.
>
> Hi Yinghai,
>
> This feature has been very useful.  Thanks for working on this change.
> I have a few comments below.
>
>
>> -v2: Update to current upstream, and remove not related stuff.
>> -v3: According to Len's request, update it to use debugfs.  - Yinghai Lu
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: Len Brown <lenb@kernel.org>
>> Cc: linux-acpi@vger.kernel.org
>>
>> ===================================================================
>> ---
>>  drivers/acpi/Kconfig   |   10 +++
>>  drivers/acpi/Makefile  |    1
>>  drivers/acpi/sci_emu.c |  145 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 156 insertions(+)
>>
>> Index: linux-2.6/drivers/acpi/Kconfig
>> ===================================================================
>> --- linux-2.6.orig/drivers/acpi/Kconfig
>> +++ linux-2.6/drivers/acpi/Kconfig
>> @@ -272,6 +272,16 @@ config ACPI_BLACKLIST_YEAR
>>         Enter 0 to disable this mechanism and allow ACPI to
>>         run by default no matter what the year.  (default)
>>
>> +config ACPI_SCI_EMULATE
>> +        bool "ACPI SCI Event Emulation Support"
>> +        depends on DEBUG_FS
>> +     default n
>> +     help
>> +       This will enable your system to emulate sci hotplug event
>> +       notification through proc file system. For example user needs to
>> +       echo "XXX 0" > /sys/kernel/debug/acpi/sci_notify (where, XXX is
>> +       a target ACPI device object name present under \_SB scope).
>> +
>>  config ACPI_DEBUG
>>       bool "Debug Statements"
>>       default n
>> Index: linux-2.6/drivers/acpi/sci_emu.c
>> ===================================================================
>> --- /dev/null
>> +++ linux-2.6/drivers/acpi/sci_emu.c
>> @@ -0,0 +1,145 @@
>> +/*
>> + *  Code to emulate SCI interrupt for Hotplug node insertion/removal
>> + */
>> +#include <linux/init.h>
>> +#include <linux/module.h>
>> +#include <linux/kernel.h>
>> +#include <linux/uaccess.h>
>> +#include <linux/debugfs.h>
>> +#include <acpi/acpi_drivers.h>
>> +
>> +#include "internal.h"
>> +
>> +#include "acpica/accommon.h"
>> +#include "acpica/acnamesp.h"
>> +#include "acpica/acevents.h"
>> +
>> +#define _COMPONENT           ACPI_SYSTEM_COMPONENT
>> +ACPI_MODULE_NAME("sci_emu");
>> +MODULE_LICENSE("GPL");
>> +
>> +static struct dentry *sci_notify_dentry;
>> +
>> +static void sci_notify_client(char *acpi_name, u32 event)
>> +{
>> +     struct acpi_namespace_node *node;
>> +     acpi_status status, status1;
>> +     acpi_handle hlsb, hsb;
>> +     union acpi_operand_object *obj_desc;
>> +
>> +     status = acpi_get_handle(NULL, "\\_SB", &hsb);
>> +     status1 = acpi_get_handle(hsb, acpi_name, &hlsb);
>
> Why do you obtain hsb for \_SB when acpi_name is supposed to be a full
> path name?  Can you simply specify a NULL like this?
>   status = acpi_get_handle(NULL, acpi_name, &hlsb);

assume those two main function is from ashok.

but assume that could make user omit \_SB_?

>
>
>> +     if (ACPI_FAILURE(status) || ACPI_FAILURE(status1)) {
>> +             pr_err(PREFIX
>> +     "acpi getting handle to <\\_SB.%s> failed inside notify_client\n",
>> +                     acpi_name);
>> +             return;
>> +     }
>> +
>> +     status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
>> +     if (ACPI_FAILURE(status)) {
>> +             pr_err(PREFIX "Acquiring acpi namespace mutext failed\n");
>> +             return;
>> +     }
>> +
>> +     node = acpi_ns_validate_handle(hlsb);
>> +     if (!node) {
>> +             acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
>> +             pr_err(PREFIX "Mapping handle to node failed\n");
>> +             return;
>> +     }
>> +
>> +     /*
>> +      * Check for internal object and make sure there is a handler
>> +      * registered for this object
>> +      */
>> +     obj_desc = acpi_ns_get_attached_object(node);
>> +     if (obj_desc) {
>> +             if (obj_desc->common_notify.notify_list[0]) {
>
> Is the above check necessary?  acpi_ev_queue_notify_request() sets up to
> call the global handler, acpi_gbl_global_notify[0], even if the object
> does not have a local handler registered.

Not sure.

maybe Len or other acpi guyes could answer your questions.

Thanks

Yinghai Lu

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

* Re: [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-04 19:17   ` Yinghai Lu
@ 2012-09-04 21:34     ` Toshi Kani
  2012-09-04 21:46       ` Yinghai Lu
  2012-09-14  0:22       ` Toshi Kani
  0 siblings, 2 replies; 9+ messages in thread
From: Toshi Kani @ 2012-09-04 21:34 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

On Tue, 2012-09-04 at 12:17 -0700, Yinghai Lu wrote:
> On Tue, Sep 4, 2012 at 9:27 AM, Toshi Kani <toshi.kani@hp.com> wrote:
> > On Mon, 2012-09-03 at 14:27 -0700, Yinghai Lu wrote:
> >> From: Ashok Raj <ashok.raj@intel.com>
> >>
> >> Emulate an ACPI SCI interrupt to emulate a hot-plug event. Useful
> >> for testing ACPI based hot-plug on systems that don't have the
> >> necessary firmware support.
> >>
> >> Enable CONFIG_ACPI_SCI_EMULATE on kernel compile.
> >>
> >> Now you will notice /sys/kernel/debug/acpi/sci_notify when new kernel is
> >> booted.
> >>
> >> echo "\_SB.PCIB 1" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-add
> >> of root bus that is corresponding to PCIB.
> >>
> >> echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify to trigger a hot-remove
> >> of root bus that is corresponding to PCIB.
> >
> > Hi Yinghai,
> >
> > This feature has been very useful.  Thanks for working on this change.
> > I have a few comments below.
> >
> >
> >> -v2: Update to current upstream, and remove not related stuff.
> >> -v3: According to Len's request, update it to use debugfs.  - Yinghai Lu
> >>
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >> Cc: Len Brown <lenb@kernel.org>
> >> Cc: linux-acpi@vger.kernel.org
> >>
> >> ===================================================================
> >> ---
> >>  drivers/acpi/Kconfig   |   10 +++
> >>  drivers/acpi/Makefile  |    1
> >>  drivers/acpi/sci_emu.c |  145 +++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 156 insertions(+)
> >>
> >> Index: linux-2.6/drivers/acpi/Kconfig
> >> ===================================================================
> >> --- linux-2.6.orig/drivers/acpi/Kconfig
> >> +++ linux-2.6/drivers/acpi/Kconfig
> >> @@ -272,6 +272,16 @@ config ACPI_BLACKLIST_YEAR
> >>         Enter 0 to disable this mechanism and allow ACPI to
> >>         run by default no matter what the year.  (default)
> >>
> >> +config ACPI_SCI_EMULATE
> >> +        bool "ACPI SCI Event Emulation Support"
> >> +        depends on DEBUG_FS
> >> +     default n
> >> +     help
> >> +       This will enable your system to emulate sci hotplug event
> >> +       notification through proc file system. For example user needs to
> >> +       echo "XXX 0" > /sys/kernel/debug/acpi/sci_notify (where, XXX is
> >> +       a target ACPI device object name present under \_SB scope).
> >> +
> >>  config ACPI_DEBUG
> >>       bool "Debug Statements"
> >>       default n
> >> Index: linux-2.6/drivers/acpi/sci_emu.c
> >> ===================================================================
> >> --- /dev/null
> >> +++ linux-2.6/drivers/acpi/sci_emu.c
> >> @@ -0,0 +1,145 @@
> >> +/*
> >> + *  Code to emulate SCI interrupt for Hotplug node insertion/removal
> >> + */
> >> +#include <linux/init.h>
> >> +#include <linux/module.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/uaccess.h>
> >> +#include <linux/debugfs.h>
> >> +#include <acpi/acpi_drivers.h>
> >> +
> >> +#include "internal.h"
> >> +
> >> +#include "acpica/accommon.h"
> >> +#include "acpica/acnamesp.h"
> >> +#include "acpica/acevents.h"
> >> +
> >> +#define _COMPONENT           ACPI_SYSTEM_COMPONENT
> >> +ACPI_MODULE_NAME("sci_emu");
> >> +MODULE_LICENSE("GPL");
> >> +
> >> +static struct dentry *sci_notify_dentry;
> >> +
> >> +static void sci_notify_client(char *acpi_name, u32 event)
> >> +{
> >> +     struct acpi_namespace_node *node;
> >> +     acpi_status status, status1;
> >> +     acpi_handle hlsb, hsb;
> >> +     union acpi_operand_object *obj_desc;
> >> +
> >> +     status = acpi_get_handle(NULL, "\\_SB", &hsb);
> >> +     status1 = acpi_get_handle(hsb, acpi_name, &hlsb);
> >
> > Why do you obtain hsb for \_SB when acpi_name is supposed to be a full
> > path name?  Can you simply specify a NULL like this?
> >   status = acpi_get_handle(NULL, acpi_name, &hlsb);
> 
> assume those two main function is from ashok.
> 
> but assume that could make user omit \_SB_?

I looked at the ACPICA code and found that:
 - If a full path is specified in acpi_name, it ignores the arg hsb
("\_SB").
 - If a relative path is specified in acpi_name, it appends the arg hsb.

So, the code looks good to me; assuming that is the intent.

However, the error message below will show up like "\_SB.\_SB.PCIB" when
a full path is specified.

> >
> >> +     if (ACPI_FAILURE(status) || ACPI_FAILURE(status1)) {
> >> +             pr_err(PREFIX
> >> +     "acpi getting handle to <\\_SB.%s> failed inside notify_client\n",
> >> +                     acpi_name);
> >> +             return;
> >> +     }
> >> +
> >> +     status = acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
> >> +     if (ACPI_FAILURE(status)) {
> >> +             pr_err(PREFIX "Acquiring acpi namespace mutext failed\n");
> >> +             return;
> >> +     }
> >> +
> >> +     node = acpi_ns_validate_handle(hlsb);
> >> +     if (!node) {
> >> +             acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
> >> +             pr_err(PREFIX "Mapping handle to node failed\n");
> >> +             return;
> >> +     }
> >> +
> >> +     /*
> >> +      * Check for internal object and make sure there is a handler
> >> +      * registered for this object
> >> +      */
> >> +     obj_desc = acpi_ns_get_attached_object(node);
> >> +     if (obj_desc) {
> >> +             if (obj_desc->common_notify.notify_list[0]) {
> >
> > Is the above check necessary?  acpi_ev_queue_notify_request() sets up to
> > call the global handler, acpi_gbl_global_notify[0], even if the object
> > does not have a local handler registered.
> 
> Not sure.
> 
> maybe Len or other acpi guyes could answer your questions.

I think this check should be removed, but would like someone to
verify...

Thanks,
-Toshi


> Thanks
> 
> Yinghai Lu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



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

* Re: [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-04 21:34     ` Toshi Kani
@ 2012-09-04 21:46       ` Yinghai Lu
  2012-09-04 21:54         ` Toshi Kani
  2012-09-14  0:22       ` Toshi Kani
  1 sibling, 1 reply; 9+ messages in thread
From: Yinghai Lu @ 2012-09-04 21:46 UTC (permalink / raw)
  To: Toshi Kani; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

On Tue, Sep 4, 2012 at 2:34 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Tue, 2012-09-04 at 12:17 -0700, Yinghai Lu wrote:
>  - If a full path is specified in acpi_name, it ignores the arg hsb
> ("\_SB").
>  - If a relative path is specified in acpi_name, it appends the arg hsb.
>
> So, the code looks good to me; assuming that is the intent.
>
> However, the error message below will show up like "\_SB.\_SB.PCIB" when
> a full path is specified.

I always use
echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify

and it works.

so assume that function can accept \_SB.PCIB and PCIB both.

Thanks

Yinghai

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

* Re: [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-04 21:46       ` Yinghai Lu
@ 2012-09-04 21:54         ` Toshi Kani
  0 siblings, 0 replies; 9+ messages in thread
From: Toshi Kani @ 2012-09-04 21:54 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

On Tue, 2012-09-04 at 14:46 -0700, Yinghai Lu wrote:
> On Tue, Sep 4, 2012 at 2:34 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> > On Tue, 2012-09-04 at 12:17 -0700, Yinghai Lu wrote:
> >  - If a full path is specified in acpi_name, it ignores the arg hsb
> > ("\_SB").
> >  - If a relative path is specified in acpi_name, it appends the arg hsb.
> >
> > So, the code looks good to me; assuming that is the intent.
> >
> > However, the error message below will show up like "\_SB.\_SB.PCIB" when
> > a full path is specified.
> 
> I always use
> echo "\_SB.PCIB 3" > /sys/kernel/debug/acpi/sci_notify
> 
> and it works.
> 
> so assume that function can accept \_SB.PCIB and PCIB both.

I agree.  And that's what I tried to say.


The error message below will be confusing when a (invalid) full path
name is specified, though.

>> +     if (ACPI_FAILURE(status) || ACPI_FAILURE(status1)) {
>> +             pr_err(PREFIX
>> +     "acpi getting handle to <\\_SB.%s> failed inside notify_client
\n",
>> +                     acpi_name);
>> +             return;
>> +     }


Thanks,
-Toshi


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

* Re: [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-04 21:34     ` Toshi Kani
  2012-09-04 21:46       ` Yinghai Lu
@ 2012-09-14  0:22       ` Toshi Kani
  2012-09-14  3:50         ` Yinghai Lu
  1 sibling, 1 reply; 9+ messages in thread
From: Toshi Kani @ 2012-09-14  0:22 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]

> > >> +
> > >> +     /*
> > >> +      * Check for internal object and make sure there is a handler
> > >> +      * registered for this object
> > >> +      */
> > >> +     obj_desc = acpi_ns_get_attached_object(node);
> > >> +     if (obj_desc) {
> > >> +             if (obj_desc->common_notify.notify_list[0]) {
> > >
> > > Is the above check necessary?  acpi_ev_queue_notify_request() sets up to
> > > call the global handler, acpi_gbl_global_notify[0], even if the object
> > > does not have a local handler registered.
> > 
> > Not sure.
> > 
> > maybe Len or other acpi guyes could answer your questions.
> 
> I think this check should be removed, but would like someone to
> verify...


Hi Yinghai,

Attached is my suggested update to your patch. It allows a SCI to be
sent to any object, and therefore can be used for testing the global
notify handler. Some drivers such as dock.c only register their handler
to the global notify handler. I also made a few minor changes. I have
been testing with this update and it is working fine. I like this
feature, so I hope we can make progress with this update.

Thanks,
-Toshi

[-- Attachment #2: 0001-ACPI-Update-CONFIG_ACPI_SCI_EMULATE-patch.patch --]
[-- Type: text/x-patch, Size: 2590 bytes --]

>From 2bd8a7998696c413dcc0bea089d7d9f8e548afcd Mon Sep 17 00:00:00 2001
From: Toshi Kani <toshi.kani@hp.com>
Date: Thu, 13 Sep 2012 17:45:38 -0600
Subject: [PATCH] ACPI: Update CONFIG_ACPI_SCI_EMULATE patch

---
 drivers/acpi/sci_emu.c |   47 +++++++++++++++--------------------------------
 1 files changed, 15 insertions(+), 32 deletions(-)

diff --git a/drivers/acpi/sci_emu.c b/drivers/acpi/sci_emu.c
index 2f13ca1..efa0f6a 100644
--- a/drivers/acpi/sci_emu.c
+++ b/drivers/acpi/sci_emu.c
@@ -23,16 +23,12 @@ static struct dentry *sci_notify_dentry;
 static void sci_notify_client(char *acpi_name, u32 event)
 {
 	struct acpi_namespace_node *node;
-	acpi_status status, status1;
-	acpi_handle hlsb, hsb;
-	union acpi_operand_object *obj_desc;
-
-	status = acpi_get_handle(NULL, "\\_SB", &hsb);
-	status1 = acpi_get_handle(hsb, acpi_name, &hlsb);
-	if (ACPI_FAILURE(status) || ACPI_FAILURE(status1)) {
-		pr_err(PREFIX
-	"acpi getting handle to <\\_SB.%s> failed inside notify_client\n",
-			acpi_name);
+	acpi_status status;
+	acpi_handle handle;
+
+	status = acpi_get_handle(NULL, acpi_name, &handle);
+	if (ACPI_FAILURE(status)) {
+		pr_err(PREFIX "Invalid ACPI device name <%s>\n", acpi_name);
 		return;
 	}
 
@@ -42,7 +38,7 @@ static void sci_notify_client(char *acpi_name, u32 event)
 		return;
 	}
 
-	node = acpi_ns_validate_handle(hlsb);
+	node = acpi_ns_validate_handle(handle);
 	if (!node) {
 		acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
 		pr_err(PREFIX "Mapping handle to node failed\n");
@@ -50,29 +46,16 @@ static void sci_notify_client(char *acpi_name, u32 event)
 	}
 
 	/*
-	 * Check for internal object and make sure there is a handler
-	 * registered for this object
+	 * Release the lock and queue the item for later
+	 * exectuion
 	 */
-	obj_desc = acpi_ns_get_attached_object(node);
-	if (obj_desc) {
-		if (obj_desc->common_notify.notify_list[0]) {
-			/*
-			 * Release the lock and queue the item for later
-			 * exectuion
-			 */
-			acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
-			status = acpi_ev_queue_notify_request(node, event);
-			if (ACPI_FAILURE(status))
-				pr_err(PREFIX "acpi_ev_queue_notify_request failed\n");
-			else
-				pr_info(PREFIX "Notify event is queued\n");
-			return;
-		}
-	} else {
-		pr_info(PREFIX "Notify handler not registered for this device\n");
-	}
-
 	acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
+	status = acpi_ev_queue_notify_request(node, event);
+	if (ACPI_FAILURE(status))
+		pr_err(PREFIX "acpi_ev_queue_notify_request failed\n");
+	else
+		pr_info(PREFIX "Notify event is queued\n");
+
 	return;
 }
 
-- 
1.7.7.6


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

* Re: [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-14  0:22       ` Toshi Kani
@ 2012-09-14  3:50         ` Yinghai Lu
  2012-09-14 14:49           ` Toshi Kani
  0 siblings, 1 reply; 9+ messages in thread
From: Yinghai Lu @ 2012-09-14  3:50 UTC (permalink / raw)
  To: Toshi Kani; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

On Thu, Sep 13, 2012 at 5:22 PM, Toshi Kani <toshi.kani@hp.com> wrote:
>> > >> +
>> > >> +     /*
>> > >> +      * Check for internal object and make sure there is a handler
>> > >> +      * registered for this object
>> > >> +      */
>> > >> +     obj_desc = acpi_ns_get_attached_object(node);
>> > >> +     if (obj_desc) {
>> > >> +             if (obj_desc->common_notify.notify_list[0]) {
>> > >
>> > > Is the above check necessary?  acpi_ev_queue_notify_request() sets up to
>> > > call the global handler, acpi_gbl_global_notify[0], even if the object
>> > > does not have a local handler registered.
>> >
>> > Not sure.
>> >
>> > maybe Len or other acpi guyes could answer your questions.
>>
>> I think this check should be removed, but would like someone to
>> verify...
>
>
> Hi Yinghai,
>
> Attached is my suggested update to your patch. It allows a SCI to be
> sent to any object, and therefore can be used for testing the global
> notify handler. Some drivers such as dock.c only register their handler
> to the global notify handler. I also made a few minor changes. I have
> been testing with this update and it is working fine. I like this
> feature, so I hope we can make progress with this update.

len, can you check them?

Thanks

Yinghai

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

* Re: [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.
  2012-09-14  3:50         ` Yinghai Lu
@ 2012-09-14 14:49           ` Toshi Kani
  0 siblings, 0 replies; 9+ messages in thread
From: Toshi Kani @ 2012-09-14 14:49 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Len Brown, linux-acpi, linux-kernel, Ashok Raj

On Thu, 2012-09-13 at 20:50 -0700, Yinghai Lu wrote:
> On Thu, Sep 13, 2012 at 5:22 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> >> > >> +
> >> > >> +     /*
> >> > >> +      * Check for internal object and make sure there is a handler
> >> > >> +      * registered for this object
> >> > >> +      */
> >> > >> +     obj_desc = acpi_ns_get_attached_object(node);
> >> > >> +     if (obj_desc) {
> >> > >> +             if (obj_desc->common_notify.notify_list[0]) {
> >> > >
> >> > > Is the above check necessary?  acpi_ev_queue_notify_request() sets up to
> >> > > call the global handler, acpi_gbl_global_notify[0], even if the object
> >> > > does not have a local handler registered.
> >> >
> >> > Not sure.
> >> >
> >> > maybe Len or other acpi guyes could answer your questions.
> >>
> >> I think this check should be removed, but would like someone to
> >> verify...
> >
> >
> > Hi Yinghai,
> >
> > Attached is my suggested update to your patch. It allows a SCI to be
> > sent to any object, and therefore can be used for testing the global
> > notify handler. Some drivers such as dock.c only register their handler
> > to the global notify handler. I also made a few minor changes. I have
> > been testing with this update and it is working fine. I like this
> > feature, so I hope we can make progress with this update.
> 
> len, can you check them?

Len and others can double check, but let me explain why this update is
necessary. acpi_ev_queue_notify_request() has the following internal
procedure and checks.

1. Call acpi_ev_is_notify_object() to make sure that notify is allowed
on the target object.

2. Get a proper list ID to notify_list[] for the notify value. Your
patch hardcodes this ID as 0, which is not correct.

3. Check if the target object has global handler or local handler
registered. Technically, your patch can copy this code to fix it.
However, I do not recommend copying such code since it is ACPICA
internal code and should not be exposed to drivers. ACPICA update can
break such code. Hence, I deleted the check from your patch.

4. If all checks passed, call acpi_ev_notify_dispatch() with the notify
request.

Feel free to add my sign-off when you update the patch. I take the
responsibility for the update.
Signed-off-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi


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

end of thread, other threads:[~2012-09-14 14:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-03 21:27 [PATCH] ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing Yinghai Lu
2012-09-04 16:27 ` Toshi Kani
2012-09-04 19:17   ` Yinghai Lu
2012-09-04 21:34     ` Toshi Kani
2012-09-04 21:46       ` Yinghai Lu
2012-09-04 21:54         ` Toshi Kani
2012-09-14  0:22       ` Toshi Kani
2012-09-14  3:50         ` Yinghai Lu
2012-09-14 14:49           ` Toshi Kani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).