Linux Input Archive on lore.kernel.org
 help / color / Atom feed
From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
To: linux-media@vger.kernel.org
Cc: linux-input@vger.kernel.org,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Nick Dyer <nick@shmanahar.org>,
	Lucas Stach <l.stach@pengutronix.de>,
	Christopher Heiny <cheiny@synaptics.com>,
	Vandana BN <bnvandana@gmail.com>
Subject: [PATCHv2 5/5] input/rmi4/rmi_smbus.c: don't increment rmiaddr
Date: Sat, 23 Nov 2019 17:27:41 +0100
Message-ID: <8dd22e21-4933-8e9c-a696-d281872c8de7@xs4all.nl> (raw)
In-Reply-To: <20191119105118.54285-6-hverkuil-cisco@xs4all.nl>

This increment of rmi_smbus in rmi_smb_read/write_block() causes
garbage to be read/written.

The first read of SMB_MAX_COUNT bytes is fine, but after that
it is nonsense. Trial-and-error showed that by dropping the
increment of rmiaddr everything is fine and the F54 function
properly works.

I tried a hack with rmi_smb_write_block() as well (writing to the
same F54 touchpad data area, then reading it back), and that
suggests that there too the rmiaddr increment has to be dropped.
It makes sense that if it has to be dropped for read, then it has
to be dropped for write as well.

It looks like the initial work with F54 was done using i2c, not smbus,
and it seems nobody ever tested F54 with smbus. The other functions
all read/write less than SMB_MAX_COUNT as far as I can tell, so this
issue was never noticed with non-F54 functions.

With this change I can read out the touchpad data correctly on my
Lenovo X1 Carbon 6th Gen laptop.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/input/rmi4/rmi_smbus.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 2407ea43de59..b313c579914f 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -163,7 +163,6 @@ static int rmi_smb_write_block(struct rmi_transport_dev *xport, u16 rmiaddr,
 		/* prepare to write next block of bytes */
 		cur_len -= SMB_MAX_COUNT;
 		databuff += SMB_MAX_COUNT;
-		rmiaddr += SMB_MAX_COUNT;
 	}
 exit:
 	mutex_unlock(&rmi_smb->page_mutex);
@@ -215,7 +214,6 @@ static int rmi_smb_read_block(struct rmi_transport_dev *xport, u16 rmiaddr,
 		/* prepare to read next block of bytes */
 		cur_len -= SMB_MAX_COUNT;
 		databuff += SMB_MAX_COUNT;
-		rmiaddr += SMB_MAX_COUNT;
 	}

 	retval = 0;
-- 
2.24.0



  parent reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-19 10:51 [PATCH 0/5] input: rmi4/synaptics fixes Hans Verkuil
2019-11-19 10:51 ` [PATCH 1/5] input/mouse/synaptics: add LEN0091 support Hans Verkuil
2019-11-23  0:17   ` Dmitry Torokhov
2019-11-19 10:51 ` [PATCH 2/5] input/rmi4/rmi_f54: fix various V4L2 compliance problems Hans Verkuil
2019-11-19 11:42   ` Lucas Stach
2019-11-23  0:19     ` Dmitry Torokhov
2019-11-19 10:51 ` [PATCH 3/5] pixfmt-tch-td16/tu16.rst: document that this is little endian Hans Verkuil
2019-11-23 16:12   ` Hans Verkuil
2019-11-25 18:51     ` Dmitry Torokhov
2019-11-19 10:51 ` [PATCH 4/5] input/rmi4/rmi_driver: check if irq_find_mapping returns 0 Hans Verkuil
2019-11-19 11:38   ` Lucas Stach
2019-11-19 11:43     ` Hans Verkuil
2019-11-23 13:53       ` Hans Verkuil
2019-11-19 10:51 ` [PATCH 5/5] input/rmi4/rmi_smbus.c: don't increment rmiaddr in rmi_smb_read_block() Hans Verkuil
2019-11-19 11:48   ` Lucas Stach
2019-11-19 12:19     ` Hans Verkuil
2019-11-23 16:27   ` Hans Verkuil [this message]
2019-12-02 18:09     ` [PATCHv2 5/5] input/rmi4/rmi_smbus.c: don't increment rmiaddr Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8dd22e21-4933-8e9c-a696-d281872c8de7@xs4all.nl \
    --to=hverkuil-cisco@xs4all.nl \
    --cc=bnvandana@gmail.com \
    --cc=cheiny@synaptics.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=l.stach@pengutronix.de \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=nick@shmanahar.org \
    --cc=p.zabel@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux Input Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-input/0 linux-input/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-input linux-input/ https://lore.kernel.org/linux-input \
		linux-input@vger.kernel.org
	public-inbox-index linux-input

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-input


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git