[RESEND] Lenovo Yoga 900 touchpad issues

[RESEND] Lenovo Yoga 900 touchpad issues

[Nish Aravamudan]

[Apologies for the resend, didn't realize I hadn't changed my GMail settings
to not use HTML.]

I have recently purchased a Lenovo Yoga 900 and most everything is working
with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
to enable the touchpad itself), I am seeing two issues:

1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
 modprobe i2c-hid` "fixes" it.

The kernel emits:

i2c_hid i2c-SYNA2B29:00: failed to reset device.
dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
PM: Device i2c-SYNA2B29:00 failed to resume: error -61

During the resume. So perhaps this is a timing issue (given that once
resumed, the
module reload does work?).

2) During boot, the following is emitted:

i2c_hid i2c-ITE8396:00: error in i2c_hid_init_report size:19 / ret_size:18

I'm not sure if this indicates a hardware or driver bug? With
i2c-hid.debug=1, I see:

i2c_hid i2c-ITE8396:00: report (len=19): 12 00 5a ff ff ff ff ff ff ff
ff ff ff ff ff ff ff ff 06
i2c_hid i2c-ITE8396:00: error in i2c_hid_init_report size:19 / ret_size:18

Any suggestions?

Thanks,
Nish

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Mika Westerberg]

On Tue, Dec 15, 2015 at 11:14:32AM -0800, Nish Aravamudan wrote:
> [Apologies for the resend, didn't realize I hadn't changed my GMail settings
> to not use HTML.]
> 
> I have recently purchased a Lenovo Yoga 900 and most everything is working
> with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
> to enable the touchpad itself), I am seeing two issues:
> 
> 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
>  modprobe i2c-hid` "fixes" it.
> 
> The kernel emits:
> 
> i2c_hid i2c-SYNA2B29:00: failed to reset device.
> dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
> PM: Device i2c-SYNA2B29:00 failed to resume: error -61
> 
> During the resume. So perhaps this is a timing issue (given that once
> resumed, the
> module reload does work?).

Linus noticed this as well and Jiri suggested the below patch which
seemed to fix the issue (although it increased resume time a bit).

I was supposed to get one Lenovo Yoga 900 here to debug this issue but
I'm still waiting for it (sloow big corporation bureaucracy takes some
time to get things purchased outside).

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 55d8f9d..52dd03a0 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1121,10 +1121,16 @@ static int i2c_hid_resume(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
 	struct hid_device *hid = ihid->hid;
-	int wake_status;
+	int wake_status, tries = 3;
 
 	enable_irq(ihid->irq);
-	ret = i2c_hid_hwreset(client);
+
+	do {
+		ret = i2c_hid_hwreset(client);
+		if (ret)
+			msleep(1000);
+	} while (tries-- > 0 && ret);
+
 	if (ret)
 		return ret;

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Jiri Kosina]

On Wed, 16 Dec 2015, Mika Westerberg wrote:

