[PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Patrick Pedersen]

As with previous generations of this device (see https://patchwork.kernel.org/patch/7887361/), the ITE
HID Sensor Hub, responsible for the accelerometer and als sensor, requires a quirk entry.

Without the entry, the Sensor Hub can't be accessed and the kernel fails to report any movements. As a result
iio-sensor-proxy receives no new data.

It shall additionally be noted that the i2c-hid 'sleep' bug (present since kernel ver. 4.3)
still affects the driver. This means that the sensor hub will not report any movement, until
the device is suspended and resumed.

Signed-off-by: Patrick Pedersen <ctx.xda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/hid/hid-ids.h        | 1 +
 drivers/hid/hid-sensor-hub.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 4f9a3938189a..b427a0bcfbe8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -565,6 +565,7 @@
 #define USB_DEVICE_ID_ITE_LENOVO_YOGA   0x8386
 #define USB_DEVICE_ID_ITE_LENOVO_YOGA2  0x8350
 #define USB_DEVICE_ID_ITE_LENOVO_YOGA900	0x8396
+#define USB_DEVICE_ID_ITE_LENOVO_YOGA910	0x8186
 
 #define USB_VENDOR_ID_JABRA		0x0b0e
 #define USB_DEVICE_ID_JABRA_SPEAK_410	0x0412
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 4ef73374a8f9..85b8425483bd 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -820,6 +820,9 @@ static const struct hid_device_id sensor_hub_devices[] = {
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_ITE,
 			USB_DEVICE_ID_ITE_LENOVO_YOGA900),
 			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
+	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_ITE,
+			USB_DEVICE_ID_ITE_LENOVO_YOGA910),
+			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
 	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB, USB_VENDOR_ID_INTEL_0,
 			0x22D8),
 			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
-- 
2.13.3

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Bastien Nocera]

On Sat, 2017-07-15 at 14:27 +0200, Patrick Pedersen wrote:
> As with previous generations of this device (see https://patchwork.ke
> rnel.org/patch/7887361/), the ITE
> HID Sensor Hub, responsible for the accelerometer and als sensor,
> requires a quirk entry.
> 
> Without the entry, the Sensor Hub can't be accessed and the kernel
> fails to report any movements. As a result
> iio-sensor-proxy receives no new data.
> 
> It shall additionally be noted that the i2c-hid 'sleep' bug (present
> since kernel ver. 4.3)
> still affects the driver. This means that the sensor hub will not
> report any movement, until
> the device is suspended and resumed.
> 
> Signed-off-by: Patrick Pedersen <ctx.xda@gmail.com>
> ---
>  drivers/hid/hid-ids.h        | 1 +
>  drivers/hid/hid-sensor-hub.c | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 4f9a3938189a..b427a0bcfbe8 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -565,6 +565,7 @@
>  #define USB_DEVICE_ID_ITE_LENOVO_YOGA   0x8386
>  #define USB_DEVICE_ID_ITE_LENOVO_YOGA2  0x8350
>  #define USB_DEVICE_ID_ITE_LENOVO_YOGA900	0x8396
> +#define USB_DEVICE_ID_ITE_LENOVO_YOGA910	0x8186
>  
>  #define USB_VENDOR_ID_JABRA		0x0b0e
>  #define USB_DEVICE_ID_JABRA_SPEAK_410	0x0412
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 4ef73374a8f9..85b8425483bd 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -820,6 +820,9 @@ static const struct hid_device_id
> sensor_hub_devices[] = {
>  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_ITE,
>  			USB_DEVICE_ID_ITE_LENOVO_YOGA900),
>  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_ITE,
> +			USB_DEVICE_ID_ITE_LENOVO_YOGA910),
> +			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
>  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_INTEL_0,
>  			0x22D8),
>  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},

At this point, wouldn't it make sense to apply the quirk to *all* ITE
devices in Lenovo Yoga laptops?

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Bastien Nocera]

On Sat, 2017-07-15 at 19:52 +0200, Patrick Pedersen wrote:
> On Sat, Jul 15, 2017 at 4:29 PM, Bastien Nocera <hadess@hadess.net>
> wrote:
> > On Sat, 2017-07-15 at 14:27 +0200, Patrick Pedersen wrote:
> > > As with previous generations of this device (see https://patchwor
> > k.ke
> > > rnel.org/patch/7887361/), the ITE
> > > HID Sensor Hub, responsible for the accelerometer and als sensor,
> > > requires a quirk entry.
> > >
> > > Without the entry, the Sensor Hub can't be accessed and the
> > kernel
> > > fails to report any movements. As a result
> > > iio-sensor-proxy receives no new data.
> > >
> > > It shall additionally be noted that the i2c-hid 'sleep' bug
> > (present
> > > since kernel ver. 4.3)
> > > still affects the driver. This means that the sensor hub will not
> > > report any movement, until
> > > the device is suspended and resumed.
> > >
> > > Signed-off-by: Patrick Pedersen <ctx.xda@gmail.com>
> > > ---
> > >  drivers/hid/hid-ids.h        | 1 +
> > >  drivers/hid/hid-sensor-hub.c | 3 +++
> > >  2 files changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > index 4f9a3938189a..b427a0bcfbe8 100644
> > > --- a/drivers/hid/hid-ids.h
> > > +++ b/drivers/hid/hid-ids.h
> > > @@ -565,6 +565,7 @@
> > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA   0x8386
> > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA2  0x8350
> > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA900     0x8396
> > > +#define USB_DEVICE_ID_ITE_LENOVO_YOGA910     0x8186
> > >
> > >  #define USB_VENDOR_ID_JABRA          0x0b0e
> > >  #define USB_DEVICE_ID_JABRA_SPEAK_410        0x0412
> > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-
> > sensor-
> > > hub.c
> > > index 4ef73374a8f9..85b8425483bd 100644
> > > --- a/drivers/hid/hid-sensor-hub.c
> > > +++ b/drivers/hid/hid-sensor-hub.c
> > > @@ -820,6 +820,9 @@ static const struct hid_device_id
> > > sensor_hub_devices[] = {
> > >       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > USB_VENDOR_ID_ITE,
> > >                       USB_DEVICE_ID_ITE_LENOVO_YOGA900),
> > >                       .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > > +     { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > USB_VENDOR_ID_ITE,
> > > +                     USB_DEVICE_ID_ITE_LENOVO_YOGA910),
> > > +                     .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > >       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > USB_VENDOR_ID_INTEL_0,
> > >                       0x22D8),
> > >                       .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > 
> > At this point, wouldn't it make sense to apply the quirk to *all*
> > ITE
> > devices in Lenovo Yoga laptops?
> 
> I'm not sure If I got your suggestion right.
> 
> Those laptops do not share the same ITE chip. ITE is simply 
> the vendor/manufacturer of the hid sensor hub. All three defined 
> yoga laptops use a different ITE sensor hub model.
> 
> To make this clear, my device, the 
> Lenovo Yoga 910 uses a ITE8186, 
> where the Yoga 900 uses a ITE8396, 
> the Yoga 2 a ITE8350 and so on
> 
> Now what "could" me done, is to detect and set the quirk
> automatically. 
> This should be doable, as the Hardware PID corresponds to the 
> model number of the Sensor Hub (ex. ITE8186 = PID 0x8186)

I'm saying that if the vendor of the device is USB_VENDOR_ID_ITE and
the manufacturer of the hardware is Lenovo in the DMI information, and
the model of hardware contains "Yoga", that HID_SENSOR_HUB_ENUM_QUIRK
be applied automatically, instead of adding quirks one-by-one.

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Patrick]

