[PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Ritesh Raj Sarraf]

Lenovo Yoga (many variants: Yoga, Yoga2 Pro, Yoga2 13, Yoga3 Pro, Yoga 3
14 etc) has multiple modles that are a hybrid laptop, working in laptop
mode as well as tablet mode.

Currently, there is no easy interface to determine the touchpad status,
which in case of the Yoga family of machines, can also be useful to
assume tablet mode status.
Note: The ideapad-laptop driver does not provide a SW_TABLET_MODE either

For a detailed discussion  on why we want either of the interfaces,
please see:
https://bugs.launchpad.net/onboard/+bug/1366421/comments/43

This patch adds a sysfs interface for read/write access under:
/sys/bus/platform/devices/VPC2004\:00/touchpad_mode

Signed-off-by: Ritesh Raj Sarraf <rrs@debian.org>
---
 .../ABI/testing/sysfs-platform-ideapad-laptop      |  9 ++++++
 drivers/platform/x86/ideapad-laptop.c              | 33 ++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
index b31e782bd985..401e37e5acbd 100644
--- a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
+++ b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
@@ -17,3 +17,12 @@ Description:
 			* 2 -> Dust Cleaning
 			* 4 -> Efficient Thermal Dissipation Mode
 
+What:		/sys/devices/platform/ideapad/touchpad_mode
+Date:		Jan 2017
+KernelVersion:	4.11
+Contact:	"Ritesh Raj Sarraf <rrs@debian.org>"
+Description:
+		Control touchpad mode.
+                        * 1 -> Switched On
+                        * 0 -> Switched Off
+
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index f46ece2ce3c4..639af45b45b6 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -423,9 +423,42 @@ static ssize_t store_ideapad_fan(struct device *dev,
 
 static DEVICE_ATTR(fan_mode, 0644, show_ideapad_fan, store_ideapad_fan);
 
+
+static ssize_t show_touchpad_mode(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	unsigned long result;
+	struct ideapad_private *priv = dev_get_drvdata(dev);
+
+	if (read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result))
+		return sprintf(buf, "-1\n");
+	return sprintf(buf, "%lu\n", result);
+}
+
+static ssize_t store_touchpad_mode(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	int ret, state;
+	struct ideapad_private *priv = dev_get_drvdata(dev);
+
+	if (!count)
+		return 0;
+	if (sscanf(buf, "%i", &state) != 1)
+		return -EINVAL;
+	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
+	if (ret < 0)
+		return -EIO;
+	return count;
+}
+
+static DEVICE_ATTR(touchpad_mode, 0644, show_touchpad_mode, store_touchpad_mode);
+
 static struct attribute *ideapad_attributes[] = {
 	&dev_attr_camera_power.attr,
 	&dev_attr_fan_mode.attr,
+	&dev_attr_touchpad_mode.attr,
 	NULL
 };
 
-- 
2.11.0

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Darren Hart]

On Tue, Feb 14, 2017 at 07:46:12PM +0530, Ritesh Raj Sarraf wrote:
> Lenovo Yoga (many variants: Yoga, Yoga2 Pro, Yoga2 13, Yoga3 Pro, Yoga 3
> 14 etc) has multiple modles that are a hybrid laptop, working in laptop
> mode as well as tablet mode.
> 
> Currently, there is no easy interface to determine the touchpad status,
> which in case of the Yoga family of machines, can also be useful to
> assume tablet mode status.
> Note: The ideapad-laptop driver does not provide a SW_TABLET_MODE either
> 
> For a detailed discussion  on why we want either of the interfaces,
> please see:
> https://bugs.launchpad.net/onboard/+bug/1366421/comments/43

Thanks for your work on this Ritesh.

I object to this statement on principle:

    "I believe only a single source will be strictly necessary, but the more the
     better, because some will work better than others."

More interfaces are most definitely *not* better :-) Ideally we would have a
single method for convertibles to indicate their state to userspace for it to
take action based on user(space)-defined policy.

