Re: [PATCH 2/2] ACER: Add support for ambient light sensor

[Marek Vasut <marex@denx.de>]

Dear joeyli,

> Hi Marek,
> 
> 於 五，2012-06-01 於 19:11 +0200，Marek Vasut 提到：
> 
> > This is the ambient light sensor found on Iconia W500.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: joeyli <jlee@suse.com>
> > ---
> > 
> >  drivers/platform/x86/acer-wmi.c |   86
> >  +++++++++++++++++++++++++++++++++++++++ 1 file changed, 86
> >  insertions(+)
> > 
> > diff --git a/drivers/platform/x86/acer-wmi.c
> > b/drivers/platform/x86/acer-wmi.c index edb6bad..a58c415 100644
> > --- a/drivers/platform/x86/acer-wmi.c
> > +++ b/drivers/platform/x86/acer-wmi.c
> > @@ -203,6 +203,7 @@ struct hotkey_function_type_aa {
> > 
> >  #define ACER_CAP_BRIGHTNESS		(1<<3)
> >  #define ACER_CAP_THREEG			(1<<4)
> >  #define ACER_CAP_ACCEL			(1<<5)
> > 
> > +#define ACER_CAP_ALS			(1<<6)
> > 
> >  #define ACER_CAP_ANY			(0xFFFFFFFF)
> >  
> >  /*
> > 
> > @@ -1378,6 +1379,63 @@ static void acer_backlight_exit(void)
> > 
> >  }
> >  
> >  /*
> > 
> > + * Ambient light sensor device
> > + */
> > +static acpi_handle alsd_handle;
> > +
> > +static int acer_als_init(void)
> > +{
> > +	union acpi_object in_obj;
> > +	struct acpi_object_list params;
> > +	struct acpi_buffer out;
> > +	union acpi_object out_obj;
> > +	acpi_status status;
> > +
> > +	status = acpi_evaluate_object(alsd_handle, "_INI", NULL, NULL);
> > +	if (ACPI_FAILURE(status))
> > +		return -1;
> > +
> > +	params.count = 1;
> > +	params.pointer = &in_obj;
> > +	in_obj.type = ACPI_TYPE_INTEGER;
> > +	/*
> > +	 * Unknown argument of both GLOV and GUPV, set to 5.
> > +	 * GLOV - G? Lower Value ?
> > +	 * GUPV - G? Upper Value ?
> > +	 */
> > +	in_obj.integer.value = 5;
> > +	out.length = sizeof(out_obj);
> > +	out.pointer = &out_obj;
> > +
> > +	acpi_evaluate_object(alsd_handle, "GLOV", &params, &out);
> > +	acpi_evaluate_object(alsd_handle, "GUPV", &params, &out);
> > +
> 
> GUPV/GLOV also call by _INI, why need evaluate it again?

What does _INI set them to though?

>             Method (_INI, 0, NotSerialized)
>             {
> ...
>                 Store (GUPV (Local1), Local2)
>                 Store (GLOV (Local1), Local3)
> 
> And, why use '5' to evaluate GUPV/GLOV?

It seemed like the value that worked the best. I think those packages defined at 
the begining of the ALSD section define some kind of profiles (maybe, cloudy, 
bright light etc). And this selects the profile.

> 
> > +	return 0;
> > +}
> > +
> > +static ssize_t acer_als_show(struct device *dev, struct device_attribute
> > *attr, +			char *buf)
> > +{
> > +	acpi_status status;
> > +	struct acpi_buffer output;
> > +	union acpi_object out_obj;
> > +
> > +	status = acpi_evaluate_object(alsd_handle, "S3WK", NULL, NULL);
> > +	if (ACPI_FAILURE(status))
> > +		return -1;
> > +
> 
> Why need evaluate S3WK before _ALI ? Looks like this method call in S3
> resume path:

It makes the sensor re-read the data and it's faster than calling _INI.

>  _WAK -> OEMW -> PRJW -> S3WK
> 
> > +	output.length = sizeof(out_obj);
> > +	output.pointer = &out_obj;
> > +	status = acpi_evaluate_object(alsd_handle, "_ALI", NULL, &output);
> > +	if (ACPI_FAILURE(status))
> > +		return -1;
> > +
> > +	return sprintf(buf, "%d\n", (u16)out_obj.integer.value);
> > +}
> > +
> > +static DEVICE_ATTR(ls_switch, S_IRUGO, acer_als_show, NULL);
> > +
> > +/*
> > 
> >   * Accelerometer device
> >   */
> >  
> >  static acpi_handle gsensor_handle;
> > 
> > @@ -1856,6 +1914,21 @@ err_free_dev:
> >  	return err;
> >  
> >  }
> > 
> > +static int __init acer_wmi_alsd_setup(void)
> > +{
> > +	int err;
> > +
> > +	err = acer_wmi_get_handle("ALSD", "ACPI0008", &alsd_handle);
> > +	if (err)
> > +		return err;
> > +
> 
> Does anybody know why "introduce ACPI ALS device driver" patches from
> Zhang Rui didn't include in acpi code base?
> 
> http://comments.gmane.org/gmane.linux.kernel/884328
> 
> 
> There have V6 patches, but didn't see those patches show up in acpi git.

Now this would be much cooler to have than this crappy hack of mine :-)

> Thanks a lot!
> Joey Lee
> 

Best regards,
Marek Vasut