On Tue, 2009-08-04 at 09:12 +0800, Zhao, Yakui wrote: > On Mon, 2009-08-03 at 17:10 +0800, Zhang Rui wrote: > > Hi, all, > > > > This is the patch set I made to introduce ACPI ALS device driver > > and a generic sysfs I/F for all the ALS devices, like ACPI ALS, > > platform ALS, etc. > > > > Patch 01 introduces the ACPI ALS device driver. > Great jobs. > But I still have two questions about the patch set. > 1. _ALP polling method > This optional object returns the recommended polling frequency for > the ambient sensor. And OSPM can use the recommended polling frequency > to poll the ambient sensor. > Of course the spec 9.2.6 also says that the use of polling is allowed > but strongly discouraged by this specification. In such case we had > better not use the polling frequency. Instead it will totally depend on > the async notification event. > Can some message be printed that the polling frequency is depreciated > when the _ALP is not zero? In fact, currently the driver just follows ACPI spec to support _ALP method for ACPI ALS device. But _ALP return value is not used at all. As we don't have any platform that _ALP is actually implemented, why not leave the code there for now. And a warning message is reasonable. > 2. what is the ALS policy and how to use it? > The main purpose of ambient sensor is to adjust the brightness > according to the current luminance environment. From this patch set it > seems that this will be done in user space. But it is not very clear how > to adjust the brightness based on the current luminance. > Can we describe it more clearly? > Generally, in order to make ALS work, user space needs to: 1. set an backlight value as the base point, i.e. the display brightness level when the display luminance adjustment value is 100%. 2. when the ambient light illuminance changes, find a proper element in /sys/class/als/als0/mappings, and get the display adjustment value. 3. use this adjustment value to get the proper display brightness level and set it via the backlight sysfs I/F. I'll add the ALS documentation to describe how to use this sysfs I/F, if the current approach is accepted. :) > > Another little issue is the memory leak in the patch 1. > When it receives the notification event(0x82), it will re-evaluate the > mapping between brightness and luminance. The memory space occupied by > previous mapping should be freed. > good catch. refreshed patch has been sent out. thanks, rui -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
