Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] HID: hid-sensor-hub: Add logical min and max
From: Jonathan Cameron @ 2013-12-02 21:07 UTC (permalink / raw)
  To: Jonathan Cameron, Jiri Kosina
  Cc: Srinivas Pandruvada, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <b2e31404-966e-4f1a-a85b-a01fa63064b9-2ueSQiBKiTY7tOexoI0I+QC/G2K4zDHf@public.gmane.org>

On 12/02/13 17:14, Jonathan Cameron wrote:
> Hi Jiri
>
> I tend to push a testing branch to catch any build issues before pushing to the real togreg branch. Normally there are only a few hours on between but sometimes it is a few days as here.  I will move these to the fixes-togreg branch - hopefully this evening...

Now applied to the fixes-togreg branch (this one does get pushed out to kernel.org
directly!)
>
> Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org> wrote:
>> On Sat, 30 Nov 2013, Jonathan Cameron wrote:
>>
>>> On 11/27/13 22:19, Srinivas Pandruvada wrote:
>>>> Exporting logical minimum and maximum of HID fields as part of the
>>>> hid sensor attribute info. This can be used for range checking and
>>>> to calculate enumeration base for NAry fields of HID sensor hub.
>>>>
>>>> Signed-off-by: Srinivas Pandruvada
>> <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>> Hi,  I would have preffred this being done in two patches, the first
>>> refactoring the function call and the second doing the min and max.
>>>
>>> Anyhow, I'll take it anyway.
>>>
>>> Applied to the togreg branch of iio.git.
>> Where is that? I don't seem to see it in 
>>
>> 	https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/log/?h=togreg

^ permalink raw reply

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Manuel Krause @ 2013-12-02 20:40 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Peter Hurley, linux-kernel, Greg KH, linux-input, linux-serial,
	Shuah Khan, Rafael J. Wysocki
In-Reply-To: <20131202190730.GA5245@core.coreip.homeip.net>

On 2013-12-02 20:07, Dmitry Torokhov wrote:
> On Mon, Dec 02, 2013 at 07:35:47PM +0100, Manuel Krause wrote:
>> On 2013-12-02 17:45, Dmitry Torokhov wrote:
>>> On Mon, Dec 02, 2013 at 08:38:16AM -0800, Dmitry Torokhov wrote:
>>>> On Monday, December 02, 2013 05:08:28 PM Manuel Krause wrote:
>>>>> On 2013-12-01 16:43, Peter Hurley wrote:
>>>>>> [ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
>>>>>> linux-serial ]
>>>>>>
>>>>>> On 11/26/2013 05:19 PM, Manuel Krause wrote:
>>>>>>> Since kernel 3.12.0 I have a problem with hibernate+resume
>>>>>>> not reactivating my serial mouse (trackball) with my HP notebook.
>>>>>>> Kernels 3.11.0 til 9 don't show this behaviour.
>>>>>>>
>>>>>>> Machine:           HP Notebook with Core2Duo CPU (Penryn)
>>>>>>> Distro:            openSUSE 12.3, 64bit, continuously updated
>>>>>>> Desktop:           KDE 4.11.3
>>>>>>> MESA & drm & Xorg: most recent ones from:
>>>>>>>
>>>>>>> http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_
>>>>>>> 12.3/x86_64/
>>>>>>>
>>>>>>> Current kernel:    3.12.1 vanilla from openSUSE repos, with
>>>>>>>
>>>>>>>                      -ck1 and BFQ patches
>>>>>>>
>>>>>>> The Logitech Trackman Marble FX is a PS/2 device and connected
>>>>>>> via an original Logitech
>>>>>>> PS/2-COM-port adapter and manually configured via my xorg.conf.
>>>>>>>
>>>>>>> At first, I blamed the -ck1 patches from Con Kolivas for this
>>>>>>> behaviour that I use in
>>>>>>> addition  to the BFQ patches, what has showed up as not right:
>>>>>>> This happens with the
>>>>>>> normal vanilla kernel
>>>>>>> schedulers for CPU and disk I/O, too.
>>>>>>>
>>>>>>> By coincidence I found a weird(!) way to reactivate the serial
>>>>>>> mouse:
>>>>>>> (1) call Hibernate (suspend-to-disk) from KDE desktop as normal
>>>>>>> (2) resume --> the PS/2 touchpad is working, the serial
>>>>>>> trackball NOT
>>>>>>> (3) call suspend-to-RAM (Sleep) from KDE, serial trackball
>>>>>>> still dead
>>>>>>> (4) execute `setserial -a /dev/ttyS0` in a konsole window or a
>>>>>>> tty* console
>>>>>>> (5) ==> serial trackball is back with all configuration from
>>>>>>> xorg.conf
>>>>>>>
>>>>>>> It's fully reproducible over multiple hibernations. This also
>>>>>>> happens when calling
>>>>>>> `pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
>>>>>>> setserial from a root shell
>>>>>>> in KDE or any tty*.
>>>>>>>
>>>>>>> Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
>>>>>>
>>>>>> Manuel,
>>>>>>
>>>>>> Please attach complete dmesgs (zipped, if necessary) of a
>>>>>> suspend/resume cycle
>>>>>> on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
>>>>>> (where resume succeeds).
>>>>>>
>>>>>> For the test configurations, please do not apply patches.
>>>>>>
>>>>>> Regards,
>>>>>> Peter Hurley
>>>>>
>>>>> Thank you very much for your reply!
>>>>> Attached you'll find a zip file with the two edited dmesg logs of
>>>>> plain vanilla kernel runs.
>>>>>
>>>>> I have to add, that the resumes _do_ succeed in both cases, only
>>>>> the serial mouse doesn't get activated after hibernate in 3.12.x
>>>>> automatically. Just scan for and compare the lines indicating
>>>>> "serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x
>>>>> the activation doesn't happen after hibernate, but after
>>>>> suspend-to-ram (sleep). That only after STR and not before a
>>>>> setserial gets my mouse back... a miracle. ;-)
>>>>
>>>> I do not see
>>>>
>>>>> [  206.577370] serial 00:08: activated
>>>>
>>>> in the restore from hibernation log of 3.12 which shoudl come from PNP
>>>> layer.
>>>>
>>>> [dtor@dtor-d630 work]$ git log --oneline v3.11..v3.12 -- drivers/pnp/
>>>> 729377d pnp: change pnp bus pm_ops to invoke pnp driver dev_pm_ops if specified
>>>> ce63e18 Merge branch 'pnp'
>>>> 8ad928d ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere
>>>> eaf140b PNP: convert PNP driver bus legacy pm_ops to dev_pm_ops
>>>>
>>>> I'd start looking into these commits.
>>>>
>>>
>>> Does the following patch fixes the issue?
>>>
>>
>> PNP: fix restoring devices after hibernation
>>
>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> On returning from hibernation 'restore; callback is called, not
>> 'resume'.
>> This fixes breakage introduced by commit
>> eaf140b60ec961252083ab8adaf67aef29a362dd
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> ---
>>   drivers/pnp/driver.c |    3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
>> index a39ee38..185a24a 100644
>> --- a/drivers/pnp/driver.c
>> +++ b/drivers/pnp/driver.c
>> @@ -235,8 +235,9 @@ static int pnp_bus_resume(struct device *dev)
>>
>>   static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
>>   	.suspend = pnp_bus_suspend,
>> -	.freeze = pnp_bus_freeze,
>>   	.resume = pnp_bus_resume,
>> +	.freeze = pnp_bus_freeze,
>> +	.restore = pnp_bus_resume,
>>   };
>>
>>   struct bus_type pnp_bus_type = {
>>
>>
>> YES! This patch fixes the issue!!! (Even if compiled with a patched
>> kernel.)   ;-)
>>
>> Many thanks for your work!
>>
>
> Thank you Manuel, but IO think the patch is not complete as we need to
> re-enable PNP devices after we make a snapshot to make sure they are
> working and can handle saving the data. Coudl you please try the patch
> below?
>

PNP: fix restoring devices after hibernation

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

On returning from hibernation 'restore; callback is called, not 
'resume'.
This fixes breakage introduced by commit
eaf140b60ec961252083ab8adaf67aef29a362dd

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
  drivers/pnp/driver.c |   12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index a39ee38..2bd5c5f 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -197,6 +197,11 @@ static int pnp_bus_freeze(struct device *dev)
  	return __pnp_bus_suspend(dev, PMSG_FREEZE);
  }

+static int pnp_bus_poweroff(struct device *dev)
+{
+	return __pnp_bus_suspend(dev, PMSG_HIBERNATE);
+}
+
  static int pnp_bus_resume(struct device *dev)
  {
  	struct pnp_dev *pnp_dev = to_pnp_dev(dev);
@@ -234,9 +239,14 @@ static int pnp_bus_resume(struct device *dev)
  }

  static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
+	/* Suspend callbacks */
  	.suspend = pnp_bus_suspend,
-	.freeze = pnp_bus_freeze,
  	.resume = pnp_bus_resume,
+	/* Hibernate callbacks */
+	.freeze = pnp_bus_freeze,
+	.thaw = pnp_bus_resume,
+	.poweroff = pnp_bus_poweroff,
+	.restore = pnp_bus_resume,
  };

  struct bus_type pnp_bus_type = {

ALSO, YES, this second patch version does cure the issue, on 
here, too.

Best regards, Manuel Krause

^ permalink raw reply related

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Shuah Khan @ 2013-12-02 19:30 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Manuel Krause, Peter Hurley, linux-kernel, Greg KH, linux-input,
	linux-serial, Rafael J. Wysocki, shuahkhan@gmail.com, Shuah Khan
In-Reply-To: <20131202190824.GB5245@core.coreip.homeip.net>

On 12/02/2013 12:08 PM, Dmitry Torokhov wrote:
> On Mon, Dec 02, 2013 at 11:44:56AM -0700, Shuah Khan wrote:

>>>
>>
>> I am glad the problem is fixed. But I am puzzled. pnp_bus_resume()
>> didn't handle restore prior to this change. state doesn't get passed
>> in to legacy resume routines. I had to add freeze to handle
>> PMSG_FREEZE case, sounds like restore is needed as well, however I
>> don't see where how restore is handled prior to this change.
>
> Take a look at drivers/base/platform.c::platform_pm_restore()
>
> Thanks.
>

Yes I see it now. Before the conversion, platform_legacy_resume() path 
is taken since pnp didn't have pm.

        if (drv->pm) {
                 if (drv->pm->restore)
                         ret = drv->pm->restore(dev);
        } else {
                 ret = platform_legacy_resume(dev);
         }

Thanks for fixing the problem.

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

^ permalink raw reply

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Dmitry Torokhov @ 2013-12-02 19:08 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Manuel Krause, Peter Hurley, linux-kernel, Greg KH, linux-input,
	linux-serial, Rafael J. Wysocki, shuahkhan@gmail.com
In-Reply-To: <529CD528.9000709@samsung.com>

On Mon, Dec 02, 2013 at 11:44:56AM -0700, Shuah Khan wrote:
> On 12/02/2013 11:35 AM, Manuel Krause wrote:
> >On 2013-12-02 17:45, Dmitry Torokhov wrote:
> >>On Mon, Dec 02, 2013 at 08:38:16AM -0800, Dmitry Torokhov wrote:
> >>>On Monday, December 02, 2013 05:08:28 PM Manuel Krause wrote:
> >>>>On 2013-12-01 16:43, Peter Hurley wrote:
> >>>>>[ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
> >>>>>linux-serial ]
> >>>>>
> >>>>>On 11/26/2013 05:19 PM, Manuel Krause wrote:
> >>>>>>Since kernel 3.12.0 I have a problem with hibernate+resume
> >>>>>>not reactivating my serial mouse (trackball) with my HP notebook.
> >>>>>>Kernels 3.11.0 til 9 don't show this behaviour.
> >>>>>>
> >>>>>>Machine:           HP Notebook with Core2Duo CPU (Penryn)
> >>>>>>Distro:            openSUSE 12.3, 64bit, continuously updated
> >>>>>>Desktop:           KDE 4.11.3
> >>>>>>MESA & drm & Xorg: most recent ones from:
> >>>>>>
> >>>>>>http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_
> >>>>>>
> >>>>>>12.3/x86_64/
> >>>>>>
> >>>>>>Current kernel:    3.12.1 vanilla from openSUSE repos, with
> >>>>>>
> >>>>>>                     -ck1 and BFQ patches
> >>>>>>
> >>>>>>The Logitech Trackman Marble FX is a PS/2 device and connected
> >>>>>>via an original Logitech
> >>>>>>PS/2-COM-port adapter and manually configured via my xorg.conf.
> >>>>>>
> >>>>>>At first, I blamed the -ck1 patches from Con Kolivas for this
> >>>>>>behaviour that I use in
> >>>>>>addition  to the BFQ patches, what has showed up as not right:
> >>>>>>This happens with the
> >>>>>>normal vanilla kernel
> >>>>>>schedulers for CPU and disk I/O, too.
> >>>>>>
> >>>>>>By coincidence I found a weird(!) way to reactivate the serial
> >>>>>>mouse:
> >>>>>>(1) call Hibernate (suspend-to-disk) from KDE desktop as normal
> >>>>>>(2) resume --> the PS/2 touchpad is working, the serial
> >>>>>>trackball NOT
> >>>>>>(3) call suspend-to-RAM (Sleep) from KDE, serial trackball
> >>>>>>still dead
> >>>>>>(4) execute `setserial -a /dev/ttyS0` in a konsole window or a
> >>>>>>tty* console
> >>>>>>(5) ==> serial trackball is back with all configuration from
> >>>>>>xorg.conf
> >>>>>>
> >>>>>>It's fully reproducible over multiple hibernations. This also
> >>>>>>happens when calling
> >>>>>>`pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
> >>>>>>setserial from a root shell
> >>>>>>in KDE or any tty*.
> >>>>>>
> >>>>>>Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
> >>>>>
> >>>>>Manuel,
> >>>>>
> >>>>>Please attach complete dmesgs (zipped, if necessary) of a
> >>>>>suspend/resume cycle
> >>>>>on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
> >>>>>(where resume succeeds).
> >>>>>
> >>>>>For the test configurations, please do not apply patches.
> >>>>>
> >>>>>Regards,
> >>>>>Peter Hurley
> >>>>
> >>>>Thank you very much for your reply!
> >>>>Attached you'll find a zip file with the two edited dmesg logs of
> >>>>plain vanilla kernel runs.
> >>>>
> >>>>I have to add, that the resumes _do_ succeed in both cases, only
> >>>>the serial mouse doesn't get activated after hibernate in 3.12.x
> >>>>automatically. Just scan for and compare the lines indicating
> >>>>"serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x
> >>>>the activation doesn't happen after hibernate, but after
> >>>>suspend-to-ram (sleep). That only after STR and not before a
> >>>>setserial gets my mouse back... a miracle. ;-)
> >>>
> >>>I do not see
> >>>
> >>>>[  206.577370] serial 00:08: activated
> >>>
> >>>in the restore from hibernation log of 3.12 which shoudl come from PNP
> >>>layer.
> >>>
> >>>[dtor@dtor-d630 work]$ git log --oneline v3.11..v3.12 -- drivers/pnp/
> >>>729377d pnp: change pnp bus pm_ops to invoke pnp driver dev_pm_ops if
> >>>specified
> >>>ce63e18 Merge branch 'pnp'
> >>>8ad928d ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3
> >>>everywhere
> >>>eaf140b PNP: convert PNP driver bus legacy pm_ops to dev_pm_ops
> >>>
> >>>I'd start looking into these commits.
> >>>
> >>
> >>Does the following patch fixes the issue?
> >>
> >
> >PNP: fix restoring devices after hibernation
> >
> >From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> >On returning from hibernation 'restore; callback is called, not 'resume'.
> >This fixes breakage introduced by commit
> >eaf140b60ec961252083ab8adaf67aef29a362dd
> >
> >Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >---
> >  drivers/pnp/driver.c |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
> >index a39ee38..185a24a 100644
> >--- a/drivers/pnp/driver.c
> >+++ b/drivers/pnp/driver.c
> >@@ -235,8 +235,9 @@ static int pnp_bus_resume(struct device *dev)
> >
> >  static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
> >      .suspend = pnp_bus_suspend,
> >-    .freeze = pnp_bus_freeze,
> >      .resume = pnp_bus_resume,
> >+    .freeze = pnp_bus_freeze,
> >+    .restore = pnp_bus_resume,
> >  };
> >
> >  struct bus_type pnp_bus_type = {
> >
> >
> >YES! This patch fixes the issue!!! (Even if compiled with a patched
> >kernel.)   ;-)
> >
> >Many thanks for your work!
> >
> >Manuel Krause
> >
> 
> I am glad the problem is fixed. But I am puzzled. pnp_bus_resume()
> didn't handle restore prior to this change. state doesn't get passed
> in to legacy resume routines. I had to add freeze to handle
> PMSG_FREEZE case, sounds like restore is needed as well, however I
> don't see where how restore is handled prior to this change.

Take a look at drivers/base/platform.c::platform_pm_restore()

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Dmitry Torokhov @ 2013-12-02 19:07 UTC (permalink / raw)
  To: Manuel Krause
  Cc: Peter Hurley, linux-kernel, Greg KH, linux-input, linux-serial,
	Shuah Khan, Rafael J. Wysocki
In-Reply-To: <529CD303.4040103@netscape.net>

On Mon, Dec 02, 2013 at 07:35:47PM +0100, Manuel Krause wrote:
> On 2013-12-02 17:45, Dmitry Torokhov wrote:
> >On Mon, Dec 02, 2013 at 08:38:16AM -0800, Dmitry Torokhov wrote:
> >>On Monday, December 02, 2013 05:08:28 PM Manuel Krause wrote:
> >>>On 2013-12-01 16:43, Peter Hurley wrote:
> >>>>[ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
> >>>>linux-serial ]
> >>>>
> >>>>On 11/26/2013 05:19 PM, Manuel Krause wrote:
> >>>>>Since kernel 3.12.0 I have a problem with hibernate+resume
> >>>>>not reactivating my serial mouse (trackball) with my HP notebook.
> >>>>>Kernels 3.11.0 til 9 don't show this behaviour.
> >>>>>
> >>>>>Machine:           HP Notebook with Core2Duo CPU (Penryn)
> >>>>>Distro:            openSUSE 12.3, 64bit, continuously updated
> >>>>>Desktop:           KDE 4.11.3
> >>>>>MESA & drm & Xorg: most recent ones from:
> >>>>>
> >>>>>http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_
> >>>>>12.3/x86_64/
> >>>>>
> >>>>>Current kernel:    3.12.1 vanilla from openSUSE repos, with
> >>>>>
> >>>>>                     -ck1 and BFQ patches
> >>>>>
> >>>>>The Logitech Trackman Marble FX is a PS/2 device and connected
> >>>>>via an original Logitech
> >>>>>PS/2-COM-port adapter and manually configured via my xorg.conf.
> >>>>>
> >>>>>At first, I blamed the -ck1 patches from Con Kolivas for this
> >>>>>behaviour that I use in
> >>>>>addition  to the BFQ patches, what has showed up as not right:
> >>>>>This happens with the
> >>>>>normal vanilla kernel
> >>>>>schedulers for CPU and disk I/O, too.
> >>>>>
> >>>>>By coincidence I found a weird(!) way to reactivate the serial
> >>>>>mouse:
> >>>>>(1) call Hibernate (suspend-to-disk) from KDE desktop as normal
> >>>>>(2) resume --> the PS/2 touchpad is working, the serial
> >>>>>trackball NOT
> >>>>>(3) call suspend-to-RAM (Sleep) from KDE, serial trackball
> >>>>>still dead
> >>>>>(4) execute `setserial -a /dev/ttyS0` in a konsole window or a
> >>>>>tty* console
> >>>>>(5) ==> serial trackball is back with all configuration from
> >>>>>xorg.conf
> >>>>>
> >>>>>It's fully reproducible over multiple hibernations. This also
> >>>>>happens when calling
> >>>>>`pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
> >>>>>setserial from a root shell
> >>>>>in KDE or any tty*.
> >>>>>
> >>>>>Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
> >>>>
> >>>>Manuel,
> >>>>
> >>>>Please attach complete dmesgs (zipped, if necessary) of a
> >>>>suspend/resume cycle
> >>>>on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
> >>>>(where resume succeeds).
> >>>>
> >>>>For the test configurations, please do not apply patches.
> >>>>
> >>>>Regards,
> >>>>Peter Hurley
> >>>
> >>>Thank you very much for your reply!
> >>>Attached you'll find a zip file with the two edited dmesg logs of
> >>>plain vanilla kernel runs.
> >>>
> >>>I have to add, that the resumes _do_ succeed in both cases, only
> >>>the serial mouse doesn't get activated after hibernate in 3.12.x
> >>>automatically. Just scan for and compare the lines indicating
> >>>"serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x
> >>>the activation doesn't happen after hibernate, but after
> >>>suspend-to-ram (sleep). That only after STR and not before a
> >>>setserial gets my mouse back... a miracle. ;-)
> >>
> >>I do not see
> >>
> >>>[  206.577370] serial 00:08: activated
> >>
> >>in the restore from hibernation log of 3.12 which shoudl come from PNP
> >>layer.
> >>
> >>[dtor@dtor-d630 work]$ git log --oneline v3.11..v3.12 -- drivers/pnp/
> >>729377d pnp: change pnp bus pm_ops to invoke pnp driver dev_pm_ops if specified
> >>ce63e18 Merge branch 'pnp'
> >>8ad928d ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere
> >>eaf140b PNP: convert PNP driver bus legacy pm_ops to dev_pm_ops
> >>
> >>I'd start looking into these commits.
> >>
> >
> >Does the following patch fixes the issue?
> >
> 
> PNP: fix restoring devices after hibernation
> 
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> On returning from hibernation 'restore; callback is called, not
> 'resume'.
> This fixes breakage introduced by commit
> eaf140b60ec961252083ab8adaf67aef29a362dd
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/pnp/driver.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
> index a39ee38..185a24a 100644
> --- a/drivers/pnp/driver.c
> +++ b/drivers/pnp/driver.c
> @@ -235,8 +235,9 @@ static int pnp_bus_resume(struct device *dev)
> 
>  static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
>  	.suspend = pnp_bus_suspend,
> -	.freeze = pnp_bus_freeze,
>  	.resume = pnp_bus_resume,
> +	.freeze = pnp_bus_freeze,
> +	.restore = pnp_bus_resume,
>  };
> 
>  struct bus_type pnp_bus_type = {
> 
> 
> YES! This patch fixes the issue!!! (Even if compiled with a patched
> kernel.)   ;-)
> 
> Many thanks for your work!
> 

Thank you Manuel, but IO think the patch is not complete as we need to
re-enable PNP devices after we make a snapshot to make sure they are
working and can handle saving the data. Coudl you please try the patch
below?

-- 
Dmitry


PNP: fix restoring devices after hibernation

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

On returning from hibernation 'restore; callback is called, not 'resume'.
This fixes breakage introduced by commit
eaf140b60ec961252083ab8adaf67aef29a362dd

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/pnp/driver.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index a39ee38..2bd5c5f 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -197,6 +197,11 @@ static int pnp_bus_freeze(struct device *dev)
 	return __pnp_bus_suspend(dev, PMSG_FREEZE);
 }
 
+static int pnp_bus_poweroff(struct device *dev)
+{
+	return __pnp_bus_suspend(dev, PMSG_HIBERNATE);
+}
+
 static int pnp_bus_resume(struct device *dev)
 {
 	struct pnp_dev *pnp_dev = to_pnp_dev(dev);
@@ -234,9 +239,14 @@ static int pnp_bus_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
+	/* Suspend callbacks */
 	.suspend = pnp_bus_suspend,
-	.freeze = pnp_bus_freeze,
 	.resume = pnp_bus_resume,
+	/* Hibernate callbacks */
+	.freeze = pnp_bus_freeze,
+	.thaw = pnp_bus_resume,
+	.poweroff = pnp_bus_poweroff,
+	.restore = pnp_bus_resume,
 };
 
 struct bus_type pnp_bus_type = {

^ permalink raw reply related

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Shuah Khan @ 2013-12-02 18:44 UTC (permalink / raw)
  To: Manuel Krause, Dmitry Torokhov
  Cc: Peter Hurley, linux-kernel, Greg KH, linux-input, linux-serial,
	Rafael J. Wysocki, Shuah Khan, shuahkhan@gmail.com
In-Reply-To: <529CD303.4040103@netscape.net>

On 12/02/2013 11:35 AM, Manuel Krause wrote:
> On 2013-12-02 17:45, Dmitry Torokhov wrote:
>> On Mon, Dec 02, 2013 at 08:38:16AM -0800, Dmitry Torokhov wrote:
>>> On Monday, December 02, 2013 05:08:28 PM Manuel Krause wrote:
>>>> On 2013-12-01 16:43, Peter Hurley wrote:
>>>>> [ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
>>>>> linux-serial ]
>>>>>
>>>>> On 11/26/2013 05:19 PM, Manuel Krause wrote:
>>>>>> Since kernel 3.12.0 I have a problem with hibernate+resume
>>>>>> not reactivating my serial mouse (trackball) with my HP notebook.
>>>>>> Kernels 3.11.0 til 9 don't show this behaviour.
>>>>>>
>>>>>> Machine:           HP Notebook with Core2Duo CPU (Penryn)
>>>>>> Distro:            openSUSE 12.3, 64bit, continuously updated
>>>>>> Desktop:           KDE 4.11.3
>>>>>> MESA & drm & Xorg: most recent ones from:
>>>>>>
>>>>>> http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_
>>>>>>
>>>>>> 12.3/x86_64/
>>>>>>
>>>>>> Current kernel:    3.12.1 vanilla from openSUSE repos, with
>>>>>>
>>>>>>                      -ck1 and BFQ patches
>>>>>>
>>>>>> The Logitech Trackman Marble FX is a PS/2 device and connected
>>>>>> via an original Logitech
>>>>>> PS/2-COM-port adapter and manually configured via my xorg.conf.
>>>>>>
>>>>>> At first, I blamed the -ck1 patches from Con Kolivas for this
>>>>>> behaviour that I use in
>>>>>> addition  to the BFQ patches, what has showed up as not right:
>>>>>> This happens with the
>>>>>> normal vanilla kernel
>>>>>> schedulers for CPU and disk I/O, too.
>>>>>>
>>>>>> By coincidence I found a weird(!) way to reactivate the serial
>>>>>> mouse:
>>>>>> (1) call Hibernate (suspend-to-disk) from KDE desktop as normal
>>>>>> (2) resume --> the PS/2 touchpad is working, the serial
>>>>>> trackball NOT
>>>>>> (3) call suspend-to-RAM (Sleep) from KDE, serial trackball
>>>>>> still dead
>>>>>> (4) execute `setserial -a /dev/ttyS0` in a konsole window or a
>>>>>> tty* console
>>>>>> (5) ==> serial trackball is back with all configuration from
>>>>>> xorg.conf
>>>>>>
>>>>>> It's fully reproducible over multiple hibernations. This also
>>>>>> happens when calling
>>>>>> `pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
>>>>>> setserial from a root shell
>>>>>> in KDE or any tty*.
>>>>>>
>>>>>> Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
>>>>>
>>>>> Manuel,
>>>>>
>>>>> Please attach complete dmesgs (zipped, if necessary) of a
>>>>> suspend/resume cycle
>>>>> on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
>>>>> (where resume succeeds).
>>>>>
>>>>> For the test configurations, please do not apply patches.
>>>>>
>>>>> Regards,
>>>>> Peter Hurley
>>>>
>>>> Thank you very much for your reply!
>>>> Attached you'll find a zip file with the two edited dmesg logs of
>>>> plain vanilla kernel runs.
>>>>
>>>> I have to add, that the resumes _do_ succeed in both cases, only
>>>> the serial mouse doesn't get activated after hibernate in 3.12.x
>>>> automatically. Just scan for and compare the lines indicating
>>>> "serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x
>>>> the activation doesn't happen after hibernate, but after
>>>> suspend-to-ram (sleep). That only after STR and not before a
>>>> setserial gets my mouse back... a miracle. ;-)
>>>
>>> I do not see
>>>
>>>> [  206.577370] serial 00:08: activated
>>>
>>> in the restore from hibernation log of 3.12 which shoudl come from PNP
>>> layer.
>>>
>>> [dtor@dtor-d630 work]$ git log --oneline v3.11..v3.12 -- drivers/pnp/
>>> 729377d pnp: change pnp bus pm_ops to invoke pnp driver dev_pm_ops if
>>> specified
>>> ce63e18 Merge branch 'pnp'
>>> 8ad928d ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3
>>> everywhere
>>> eaf140b PNP: convert PNP driver bus legacy pm_ops to dev_pm_ops
>>>
>>> I'd start looking into these commits.
>>>
>>
>> Does the following patch fixes the issue?
>>
>
> PNP: fix restoring devices after hibernation
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> On returning from hibernation 'restore; callback is called, not 'resume'.
> This fixes breakage introduced by commit
> eaf140b60ec961252083ab8adaf67aef29a362dd
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>   drivers/pnp/driver.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
> index a39ee38..185a24a 100644
> --- a/drivers/pnp/driver.c
> +++ b/drivers/pnp/driver.c
> @@ -235,8 +235,9 @@ static int pnp_bus_resume(struct device *dev)
>
>   static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
>       .suspend = pnp_bus_suspend,
> -    .freeze = pnp_bus_freeze,
>       .resume = pnp_bus_resume,
> +    .freeze = pnp_bus_freeze,
> +    .restore = pnp_bus_resume,
>   };
>
>   struct bus_type pnp_bus_type = {
>
>
> YES! This patch fixes the issue!!! (Even if compiled with a patched
> kernel.)   ;-)
>
> Many thanks for your work!
>
> Manuel Krause
>

I am glad the problem is fixed. But I am puzzled. pnp_bus_resume() 
didn't handle restore prior to this change. state doesn't get passed in 
to legacy resume routines. I had to add freeze to handle PMSG_FREEZE 
case, sounds like restore is needed as well, however I don't see where 
how restore is handled prior to this change.

-- Shuah

-- 
Shuah Khan
Senior Linux Kernel Developer - Open Source Group
Samsung Research America(Silicon Valley)
shuah.kh@samsung.com | (970) 672-0658

^ permalink raw reply

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Manuel Krause @ 2013-12-02 18:35 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Peter Hurley, linux-kernel, Greg KH, linux-input, linux-serial,
	Shuah Khan, Rafael J. Wysocki
In-Reply-To: <20131202164525.GA32406@core.coreip.homeip.net>

On 2013-12-02 17:45, Dmitry Torokhov wrote:
> On Mon, Dec 02, 2013 at 08:38:16AM -0800, Dmitry Torokhov wrote:
>> On Monday, December 02, 2013 05:08:28 PM Manuel Krause wrote:
>>> On 2013-12-01 16:43, Peter Hurley wrote:
>>>> [ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
>>>> linux-serial ]
>>>>
>>>> On 11/26/2013 05:19 PM, Manuel Krause wrote:
>>>>> Since kernel 3.12.0 I have a problem with hibernate+resume
>>>>> not reactivating my serial mouse (trackball) with my HP notebook.
>>>>> Kernels 3.11.0 til 9 don't show this behaviour.
>>>>>
>>>>> Machine:           HP Notebook with Core2Duo CPU (Penryn)
>>>>> Distro:            openSUSE 12.3, 64bit, continuously updated
>>>>> Desktop:           KDE 4.11.3
>>>>> MESA & drm & Xorg: most recent ones from:
>>>>>
>>>>> http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_
>>>>> 12.3/x86_64/
>>>>>
>>>>> Current kernel:    3.12.1 vanilla from openSUSE repos, with
>>>>>
>>>>>                      -ck1 and BFQ patches
>>>>>
>>>>> The Logitech Trackman Marble FX is a PS/2 device and connected
>>>>> via an original Logitech
>>>>> PS/2-COM-port adapter and manually configured via my xorg.conf.
>>>>>
>>>>> At first, I blamed the -ck1 patches from Con Kolivas for this
>>>>> behaviour that I use in
>>>>> addition  to the BFQ patches, what has showed up as not right:
>>>>> This happens with the
>>>>> normal vanilla kernel
>>>>> schedulers for CPU and disk I/O, too.
>>>>>
>>>>> By coincidence I found a weird(!) way to reactivate the serial
>>>>> mouse:
>>>>> (1) call Hibernate (suspend-to-disk) from KDE desktop as normal
>>>>> (2) resume --> the PS/2 touchpad is working, the serial
>>>>> trackball NOT
>>>>> (3) call suspend-to-RAM (Sleep) from KDE, serial trackball
>>>>> still dead
>>>>> (4) execute `setserial -a /dev/ttyS0` in a konsole window or a
>>>>> tty* console
>>>>> (5) ==> serial trackball is back with all configuration from
>>>>> xorg.conf
>>>>>
>>>>> It's fully reproducible over multiple hibernations. This also
>>>>> happens when calling
>>>>> `pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
>>>>> setserial from a root shell
>>>>> in KDE or any tty*.
>>>>>
>>>>> Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
>>>>
>>>> Manuel,
>>>>
>>>> Please attach complete dmesgs (zipped, if necessary) of a
>>>> suspend/resume cycle
>>>> on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
>>>> (where resume succeeds).
>>>>
>>>> For the test configurations, please do not apply patches.
>>>>
>>>> Regards,
>>>> Peter Hurley
>>>
>>> Thank you very much for your reply!
>>> Attached you'll find a zip file with the two edited dmesg logs of
>>> plain vanilla kernel runs.
>>>
>>> I have to add, that the resumes _do_ succeed in both cases, only
>>> the serial mouse doesn't get activated after hibernate in 3.12.x
>>> automatically. Just scan for and compare the lines indicating
>>> "serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x
>>> the activation doesn't happen after hibernate, but after
>>> suspend-to-ram (sleep). That only after STR and not before a
>>> setserial gets my mouse back... a miracle. ;-)
>>
>> I do not see
>>
>>> [  206.577370] serial 00:08: activated
>>
>> in the restore from hibernation log of 3.12 which shoudl come from PNP
>> layer.
>>
>> [dtor@dtor-d630 work]$ git log --oneline v3.11..v3.12 -- drivers/pnp/
>> 729377d pnp: change pnp bus pm_ops to invoke pnp driver dev_pm_ops if specified
>> ce63e18 Merge branch 'pnp'
>> 8ad928d ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere
>> eaf140b PNP: convert PNP driver bus legacy pm_ops to dev_pm_ops
>>
>> I'd start looking into these commits.
>>
>
> Does the following patch fixes the issue?
>

PNP: fix restoring devices after hibernation

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

On returning from hibernation 'restore; callback is called, not 
'resume'.
This fixes breakage introduced by commit
eaf140b60ec961252083ab8adaf67aef29a362dd

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
  drivers/pnp/driver.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index a39ee38..185a24a 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -235,8 +235,9 @@ static int pnp_bus_resume(struct device *dev)

  static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
  	.suspend = pnp_bus_suspend,
-	.freeze = pnp_bus_freeze,
  	.resume = pnp_bus_resume,
+	.freeze = pnp_bus_freeze,
+	.restore = pnp_bus_resume,
  };

  struct bus_type pnp_bus_type = {


YES! This patch fixes the issue!!! (Even if compiled with a 
patched kernel.)   ;-)

Many thanks for your work!

Manuel Krause


^ permalink raw reply related

* Re: [PATCH] hid: add NOGET quirk and device id for Logitech Dual Action gamepads support
From: Zawullon @ 2013-12-02 17:57 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1312021512460.24931@pobox.suse.cz>



02.12.2013 18:13, Jiri Kosina wrote:

> Please provide your Signed-off-by: line so that I could apply the patch. 
> Thanks.

Oh, of course. Sorry.

Signed-off-by: Vitaly Katraew <zawullon@gmail.com>


^ permalink raw reply

* Re: [PATCH 1/2] HID: hid-sensor-hub: Add logical min and max
From: Jonathan Cameron @ 2013-12-02 17:14 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Srinivas Pandruvada, linux-input, linux-iio
In-Reply-To: <alpine.LNX.2.00.1312021408290.24931@pobox.suse.cz>

Hi Jiri

I tend to push a testing branch to catch any build issues before pushing to the real togreg branch. Normally there are only a few hours on between but sometimes it is a few days as here.  I will move these to the fixes-togreg branch - hopefully this evening...

Jiri Kosina <jkosina@suse.cz> wrote:
>On Sat, 30 Nov 2013, Jonathan Cameron wrote:
>
>> On 11/27/13 22:19, Srinivas Pandruvada wrote:
>> > Exporting logical minimum and maximum of HID fields as part of the
>> > hid sensor attribute info. This can be used for range checking and
>> > to calculate enumeration base for NAry fields of HID sensor hub.
>> > 
>> > Signed-off-by: Srinivas Pandruvada
><srinivas.pandruvada@linux.intel.com>
>> Hi,  I would have preffred this being done in two patches, the first
>> refactoring the function call and the second doing the min and max.
>> 
>> Anyhow, I'll take it anyway.
>> 
>> Applied to the togreg branch of iio.git.
>
>Where is that? I don't seem to see it in 
>
>	https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/log/?h=togreg

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* Re: [PATCH 2/2] iio: hid-sensors: Fix power and report state
From: Jonathan Cameron @ 2013-12-02 17:11 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: jkosina-AlSwsSmVLrQ, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Greg KH
In-Reply-To: <529CB329.2020101-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>



Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>On 11/30/2013 03:28 AM, Jonathan Cameron wrote:
>> On 11/27/13 22:19, Srinivas Pandruvada wrote:
>>> In the original HID sensor hub firmwares all Named array enums were
>>> to 0-based. But the most recent hub implemented as 1-based,
>>> because of the implementation by one of the major OS vendor.
>>> Using logical minimum for the field as the base of enum. So we add
>>> logical minimum to the selector values before setting those fields.
>>> Some sensor hub FWs already changed logical minimum from 0 to 1
>>> to reflect this and hope every other vendor will follow.
>>> There is no easy way to add a common HID quirk for NAry elements,
>>> even if the standard specifies these field as NAry, the collection
>>> used to describe selectors is still just "logical".
>>>
>>> Signed-off-by: Srinivas Pandruvada
><srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> Looks like a good solution to me.
>>
>> This one is a little interesting.  Technically I 'believe' we don't
>have
>> a bug as it is possible to make these devices work via the kconfig
>option
>> and it definitely isn't a regression.  As such I have applied this to
>the
>> branch intended for the next kernel cycled (togreg) rather than to
>the
>> fixes branch.
>The intention of the patch (originally submitted with a quirk and
>module 
>param, which was commented on by you and Jiri),
>is also to bring report state enumeration also under the quirk to have 
>"1" based same as power state.
>So there is a fix to include reporting state enum also. So if possible,
>
>I would like to treat as bug fix.

Will do then.
>
>Thanks,
>Srinivas
>> If people have a very strong feeling about this then shout reasonably
>quickly - I'll
>> probably hold off sending that branch to Greg for a few days anyway.
>>
>> I've cc'd GregKH to see if he has any input on the path this should
>take.
>>
>> Jonathan
>>
>>> ---
>>>   drivers/iio/common/hid-sensors/Kconfig              |  9 ---------
>>>   drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 20
>+++++++++++++++-----
>>>   include/linux/hid-sensor-ids.h                      | 12
>++++++++++++
>>>   3 files changed, 27 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/iio/common/hid-sensors/Kconfig
>b/drivers/iio/common/hid-sensors/Kconfig
>>> index 1178121..39188b7 100644
>>> --- a/drivers/iio/common/hid-sensors/Kconfig
>>> +++ b/drivers/iio/common/hid-sensors/Kconfig
>>> @@ -25,13 +25,4 @@ config HID_SENSOR_IIO_TRIGGER
>>>   	  If this driver is compiled as a module, it will be named
>>>   	  hid-sensor-trigger.
>>>
>>> -config HID_SENSOR_ENUM_BASE_QUIRKS
>>> -	bool "ENUM base quirks for HID Sensor IIO drivers"
>>> -	depends on HID_SENSOR_IIO_COMMON
>>> -	help
>>> -	  Say yes here to build support for sensor hub FW using
>>> -	  enumeration, which is using 1 as base instead of 0.
>>> -	  Since logical minimum is still set 0 instead of 1,
>>> -	  there is no easy way to differentiate.
>>> -
>>>   endmenu
>>> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>>> index b6e77e0..781d3fa 100644
>>> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>>> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>>> @@ -33,24 +33,34 @@ static int
>hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
>>>   {
>>>   	struct hid_sensor_common *st = iio_trigger_get_drvdata(trig);
>>>   	int state_val;
>>> +	int report_val;
>>>
>>>   	if (state) {
>>>   		if (sensor_hub_device_open(st->hsdev))
>>>   			return -EIO;
>>> -	} else
>>> +		state_val =
>>> +		HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM;
>>> +		report_val =
>>> +		HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM;
>>> +
>>> +	} else {
>>>   		sensor_hub_device_close(st->hsdev);
>>> +		state_val =
>>> +		HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM;
>>> +		report_val =
>>> +		HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM;
>>> +	}
>>>
>>> -	state_val = state ? 1 : 0;
>>> -	if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS))
>>> -		++state_val;
>>>   	st->data_ready = state;
>>> +	state_val += st->power_state.logical_minimum;
>>> +	report_val += st->report_state.logical_minimum;
>>>   	sensor_hub_set_feature(st->hsdev, st->power_state.report_id,
>>>   					st->power_state.index,
>>>   					(s32)state_val);
>>>
>>>   	sensor_hub_set_feature(st->hsdev, st->report_state.report_id,
>>>   					st->report_state.index,
>>> -					(s32)state_val);
>>> +					(s32)report_val);
>>>
>>>   	return 0;
>>>   }
>>> diff --git a/include/linux/hid-sensor-ids.h
>b/include/linux/hid-sensor-ids.h
>>> index 4f945d3..8323775 100644
>>> --- a/include/linux/hid-sensor-ids.h
>>> +++ b/include/linux/hid-sensor-ids.h
>>> @@ -117,4 +117,16 @@
>>>   #define HID_USAGE_SENSOR_PROP_REPORT_STATE			0x200316
>>>   #define HID_USAGE_SENSOR_PROY_POWER_STATE			0x200319
>>>
>>> +/* Power state enumerations */
>>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_UNDEFINED_ENUM		0x00
>>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM		0x01
>>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D1_LOW_POWER_ENUM		0x02
>>> +#define
>HID_USAGE_SENSOR_PROP_POWER_STATE_D2_STANDBY_WITH_WAKE_ENUM	0x03
>>> +#define
>HID_USAGE_SENSOR_PROP_POWER_STATE_D3_SLEEP_WITH_WAKE_ENUM	0x04
>>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM		0x05
>>> +
>>> +/* Report State enumerations */
>>> +#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM		0x00
>>> +#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM		0x01
>>> +
>>>   #endif
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>linux-input" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Dmitry Torokhov @ 2013-12-02 16:45 UTC (permalink / raw)
  To: Manuel Krause
  Cc: Peter Hurley, linux-kernel, Greg KH, linux-input, linux-serial,
	Shuah Khan, Rafael J. Wysocki
In-Reply-To: <19253701.sgP3OieFUq@dtor-d630.eng.vmware.com>

On Mon, Dec 02, 2013 at 08:38:16AM -0800, Dmitry Torokhov wrote:
> On Monday, December 02, 2013 05:08:28 PM Manuel Krause wrote:
> > On 2013-12-01 16:43, Peter Hurley wrote:
> > > [ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
> > > linux-serial ]
> > > 
> > > On 11/26/2013 05:19 PM, Manuel Krause wrote:
> > >> Since kernel 3.12.0 I have a problem with hibernate+resume
> > >> not reactivating my serial mouse (trackball) with my HP notebook.
> > >> Kernels 3.11.0 til 9 don't show this behaviour.
> > >> 
> > >> Machine:           HP Notebook with Core2Duo CPU (Penryn)
> > >> Distro:            openSUSE 12.3, 64bit, continuously updated
> > >> Desktop:           KDE 4.11.3
> > >> MESA & drm & Xorg: most recent ones from:
> > >> 
> > >> http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_
> > >> 12.3/x86_64/
> > >> 
> > >> Current kernel:    3.12.1 vanilla from openSUSE repos, with
> > >> 
> > >>                     -ck1 and BFQ patches
> > >> 
> > >> The Logitech Trackman Marble FX is a PS/2 device and connected
> > >> via an original Logitech
> > >> PS/2-COM-port adapter and manually configured via my xorg.conf.
> > >> 
> > >> At first, I blamed the -ck1 patches from Con Kolivas for this
> > >> behaviour that I use in
> > >> addition  to the BFQ patches, what has showed up as not right:
> > >> This happens with the
> > >> normal vanilla kernel
> > >> schedulers for CPU and disk I/O, too.
> > >> 
> > >> By coincidence I found a weird(!) way to reactivate the serial
> > >> mouse:
> > >> (1) call Hibernate (suspend-to-disk) from KDE desktop as normal
> > >> (2) resume --> the PS/2 touchpad is working, the serial
> > >> trackball NOT
> > >> (3) call suspend-to-RAM (Sleep) from KDE, serial trackball
> > >> still dead
> > >> (4) execute `setserial -a /dev/ttyS0` in a konsole window or a
> > >> tty* console
> > >> (5) ==> serial trackball is back with all configuration from
> > >> xorg.conf
> > >> 
> > >> It's fully reproducible over multiple hibernations. This also
> > >> happens when calling
> > >> `pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
> > >> setserial from a root shell
> > >> in KDE or any tty*.
> > >> 
> > >> Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
> > > 
> > > Manuel,
> > > 
> > > Please attach complete dmesgs (zipped, if necessary) of a
> > > suspend/resume cycle
> > > on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
> > > (where resume succeeds).
> > > 
> > > For the test configurations, please do not apply patches.
> > > 
> > > Regards,
> > > Peter Hurley
> > 
> > Thank you very much for your reply!
> > Attached you'll find a zip file with the two edited dmesg logs of
> > plain vanilla kernel runs.
> > 
> > I have to add, that the resumes _do_ succeed in both cases, only
> > the serial mouse doesn't get activated after hibernate in 3.12.x
> > automatically. Just scan for and compare the lines indicating
> > "serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x
> > the activation doesn't happen after hibernate, but after
> > suspend-to-ram (sleep). That only after STR and not before a
> > setserial gets my mouse back... a miracle. ;-)
> 
> I do not see
> 
> > [  206.577370] serial 00:08: activated
> 
> in the restore from hibernation log of 3.12 which shoudl come from PNP
> layer.
> 
> [dtor@dtor-d630 work]$ git log --oneline v3.11..v3.12 -- drivers/pnp/ 
> 729377d pnp: change pnp bus pm_ops to invoke pnp driver dev_pm_ops if specified
> ce63e18 Merge branch 'pnp'
> 8ad928d ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere
> eaf140b PNP: convert PNP driver bus legacy pm_ops to dev_pm_ops
> 
> I'd start looking into these commits.
> 

Does the following patch fixes the issue?

-- 
Dmitry

PNP: fix restoring devices after hibernation

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

On returning from hibernation 'restore; callback is called, not 'resume'.
This fixes breakage introduced by commit
eaf140b60ec961252083ab8adaf67aef29a362dd

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/pnp/driver.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index a39ee38..185a24a 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -235,8 +235,9 @@ static int pnp_bus_resume(struct device *dev)
 
 static const struct dev_pm_ops pnp_bus_dev_pm_ops = {
 	.suspend = pnp_bus_suspend,
-	.freeze = pnp_bus_freeze,
 	.resume = pnp_bus_resume,
+	.freeze = pnp_bus_freeze,
+	.restore = pnp_bus_resume,
 };
 
 struct bus_type pnp_bus_type = {

^ permalink raw reply related

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Dmitry Torokhov @ 2013-12-02 16:38 UTC (permalink / raw)
  To: Manuel Krause
  Cc: Peter Hurley, linux-kernel, Greg KH, linux-input, linux-serial
In-Reply-To: <529CB07C.2040609@netscape.net>

On Monday, December 02, 2013 05:08:28 PM Manuel Krause wrote:
> On 2013-12-01 16:43, Peter Hurley wrote:
> > [ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
> > linux-serial ]
> > 
> > On 11/26/2013 05:19 PM, Manuel Krause wrote:
> >> Since kernel 3.12.0 I have a problem with hibernate+resume
> >> not reactivating my serial mouse (trackball) with my HP notebook.
> >> Kernels 3.11.0 til 9 don't show this behaviour.
> >> 
> >> Machine:           HP Notebook with Core2Duo CPU (Penryn)
> >> Distro:            openSUSE 12.3, 64bit, continuously updated
> >> Desktop:           KDE 4.11.3
> >> MESA & drm & Xorg: most recent ones from:
> >> 
> >> http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_
> >> 12.3/x86_64/
> >> 
> >> Current kernel:    3.12.1 vanilla from openSUSE repos, with
> >> 
> >>                     -ck1 and BFQ patches
> >> 
> >> The Logitech Trackman Marble FX is a PS/2 device and connected
> >> via an original Logitech
> >> PS/2-COM-port adapter and manually configured via my xorg.conf.
> >> 
> >> At first, I blamed the -ck1 patches from Con Kolivas for this
> >> behaviour that I use in
> >> addition  to the BFQ patches, what has showed up as not right:
> >> This happens with the
> >> normal vanilla kernel
> >> schedulers for CPU and disk I/O, too.
> >> 
> >> By coincidence I found a weird(!) way to reactivate the serial
> >> mouse:
> >> (1) call Hibernate (suspend-to-disk) from KDE desktop as normal
> >> (2) resume --> the PS/2 touchpad is working, the serial
> >> trackball NOT
> >> (3) call suspend-to-RAM (Sleep) from KDE, serial trackball
> >> still dead
> >> (4) execute `setserial -a /dev/ttyS0` in a konsole window or a
> >> tty* console
> >> (5) ==> serial trackball is back with all configuration from
> >> xorg.conf
> >> 
> >> It's fully reproducible over multiple hibernations. This also
> >> happens when calling
> >> `pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
> >> setserial from a root shell
> >> in KDE or any tty*.
> >> 
> >> Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
> > 
> > Manuel,
> > 
> > Please attach complete dmesgs (zipped, if necessary) of a
> > suspend/resume cycle
> > on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
> > (where resume succeeds).
> > 
> > For the test configurations, please do not apply patches.
> > 
> > Regards,
> > Peter Hurley
> 
> Thank you very much for your reply!
> Attached you'll find a zip file with the two edited dmesg logs of
> plain vanilla kernel runs.
> 
> I have to add, that the resumes _do_ succeed in both cases, only
> the serial mouse doesn't get activated after hibernate in 3.12.x
> automatically. Just scan for and compare the lines indicating
> "serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x
> the activation doesn't happen after hibernate, but after
> suspend-to-ram (sleep). That only after STR and not before a
> setserial gets my mouse back... a miracle. ;-)

I do not see

> [  206.577370] serial 00:08: activated

in the restore from hibernation log of 3.12 which shoudl come from PNP
layer.

[dtor@dtor-d630 work]$ git log --oneline v3.11..v3.12 -- drivers/pnp/ 
729377d pnp: change pnp bus pm_ops to invoke pnp driver dev_pm_ops if specified
ce63e18 Merge branch 'pnp'
8ad928d ACPI / PM: Use ACPI_STATE_D3_COLD instead of ACPI_STATE_D3 everywhere
eaf140b PNP: convert PNP driver bus legacy pm_ops to dev_pm_ops

I'd start looking into these commits.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/2] iio: hid-sensors: Fix power and report state
From: Srinivas Pandruvada @ 2013-12-02 16:19 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: jkosina-AlSwsSmVLrQ, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Greg KH
In-Reply-To: <5299CBD0.4030403-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On 11/30/2013 03:28 AM, Jonathan Cameron wrote:
> On 11/27/13 22:19, Srinivas Pandruvada wrote:
>> In the original HID sensor hub firmwares all Named array enums were
>> to 0-based. But the most recent hub implemented as 1-based,
>> because of the implementation by one of the major OS vendor.
>> Using logical minimum for the field as the base of enum. So we add
>> logical minimum to the selector values before setting those fields.
>> Some sensor hub FWs already changed logical minimum from 0 to 1
>> to reflect this and hope every other vendor will follow.
>> There is no easy way to add a common HID quirk for NAry elements,
>> even if the standard specifies these field as NAry, the collection
>> used to describe selectors is still just "logical".
>>
>> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Looks like a good solution to me.
>
> This one is a little interesting.  Technically I 'believe' we don't have
> a bug as it is possible to make these devices work via the kconfig option
> and it definitely isn't a regression.  As such I have applied this to the
> branch intended for the next kernel cycled (togreg) rather than to the
> fixes branch.
The intention of the patch (originally submitted with a quirk and module 
param, which was commented on by you and Jiri),
is also to bring report state enumeration also under the quirk to have 
"1" based same as power state.
So there is a fix to include reporting state enum also. So if possible, 
I would like to treat as bug fix.

Thanks,
Srinivas
> If people have a very strong feeling about this then shout reasonably quickly - I'll
> probably hold off sending that branch to Greg for a few days anyway.
>
> I've cc'd GregKH to see if he has any input on the path this should take.
>
> Jonathan
>
>> ---
>>   drivers/iio/common/hid-sensors/Kconfig              |  9 ---------
>>   drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 20 +++++++++++++++-----
>>   include/linux/hid-sensor-ids.h                      | 12 ++++++++++++
>>   3 files changed, 27 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/iio/common/hid-sensors/Kconfig b/drivers/iio/common/hid-sensors/Kconfig
>> index 1178121..39188b7 100644
>> --- a/drivers/iio/common/hid-sensors/Kconfig
>> +++ b/drivers/iio/common/hid-sensors/Kconfig
>> @@ -25,13 +25,4 @@ config HID_SENSOR_IIO_TRIGGER
>>   	  If this driver is compiled as a module, it will be named
>>   	  hid-sensor-trigger.
>>
>> -config HID_SENSOR_ENUM_BASE_QUIRKS
>> -	bool "ENUM base quirks for HID Sensor IIO drivers"
>> -	depends on HID_SENSOR_IIO_COMMON
>> -	help
>> -	  Say yes here to build support for sensor hub FW using
>> -	  enumeration, which is using 1 as base instead of 0.
>> -	  Since logical minimum is still set 0 instead of 1,
>> -	  there is no easy way to differentiate.
>> -
>>   endmenu
>> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> index b6e77e0..781d3fa 100644
>> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
>> @@ -33,24 +33,34 @@ static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig,
>>   {
>>   	struct hid_sensor_common *st = iio_trigger_get_drvdata(trig);
>>   	int state_val;
>> +	int report_val;
>>
>>   	if (state) {
>>   		if (sensor_hub_device_open(st->hsdev))
>>   			return -EIO;
>> -	} else
>> +		state_val =
>> +		HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM;
>> +		report_val =
>> +		HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM;
>> +
>> +	} else {
>>   		sensor_hub_device_close(st->hsdev);
>> +		state_val =
>> +		HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM;
>> +		report_val =
>> +		HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM;
>> +	}
>>
>> -	state_val = state ? 1 : 0;
>> -	if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS))
>> -		++state_val;
>>   	st->data_ready = state;
>> +	state_val += st->power_state.logical_minimum;
>> +	report_val += st->report_state.logical_minimum;
>>   	sensor_hub_set_feature(st->hsdev, st->power_state.report_id,
>>   					st->power_state.index,
>>   					(s32)state_val);
>>
>>   	sensor_hub_set_feature(st->hsdev, st->report_state.report_id,
>>   					st->report_state.index,
>> -					(s32)state_val);
>> +					(s32)report_val);
>>
>>   	return 0;
>>   }
>> diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
>> index 4f945d3..8323775 100644
>> --- a/include/linux/hid-sensor-ids.h
>> +++ b/include/linux/hid-sensor-ids.h
>> @@ -117,4 +117,16 @@
>>   #define HID_USAGE_SENSOR_PROP_REPORT_STATE			0x200316
>>   #define HID_USAGE_SENSOR_PROY_POWER_STATE			0x200319
>>
>> +/* Power state enumerations */
>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_UNDEFINED_ENUM		0x00
>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM		0x01
>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D1_LOW_POWER_ENUM		0x02
>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D2_STANDBY_WITH_WAKE_ENUM	0x03
>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D3_SLEEP_WITH_WAKE_ENUM	0x04
>> +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM		0x05
>> +
>> +/* Report State enumerations */
>> +#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM		0x00
>> +#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM		0x01
>> +
>>   #endif
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: 3.12.x looses serial mouse over hibernate + resume
From: Manuel Krause @ 2013-12-02 16:08 UTC (permalink / raw)
  To: Peter Hurley
  Cc: linux-kernel, Greg KH, Dmitry Torokhov, linux-input, linux-serial
In-Reply-To: <529B591F.1020909@hurleysoftware.com>

[-- Attachment #1: Type: text/plain, Size: 2832 bytes --]

On 2013-12-01 16:43, Peter Hurley wrote:
> [ +cc Dmitry Torokhov, Greg Kroah-Hartman, linux-input,
> linux-serial ]
>
> On 11/26/2013 05:19 PM, Manuel Krause wrote:
>> Since kernel 3.12.0 I have a problem with hibernate+resume
>> not reactivating my serial mouse (trackball) with my HP notebook.
>> Kernels 3.11.0 til 9 don't show this behaviour.
>>
>> Machine:           HP Notebook with Core2Duo CPU (Penryn)
>> Distro:            openSUSE 12.3, 64bit, continuously updated
>> Desktop:           KDE 4.11.3
>> MESA & drm & Xorg: most recent ones from:
>>
>> http://download.opensuse.org/repositories/home:/pontostroy:/X11/openSUSE_12.3/x86_64/
>>
>> Current kernel:    3.12.1 vanilla from openSUSE repos, with
>>                     -ck1 and BFQ patches
>>
>> The Logitech Trackman Marble FX is a PS/2 device and connected
>> via an original Logitech
>> PS/2-COM-port adapter and manually configured via my xorg.conf.
>>
>> At first, I blamed the -ck1 patches from Con Kolivas for this
>> behaviour that I use in
>> addition  to the BFQ patches, what has showed up as not right:
>> This happens with the
>> normal vanilla kernel
>> schedulers for CPU and disk I/O, too.
>>
>> By coincidence I found a weird(!) way to reactivate the serial
>> mouse:
>> (1) call Hibernate (suspend-to-disk) from KDE desktop as normal
>> (2) resume --> the PS/2 touchpad is working, the serial
>> trackball NOT
>> (3) call suspend-to-RAM (Sleep) from KDE, serial trackball
>> still dead
>> (4) execute `setserial -a /dev/ttyS0` in a konsole window or a
>> tty* console
>> (5) ==> serial trackball is back with all configuration from
>> xorg.conf
>>
>> It's fully reproducible over multiple hibernations. This also
>> happens when calling
>> `pm-hibernate` (to-disk) and `pm-suspend` (to-RAM) and the
>> setserial from a root shell
>> in KDE or any tty*.
>>
>> Please, _always_CC_me_ -- as I'm not on the kernel mailing list.
>
> Manuel,
>
> Please attach complete dmesgs (zipped, if necessary) of a
> suspend/resume cycle
> on a vanilla 3.12.x (where resume fails) _and_ a vanilla 3.11.x
> (where resume succeeds).
>
> For the test configurations, please do not apply patches.
>
> Regards,
> Peter Hurley
>

Thank you very much for your reply!
Attached you'll find a zip file with the two edited dmesg logs of 
plain vanilla kernel runs.

I have to add, that the resumes _do_ succeed in both cases, only 
the serial mouse doesn't get activated after hibernate in 3.12.x 
automatically. Just scan for and compare the lines indicating 
"serial 00:08: disabled" or "serial 00:08: activated". In 3.12.x 
the activation doesn't happen after hibernate, but after 
suspend-to-ram (sleep). That only after STR and not before a 
setserial gets my mouse back... a miracle. ;-)

Best regards, and, please, tell me if you need more information,

Manuel Krause


[-- Attachment #2: 3.12.x-looses-serial-mouse-over-hibernate.zip --]
[-- Type: application/zip, Size: 34983 bytes --]

^ permalink raw reply

* Re: hid-debug and CONFIG_DEBUG_FS
From: Jiri Kosina @ 2013-12-02 16:07 UTC (permalink / raw)
  To: Srinivas Pandruvada; +Cc: linux-input
In-Reply-To: <520BAD40.8000909@linux.intel.com>

On Wed, 14 Aug 2013, Srinivas Pandruvada wrote:

> I am implementing some hid driver which require relatively higher throughput.
> HID debug relies on CONFIG_DEBUG_FS, which is by default turned on by many
> distros. These debugs causes issues with the throughput, because each report
> and event is logged.
> 
> What do you think about using something other than CONFIG_DEBUG_FS for
> enabling HID debug (something like CONFIG_HID_DEBUG_FS..)?

Hi Srinivas,

do you encounter performance penalty even if there are no debugfs readers?

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: kye: Fix missing break in kye_report_fixup()
From: Benjamin Tissoires @ 2013-12-02 14:34 UTC (permalink / raw)
  To: Ben Hutchings, Jiri Kosina; +Cc: Adam Kulagowski, linux-input
In-Reply-To: <1385838747.2760.4.camel@deadeye.wl.decadent.org.uk>

On 30/11/13 14:12, Ben Hutchings wrote:
> The change to support Genius Manticore Keyboard also changed behaviour
> for Genius Gx Imperator Keyboard, as there is no break between the
> cases.  This is presumably a mistake.
> 
> Reported by Coverity as CID 1134029.
> 
> Fixes: 4a2c94c9b6c0 ('HID: kye: Add report fixup for Genius Manticore Keyboard')
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  drivers/hid/hid-kye.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c
> index ecb5ca6..e776963 100644
> --- a/drivers/hid/hid-kye.c
> +++ b/drivers/hid/hid-kye.c
> @@ -341,6 +341,7 @@ static __u8 *kye_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>  	case USB_DEVICE_ID_GENIUS_GX_IMPERATOR:
>  		rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 83,
>  					"Genius Gx Imperator Keyboard");
> +		break;
>  	case USB_DEVICE_ID_GENIUS_MANTICORE:
>  		rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 104,
>  					"Genius Manticore Keyboard");
> 

Jiri already applied this one, but FWIW:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks for spotting this,
Benjamin

^ permalink raw reply

* Re: [PATCH v4 1/1] Input: Improve the performance of alps v5-protocol's touchpad
From: Tommy Will @ 2013-12-02 14:16 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Kevin Cernekee, david turvene, linux-input, Niels de Vos,
	Yunkang Tang
In-Reply-To: <CA+F6ckMRwq_1TbBvzQjqGFRBTdJxH74rUTuDUMmdL8c8Nda_Rw@mail.gmail.com>

Hi Dmitry,

I'm suddenly aware that change like below would be better.
Do you think so?

{ { 0x73, 0x03, 0x50 }, 0x0d, ALPS_PROTO_V5, 0xc8, 0xc8, 0 }
 => { { 0x73, 0x03, 0x50 }, 0x0d, ALPS_PROTO_V5, 0xc8, 0xd8, 0 }

Thanks
-- 
Best Regards,
Tommy


2013/12/2 Tommy Will <tommywill2011@gmail.com>:
> Hi Dmitry,
>
> Thanks for your review.
>
> 2013/12/2 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
>
>>> +     /* Ignore stick point data */
>>> +     if (packet[0] == 0xD8)
>>> +             return;
>
>> Why are we ignoring trackstick data? If there Dolphin models with
>> tracksticks that would basically "break" the device for users compared
>> withe the emulated Explorer PS/2 mode.
>
> I'm sorry, I think I used the wrong comment here...
> Use comment /* Check if it's an valid touchpad packet */ would be better
>
> From the V5's spec, "0xC8" is the checkmask for 6byte Raw mode and
> 0x10 is the mask for trackpoint's / touchpad's packet.
> So, 0xC8 means current packet is from touchpad and 0xD8 for trackpoint.
> In current source code, what I wanted to do is just add a protection
> logic to double check if it's a valid touchpad packet.
>
> About your concern, I think there would be no problem because:
> 1) In initialization, we did not initialize trackpoint device as Raw
> mode and it should always report 3Byte-packet. And in the first of
>     alps_process_byte(), 3byte packet would be processed separately.
> 2) I checked with my Japanese colleague and in fact, there is no V5
> device with trackpoint + touchpad in the world, although V5's spec
>    designed the case of trackpoint's packet.
>
> Thanks
> --
> Best Regards,
> Tommy

^ permalink raw reply

* Re: [PATCH] hid: add NOGET quirk and device id for Logitech Dual Action gamepads support
From: Jiri Kosina @ 2013-12-02 14:13 UTC (permalink / raw)
  To: Zawullon; +Cc: linux-input, linux-kernel
In-Reply-To: <5299B520.9060200@gmail.com>

On Sat, 30 Nov 2013, Zawullon wrote:

> Issue description: I have two Logitech Dual Action gamepads, both have
> same Vendor/Device id pair. Newest gamepad (A) can switch between old mode (HID)
> and XBox gamepad emulation mode. Old gamepad (B) can only work in HID mode.
> In HID mode gamepad A sends many EPIPE errors during initialization and was
> disconnected immediately after connect  to usb port. It works fine in Win and
> Mac. After adding NOGET quirk in driver, it was working properly.
> Gamepad B works fine before and after changes. I tested both gamepads
> with 3.8.0 and 3.11.6 kernels  with modified driver. Follow patch can apply
> for current git kernel version. I can send pcap log from usb bus with both
> gamepads or any other additional information if it is needed
> 

Please provide your Signed-off-by: line so that I could apply the patch. 
Thanks.

> diff -uprN -X linux-git/Documentation/dontdiff linux-git/drivers/hid/hid-ids.h linux-my/drivers/hid/hid-ids.h
> --- linux-git/drivers/hid/hid-ids.h	2013-11-30 13:29:27.937351968 +0400
> +++ linux-my/drivers/hid/hid-ids.h	2013-11-30 13:46:05.201378674 +0400
> @@ -552,6 +552,7 @@
>  #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD_CORD	0xc20a
>  #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD	0xc211
>  #define USB_DEVICE_ID_LOGITECH_EXTREME_3D	0xc215
> +#define USB_DEVICE_ID_LOGITECH_DUAL_ACTION	0xc216
>  #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2	0xc218
>  #define USB_DEVICE_ID_LOGITECH_RUMBLEPAD2_2	0xc219
>  #define USB_DEVICE_ID_LOGITECH_WINGMAN_F3D	0xc283
> diff -uprN -X linux-git/Documentation/dontdiff linux-git/drivers/hid/hid-lg.c linux-my/drivers/hid/hid-lg.c
> --- linux-git/drivers/hid/hid-lg.c	2013-11-30 13:29:27.937351968 +0400
> +++ linux-my/drivers/hid/hid-lg.c	2013-11-30 13:46:05.201378674 +0400
> @@ -758,6 +758,8 @@ static const struct hid_device_id lg_dev
>  
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_EXTREME_3D),
>  		.driver_data = LG_NOGET },
> +	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_DUAL_ACTION),
> +		.driver_data = LG_NOGET },
>  	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WHEEL),
>  		.driver_data = LG_NOGET | LG_FF4 },
>  
> 

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 1/2] HID: hid-sensor-hub: Add logical min and max
From: Jiri Kosina @ 2013-12-02 13:08 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Srinivas Pandruvada, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5299CCAA.6090100-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Sat, 30 Nov 2013, Jonathan Cameron wrote:

> On 11/27/13 22:19, Srinivas Pandruvada wrote:
> > Exporting logical minimum and maximum of HID fields as part of the
> > hid sensor attribute info. This can be used for range checking and
> > to calculate enumeration base for NAry fields of HID sensor hub.
> > 
> > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Hi,  I would have preffred this being done in two patches, the first
> refactoring the function call and the second doing the min and max.
> 
> Anyhow, I'll take it anyway.
> 
> Applied to the togreg branch of iio.git.

Where is that? I don't seem to see it in 

	https://git.kernel.org/cgit/linux/kernel/git/jic23/iio.git/log/?h=togreg

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH v2] Input:Add support for DualPoint device on Dell XT2 model
From: Tommy Will @ 2013-12-02  7:50 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Kevin Cernekee, david turvene, linux-input, Niels de Vos,
	Justin Clift, Yunkang Tang
In-Reply-To: <20131202063907.GC21404@core.coreip.homeip.net>

Hi Dmitry

2013/12/2 Dmitry Torokhov <dmitry.torokhov@gmail.com>:
> On Wed, Nov 27, 2013 at 04:19:05PM +0800, Tommy Will wrote:

>
> OK, fair enough. I will apply the patch, but I think the ALPS driver is
> becoming a bit too unwieldy. It looks like we should split it into
> alps-st (that woudl handle older single-touch models) and alps-mt (for
> the newer multi-touch ones) and maybe a support library for the common
> code.

Thanks for your suggestion. I would consider your idea and try to make
the patch later.

To be honest, not only splitting the file into alps-st and alps-mt, I
also want to rewrite some of ALPS driver's logic.
Some source code seems not exactly and some are hard to be reused later.
However it may cause huge changes and I'm not sure if such
modification would have side-effect or pass the
review and be applied.

# To be honest, I have no confidence to rewrite the logic without
causing any side-effect. Too many models exists
and I could not have chance to test all of them.

-- 
Best Regards,
Tommy

^ permalink raw reply

* Re: [PATCH v4 1/1] Input: Improve the performance of alps v5-protocol's touchpad
From: Tommy Will @ 2013-12-02  7:37 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Kevin Cernekee, david turvene, linux-input, Niels de Vos,
	Yunkang Tang
In-Reply-To: <20131202064121.GD21404@core.coreip.homeip.net>

Hi Dmitry,

Thanks for your review.

2013/12/2 Dmitry Torokhov <dmitry.torokhov@gmail.com>:

>> +     /* Ignore stick point data */
>> +     if (packet[0] == 0xD8)
>> +             return;

> Why are we ignoring trackstick data? If there Dolphin models with
> tracksticks that would basically "break" the device for users compared
> withe the emulated Explorer PS/2 mode.

I'm sorry, I think I used the wrong comment here...
Use comment /* Check if it's an valid touchpad packet */ would be better

>From the V5's spec, "0xC8" is the checkmask for 6byte Raw mode and
0x10 is the mask for trackpoint's / touchpad's packet.
So, 0xC8 means current packet is from touchpad and 0xD8 for trackpoint.
In current source code, what I wanted to do is just add a protection
logic to double check if it's a valid touchpad packet.

About your concern, I think there would be no problem because:
1) In initialization, we did not initialize trackpoint device as Raw
mode and it should always report 3Byte-packet. And in the first of
    alps_process_byte(), 3byte packet would be processed separately.
2) I checked with my Japanese colleague and in fact, there is no V5
device with trackpoint + touchpad in the world, although V5's spec
   designed the case of trackpoint's packet.

Thanks
-- 
Best Regards,
Tommy

^ permalink raw reply

* Re: [PATCH v4 1/1] Input: Improve the performance of alps v5-protocol's touchpad
From: Dmitry Torokhov @ 2013-12-02  6:41 UTC (permalink / raw)
  To: Yunkang Tang; +Cc: cernekee, dturvene, linux-input, ndevos, yunkang.tang
In-Reply-To: <1382355711-3211-1-git-send-email-yunkang.tang@cn.alps.com>

Hi Yunkang,

On Mon, Oct 21, 2013 at 07:41:50PM +0800, Yunkang Tang wrote:
> +static void alps_process_packet_v5(struct psmouse *psmouse)
> +{
> +	unsigned char *packet = psmouse->packet;
> +
> +	/* Ignore stick point data */
> +	if (packet[0] == 0xD8)
> +		return;

Why are we ignoring trackstick data? If there Dolphin models with
tracksticks that would basically "break" the device for users compared
withe the emulated Explorer PS/2 mode.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2] Input:Add support for DualPoint device on Dell XT2 model
From: Dmitry Torokhov @ 2013-12-02  6:39 UTC (permalink / raw)
  To: Tommy Will
  Cc: Kevin Cernekee, david turvene, linux-input, Niels de Vos,
	Justin Clift, Yunkang Tang
In-Reply-To: <CA+F6ckOPi2tmQxLtsWkcyvEb3ZnP2BK50Jj-9iZrXpBm11M56A@mail.gmail.com>

On Wed, Nov 27, 2013 at 04:19:05PM +0800, Tommy Will wrote:
> Hi Dmitry,
> 
> Thanks for your reply.
> 
> >
> > It would help greatly if you could document what parameters the new
> > initialization routine sets.
> >
> Ok, please let me explain what I did in initialization.
> 
> >+static int alps_hw_init_v6(struct psmouse *psmouse)
> >+{
> >+     unsigned char param[2] = {0xC8, 0x14};
> >+
> >+     /* Enter passthrough mode to let trackpoint enter 6byte raw mode */
> >+     if (alps_passthrough_mode_v2(psmouse, true))
> >+             return -1;
> 
> In order to initialize the trackpoint, we must first send command to
> enter passthrough mode so that the following
> command can directly be sent to trackpoint device.
> 
> +
> >+     if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> >+         ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> >+         ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11) ||
> >+         ps2_command(&psmouse->ps2dev, &param[0], PSMOUSE_CMD_SETRATE) ||
> >+         ps2_command(&psmouse->ps2dev, &param[1], PSMOUSE_CMD_SETRATE))
> >+             return -1;
> >+
> 
> Here are the commands to let trackpoint device enter 6byte raw mode.
> Because of the special MPU being
> used in this device, trackpoint's packet would always be encapsulated
> to the same packet size as Touchpad's.
> So, when touchpad is in 6byte mode, we should also let trackpoint in
> 6byte mode or trackpoint's output data would be messed.
> That's what spec requested.
> 
> +     if (alps_passthrough_mode_v2(psmouse, false))
> +             return -1;
> +
> 
> Stop talking with trackpoint.
> 
> +     if (alps_absolute_mode_v6(psmouse)) {
> +             psmouse_err(psmouse, "Failed to enable absolute mode\n");
> +             return -1;
> +     }
> +
> 
> Let touchpad enter 6byte raw mode.
> 
> +     return 0;
> +}
> 
> And in alps_absolute_mode_v6() function. Please notice that there are
> two kinds of
> 6byte mode for this device. Let's temporary define them as RawModeA & RawModeB.
> Method to enter RawModeA:   Send ALPS magic knock commad [F5 F5 F5
> F5]. (Same as protocol v2)
> Method to enter RawModeB:   Write 0x181 to 0x000 register. (New)
> 
> I checked spec and found RawModeB was suggested and now being used in
> Windows driver.
> Maybe the reason is it could provide a more accurate data output.
> In RawModeA, data output range is X:[0-1023], Y:[0-767], but
> in RawModeB, its range is X:[0-2047], Y:[0-1535].
> 
> #Probably have any other reason that I did not know, so, I chose a
> safety way that was suggested
> by the spec although it needs adding much more source code.
> 
> 
> > +static int alps_absolute_mode_v6(struct psmouse *psmouse)
> > +{
> > +     u16 reg_val = 0x181;
> > +     int ret = -1;
> > +
> 
> What we want to do is write 0x181 to 0x000 register. That would let
> device enter RawModeB.
> 
> > +     /* enter monitor mode, to write the register */
> > +     if (alps_monitor_mode(psmouse, true))
> > +             return -1;
> > +
> > +     ret = alps_monitor_mode_write_reg(psmouse, 0x000, reg_val);
> > +
> > +     if (alps_monitor_mode(psmouse, false))
> > +             ret = -1;
> > +
> > +     return ret;
> > +}
> 
> 
> >>    3. Add new packet process logic.
> >
> > I am curious why we need the new packet processing logic. Apparently the
> > touchpad can work in the original mode that we already know how to
> > parse; we just need to make sure the trackpoint is initialized properly.
> > Or yet another protocol flavor is a must?
> >
> Yes, you're right, as long as initialized trackpoint to 6byte raw mode
> and add decode logic into
> alps_process_packet_v1_v2(), both touchpad and trackpoint can work correctly.
> 
> However, as I wrote in previous, spec suggested me to use another
> protocol to co-work with
> trackpoint's 6byte mode. So, I just did follow the spec.
> 
> BTW, could you please let me know if you have any concern of adding a
> new protocol?
> In my opinion, assume that there would be no side-effect by using
> original touchpad
> protocol + new 6byte trackpoint protocol, although I could reuse the
> alps_process_packet_v1_v2(),
> I had to add a new flag to adjust the original logic to support
> trackpoint's 6byte decoding,
> which would also make source code harder to understand.

OK, fair enough. I will apply the patch, but I think the ALPS driver is
becoming a bit too unwieldy. It looks like we should split it into
alps-st (that woudl handle older single-touch models) and alps-mt (for
the newer multi-touch ones) and maybe a support library for the common
code.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: ads7846 - Use IS_ENABLED() macro
From: Dmitry Torokhov @ 2013-12-02  6:12 UTC (permalink / raw)
  To: Fabio Estevam; +Cc: linux-input, Fabio Estevam
In-Reply-To: <1385951651-19605-1-git-send-email-festevam@gmail.com>

On Mon, Dec 02, 2013 at 12:34:11AM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Using the IS_ENABLED() macro can make the code shorter and simpler
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Applied, thank you.

> ---
>  drivers/input/touchscreen/ads7846.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index ea19536..5695786 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -101,7 +101,7 @@ struct ads7846 {
>  	struct spi_device	*spi;
>  	struct regulator	*reg;
>  
> -#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
> +#if IS_ENABLED(CONFIG_HWMON)
>  	struct attribute_group	*attr_group;
>  	struct device		*hwmon;
>  #endif
> @@ -421,7 +421,7 @@ static int ads7845_read12_ser(struct device *dev, unsigned command)
>  	return status;
>  }
>  
> -#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
> +#if IS_ENABLED(CONFIG_HWMON)
>  
>  #define SHOW(name, var, adjust) static ssize_t \
>  name ## _show(struct device *dev, struct device_attribute *attr, char *buf) \
> -- 
> 1.8.1.2
> 

-- 
Dmitry

^ permalink raw reply

* [PATCH] Input: ads7846 - Use IS_ENABLED() macro
From: Fabio Estevam @ 2013-12-02  2:34 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, Fabio Estevam

From: Fabio Estevam <fabio.estevam@freescale.com>

Using the IS_ENABLED() macro can make the code shorter and simpler

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/input/touchscreen/ads7846.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index ea19536..5695786 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -101,7 +101,7 @@ struct ads7846 {
 	struct spi_device	*spi;
 	struct regulator	*reg;
 
-#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
+#if IS_ENABLED(CONFIG_HWMON)
 	struct attribute_group	*attr_group;
 	struct device		*hwmon;
 #endif
@@ -421,7 +421,7 @@ static int ads7845_read12_ser(struct device *dev, unsigned command)
 	return status;
 }
 
-#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
+#if IS_ENABLED(CONFIG_HWMON)
 
 #define SHOW(name, var, adjust) static ssize_t \
 name ## _show(struct device *dev, struct device_attribute *attr, char *buf) \
-- 
1.8.1.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox