Re: [PATCH] input: edt-ft5x06 - Touchscreen driver for FT5x06 based EDT displays

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

 



On Thu, Jun 21, 2012 at 04:53:54PM +0200, Simon Budig wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 06/21/2012 12:04 PM, Dmitry Torokhov wrote:
> >> Ok, the bad news is, that it doesn't work with your changes.
> >> There was one oops I was able to resolve (i2c_set_clientdata must
> >> happen before sysfs_create_group) but the touch also failed to
> >> deliver input events and mode switching doesn't work.
> > 
> > Hmm, OK, let me ponder this one a bit as well.
> 
> Ok, found some of the problems. For starters I failed to realize that
> you moved the irq initialization to the board file into the i2c-client
> structure (which I did not initialize in my board file). This of
> course explains why the touch seemed unresponsive...
> 
> The problem with mode switching was, that tsdata->factory_mode was
> shuffeled around *after* some of the calls to _register_write/read.
> This function however changes its way of operation depending on the
> mode, you need to send different read/write commands depending on the
> mode of the chip. So it is essential to change factory_mode before
> invoking these functions.

I see.

> 
> Hmm, maybe this should be an explicit function parameter.
> 
> Then you implemented edt_ft5x06_i2c_attr_is_visible, but I fear this
> is not working as you expect it to work. It has an effect on startup
> time of the driver, but not when later switching the mode. This had
> the effect that raw_data did not appear when switching to factory mode.

Right, what was missing is a call to sysfs_update_group() after changing
factory mode setting.

> 
> I am unsure regarding your changes to the handling of the platform
> data. Is it guaranteed that the platform data sticks around for the
> life time of the driver?

It has to, otherwise compiling driver as a module and/or
unbinding/rebinding would not work.

> Some of the data in the board file is marked
> __initdata, which to my understanding means, that the kernel might
> discard it at some point. That is why I copied the values into my
> private structure. Is it safe to change that?

You'll need to remove this __initdata markings.

> 
> For the pin handling: you release the reset-pin after having done the
> reset. Is this wise? This enables the user to use e.g. the
> gpio-filesystem to reset the touch screen, potentially confusing the
> driver and messing up the system.

Hm, I do not have a strong opinion here.

Thanks.

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