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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 27, 2012 at 10:26 PM, Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> On Friday, January 27, 2012 10:01:22 PM Javier Martinez Canillas wrote:
>> On Fri, Jan 27, 2012 at 7:18 PM, Dmitry Torokhov
>>
>> <dmitry.torokhov@xxxxxxxxx> wrote:
>> > On Fri, Jan 27, 2012 at 04:57:12PM +0100, Javier Martinez Canillas wrote:
>> >> On Fri, Jan 27, 2012 at 9:18 AM, Dmitry Torokhov
>> >>
>> >> <dmitry.torokhov@xxxxxxxxx> wrote:
>> >> > On Thu, Jan 26, 2012 at 01:12:50AM +0100, Javier Martinez Canillas
> wrote:
>> >> >> On Tue, Jan 24, 2012 at 8:54 AM, Dmitry Torokhov
>> >> >>
>> >> >> <dmitry.torokhov@xxxxxxxxx> wrote:
>> >> >> > On Tue, Jan 24, 2012 at 08:26:39AM +0100, Javier Martinez Canillas
> wrote:
>> >> >> >> On Fri, Jan 20, 2012 at 1:57 AM, Javier Martinez Canillas
>> >> >> >>
>> >> >> >> <javier@xxxxxxxxxxxx> wrote:
>> >> >> >> > 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
>> >> >> >> > <javier@xxxxxxxxxxxx>
>> >> >> >> > ---
>> >> >> >> >
>> >> >> >> > 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!
>> >>
>> >> > 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.
>> >>
>> >> > 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.
>> >>
>> >> > 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.

>> 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.

>>
>> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux