On 8/19/09, Corentin Chary <corentin.chary@xxxxxxxxx> wrote: > Hi, > Thanks for the review. > There is more documentation patchs in acpi4asus and asus-laptop > branches as well, maybe you can review them too ? <homer>Mmm, documentation Generic sysfs ABI docs: I assume you will CC Richard Purdie on the generic sysfs docs which include his address. The versions/dates don't look right - the backlight and lcd interfaces were introduced a long time before 2.6.29. I know the backlight device is well used. IMO it should go under "stable" instead of "testing". I don't know whether the other two deserve similar treatment. They're certainly simple enough that we can expect them to remain stable. The backlight doc could also use "Users: HAL". It would be nice to update Documentation/leds-class.txt at the same time, since it still says the maximum brightness value is fixed at 255. > + You can change triggers in a similar manner to the way an IO scheduler > + is chosen . Trigger specific parameters can appear in There's a stray space here, before the full stop on the second line. The backlight and led docs are well overdue, I think these will be very welcome. acpi4asus / asus-laptop: I wouldn't call /sys/devices/platform/asus-laptop/infos a sysfs abi :-P. I don't think it should be documented as such (but feel free to mention it in Documentation/laptops/asus-laptop.txt. "Mixing types, expressing multiple lines of data, and doing fancy formatting of data is heavily frowned upon. Doing these things may get you publically humiliated and your code rewritten without notice." I don't see any typos in the ABI. Other than that, no comment. > + This driver provides support for extra features of ACPI-compatible ASUS laptops. > + It may also support some MEDION, JVC or VICTOR laptops (such as MEDION 9675 or > + VICTOR XP7210 for example). It makes all the extra buttons generate standard > + ACPI events that go through /proc/acpi/events, and (on some models) adds support > + for changing the display brightness and output, switching the LCD backlight on > + and off, and most importantly, allows you to blink those fancy LEDs intended for > + reporting mail and wireless status. You mention ACPI events, perhaps you should mention input events here as well. > + This driver superseed the old asus_acpi driver. *supercedes > + Kernel 2.6.X sources, configured for you computer, with ACPI support. s/you/your/ > + - GPS enable and disable > + - video output switching > + - Ambient Light Sensor on and off s/video/Video > + That's all, now, all the events generated by the hotkeys of your laptop > + should be reported in your /proc/acpi/event entry. You can check it, if you > + are root, by doing a "cat /proc/acpi/event" and pressing those hotkeys, you > + should see them appear. You won't be able to open /proc/acpi/event if it is > + already opened by another program, so check if you haven't a daemon running > + (like acpid), before reporting a bug :) Hmm, most people do run acpid so this is not too helpful :). I would mention acpi_listen. > + You can modify LEDs be echoing values to /sys/class/leds/asus:*/brightness : > + echo 1 > /sys/class/leds/asus:mail/brightness asus::mail. There should be two colons, I think. > + will switch the mail LED on ... Just one full stop please. > + You can as well know if they are on/off by reading their content and use > + kernel triggers like ide-disk or heartbeat. s/as well/also/ > + wapf: WAPF defines the behavior of the Fn+Fx wlan key > + The significance of values is yet to be found, but > + most of the time: > + - 0x0 will do nothing > + - 0x1 will allow to control the device with Fn+Fx key. > + - 0x4 will send an ACPI event (0x88) while pressing the Fn+Fx key > + - 0x5 like 0x1 or 0x4 It would be good to know what the default value is. Regards Alan -- 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