> > [Apologies for the resend, didn't realize I hadn't changed my GMail settings
> > to not use HTML.]
> > 
> > I have recently purchased a Lenovo Yoga 900 and most everything is working
> > with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
> > to enable the touchpad itself), I am seeing two issues:
> > 
> > 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
> >  modprobe i2c-hid` "fixes" it.
> > 
> > The kernel emits:
> > 
> > i2c_hid i2c-SYNA2B29:00: failed to reset device.
> > dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
> > PM: Device i2c-SYNA2B29:00 failed to resume: error -61
> > 
> > During the resume. So perhaps this is a timing issue (given that once
> > resumed, the
> > module reload does work?).
> 
> Linus noticed this as well and Jiri suggested the below patch which
> seemed to fix the issue (although it increased resume time a bit).
> 
> I was supposed to get one Lenovo Yoga 900 here to debug this issue but
> I'm still waiting for it (sloow big corporation bureaucracy takes some
> time to get things purchased outside).
> 
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 55d8f9d..52dd03a0 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -1121,10 +1121,16 @@ static int i2c_hid_resume(struct device *dev)
>  	struct i2c_client *client = to_i2c_client(dev);
>  	struct i2c_hid *ihid = i2c_get_clientdata(client);
>  	struct hid_device *hid = ihid->hid;
> -	int wake_status;
> +	int wake_status, tries = 3;
>  
>  	enable_irq(ihid->irq);
> -	ret = i2c_hid_hwreset(client);
> +
> +	do {
> +		ret = i2c_hid_hwreset(client);
> +		if (ret)
> +			msleep(1000);
> +	} while (tries-- > 0 && ret);
> +
>  	if (ret)
>  		return ret;

As a possible alternative, please test the patch above on top of for-next 
branch of 

	git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git

It contains 64bebefcf3 ("HID: enable hid device to suspend/resume 
asynchronously") and knowing whether that changes something might be 
interesting datapoint as well.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Nish Aravamudan]

Hi Jiri,

On Wed, Dec 16, 2015 at 5:18 AM, Jiri Kosina <jikos@kernel.org> wrote:
> On Wed, 16 Dec 2015, Mika Westerberg wrote:
>
>> > [Apologies for the resend, didn't realize I hadn't changed my GMail settings
>> > to not use HTML.]
>> >
>> > I have recently purchased a Lenovo Yoga 900 and most everything is working
>> > with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
>> > to enable the touchpad itself), I am seeing two issues:
>> >
>> > 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
>> >  modprobe i2c-hid` "fixes" it.
>> >
>> > The kernel emits:
>> >
>> > i2c_hid i2c-SYNA2B29:00: failed to reset device.
>> > dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
>> > PM: Device i2c-SYNA2B29:00 failed to resume: error -61
>> >
>> > During the resume. So perhaps this is a timing issue (given that once
>> > resumed, the
>> > module reload does work?).
>>
>> Linus noticed this as well and Jiri suggested the below patch which
>> seemed to fix the issue (although it increased resume time a bit).
>>
>> I was supposed to get one Lenovo Yoga 900 here to debug this issue but
>> I'm still waiting for it (sloow big corporation bureaucracy takes some
>> time to get things purchased outside).
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index 55d8f9d..52dd03a0 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -1121,10 +1121,16 @@ static int i2c_hid_resume(struct device *dev)
>>       struct i2c_client *client = to_i2c_client(dev);
>>       struct i2c_hid *ihid = i2c_get_clientdata(client);
>>       struct hid_device *hid = ihid->hid;
>> -     int wake_status;
>> +     int wake_status, tries = 3;
>>
>>       enable_irq(ihid->irq);
>> -     ret = i2c_hid_hwreset(client);
>> +
>> +     do {
>> +             ret = i2c_hid_hwreset(client);
>> +             if (ret)
>> +                     msleep(1000);
>> +     } while (tries-- > 0 && ret);
>> +
>>       if (ret)
>>               return ret;
>
> As a possible alternative, please test the patch above on top of for-next
> branch of
>
>         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
>
> It contains 64bebefcf3 ("HID: enable hid device to suspend/resume
> asynchronously") and knowing whether that changes something might be
> interesting datapoint as well.

What should I be looking for to be different wrt. that tree? Should I
not see the failure to reset the device? Or would it be (relatively)
speedier than the stock kernel?

Thanks,
Nish

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Benjamin Tissoires]

