Hi Dmitry!
First: THANKS for fixing my driver.
Second: sorry for slow answer :(
On Thu, Apr 23, 2020 at 4:05 AM Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx> wrote:
> > +config TOUCHSCREEN_CY8CTMA140
> > + tristate "cy8ctma140 touchscreen"
> > + depends on I2C
> > + depends on GPIOLIB || COMPILE_TEST
>
> Why do we need gpiolib here?
Not really. Dropped it.
> > + ret = __i2c_transfer(ts->client->adapter, msg, ARRAY_SIZE(msg));
>
> Why do we use "unlocked" __i2c_transfer()? Can't we use normal
> i2c_transfer()? For __i2c_transfer() someone has to lock the segment...
Good point. Your patch works fine here.
> Hmm, I think if we do use loop the code is actually _much_ simpler,
> please see the patch below.
Indeed. Works like a charm after some fixes.
> > + addr[0] = CY8CTMA140_GET_FW_INFO;
> > + ret = i2c_master_send(ts->client, addr, 1);
> > + if (ret < 0) {
> > + dev_err(ts->dev, "error sending FW info message\n");
> > + return ret;
> > + }
> > + ret = i2c_master_recv(ts->client, buf, 5);
> > + if (ret < 0) {
> > + dev_err(ts->dev, "error recieveing FW info message\n");
> > + return ret;
> > + }
>
> Does it have to be 2 separate transactions? Can we use write/read
> i2c_transfer() here?
I think it is necessary for the FW info test, this is not a standard
I2C transaction, notice that addr[0] does not contain the I2C
address of the device.
I tried to do it some other way but this was the
only way I could get it to work and detect the device properly.
> Can you please try the patch below on top of yours?
Folded it in and will resend with my changes as v6.
> @@ -187,26 +144,28 @@ static int cy8ctma140_init(struct cy8ctma140 *ts)
> {
> u8 addr[1];
> u8 buf[5];
> + int error;
> int ret;
>
> addr[0] = CY8CTMA140_GET_FW_INFO;
> - ret = i2c_master_send(ts->client, addr, 1);
> - if (ret < 0) {
> + error = i2c_master_send(ts->client, addr, 1);
> + if (error) {
This didn't work as i2c_master_send() returns the number of
bytes sent. I reverted this function back to using ret.
After that everything started to work!
Yours,
Linus Walleij