Obviously we don't want GNOME, KDE, XFCE, X, Wayland, DBUS, etc. etc. all having
to check different sys files for every known laptop all using different names,
formats, and values.

That said, we need to make these systems usable, and as there appears not to be
an accepted standard way of doing this, I don't object to the approach. However,
you mentioned in the bug comments (#64) on 2/12:

    "I have attached the final patch that I had proposed, but for whatever
    reasons, the maintainer isn't convinced that that interface is needed."

This is the only version of this patch I have seen. Who objected to the patch?

I would like to hear from Rafael and Dmitry, for their opinion on an ACPI or INPUT
interface for indicating TABLET_MODE to userspace. Even if we accept this patch
as is, we should be thinking about how to do this in a standard way.

> 
> This patch adds a sysfs interface for read/write access under:
> /sys/bus/platform/devices/VPC2004\:00/touchpad_mode
> 
> Signed-off-by: Ritesh Raj Sarraf <rrs@debian.org>
> ---
>  .../ABI/testing/sysfs-platform-ideapad-laptop      |  9 ++++++
>  drivers/platform/x86/ideapad-laptop.c              | 33 ++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> index b31e782bd985..401e37e5acbd 100644
> --- a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> +++ b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> @@ -17,3 +17,12 @@ Description:
>  			* 2 -> Dust Cleaning
>  			* 4 -> Efficient Thermal Dissipation Mode
>  
> +What:		/sys/devices/platform/ideapad/touchpad_mode
> +Date:		Jan 2017
> +KernelVersion:	4.11
> +Contact:	"Ritesh Raj Sarraf <rrs@debian.org>"
> +Description:
> +		Control touchpad mode.
> +                        * 1 -> Switched On
> +                        * 0 -> Switched Off

Please use consistent whitespace for indentation, in this case, use tabs for the
above two lines.

> +
> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
> index f46ece2ce3c4..639af45b45b6 100644
> --- a/drivers/platform/x86/ideapad-laptop.c
> +++ b/drivers/platform/x86/ideapad-laptop.c
> @@ -423,9 +423,42 @@ static ssize_t store_ideapad_fan(struct device *dev,
>  
>  static DEVICE_ATTR(fan_mode, 0644, show_ideapad_fan, store_ideapad_fan);
>  
> +
> +static ssize_t show_touchpad_mode(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	unsigned long result;
> +	struct ideapad_private *priv = dev_get_drvdata(dev);

For consistency of coding style, please list longest variable declarations
first, and proceed in decreasing line length. "Reverse Christmas Tree Order".

> +
> +	if (read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result))
> +		return sprintf(buf, "-1\n");
> +	return sprintf(buf, "%lu\n", result);
> +}
> +
> +static ssize_t store_touchpad_mode(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	int ret, state;
> +	struct ideapad_private *priv = dev_get_drvdata(dev);

Reverse christmas tree order

> +
> +	if (!count)
> +		return 0;
> +	if (sscanf(buf, "%i", &state) != 1)
> +		return -EINVAL;

Please use kstrtoint, and no need to check for count as this function won't get
called if it is 0.

> +	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
> +	if (ret < 0)
> +		return -EIO;
> +	return count;
> +}
> +
> +static DEVICE_ATTR(touchpad_mode, 0644, show_touchpad_mode, store_touchpad_mode);

Please use:

static DEVICE_ATTR_RW(touchpad_mode);

You'll need to move the show_ and store_ prefixes to be postfixes, see
toshiba-acpi.c for several examples.

> +
>  static struct attribute *ideapad_attributes[] = {
>  	&dev_attr_camera_power.attr,
>  	&dev_attr_fan_mode.attr,
> +	&dev_attr_touchpad_mode.attr,
>  	NULL
>  };
>  
> -- 
> 2.11.0
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Andy Shevchenko]

On Fri, Feb 17, 2017 at 5:33 AM, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Feb 14, 2017 at 07:46:12PM +0530, Ritesh Raj Sarraf wrote:
>> Lenovo Yoga (many variants: Yoga, Yoga2 Pro, Yoga2 13, Yoga3 Pro, Yoga 3
>> 14 etc) has multiple modles that are a hybrid laptop, working in laptop
>> mode as well as tablet mode.