On Dec 16 2015 or thereabouts, Nish Aravamudan wrote:
> Hi Jiri,
> 
> On Wed, Dec 16, 2015 at 5:18 AM, Jiri Kosina <jikos@kernel.org> wrote:
> > On Wed, 16 Dec 2015, Mika Westerberg wrote:
> >
> >> > [Apologies for the resend, didn't realize I hadn't changed my GMail settings
> >> > to not use HTML.]
> >> >
> >> > I have recently purchased a Lenovo Yoga 900 and most everything is working
> >> > with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
> >> > to enable the touchpad itself), I am seeing two issues:
> >> >
> >> > 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
> >> >  modprobe i2c-hid` "fixes" it.
> >> >
> >> > The kernel emits:
> >> >
> >> > i2c_hid i2c-SYNA2B29:00: failed to reset device.
> >> > dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
> >> > PM: Device i2c-SYNA2B29:00 failed to resume: error -61
> >> >
> >> > During the resume. So perhaps this is a timing issue (given that once
> >> > resumed, the
> >> > module reload does work?).
> >>
> >> Linus noticed this as well and Jiri suggested the below patch which
> >> seemed to fix the issue (although it increased resume time a bit).
> >>
> >> I was supposed to get one Lenovo Yoga 900 here to debug this issue but
> >> I'm still waiting for it (sloow big corporation bureaucracy takes some
> >> time to get things purchased outside).
> >>
> >> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> >> index 55d8f9d..52dd03a0 100644
> >> --- a/drivers/hid/i2c-hid/i2c-hid.c
> >> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> >> @@ -1121,10 +1121,16 @@ static int i2c_hid_resume(struct device *dev)
> >>       struct i2c_client *client = to_i2c_client(dev);
> >>       struct i2c_hid *ihid = i2c_get_clientdata(client);
> >>       struct hid_device *hid = ihid->hid;
> >> -     int wake_status;
> >> +     int wake_status, tries = 3;
> >>
> >>       enable_irq(ihid->irq);
> >> -     ret = i2c_hid_hwreset(client);
> >> +
> >> +     do {
> >> +             ret = i2c_hid_hwreset(client);
> >> +             if (ret)
> >> +                     msleep(1000);
> >> +     } while (tries-- > 0 && ret);
> >> +
> >>       if (ret)
> >>               return ret;
> >
> > As a possible alternative, please test the patch above on top of for-next
> > branch of
> >
> >         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
> >
> > It contains 64bebefcf3 ("HID: enable hid device to suspend/resume
> > asynchronously") and knowing whether that changes something might be
> > interesting datapoint as well.
> 
> What should I be looking for to be different wrt. that tree? Should I
> not see the failure to reset the device? Or would it be (relatively)
> speedier than the stock kernel?
> 

Basically, does the resume time gets better? Linus experienced something
like 9 secs of resume before this patch, and now I hope it should be
saner (the touchpad might still be a little bit long to respond).

Also, we have a bug report concerning this laptop and it looks like the
i2c controller needs to be updated (see
https://bugzilla.redhat.com/show_bug.cgi?id=1275718).

I am not entirely sure why those on the rhbz needs to update the i2c
controller while you don't have to, but it may also be worth checking if
the i2c patch series mentioned in this rhbz (comment #20) fixes by itself
the resume issue (without Jiri's patch).

Cheers,
Benjamin

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Nish Aravamudan]

Hi Benjamin,

On Thu, Dec 17, 2015 at 1:53 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Dec 16 2015 or thereabouts, Nish Aravamudan wrote:
>> Hi Jiri,
>>
>> On Wed, Dec 16, 2015 at 5:18 AM, Jiri Kosina <jikos@kernel.org> wrote:
>> > On Wed, 16 Dec 2015, Mika Westerberg wrote:
>> >
>> >> > [Apologies for the resend, didn't realize I hadn't changed my GMail settings
>> >> > to not use HTML.]
>> >> >
>> >> > I have recently purchased a Lenovo Yoga 900 and most everything is working
>> >> > with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
>> >> > to enable the touchpad itself), I am seeing two issues:
>> >> >
>> >> > 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
>> >> >  modprobe i2c-hid` "fixes" it.
>> >> >
>> >> > The kernel emits:
>> >> >
>> >> > i2c_hid i2c-SYNA2B29:00: failed to reset device.
>> >> > dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
>> >> > PM: Device i2c-SYNA2B29:00 failed to resume: error -61
>> >> >
>> >> > During the resume. So perhaps this is a timing issue (given that once
>> >> > resumed, the
>> >> > module reload does work?).
>> >>
>> >> Linus noticed this as well and Jiri suggested the below patch which
>> >> seemed to fix the issue (although it increased resume time a bit).
>> >>
>> >> I was supposed to get one Lenovo Yoga 900 here to debug this issue but
>> >> I'm still waiting for it (sloow big corporation bureaucracy takes some
>> >> time to get things purchased outside).
>> >>
>> >> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> >> index 55d8f9d..52dd03a0 100644
>> >> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> >> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> >> @@ -1121,10 +1121,16 @@ static int i2c_hid_resume(struct device *dev)
>> >>       struct i2c_client *client = to_i2c_client(dev);
>> >>       struct i2c_hid *ihid = i2c_get_clientdata(client);
>> >>       struct hid_device *hid = ihid->hid;
>> >> -     int wake_status;
>> >> +     int wake_status, tries = 3;
>> >>
>> >>       enable_irq(ihid->irq);
>> >> -     ret = i2c_hid_hwreset(client);
>> >> +
>> >> +     do {
>> >> +             ret = i2c_hid_hwreset(client);
>> >> +             if (ret)
>> >> +                     msleep(1000);
>> >> +     } while (tries-- > 0 && ret);
>> >> +
>> >>       if (ret)
>> >>               return ret;
>> >
>> > As a possible alternative, please test the patch above on top of for-next
>> > branch of
>> >
>> >         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
>> >
>> > It contains 64bebefcf3 ("HID: enable hid device to suspend/resume
>> > asynchronously") and knowing whether that changes something might be
>> > interesting datapoint as well.
>>
>> What should I be looking for to be different wrt. that tree? Should I
>> not see the failure to reset the device? Or would it be (relatively)
>> speedier than the stock kernel?
>>
>
> Basically, does the resume time gets better? Linus experienced something
> like 9 secs of resume before this patch, and now I hope it should be
> saner (the touchpad might still be a little bit long to respond).
>
> Also, we have a bug report concerning this laptop and it looks like the
> i2c controller needs to be updated (see
> https://bugzilla.redhat.com/show_bug.cgi?id=1275718).
>
> I am not entirely sure why those on the rhbz needs to update the i2c
> controller while you don't have to, but it may also be worth checking if
> the i2c patch series mentioned in this rhbz (comment #20) fixes by itself
> the resume issue (without Jiri's patch).

Yep, I applied the series from https://lkml.org/lkml/2015/11/30/441
already. Without that series, I don't have a working touchscreen and
touchpad. Touchpad resume doesn't work with just that series.

With that series + Jiri's patch it seems to resume pretty quickly. But
I guess I don't see exactly what would change with the resume if the
same loop is present on top of hid:for-next? Still on my list to test,
though.

Also, even with that series, there does appear maybe there is an I2C
issue, though:

i2c_hid i2c-ITE8396:00: error in i2c_hid_init_report size:19 / ret_size:18

as I mentioned in my first e-mail; which might be affecting the IIO sensors?

-Nish

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Benjamin Tissoires]

On Dec 17 2015 or thereabouts, Nish Aravamudan wrote:
> Hi Benjamin,
> 
> On Thu, Dec 17, 2015 at 1:53 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > On Dec 16 2015 or thereabouts, Nish Aravamudan wrote:
> >> Hi Jiri,
> >>
> >> On Wed, Dec 16, 2015 at 5:18 AM, Jiri Kosina <jikos@kernel.org> wrote:
> >> > On Wed, 16 Dec 2015, Mika Westerberg wrote:
> >> >
> >> >> > [Apologies for the resend, didn't realize I hadn't changed my GMail settings
> >> >> > to not use HTML.]
> >> >> >
> >> >> > I have recently purchased a Lenovo Yoga 900 and most everything is working
> >> >> > with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
> >> >> > to enable the touchpad itself), I am seeing two issues:
> >> >> >
> >> >> > 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
> >> >> >  modprobe i2c-hid` "fixes" it.
> >> >> >
> >> >> > The kernel emits:
> >> >> >
> >> >> > i2c_hid i2c-SYNA2B29:00: failed to reset device.
> >> >> > dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
> >> >> > PM: Device i2c-SYNA2B29:00 failed to resume: error -61
> >> >> >
> >> >> > During the resume. So perhaps this is a timing issue (given that once
> >> >> > resumed, the
> >> >> > module reload does work?).
> >> >>
> >> >> Linus noticed this as well and Jiri suggested the below patch which
> >> >> seemed to fix the issue (although it increased resume time a bit).
> >> >>
> >> >> I was supposed to get one Lenovo Yoga 900 here to debug this issue but
> >> >> I'm still waiting for it (sloow big corporation bureaucracy takes some
> >> >> time to get things purchased outside).
> >> >>
> >> >> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> >> >> index 55d8f9d..52dd03a0 100644
> >> >> --- a/drivers/hid/i2c-hid/i2c-hid.c
> >> >> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> >> >> @@ -1121,10 +1121,16 @@ static int i2c_hid_resume(struct device *dev)
> >> >>       struct i2c_client *client = to_i2c_client(dev);
> >> >>       struct i2c_hid *ihid = i2c_get_clientdata(client);
> >> >>       struct hid_device *hid = ihid->hid;
> >> >> -     int wake_status;
> >> >> +     int wake_status, tries = 3;
> >> >>
> >> >>       enable_irq(ihid->irq);
> >> >> -     ret = i2c_hid_hwreset(client);
> >> >> +
> >> >> +     do {
> >> >> +             ret = i2c_hid_hwreset(client);
> >> >> +             if (ret)
> >> >> +                     msleep(1000);
> >> >> +     } while (tries-- > 0 && ret);
> >> >> +
> >> >>       if (ret)
> >> >>               return ret;
> >> >
> >> > As a possible alternative, please test the patch above on top of for-next
> >> > branch of
> >> >
> >> >         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
> >> >
> >> > It contains 64bebefcf3 ("HID: enable hid device to suspend/resume
> >> > asynchronously") and knowing whether that changes something might be
> >> > interesting datapoint as well.
> >>
> >> What should I be looking for to be different wrt. that tree? Should I
> >> not see the failure to reset the device? Or would it be (relatively)
> >> speedier than the stock kernel?
> >>
> >
> > Basically, does the resume time gets better? Linus experienced something
> > like 9 secs of resume before this patch, and now I hope it should be
> > saner (the touchpad might still be a little bit long to respond).
> >
> > Also, we have a bug report concerning this laptop and it looks like the
> > i2c controller needs to be updated (see
> > https://bugzilla.redhat.com/show_bug.cgi?id=1275718).
> >
> > I am not entirely sure why those on the rhbz needs to update the i2c
> > controller while you don't have to, but it may also be worth checking if
> > the i2c patch series mentioned in this rhbz (comment #20) fixes by itself
> > the resume issue (without Jiri's patch).
> 
> Yep, I applied the series from https://lkml.org/lkml/2015/11/30/441
> already. Without that series, I don't have a working touchscreen and
> touchpad. Touchpad resume doesn't work with just that series.

Oh, OK then. Now I start understanding the whole thing.

> 
> With that series + Jiri's patch it seems to resume pretty quickly. But
> I guess I don't see exactly what would change with the resume if the
> same loop is present on top of hid:for-next? Still on my list to test,
> though.

The difference is that now, if the touchpad takes 5 seconds to resume,
it should not block the whole resume time given that it is run in its
own thread.

> 
> Also, even with that series, there does appear maybe there is an I2C
> issue, though:
> 
> i2c_hid i2c-ITE8396:00: error in i2c_hid_init_report size:19 / ret_size:18

I wouldn't worry too much about that. The device does not correctly answer,
but most of the time, this is harmless.

> 
> as I mentioned in my first e-mail; which might be affecting the IIO sensors?

If iio-sensor-proxy works under gnome, there is no need to panic :)

Cheers,
Benjamin

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Nish Aravamudan]

On Thu, Dec 17, 2015 at 9:28 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> On Dec 17 2015 or thereabouts, Nish Aravamudan wrote:
>> Hi Benjamin,
>>
>> On Thu, Dec 17, 2015 at 1:53 AM, Benjamin Tissoires
>> <benjamin.tissoires@redhat.com> wrote:
>> > On Dec 16 2015 or thereabouts, Nish Aravamudan wrote:
>> >> Hi Jiri,
>> >>
>> >> On Wed, Dec 16, 2015 at 5:18 AM, Jiri Kosina <jikos@kernel.org> wrote:
>> >> > On Wed, 16 Dec 2015, Mika Westerberg wrote:
>> >> >
>> >> >> > [Apologies for the resend, didn't realize I hadn't changed my GMail settings
>> >> >> > to not use HTML.]
>> >> >> >
>> >> >> > I have recently purchased a Lenovo Yoga 900 and most everything is working
>> >> >> > with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
>> >> >> > to enable the touchpad itself), I am seeing two issues:
>> >> >> >
>> >> >> > 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
>> >> >> >  modprobe i2c-hid` "fixes" it.
>> >> >> >
>> >> >> > The kernel emits:
>> >> >> >
>> >> >> > i2c_hid i2c-SYNA2B29:00: failed to reset device.
>> >> >> > dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
>> >> >> > PM: Device i2c-SYNA2B29:00 failed to resume: error -61
>> >> >> >
>> >> >> > During the resume. So perhaps this is a timing issue (given that once
>> >> >> > resumed, the
>> >> >> > module reload does work?).
>> >> >>
>> >> >> Linus noticed this as well and Jiri suggested the below patch which
>> >> >> seemed to fix the issue (although it increased resume time a bit).
>> >> >>
>> >> >> I was supposed to get one Lenovo Yoga 900 here to debug this issue but
>> >> >> I'm still waiting for it (sloow big corporation bureaucracy takes some
>> >> >> time to get things purchased outside).
>> >> >>
>> >> >> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> >> >> index 55d8f9d..52dd03a0 100644
>> >> >> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> >> >> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> >> >> @@ -1121,10 +1121,16 @@ static int i2c_hid_resume(struct device *dev)
>> >> >>       struct i2c_client *client = to_i2c_client(dev);
>> >> >>       struct i2c_hid *ihid = i2c_get_clientdata(client);
>> >> >>       struct hid_device *hid = ihid->hid;
>> >> >> -     int wake_status;
>> >> >> +     int wake_status, tries = 3;
>> >> >>
>> >> >>       enable_irq(ihid->irq);
>> >> >> -     ret = i2c_hid_hwreset(client);
>> >> >> +
>> >> >> +     do {
>> >> >> +             ret = i2c_hid_hwreset(client);
>> >> >> +             if (ret)
>> >> >> +                     msleep(1000);
>> >> >> +     } while (tries-- > 0 && ret);
>> >> >> +
>> >> >>       if (ret)
>> >> >>               return ret;
>> >> >
>> >> > As a possible alternative, please test the patch above on top of for-next
>> >> > branch of
>> >> >
>> >> >         git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
>> >> >
>> >> > It contains 64bebefcf3 ("HID: enable hid device to suspend/resume
>> >> > asynchronously") and knowing whether that changes something might be
>> >> > interesting datapoint as well.
>> >>
>> >> What should I be looking for to be different wrt. that tree? Should I
>> >> not see the failure to reset the device? Or would it be (relatively)
>> >> speedier than the stock kernel?
>> >>
>> >
>> > Basically, does the resume time gets better? Linus experienced something
>> > like 9 secs of resume before this patch, and now I hope it should be
>> > saner (the touchpad might still be a little bit long to respond).
>> >
>> > Also, we have a bug report concerning this laptop and it looks like the
>> > i2c controller needs to be updated (see
>> > https://bugzilla.redhat.com/show_bug.cgi?id=1275718).
>> >
>> > I am not entirely sure why those on the rhbz needs to update the i2c
>> > controller while you don't have to, but it may also be worth checking if
>> > the i2c patch series mentioned in this rhbz (comment #20) fixes by itself
>> > the resume issue (without Jiri's patch).
>>
>> Yep, I applied the series from https://lkml.org/lkml/2015/11/30/441
>> already. Without that series, I don't have a working touchscreen and
>> touchpad. Touchpad resume doesn't work with just that series.
>
> Oh, OK then. Now I start understanding the whole thing.
>
>>
>> With that series + Jiri's patch it seems to resume pretty quickly. But
>> I guess I don't see exactly what would change with the resume if the
>> same loop is present on top of hid:for-next? Still on my list to test,
>> though.
>
> The difference is that now, if the touchpad takes 5 seconds to resume,
> it should not block the whole resume time given that it is run in its
> own thread.

Ah, I see! Ok, will test and time with and without hid:for-next.

>> Also, even with that series, there does appear maybe there is an I2C
>> issue, though:
>>
>> i2c_hid i2c-ITE8396:00: error in i2c_hid_init_report size:19 / ret_size:18
>
> I wouldn't worry too much about that. The device does not correctly answer,
> but most of the time, this is harmless.
>
>>
>> as I mentioned in my first e-mail; which might be affecting the IIO sensors?
>
> If iio-sensor-proxy works under gnome, there is no need to panic :)

Heh, that's why I actually started digging into a separate thread,
iio-sensor-proxy not working on this device :)

-Nish

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Nish Aravamudan]

On Wed, Dec 16, 2015 at 1:28 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Tue, Dec 15, 2015 at 11:14:32AM -0800, Nish Aravamudan wrote:
>> [Apologies for the resend, didn't realize I hadn't changed my GMail settings
>> to not use HTML.]
>>
>> I have recently purchased a Lenovo Yoga 900 and most everything is working
>> with a slightly modified 4.4-rc5 (https://lkml.org/lkml/2015/11/30/441 applied
>> to enable the touchpad itself), I am seeing two issues:
>>
>> 1) On suspend/resume, the touchpad is non-functional. A `modprobe -r i2c-hid;
>>  modprobe i2c-hid` "fixes" it.
>>
>> The kernel emits:
>>
>> i2c_hid i2c-SYNA2B29:00: failed to reset device.
>> dpm_run_callback(): i2c_hid_resume+0x0/0xc0 [i2c_hid] returns -61
>> PM: Device i2c-SYNA2B29:00 failed to resume: error -61
>>
>> During the resume. So perhaps this is a timing issue (given that once
>> resumed, the
>> module reload does work?).
>
> Linus noticed this as well and Jiri suggested the below patch which
> seemed to fix the issue (although it increased resume time a bit).
>
> I was supposed to get one Lenovo Yoga 900 here to debug this issue but
> I'm still waiting for it (sloow big corporation bureaucracy takes some
> time to get things purchased outside).

With the patch applied to my patched 4.4-rc5, things seem to be
working now. I do get one "failed to reset device" message in the
logs, but then I'm guessing the second one succeeds and I don't see
the "failed to resume" message.

-Nish

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Mika Westerberg]

On Wed, Dec 16, 2015 at 02:58:11PM -0800, Nish Aravamudan wrote:
> With the patch applied to my patched 4.4-rc5, things seem to be
> working now. I do get one "failed to reset device" message in the
> logs, but then I'm guessing the second one succeeds and I don't see
> the "failed to resume" message.

I finally got the yoga 900 machine.

This is what happens when I have "i2c_hid.debug=1" in the kernel command
line (I stripped out non-related messages and added few more debug
prints):

<resume by pressing power button>

  [   72.410282] i2c_hid i2c-SYNA2B29:00: enabled IRQ
  [   72.410285] i2c_hid i2c-SYNA2B29:00: i2c_hid_hwreset
  [   72.410288] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_power
  [   72.410291] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 00 08
  [   72.412879] i2c_hid i2c-SYNA2B29:00: resetting...
  [   72.412883] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 00 01

Now we sent out reset command and wait for the interrupt to happen.

  [   72.413248] i2c_hid i2c-SYNA2B29:00: input: 06 00 01 00 00 00

