Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 30 Sep 2009 22:12:55 +0400 | From | Alexey Starikovskiy <> | 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@canonical.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |