Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [v2] input: drv260x: Add TI drv260x haptics driver
From: Murphy, Dan @ 2014-07-28 18:02 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org
In-Reply-To: <20140728174334.GD8982@core.coreip.homeip.net>

Dmitry

Thanks for the comments

On 07/28/2014 12:43 PM, Dmitry Torokhov wrote:
> Hi Dan,
> 
> On Mon, Jul 28, 2014 at 11:53:23AM -0500, Dan Murphy wrote:
>> Add the TI drv260x haptics/vibrator driver.
>> This device uses the input force feedback
>> to produce a wave form to driver an
>> ERM or LRA actuator device.
>>
>> The initial driver supports the devices
>> real time playback mode.  But the device
>> has additional wave patterns in ROM.
> 
> As it presented the device appears to be a memoryless device, however
> you present it to the rest of the system as if it can support playback
> of multiple effects simultaneously, which is incorrect.
> 
> My guess that you need to engage the memoryless input library to schedule
> handling multiple effects for your device, including ramping up, ramping
> down, stopping playback when effects runs out, etc.
> 
> Thanks.
>  
> 


I had written the driver originally to be a memless device.

But when calling the driver from user space I was not getting the duration,
strength of any information from the ff_device structure except the type.
The values in the structure were all being passed as 0's.

I will attempt it again as a memless device.

Dan

-- 
------------------
Dan Murphy

^ permalink raw reply

* Re: [v2] input: drv260x: Add TI drv260x haptics driver
From: simon @ 2014-07-28 17:59 UTC (permalink / raw)
  To: Dan Murphy
  Cc: linux-input, linux-kernel, devicetree, "Michal Malý",
	Elias Vanderstuyft
In-Reply-To: <20140728174334.GD8982@core.coreip.homeip.net>


>> The initial driver supports the devices
>> real time playback mode.  But the device
>> has additional wave patterns in ROM.
>
> As it presented the device appears to be a memoryless device, however
> you present it to the rest of the system as if it can support playback
> of multiple effects simultaneously, which is incorrect.
>
> My guess that you need to engage the memoryless input library to schedule
> handling multiple effects for your device, including ramping up, ramping
> down, stopping playback when effects runs out, etc.

Hi Dan,
Elias and Michal (cc'ed) are working on a kernel/userland library to
handle sending multiple force feedback signals to 'simple' devices,
perhaps you should engage with them.

Simon


^ permalink raw reply

* Re: Power-managing devices that are not of interest at some point in time
From: Patrik Fimml @ 2014-07-28 19:58 UTC (permalink / raw)
  To: hadess
  Cc: Dmitry Torokhov, Rafael J. Wysocki, Alan Stern, Patrik Fimml,
	linux-pm, Benson Leung, linux-input, linux-kernel
In-Reply-To: <00d187033b8082c2400a38175f7a4dd8@hadess.net>

(Note: Your email arrived here with lines wrapped in a hard-to-read way,
not sure where the problem lies.)

On Tue, Jul 22, 2014 at 01:23:14AM +0200, hadess@hadess.net wrote:
> On 2014-07-19 01:16, Dmitry Torokhov wrote:
> <snip>
> >I'd say no.
> >
> >Anyway, even though it is very tempting to declare inhibit a
> >"deeper" state of
> >runtime suspend maybe you are right and inhibit should really be
> >separate from
> >PM and drivers would have to sort out all the possible state
> >permutations.
> >
> >Considering input devices:
> >
> >input_open(): check if device is inhibited, if so do nothing.
> >Otherwise try
> >waking up itself and parent (via pm_runtime_get_sync() on itself),
> >this will
> >power up the device. Do additional configuration if needed.
> >
> >input_close(): check if device is inhibited, if not do
> >pm_runtime_put (_sync?
> >to make sure we power off properly and not leave device up and
> >running? or
> >should we power down manually not waiting for runtime PM)?
> >
> >inhibit(): check if device is opened, if opened do
> >pm_runtime_put_sync().
> >
> >uninhibit(): if device is opened do pm_runtime_get_sync(), let
> >runtime PM
> >bring up the device. Do additional config if needed -> very similar to
> >input_open(), different condition.
> >
> >runtime_suspend(): power down the device. If not inhibited enable
> >as wakeup
> >source.
> >
> >runtime_resume(): power up the device if device is opened and not
> >inhibited.
> >
> >system_suspend(): check if device is opened, not inhibited and not in
> >runtimesuspend  already; power down.
> >
> >system_resume(): power up the device if it is opened and not
> >inhibited. I
> >guess it's OK to wake up device that shoudl be runtime-PM-idle
> >since it will
> >go to back sleep shortly.
> >
> >Ugh.. This is complicated...
> 
> Seriously complicated. The compositor and/or X.org can handle most
> of that for
> input devices already. For the camera, you want the application to
> know that
> the device is present, but not usable, instead of making it vanish
> or rendering
> it unusable. And if you wanted to implement this in the kernel, even
> with the aid
> of a user-space daemon, you're still missing the most important
> part, the device
> tagging.

I agree that the behaviour for e.g. the camera is debatable. Our primary
concern is input devices for now, but we were trying to see whether such
a functionality might be desired for other device classes as well.

> Once you've tagged the devices which would need to be left alone
> when the lid is
> closed, or the laptop is in tablet mode, see if the problem can be
> solved within
> user-space alone. I'm fairly certain it wouldn't take too long to
> fix Xorg
> or a state-of-the-art compositor to behave properly.

This would not allow you to power down the touchscreen more than to a
"idle" state without further modifications to drivers.

We also want to make sure that the kernel ignores input from these
devices for S3 suspend, so that stray touch inputs do not abort a
suspend that is about to happen.

And also, I think Xorg is not necessarily the proper layer for this.
It's a bit theoretical, but: If there were other users of the input
device, they would also want to ignore the events.

> All I see right now is making it harder to write devices drivers
> with no real benefits.

I don't see this problem at all. Just like drivers don't have to
implement runtime PM, they wouldn't be required to implement inhibit()
and its counterpart. But if they do implement such functionality, we
give them a standard interface to expose it.

If we stick with the example of input devices, we can even have a simple
default implementation in the device class (skipping any generated
events) and bring the basic behaviour to unmodified drivers for free.

Even if you did not have the "inhibit" interface, if you wanted to power
down the touchscreen completely when it's not accessible, this would
require very similar driver modifications.

Regards,
Patrik

^ permalink raw reply

* Re: Power-managing devices that are not of interest at some point in time
From: Patrik Fimml @ 2014-07-28 20:01 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rafael J. Wysocki, Alan Stern, Patrik Fimml, Bastien Nocera,
	linux-pm, Benson Leung, linux-input, linux-kernel
In-Reply-To: <1451627.FURGiR949u@dtor-glaptop>

On Fri, Jul 18, 2014 at 04:16:50PM -0700, Dmitry Torokhov wrote:
> [...]
> Anyway, even though it is very tempting to declare inhibit a "deeper" state of 
> runtime suspend maybe you are right and inhibit should really be separate from 
> PM and drivers would have to sort out all the possible state permutations.
> 
> Considering input devices:
> 
> input_open(): check if device is inhibited, if so do nothing. Otherwise try 
> waking up itself and parent (via pm_runtime_get_sync() on itself), this will 
> power up the device. Do additional configuration if needed.
> 
> input_close(): check if device is inhibited, if not do pm_runtime_put (_sync? 
> to make sure we power off properly and not leave device up and running? or 
> should we power down manually not waiting for runtime PM)?
> 
> inhibit(): check if device is opened, if opened do pm_runtime_put_sync().
> 
> uninhibit(): if device is opened do pm_runtime_get_sync(), let runtime PM 
> bring up the device. Do additional config if needed -> very similar to 
> input_open(), different condition.
> 
> runtime_suspend(): power down the device. If not inhibited enable as wakeup 
> source.
> 
> runtime_resume(): power up the device if device is opened and not inhibited.
> 
> system_suspend(): check if device is opened, not inhibited and not in 
> runtimesuspend  already; power down.
> 
> system_resume(): power up the device if it is opened and not inhibited. I 
> guess it's OK to wake up device that shoudl be runtime-PM-idle since it will 
> go to back sleep shortly.
> 
> Ugh.. This is complicated...

There might be more elegant ways to implement this. It might make sense
to factor out power transitions. One could have a function that derives
the appropriate power state given the circumstances (i.e.
suspend/inhibit/runtime suspend) and then performs the needed
transition. This is something one would have to see when actually
implementing this, I might experiment with this approach a bit.

Regards,
Patrik

^ permalink raw reply

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Yufeng Shen @ 2014-07-28 20:20 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Nick Dyer, Dmitry Torokhov, benson Leung, Daniel Kurtz,
	Henrik Rydberg, Joonyoung Shim, Alan Bowens, linux-input,
	linux-kernel@vger.kernel.org, Peter Meerwald, Olof Johansson,
	Sekhar Nori
In-Reply-To: <53D2B8D0.5090000@wwwdotorg.org>

On Fri, Jul 25, 2014 at 4:06 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 07/25/2014 08:10 AM, Nick Dyer wrote:
>>
>> On 24/07/14 22:19, Stephen Warren wrote:
>
> ...
>
>>> I've uploaded 2 logs to:
>>>
>>> http://avon.wwwdotorg.org/downloads/mxt-logs/
>>> (note there's no directory indexing, so manually add the filenames below to
>>> the URL)
>>>
>>> mxt-save-no-movement.xml
>>>
>>> This is with the whole series applied. Neither mouse movement nor clicks
>>> works. I tried mxt-app --reset and it made no difference to the dump results.
>>>
>>> mxt-save-move-ok-no-clicking.xml
>>>
>>> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
>>> reverted; mouse movement works, but clicking doesn't.
>>
>>
>> Great, this has identified the issue with mouse movement (touch).
>>
>> The config programmed into the NVRAM on your touch controller has the first
>> byte of the T9 touchscreen object set to zero. This is the CTRL byte which
>> enables/disables the touch object and what it reports. It is relying on
>> this to enable the touchscreen on resume:
>>
>> https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
>>
>> My "use deep sleep mode when stopped" patch stops the driver writing to the
>> T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
>> used (ie zero, disabled). Going forward, deep sleep is more generic.
>> Indeed, newer chips do not have T9 at all, or they might be using other
>> touch objects. The deep sleep mode is a lower power state to be in, and is
>> what Atmel recommends.
>>
>> However, it does mean changing the maxtouch cfg - you can write the 0x83 to
>> the first byte of T9 and save it to NVRAM, by doing:
>>
>> mxt-app [device] -W -T9 83
>> mxt-app [device] --backup
>
>
> If I do that, then both mouse movement and "touch" clicks work:-)
>
> (Dmitry, I guess that means it's fine to go ahead and apply "Input: atmel_mxt_ts - use deep sleep mode when stopped".)
>
> I wonder why the configuration NVRAM in my device was incorrect though? I'm CCing a few ChromeOS people to try and find out any relevant history for the touchpad NVRAM settings on Venice2. Perhaps this is simply something that wasn't noticed because the driver used to initialize this configuration bit, so nobody realized the config in NVRAM was wrong.
>

Where did you get the configuration file ? It is possible that we rely
too much on mxt_start to turn on the T9.CTRL bit and have neglected
its setting in the config file.
If you can tell me where you get the config file I can do a check.


> ...
>
>> About the clicking - what does getevent -lp show? It should show the
>> BTN_LEFT key. If that is working correctly, then the driver isn't parsing
>> the messages correctly, it would be useful if you could add a
>> mxt_dump_message() call to mxt_input_button() and capture some dmesg output
>> of pressing the button.
>
>
> I'm not sure what getevent is, but I think I tracked down the issue anyway.
>
> With the NVRAM config fix you mentioned, "touch" clicks work OK. However, as such as I do a "physical" click (push the touchpad down), all kinds of click stop working. I think the answer lies in evtest logs:
>
> First "touch" click (with touch pressure/position events removed):
>
>> Event: time 1406318070.891773, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
>> Event: time 1406318070.891773, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
>
> ...
>>
>> Event: time 1406318070.941752, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
>> Event: time 1406318070.941752, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0
>
>
> The second "touch" click generates the same events. Note how all the events are correctly mirrored between down/up events.
>
> Now, the first "physical" click:
>
>> Event: time 1406318085.303424, type 1 (EV_KEY), code 272 (BTN_LEFT), value 1
>
> ...
>>
>> Event: time 1406318085.319818, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 1
>> Event: time 1406318085.319818, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 1
>
> ...
>>
>> Event: time 1406318085.515763, type 1 (EV_KEY), code 330 (BTN_TOUCH), value 0
>> Event: time 1406318085.515763, type 1 (EV_KEY), code 325 (BTN_TOOL_FINGER), value 0
>
>
> Note the extra BTN_LEFT down event, which has no corresponding "up" event. After this, subsequent "physical" clicks don't generate any more BTN_LEFT events (down or up) at all, and a USB mouse's BTN_LEFT events are ignored, I suppose since the system still thinks the touchpad's left button is being held down.
>
> Is this a driver bug (not generating the correct BTN_LEFT events), or some other touchpad HW/NVRAM configuration problem?

^ permalink raw reply

* Re: [v2] input: drv260x: Add TI drv260x haptics driver
From: Murphy, Dan @ 2014-07-28 21:21 UTC (permalink / raw)
  To: simon@mungewell.org
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, Michal Malý, Elias Vanderstuyft
In-Reply-To: <5adf0339146a7b655a77053195aa892c.squirrel@mungewell.org>

Dmitry

On 07/28/2014 12:59 PM, simon@mungewell.org wrote:
> 
>>> The initial driver supports the devices
>>> real time playback mode.  But the device
>>> has additional wave patterns in ROM.
>>
>> As it presented the device appears to be a memoryless device, however
>> you present it to the rest of the system as if it can support playback
>> of multiple effects simultaneously, which is incorrect.
>>
>> My guess that you need to engage the memoryless input library to schedule
>> handling multiple effects for your device, including ramping up, ramping
>> down, stopping playback when effects runs out, etc.
> 
> Hi Dan,
> Elias and Michal (cc'ed) are working on a kernel/userland library to
> handle sending multiple force feedback signals to 'simple' devices,
> perhaps you should engage with them.
> 
> Simon
> 
> 

OK I looked at the memless devices and got it to work.
Don't know why replay.length is always 0 but at least I got the
magnitude.  It seems that the input framework expects the user space
to manage the on/off of the vibrator in RUMBLE mode.  Unless
I missed something while going through the code.

This family of devices has the ability to do real time playback,
play an index in a pre-programmed library and have feedback waveforms programmed
into the device to be played.

The RTP mode is covered in this patch set.  But I am also interested in
setting the pre-programmed libraries as well as setting my own custom waveform.

I believe the custom is handled via the FF_PERIODIC with the waveform being FF_CUSTOM.
I am not really a big fan of mixing the FF_CUSTOM in the periodic.  This could be broken
out so that the driver can handle the custom waveforms.

But I am not sure with the existing framework we can set a library or index of the 
library. (Short of adding sysfs nodes which I don't want to do)

My thought would be to add FF_LIBRARY as a force feedback type and create a structure like

struct ff_library {
	__u16 library;
	__u16 index;
};

to be able to program the driver to play a certain library/index combo when play is called.
Of course the driver would have to set the max libraries and max indices within the library.

Let me know if this is something we can add and I can RFC a patch for it.

Dan

P.S.  Got the P.S. will use devm as suggested.

-- 
------------------
Dan Murphy

^ permalink raw reply

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Stephen Warren @ 2014-07-28 21:23 UTC (permalink / raw)
  To: Yufeng Shen
  Cc: Nick Dyer, Dmitry Torokhov, benson Leung, Daniel Kurtz,
	Henrik Rydberg, Joonyoung Shim, Alan Bowens, linux-input,
	linux-kernel@vger.kernel.org, Peter Meerwald, Olof Johansson,
	Sekhar Nori
In-Reply-To: <CAPDwgkNrBRJRR6WtoPSfs8Yq=ui-Ptjju4mJ=9h=95YVFuxw7g@mail.gmail.com>

On 07/28/2014 02:20 PM, Yufeng Shen wrote:
> On Fri, Jul 25, 2014 at 4:06 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> On 07/25/2014 08:10 AM, Nick Dyer wrote:
>>>
>>> On 24/07/14 22:19, Stephen Warren wrote:
>>
>> ...
>>
>>>> I've uploaded 2 logs to:
>>>>
>>>> http://avon.wwwdotorg.org/downloads/mxt-logs/
>>>> (note there's no directory indexing, so manually add the filenames below to
>>>> the URL)
>>>>
>>>> mxt-save-no-movement.xml
>>>>
>>>> This is with the whole series applied. Neither mouse movement nor clicks
>>>> works. I tried mxt-app --reset and it made no difference to the dump results.
>>>>
>>>> mxt-save-move-ok-no-clicking.xml
>>>>
>>>> This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
>>>> reverted; mouse movement works, but clicking doesn't.
>>>
>>>
>>> Great, this has identified the issue with mouse movement (touch).
>>>
>>> The config programmed into the NVRAM on your touch controller has the first
>>> byte of the T9 touchscreen object set to zero. This is the CTRL byte which
>>> enables/disables the touch object and what it reports. It is relying on
>>> this to enable the touchscreen on resume:
>>>
>>> https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
>>>
>>> My "use deep sleep mode when stopped" patch stops the driver writing to the
>>> T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
>>> used (ie zero, disabled). Going forward, deep sleep is more generic.
>>> Indeed, newer chips do not have T9 at all, or they might be using other
>>> touch objects. The deep sleep mode is a lower power state to be in, and is
>>> what Atmel recommends.
>>>
>>> However, it does mean changing the maxtouch cfg - you can write the 0x83 to
>>> the first byte of T9 and save it to NVRAM, by doing:
>>>
>>> mxt-app [device] -W -T9 83
>>> mxt-app [device] --backup
>>
>>
>> If I do that, then both mouse movement and "touch" clicks work:-)
>>
>> (Dmitry, I guess that means it's fine to go ahead and apply "Input: atmel_mxt_ts - use deep sleep mode when stopped".)
>>
>> I wonder why the configuration NVRAM in my device was incorrect though? I'm CCing a few ChromeOS people to try and find out any relevant history for the touchpad NVRAM settings on Venice2. Perhaps this is simply something that wasn't noticed because the driver used to initialize this configuration bit, so nobody realized the config in NVRAM was wrong.
>
> Where did you get the configuration file ? It is possible that we rely
> too much on mxt_start to turn on the T9.CTRL bit and have neglected
> its setting in the config file.
> If you can tell me where you get the config file I can do a check.

It was already flashed into the touchpad when I received the board. I 
did try to track down the firmware/config files a few months ago, but 
didn't manage to; I was told since they were already flashed so I didn't 
need them. The board is Venice2.

^ permalink raw reply

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Stephen Warren @ 2014-07-28 23:42 UTC (permalink / raw)
  To: Yufeng Shen, Nick Dyer
  Cc: Dmitry Torokhov, benson Leung, Daniel Kurtz, Henrik Rydberg,
	Joonyoung Shim, Alan Bowens, linux-input,
	linux-kernel@vger.kernel.org, Peter Meerwald, Olof Johansson,
	Sekhar Nori
In-Reply-To: <53D6BF50.3070306@wwwdotorg.org>

On 07/28/2014 03:23 PM, Stephen Warren wrote:
> On 07/28/2014 02:20 PM, Yufeng Shen wrote:
...
>> Where did you get the configuration file ? It is possible that we rely
>> too much on mxt_start to turn on the T9.CTRL bit and have neglected
>> its setting in the config file.
>> If you can tell me where you get the config file I can do a check.
>
> It was already flashed into the touchpad when I received the board. I
> did try to track down the firmware/config files a few months ago, but
> didn't manage to; I was told since they were already flashed so I didn't
> need them. The board is Venice2.

OK, I received the configuration and firmware file that's supposed to be 
in the touchpad.

I can see that the config file I was given has the "83" byte in the T9 
configuration, and in fact /almost/ exactly matches the configuration I 
have. I don't know why my T9 configuration was wrong before, but I 
suspect it's not worth trying to track that down.

Anyway, here's the diff between the two config files:

> # diff -u mxt-save-after-t9-83-write.xml 224sl.raw
> --- mxt-save-after-t9-83-write.xml	2014-07-25 19:41:45.000000000 +0000
> +++ 224sl.raw	2014-07-28 23:25:49.000000000 +0000
> @@ -1,8 +1,7 @@
>  OBP_RAW V1
>  82 01 10 AA 12 0C 16
>  F5AF33
> -000000
> -0025 0000 0082 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +E21E65
>  0026 0000 0008 00 00 00 00 00 00 00 00
>  0007 0000 0004 20 10 32 00
>  0008 0000 000A 1E 00 28 28 00 00 00 00 00 00

It seems that the T25(?) entry is missing in the new/expected 
configuration file. I figured I'd try out the new/expected configuration 
file, so did:

# ./obp-utils/mxt-app -d i2c-dev:1-004b --load 224sl.raw
# ./obp-utils/mxt-app -d i2c-dev:1-004b --save 
mxt-save-after-loading-224sl.raw.xml

At this point, mxt-save-after-loading-224sl.raw.xml contains identical 
content to mxt-save-after-t9-83-write.xml (my previous backup). It looks 
like the new configuration isn't being loaded correctly, or perhaps 
configuration loading doesn't delete entries that are simply not in the 
new configuration file?

I subsequently did the following in case --save is reading from the 
NVRAM rather than RAM:

# ./obp-utils/mxt-app -d i2c-dev:1-004b --backup
# ./obp-utils/mxt-app -d i2c-dev:1-004b --save 
mxt-save-after-loading-224sl.raw.xml

... but that made no difference.

I haven't yet tried upgrading or otherwise using the new firmware image. 
I'd like to make sure config load/save is fully working first, in case 
there's any common problem between the two.

^ permalink raw reply

* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Yufeng Shen @ 2014-07-29  0:10 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Nick Dyer, Dmitry Torokhov, benson Leung, Daniel Kurtz,
	Henrik Rydberg, Joonyoung Shim, Alan Bowens, linux-input,
	linux-kernel@vger.kernel.org, Peter Meerwald, Olof Johansson,
	Sekhar Nori
In-Reply-To: <53D6DFE7.3040901@wwwdotorg.org>

On Mon, Jul 28, 2014 at 7:42 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/28/2014 03:23 PM, Stephen Warren wrote:
>>
>> On 07/28/2014 02:20 PM, Yufeng Shen wrote:
>
> ...
>
>>> Where did you get the configuration file ? It is possible that we rely
>>> too much on mxt_start to turn on the T9.CTRL bit and have neglected
>>> its setting in the config file.
>>> If you can tell me where you get the config file I can do a check.
>>
>>
>> It was already flashed into the touchpad when I received the board. I
>> did try to track down the firmware/config files a few months ago, but
>> didn't manage to; I was told since they were already flashed so I didn't
>> need them. The board is Venice2.
>
>
> OK, I received the configuration and firmware file that's supposed to be in
> the touchpad.
>
> I can see that the config file I was given has the "83" byte in the T9
> configuration, and in fact /almost/ exactly matches the configuration I
> have. I don't know why my T9 configuration was wrong before, but I suspect
> it's not worth trying to track that down.
>
> Anyway, here's the diff between the two config files:
>
>> # diff -u mxt-save-after-t9-83-write.xml 224sl.raw
>> --- mxt-save-after-t9-83-write.xml      2014-07-25 19:41:45.000000000
>> +0000
>> +++ 224sl.raw   2014-07-28 23:25:49.000000000 +0000
>> @@ -1,8 +1,7 @@
>>  OBP_RAW V1
>>  82 01 10 AA 12 0C 16
>>  F5AF33
>> -000000
>> -0025 0000 0082 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> 00 00 00 00 00 00 00 00 00 00 00
>> +E21E65
>>  0026 0000 0008 00 00 00 00 00 00 00 00
>>  0007 0000 0004 20 10 32 00
>>  0008 0000 000A 1E 00 28 28 00 00 00 00 00 00
>
>
> It seems that the T25(?) entry is missing in the new/expected configuration
> file. I figured I'd try out the new/expected configuration file, so did:
>

T37 (0x25) is DEBUG_DIAGNOSTIC object which the host can read debugging info
from. It is not useful to have a initial config for it so usually CrOS
system would just
don't include configuration for this object.

Nick, I want to confirm with you that does T37 contribute to config
checksum computation ?

> # ./obp-utils/mxt-app -d i2c-dev:1-004b --load 224sl.raw
> # ./obp-utils/mxt-app -d i2c-dev:1-004b --save
> mxt-save-after-loading-224sl.raw.xml
>
> At this point, mxt-save-after-loading-224sl.raw.xml contains identical
> content to mxt-save-after-t9-83-write.xml (my previous backup). It looks
> like the new configuration isn't being loaded correctly, or perhaps
> configuration loading doesn't delete entries that are simply not in the new
> configuration file?
>

Yeah, I would guess since T37 is not in the config, so whatever in the NVRAM
stays the same and when you --save its original value gets dumped.

> I subsequently did the following in case --save is reading from the NVRAM
> rather than RAM:
>
> # ./obp-utils/mxt-app -d i2c-dev:1-004b --backup
> # ./obp-utils/mxt-app -d i2c-dev:1-004b --save
> mxt-save-after-loading-224sl.raw.xml
>
> ... but that made no difference.
>
> I haven't yet tried upgrading or otherwise using the new firmware image. I'd
> like to make sure config load/save is fully working first, in case there's
> any common problem between the two.

^ permalink raw reply

* [PATCH] alps: rushmore and v7 resolution support
From: Hans de Goede @ 2014-07-29  8:07 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Yunkang Tang, linux-input, Hans de Goede

Add support for querying the physical size from the touchpad for rushmore
and v7 touchpads, and use that to tell userspace the device resolution.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/mouse/alps.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/input/mouse/alps.h |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index ac9fdbd..1531387 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1783,6 +1783,45 @@ error:
 	return -1;
 }
 
+static int alps_get_v3_v7_resolution(struct psmouse *psmouse, int reg_pitch)
+{
+	int reg, x_pitch, y_pitch, x_electrode, y_electrode, x_phys, y_phys;
+	struct alps_data *priv = psmouse->private;
+
+	reg = alps_command_mode_read_reg(psmouse, reg_pitch);
+	if (reg < 0)
+		return reg;
+
+	x_pitch = (char)(reg << 4) >> 4; /* sign extend lower 4 bits */
+	x_pitch = 50 + 2 * x_pitch; /* In 0.1 mm units */
+
+	y_pitch = (char)reg >> 4; /* sign extend upper 4 bits */
+	y_pitch = 36 + 2 * y_pitch; /* In 0.1 mm units */
+
+	reg = alps_command_mode_read_reg(psmouse, reg_pitch + 1);
+	if (reg < 0)
+		return reg;
+
+	x_electrode = (char)(reg << 4) >> 4; /* sign extend lower 4 bits */
+	x_electrode = 17 + x_electrode;
+
+	y_electrode = (char)reg >> 4; /* sign extend upper 4 bits */
+	y_electrode = 13 + y_electrode;
+
+	x_phys = x_pitch * (x_electrode - 1); /* In 0.1 mm units */
+	y_phys = y_pitch * (y_electrode - 1); /* In 0.1 mm units */
+
+	priv->x_res = priv->x_max * 10 / x_phys; /* units / mm */
+	priv->y_res = priv->y_max * 10 / y_phys; /* units / mm */
+
+	psmouse_info(psmouse,
+		"alps: pitch %dx%d num-electrodes %dx%d physical size %dx%d mm res %dx%d\n",
+		x_pitch, y_pitch, x_electrode, y_electrode,
+		x_phys / 10, y_phys / 10, priv->x_res, priv->y_res);
+
+	return 0;
+}
+
 static int alps_hw_init_rushmore_v3(struct psmouse *psmouse)
 {
 	struct alps_data *priv = psmouse->private;
@@ -1803,6 +1842,9 @@ static int alps_hw_init_rushmore_v3(struct psmouse *psmouse)
 	    alps_command_mode_write_reg(psmouse, 0xc2cb, 0x00))
 		goto error;
 
+	if (alps_get_v3_v7_resolution(psmouse, 0xc2da))
+		goto error;
+
 	reg_val = alps_command_mode_read_reg(psmouse, 0xc2c6);
 	if (reg_val == -1)
 		goto error;
@@ -1988,6 +2030,9 @@ static int alps_hw_init_v7(struct psmouse *psmouse)
 	    alps_command_mode_read_reg(psmouse, 0xc2d9) == -1)
 		goto error;
 
+	if (alps_get_v3_v7_resolution(psmouse, 0xc397))
+		goto error;
+
 	if (alps_command_mode_write_reg(psmouse, 0xc2c9, 0x64))
 		goto error;
 
@@ -2221,6 +2266,9 @@ static void alps_set_abs_params_mt(struct alps_data *priv,
 	input_set_abs_params(dev1, ABS_MT_POSITION_X, 0, priv->x_max, 0, 0);
 	input_set_abs_params(dev1, ABS_MT_POSITION_Y, 0, priv->y_max, 0, 0);
 
+	input_abs_set_res(dev1, ABS_MT_POSITION_X, priv->x_res);
+	input_abs_set_res(dev1, ABS_MT_POSITION_Y, priv->y_res);
+
 	input_mt_init_slots(dev1, MAX_TOUCHES, INPUT_MT_POINTER |
 		INPUT_MT_DROP_UNUSED | INPUT_MT_TRACK | INPUT_MT_SEMI_MT);
 
diff --git a/drivers/input/mouse/alps.h b/drivers/input/mouse/alps.h
index a98ac9b..66240b4 100644
--- a/drivers/input/mouse/alps.h
+++ b/drivers/input/mouse/alps.h
@@ -174,6 +174,8 @@ struct alps_data {
 	int y_max;
 	int x_bits;
 	int y_bits;
+	unsigned int x_res;
+	unsigned int y_res;
 
 	int (*hw_init)(struct psmouse *psmouse);
 	void (*process_packet)(struct psmouse *psmouse);
-- 
2.0.1


^ permalink raw reply related

* Re: [PATCH] HID: rmi: change logging level of log messages related to unexpected reports
From: Jiri Kosina @ 2014-07-29  9:10 UTC (permalink / raw)
  To: Benjamin Tissoires  ; +Cc: Andrew Duggan, linux-input, linux-kernel
In-Reply-To: <20140715190723.GC21045@mail.corp.redhat.com>

On Tue, 15 Jul 2014, Benjamin Tissoires   wrote:

> On Jul 11 2014 or thereabouts, Andrew Duggan wrote:
> > Userspace tools may use hidraw to perform operations on the device from userspace while
> > hid-rmi is bound to the device. This can cause hid-rmi to print error messages when its
> > ->raw_event() callback gets called as the reports pass through the HID stack. In this case
> > receiving responses which were not initiated by hid-rmi is not actually an error so the resulting
> > error messages are incorrect and misleading. This patch changes the log messages to debug so
> > that the messages can be turned on in the event that there is a problem and there is not
> > a userspace tool running.
> > 
> > Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> > ---
> 
> Fair enough
> 
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

[ sorry for late response, I am just re-appearing from vacation-land ]

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: rmi: add additional checks for the existence of optional queries in order to compute the address of Query 12.
From: Jiri Kosina @ 2014-07-29  9:10 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Andrew Duggan, linux-input, linux-kernel
In-Reply-To: <20140715190702.GB21045@mail.corp.redhat.com>

On Tue, 15 Jul 2014, Benjamin Tissoires wrote:

> On Jul 11 2014 or thereabouts, Andrew Duggan wrote:
> > There are additional queries which are optional and may not be present depending on the configuration of the firmware. Knowing which queries are present is needed to properly compute the address of Query 12 and all subsequent queries. Additional bits in Query 1 are used to indicate the presence of these optional queries.
> > 
> > Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> > ---
> 
> Looks good to me
> 
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Applied.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: i2c-hid: call the hid driver's suspend and resume callbacks
From: Jiri Kosina @ 2014-07-29  9:11 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Andrew Duggan, linux-input, linux-kernel, Vincent Huang,
	Mika Westerberg
In-Reply-To: <20140717170031.GC27586@mail.corp.redhat.com>

On Thu, 17 Jul 2014, Benjamin Tissoires wrote:

> > Currently, the i2c-hid driver does not call the suspend, resume, and
> > reset_resume callbacks in the hid_driver struct when those events occur.
> > This means that HID drivers for i2c-hid devices will not be able to execute
> > commands which may be needed during suspend or resume. One example is when a
> > touchpad using the hid-multitouch driver gets reset by i2c-hid coming out of
> > resume. Since the reset_resume callback never gets called the device is never
> > put back into the correct input mode. This patch calls the suspend and resume
> > callbacks and tries to duplicate the functionality of the usb-hid driver.
> > 
> > Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
> > Signed-off-by: Vincent Huang <vincent.huang@tw.synaptics.com>
> > ---
> > Hi Benjamin,
> > 
> > This is the patch which Vincent and I came up with to fix the behavior which we were seeing with touchpads not being operational after resuming. We only use the reset_resume callback since i2c_hid_resume is always doing a hardware reset and it doesn't seem like there is a condition in which the standard resume callback would be appropriate. Also, is a full hardware reset always needed when resuming?
> 
> Hi Andrew,
> 
> well, the PM management of i2c_hid has been handled by Mika (in CC) and
> I am unfortunately not able to have a strong opinion on this. I still
> lack of hardware properly supporting the suspend/resume mechanism.
> 
> So, you are in charge of that. IIRC, nothing prevents from not calling
> hw_reset, but I guess this was the safer implementation we could have.
> 
> Anyway, the patch looks good:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

I am queuing this for 3.17. Mika, if you have any objections, please speak 
up.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCHES v2] Add support for more Huion tablets
From: Jiri Kosina @ 2014-07-29  9:22 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Nikolai Kondrashov, linux-input, DIGImend-devel
In-Reply-To: <CAN+gG=EPK-KENSFBYLEaX_OzaAUHrvU=1mnBgmPoCQgM9rLuXA@mail.gmail.com>

On Mon, 28 Jul 2014, Benjamin Tissoires wrote:

> > This is the second version of the patch set adding support for more Huion
> > tablets [1].
> >
> > This version has the "hid: huion: Invert in-range on specific product" patch
> > removed as requested by Benjamin Tissoires.
> >
> > Tested with Huion H610N.
> >
> > Nick
> >
> > [PATCH 1/4] hid: huion: Use "tablet" instead of specific model
> > [PATCH 2/4] hid: huion: Don't ignore other interfaces
> > [PATCH 3/4] hid: huion: Switch to generating report descriptor
> > [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID
> >
> >  drivers/hid/hid-core.c  |   2 +-
> >  drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++----------------
> >  drivers/hid/hid-ids.h   |   2 +-
> >  3 files changed, 177 insertions(+), 92 deletions(-)
> >
> > [1] http://thread.gmane.org/gmane.linux.kernel.input/37187
> 
> Just so that the info does not get lost, the v2 of the patch series is still:
> 
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Queuing for 3.17, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH v5 0/4] HID: lenovo: Add support for Lenovo Compact Keyboard
From: Jiri Kosina @ 2014-07-29  9:25 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: Jamie Lentin, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <20140725160527.13cbeae1108fce5e09b6daae@ao2.it>

On Fri, 25 Jul 2014, Antonio Ospite wrote:

> > This patchset follows on from my previous attempts to add support for
> > these keyboards from Lenovo.
> > 
> > Antonio, I've used cptkbd_data and left data_pointer for now. It'd be
> > an easy patch to change data_pointer to tpkbd_data, but is drifting
> > away from what this patchset is supposed to do. Could send that
> > afterwards though.
> 
> Sure, no problem. I think we are good to go.

I am queuing this patchset for 3.17, thanks everybody.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH v2] HID: rmi: check that report ids exist in the report_id_hash before accessing their size
From: Jiri Kosina @ 2014-07-29  9:29 UTC (permalink / raw)
  To: Andrew Duggan; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <1405638885-8014-1-git-send-email-aduggan@synaptics.com>

On Thu, 17 Jul 2014, Andrew Duggan wrote:

> It is possible that the hid-rmi driver could get loaded onto a device which does not have the
> expected report ids. This should not happen because it would indicate that the hid-rmi driver is
> not compatible with that device. However, if it does happen it should return an error from probe
> instead of dereferencing a null pointer.
> 
> related bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=80091
> 
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] HID: rmi: only bind the hid-rmi driver to the mouse interface of composite USB devices
From: Jiri Kosina @ 2014-07-29  9:29 UTC (permalink / raw)
  To: Andrew Duggan; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <1405638885-8014-2-git-send-email-aduggan@synaptics.com>

