Re: [PATCH] Documentation: Add entry for dell-laptop sysfs interface

[Gabriele Mazzotta]

On Tuesday 30 December 2014 21:31:49 Brian Norris wrote:
> Hi,
> 
> On Wed, Dec 03, 2014 at 06:41:33PM +0100, Gabriele Mazzotta wrote:
> > Add the documentation for the new sysfs interface of dell-laptop
> > that allows to configure the keyboard illumination on Dell systems.
> > 
> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@xxxxxxxxx>
> > Signed-off-by: Pali Rohár <pali.rohar@xxxxxxxxx>
> > ---
> >  .../ABI/testing/sysfs-platform-dell-laptop         | 60 ++++++++++++++++++++++
> >  1 file changed, 60 insertions(+)
> >  create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-laptop
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-platform-dell-laptop b/Documentation/ABI/testing/sysfs-platform-dell-laptop
> > new file mode 100644
> > index 0000000..7969443
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/sysfs-platform-dell-laptop
> > @@ -0,0 +1,60 @@
> > +What:		/sys/class/leds/dell::kbd_backlight/als_setting
> > +Date:		December 2014
> > +KernelVersion:	3.19
> > +Contact:	Gabriele Mazzotta <gabriele.mzt@xxxxxxxxx>,
> > +		Pali Rohár <pali.rohar@xxxxxxxxx>
> > +Description:
> > +		This file allows to control the automatic keyboard
> > +		illumination mode on some systems that have an ambient
> > +		light sensor. Write 1 to this file to enable the auto
> > +		mode, 0 to disable it.
> [...]
> 
> This entry appears wrong, or at least incomplete. My system boots with a
> default value of 18, and the 'set' implementation accepts any value from
> 0 to 255.
> 
> According to the comments in dell-laptop.c, the value actually means
> something different than on/off:
> 
>   cbArg3, byte0  Desired setting of ALS value that turns the light on or off.
> 
> Admittedly, I'm not clear on what the intended interface is, as I've
> never had the ALS / keyboard backlight controls all working properly on
> my laptop in the first place, but I thought this would be the right
> place to ask about getting the documentation and driver in sync.

Hi,

You are perfectly right, the documentation is wrong and I'm sorry for
that. als_setting should allow you to define when the keyboard
backlight has to be turned on or off depending on the value reported
by the ambient light sensor.

Please note that not everything that is in libsmbios [1] is implemented
in the kernel driver. Take a look at it, recently there have been few
changes related to the ALS settings. Probably they are not needed given
that you are able to set the threshold with the current kernel driver,
but you might find something interesting.

I'll submit a patch to correct the documentation, thanks for reporting.

[1] http://linux.dell.com/git/libsmbios.git

> NB: I came across this because
> 
>    (1) I'd really like to be able to turn the keyboard backlight on, the
>    timeout off, and the ALS off; and
> 
>    (2) I noticed there was a new driver patch in 3.19 (nice! thanks!),
>    where previously there must have been some kind of half-functional
>    working driver -- I could get backlight control, but it always timed
>    out too quickly.
> 
> It seems like /sys/.../als_setting is documented to handle the latter
> part of (1), but its implementation does not match.
> 
> Thanks,
> Brian
> 
> P.S. The author of this page [1] might be interested in these new driver
> additions.
> 
> [1] http://www.cs.bham.ac.uk/~axs/laptop/dell-e6410-fn-keys.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/