There are two functions for add/remove EC query handler functions, which
exported without any definition in header files
Patch against current checkout of linux-acpi 2.6.31 is below.
In this patch, the following definitions has been added into
include/linux/acpi.h
- typedef: acpi_ec_query_func
- struct: acpi_ec
- fucntions: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
And the following definitions has been removed from driver/acpi/ec.c
- typedef: acpi_ec_query_func
- struct: acpi_ec
So that, the following externs and typedef could be remove from
drivers/acpi/sbshc.c
- typedef: acpi_ec_query_func
- externs: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
Signed-off-by: Ike Panhc <[email protected]>
---
drivers/acpi/ec.c | 22 +++++-----------------
drivers/acpi/sbshc.c | 8 --------
include/linux/acpi.h | 21 +++++++++++++++++++++
3 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index f707960..8950c4f 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -43,6 +43,7 @@
#include <acpi/acpi_bus.h>
#include <acpi/acpi_drivers.h>
#include <linux/dmi.h>
+#include <linux/acpi.h>
#define ACPI_EC_CLASS "embedded_controller"
#define ACPI_EC_DEVICE_NAME "Embedded Controller"
@@ -80,10 +81,6 @@ enum {
* OpReg are installed */
};
-/* If we find an EC via the ECDT, we need to keep a ptr to its context */
-/* External interfaces use first EC only, so remember */
-typedef int (*acpi_ec_query_func) (void *data);
-
struct acpi_ec_query_handler {
struct list_head node;
acpi_ec_query_func func;
@@ -104,19 +101,10 @@ struct transaction {
bool done;
};
-static struct acpi_ec {
- acpi_handle handle;
- unsigned long gpe;
- unsigned long command_addr;
- unsigned long data_addr;
- unsigned long global_lock;
- unsigned long flags;
- struct mutex lock;
- wait_queue_head_t wait;
- struct list_head list;
- struct transaction *curr;
- spinlock_t curr_lock;
-} *boot_ec, *first_ec;
+/* If we find an EC via the ECDT, we need to keep a ptr to its context */
+/* External interfaces use first EC only, so remember */
+static struct acpi_ec *boot_ec;
+static struct acpi_ec *first_ec;
static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index d933980..d5550a5 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -250,12 +250,6 @@ static int smbus_alarm(void *context)
return 0;
}
-typedef int (*acpi_ec_query_func) (void *data);
-
-extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
- acpi_handle handle, acpi_ec_query_func func,
- void *data);
-
static int acpi_smbus_hc_add(struct acpi_device *device)
{
int status;
@@ -292,8 +286,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
return 0;
}
-extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
-
static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
{
struct acpi_smb_hc *hc;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dfcd920..259eacb 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -145,12 +145,33 @@ struct acpi_pci_driver {
int acpi_pci_register_driver(struct acpi_pci_driver *driver);
void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
+typedef int (*acpi_ec_query_func) (void *data);
+
+struct acpi_ec {
+ acpi_handle handle;
+ unsigned long gpe;
+ unsigned long command_addr;
+ unsigned long data_addr;
+ unsigned long global_lock;
+ unsigned long flags;
+ struct mutex lock;
+ wait_queue_head_t wait;
+ struct list_head list;
+ struct transaction *curr;
+ spinlock_t curr_lock;
+};
+
extern int ec_read(u8 addr, u8 *val);
extern int ec_write(u8 addr, u8 val);
extern int ec_transaction(u8 command,
const u8 *wdata, unsigned wdata_len,
u8 *rdata, unsigned rdata_len,
int force_poll);
+extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
+ acpi_handle handle,
+ acpi_ec_query_func func, void *data);
+extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
+
#if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
--
1.6.0.4
These functions are never used anywhere, but sbshc.c.
What is the reason to make them known to the whole kernel?
Ike Panhc пишет:
> There are two functions for add/remove EC query handler functions, which
> exported without any definition in header files
>
> Patch against current checkout of linux-acpi 2.6.31 is below.
>
> In this patch, the following definitions has been added into
> include/linux/acpi.h
> - typedef: acpi_ec_query_func
> - struct: acpi_ec
> - fucntions: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
>
> And the following definitions has been removed from driver/acpi/ec.c
> - typedef: acpi_ec_query_func
> - struct: acpi_ec
>
> So that, the following externs and typedef could be remove from
> drivers/acpi/sbshc.c
> - typedef: acpi_ec_query_func
> - externs: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
>
> Signed-off-by: Ike Panhc <[email protected]>
> ---
> drivers/acpi/ec.c | 22 +++++-----------------
> drivers/acpi/sbshc.c | 8 --------
> include/linux/acpi.h | 21 +++++++++++++++++++++
> 3 files changed, 26 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index f707960..8950c4f 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -43,6 +43,7 @@
> #include <acpi/acpi_bus.h>
> #include <acpi/acpi_drivers.h>
> #include <linux/dmi.h>
> +#include <linux/acpi.h>
>
> #define ACPI_EC_CLASS "embedded_controller"
> #define ACPI_EC_DEVICE_NAME "Embedded Controller"
> @@ -80,10 +81,6 @@ enum {
> * OpReg are installed */
> };
>
> -/* If we find an EC via the ECDT, we need to keep a ptr to its context */
> -/* External interfaces use first EC only, so remember */
> -typedef int (*acpi_ec_query_func) (void *data);
> -
> struct acpi_ec_query_handler {
> struct list_head node;
> acpi_ec_query_func func;
> @@ -104,19 +101,10 @@ struct transaction {
> bool done;
> };
>
> -static struct acpi_ec {
> - acpi_handle handle;
> - unsigned long gpe;
> - unsigned long command_addr;
> - unsigned long data_addr;
> - unsigned long global_lock;
> - unsigned long flags;
> - struct mutex lock;
> - wait_queue_head_t wait;
> - struct list_head list;
> - struct transaction *curr;
> - spinlock_t curr_lock;
> -} *boot_ec, *first_ec;
> +/* If we find an EC via the ECDT, we need to keep a ptr to its context */
> +/* External interfaces use first EC only, so remember */
> +static struct acpi_ec *boot_ec;
> +static struct acpi_ec *first_ec;
>
> static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
>
> diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
> index d933980..d5550a5 100644
> --- a/drivers/acpi/sbshc.c
> +++ b/drivers/acpi/sbshc.c
> @@ -250,12 +250,6 @@ static int smbus_alarm(void *context)
> return 0;
> }
>
> -typedef int (*acpi_ec_query_func) (void *data);
> -
> -extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> - acpi_handle handle, acpi_ec_query_func func,
> - void *data);
> -
> static int acpi_smbus_hc_add(struct acpi_device *device)
> {
> int status;
> @@ -292,8 +286,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
> return 0;
> }
>
> -extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
> -
> static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
> {
> struct acpi_smb_hc *hc;
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dfcd920..259eacb 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -145,12 +145,33 @@ struct acpi_pci_driver {
> int acpi_pci_register_driver(struct acpi_pci_driver *driver);
> void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
>
> +typedef int (*acpi_ec_query_func) (void *data);
> +
> +struct acpi_ec {
> + acpi_handle handle;
> + unsigned long gpe;
> + unsigned long command_addr;
> + unsigned long data_addr;
> + unsigned long global_lock;
> + unsigned long flags;
> + struct mutex lock;
> + wait_queue_head_t wait;
> + struct list_head list;
> + struct transaction *curr;
> + spinlock_t curr_lock;
> +};
> +
> extern int ec_read(u8 addr, u8 *val);
> extern int ec_write(u8 addr, u8 val);
> extern int ec_transaction(u8 command,
> const u8 *wdata, unsigned wdata_len,
> u8 *rdata, unsigned rdata_len,
> int force_poll);
> +extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> + acpi_handle handle,
> + acpi_ec_query_func func, void *data);
> +extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
> +
>
> #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
>
>
I had sent a new driver for Lenovo SL series laptops [1], and it uses those
functions to detect hotkeys. Since it has been exported for other modules,
I think it is reasonable to write in header file to make sure everyone has
the same definition.
[1] http://patchwork.kernel.org/patch/49912/
Alexey Starikovskiy wrote:
> These functions are never used anywhere, but sbshc.c.
> What is the reason to make them known to the whole kernel?
>
> Ike Panhc пишет:
>> There are two functions for add/remove EC query handler functions, which
>> exported without any definition in header files
>>
>> Patch against current checkout of linux-acpi 2.6.31 is below.
>>
>> In this patch, the following definitions has been added into
>> include/linux/acpi.h
>> - typedef: acpi_ec_query_func
>> - struct: acpi_ec
>> - fucntions: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
>>
>> And the following definitions has been removed from driver/acpi/ec.c
>> - typedef: acpi_ec_query_func
>> - struct: acpi_ec
>>
>> So that, the following externs and typedef could be remove from
>> drivers/acpi/sbshc.c
>> - typedef: acpi_ec_query_func
>> - externs: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
>>
>> Signed-off-by: Ike Panhc <[email protected]>
>> ---
>> drivers/acpi/ec.c | 22 +++++-----------------
>> drivers/acpi/sbshc.c | 8 --------
>> include/linux/acpi.h | 21 +++++++++++++++++++++
>> 3 files changed, 26 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
>> index f707960..8950c4f 100644
>> --- a/drivers/acpi/ec.c
>> +++ b/drivers/acpi/ec.c
>> @@ -43,6 +43,7 @@
>> #include <acpi/acpi_bus.h>
>> #include <acpi/acpi_drivers.h>
>> #include <linux/dmi.h>
>> +#include <linux/acpi.h>
>>
>> #define ACPI_EC_CLASS "embedded_controller"
>> #define ACPI_EC_DEVICE_NAME "Embedded Controller"
>> @@ -80,10 +81,6 @@ enum {
>> * OpReg are installed */
>> };
>>
>> -/* If we find an EC via the ECDT, we need to keep a ptr to its
>> context */
>> -/* External interfaces use first EC only, so remember */
>> -typedef int (*acpi_ec_query_func) (void *data);
>> -
>> struct acpi_ec_query_handler {
>> struct list_head node;
>> acpi_ec_query_func func;
>> @@ -104,19 +101,10 @@ struct transaction {
>> bool done;
>> };
>>
>> -static struct acpi_ec {
>> - acpi_handle handle;
>> - unsigned long gpe;
>> - unsigned long command_addr;
>> - unsigned long data_addr;
>> - unsigned long global_lock;
>> - unsigned long flags;
>> - struct mutex lock;
>> - wait_queue_head_t wait;
>> - struct list_head list;
>> - struct transaction *curr;
>> - spinlock_t curr_lock;
>> -} *boot_ec, *first_ec;
>> +/* If we find an EC via the ECDT, we need to keep a ptr to its
>> context */
>> +/* External interfaces use first EC only, so remember */
>> +static struct acpi_ec *boot_ec;
>> +static struct acpi_ec *first_ec;
>>
>> static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
>>
>> diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
>> index d933980..d5550a5 100644
>> --- a/drivers/acpi/sbshc.c
>> +++ b/drivers/acpi/sbshc.c
>> @@ -250,12 +250,6 @@ static int smbus_alarm(void *context)
>> return 0;
>> }
>>
>> -typedef int (*acpi_ec_query_func) (void *data);
>> -
>> -extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
>> - acpi_handle handle, acpi_ec_query_func func,
>> - void *data);
>> -
>> static int acpi_smbus_hc_add(struct acpi_device *device)
>> {
>> int status;
>> @@ -292,8 +286,6 @@ static int acpi_smbus_hc_add(struct acpi_device
>> *device)
>> return 0;
>> }
>>
>> -extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8
>> query_bit);
>> -
>> static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
>> {
>> struct acpi_smb_hc *hc;
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index dfcd920..259eacb 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -145,12 +145,33 @@ struct acpi_pci_driver {
>> int acpi_pci_register_driver(struct acpi_pci_driver *driver);
>> void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
>>
>> +typedef int (*acpi_ec_query_func) (void *data);
>> +
>> +struct acpi_ec {
>> + acpi_handle handle;
>> + unsigned long gpe;
>> + unsigned long command_addr;
>> + unsigned long data_addr;
>> + unsigned long global_lock;
>> + unsigned long flags;
>> + struct mutex lock;
>> + wait_queue_head_t wait;
>> + struct list_head list;
>> + struct transaction *curr;
>> + spinlock_t curr_lock;
>> +};
>> +
>> extern int ec_read(u8 addr, u8 *val);
>> extern int ec_write(u8 addr, u8 val);
>> extern int ec_transaction(u8 command,
>> const u8 *wdata, unsigned wdata_len,
>> u8 *rdata, unsigned rdata_len,
>> int force_poll);
>> +extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
>> + acpi_handle handle,
>> + acpi_ec_query_func func, void *data);
>> +extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8
>> query_bit);
>> +
>>
>> #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
>>
>>
>
>
--
Ike Panhc <[email protected]>
Hardware Enable Team - Kernel Team - Platform Team - Canonical
Ubuntu - Linux for human beings | http://www.ubuntu.com | http://www.canonical.com
Ike Panhc пишет:
> I had sent a new driver for Lenovo SL series laptops [1], and it uses those
> functions to detect hotkeys. Since it has been exported for other modules,
> I think it is reasonable to write in header file to make sure everyone has
> the same definition.
>
> [1] http://patchwork.kernel.org/patch/49912/
>
If you need these functions to be visible, you should mention that in
patch description.
You should also add a comment that if someone adds notification handler,
the original one
is not going to be executed until handler is un-registered. This was not
needed while it was internal interface,
but with it becoming public, it needs some warnings/descriptions.
Regards,
Alex.
BTW, what is wrong with default hotkey handlers?
> Alexey Starikovskiy wrote:
>
>> These functions are never used anywhere, but sbshc.c.
>> What is the reason to make them known to the whole kernel?
>>
>> Ike Panhc пишет:
>>
>>> There are two functions for add/remove EC query handler functions, which
>>> exported without any definition in header files
>>>
>>> Patch against current checkout of linux-acpi 2.6.31 is below.
>>>
>>> In this patch, the following definitions has been added into
>>> include/linux/acpi.h
>>> - typedef: acpi_ec_query_func
>>> - struct: acpi_ec
>>> - fucntions: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
>>>
>>> And the following definitions has been removed from driver/acpi/ec.c
>>> - typedef: acpi_ec_query_func
>>> - struct: acpi_ec
>>>
>>> So that, the following externs and typedef could be remove from
>>> drivers/acpi/sbshc.c
>>> - typedef: acpi_ec_query_func
>>> - externs: acpi_ec_add_query_handler, acpi_ec_remove_query_handler
>>>
>>> Signed-off-by: Ike Panhc <[email protected]>
>>> ---
>>> drivers/acpi/ec.c | 22 +++++-----------------
>>> drivers/acpi/sbshc.c | 8 --------
>>> include/linux/acpi.h | 21 +++++++++++++++++++++
>>> 3 files changed, 26 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
>>> index f707960..8950c4f 100644
>>> --- a/drivers/acpi/ec.c
>>> +++ b/drivers/acpi/ec.c
>>> @@ -43,6 +43,7 @@
>>> #include <acpi/acpi_bus.h>
>>> #include <acpi/acpi_drivers.h>
>>> #include <linux/dmi.h>
>>> +#include <linux/acpi.h>
>>>
>>> #define ACPI_EC_CLASS "embedded_controller"
>>> #define ACPI_EC_DEVICE_NAME "Embedded Controller"
>>> @@ -80,10 +81,6 @@ enum {
>>> * OpReg are installed */
>>> };
>>>
>>> -/* If we find an EC via the ECDT, we need to keep a ptr to its
>>> context */
>>> -/* External interfaces use first EC only, so remember */
>>> -typedef int (*acpi_ec_query_func) (void *data);
>>> -
>>> struct acpi_ec_query_handler {
>>> struct list_head node;
>>> acpi_ec_query_func func;
>>> @@ -104,19 +101,10 @@ struct transaction {
>>> bool done;
>>> };
>>>
>>> -static struct acpi_ec {
>>> - acpi_handle handle;
>>> - unsigned long gpe;
>>> - unsigned long command_addr;
>>> - unsigned long data_addr;
>>> - unsigned long global_lock;
>>> - unsigned long flags;
>>> - struct mutex lock;
>>> - wait_queue_head_t wait;
>>> - struct list_head list;
>>> - struct transaction *curr;
>>> - spinlock_t curr_lock;
>>> -} *boot_ec, *first_ec;
>>> +/* If we find an EC via the ECDT, we need to keep a ptr to its
>>> context */
>>> +/* External interfaces use first EC only, so remember */
>>> +static struct acpi_ec *boot_ec;
>>> +static struct acpi_ec *first_ec;
>>>
>>> static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
>>>
>>> diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
>>> index d933980..d5550a5 100644
>>> --- a/drivers/acpi/sbshc.c
>>> +++ b/drivers/acpi/sbshc.c
>>> @@ -250,12 +250,6 @@ static int smbus_alarm(void *context)
>>> return 0;
>>> }
>>>
>>> -typedef int (*acpi_ec_query_func) (void *data);
>>> -
>>> -extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
>>> - acpi_handle handle, acpi_ec_query_func func,
>>> - void *data);
>>> -
>>> static int acpi_smbus_hc_add(struct acpi_device *device)
>>> {
>>> int status;
>>> @@ -292,8 +286,6 @@ static int acpi_smbus_hc_add(struct acpi_device
>>> *device)
>>> return 0;
>>> }
>>>
>>> -extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8
>>> query_bit);
>>> -
>>> static int acpi_smbus_hc_remove(struct acpi_device *device, int type)
>>> {
>>> struct acpi_smb_hc *hc;
>>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>>> index dfcd920..259eacb 100644
>>> --- a/include/linux/acpi.h
>>> +++ b/include/linux/acpi.h
>>> @@ -145,12 +145,33 @@ struct acpi_pci_driver {
>>> int acpi_pci_register_driver(struct acpi_pci_driver *driver);
>>> void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
>>>
>>> +typedef int (*acpi_ec_query_func) (void *data);
>>> +
>>> +struct acpi_ec {
>>> + acpi_handle handle;
>>> + unsigned long gpe;
>>> + unsigned long command_addr;
>>> + unsigned long data_addr;
>>> + unsigned long global_lock;
>>> + unsigned long flags;
>>> + struct mutex lock;
>>> + wait_queue_head_t wait;
>>> + struct list_head list;
>>> + struct transaction *curr;
>>> + spinlock_t curr_lock;
>>> +};
>>> +
>>> extern int ec_read(u8 addr, u8 *val);
>>> extern int ec_write(u8 addr, u8 val);
>>> extern int ec_transaction(u8 command,
>>> const u8 *wdata, unsigned wdata_len,
>>> u8 *rdata, unsigned rdata_len,
>>> int force_poll);
>>> +extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
>>> + acpi_handle handle,
>>> + acpi_ec_query_func func, void *data);
>>> +extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8
>>> query_bit);
>>> +
>>>
>>> #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
>>>
>>>
>>>
>>
>
>
>