On Thu, 17 Jul 2014, Andrew Duggan wrote:

> On composite HID devices there may be multiple HID devices on separate interfaces, but hid-rmi
> should only bind to the mouse interface. One example is the Dell Venue 11 Pro's keyboard dock
> which contains a composite USB device with a HID touchpad and HID keyboard on separate intefaces.
> Since the USB Vendor ID is Synaptic's, hid-core is currently trying to bind hid-rmi to all\of
> the HID devices. This patch ensures that hid-rmi only binds to the mouse interface.
> 
> related bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=80091
> 
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [v2] input: drv260x: Add TI drv260x haptics driver
From: Mark Rutland @ 2014-07-29  9:37 UTC (permalink / raw)
  To: Dan Murphy
  Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, dmitry.torokhov@gmail.com
In-Reply-To: <1406566403-1436-1-git-send-email-dmurphy@ti.com>

On Mon, Jul 28, 2014 at 05:53:23PM +0100, Dan Murphy wrote:
> Add the TI drv260x haptics/vibrator driver.
> This device uses the input force feedback
> to produce a wave form to driver an
> ERM or LRA actuator device.
> 
> The initial driver supports the devices
> real time playback mode.  But the device
> has additional wave patterns in ROM.
> 
> This functionality will be added in
> future patchsets.
> 
> Product data sheet is located here:
> http://www.ti.com/product/drv2605
> 
> Signed-off-by: Dan Murphy <dmurphy@ti.com>
> ---
> 
> v2 - Fixed binding doc and patch headline - https://patchwork.kernel.org/patch/4619641/
> 
>  .../devicetree/bindings/input/ti,drv260x.txt       |   44 ++
>  drivers/input/misc/Kconfig                         |    9 +
>  drivers/input/misc/Makefile                        |    1 +
>  drivers/input/misc/drv260x.c                       |  537 ++++++++++++++++++++
>  include/dt-bindings/input/ti-drv260x.h             |   30 ++
>  include/linux/input/drv260x.h                      |  181 +++++++
>  6 files changed, 802 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/ti,drv260x.txt
>  create mode 100644 drivers/input/misc/drv260x.c
>  create mode 100644 include/dt-bindings/input/ti-drv260x.h
>  create mode 100644 include/linux/input/drv260x.h
> 
> diff --git a/Documentation/devicetree/bindings/input/ti,drv260x.txt b/Documentation/devicetree/bindings/input/ti,drv260x.txt
> new file mode 100644
> index 0000000..930429b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/ti,drv260x.txt
> @@ -0,0 +1,44 @@
> +Texas Instruments - drv260x Haptics driver family
> +
> +The drv260x family serial control bus communicates through I2C protocols
> +
> +Required properties:
> +       - compatible - One of:
> +               "ti,drv2604" - DRV2604
> +               "ti,drv2605" - DRV2605
> +               "ti,drv2605l" - DRV2605L
> +       - reg -  I2C slave address
> +       - mode - Power up mode of the chip (defined in include/dt-bindings/input/ti-drv260x.h)
> +               DRV260X_RTP_MODE - Real time playback mode
> +               DRV260X_LRA_MODE - Linear Resonance Actuator mode (Piezoelectric)
> +               DRV260X_ERM_MODE - Eccentric Rotating Mass mode (Rotary vibrator)

What exactly are these, and why does the kernel not decide?

> +       - library_sel - Library to use at power up (defined in include/dt-bindings/input/ti-drv260x.h)

Please s/_/-/ in all property names.

> +               DRV260X_LIB_A - Pre-programmed Library
> +               DRV260X_LIB_B - Pre-programmed Library
> +               DRV260X_LIB_C - Pre-programmed Library
> +               DRV260X_LIB_D - Pre-programmed Library
> +               DRV260X_LIB_E - Pre-programmed Library
> +               DRV260X_LIB_F - Pre-programmed Library

What exactly are these, and why does the kernel not decide?

> +
> +Optional properties:
> +       - enable-gpio - gpio pin to enable/disable the device.
> +       - vib_rated_voltage - The rated voltage of the actuator.  If this is not
> +                                                 set then the value will be defaulted to 0x90 in the
> +                                                 driver.

What units is this in?

Don't mention the driver, just say "if not 0x90" (but with a better
description of what 0x90 actually means).

> +       - vib_overdrive_voltage - The overdrive voltage of the actuator.
> +                                                       If this is not set then the value
> +                                                       will be defaulted to 0x90 in the driver.

Similarly on both points.

[...]

> +static int drv260x_probe(struct i2c_client *client,
> +                          const struct i2c_device_id *id)
> +{
> +       struct drv260x_data *haptics;
> +       struct device_node *np = client->dev.of_node;
> +       struct drv260x_platform_data *pdata = client->dev.platform_data;
> +       struct ff_device *ff;
> +       int ret;
> +       int voltage;
> +
> +       haptics = devm_kzalloc(&client->dev, sizeof(*haptics), GFP_KERNEL);
> +       if (!haptics)
> +               return -ENOMEM;
> +
> +       haptics->rated_voltage = DRV260X_DEF_OD_CLAMP_VOLT;
> +       haptics->rated_voltage = DRV260X_DEF_RATED_VOLT;
> +
> +       if (np) {
> +               ret = of_property_read_u32(np, "mode", &haptics->mode);
> +               if (ret < 0) {
> +                       dev_err(&client->dev,
> +                               "%s: No entry for mode\n", __func__);
> +
> +                       return ret;
> +               }

No sanity checking on the actual value?

As far as I can see, haptics->mode is an int, not a u32.

> +               ret = of_property_read_u32(np, "library_sel",
> +                                       &haptics->library);
> +               if (ret < 0) {
> +                       dev_err(&client->dev,
> +                               "%s: No entry for library selection\n", __func__);
> +
> +                       return ret;
> +               }

Sanity checking?

haptics->library is not a u32.

> +               ret = of_property_read_u32(np, "vib_rated_voltage",
> +                                       &voltage);
> +               if (!ret)
> +                       haptics->rated_voltage = drv260x_calculate_voltage(voltage);
> +
> +
> +               ret = of_property_read_u32(np, "vib_overdrive_voltage",
> +                                       &voltage);
> +               if (!ret)
> +                       haptics->overdrive_voltage = drv260x_calculate_voltage(voltage);
> +

And again, on both points.

> +       } else if (pdata) {
> +               haptics->mode = pdata->mode;
> +               haptics->library = pdata->library_selection;
> +               if (pdata->vib_overdrive_voltage)
> +                       haptics->overdrive_voltage = drv260x_calculate_voltage(pdata->vib_overdrive_voltage);
> +               if (pdata->vib_rated_voltage)
> +                       haptics->rated_voltage = drv260x_calculate_voltage(pdata->vib_rated_voltage);
> +       } else {
> +               dev_err(&client->dev, "Platform data not set\n");
> +               return -ENODEV;
> +       }
> +
> +       haptics->regulator = regulator_get(&client->dev, "vbat");

This wasn't in the binding.

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH v2] HID: cp2112: add I2C mode
From: Jiri Kosina @ 2014-07-29  9:38 UTC (permalink / raw)
  To: Antonio Borneo
  Cc: linux-input, David Barksdale, linux-kernel, Benjamin Tissoires
In-Reply-To: <1404775539-10727-1-git-send-email-borneo.antonio@gmail.com>

On Tue, 8 Jul 2014, Antonio Borneo wrote:

> cp2112 supports single I2C read/write transactions.
> It can't combine I2C transactions.
> 
> Add master_xfer, using similar code flow as for smbus_xfer.
> 
> Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> ChangeLog v1->v2:
> - In cp2112_i2c_xfer() set variable "ret" directly with the
>   expected return value: either an error number or the number
>   of transferred messages.

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: Trouble with an HP stylus
From: Jiri Kosina @ 2014-07-29  9:50 UTC (permalink / raw)
  To: Éric Brunet; +Cc: linux-input
In-Reply-To: <2325454.r4RE9vHMoF@romarin>

On Thu, 24 Jul 2014, Éric Brunet wrote:

> I have a HP EliteBook Revolve 810 (which is a hybrid laptop/tablet) with
> the HP active pen that goes with it. I have installed fedora 20 (kernel
> 3.15.6-200.fc20.x86_64) on this, and the pen does not work out of the box.
> Playing with evtest, it appears that the kernel reports the X coordinate in
> ABS_X and in ABS_Y, while the Y coordinate goes into ABS_Z and ABS_RX; here
> are a few lines of output:
> 
> Event: time 1406153996.865044, -------------- SYN_REPORT ------------
> Event: time 1406153996.873047, type 3 (EV_ABS), code 0 (ABS_X), value 1087
> Event: time 1406153996.873047, type 3 (EV_ABS), code 1 (ABS_Y), value 1087
> Event: time 1406153996.873047, type 3 (EV_ABS), code 2 (ABS_Z), value 1599
> Event: time 1406153996.873047, type 3 (EV_ABS), code 3 (ABS_RX), value 1599
> 
> Of course, XOrg's evdev driver is confused and the pointer is stuck on the
> diagonal X=Y.
> 
> I guess this hardware needs a new quirk to usbhid, unless there is already
> a way to correct it ?
> 
> Here is the content of /sys/class/input/event8/device/uevent:
> 
> PRODUCT=3/3eb/840b/111
> NAME="Atmel Atmel maXTouch Digitizer Pen"
> PHYS="usb-0000:00:1a.0-1.1/input0"
> UNIQ=""
> PROP=0
> EV=1b
> KEY=c01 1 0 0 0 0
> ABS=100000f
> MSC=10
> MODALIAS=input:b0003v03EBp840Be0111-
> e0,1,3,4,k100,140,14A,14B,ra0,1,2,3,18,m4,lsfw
> 
> I hope I gave enough information on this. I don't read the mailing list,
> please CC any answer.

Does the patch below fix the problem for you please?






diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 31e6727..d48574d 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -97,6 +97,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_SYMBOL, USB_DEVICE_ID_SYMBOL_SCANNER_2, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_TPV, USB_DEVICE_ID_TPV_OPTICAL_TOUCHSCREEN, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD, HID_QUIRK_NOGET },
+	{ 0x03EB, 0x840B, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_PF1209, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_WP4030U, HID_QUIRK_MULTI_INPUT },
 	{ USB_VENDOR_ID_UCLOGIC, USB_DEVICE_ID_UCLOGIC_TABLET_KNA5, HID_QUIRK_MULTI_INPUT },

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH] fix for jabra gn9350e (hid)
From: Jiri Kosina @ 2014-07-29  9:58 UTC (permalink / raw)
  To: Patrick Plattes; +Cc: linux-input, linux-kernel
In-Reply-To: <1404543486-21080-1-git-send-email-patrick@erdbeere.net>

On Sat, 5 Jul 2014, Patrick Plattes wrote:

> Ignore Jabra GN9350E HID interface. USB audio is working nicely,
> but registering as HID blocks USB mouse buttons. Since special userspace
> programs are needed we will avoid attaching usbhid drivers in general.

Applied.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] hid: usbhid: Use flag HID_DISCONNECTED when a usb device is removed
From: Jiri Kosina @ 2014-07-29 11:06 UTC (permalink / raw)
  To: Reyad Attiyat; +Cc: linux-input, srinivas.pandruvada, linux-kernel
In-Reply-To: <1406265181-4009-1-git-send-email-reyad.attiyat@gmail.com>

On Fri, 25 Jul 2014, Reyad Attiyat wrote:

> Set disconnected flag in struct usbhid when a usb device
> is removed. Check for disconnected flag before sending urb
> requests. This prevents a kernel panic when a hid driver calls
> hid_hw_request() after removing a usb device.
> 
> Signed-off-by: Reyad Attiyat <reyad.attiyat@gmail.com>

I've included the full oops output into the changelog and applied, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 3/3] driver:gpio remove all usage of gpio_remove retval in driver
From: Tomi Valkeinen @ 2014-07-29 11:30 UTC (permalink / raw)
  To: Linus Walleij, abdoulaye berthe, arm@kernel.org, Ralf Baechle,
	Benjamin Herrenschmidt, Rafał Miłecki, Jiri Kosina,
	Dmitry Torokhov, Bryan Wu, Mauro Carvalho Chehab, Lee Jones,
	Samuel Ortiz, Matthew Garrett, Michael Buesch, Greg KH,
	Mark Brown, Liam Girdwood
  Cc: Alexandre Courbot, linux-gpio@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mips,
	linux-sh@vger.kernel.org, Linux Input,
	linux-media@vger.kernel.org, linux-serial@vger.kernel.org,
	linux-fbdev@vger.kernel.org, alsa-devel@alsa-project.org
In-Reply-To: <CACRpkdasp9bLULT7NJM9nYX58rRSsQKXFddOLz9Ah6kp-j-3=Q@mail.gmail.com>

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

On 22/07/14 18:08, Linus Walleij wrote:
> On Sat, Jul 12, 2014 at 10:30 PM, abdoulaye berthe <berthe.ab@gmail.com> wrote:
> 
> Heads up. Requesting ACKs for this patch or I'm atleast warning that it will be
> applied. We're getting rid of the return value from gpiochip_remove().

>>  drivers/video/fbdev/via/via-gpio.c             | 10 +++-------
> 
> Tomi can you ACK this?

Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: PATCH change for HID_BATTERY_STRENGTH kconfig
From: Jiri Kosina @ 2014-07-29 11:39 UTC (permalink / raw)
  To: Daniel Nicoletti; +Cc: linux-input
In-Reply-To: <CACo8zOcm3B7WdQpHA3J2ZtP6AWUrtAo_L63mpYJR0w13jNwLpQ@mail.gmail.com>

On Mon, 23 Jun 2014, Daniel Nicoletti wrote:

> 2013-05-23 19:16 GMT-03:00 Jiri Kosina <jkosina@suse.cz>:
> > On Mon, 13 May 2013, Daniel Nicoletti wrote:
> >
> >> Hi,
> >> I'd like to propose this patch while removes the need for
> >> hid to be compiled build-in, as this is the same behavior
> >> that WiiMote and Wacom have.
> >> Is this ok?
> >
> > Makes sense to me. Could you please resend with proper changelog and
> > Signed-off-by: line?
> >
> > Thanks.
> 
> Sorry it took more than a year for me to do this,
> so I called git send-email, and so far the CC and
> linux-input didn't get it, tho it said result OK.
> git send-email --to "Jiri Kosina <jkosina@suse.cz>" --cc
> "linux-input@vger.kernel.org"
> 0001-HID-Allows-for-HID_BATTERY_STRENGTH-config-be-enable.patch
> 
> I'm pasting it here hoping gmail doesn't screw
> it, if you prefer that I send it using the above
> command mind you telling what I did wrong?
> 
> Thanks.
> 
> >From df740fd81dffbb74e13c622392fe9a945006fd3c Mon Sep 17 00:00:00 2001
> From: Daniel Nicoletti <dantti12@gmail.com>
> Date: Mon, 23 Jun 2014 10:31:47 -0300
> Subject: [PATCH] HID: Allows for HID_BATTERY_STRENGTH config be enabled
>  without the need for HID module to be built-in.
> 

It'd be nice to insert a few sentences of changelog here.

Thanks.

> Signed-off-by: Daniel Nicoletti <dantti12@gmail.com>
> ---
>  drivers/hid/Kconfig | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index fb52f3f..d6d85ad 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -27,7 +27,8 @@ if HID
> 
>  config HID_BATTERY_STRENGTH
>         bool "Battery level reporting for HID devices"
> -       depends on HID && POWER_SUPPLY && HID = POWER_SUPPLY
> +       depends on HID
> +       select POWER_SUPPLY
>         default n
>         ---help---
>         This option adds support of reporting battery strength (for HID devices
> -- 
> 2.0.0
> 

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Hello
From: Noemi Alvarez @ 2014-07-29  8:23 UTC (permalink / raw)



I want to keep up with you with hope for friendship if you are interested.
If you don't mind i will like you to write me back. I am waiting to read
from you, because i have something important and urgent to discuss with
you. I will also send some of my beautiful photos to you.


^ permalink raw reply


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