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

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

[Darren Hart]

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@gmail.com>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

If nobody else would prefer to take this in their tree, I'll submit it through
platform-drivers-x86 when I send the corresponding driver patch.

-- 
Darren Hart
Intel Open Source Technology Center

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

[Gabriele Mazzotta]

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@gmail.com>
Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 .../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@gmail.com>,
+		Pali Rohár <pali.rohar@gmail.com>
+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.
+
+What:		/sys/class/leds/dell::kbd_backlight/start_triggers
+Date:		December 2014
+KernelVersion:	3.19
+Contact:	Gabriele Mazzotta <gabriele.mzt@gmail.com>,
+		Pali Rohár <pali.rohar@gmail.com>
+Description:
+		This file allows to control the input triggers that
+		turn on the keyboard backlight illumination that is
+		disabled because of inactivity.
+		Read the file to see the triggers available. The ones
+		enabled are preceded by '+', those disabled by '-'.
+
+		To enable a trigger, write its name preceded by '+' to
+		this file. To disable a trigger, write its name preceded
+		by '-' instead.
+
+		For example, to enable the keyboard as trigger run:
+		    echo +keyboard > /sys/class/leds/dell::kbd_backlight/start_triggers
+		To disable it:
+		    echo -keyboard > /sys/class/leds/dell::kbd_backlight/start_triggers
+
+		Note that not all the available triggers can be configured.
+
+What:		/sys/class/leds/dell::kbd_backlight/stop_timeout
+Date:		December 2014
+KernelVersion:	3.19
+Contact:	Gabriele Mazzotta <gabriele.mzt@gmail.com>,
+		Pali Rohár <pali.rohar@gmail.com>
+Description:
+		This file allows to specify the interval after which the
+		keyboard illumination is disabled because of inactivity.
+		The timeouts are expressed in seconds, minutes, hours and
+		days, for which the symbols are 's', 'm', 'h' and 'd'
+		respectively.
+
+		To configure the timeout, write to this file a value along
+		with any the above units. If no unit is specified, the value
+		is assumed to be expressed in seconds.
+
+		For example, to set the timeout to 10 minutes run:
+		    echo 10m > /sys/class/leds/dell::kbd_backlight/stop_timeout
+
+		Note that when this file is read, the returned value might be
+		expressed in a different unit than the one used when the timeout
+		was set.
+
+		Also note that only some timeouts are supported and that
+		some systems might fall back to a specific timeout in case
+		an invalid timeout is written to this file.
-- 
2.1.3

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

[Darren Hart]

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.

Queued to for-next.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

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

[Brian Norris]

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@gmail.com>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
>  .../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@gmail.com>,
> +		Pali Rohár <pali.rohar@gmail.com>
> +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.

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

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@gmail.com>
> > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > ---
> >  .../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@gmail.com>,
> > +		Pali Rohár <pali.rohar@gmail.com>
> > +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

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

[Darren Hart]

On Wed, Dec 31, 2014 at 12:20:59PM +0100, Gabriele Mazzotta wrote:
> 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@gmail.com>
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > ---
> > >  .../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@gmail.com>,
> > > +		Pali Rohár <pali.rohar@gmail.com>
> > > +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.

Thanks Gabriele, let's make sure to get this into this rc cycle, sooner the
better.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

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

[Darren Hart]

On Wed, Dec 31, 2014 at 12:20:59PM +0100, Gabriele Mazzotta wrote:
> 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@gmail.com>
> > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > ---
> > >  .../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@gmail.com>,
> > > +		Pali Rohár <pali.rohar@gmail.com>
> > > +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.

Hi Gabriele,

Have I missed a patch from you for this? I'd like to see this corrected in the
3.19 rc cycle.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

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

[Brian Norris]

On Fri, Jan 9, 2015 at 10:57 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Wed, Dec 31, 2014 at 12:20:59PM +0100, Gabriele Mazzotta wrote:
>> 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.
>
> Have I missed a patch from you for this? I'd like to see this corrected in the
> 3.19 rc cycle.

https://lkml.org/lkml/2015/1/2/63

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

[Gabriele Mazzotta]

On Friday 09 January 2015 22:57:14 Darren Hart wrote:
> On Wed, Dec 31, 2014 at 12:20:59PM +0100, Gabriele Mazzotta wrote:
> > 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@gmail.com>
> > > > Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> > > > ---
> > > >  .../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@gmail.com>,
> > > > +		Pali Rohár <pali.rohar@gmail.com>
> > > > +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.
> 
> Hi Gabriele,
> 
> Have I missed a patch from you for this? I'd like to see this corrected in the
> 3.19 rc cycle.
> 
> Thanks,

I'm sorry, I must have done something wrong when I sent the patch, the
CC list is not complete.

Thanks for the link Brian.