diff mbox

ACPI: Enable SCI_EMULATE to manually simulate physical hotplug testing.

Message ID 1347582132.3569.13.camel@misato.fc.hp.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Toshi Kani Sept. 14, 2012, 12:22 a.m. UTC 
> > >> +
> > >> +     /*
> > >> +      * 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

Comments

Yinghai Lu Sept. 14, 2012, 3:50 a.m. UTC | #1 
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
--
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
Toshi Kani Sept. 14, 2012, 2:49 p.m. UTC | #2 
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

--
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
diff mbox

Patch

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