Allow notifications on method execution
Subsystems (2)
RFC 1/1 Allow notifications on method execution
Various bits of hardware (some Thinkpads, for example) will execute ACPI methods in response to events but won't generate any notifications. It would be nice to be able to receive notifications on those updates in order to avoid polling. How about something like the following?
drivers/acpi/events/evxface.c +72 -0
@@ -549,6 +549,78 @@ ACPI_EXPORT_SYMBOL(acpi_remove_notify_handler)/******************************************************************************* * +* FUNCTION: acpi_install_method_handler +* +* PARAMETERS: Oathname - The method for which notifies will be handled +* Handler - Address of the handler +* Context - Value passed to the handler on each call +* +* RETURN: Status +* +* DESCRIPTION: Install a handler for notifies on ACPI method execution +* +******************************************************************************/ + +acpi_status +acpi_install_method_handler (char *pathname,acpi_method_handler handler, +void *context) +{ +struct acpi_namespace_node *node; +union acpi_operand_object *method; + +acpi_ns_get_node (ACPI_NS_ALL , pathname,0 , &node); +if (!node) +return_ACPI_STATUS (AE_BAD_PARAMETER ); + + method =acpi_ns_get_attached_object (node); + +if (method->method.notify) +return_ACPI_STATUS (AE_ALREADY_EXISTS ); + + method->method.notify = handler; + method->method.context = context; + +return_ACPI_STATUS (AE_OK ); +} +ACPI_EXPORT_SYMBOL (acpi_install_method_handler) + +/******************************************************************************* +* +* FUNCTION: acpi_remove_method_handler +* +* PARAMETERS: Pathname - The method for which notifies will be handled +* Handler - Address of the handler +* +* RETURN: Status +* +* DESCRIPTION: Remove a handler for notifies on ACPI method execution +* +******************************************************************************/ + +acpi_status +acpi_remove_method_handler (char *pathname,acpi_method_handler handler) +{ +struct acpi_namespace_node *node; +union acpi_operand_object *method; + +acpi_ns_get_node (ACPI_NS_ALL , pathname,0 , &node); +if (!node) +return_ACPI_STATUS (AE_BAD_PARAMETER ); + + method =acpi_ns_get_attached_object (node); + +if (method->method.notify != handler) +return_ACPI_STATUS (AE_BAD_PARAMETER ); + + method->method.notify =NULL ; + method->method.context =NULL ; + +return_ACPI_STATUS (AE_OK ); +} +ACPI_EXPORT_SYMBOL (acpi_remove_method_handler) + +/******************************************************************************* +* * FUNCTION: acpi_install_gpe_handler * * PARAMETERS: gpe_device - Namespace node for the GPE (NULL for FADT
drivers/acpi/parser/psparse.c +10 -0
@@ -445,6 +445,7 @@ acpi_status acpi_ps_parse_aml(struct acpi_walk_state *walk_state)struct acpi_thread_state *thread;struct acpi_thread_state *prev_walk_list = acpi_gbl_current_walk_list;struct acpi_walk_state *previous_walk_state; +struct acpi_namespace_node *method_node;ACPI_FUNCTION_TRACE (ps_parse_aml);
@@ -488,6 +489,15 @@ acpi_status acpi_ps_parse_aml(struct acpi_walk_state *walk_state) status =AE_OK ;while (walk_state) { + method_node = walk_state->method_call_node; + +if (method_node) { +union acpi_operand_object *method = +acpi_ns_get_attached_object (method_node); +if (method->method.notify) + method->method.notify (method->method.context); + } +if (ACPI_SUCCESS (status)) {/* * The parse_loop executes AML until the method terminates
include/acpi/acobject.h +2 -0
@@ -183,6 +183,8 @@ struct acpi_object_method {u32 aml_length;u8 thread_count;acpi_owner_id owner_id; +acpi_method_handler notify; +void *context; };/******************************************************************************
include/acpi/acpixf.h +7 -0
@@ -212,6 +212,13 @@ acpi_remove_notify_handler(acpi_handle device, u32 handler_type, acpi_notify_handler handler);acpi_status +acpi_install_method_handler (char *pathname,acpi_method_handler handler, +void *context); + +acpi_status +acpi_remove_method_handler (char *pathname,acpi_method_handler handler); + +acpi_status acpi_install_address_space_handler(acpi_handle device, acpi_adr_space_type space_id, acpi_adr_space_handler handler,
include/acpi/actypes.h +3 -0
@@ -768,6 +768,9 @@ void (*acpi_notify_handler) (acpi_handle device, u32 value, void *context);typedef void (*acpi_object_handler ) (acpi_handle object,u32 function,void *data); +typedef +void (*acpi_method_handler ) (void *context); +typedef acpi_status (*acpi_init_handler ) (acpi_handle object,u32 function);#define ACPI_INIT_DEVICE_INI 1
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 4 lines · Matthew Garrett
Various bits of hardware (some Thinkpads, for example) will execute ACPI methods in response to events but won't generate any notifications. It would be nice to be able to receive notifications on those updates in order to avoid polling. How about something like the following?
No comment on the code itself, but I have some about the whole idea. If I missed the point, please explain it to me.
Let me start pointing out that I am not really opposed to your idea, but I'd like to know if it is the best way to go about it, first.
With such ACPI notifies, you could have HAL do some of the stuff kernel drivers are doing. So far so good (well, sort of).
But that is NOT necessarily the best, or even the correct way to do it. You could use ACPI "traps" sending events to userspace to know brightness changed, but the proper way to have those events would be for the backlight class to send uevents for such stuff, instead.
Remember that if ACPI core sends too much data to userspace directly, we can't easily fix stuff in the drivers to present a more or less sanitized view of things to userspace.
So I'd really like to know what you need it for? What sort of hardware events you'd like to be notified that thinkpad-acpi is not notifying you about, for example?
Avoiding polling is ALWAYS a good thing. But being careful about what you export to userspace is ALWAYS wise as well. I believe we can do both.
Even if it means we need to go around fixing half-completed sysfs classes until they're good enough an API for us and userspace. rfkill is a prime example of the effort that might be needed.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 2 lines · Henrique de Moraes Holschuh
With such ACPI notifies, you could have HAL do some of the stuff kernel drivers are doing. So far so good (well, sort of).
That's not what I was thinking. Right now, older Thinkpads need polling if you want to get some keyboard events. However, the BIOS seems to impleemnt this by calling the ACPI CMOS update methods. Rather than polling, we can therefore make it event driven. The events won't go out to userspace, they'll just call a callback in the driver which then generates input events or uevents or whatever.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 10 lines · Matthew Garrett
Quoted 2 lines · Henrique de Moraes Holschuh
With such ACPI notifies, you could have HAL do some of the stuff kernel drivers are doing. So far so good (well, sort of).
That's not what I was thinking. Right now, older Thinkpads need polling if you want to get some keyboard events. However, the BIOS seems to impleemnt this by calling the ACPI CMOS update methods. Rather than polling, we can therefore make it event driven. The events won't go out to userspace, they'll just call a callback in the driver which then generates input events or uevents or whatever.
Sounds like a good idea, but in order to do that, we just need to let kernel drivers hook to GPE handlers somehow. I'd be quite happy to add support for it to thinkpad-acpi.
I thought your patch did a lot more than this. If I am wrong about it, I apologise.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 3 lines · Henrique de Moraes Holschuh
Sounds like a good idea, but in order to do that, we just need to let kernel drivers hook to GPE handlers somehow. I'd be quite happy to add support for it to thinkpad-acpi.
I expected that the method name would probably be more consistent, whereas the GPE number might change in a way that was difficult to hook. There's only three CMOS update methods in the range of Thinkpad BIOSes, IIRC?
Quoted 2 lines
I thought your patch did a lot more than this. If I am wrong about it, I apologise.
No problem :)
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 9 lines · Matthew Garrett
Quoted 3 lines · Henrique de Moraes Holschuh
Sounds like a good idea, but in order to do that, we just need to let kernel drivers hook to GPE handlers somehow. I'd be quite happy to add support for it to thinkpad-acpi.
I expected that the method name would probably be more consistent, whereas the GPE number might change in a way that was difficult to hook. There's only three CMOS update methods in the range of Thinkpad BIOSes, IIRC?
True. Well, both would work, and I'd have to add this as a whitelist anyway (which is NOT a problem, as we are talking about old thinkpads anyway, and there is a finite, non-extremely-huge number of those).
It works marginally better if one traps the GPE, as that looks a lot more like what one gets in a modern thinkpad instead of what one gets from the NVRAM (look at the nvram polling code in thinkpad-acpi to understand what I mean. Some not so pretty crap was required in there due to deal with loss of information in the VOLUME/MUTE handling...)
Hotfixing the AML would also allow one to do all of this and more (maybe hell will freeze over and Len will let us do it :-) ).
Anyway, if we add notifications for normal nodes, please add them for GPEs too.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 5 lines · Henrique de Moraes Holschuh
It works marginally better if one traps the GPE, as that looks a lot more like what one gets in a modern thinkpad instead of what one gets from the NVRAM (look at the nvram polling code in thinkpad-acpi to understand what I mean. Some not so pretty crap was required in there due to deal with loss of information in the VOLUME/MUTE handling...)
Heh. Yeah, that's true.
Quoted 2 lines
Anyway, if we add notifications for normal nodes, please add them for GPEs too.
I'll look into that. With luck it'd let us remove polling from some of the other laptop hotkey drivers too.
Signature · 1 line
Matthew Garrett | [email protected]
Quoted 4 lines · Matthew Garrett
Various bits of hardware (some Thinkpads, for example) will execute ACPI methods in response to events but won't generate any notifications. It would be nice to be able to receive notifications on those updates in order to avoid polling. How about something like the following?
Very good idea. From the description the notification can be received when some ACPI methods are executed.
But it touches the ACPICA source code. Maybe it is uneasy to accept. And I have a question about this.
Asynchronous or synchronous notification event? It seems that
synchronous notification event is used in the following. If there is too much to do in the notification function, maybe some mutex will be blocked for too much time, which will have impact on the ACPI performance.
Thanks.
Quoted 158 lines
drivers/acpi/events/evxface.c +72
-0@@ -549,6 +549,78 @@ ACPI_EXPORT_SYMBOL(acpi_remove_notify_handler)/******************************************************************************* * +* FUNCTION: acpi_install_method_handler +* +* PARAMETERS: Oathname - The method for which notifies will be handled +* Handler - Address of the handler +* Context - Value passed to the handler on each call +* +* RETURN: Status +* +* DESCRIPTION: Install a handler for notifies on ACPI method execution +* +******************************************************************************/ + +acpi_status +acpi_install_method_handler (char *pathname,acpi_method_handler handler, +void *context) +{ +struct acpi_namespace_node *node; +union acpi_operand_object *method; + +acpi_ns_get_node (ACPI_NS_ALL , pathname,0 , &node); +if (!node) +return_ACPI_STATUS (AE_BAD_PARAMETER ); + + method =acpi_ns_get_attached_object (node); + +if (method->method.notify) +return_ACPI_STATUS (AE_ALREADY_EXISTS ); + + method->method.notify = handler; + method->method.context = context; + +return_ACPI_STATUS (AE_OK ); +} +ACPI_EXPORT_SYMBOL (acpi_install_method_handler) + +/******************************************************************************* +* +* FUNCTION: acpi_remove_method_handler +* +* PARAMETERS: Pathname - The method for which notifies will be handled +* Handler - Address of the handler +* +* RETURN: Status +* +* DESCRIPTION: Remove a handler for notifies on ACPI method execution +* +******************************************************************************/ + +acpi_status +acpi_remove_method_handler (char *pathname,acpi_method_handler handler) +{ +struct acpi_namespace_node *node; +union acpi_operand_object *method; + +acpi_ns_get_node (ACPI_NS_ALL , pathname,0 , &node); +if (!node) +return_ACPI_STATUS (AE_BAD_PARAMETER ); + + method =acpi_ns_get_attached_object (node); + +if (method->method.notify != handler) +return_ACPI_STATUS (AE_BAD_PARAMETER ); + + method->method.notify =NULL ; + method->method.context =NULL ; + +return_ACPI_STATUS (AE_OK ); +} +ACPI_EXPORT_SYMBOL (acpi_remove_method_handler) + +/******************************************************************************* +* * FUNCTION: acpi_install_gpe_handler * * PARAMETERS: gpe_device - Namespace node for the GPE (NULL for FADT drivers/acpi/parser/psparse.c +10
-0@@ -445,6 +445,7 @@ acpi_status acpi_ps_parse_aml(struct acpi_walk_state *walk_state)struct acpi_thread_state *thread;struct acpi_thread_state *prev_walk_list = acpi_gbl_current_walk_list;struct acpi_walk_state *previous_walk_state; +struct acpi_namespace_node *method_node;ACPI_FUNCTION_TRACE (ps_parse_aml);@@ -488,6 +489,15 @@ acpi_status acpi_ps_parse_aml(struct acpi_walk_state *walk_state) status =AE_OK ;while (walk_state) { + method_node = walk_state->method_call_node; + +if (method_node) { +union acpi_operand_object *method = +acpi_ns_get_attached_object (method_node); +if (method->method.notify) + method->method.notify (method->method.context); + } +if (ACPI_SUCCESS (status)) {/* * The parse_loop executes AML until the method terminates include/acpi/acobject.h +2
-0@@ -183,6 +183,8 @@ struct acpi_object_method {u32 aml_length;u8 thread_count;acpi_owner_id owner_id; +acpi_method_handler notify; +void *context; };/****************************************************************************** include/acpi/acpixf.h +7
-0@@ -212,6 +212,13 @@ acpi_remove_notify_handler(acpi_handle device, u32 handler_type, acpi_notify_handler handler);acpi_status +acpi_install_method_handler (char *pathname,acpi_method_handler handler, +void *context); + +acpi_status +acpi_remove_method_handler (char *pathname,acpi_method_handler handler); + +acpi_status acpi_install_address_space_handler(acpi_handle device, acpi_adr_space_type space_id, acpi_adr_space_handler handler,include/acpi/actypes.h +3
-0@@ -768,6 +768,9 @@ void (*acpi_notify_handler) (acpi_handle device, u32 value, void *context);typedef void (*acpi_object_handler ) (acpi_handle object,u32 function,void *data); +typedef +void (*acpi_method_handler ) (void *context); +typedef acpi_status (*acpi_init_handler ) (acpi_handle object,u32 function);#define ACPI_INIT_DEVICE_INI 1
Quoted 10 lines · Zhao Yakui
Quoted 4 lines · Matthew Garrett
Various bits of hardware (some Thinkpads, for example) will execute ACPI methods in response to events but won't generate any notifications. It would be nice to be able to receive notifications on those updates in order to avoid polling. How about something like the following?
Very good idea. From the description the notification can be received when some ACPI methods are executed.
But it touches the ACPICA source code. Maybe it is uneasy to accept. And I have a question about this.
Asynchronous or synchronous notification event? It seems that
I'd do fine with an asynchronous blocking or SCRU (blocking) notify chain for thinkpad-acpi. No need for synchronous operation, I'd not be using this API to hotpatch the AML or anything of the sort.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh
Quoted 12 lines · Henrique de Moraes Holschuh
Quoted 10 lines · Zhao Yakui
Quoted 4 lines · Matthew Garrett
Various bits of hardware (some Thinkpads, for example) will execute ACPI methods in response to events but won't generate any notifications. It would be nice to be able to receive notifications on those updates in order to avoid polling. How about something like the following?
Very good idea. From the description the notification can be received when some ACPI methods are executed.
But it touches the ACPICA source code. Maybe it is uneasy to accept. And I have a question about this.
Asynchronous or synchronous notification event? It seems that
Quoted 3 lines
I'd do fine with an asynchronous blocking or SCRU (blocking) notify chain for thinkpad-acpi. No need for synchronous operation, I'd not be using this API to hotpatch the AML or anything of the sort.
But I *really* do think you don't want a table-wide notify chain, something more fine-grained is better, or it will be a performance hog if too many drivers use it.
Signature · 4 lines
"One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh