Re: [PATCH v10 1/3] Input: cyttsp - Cypress TTSP capacitive multi-touch screen support

On Fri, Jan 27, 2012 at 10:26 PM, Dmitry Torokhov
[off-list ref] wrote:

On Friday, January 27, 2012 10:01:22 PM Javier Martinez Canillas wrote:

quoted

On Fri, Jan 27, 2012 at 7:18 PM, Dmitry Torokhov

[off-list ref] wrote:

quoted

On Fri, Jan 27, 2012 at 04:57:12PM +0100, Javier Martinez Canillas wrote:

quoted

On Fri, Jan 27, 2012 at 9:18 AM, Dmitry Torokhov

[off-list ref] wrote:

quoted

On Thu, Jan 26, 2012 at 01:12:50AM +0100, Javier Martinez Canillas

wrote:

quoted

quoted

quoted

quoted

quoted

On Tue, Jan 24, 2012 at 8:54 AM, Dmitry Torokhov

[off-list ref] wrote:

quoted

On Tue, Jan 24, 2012 at 08:26:39AM +0100, Javier Martinez Canillas

wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

On Fri, Jan 20, 2012 at 1:57 AM, Javier Martinez Canillas

[off-list ref] wrote:

quoted

Cypress TrueTouch(tm) Standard Product controllers are
found in
a wide range of embedded devices. This driver add
support for a
variety of TTSP controllers.

Since the hardware is capable of tracking identifiable
contacts, multi-touch protocol type B (stateful) is
used to report contact information.

The driver is composed of a core driver that process
the data sent by the contacts and a set of bus
specific interface modules. This patch adds the base
core TTSP driver.

Signed-off-by: Javier Martinez Canillas
[off-list ref]
---

Changes for v10: Fix issues called out by Dmitry
Torokhov
       - Remove use_sleep and put device to sleep
unconditionally on suspend - Cleanup
cyttsp_power_on() and remove cyttsp_bl_app_valid()
function

 drivers/input/touchscreen/Kconfig       |   31 ++
 drivers/input/touchscreen/Makefile      |    3 +
 drivers/input/touchscreen/cyttsp_core.c |  682
+++++++++++++++++++++++++++++++
drivers/input/touchscreen/cyttsp_core.h |  141
+++++++ include/linux/input/cyttsp.h            |
68 +++
 5 files changed, 925 insertions(+), 0 deletions(-)

Hello Dmitry,

Any comments on this version?

Looking at it... If you do not hear from me by Wednesday
please ping me again.

Thanks.

--
Dmitry

ping :)

Is it still Wednesday by any chance? ;)

Hi Dmitry,

Thanks for the review and the cleanup patch!

quoted

Anyway, the driver looks pretty good, still below are some changes
that I'd like to get in as well:

- do not return EAGAIN when operation times out, EIO I believe
suits
 better.
- introduce ttsp_send_command() to replace host of custom
functions.
- reduce numver of states to 3 - IDLE, ACTIVE and BL mode.
 Suspended/full power is already covered by "suspended" attribute.
- streamline some functions.

Seems that your patch handlers all these issues.

quoted

Please see the FIXME comment in cyttsp_enable() - is there a
generic way to wake up the device, similarly to the way we put it
to sleep?>>

I guess that CY_OPERATE_MODE works but I have to try it since I don't
have the hw data-sheet.

quoted

Please tell me if the device still works with this patch.

Thanks!

--
Dmitry

Strange enough, it doesn't apply cleanly on my tree...

with patch -p1 < your_patch

It should since only the files for this driver are modified and I've
only applied the v10 patches I sent to you.
Anyway I will manually do the changes, try it and resend to you.

Hmm, not sure why it gives you trouble, I tried not to change anything
when applying your v10 patches except for folding them all together.

I am attaching the version of the driver I've used as a base - maybe it
will save you some time instead of applying the changes manually.

Also, could you please send changes needed to make my patch work as
incremental patch so that I can apply on top of mime locally and fold
all 3 together? It will be easier to see what exactly changed.

Thanks!

--
Dmitry

Hi Dmitry,

Using that merged patch as a base your patch applies cleanly (not sure
what happened with my branch)

Sadly the driver is not working with your modifications.

I was afraid it could happen - hazards of chaging the code without the way of
testing it...  I'll try to double check and see what I might have done wrong.
What device did you test - I2C or SPI? Or do they both fail?

Hi Dmitry,

Just sent an email with the fix of the two bugs I found in your
clean-up patch. With those two fixes the device is working again.

Also I remove the wakeup() platform function. That function told me
Kevin that was in the original Android driver because the device can
be wakeup by flipping the direction of the GPIO ping connected to the
touchscreen IRQ line. But also the device wakes up when there is an
I2C or SPI slave memory address match and by reading one of the device
registers. So trying to read will fail if the device is in sleep mode
but it will wake up the device so the first retry will succeed. I
documented that hardware behavior in the driver to make it clear.

I have only an device connected through I2C device but Kevin has been
testing with a device using SPI.

quoted

I'll make it
work changing as less as possible what was in your patch. Only to make
the device work again and will send you as an incremental patch as you
suggested.

Thank you for your patience with me.

Thank you for your patience and your reviews.

quoted

Just to be clear, do you want me to send as an incremental patch of my
series (v10) or as an incremental patch of your last patch?

Incremental to mine please - so in your working branch you should have the
original change + my patch as a separate commit + your commit fixing my mess.

--
Dmitry

Perfect, the patch I sent is an incremental one on top of my three
last patches (v10) + your clean-up patch.

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html