On Sat, Jul 15, 2017 at 07:58:10PM +0200, Bastien Nocera wrote:
> On Sat, 2017-07-15 at 19:52 +0200, Patrick Pedersen wrote:
> > On Sat, Jul 15, 2017 at 4:29 PM, Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
> > wrote:
> > > On Sat, 2017-07-15 at 14:27 +0200, Patrick Pedersen wrote:
> > > > As with previous generations of this device (see https://patchwor
> > > k.ke
> > > > rnel.org/patch/7887361/), the ITE
> > > > HID Sensor Hub, responsible for the accelerometer and als sensor,
> > > > requires a quirk entry.
> > > >
> > > > Without the entry, the Sensor Hub can't be accessed and the
> > > kernel
> > > > fails to report any movements. As a result
> > > > iio-sensor-proxy receives no new data.
> > > >
> > > > It shall additionally be noted that the i2c-hid 'sleep' bug
> > > (present
> > > > since kernel ver. 4.3)
> > > > still affects the driver. This means that the sensor hub will not
> > > > report any movement, until
> > > > the device is suspended and resumed.
> > > >
> > > > Signed-off-by: Patrick Pedersen <ctx.xda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > >  drivers/hid/hid-ids.h        | 1 +
> > > >  drivers/hid/hid-sensor-hub.c | 3 +++
> > > >  2 files changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > > index 4f9a3938189a..b427a0bcfbe8 100644
> > > > --- a/drivers/hid/hid-ids.h
> > > > +++ b/drivers/hid/hid-ids.h
> > > > @@ -565,6 +565,7 @@
> > > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA   0x8386
> > > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA2  0x8350
> > > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA900     0x8396
> > > > +#define USB_DEVICE_ID_ITE_LENOVO_YOGA910     0x8186
> > > >
> > > >  #define USB_VENDOR_ID_JABRA          0x0b0e
> > > >  #define USB_DEVICE_ID_JABRA_SPEAK_410        0x0412
> > > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-
> > > sensor-
> > > > hub.c
> > > > index 4ef73374a8f9..85b8425483bd 100644
> > > > --- a/drivers/hid/hid-sensor-hub.c
> > > > +++ b/drivers/hid/hid-sensor-hub.c
> > > > @@ -820,6 +820,9 @@ static const struct hid_device_id
> > > > sensor_hub_devices[] = {
> > > >       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > > USB_VENDOR_ID_ITE,
> > > >                       USB_DEVICE_ID_ITE_LENOVO_YOGA900),
> > > >                       .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > > > +     { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > > USB_VENDOR_ID_ITE,
> > > > +                     USB_DEVICE_ID_ITE_LENOVO_YOGA910),
> > > > +                     .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > > >       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > > USB_VENDOR_ID_INTEL_0,
> > > >                       0x22D8),
> > > >                       .driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > > 
> > > At this point, wouldn't it make sense to apply the quirk to *all*
> > > ITE
> > > devices in Lenovo Yoga laptops?
> > 
> > I'm not sure If I got your suggestion right.
> > 
> > Those laptops do not share the same ITE chip. ITE is simply 
> > the vendor/manufacturer of the hid sensor hub. All three defined 
> > yoga laptops use a different ITE sensor hub model.
> > 
> > To make this clear, my device, the 
> > Lenovo Yoga 910 uses a ITE8186, 
> > where the Yoga 900 uses a ITE8396, 
> > the Yoga 2 a ITE8350 and so on
> > 
> > Now what "could" me done, is to detect and set the quirk
> > automatically. 
> > This should be doable, as the Hardware PID corresponds to the 
> > model number of the Sensor Hub (ex. ITE8186 = PID 0x8186)
> 
> I'm saying that if the vendor of the device is USB_VENDOR_ID_ITE and
> the manufacturer of the hardware is Lenovo in the DMI information, and
> the model of hardware contains "Yoga", that HID_SENSOR_HUB_ENUM_QUIRK
> be applied automatically, instead of adding quirks one-by-one.

First of all, I would like to appologise that my previous reply was
not caught by the mailing list. I had a frustrating time getting
mutt to work and decided to use the gmail web interface. Turns out
that was a bad idea. Fortunately it seems like I got things right 
this time.

Now to the automated ITE chip detection you've proposed. I likely lack
the driver and kernel knowledge to implement a feature of such. In the 
upcoming days I will to do some research on the HID drivers and see what 
I can do. The goal of this patch was to simply fix the issue for my device,
not to find a new or different solution.

Patrick

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Arek Burdach]

Hi Patrick,

On 15.07.2017 14:27, Patrick Pedersen wrote:
> It shall additionally be noted that the i2c-hid 'sleep' bug (present since kernel ver. 4.3)
> still affects the driver. This means that the sensor hub will not report any movement, until
> the device is suspended and resumed.
>
Do you have workaround for that? In my case suspending and resuming 
doesn't help. Sensor reporting is backing to work in unpredictable way. 
What I've tested:

- kernel v4.13-rc1 with your patch applied
- watch -n1 cat /sys/bus/iio/devices/iio\:device*/*_raw <- value not 
changing
- suspend
- resume
- watch -n1 cat /sys/bus/iio/devices/iio\:device*/*_raw <- still not 
changing

If you have some workaround scenario please add it to bug reported be 
me: https://bugzilla.kernel.org/show_bug.cgi?id=195681

Cheers,
Arek

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Brian Masney]

On Sun, Jul 16, 2017 at 09:39:19AM +0200, Arek Burdach wrote:
> Hi Patrick,
> 
> On 15.07.2017 14:27, Patrick Pedersen wrote:
> > It shall additionally be noted that the i2c-hid 'sleep' bug (present since kernel ver. 4.3)
> > still affects the driver. This means that the sensor hub will not report any movement, until
> > the device is suspended and resumed.
> > 
> Do you have workaround for that? In my case suspending and resuming doesn't
> help. Sensor reporting is backing to work in unpredictable way. What I've
> tested:
> 
> - kernel v4.13-rc1 with your patch applied
> - watch -n1 cat /sys/bus/iio/devices/iio\:device*/*_raw <- value not
> changing
> - suspend
> - resume
> - watch -n1 cat /sys/bus/iio/devices/iio\:device*/*_raw <- still not
> changing
> 
> If you have some workaround scenario please add it to bug reported be me:
> https://bugzilla.kernel.org/show_bug.cgi?id=195681

I have a Lenovo Yoga 910-13IKB and verified that the patch works
correctly for me on an upstream 4.12 kernel. The ALS sensor and
accelerometer function correctly without suspending the laptop with
this patch. The difference may be due the following kernel boot options
that I use:

	i915.enable_rc6=0 pci=noaer intel_pstate=disable

I have to disable power management for the CPU/GPU (i915.enable_rc6=0)
since I can only suspend and resume once on this laptop. Suspending a
second time will cause the laptop to hang before the suspend finishes.

The pci=noaer is likely not needed for UEFI firmware revisions 2JCN36WW
and newer. I'm stuck on UEFI firmware revision 2JCN28WW, which is what
came preinstalled on the system. Lenovo's UEFI update tool requires
Windows, which I no longer have.

I need to get on the latest UEFI revision and go through
basic-pm-debugging.txt if the power management issues are still present.

Brian

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Arek Burdach]

On 16.07.2017 12:23, Brian Masney wrote:
> On Sun, Jul 16, 2017 at 09:39:19AM +0200, Arek Burdach wrote:
>> Hi Patrick,
>>
>> On 15.07.2017 14:27, Patrick Pedersen wrote:
>>> It shall additionally be noted that the i2c-hid 'sleep' bug (present since kernel ver. 4.3)
>>> still affects the driver. This means that the sensor hub will not report any movement, until
>>> the device is suspended and resumed.
>>>
>> Do you have workaround for that? In my case suspending and resuming doesn't
>> help. Sensor reporting is backing to work in unpredictable way. What I've
>> tested:
>>
>> - kernel v4.13-rc1 with your patch applied
>> - watch -n1 cat /sys/bus/iio/devices/iio\:device*/*_raw <- value not
>> changing
>> - suspend
>> - resume
>> - watch -n1 cat /sys/bus/iio/devices/iio\:device*/*_raw <- still not
>> changing
>>
>> If you have some workaround scenario please add it to bug reported be me:
>> https://bugzilla.kernel.org/show_bug.cgi?id=195681
> I have a Lenovo Yoga 910-13IKB
So do I
>   and verified that the patch works
> correctly for me on an upstream 4.12 kernel. The ALS sensor and
> accelerometer function correctly without suspending the laptop with
> this patch. The difference may be due the following kernel boot options
> that I use:
>
> 	i915.enable_rc6=0 pci=noaer intel_pstate=disable
>
> I have to disable power management for the CPU/GPU (i915.enable_rc6=0)
> since I can only suspend and resume once on this laptop. Suspending a
> second time will cause the laptop to hang before the suspend finishes.
>
> The pci=noaer is likely not needed for UEFI firmware revisions 2JCN36WW
> and newer. I'm stuck on UEFI firmware revision 2JCN28WW, which is what
> came preinstalled on the system. Lenovo's UEFI update tool requires
> Windows, which I no longer have.
>
> I need to get on the latest UEFI revision and go through
> basic-pm-debugging.txt if the power management issues are still present.
I have installed 2JCN36WW and: pci=noaer intel_pstate=disable flags are 
never need - suspending works ok every time.

I investigated that having iio-sensor-proxy installed may cause sensors 
stopping to work. Looks like only one option can work in the same time - 
either iio-sensor-proxy service or sensor reporting of raw data.

Arek

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Srinivas Pandruvada]

On Sat, 2017-07-15 at 14:27 +0200, Patrick Pedersen wrote:
> As with previous generations of this device (see https://patchwork.ke
> rnel.org/patch/7887361/), the ITE
> HID Sensor Hub, responsible for the accelerometer and als sensor,
> requires a quirk entry.

I want to get rid of these quirks as this is now the defacto standard.
Can you try with this change without any quirk.

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 16ade0a..1fb407f 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -110,6 +110,9 @@ static int _hid_sensor_power_state(struct
hid_sensor_common *st, bool state)
        int report_val;
        s32 poll_value = 0;
 
+       st->power_state.logical_minimum = 1;
+       st->report_state.logical_minimum = 1;
+
        if (state) {
                if (!atomic_read(&st->user_requested_state))
                        return 0;


Thanks,
Srinivas

> 
> Without the entry, the Sensor Hub can't be accessed and the kernel
> fails to report any movements. As a result
> iio-sensor-proxy receives no new data.
> 
> It shall additionally be noted that the i2c-hid 'sleep' bug (present
> since kernel ver. 4.3)
> still affects the driver. This means that the sensor hub will not
> report any movement, until
> the device is suspended and resumed.
> 
> Signed-off-by: Patrick Pedersen <ctx.xda@gmail.com>
> ---
>  drivers/hid/hid-ids.h        | 1 +
>  drivers/hid/hid-sensor-hub.c | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 4f9a3938189a..b427a0bcfbe8 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -565,6 +565,7 @@
>  #define USB_DEVICE_ID_ITE_LENOVO_YOGA   0x8386
>  #define USB_DEVICE_ID_ITE_LENOVO_YOGA2  0x8350
>  #define USB_DEVICE_ID_ITE_LENOVO_YOGA900	0x8396
> +#define USB_DEVICE_ID_ITE_LENOVO_YOGA910	0x8186
>  
>  #define USB_VENDOR_ID_JABRA		0x0b0e
>  #define USB_DEVICE_ID_JABRA_SPEAK_410	0x0412
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> hub.c
> index 4ef73374a8f9..85b8425483bd 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -820,6 +820,9 @@ static const struct hid_device_id
> sensor_hub_devices[] = {
>  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_ITE,
>  			USB_DEVICE_ID_ITE_LENOVO_YOGA900),
>  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_ITE,
> +			USB_DEVICE_ID_ITE_LENOVO_YOGA910),
> +			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
>  	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> USB_VENDOR_ID_INTEL_0,
>  			0x22D8),
>  			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Srinivas Pandruvada]

On Sat, 2017-07-15 at 22:05 +0200, Patrick wrote:
> On Sat, Jul 15, 2017 at 07:58:10PM +0200, Bastien Nocera wrote:
> > 
> > On Sat, 2017-07-15 at 19:52 +0200, Patrick Pedersen wrote:
> > > 
> > > On Sat, Jul 15, 2017 at 4:29 PM, Bastien Nocera <hadess@hadess.ne
> > > t>
> > > wrote:
> > > > 
> > > > On Sat, 2017-07-15 at 14:27 +0200, Patrick Pedersen wrote:
> > > > > 
> > > > > As with previous generations of this device (see https://patc
> > > > > hwor
> > > > k.ke
> > > > > 
> > > > > rnel.org/patch/7887361/), the ITE
> > > > > HID Sensor Hub, responsible for the accelerometer and als
> > > > > sensor,
> > > > > requires a quirk entry.
> > > > > 
> > > > > Without the entry, the Sensor Hub can't be accessed and the
> > > > kernel
> > > > > 
> > > > > fails to report any movements. As a result
> > > > > iio-sensor-proxy receives no new data.
> > > > > 
> > > > > It shall additionally be noted that the i2c-hid 'sleep' bug
> > > > (present
> > > > > 
> > > > > since kernel ver. 4.3)
> > > > > still affects the driver. This means that the sensor hub will
> > > > > not
> > > > > report any movement, until
> > > > > the device is suspended and resumed.
Can you try some tests for this for test? I want to see if sensors are
powered off or transport didn't wake up? So forcing the sensors to keep
power on.

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 16ade0a..a70df7e 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -110,6 +110,9 @@ static int _hid_sensor_power_state(struct
hid_sensor_common *st, bool state)
        int report_val;
        s32 poll_value = 0;
 
+       st->power_state.logical_minimum = 1;
+       st->report_state.logical_minimum = 1;
+
        if (state) {
                if (!atomic_read(&st->user_requested_state))
                        return 0;
@@ -146,6 +149,9 @@ static int _hid_sensor_power_state(struct
hid_sensor_common *st, bool state)
                        HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS
_ENUM);
        }
 
+       state_val = 0;
+       report_val = 0;
+       
        if (state_val >= 0) {
                state_val += st->power_state.logical_minimum;
                sensor_hub_set_feature(st->hsdev, st-
>power_state.report_id,





> > > > > 
> > > > > Signed-off-by: Patrick Pedersen <ctx.xda@gmail.com>
> > > > > ---
> > > > >  drivers/hid/hid-ids.h        | 1 +
> > > > >  drivers/hid/hid-sensor-hub.c | 3 +++
> > > > >  2 files changed, 4 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > > > index 4f9a3938189a..b427a0bcfbe8 100644
> > > > > --- a/drivers/hid/hid-ids.h
> > > > > +++ b/drivers/hid/hid-ids.h
> > > > > @@ -565,6 +565,7 @@
> > > > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA   0x8386
> > > > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA2  0x8350
> > > > >  #define USB_DEVICE_ID_ITE_LENOVO_YOGA900     0x8396
> > > > > +#define USB_DEVICE_ID_ITE_LENOVO_YOGA910     0x8186
> > > > > 
> > > > >  #define USB_VENDOR_ID_JABRA          0x0b0e
> > > > >  #define USB_DEVICE_ID_JABRA_SPEAK_410        0x0412
> > > > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-
> > > > sensor-
> > > > > 
> > > > > hub.c
> > > > > index 4ef73374a8f9..85b8425483bd 100644
> > > > > --- a/drivers/hid/hid-sensor-hub.c
> > > > > +++ b/drivers/hid/hid-sensor-hub.c
> > > > > @@ -820,6 +820,9 @@ static const struct hid_device_id
> > > > > sensor_hub_devices[] = {
> > > > >       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > > > USB_VENDOR_ID_ITE,
> > > > >                       USB_DEVICE_ID_ITE_LENOVO_YOGA900),
> > > > >                       .driver_data =
> > > > > HID_SENSOR_HUB_ENUM_QUIRK},
> > > > > +     { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > > > USB_VENDOR_ID_ITE,
> > > > > +                     USB_DEVICE_ID_ITE_LENOVO_YOGA910),
> > > > > +                     .driver_data =
> > > > > HID_SENSOR_HUB_ENUM_QUIRK},
> > > > >       { HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > > > > USB_VENDOR_ID_INTEL_0,
> > > > >                       0x22D8),
> > > > >                       .driver_data =
> > > > > HID_SENSOR_HUB_ENUM_QUIRK},
> > > > At this point, wouldn't it make sense to apply the quirk to
> > > > *all*
> > > > ITE
> > > > devices in Lenovo Yoga laptops?
> > > I'm not sure If I got your suggestion right.
> > > 
> > > Those laptops do not share the same ITE chip. ITE is simply 
> > > the vendor/manufacturer of the hid sensor hub. All three defined 
> > > yoga laptops use a different ITE sensor hub model.
> > > 
> > > To make this clear, my device, the 
> > > Lenovo Yoga 910 uses a ITE8186, 
> > > where the Yoga 900 uses a ITE8396, 
> > > the Yoga 2 a ITE8350 and so on
> > > 
> > > Now what "could" me done, is to detect and set the quirk
> > > automatically. 
> > > This should be doable, as the Hardware PID corresponds to the 
> > > model number of the Sensor Hub (ex. ITE8186 = PID 0x8186)
> > I'm saying that if the vendor of the device is USB_VENDOR_ID_ITE
> > and
> > the manufacturer of the hardware is Lenovo in the DMI information,
> > and
> > the model of hardware contains "Yoga", that
> > HID_SENSOR_HUB_ENUM_QUIRK
> > be applied automatically, instead of adding quirks one-by-one.
> First of all, I would like to appologise that my previous reply was
> not caught by the mailing list. I had a frustrating time getting
> mutt to work and decided to use the gmail web interface. Turns out
> that was a bad idea. Fortunately it seems like I got things right 
> this time.
> 
> Now to the automated ITE chip detection you've proposed. I likely
> lack
> the driver and kernel knowledge to implement a feature of such. In
> the 
> upcoming days I will to do some research on the HID drivers and see
> what 
> I can do. The goal of this patch was to simply fix the issue for my
> device,
> not to find a new or different solution.
> 
> Patrick

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Bastien Nocera]

On Mon, 2017-07-17 at 13:19 -0700, Srinivas Pandruvada wrote:
> On Sat, 2017-07-15 at 22:05 +0200, Patrick wrote:
> > On Sat, Jul 15, 2017 at 07:58:10PM +0200, Bastien Nocera wrote:
> > > 
> > > On Sat, 2017-07-15 at 19:52 +0200, Patrick Pedersen wrote:
> > > > 
> > > > On Sat, Jul 15, 2017 at 4:29 PM, Bastien Nocera <hadess@hadess.
> > > > ne
> > > > t>
> > > > wrote:
> > > > > 
> > > > > On Sat, 2017-07-15 at 14:27 +0200, Patrick Pedersen wrote:
> > > > > > 
> > > > > > As with previous generations of this device (see https://pa
> > > > > > tc
> > > > > > hwor
> > > > > 
> > > > > k.ke
> > > > > > 
> > > > > > rnel.org/patch/7887361/), the ITE
> > > > > > HID Sensor Hub, responsible for the accelerometer and als
> > > > > > sensor,
> > > > > > requires a quirk entry.
> > > > > > 
> > > > > > Without the entry, the Sensor Hub can't be accessed and the
> > > > > 
> > > > > kernel
> > > > > > 
> > > > > > fails to report any movements. As a result
> > > > > > iio-sensor-proxy receives no new data.
> > > > > > 
> > > > > > It shall additionally be noted that the i2c-hid 'sleep' bug
> > > > > 
> > > > > (present
> > > > > > 
> > > > > > since kernel ver. 4.3)
> > > > > > still affects the driver. This means that the sensor hub
> > > > > > will
> > > > > > not
> > > > > > report any movement, until
> > > > > > the device is suspended and resumed.

Those are the same symptoms as the problem I reported here:
https://www.spinics.net/lists/linux-iio/msg29983.html

> Can you try some tests for this for test? I want to see if sensors
> are
> powered off or transport didn't wake up? So forcing the sensors to
> keep
> power on.

I tested the patch below using a HID sensor (ColorHug ALS) and it
failed in the same way with and without the patch you have below.

Hans also had an idea that this might be due to the PM suspend. The 3-
second sleep that you need to remove from iio-sensor-proxy matches the
default autosuspend delay at:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iio/common/hid-sensors/hid-sensor-trigger.c#n285

But setting "->common_attributes.data_ready" to 1 in each of the
driver's original states didn't work out.

Knowing that it affects both i2c and USB is a good bit of information
though, meaning that the problem probably lies in the IIO subsystem.

Or possibly a change of behaviour in the PM subsystem which made the
IIO trigger stop working. If Rafael could do a review on the code
linked above, that'd be really helpful.

Note that in addition to the patches I mentioned in the kernel thread
above, you'll need to revert this patch in iio-sensor-proxy:
https://github.com/hadess/iio-sensor-proxy/commit/5468452f7a72566d2e6ddc44f9396d3d088fa9fb

Otherwise things work :/

> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index 16ade0a..a70df7e 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -110,6 +110,9 @@ static int _hid_sensor_power_state(struct
> hid_sensor_common *st, bool state)
>         int report_val;
>         s32 poll_value = 0;
>  
> +       st->power_state.logical_minimum = 1;
> +       st->report_state.logical_minimum = 1;
> +
>         if (state) {
>                 if (!atomic_read(&st->user_requested_state))
>                         return 0;
> @@ -146,6 +149,9 @@ static int _hid_sensor_power_state(struct
> hid_sensor_common *st, bool state)
>                         HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVEN
> TS
> _ENUM);
>         }
>  
> +       state_val = 0;
> +       report_val = 0;
> +       
>         if (state_val >= 0) {
>                 state_val += st->power_state.logical_minimum;
>                 sensor_hub_set_feature(st->hsdev, st-
> > power_state.report_id,

Re: [PATCH] HID: Add quirk for Lenovo Yoga 910 with ITE Chips

[Patrick]

Sorry for the late response.

I can confirm that the ITE8186 hub successfully reports input data
on the latest linux-next release with the linked patch applied and no
quirks added.

On Mon, Jul 17, 2017 at 12:58:14PM -0700, Srinivas Pandruvada wrote:
> On Sat, 2017-07-15 at 14:27 +0200, Patrick Pedersen wrote:
> > As with previous generations of this device (see https://patchwork.ke
> > rnel.org/patch/7887361/), the ITE
> > HID Sensor Hub, responsible for the accelerometer and als sensor,
> > requires a quirk entry.
> 
> I want to get rid of these quirks as this is now the defacto standard.
> Can you try with this change without any quirk.
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index 16ade0a..1fb407f 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -110,6 +110,9 @@ static int _hid_sensor_power_state(struct
> hid_sensor_common *st, bool state)
> ????????????????int report_val;
> ????????????????s32 poll_value = 0;
> ??
> +??????????????st->power_state.logical_minimum = 1;
> +??????????????st->report_state.logical_minimum = 1;
> +
> ????????????????if (state) {
> ????????????????????????????????if (!atomic_read(&st->user_requested_state))
> ????????????????????????????????????????????????return 0;
> 
> 
> Thanks,
> Srinivas
> 
> > 
> > Without the entry, the Sensor Hub can't be accessed and the kernel
> > fails to report any movements. As a result
> > iio-sensor-proxy receives no new data.
> > 
> > It shall additionally be noted that the i2c-hid 'sleep' bug (present
> > since kernel ver. 4.3)
> > still affects the driver. This means that the sensor hub will not
> > report any movement, until
> > the device is suspended and resumed.
> > 
> > Signed-off-by: Patrick Pedersen <ctx.xda@gmail.com>
> > ---
> > ??drivers/hid/hid-ids.h????????????????| 1 +
> > ??drivers/hid/hid-sensor-hub.c | 3 +++
> > ??2 files changed, 4 insertions(+)
> > 
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index 4f9a3938189a..b427a0bcfbe8 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -565,6 +565,7 @@
> > ??#define USB_DEVICE_ID_ITE_LENOVO_YOGA??????0x8386
> > ??#define USB_DEVICE_ID_ITE_LENOVO_YOGA2????0x8350
> > ??#define USB_DEVICE_ID_ITE_LENOVO_YOGA900	0x8396
> > +#define USB_DEVICE_ID_ITE_LENOVO_YOGA910	0x8186
> > ??
> > ??#define USB_VENDOR_ID_JABRA		0x0b0e
> > ??#define USB_DEVICE_ID_JABRA_SPEAK_410	0x0412
> > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-
> > hub.c
> > index 4ef73374a8f9..85b8425483bd 100644
> > --- a/drivers/hid/hid-sensor-hub.c
> > +++ b/drivers/hid/hid-sensor-hub.c
> > @@ -820,6 +820,9 @@ static const struct hid_device_id
> > sensor_hub_devices[] = {
> > ??	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > USB_VENDOR_ID_ITE,
> > ??			USB_DEVICE_ID_ITE_LENOVO_YOGA900),
> > ??			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > +	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > USB_VENDOR_ID_ITE,
> > +			USB_DEVICE_ID_ITE_LENOVO_YOGA910),
> > +			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},
> > ??	{ HID_DEVICE(HID_BUS_ANY, HID_GROUP_SENSOR_HUB,
> > USB_VENDOR_ID_INTEL_0,
> > ??			0x22D8),
> > ??			.driver_data = HID_SENSOR_HUB_ENUM_QUIRK},