Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/