Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

2014-07-28 Thread Yufeng Shen
On Mon, Jul 28, 2014 at 7:42 PM, Stephen Warren  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.0
>> +
>> +++ 224sl.raw   2014-07-28 23:25:49.0 +
>> @@ -1,8 +1,7 @@
>>  OBP_RAW V1
>>  82 01 10 AA 12 0C 16
>>  F5AF33
>> -00
>> -0025  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  0008 00 00 00 00 00 00 00 00
>>  0007  0004 20 10 32 00
>>  0008  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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

2014-07-28 Thread Yufeng Shen
On Fri, Jul 25, 2014 at 4:06 PM, Stephen Warren  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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

2014-07-28 Thread Yufeng Shen
On Fri, Jul 25, 2014 at 4:06 PM, Stephen Warren swar...@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?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc

2014-07-28 Thread Yufeng Shen
On Mon, Jul 28, 2014 at 7:42 PM, Stephen Warren swar...@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.0
 +
 +++ 224sl.raw   2014-07-28 23:25:49.0 +
 @@ -1,8 +1,7 @@
  OBP_RAW V1
  82 01 10 AA 12 0C 16
  F5AF33
 -00
 -0025  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  0008 00 00 00 00 00 00 00 00
  0007  0004 20 10 32 00
  0008  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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata

2014-05-26 Thread Yufeng Shen
On Sat, May 24, 2014 at 8:41 AM, Nick Dyer  wrote:
> Yufeng Shen wrote:
>> On Thu, May 22, 2014 at 10:29 AM, Nick Dyer  wrote:
>>> Dmitry Torokhov wrote:
>>>>>> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data 
>>>>>> is
>>>>>> provided.
>>>>
>>>> I think if there is no platform data we should use 0 as IRQ falgs and
>>>> assume that IRQ line is properly configured by the board code or via
>>>> device tree.
>>>
>>> Benson/Yufeng - do you still have a requirement to probe without platform
>>> data or device tree? I'm just merging in some changes to add device tree
>>> support, and it would simplify things a bit if I can drop this patch.
>>
>> It has been working for quite a while for boards/devices that don't
>> provide platform data. If we drop the default IRQ flags, sure we can add
>> code for each board to configure the IRQ separately, but that's just
>> adding extra work. Is there strong reason why we should not do the
>> default setting in the driver if it is not already configured in
>> platform data?
>
> OK, I will keep it in my tree for the moment, since you are using it.
>
> The reason I checked is that in general, I would like to be conservative
> about what is pushed upstream, because it will need maintaining for a long
> time.
>
> The other reason is that in fact Atmel recommend IRQF_TRIGGER_LOW for these
> chips, not IRQF_TRIGGER_FALLING, so there is a bit of an inconsistency here.

I think I chose IRQF_TRIGGER_FALLING is because when I do a search in
the upstream
code where platform is configured, the irq is always set to be
IRQF_TRIGGER_FALLING
so I was assuming  IRQF_TRIGGER_FALLING is a safe bet.

But you would definitely know better than me on this since the atmel
chips that I have
access to are quite limited.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata

2014-05-26 Thread Yufeng Shen
On Mon, May 26, 2014 at 1:23 AM, Dmitry Torokhov
 wrote:
>
> On Fri, May 23, 2014 at 12:37:46PM -0400, Yufeng Shen wrote:
> > On Thu, May 22, 2014 at 10:29 AM, Nick Dyer  wrote:
> > >
> > > Dmitry Torokhov wrote:
> > > > On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote:
> > > >>> From: Yufeng Shen 
> > > >>> This is the preparation for supporting the code path when there is
> > > >>> platform data provided and still boot the device into a sane state
> > > >>> with backup NVRAM config.
> > > >>>
> > > >>> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform 
> > > >>> data is
> > > >>> provided.
> > > >
> > > > I think if there is no platform data we should use 0 as IRQ falgs and
> > > > assume that IRQ line is properly configured by the board code or via
> > > > device tree.
> > >
> > > Beson/Yufeng - do you still have a requirement to probe without platform
> > > data or device tree? I'm just merging in some changes to add device tree
> > > support, and it would simplify things a bit if I can drop this patch.
> >
> >
> > It has been working for quite a while for boards/devices that don't
> > provide platform
> > data. If we drop the default IRQ flags, sure we can add code for each
> > board to configure
> > the IRQ separately, but that's just adding extra work. Is there strong
> > reason why we
> > should not do the default setting in the driver if it is not already
> > configured in platform
> > data ?
>
>
> I am not saying that board code needs to add platform data. I am saying
> that the board code needs to set up interrupt properly (via
> irq_set_irq_type() or similar) and then the driver can use 0 as irqflags
> argument in request_irq() in absence of DT/platform data.
>
> Thanks.
>