We get interrupt here but the received message did not have first two
bytes zeroed as expected by the driver and the i2c-hid spec so we think
it is an input report!

  [   72.413266] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_or_send_report
  [   72.413270] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 3f 03 0f 23 00 04 00 0f 01

  [   72.413416] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: waiting...

Here we start actually waiting for the interrupt which already
happened.

  [   72.413751] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_or_send_report
  [   72.413755] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=25 00 17 00 09 01 42 00 2e 00 19 19 00 10 cc 06 74 04 0f 19 00 00 00 00 00
  [   77.413135] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: finished.
  [   77.413137] i2c_hid i2c-SYNA2B29:00: failed to reset device.

And after 5 seconds we report error.

  [   77.413138] i2c_hid i2c-SYNA2B29:00: i2c_hid_set_power
  [   77.413139] i2c_hid i2c-SYNA2B29:00: __i2c_hid_command: cmd=22 00 01 08
  [   77.415030] i2c_hid i2c-SYNA2B29:00: reset returned: -61
  [   77.415035] dpm_run_callback(): i2c_hid_resume+0x0/0xe0 returns -61
  [   77.415037] PM: Device i2c-SYNA2B29:00 failed to resume: error -61

On another occasion the faulty input report was received immediatelly
after we call i2c_hid_set_power().

With below hack patch suspend/resume works fine but it is far from being
suitable for merging. Still, it would be nice if you could try it out
and see if it helps in your case.

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 10bd8e6..1c6969f 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -354,14 +354,19 @@ static int i2c_hid_hwreset(struct i2c_client *client)
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
 	int ret;
 
+	disable_irq(ihid->irq);
+
 	i2c_hid_dbg(ihid, "%s\n", __func__);
 
 	ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
-	if (ret)
+	if (ret) {
+		enable_irq(ihid->irq);
 		return ret;
+	}
 
 	i2c_hid_dbg(ihid, "resetting...\n");
 
+	enable_irq(ihid->irq);
 	ret = i2c_hid_command(client, &hid_reset_cmd, NULL, 0);
 	if (ret) {
 		dev_err(&client->dev, "failed to reset device.\n");
@@ -390,16 +395,14 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
 		return;
 	}
 
-	ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
-
-	if (!ret_size) {
-		/* host or device initiated RESET completed */
-		if (test_and_clear_bit(I2C_HID_RESET_PENDING, &ihid->flags))
-			wake_up(&ihid->wait);
+	/* host or device initiated RESET completed */
+	if (test_and_clear_bit(I2C_HID_RESET_PENDING, &ihid->flags)) {
+		wake_up(&ihid->wait);
 		return;
 	}
 
-	if (ret_size > size) {
+	ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
+	if (ret_size > size || !ret_size) {
 		dev_err(&ihid->client->dev, "%s: incomplete report (%d/%d)\n",
 			__func__, size, ret_size);
 		return;

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Mika Westerberg]

On Fri, Dec 18, 2015 at 04:42:09PM +0200, Mika Westerberg wrote:
> On another occasion the faulty input report was received immediatelly
> after we call i2c_hid_set_power().
> 
> With below hack patch suspend/resume works fine but it is far from being
> suitable for merging. Still, it would be nice if you could try it out
> and see if it helps in your case.

Actually it looks like we can handle this by just ignoring input reports
while we are resetting the device. Following seems to work as well.

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 10bd8e6..eeaf33a 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -352,13 +352,24 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
 static int i2c_hid_hwreset(struct i2c_client *client)
 {
 	struct i2c_hid *ihid = i2c_get_clientdata(client);
+	bool started;
 	int ret;
 
 	i2c_hid_dbg(ihid, "%s\n", __func__);
 
+	/*
+	 * Some touchpads seem to send input reports once their power is
+	 * turned back on after resume. This confuses our reset logic
+	 * below.
+	 *
+	 * Prevent handling any input reports while we are resetting the
+	 * device.
+	 */
+	started = test_and_clear_bit(I2C_HID_STARTED, &ihid->flags);
+
 	ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
 	if (ret)
-		return ret;
+		goto out;
 
 	i2c_hid_dbg(ihid, "resetting...\n");
 
@@ -366,10 +377,14 @@ static int i2c_hid_hwreset(struct i2c_client *client)
 	if (ret) {
 		dev_err(&client->dev, "failed to reset device.\n");
 		i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
-		return ret;
+		goto out;
 	}
 
-	return 0;
+out:
+	if (started)
+		set_bit(I2C_HID_STARTED, &ihid->flags);
+
+	return ret;
 }
 
 static void i2c_hid_get_input(struct i2c_hid *ihid)

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Benjamin Tissoires]

On Dec 18 2015 or thereabouts, Mika Westerberg wrote:
> On Fri, Dec 18, 2015 at 04:42:09PM +0200, Mika Westerberg wrote:
> > On another occasion the faulty input report was received immediatelly
> > after we call i2c_hid_set_power().
> > 
> > With below hack patch suspend/resume works fine but it is far from being
> > suitable for merging. Still, it would be nice if you could try it out
> > and see if it helps in your case.
> 
> Actually it looks like we can handle this by just ignoring input reports
> while we are resetting the device. Following seems to work as well.

Yes, I like this one better. I think, it is still prone to races, but
it's a much better option.

After turning this around, I think I finally got what was going on (and
yes, it's basically a race that should have been caught long ago):
- during resume, the i2c-hid driver calls reset, which is a long (few
  ms) operation.
- hid-multitouch is also called during resume, and right after i2c-hid
- hid-multiotuch immediately emits for touchpads a set input mode (this
  was unseen with touchscreens because they do not have to be switched
  into a proper mode)
- there is a race between hid-multitouch accessing the features while
  the device is still resetting. And our reset interrupt never gets to
  us because there was an other operation in progress to request/set
  reports

So the actual fix would be to make hid-multitouch wait until we reset
i2c-hid.

This should be done by either a mutex or a spinlock in
i2c_hid_output_raw_report() and i2c_hid_resume() that would protect a
flag set during suspend and cleared after resume.

(not sure if this is clear :-P )

Cheers,
Benjamin

> 
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 10bd8e6..eeaf33a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -352,13 +352,24 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
>  static int i2c_hid_hwreset(struct i2c_client *client)
>  {
>  	struct i2c_hid *ihid = i2c_get_clientdata(client);
> +	bool started;
>  	int ret;
>  
>  	i2c_hid_dbg(ihid, "%s\n", __func__);
>  
> +	/*
> +	 * Some touchpads seem to send input reports once their power is
> +	 * turned back on after resume. This confuses our reset logic
> +	 * below.
> +	 *
> +	 * Prevent handling any input reports while we are resetting the
> +	 * device.
> +	 */
> +	started = test_and_clear_bit(I2C_HID_STARTED, &ihid->flags);
> +
>  	ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
>  	if (ret)
> -		return ret;
> +		goto out;
>  
>  	i2c_hid_dbg(ihid, "resetting...\n");
>  
> @@ -366,10 +377,14 @@ static int i2c_hid_hwreset(struct i2c_client *client)
>  	if (ret) {
>  		dev_err(&client->dev, "failed to reset device.\n");
>  		i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
> -		return ret;
> +		goto out;
>  	}
>  
> -	return 0;
> +out:
> +	if (started)
> +		set_bit(I2C_HID_STARTED, &ihid->flags);
> +
> +	return ret;
>  }
>  
>  static void i2c_hid_get_input(struct i2c_hid *ihid)

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Mika Westerberg]

On Fri, Dec 18, 2015 at 05:10:31PM +0100, Benjamin Tissoires wrote:
> After turning this around, I think I finally got what was going on (and
> yes, it's basically a race that should have been caught long ago):
> - during resume, the i2c-hid driver calls reset, which is a long (few
>   ms) operation.
> - hid-multitouch is also called during resume, and right after i2c-hid
> - hid-multiotuch immediately emits for touchpads a set input mode (this
>   was unseen with touchscreens because they do not have to be switched
>   into a proper mode)
> - there is a race between hid-multitouch accessing the features while
>   the device is still resetting. And our reset interrupt never gets to
>   us because there was an other operation in progress to request/set
>   reports
> 
> So the actual fix would be to make hid-multitouch wait until we reset
> i2c-hid.

The driver used for touchpad is actually hid-rmi but you are right, it
tries to set rmi mode (or something) during its rmi_post_reset() that
then confuses the i2c-hid driver.

Thanks for the analysis :-)

> This should be done by either a mutex or a spinlock in
> i2c_hid_output_raw_report() and i2c_hid_resume() that would protect a
> flag set during suspend and cleared after resume.

Makes sense. I'll make a patch that does this and submit it soon.

Re: [RESEND] Lenovo Yoga 900 touchpad issues

[Nish Aravamudan]

On Fri, Dec 18, 2015 at 7:38 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> On Fri, Dec 18, 2015 at 04:42:09PM +0200, Mika Westerberg wrote:
>> On another occasion the faulty input report was received immediatelly
>> after we call i2c_hid_set_power().
>>
>> With below hack patch suspend/resume works fine but it is far from being
>> suitable for merging. Still, it would be nice if you could try it out
>> and see if it helps in your case.
>
> Actually it looks like we can handle this by just ignoring input reports
> while we are resetting the device. Following seems to work as well.

FWIW,

Tested-by: Nishanth Aravamudan <nish.aravamudan@gmail.com>

and if you can Cc me on any follow-up/alternative patches, I will be
happy to test those too!

Thanks!
-Nish

> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 10bd8e6..eeaf33a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -352,13 +352,24 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
>  static int i2c_hid_hwreset(struct i2c_client *client)
>  {
>         struct i2c_hid *ihid = i2c_get_clientdata(client);
> +       bool started;
>         int ret;
>
>         i2c_hid_dbg(ihid, "%s\n", __func__);
>
> +       /*
> +        * Some touchpads seem to send input reports once their power is
> +        * turned back on after resume. This confuses our reset logic
> +        * below.
> +        *
> +        * Prevent handling any input reports while we are resetting the
> +        * device.
> +        */
> +       started = test_and_clear_bit(I2C_HID_STARTED, &ihid->flags);
> +
>         ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
>         if (ret)
> -               return ret;
> +               goto out;
>
>         i2c_hid_dbg(ihid, "resetting...\n");
>
> @@ -366,10 +377,14 @@ static int i2c_hid_hwreset(struct i2c_client *client)
>         if (ret) {
>                 dev_err(&client->dev, "failed to reset device.\n");
>                 i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
> -               return ret;
> +               goto out;
>         }
>
> -       return 0;
> +out:
> +       if (started)
> +               set_bit(I2C_HID_STARTED, &ihid->flags);
> +
> +       return ret;
>  }
>
>  static void i2c_hid_get_input(struct i2c_hid *ihid)