> That said, we need to make these systems usable, and as there appears not to be
> an accepted standard way of doing this, I don't object to the approach. However,
> you mentioned in the bug comments (#64) on 2/12:
>
>     "I have attached the final patch that I had proposed, but for whatever
>     reasons, the maintainer isn't convinced that that interface is needed."
>
> This is the only version of this patch I have seen.

This patch is actually a v2 that includes maintainers as I had asked
and additional paragraph to explain why we need such interface.

> Who objected to the patch?

I was trying to understand why /dev/input/eventX can not be used for a such.

> I would like to hear from Rafael and Dmitry, for their opinion on an ACPI or INPUT
> interface for indicating TABLET_MODE to userspace. Even if we accept this patch
> as is, we should be thinking about how to do this in a standard way.

Good point!

>> +
>> +     if (!count)
>> +             return 0;
>> +     if (sscanf(buf, "%i", &state) != 1)
>> +             return -EINVAL;
>
> Please use kstrtoint, and no need to check for count as this function won't get
> called if it is 0.

I guess Ritesh followed existing style in the code.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Dmitry Torokhov]

On Fri, Feb 17, 2017 at 09:39:20AM +0200, Andy Shevchenko wrote:
> On Fri, Feb 17, 2017 at 5:33 AM, Darren Hart <dvhart@infradead.org> wrote:
> > On Tue, Feb 14, 2017 at 07:46:12PM +0530, Ritesh Raj Sarraf wrote:
> >> Lenovo Yoga (many variants: Yoga, Yoga2 Pro, Yoga2 13, Yoga3 Pro, Yoga 3
> >> 14 etc) has multiple modles that are a hybrid laptop, working in laptop
> >> mode as well as tablet mode.
> 
> > That said, we need to make these systems usable, and as there appears not to be
> > an accepted standard way of doing this, I don't object to the approach. However,
> > you mentioned in the bug comments (#64) on 2/12:
> >
> >     "I have attached the final patch that I had proposed, but for whatever
> >     reasons, the maintainer isn't convinced that that interface is needed."
> >
> > This is the only version of this patch I have seen.
> 
> This patch is actually a v2 that includes maintainers as I had asked
> and additional paragraph to explain why we need such interface.
> 
> > Who objected to the patch?
> 
> I was trying to understand why /dev/input/eventX can not be used for a such.
> 
> > I would like to hear from Rafael and Dmitry, for their opinion on an ACPI or INPUT
> > interface for indicating TABLET_MODE to userspace. Even if we accept this patch
> > as is, we should be thinking about how to do this in a standard way.

Kernel should send EV_SW/SW_TABLET_MODE to communicate tablet mode to
userspace; clients should be listening to these events.

It looks like the patch is trying to provide a way to disable the
touchpad when transitioning in tablet mode. In ChromeOS we have a notion
of "inhibiting" input devices, where userspace policy daemon tells the
kernel that it is not interested in events from a given device and input
core will suppress events from such device (and driver may optionally
put device into low power mode, if it chooses to do so). But in this
case the control seems to be totally outside of the input driver (which
I suspect is PS/2 device), so the policy daemon would have to have
specific knowledge of this new knob.

BTW, I am not sure if this is actually reliable: if system goes to S3
with lid open and user changes it into tablet form, nobody will tell the
EC that touchpad should be ignored and it will wake up the tablet.

Thanks.

-- 
Dmitry

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Ritesh Raj Sarraf]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hello Dmitry,

On Fri, 2017-02-17 at 00:48 -0800, Dmitry Torokhov wrote:
> It looks like the patch is trying to provide a way to disable the
> touchpad when transitioning in tablet mode. 

The keyboard/touchpad get disabled, by hardware, when flipped to tablet mode.
Since currently, the driver has no way to expose a tablet-mode interface, we
relied on this behavior, for the Lenovo Yoga device types.

> In ChromeOS we have a notion
> of "inhibiting" input devices, where userspace policy daemon tells the
> kernel that it is not interested in events from a given device and input
> core will suppress events from such device (and driver may optionally
> put device into low power mode, if it chooses to do so). But in this
> case the control seems to be totally outside of the input driver (which
> I suspect is PS/2 device), so the policy daemon would have to have
> specific knowledge of this new knob.
> 
> BTW, I am not sure if this is actually reliable: if system goes to S3
> with lid open and user changes it into tablet form, nobody will tell the
> EC that touchpad should be ignored and it will wake up the tablet.

I am not sure what you mean here. But:

root@learner:/sys/bus/platform/devices/VPC2004:00# When in normal mode^C
root@learner:/sys/bus/platform/devices/VPC2004:00# cat touchpad_mode 
1

S3 Suspend here. THen flip hw to tablet mode. Now, hit power btn to Resume.

root@learner:/sys/bus/platform/devices/VPC2004:00# Now in tablet mode^C
root@learner:/sys/bus/platform/devices/VPC2004:00# cat touchpad_mode 
0

- -- 
Ritesh Raj Sarraf | http://people.debian.org/~rrs
Debian - The Universal Operating System
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEQCVDstmIVAB/Yn02pjpYo/LhdWkFAlimwAQACgkQpjpYo/Lh
dWkzIw/+NMiQ4A/OplUP/g6mTfba5zyuN0g9Or6RwzXyPrFsWbPAN4E6pcNIM1OT
oy8RAnrqdSQymVH5atbR/ysPwlzG19+LWB4FVlVdkDMKkVonAHmUX+mL9+KFa7E5
+RmpS9Hzc6cLv1kpKMUeLD7j9U3I4NkUEsxDZwLk916KE6fCthGsmbn1gGADv3oT
/lErDW7MLaImR67cmHmRhfhIotQPokcn3OVT/Eb7K8UjvXQLlWnDxMdxlQ7xDTua
aJ3c/lB+FQytwp/JBSbNpzspNcCX5mmkh6PERXhxLJzL1CpCz3qnwKwKyeoGFcjv
goj+CTZNsnGqaBuZThNI2vVWJRHqGCX2qp3IyWrSzNa6j5SfMTlgr0a/WsI7rpiY
4+81OXoeCVcr+ZGO4wtsvR1WDWvXqb+0bWYByNu85L/Rj9KWzxfh1mbtahG6odi8
bp7/+1cy55+d4XnX2/QHgkXdSCelBPtwH+aJUBp5av1N6t88+Dat8p3EyK30Cyp/
XfdglsfKWvmJX+hLBgR75ejy3cvr5iBIuExuULZNd8qrj+4dYDt4cYQB/MnttLt4
HZ1c/qSjMTDPdMBb4s3C844Pgk6QsKHJwLEl2zTN8HqNTMdwHBktyFPEzsRqV6Wn
Kea6z4lFUwS17LCaq/bhCwvYcFJm2XSimtXWj1a50YdwEgAiT5o=
=nuhe
-----END PGP SIGNATURE-----

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Dmitry Torokhov]

On Fri, Feb 17, 2017 at 02:49:00PM +0530, Ritesh Raj Sarraf wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA512
> 
> Hello Dmitry,
> 
> On Fri, 2017-02-17 at 00:48 -0800, Dmitry Torokhov wrote:
> > It looks like the patch is trying to provide a way to disable the
> > touchpad when transitioning in tablet mode. 
> 
> The keyboard/touchpad get disabled, by hardware, when flipped to tablet mode.
> Since currently, the driver has no way to expose a tablet-mode interface, we
> relied on this behavior, for the Lenovo Yoga device types.
> 
> > In ChromeOS we have a notion
> > of "inhibiting" input devices, where userspace policy daemon tells the
> > kernel that it is not interested in events from a given device and input
> > core will suppress events from such device (and driver may optionally
> > put device into low power mode, if it chooses to do so). But in this
> > case the control seems to be totally outside of the input driver (which
> > I suspect is PS/2 device), so the policy daemon would have to have
> > specific knowledge of this new knob.
> > 
> > BTW, I am not sure if this is actually reliable: if system goes to S3
> > with lid open and user changes it into tablet form, nobody will tell the
> > EC that touchpad should be ignored and it will wake up the tablet.
> 
> I am not sure what you mean here. But:
> 
> root@learner:/sys/bus/platform/devices/VPC2004:00# When in normal mode^C
> root@learner:/sys/bus/platform/devices/VPC2004:00# cat touchpad_mode 
> 1
> 
> S3 Suspend here. THen flip hw to tablet mode. Now, hit power btn to Resume.
> 
> root@learner:/sys/bus/platform/devices/VPC2004:00# Now in tablet mode^C
> root@learner:/sys/bus/platform/devices/VPC2004:00# cat touchpad_mode 
> 0

OK, if firmware disables the touchpad automatically, then it is fine.

I am wondering about the utility of the new interface though. Are there
already major users of it? If there are no users, then there is no point
in adding it, it will stay there unused and forgotten by anyone.
Teaching ideapad to emit SW_TABLET_MODE is more useful IMO.

Thanks.

-- 
Dmitry

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Ritesh Raj Sarraf]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hello Dmitry,

On Fri, 2017-02-17 at 11:52 -0800, Dmitry Torokhov wrote:
> OK, if firmware disables the touchpad automatically, then it is fine.
> 
> I am wondering about the utility of the new interface though. Are there
> already major users of it? 

Not yet. I proposed this interface because we want something like this for
Onboard. For detailed discussion, please see:
https://bugs.launchpad.net/onboard/+bug/1366421/comments/43

hp-wmi and thinkpad-acpi do provide a similar interface, but for tablet.


> If there are no users, then there is no point
> in adding it, it will stay there unused and forgotten by anyone.
> Teaching ideapad to emit SW_TABLET_MODE is more useful IMO.

If accepted, Onboard will be a consumer.

Yes. SW_TABLET_MODE would be ideal. But looking at the drive's code, I don't
know if it has an EC command for it.

- -- 
Ritesh Raj Sarraf | http://people.debian.org/~rrs
Debian - The Universal Operating System
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEQCVDstmIVAB/Yn02pjpYo/LhdWkFAlioGScACgkQpjpYo/Lh
dWnEvA/5AfQQYaBiOsunZuR6OcM1BrFy1vpSGAToRNdWkaV18p7IfZGSkDUbZ3U+
rLdL5URnD5/CsHhcOPpUp4IhM4wdce+Rxc4T4LdEyjqZ29IlGrA7k21i2fJXfEws
5Mjd+pN01PitXGGBjc/BcX0ol37pcXyLfMpl3UKvE44LU2CwU5sahnSDyHgM+G77
Gnoy5Po0eLAQoOPAJIsLwSaxJlzoZAknZWuAwqQfNXxXdxVFfTr5OmV9pcTx2vgw
cPdg+8lrU8M7w2SNrK91AvccnXLV5T/rNOolk3VHoH/AQJdgxbiZKdkv0jyg/WKp
TyGIkgwNSFjYeph/JZ/IqXkOnkuB/TcKQq7uVKN5b71U2VouV6gEe/2RcOQ+lSYg
au3NX4Wq6Ot9MSMfqwIZ0+TgazGGjbUlFLhf1TFRZ9o8ERsZrtaXsC5c1U3egAS/
XZWuiI0CI5c3k1WTxWpy76pX963gh+3d9kejanWug2LIriIALmD5/ryN56Jml41S
3I3JnnuiLuwnZ/t/ehA3c9JVhsbTJg4dBbZE9tKgnAjIV/MmmbE9kCtnCRcBuFUa
W2vViMxv2eVBfiHP3Z8TC7E0UUqe69htDEomFfwDpQ1oSqDLzm6gafH8LStbYML8
j+/qGUTyg8d3yP/jrUL5UDO6dMkj+FV5bNi4qlue4uVLV7bx4o8=
=flDx
-----END PGP SIGNATURE-----

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Ritesh Raj Sarraf]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Hello Darren,

On Thu, 2017-02-16 at 19:33 -0800, Darren Hart wrote:
> On Tue, Feb 14, 2017 at 07:46:12PM +0530, Ritesh Raj Sarraf wrote:
> > 
> > 
> That said, we need to make these systems usable, and as there appears not to
> be
> an accepted standard way of doing this, I don't object to the approach.
> However,
> you mentioned in the bug comments (#64) on 2/12:
> 
>     "I have attached the final patch that I had proposed, but for whatever
>     reasons, the maintainer isn't convinced that that interface is needed."
> 
> This is the only version of this patch I have seen. Who objected to the patch?
> 

Sorry. Nobody objected to it. I didn't hear back after last conversation with
Andy and wrongly presumed that adding that interface was a no go.


> I would like to hear from Rafael and Dmitry, for their opinion on an ACPI or
> INPUT
> interface for indicating TABLET_MODE to userspace. Even if we accept this
> patch
> as is, we should be thinking about how to do this in a standard way.
> 

Yes. That would be ideal. But as you mentioned, currently, not all drivers do
it. So my hope was to have this touchpad interface exposed, which could be used
by applications to derive tablet status.

Lenovo Yogas, when flipped to tablet mode orientation, disables the keyboard and
touchpad.


> > 
> > This patch adds a sysfs interface for read/write access under:
> > /sys/bus/platform/devices/VPC2004\:00/touchpad_mode
> > 

Can you please help here ?

Documentation/ABI/testing/sysfs-platform-ideapad-laptop mentions that the sysfs
path would be /sys/devices/platform/ideapad/ whereas what I actually see on my
machine is /sys/bus/platform/devices/VPC2004:00/

> > Signed-off-by: Ritesh Raj Sarraf <rrs@debian.org>
> > ---
> >  .../ABI/testing/sysfs-platform-ideapad-laptop      |  9 ++++++
> >  drivers/platform/x86/ideapad-laptop.c              | 33
> > ++++++++++++++++++++++
> >  2 files changed, 42 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> > b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> > index b31e782bd985..401e37e5acbd 100644
> > --- a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> > +++ b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
> > @@ -17,3 +17,12 @@ Description:
> >  			* 2 -> Dust Cleaning
> >  			* 4 -> Efficient Thermal Dissipation Mode
> >  
> > +What:		/sys/devices/platform/ideapad/touchpad_mode
> > +Date:		Jan 2017
> > +KernelVersion:	4.11
> > +Contact:	"Ritesh Raj Sarraf <rrs@debian.org>"
> > +Description:
> > +		Control touchpad mode.
> > +                        * 1 -> Switched On
> > +                        * 0 -> Switched Off
> 
> Please use consistent whitespace for indentation, in this case, use tabs for
> the
> above two lines.
> 

Thanks. Done (and fixed my .vimrc too).

> > +
> > diff --git a/drivers/platform/x86/ideapad-laptop.c
> > b/drivers/platform/x86/ideapad-laptop.c
> > index f46ece2ce3c4..639af45b45b6 100644
> > --- a/drivers/platform/x86/ideapad-laptop.c
> > +++ b/drivers/platform/x86/ideapad-laptop.c
> > @@ -423,9 +423,42 @@ static ssize_t store_ideapad_fan(struct device *dev,
> >  
> >  static DEVICE_ATTR(fan_mode, 0644, show_ideapad_fan, store_ideapad_fan);
> >  
> > +
> > +static ssize_t show_touchpad_mode(struct device *dev,
> > +				struct device_attribute *attr,
> > +				char *buf)
> > +{
> > +	unsigned long result;
> > +	struct ideapad_private *priv = dev_get_drvdata(dev);
> 
> For consistency of coding style, please list longest variable declarations
> first, and proceed in decreasing line length. "Reverse Christmas Tree Order".
> 

Thank you. Done.

> > +
> > +	if (read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result))
> > +		return sprintf(buf, "-1\n");
> > +	return sprintf(buf, "%lu\n", result);
> > +}
> > +
> > +static ssize_t store_touchpad_mode(struct device *dev,
> > +				 struct device_attribute *attr,
> > +				 const char *buf, size_t count)
> > +{
> > +	int ret, state;
> > +	struct ideapad_private *priv = dev_get_drvdata(dev);
> 
> Reverse christmas tree order
> 
> > +
> > +	if (!count)
> > +		return 0;
> > +	if (sscanf(buf, "%i", &state) != 1)
> > +		return -EINVAL;
> 
> Please use kstrtoint, and no need to check for count as this function won't
> get
> called if it is 0.
> 

Done. Thank you.

> > +	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
> > +	if (ret < 0)
> > +		return -EIO;
> > +	return count;
> > +}
> > +
> > +static DEVICE_ATTR(touchpad_mode, 0644, show_touchpad_mode,
> > store_touchpad_mode);
> 
> Please use:
> 
> static DEVICE_ATTR_RW(touchpad_mode);
> 
> You'll need to move the show_ and store_ prefixes to be postfixes, see
> toshiba-acpi.c for several examples.
> 

Thank you. I am building the kernel the test/verify the changes. Will post the
revised patch soon.

> > +
> >  static struct attribute *ideapad_attributes[] = {
> >  	&dev_attr_camera_power.attr,
> >  	&dev_attr_fan_mode.attr,
> > +	&dev_attr_touchpad_mode.attr,
> >  	NULL
> >  };
> >  
> > -- 
> > 2.11.0
> > 
> > 
> 
> 
- -- 
Ritesh Raj Sarraf | http://people.debian.org/~rrs
Debian - The Universal Operating System
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEQCVDstmIVAB/Yn02pjpYo/LhdWkFAlimvMEACgkQpjpYo/Lh
dWlbUQ/8Cyj591QiHofqg9NsVcHMJ//1+sEy4rWSKykW+O0+Wss+GTfF7nufllV9
hU1rRQ3TVcZL/4zeFG+nVS9d8r4TbYxcAGisr8nxucHJmD9pDN9blVRP5rT7Meen
zWvy2vSZRVTMo/SrLObtwzTUrXjIiOZWZKwjiYiJYZ7T55tvj5i8+5byDb7ITlzJ
gmiNSrTByg7Q3Jr5IPc0nzSU84Iq1dV9N6Ym5FtYJhNssNe3I9vEZQCfEHEpFHID
7/N4GFHScSVCPb9TgdJ61mR9PbsWtiNraW297F9CsVEFwNseq8IAfZLXYCdpdWV8
BysBb28WCaY42D3hIl0ycDIFYuf+ydmu1C+nQlDYgjJ9i3j8TUhGCqP0pQpINXXb
yrVFIb5/ytHxzOgENwJgsycZT1sSTuc0PQzJlzaIkjR00tAcXOop9tNEH9sIFC6z
xkYoPcMWG/hO6eihV+62+04aySSjXID7/BD/5KBZpypZ+spSKyLqhSBaohjbIQ64
WBRFOCZaqc465WzhEJTIfLBao6OT18TYXUPeq5e6a2AhrsJHUzVzw0cJdx2MY/vO
JCkoFWokgDpGmHn/kc/CYaTKyE7IrWnQUGsQWHLAdaLeaqC8R1YsWG0eJs/Wvl6d
/rdzjHUv4dKv/CGpZ9V+P3P3EaKMyB3FDg9Q46D2uBDHPE+IJ4A=
=Oeu5
-----END PGP SIGNATURE-----

Re: [PATCH] platform/x86: ideapad-laptop: Add sysfs interface for touchpad state

[Ritesh Raj Sarraf]

[-- Attachment #1.1: Type: text/plain, Size: 664 bytes --]

On Fri, 2017-02-17 at 14:35 +0530, Ritesh Raj Sarraf wrote:
> > Please use:
> > 
> > static DEVICE_ATTR_RW(touchpad_mode);
> > 
> > You'll need to move the show_ and store_ prefixes to be postfixes, see
> > toshiba-acpi.c for several examples.
> > 
> 
> Thank you. I am building the kernel the test/verify the changes. Will post the
> revised patch soon.


I have included the changes you mentioned, built and tested locally on my Lenovo
Yoga2 13, and posted (git send-email) to all involved parties.

I have also attached the patch in this reply.


-- 
Ritesh Raj Sarraf | http://people.debian.org/~rrs
Debian - The Universal Operating System

[-- Attachment #1.2: Type: text/x-patch, Size: 3205 bytes --]

From 6b874d4f786a1a10862f843da45804485b6d0e72 Mon Sep 17 00:00:00 2001
From: Ritesh Raj Sarraf <rrs@debian.org>
Date: Mon, 30 Jan 2017 15:05:48 +0530
Subject: [PATCH v3] Add sysfs interface for touchpad state

Lenovo Yoga (many variants: Yoga, Yoga2 Pro, Yoga2 13, Yoga3 Pro, Yoga 3
14 etc) has multiple modles that are a hybrid laptop, working in laptop
mode as well as tablet mode.

Currently, there is no easy interface to determine the touchpad status,
which in case of the Yoga family of machines, can also be useful to
assume tablet mode status.
Note: The ideapad-laptop driver does not provide a SW_TABLET_MODE either

For a detailed discussion  on why we want either of the interfaces,
please see:
https://bugs.launchpad.net/onboard/+bug/1366421/comments/43

This patch adds a sysfs interface for read/write access under:
/sys/bus/platform/devices/VPC2004\:00/touchpad_mode

v3:
Include Darren Hart's comments
Changed sysfs inteface from "touchpad_mode" to "touchpad"

v2:
Include Andy Shevchenko's comments

Signed-off-by: Ritesh Raj Sarraf <rrs@debian.org>
---
 .../ABI/testing/sysfs-platform-ideapad-laptop      |  8 +++++
 drivers/platform/x86/ideapad-laptop.c              | 36 ++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
index b31e782bd985..5d24f1e8e6ef 100644
--- a/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
+++ b/Documentation/ABI/testing/sysfs-platform-ideapad-laptop
@@ -17,3 +17,11 @@ Description:
 			* 2 -> Dust Cleaning
 			* 4 -> Efficient Thermal Dissipation Mode
 
+What:		/sys/devices/platform/ideapad/touchpad
+Date:		Feb 2017
+KernelVersion:	4.11
+Contact:	"Ritesh Raj Sarraf <rrs@debian.org>"
+Description:
+		Control touchpad mode.
+			* 1 -> Switched On
+			* 0 -> Switched Off
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index f46ece2ce3c4..aff1a561c9ec 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -423,9 +423,45 @@ static ssize_t store_ideapad_fan(struct device *dev,
 
 static DEVICE_ATTR(fan_mode, 0644, show_ideapad_fan, store_ideapad_fan);
 
+
+static ssize_t touchpad_show(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	struct ideapad_private *priv = dev_get_drvdata(dev);
+	unsigned long result;
+
+	if (read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &result))
+		return sprintf(buf, "-1\n");
+	return sprintf(buf, "%lu\n", result);
+}
+
+static ssize_t touchpad_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct ideapad_private *priv = dev_get_drvdata(dev);
+	int ret, state;
+
+	ret = kstrtoint(buf, 0, &state);
+	if (ret)
+		return ret;
+
+	if (state != 0 && state != 1)
+		return -EINVAL;
+
+	ret = write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, state);
+	if (ret < 0)
+		return -EIO;
+	return count;
+}
+
+static DEVICE_ATTR_RW(touchpad);
+
 static struct attribute *ideapad_attributes[] = {
 	&dev_attr_camera_power.attr,
 	&dev_attr_fan_mode.attr,
+	&dev_attr_touchpad.attr,
 	NULL
 };
 
-- 
2.11.0


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]