Re: [PATCH] ACER: Add support for accelerometer sensor

[Marek Vasut <marex@denx.de>]

Dear joeyli,

> Hi Marek,
> 
> 於 一，2012-05-28 於 01:44 +0200，Marek Vasut 提到：
> 
> > This device is present on Iconia Tab W500.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: joeyli <jlee@suse.com>
> > ---
> > 
> >  drivers/platform/x86/acer-wmi.c |  110
> >  +++++++++++++++++++++++++++++++++++++++ 1 file changed, 110
> >  insertions(+)
> > 
> > diff --git a/drivers/platform/x86/acer-wmi.c
> > b/drivers/platform/x86/acer-wmi.c index c1a3fd8..2df0673 100644
> > --- a/drivers/platform/x86/acer-wmi.c
> > +++ b/drivers/platform/x86/acer-wmi.c
> > @@ -95,6 +95,7 @@
> > MODULE_ALIAS("wmi:676AA15E-6A47-4D9F-A2CC-1E6D18D14026");
> > 
> >  enum acer_wmi_event_ids {
> >  
> >  	WMID_HOTKEY_EVENT = 0x1,
> > 
> > +	WMID_ACCEL_EVENT = 0x5,
> > 
> >  };
> >  
> >  static const struct key_entry acer_wmi_keymap[] = {
> > 
> > @@ -130,6 +131,7 @@ static const struct key_entry acer_wmi_keymap[] = {
> > 
> >  };
> >  
> >  static struct input_dev *acer_wmi_input_dev;
> > 
> > +static struct input_dev *acer_wmi_accel_dev;
> > 
> >  struct event_return_value {
> >  
> >  	u8 function;
> > 
> > @@ -145,6 +147,7 @@ struct event_return_value {
> > 
> >  #define ACER_WMID3_GDS_THREEG		(1<<6)	/* 3G */
> >  #define ACER_WMID3_GDS_WIMAX		(1<<7)	/* WiMAX */
> >  #define ACER_WMID3_GDS_BLUETOOTH	(1<<11)	/* BT */
> > 
> > +#define ACER_WMID3_GDS_ACCEL		(1<<2)	/* Accelerometer */
> 
> Please kindly share your dmidecode information:
>  # dmidecode > dmidecode.log

Will do.

> How did you know '1<<2' reflects to G-sensor? As I know this is a
> reserved bit did not use before.

Honestly? Wild guess.

> >  struct lm_input_params {
> >  
> >  	u8 function_num;        /* Function Number */
> > 
> > @@ -200,6 +203,7 @@ struct hotkey_function_type_aa {
> > 
> >  #define ACER_CAP_BLUETOOTH		(1<<2)
> >  #define ACER_CAP_BRIGHTNESS		(1<<3)
> >  #define ACER_CAP_THREEG			(1<<4)
> > 
> > +#define ACER_CAP_ACCEL			(1<<5)
> > 
> >  #define ACER_CAP_ANY			(0xFFFFFFFF)
> >  
> >  /*
> > 
> > @@ -1150,6 +1154,8 @@ static void type_aa_dmi_decode(const struct
> > dmi_header *header, void *dummy)
> > 
> >  		interface->capability |= ACER_CAP_THREEG;
> >  	
> >  	if (type_aa->commun_func_bitmap & ACER_WMID3_GDS_BLUETOOTH)
> >  	
> >  		interface->capability |= ACER_CAP_BLUETOOTH;
> > 
> > +	if (type_aa->others_func_bitmap & ACER_WMID3_GDS_ACCEL)
> > +		interface->capability |= ACER_CAP_ACCEL;
> > 
> >  	commun_fn_key_number = type_aa->commun_fn_key_number;
> >  
> >  }
> > 
> > @@ -1375,6 +1381,61 @@ static void acer_backlight_exit(void)
> > 
> >  }
> >  
> >  /*
> > 
> > + * Accelerometer device
> > + */
> > +#define ACER_GSENSOR_HANDLE	"\\_SB.PCI0.LPC0.SENR"
> > +#define ACER_GSENSOR_INIT	"\\_SB.PCI0.LPC0.SENR._INI"
> > +#define ACER_GSENSOR_RDVL	"\\_SB.PCI0.LPC0.SENR.RDVL"
> 
> I think that will be better don't hard code the path of device in
> driver, because different models have different path naming.

Certainly, how should I do it the correct way please?

> IMHO, please reference thinkpad_acpi driver like the following:
> 
> vi drivers/platform/x86/thinkpad_acpi.c
> 
> #define TPACPI_ACPI_EC_HID              "PNP0C09"
> ...
> static acpi_handle ec_handle;
> ...
> 
> static int __init probe_for_thinkpad(void)
> {
> ...
>         /* The EC handler is required */
>         tpacpi_acpi_handle_locate("ec", TPACPI_ACPI_EC_HID, &ec_handle);
> 
> static void __init tpacpi_acpi_handle_locate(const char *name,
> {
> ...
>         memset(&device_found, 0, sizeof(device_found));
>         status = acpi_get_devices(hid, tpacpi_acpi_handle_locate_callback,
>                                   (void *)name, &device_found);

I see! Will do (incl. the other driver) Thanks for your guidance here, really, 
thanks a lot!

> > @@ -2118,6 +2225,9 @@ static void __exit acer_wmi_exit(void)
> > 
> >  	if (wmi_has_guid(ACERWMID_EVENT_GUID))
> >  	
> >  		acer_wmi_input_destroy();
> > 
> > +	if (has_cap(ACER_CAP_ACCEL))
> > +		acer_wmi_accel_destroy();
> > +
> > 
> >  	remove_sysfs(acer_platform_device);
> >  	remove_debugfs();
> >  	platform_device_unregister(acer_platform_device);
> 
> other stuff looks good to me.

Thanks. Btw I'll update the CC list in V2.

> 
> Thanks a lot!
> Joey Lee