| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 30 Sep 2009 22:12:55 +0400
From: Alexey Starikovskiy <aystarik@...il.com>
To: Ike Panhc <ike.pan@...onical.com>
CC: linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ACPI: Add EC event managerment functions into header
file
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 <ike.pan@...onical.com>
>>> ---
>>> 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)
>>>
>>>
>>>
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists