Re: [PATCH] Add sysfs interface for touchpad state

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Andy,

On Mon, 2017-01-30 at 22:25 +0200, Andy Shevchenko wrote:
> On Mon, Jan 30, 2017 at 12:57 PM, Ritesh Raj Sarraf <rrs@xxxxxxxxxx> 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.
> 
> The main question is who is the initiator of the mode change? Do you
> need to do anything in UI to prepare OS for such switch?
> 

The human user is the only initiator. The hardware has 360 degree rotation.
There is nothing special needed in the UI, or the OS, to get into tablet mode.
Just flip the hardware 360 degree into tablet mode.

More details on the hardware series can be found at:
http://shop.lenovo.com/us/en/laptops/lenovo/yoga-laptop-series/yoga-laptop-2-13/


> > There is no interface available to query the physical state of these
> > devices. Also, from the driver, it doesn't look to be having an EC
> > command to get that information from the acpi driver.
> > 
> > When in tablet mode, the hardware keyboard and touchpad get disabled.
> > This could be one sub-optimal way to assume that under such condition
> > (touchpad disabled) the machine is in Tablet-Mode. Other than this, I've
> > not come across another way to determine the physical state of the
> > hardware.
> > 
> > Currently, some of the hardware status is provided through debugfs,
> > which is useless for unprivileged processes. (This already has many
> > broken reports: radio and wifi, which are active/on as I write this)
> 
> This should be fixed.
> 

Thanks.

> > This patch adds a sysfs interface for unprivileged process access under:
> > /sys/bus/platform/devices/VPC2004\:00/touchpad_state
> > 
> > rrs@learner:/sys/bus/platform/devices/VPC2004:00$ cat touchpad_state
> > 1
> > 2017-01-30 / 16:12:00 ♒♒♒  ☺
> > rrs@learner:/sys/bus/platform/devices/VPC2004:00$ cat touchpad_state
> > 0
> > 2017-01-30 / 16:12:06 ♒♒♒  ☺
> 
> You introduce an ABI without documenting it.
> 

Sorry about that. I am no kernel developer, just wanting to fix my hardware.

I looked into the doc. The documentation mentions the sysfs path:
'/sys/devices/platform/ideapad/fan_mode', which is what we would also want to
use.

But the driver only shows the interface through:
'/sys/devices/platform/devices/VPC2004:00/'

Is this an expected behavior ?

> > 
> > 
> > +
> > +static ssize_t show_touchpad_state(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");
> 
> That is bad. Return the error code what read_ec_data() returns. If it
> is not suitable convert it here.
> 

All that read_ec_data() can return are 0 and -1.

The other 2 consumers, show_ideapad_cam() and show_ideapad_fan() are doing the
same. 

> > +       return sprintf(buf, "%lu\n", result);
> > +}
> > +
> > +static ssize_t store_touchpad_state(struct device *dev,
> > +                                struct device_attribute *attr,
> > +                                const char *buf, size_t count)
> > +{
> > +        /* Don't enable writing here */
> > +        return -1;
> 
> And why you can't just drop this completely?
> 

I did that. But it was failing to build. But anyways, please see my revised
patch along with write support.

> > +}
> > +
> > +static DEVICE_ATTR(touchpad_state, 0644, show_touchpad_state,
> > store_touchpad_state);
> > +
> >  static struct attribute *ideapad_attributes[] = {
> >         &dev_attr_camera_power.attr,
> >         &dev_attr_fan_mode.attr,
> > +       &dev_attr_touchpad_state.attr,
> 
> Is it really state, btw? Reading your commit message I would assume
> this is also "mode" for touchpad.
> 

Actually, yes. It'd be better to have it as a mode, along with allowing to
write. That way it will be consistent with the other 2 (cam and fan) interfaces.

Please find attached revised patch along with write support and ABI
Documentation changes.

Thanks,
Ritesh

-- 
Ritesh Raj Sarraf | http://people.debian.org/~rrs
Debian - The Universal Operating System
From 8a3980bb15e31a5c17e7541bc60cbe4bc56bf604 Mon Sep 17 00:00:00 2001
From: Ritesh Raj Sarraf <rrs@xxxxxxxxxx>
Date: Mon, 30 Jan 2017 15:05:48 +0530
Subject: [PATCH] Add sysfs interface for touchpad state
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.

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

rrs@learner:~$ cat /sys/bus/platform/devices/VPC2004\:00/touchpad_mode
1
2017-01-31 / 16:58:46 ♒♒♒  ☺
rrs@learner:~$ su
Password:
root@learner:/home/rrs# echo 0 > /sys/bus/platform/devices/VPC2004\:00/touchpad_mode
root@learner:/home/rrs# cat  /sys/bus/platform/devices/VPC2004\:00/touchpad_mode
0
root@learner:/home/rrs# echo 1 > /sys/bus/platform/devices/VPC2004\:00/touchpad_mode
root@learner:/home/rrs# cat  /sys/bus/platform/devices/VPC2004\:00/touchpad_mode
1
root@learner:/home/rrs# exit
2017-01-31 / 16:59:26 ♒♒♒  ☺
rrs@learner:~$

Enable write. Change name to _mode

Update ABI documentation for ideapad-laptop

Signed-off-by: Ritesh Raj Sarraf <rrs@xxxxxxxxxx>
---
 .../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@xxxxxxxxxx>"
+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 a7614fc542b5..e612e0764ba6 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

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux