Hi Dmitry, On Thu, Apr 27, 2017 at 04:56:45PM -0700, Dmitry Torokhov wrote: > On Fri, Apr 28, 2017 at 08:41:56AM +0900, Andi Shyti wrote: > > On Wed, Apr 26, 2017 at 05:39:18PM -0700, Dmitry Torokhov wrote: > > > On Mon, Mar 27, 2017 at 10:07:43PM +0900, Andi Shyti wrote: > > > > +static irqreturn_t stmfts_irq_handler(int irq, void *dev) > > > > +{ > > > > + struct stmfts_data *sdata = dev; > > > > + int ret; > > > > + > > > > + mutex_lock(&sdata->mutex); > > > > + ret = i2c_smbus_read_i2c_block_data(sdata->client, > > > > + STMFTS_READ_ONE_EVENT, > > > > + STMFTS_EVENT_SIZE, sdata->data); > > > > + > > > > + if (ret < 0 || ret != STMFTS_EVENT_SIZE) > > > > + goto exit; > > > > > > Why do we split read into 2 chunks? Can we issue STMFTS_READ_ALL_EVENT > > > right away instead of reading first event, analyzing it, and then (maybe) > > > fetching the rest? > > > > The reason is that I don't need to read all the events at once > > anytime, for example debug events or confirmation events normally > > occur with a single event in the fifo. In this case I would read > > only 32bytes instead of 256bytes. > > > > Unfortunately there are no other ways to know how many events are > > in the queue beforehand. > > > > There are some "magic" commands to figure that out, but this is > > specific to the Samsung's version of the stmfts and I don't want > > to push it to everyone else. > > > > The difference between this version of the driver and the > > previous one is that in this one if I stress-use of the > > touchscreen, the throughput is optimised (e.g. if I use more > > fingers). > > Before I was reading single events at time, establishing for each > > read an i2c "handshake", this was de-synchronizing the protocol. > > > > > Also, why do we use smbus protocol for the first event and i2c for the > > > rest? > > > > Standing to the datasheet, the device is smbus compatible and it > > should use smbus all the time. The problem is that here the > > protocol is broken in case I want to read out the full FIFO, > > which has a total of 256bytes and I have to force the read by > > using the function "stmfts_read_i2c_block_data()". > > > > Personally I don't like these kind of i2c reads, because they > > duplicate code, the SMBUS does that already, this is why in the > > previous version I was reading the events one by one. > > > > Do you think it is better to make a single read of all the fifo? > > It depends on what the common case is. It looks like for touch data you > always do 2 i2c transactions per interrupt. I wonder if doing it once > and paying the price of overhead for debug a nd confirmation events is > not worth it. makes sense, indeed, because I do it all the time when for touch events (which are the most) not only when the FIFO is not full. I will do it this way, then... Thanks! Andi -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html