On Wed, Aug 19, 2009 at 6:51 PM, Alan Jenkins<sourcejedi.lkml@xxxxxxxxxxxxxx> wrote: > 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 > Wow, thanks for the extensive review, I'll fix that when I'm back (next week). -- Corentin Chary http://xf.iksaif.net - http://uffs.org -- 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