So my argument is mainly based on that the existing code is working (boards
that do not have platform data are relying on the driver to set the
default irq),
and change the default value would need extra work to setup the irq, say as you
suggested through irq_set_irq_type(). I am no expert in irq so it could be true
that your suggested way is indeed better. I am in favor of keeping
this patch is only
because it requires no extra change for existing code that are using it.



> --
> Dmitry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata

2014-05-26 Thread Yufeng Shen
On Mon, May 26, 2014 at 1:23 AM, Dmitry Torokhov
dmitry.torok...@gmail.com wrote:

 On Fri, May 23, 2014 at 12:37:46PM -0400, Yufeng Shen wrote:
  On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote:
  
   Dmitry Torokhov wrote:
On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote:
From: Yufeng Shen mile...@chromium.org
This is the preparation for supporting the code path when there is
platform data provided and still boot the device into a sane state
with backup NVRAM config.
   
Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform 
data is
provided.
   
I think if there is no platform data we should use 0 as IRQ falgs and
assume that IRQ line is properly configured by the board code or via
device tree.
  
   Beson/Yufeng - do you still have a requirement to probe without platform
   data or device tree? I'm just merging in some changes to add device tree
   support, and it would simplify things a bit if I can drop this patch.
 
 
  It has been working for quite a while for boards/devices that don't
  provide platform
  data. If we drop the default IRQ flags, sure we can add code for each
  board to configure
  the IRQ separately, but that's just adding extra work. Is there strong
  reason why we
  should not do the default setting in the driver if it is not already
  configured in platform
  data ?


 I am not saying that board code needs to add platform data. I am saying
 that the board code needs to set up interrupt properly (via
 irq_set_irq_type() or similar) and then the driver can use 0 as irqflags
 argument in request_irq() in absence of DT/platform data.

 Thanks.


So my argument is mainly based on that the existing code is working (boards
that do not have platform data are relying on the driver to set the
default irq),
and change the default value would need extra work to setup the irq, say as you
suggested through irq_set_irq_type(). I am no expert in irq so it could be true
that your suggested way is indeed better. I am in favor of keeping
this patch is only
because it requires no extra change for existing code that are using it.



 --
 Dmitry
 --
 To unsubscribe from this list: send the line unsubscribe linux-input in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata

2014-05-26 Thread Yufeng Shen
On Sat, May 24, 2014 at 8:41 AM, Nick Dyer nick.d...@itdev.co.uk wrote:
 Yufeng Shen wrote:
 On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote:
 Dmitry Torokhov wrote:
 Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data 
 is
 provided.

 I think if there is no platform data we should use 0 as IRQ falgs and
 assume that IRQ line is properly configured by the board code or via
 device tree.

 Benson/Yufeng - do you still have a requirement to probe without platform
 data or device tree? I'm just merging in some changes to add device tree
 support, and it would simplify things a bit if I can drop this patch.

 It has been working for quite a while for boards/devices that don't
 provide platform data. If we drop the default IRQ flags, sure we can add
 code for each board to configure the IRQ separately, but that's just
 adding extra work. Is there strong reason why we should not do the
 default setting in the driver if it is not already configured in
 platform data?

 OK, I will keep it in my tree for the moment, since you are using it.

 The reason I checked is that in general, I would like to be conservative
 about what is pushed upstream, because it will need maintaining for a long
 time.

 The other reason is that in fact Atmel recommend IRQF_TRIGGER_LOW for these
 chips, not IRQF_TRIGGER_FALLING, so there is a bit of an inconsistency here.

I think I chose IRQF_TRIGGER_FALLING is because when I do a search in
the upstream
code where platform is configured, the irq is always set to be
IRQF_TRIGGER_FALLING
so I was assuming  IRQF_TRIGGER_FALLING is a safe bet.

But you would definitely know better than me on this since the atmel
chips that I have
access to are quite limited.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata

2014-05-23 Thread Yufeng Shen
On Thu, May 22, 2014 at 10:29 AM, Nick Dyer  wrote:
>
> Dmitry Torokhov wrote:
> > On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote:
> >>> From: Yufeng Shen 
> >>> This is the preparation for supporting the code path when there is
> >>> platform data provided and still boot the device into a sane state
> >>> with backup NVRAM config.
> >>>
> >>> Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data 
> >>> is
> >>> provided.
> >
> > I think if there is no platform data we should use 0 as IRQ falgs and
> > assume that IRQ line is properly configured by the board code or via
> > device tree.
>
> Beson/Yufeng - do you still have a requirement to probe without platform
> data or device tree? I'm just merging in some changes to add device tree
> support, and it would simplify things a bit if I can drop this patch.


It has been working for quite a while for boards/devices that don't
provide platform
data. If we drop the default IRQ flags, sure we can add code for each
board to configure
the IRQ separately, but that's just adding extra work. Is there strong
reason why we
should not do the default setting in the driver if it is not already
configured in platform
data ?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 20/51] Input: atmel_mxt_ts - Set default irqflags when there is no pdata

