All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ACPI: Add EC event managerment functions into header file
@ 2009-09-28 21:30 Ike Panhc
  2009-09-28 22:16   ` Alexey Starikovskiy
  0 siblings, 1 reply; 7+ messages in thread
From: Ike Panhc @ 2009-09-28 21:30 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-kernel

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)
 
-- 
1.6.0.4


^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: Add EC event managerment functions into header file
  2009-09-28 21:30 [PATCH] ACPI: Add EC event managerment functions into header file Ike Panhc
@ 2009-09-28 22:16   ` Alexey Starikovskiy
  0 siblings, 0 replies; 7+ messages in thread
From: Alexey Starikovskiy @ 2009-09-28 22:16 UTC (permalink / raw)
  To: Ike Panhc; +Cc: linux-acpi, linux-kernel

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-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: Add EC event managerment functions into header file
@ 2009-09-28 22:16   ` Alexey Starikovskiy
  0 siblings, 0 replies; 7+ messages in thread
From: Alexey Starikovskiy @ 2009-09-28 22:16 UTC (permalink / raw)
  To: Ike Panhc; +Cc: linux-acpi, linux-kernel

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)
>  
>   


^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: Add EC event managerment functions into header file
  2009-09-28 22:16   ` Alexey Starikovskiy
@ 2009-09-29  2:28     ` Ike Panhc
  -1 siblings, 0 replies; 7+ messages in thread
From: Ike Panhc @ 2009-09-29  2:28 UTC (permalink / raw)
  To: Alexey Starikovskiy; +Cc: linux-acpi, linux-kernel

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 <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)
>>  
>>   
> 
> 


-- 
Ike Panhc <ike.pan@canonical.com>
Hardware Enable Team - Kernel Team - Platform Team - Canonical
Ubuntu - Linux for human beings | www.ubuntu.com | www.canonical.com
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: Add EC event managerment functions into header file
@ 2009-09-29  2:28     ` Ike Panhc
  0 siblings, 0 replies; 7+ messages in thread
From: Ike Panhc @ 2009-09-29  2:28 UTC (permalink / raw)
  To: Alexey Starikovskiy; +Cc: linux-acpi, linux-kernel

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 <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)
>>  
>>   
> 
> 


-- 
Ike Panhc <ike.pan@canonical.com>
Hardware Enable Team - Kernel Team - Platform Team - Canonical
Ubuntu - Linux for human beings | www.ubuntu.com | www.canonical.com

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: Add EC event managerment functions into header file
  2009-09-29  2:28     ` Ike Panhc
@ 2009-09-30 18:12       ` Alexey Starikovskiy
  -1 siblings, 0 replies; 7+ messages in thread
From: Alexey Starikovskiy @ 2009-09-30 18:12 UTC (permalink / raw)
  To: Ike Panhc; +Cc: linux-acpi, linux-kernel

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-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH] ACPI: Add EC event managerment functions into header file
@ 2009-09-30 18:12       ` Alexey Starikovskiy
  0 siblings, 0 replies; 7+ messages in thread
From: Alexey Starikovskiy @ 2009-09-30 18:12 UTC (permalink / raw)
  To: Ike Panhc; +Cc: linux-acpi, linux-kernel

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)
>>>  
>>>   
>>>       
>>     
>
>
>   


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-09-30 18:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-28 21:30 [PATCH] ACPI: Add EC event managerment functions into header file Ike Panhc
2009-09-28 22:16 ` Alexey Starikovskiy
2009-09-28 22:16   ` Alexey Starikovskiy
2009-09-29  2:28   ` Ike Panhc
2009-09-29  2:28     ` Ike Panhc
2009-09-30 18:12     ` Alexey Starikovskiy
2009-09-30 18:12       ` Alexey Starikovskiy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.