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