2014-05-23 Thread Yufeng Shen
On Thu, May 22, 2014 at 10:29 AM, Nick Dyer nick.d...@itdev.co.uk wrote:

 Dmitry Torokhov wrote:
  On Thu, Jul 18, 2013 at 07:17:44PM +0200, rydb...@euromail.se wrote:
  From: Yufeng Shen mile...@chromium.org
  This is the preparation for supporting the code path when there is
  platform data provided and still boot the device into a sane state
  with backup NVRAM config.
 
  Make the irqflags default to be IRQF_TRIGGER_FALLING if no platform data 
  is
  provided.
 
  I think if there is no platform data we should use 0 as IRQ falgs and
  assume that IRQ line is properly configured by the board code or via
  device tree.

 Beson/Yufeng - do you still have a requirement to probe without platform
 data or device tree? I'm just merging in some changes to add device tree
 support, and it would simplify things a bit if I can drop this patch.


It has been working for quite a while for boards/devices that don't
provide platform
data. If we drop the default IRQ flags, sure we can add code for each
board to configure
the IRQ separately, but that's just adding extra work. Is there strong
reason why we
should not do the default setting in the driver if it is not already
configured in platform
data ?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Add quirk HID_QUIRK_NO_INIT_REPORTS for RNDPLUS touchscreen

2014-03-21 Thread Yufeng Shen
There is timeout error during initialization:
kernel: [   14.167086] hid-multitouch 0003:2512:5003.0001: usb_submit_urb(ctrl) 
failed: -1
kernel: [   14.167135] hid-multitouch 0003:2512:5003.0001: timeout initializing 
reports
kernel: [   14.167407] input: RNDPLUS Co., LTD   PULSEIR TSR4601 as 
/devices/pci:00/:00:1d.0/usb2/2-1/2-1.3/2-1.3:1.0/input/input14
kernel: [   14.167975] cpufreq_interactive: monitoring input on RNDPLUS Co., 
LTD   PULSEIR TSR4601
kernel: [   14.168266] hid-multitouch 0003:2512:5003.0001: 
input,hiddev0,hidraw1: USB HID v1.10 Mouse [RNDPLUS Co., LTD   PULSEIR TSR4601] 
on usb-:00:1d.0-1.3/input0

Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.

Signed-off-by: Yufeng Shen 
---
 drivers/hid/hid-ids.h   | 5 +
 drivers/hid/usbhid/hid-quirks.c | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f9304cb..772f937 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -442,6 +442,11 @@
 #define USB_DEVICE_ID_IDEACOM_IDC6650  0x6650
 #define USB_DEVICE_ID_IDEACOM_IDC6651  0x6651
 
+#define USB_VENDOR_ID_IDSPULSE  0x2512
+#define USB_DEVICE_ID_IDSPULSE_EVIR10x5003
+#define USB_DEVICE_ID_IDSPULSE_EVIR20x5004
+#define USB_DEVICE_ID_IDSPULSE_EVIR30x5005
+
 #define USB_VENDOR_ID_ILITEK   0x222a
 #define USB_DEVICE_ID_ILITEK_MULTITOUCH0x0001
 
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 0db9a67..e59ad07 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -72,6 +72,9 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_ELO, USB_DEVICE_ID_ELO_TS2700, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, 
HID_QUIRK_NOGET },
+   { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR1, 
HID_QUIRK_NO_INIT_REPORTS },
+   { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR2, 
HID_QUIRK_NO_INIT_REPORTS },
+   { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR3, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, 
HID_QUIRK_NO_INIT_REPORTS },
-- 
1.9.1.423.g4596e3a

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] Add quirk HID_QUIRK_NO_INIT_REPORTS for RNDPLUS touchscreen

2014-03-21 Thread Yufeng Shen
There is timeout error during initialization:
kernel: [   14.167086] hid-multitouch 0003:2512:5003.0001: usb_submit_urb(ctrl) 
failed: -1
kernel: [   14.167135] hid-multitouch 0003:2512:5003.0001: timeout initializing 
reports
kernel: [   14.167407] input: RNDPLUS Co., LTD   PULSEIR TSR4601 as 
/devices/pci:00/:00:1d.0/usb2/2-1/2-1.3/2-1.3:1.0/input/input14
kernel: [   14.167975] cpufreq_interactive: monitoring input on RNDPLUS Co., 
LTD   PULSEIR TSR4601
kernel: [   14.168266] hid-multitouch 0003:2512:5003.0001: 
input,hiddev0,hidraw1: USB HID v1.10 Mouse [RNDPLUS Co., LTD   PULSEIR TSR4601] 
on usb-:00:1d.0-1.3/input0

Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.

Signed-off-by: Yufeng Shen mile...@chromium.org
---
 drivers/hid/hid-ids.h   | 5 +
 drivers/hid/usbhid/hid-quirks.c | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f9304cb..772f937 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -442,6 +442,11 @@
 #define USB_DEVICE_ID_IDEACOM_IDC6650  0x6650
 #define USB_DEVICE_ID_IDEACOM_IDC6651  0x6651
 
+#define USB_VENDOR_ID_IDSPULSE  0x2512
+#define USB_DEVICE_ID_IDSPULSE_EVIR10x5003
+#define USB_DEVICE_ID_IDSPULSE_EVIR20x5004
+#define USB_DEVICE_ID_IDSPULSE_EVIR30x5005
+
 #define USB_VENDOR_ID_ILITEK   0x222a
 #define USB_DEVICE_ID_ILITEK_MULTITOUCH0x0001
 
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 0db9a67..e59ad07 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -72,6 +72,9 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_ELO, USB_DEVICE_ID_ELO_TS2700, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, 
HID_QUIRK_NOGET },
+   { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR1, 
HID_QUIRK_NO_INIT_REPORTS },
+   { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR2, 
HID_QUIRK_NO_INIT_REPORTS },
+   { USB_VENDOR_ID_IDSPULSE, USB_DEVICE_ID_IDSPULSE_EVIR3, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, 
HID_QUIRK_NO_INIT_REPORTS },
-- 
1.9.1.423.g4596e3a

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] HID: usbhid: quirk for CY-TM75 75 inch Touch Overlay

2014-01-27 Thread Yufeng Shen
There is timeout error during initialization:
kernel: [   11.733104] hid-multitouch 0003:1870:0110.0001: usb_submit_urb(ctrl) 
failed: -1
kernel: [   11.734093] hid-multitouch 0003:1870:0110.0001: timeout initializing 
reports

Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.

Signed-off-by: Yufeng Shen 
---
 drivers/hid/hid-ids.h   | 1 +
 drivers/hid/usbhid/hid-quirks.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f9304cb..ece2997 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -636,6 +636,7 @@
 
 #define USB_VENDOR_ID_NEXIO0x1870
 #define USB_DEVICE_ID_NEXIO_MULTITOUCH_420 0x010d
+#define USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750 0x0110
 
 #define USB_VENDOR_ID_NEXTWINDOW   0x1926
 #define USB_DEVICE_ID_NEXTWINDOW_TOUCHSCREEN   0x0003
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 0db9a67..be5270a 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -74,6 +74,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, 
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, 
HID_QUIRK_NO_INIT_REPORTS },
+   { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1, 
HID_QUIRK_NO_INIT_REPORTS },
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] HID: usbhid: quirk for CY-TM75 75 inch Touch Overlay

2014-01-27 Thread Yufeng Shen
There is timeout error during initialization:
kernel: [   11.733104] hid-multitouch 0003:1870:0110.0001: usb_submit_urb(ctrl) 
failed: -1
kernel: [   11.734093] hid-multitouch 0003:1870:0110.0001: timeout initializing 
reports

Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.

Signed-off-by: Yufeng Shen mile...@chromium.org
---
 drivers/hid/hid-ids.h   | 1 +
 drivers/hid/usbhid/hid-quirks.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f9304cb..ece2997 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -636,6 +636,7 @@
 
 #define USB_VENDOR_ID_NEXIO0x1870
 #define USB_DEVICE_ID_NEXIO_MULTITOUCH_420 0x010d
+#define USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750 0x0110
 
 #define USB_VENDOR_ID_NEXTWINDOW   0x1926
 #define USB_DEVICE_ID_NEXTWINDOW_TOUCHSCREEN   0x0003
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 0db9a67..be5270a 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -74,6 +74,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, 
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL, 
HID_QUIRK_NO_INIT_REPORTS },
+   { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN, 
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1, 
HID_QUIRK_NO_INIT_REPORTS },
-- 
1.8.5.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Atmel updates to atmel_mxt_ts touch controller driver - v5 Daniel Kurtz , Henrik Rydberg , Joonyoung Shim , alan.bow...@atmel.co

2013-06-07 Thread Yufeng Shen
Acked-by: Yufeng Shen 

Test the patch series on Chromebook Pixel with kernel 3.8.
Touchscreen & trackpad work, FW/Config update work.

On Thu, Jun 6, 2013 at 3:46 PM, Dmitry Torokhov
 wrote:
> On Thu, Jun 06, 2013 at 03:40:47PM -0400, Yufeng Shen wrote:
>> On Thu, Jun 6, 2013 at 3:18 PM, Dmitry Torokhov
>> wrote:
>>
>> > On Wed, Jun 05, 2013 at 06:36:53PM +0100, Nick Dyer wrote:
>> > > The following patches are an updated series of patches to the
>> > atmel_mxt_ts
>> > > touch driver. They should apply cleanly to input/next.
>> > >
>> > > This is a combined patchset, I've been working to merge my changes with
>> > the
>> > > changes from the Chromium team. It's undergone a lot of testing and
>> > review
>> > > over the last few months and I believe it is now ready to go upstream.
>> > >
>> > > Most of these changes have been maintained and tested out-of-tree in
>> > some form
>> > > for a long time. I apologise for the backlog.
>> > >
>> > > We also provide a set of user-space utilities as open source which are
>> > > available from github and work well with this driver:
>> > >   https://github.com/atmel-maxtouch/obp-utils
>> > >
>> > > You can see the in-between versions at
>> > >   https://github.com/ndyer/linux/
>> > >
>> >
>> > Daniel, Henrik,
>> >
>> > Any comments befroe I start pulling parts of this in?
>> >
>>
>>
>> Can you hold this on for a few days ?  I would like to verify this patch
>> series from chromium side.
>> I probably would do it tomorrow and next monday and send back ack asap.
>
> Sure, holding on is what I do best ;)
>
> Thanks.
>
> --
> Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 44/53] Input: atmel_mxt_ts - Verify Information Block checksum on probe

2013-06-07 Thread Yufeng Shen
On Wed, Jun 5, 2013 at 1:37 PM, Nick Dyer  wrote:
> By reading the information block and the object table into a contiguous region
> of memory, we can verify the checksum at probe time. This means we verify that
> we are indeed talking to a chip that supports object protocol correctly. We
> also detect I2C comms problems much earlier, resulting in easier diagnosis.
>
> Signed-off-by: Nick Dyer 
> Acked-by: Benson Leung 
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c |  181 
> ++
>  1 file changed, 111 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c 
> b/drivers/input/touchscreen/atmel_mxt_ts.c
> index bbfea7a..e0ff017 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -194,7 +194,8 @@ struct mxt_data {
> char phys[64];  /* device physical location */
> struct mxt_platform_data *pdata;
> struct mxt_object *object_table;
> -   struct mxt_info info;
> +   struct mxt_info *info;
> +   void *raw_info_block;
> unsigned int irq;
> unsigned int max_x;
> unsigned int max_y;
> @@ -365,12 +366,16 @@ static int mxt_lookup_bootloader_address(struct 
> mxt_data *data, bool retry)
>  {
> u8 appmode = data->client->addr;
> u8 bootloader;
> +   u8 family_id = 0;
> +
> +   if (data->info)
> +   family_id = data->info->family_id;
>
> switch (appmode) {
> case 0x4a:
> case 0x4b:
> /* Chips after 1664S use different scheme */
> -   if (retry || data->info.family_id >= 0xa2) {
> +   if (retry || family_id >= 0xa2) {
> bootloader = appmode - 0x24;
> break;
> }
> @@ -610,7 +615,7 @@ mxt_get_object(struct mxt_data *data, u8 type)
> struct mxt_object *object;
> int i;
>
> -   for (i = 0; i < data->info.object_num; i++) {
> +   for (i = 0; i < data->info->object_num; i++) {
> object = data->object_table + i;
> if (object->type == type)
> return object;
> @@ -1249,13 +1254,13 @@ static int mxt_check_reg_init(struct mxt_data *data)
> data_pos += offset;
> }
>
> -   if (cfg_info.family_id != data->info.family_id) {
> +   if (cfg_info.family_id != data->info->family_id) {
> dev_err(dev, "Family ID mismatch!\n");
> ret = -EINVAL;
> goto release;
> }
>
> -   if (cfg_info.variant_id != data->info.variant_id) {
> +   if (cfg_info.variant_id != data->info->variant_id) {
> dev_err(dev, "Variant ID mismatch!\n");
> ret = -EINVAL;
> goto release;
> @@ -1302,7 +1307,7 @@ static int mxt_check_reg_init(struct mxt_data *data)
>
> /* Malloc memory to store configuration */
> cfg_start_ofs = MXT_OBJECT_START
> -   + data->info.object_num * sizeof(struct mxt_object)
> +   + data->info->object_num * sizeof(struct mxt_object)
> + MXT_INFO_CHECKSUM_SIZE;
> config_mem_size = data->mem_size - cfg_start_ofs;
> config_mem = kzalloc(config_mem_size, GFP_KERNEL);
> @@ -1510,24 +1515,12 @@ static int mxt_acquire_irq(struct mxt_data *data)
> return 0;
>  }
>
> -static int mxt_get_info(struct mxt_data *data)
> -{
> -   struct i2c_client *client = data->client;
> -   struct mxt_info *info = >info;
> -   int error;
> -
> -   /* Read 7-byte info block starting at address 0 */
> -   error = __mxt_read_reg(client, 0, sizeof(*info), info);
> -   if (error)
> -   return error;
> -
> -   return 0;
> -}
> -
>  static void mxt_free_object_table(struct mxt_data *data)
>  {
> -   kfree(data->object_table);
> +   kfree(data->raw_info_block);
> data->object_table = NULL;
> +   data->info = NULL;
> +   data->raw_info_block = NULL;
> kfree(data->msg_buf);
> data->msg_buf = NULL;
> data->enable_reporting = false;
> @@ -1549,25 +1542,17 @@ static void mxt_free_object_table(struct mxt_data 
> *data)
> data->max_reportid = 0;
>  }
>
> -static int mxt_get_object_table(struct mxt_data *data)
> +static int mxt_parse_object_table(struct mxt_data *data)
>  {
> struct i2c_client *client = data->client;
> -   size_t table_size;
> -   int error;
> int i;
> u8 reportid;
> u16 end_address;
>
> -   table_size = data->info.object_num * sizeof(struct mxt_object);
> -   error = __mxt_read_reg(client, MXT_OBJECT_START, table_size,
> -   data->object_table);
> -   if (error)
> -   return error;
> -
> /* Valid Report IDs start counting from 1 */
> reportid = 1;
> data->mem_size = 0;
> -   for (i = 0; i < data->info.object_num; i++) {
> +

Re: [PATCH 44/53] Input: atmel_mxt_ts - Verify Information Block checksum on probe

2013-06-07 Thread Yufeng Shen
On Wed, Jun 5, 2013 at 1:37 PM, Nick Dyer nick.d...@itdev.co.uk wrote:
 By reading the information block and the object table into a contiguous region
 of memory, we can verify the checksum at probe time. This means we verify that
 we are indeed talking to a chip that supports object protocol correctly. We
 also detect I2C comms problems much earlier, resulting in easier diagnosis.

 Signed-off-by: Nick Dyer nick.d...@itdev.co.uk
 Acked-by: Benson Leung ble...@chromium.org
 ---
  drivers/input/touchscreen/atmel_mxt_ts.c |  181 
 ++
  1 file changed, 111 insertions(+), 70 deletions(-)

 diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c 
 b/drivers/input/touchscreen/atmel_mxt_ts.c
 index bbfea7a..e0ff017 100644
 --- a/drivers/input/touchscreen/atmel_mxt_ts.c
 +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
 @@ -194,7 +194,8 @@ struct mxt_data {
 char phys[64];  /* device physical location */
 struct mxt_platform_data *pdata;
 struct mxt_object *object_table;
 -   struct mxt_info info;
 +   struct mxt_info *info;
 +   void *raw_info_block;
 unsigned int irq;
 unsigned int max_x;
 unsigned int max_y;
 @@ -365,12 +366,16 @@ static int mxt_lookup_bootloader_address(struct 
 mxt_data *data, bool retry)
  {
 u8 appmode = data-client-addr;
 u8 bootloader;
 +   u8 family_id = 0;
 +
 +   if (data-info)
 +   family_id = data-info-family_id;

 switch (appmode) {
 case 0x4a:
 case 0x4b:
 /* Chips after 1664S use different scheme */
 -   if (retry || data-info.family_id = 0xa2) {
 +   if (retry || family_id = 0xa2) {
 bootloader = appmode - 0x24;
 break;
 }
 @@ -610,7 +615,7 @@ mxt_get_object(struct mxt_data *data, u8 type)
 struct mxt_object *object;
 int i;

 -   for (i = 0; i  data-info.object_num; i++) {
 +   for (i = 0; i  data-info-object_num; i++) {
 object = data-object_table + i;
 if (object-type == type)
 return object;
 @@ -1249,13 +1254,13 @@ static int mxt_check_reg_init(struct mxt_data *data)
 data_pos += offset;
 }

 -   if (cfg_info.family_id != data-info.family_id) {
 +   if (cfg_info.family_id != data-info-family_id) {
 dev_err(dev, Family ID mismatch!\n);
 ret = -EINVAL;
 goto release;
 }

 -   if (cfg_info.variant_id != data-info.variant_id) {
 +   if (cfg_info.variant_id != data-info-variant_id) {
 dev_err(dev, Variant ID mismatch!\n);
 ret = -EINVAL;
 goto release;
 @@ -1302,7 +1307,7 @@ static int mxt_check_reg_init(struct mxt_data *data)

 /* Malloc memory to store configuration */
 cfg_start_ofs = MXT_OBJECT_START
 -   + data-info.object_num * sizeof(struct mxt_object)
 +   + data-info-object_num * sizeof(struct mxt_object)
 + MXT_INFO_CHECKSUM_SIZE;
 config_mem_size = data-mem_size - cfg_start_ofs;
 config_mem = kzalloc(config_mem_size, GFP_KERNEL);
 @@ -1510,24 +1515,12 @@ static int mxt_acquire_irq(struct mxt_data *data)
 return 0;
  }

 -static int mxt_get_info(struct mxt_data *data)
 -{
 -   struct i2c_client *client = data-client;
 -   struct mxt_info *info = data-info;
 -   int error;
 -
 -   /* Read 7-byte info block starting at address 0 */
 -   error = __mxt_read_reg(client, 0, sizeof(*info), info);
 -   if (error)
 -   return error;
 -
 -   return 0;
 -}
 -
  static void mxt_free_object_table(struct mxt_data *data)
  {
 -   kfree(data-object_table);
 +   kfree(data-raw_info_block);
 data-object_table = NULL;
 +   data-info = NULL;
 +   data-raw_info_block = NULL;
 kfree(data-msg_buf);
 data-msg_buf = NULL;
 data-enable_reporting = false;
 @@ -1549,25 +1542,17 @@ static void mxt_free_object_table(struct mxt_data 
 *data)
 data-max_reportid = 0;
  }

 -static int mxt_get_object_table(struct mxt_data *data)
 +static int mxt_parse_object_table(struct mxt_data *data)
  {
 struct i2c_client *client = data-client;
 -   size_t table_size;
 -   int error;
 int i;
 u8 reportid;
 u16 end_address;

 -   table_size = data-info.object_num * sizeof(struct mxt_object);
 -   error = __mxt_read_reg(client, MXT_OBJECT_START, table_size,
 -   data-object_table);
 -   if (error)
 -   return error;
 -
 /* Valid Report IDs start counting from 1 */
 reportid = 1;
 data-mem_size = 0;
 -   for (i = 0; i  data-info.object_num; i++) {
 +   for (i = 0; i  data-info-object_num; i++) {
 struct mxt_object *object = data-object_table + 

Re: Atmel updates to atmel_mxt_ts touch controller driver - v5 Daniel Kurtz djku...@chromium.org, Henrik Rydberg rydb...@euromail.se, Joonyoung Shim jy0922.s...@samsung.com, alan.bow...@atmel.co

2013-06-07 Thread Yufeng Shen
Acked-by: Yufeng Shen mile...@chromium.org

Test the patch series on Chromebook Pixel with kernel 3.8.
Touchscreen  trackpad work, FW/Config update work.

On Thu, Jun 6, 2013 at 3:46 PM, Dmitry Torokhov
dmitry.torok...@gmail.com wrote:
 On Thu, Jun 06, 2013 at 03:40:47PM -0400, Yufeng Shen wrote:
 On Thu, Jun 6, 2013 at 3:18 PM, Dmitry Torokhov
 dmitry.torok...@gmail.comwrote:

  On Wed, Jun 05, 2013 at 06:36:53PM +0100, Nick Dyer wrote:
   The following patches are an updated series of patches to the
  atmel_mxt_ts
   touch driver. They should apply cleanly to input/next.
  
   This is a combined patchset, I've been working to merge my changes with
  the
   changes from the Chromium team. It's undergone a lot of testing and
  review
   over the last few months and I believe it is now ready to go upstream.
  
   Most of these changes have been maintained and tested out-of-tree in
  some form
   for a long time. I apologise for the backlog.
  
   We also provide a set of user-space utilities as open source which are
   available from github and work well with this driver:
 https://github.com/atmel-maxtouch/obp-utils
  
   You can see the in-between versions at
 https://github.com/ndyer/linux/
  
 
  Daniel, Henrik,
 
  Any comments befroe I start pulling parts of this in?
 


 Can you hold this on for a few days ?  I would like to verify this patch
 series from chromium side.
 I probably would do it tomorrow and next monday and send back ack asap.

 Sure, holding on is what I do best ;)

 Thanks.

 --
 Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RFC/PATCH] Input: release MT fingers on device reset/disconnect

2012-09-21 Thread Yufeng Shen
We are already releasing all pressed keys on device reset and
disconnect. This patch adds the finger release for multi-touch
device.

Different than the key release logic that only the pressed keys
are released, here all the possible fingers are released even
it is not recorded as touched down in the input_dev's internal
state. The motivation is that while releasing an alreay released
finger won't do harm to downstream, it is helpful in the case
when the downstream is out of sync with input_dev's internal
state, we are bringing them back to in sync by cleaning up both
input_dev and downstream's finger states.

Signed-off-by: Yufeng Shen 
---
 drivers/input/input.c |   34 --
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 768e46b..ebdc450 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -584,6 +584,34 @@ void input_close_device(struct input_handle *handle)
 EXPORT_SYMBOL(input_close_device);
 
 /*
+ * Simulate finger release events for all possible fingers. It has no harm
+ * to release an alreay released finger, but has the benefit that if the
+ * downstream finger states are out of sync with input_dev's finger states,
+ * we are helping to clean them up.
+ * The function must be called with dev->event_lock held.
+ */
+static void input_dev_release_mt(struct input_dev *dev)
+{
+   int i;
+
+   if (!dev->mt)
+   return;
+
+   if (!is_event_supported(EV_ABS, dev->evbit, EV_MAX) ||
+   !is_event_supported(ABS_MT_SLOT, dev->absbit, ABS_MAX) ||
+   !is_event_supported(ABS_MT_TRACKING_ID, dev->absbit, ABS_MAX))
+   return;
+
+   for (i = 0; i < dev->mtsize; i++) {
+   input_mt_set_value(>mt[i], ABS_MT_TRACKING_ID, -1);
+   input_pass_event(dev, EV_ABS, ABS_MT_SLOT, i);
+   input_pass_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+   }
+
+   input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
+}
+
+/*
  * Simulate keyup events for all keys that are marked as pressed.
  * The function must be called with dev->event_lock held.
  */
@@ -627,6 +655,7 @@ static void input_disconnect_device(struct input_dev *dev)
 * reach any handlers.
 */
input_dev_release_keys(dev);
+   input_dev_release_mt(dev);
 
list_for_each_entry(handle, >h_list, d_node)
handle->open = 0;
@@ -1569,7 +1598,7 @@ static void input_dev_toggle(struct input_dev *dev, bool 
activate)
  * @dev: input device whose state needs to be reset
  *
  * This function tries to reset the state of an opened input device and
- * bring internal state and state if the hardware in sync with each other.
+ * bring internal state and state of the hardware in sync with each other.
  * We mark all keys as released, restore LED state, repeat rate, etc.
  */
 void input_reset_device(struct input_dev *dev)
@@ -1581,10 +1610,11 @@ void input_reset_device(struct input_dev *dev)
 
/*
 * Keys that have been pressed at suspend time are unlikely
-* to be still pressed when we resume.
+* to be still pressed when we resume. Same logic for MT.
 */
spin_lock_irq(>event_lock);
input_dev_release_keys(dev);
+   input_dev_release_mt(dev);
spin_unlock_irq(>event_lock);
}
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[RFC/PATCH] Input: release MT fingers on device reset/disconnect

2012-09-21 Thread Yufeng Shen
We are already releasing all pressed keys on device reset and
disconnect. This patch adds the finger release for multi-touch
device.

Different than the key release logic that only the pressed keys
are released, here all the possible fingers are released even
it is not recorded as touched down in the input_dev's internal
state. The motivation is that while releasing an alreay released
finger won't do harm to downstream, it is helpful in the case
when the downstream is out of sync with input_dev's internal
state, we are bringing them back to in sync by cleaning up both
input_dev and downstream's finger states.

Signed-off-by: Yufeng Shen mile...@chromium.org
---
 drivers/input/input.c |   34 --
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/input/input.c b/drivers/input/input.c
index 768e46b..ebdc450 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -584,6 +584,34 @@ void input_close_device(struct input_handle *handle)
 EXPORT_SYMBOL(input_close_device);
 
 /*
+ * Simulate finger release events for all possible fingers. It has no harm
+ * to release an alreay released finger, but has the benefit that if the
+ * downstream finger states are out of sync with input_dev's finger states,
+ * we are helping to clean them up.
+ * The function must be called with dev-event_lock held.
+ */
+static void input_dev_release_mt(struct input_dev *dev)
+{
+   int i;
+
+   if (!dev-mt)
+   return;
+
+   if (!is_event_supported(EV_ABS, dev-evbit, EV_MAX) ||
+   !is_event_supported(ABS_MT_SLOT, dev-absbit, ABS_MAX) ||
+   !is_event_supported(ABS_MT_TRACKING_ID, dev-absbit, ABS_MAX))
+   return;
+
+   for (i = 0; i  dev-mtsize; i++) {
+   input_mt_set_value(dev-mt[i], ABS_MT_TRACKING_ID, -1);
+   input_pass_event(dev, EV_ABS, ABS_MT_SLOT, i);
+   input_pass_event(dev, EV_ABS, ABS_MT_TRACKING_ID, -1);
+   }
+
+   input_pass_event(dev, EV_SYN, SYN_REPORT, 1);
+}
+
+/*
  * Simulate keyup events for all keys that are marked as pressed.
  * The function must be called with dev-event_lock held.
  */
@@ -627,6 +655,7 @@ static void input_disconnect_device(struct input_dev *dev)
 * reach any handlers.
 */
input_dev_release_keys(dev);
+   input_dev_release_mt(dev);
 
list_for_each_entry(handle, dev-h_list, d_node)
handle-open = 0;
@@ -1569,7 +1598,7 @@ static void input_dev_toggle(struct input_dev *dev, bool 
activate)
  * @dev: input device whose state needs to be reset
  *
  * This function tries to reset the state of an opened input device and
- * bring internal state and state if the hardware in sync with each other.
+ * bring internal state and state of the hardware in sync with each other.
  * We mark all keys as released, restore LED state, repeat rate, etc.
  */
 void input_reset_device(struct input_dev *dev)
@@ -1581,10 +1610,11 @@ void input_reset_device(struct input_dev *dev)
 
/*
 * Keys that have been pressed at suspend time are unlikely
-* to be still pressed when we resume.
+* to be still pressed when we resume. Same logic for MT.
 */
spin_lock_irq(dev-event_lock);
input_dev_release_keys(dev);
+   input_dev_release_mt(dev);
spin_unlock_irq(dev-event_lock);
}
 
-- 
1.7.7.3

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/