| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <52D0824F.4040908@synaptics.com>
Date: Fri, 10 Jan 2014 15:29:19 -0800
From: Christopher Heiny <cheiny@...aptics.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
CC: Andrew Duggan <aduggan@...aptics.com>,
Vincent Huang <vincent.huang@...synaptics.com>,
Vivian Ly <vly@...aptics.com>,
Daniel Rosenberg <daniel.rosenberg@...aptics.com>,
Linus Walleij <linus.walleij@...ricsson.com>,
Benjamin Tissoires <benjamin.tissoires@...hat.com>,
Linux Input <linux-input@...r.kernel.org>,
Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 4/4] Input: synaptics-rmi4 - switch to using i2c_transfer()
On 01/09/2014 11:44 PM, Dmitry Torokhov wrote:
> Instead of using 2 separate transactions when reading from the device let's
> use i2c_transfer. Because we now have single point of failure I had to
> change how we collect statistics. I elected to drop control data from the
> stats and only track number of bytes read/written for the device data.
>
> Also, since we are not prepared to deal with short reads and writes change
> read_block_data and write_block_data to indicate error if we detect short
> transfers.
We tried this change once before a couple of years ago, but the
conversion was unsuccessful on some older platforms. I've tested it on
some more current platforms, though, and it works there. The old
platforms are running 2.6.xx series kernels, and don't look likely ever
to be updated, So....
Acked-by: Christopher Heiny <cheiny@...aptics.com>
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
> drivers/input/rmi4/rmi_i2c.c | 71 ++++++++++++++++++++++++--------------------
> 1 file changed, 39 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index c176218..51f5bc8 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -57,22 +57,17 @@ struct rmi_i2c_xport {
> */
> static int rmi_set_page(struct rmi_i2c_xport *rmi_i2c, u8 page)
> {
> - struct rmi_transport_dev *xport = &rmi_i2c->xport;
> struct i2c_client *client = rmi_i2c->client;
> u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
> int retval;
>
> - dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
> - txbuf[0], txbuf[1]);
> - xport->stats.tx_count++;
> - xport->stats.tx_bytes += sizeof(txbuf);
> retval = i2c_master_send(client, txbuf, sizeof(txbuf));
> if (retval != sizeof(txbuf)) {
> - xport->stats.tx_errs++;
> dev_err(&client->dev,
> "%s: set page failed: %d.", __func__, retval);
> return (retval < 0) ? retval : -EIO;
> }
> +
> rmi_i2c->page = page;
> return 0;
> }
> @@ -107,22 +102,27 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
>
> if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
> retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
> - if (retval < 0)
> + if (retval)
> goto exit;
> }
>
> + retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
> + if (retval == tx_size)
> + retval = 0;
> + else if (retval >= 0)
> + retval = -EIO;
> +
> +exit:
> dev_dbg(&client->dev,
> - "writes %zd bytes at %#06x: %*ph\n", len, addr, (int)len, buf);
> + "write %zd bytes at %#06x: %d (%*ph)\n",
> + len, addr, retval, (int)len, buf);
>
> xport->stats.tx_count++;
> - xport->stats.tx_bytes += tx_size;
> - retval = i2c_master_send(client, rmi_i2c->tx_buf, tx_size);
> - if (retval < 0)
> + if (retval)
> xport->stats.tx_errs++;
> else
> - retval--; /* don't count the address byte */
> + xport->stats.tx_bytes += len;
>
> -exit:
> mutex_unlock(&rmi_i2c->page_mutex);
> return retval;
> }
> @@ -133,40 +133,47 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
> struct rmi_i2c_xport *rmi_i2c =
> container_of(xport, struct rmi_i2c_xport, xport);
> struct i2c_client *client = rmi_i2c->client;
> - u8 txbuf[1] = {addr & 0xff};
> + u8 addr_offset = addr & 0xff;
> int retval;
> + struct i2c_msg msgs[] = {
> + {
> + .addr = client->addr,
> + .len = sizeof(addr_offset),
> + .buf = &addr_offset,
> + },
> + {
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .len = len,
> + .buf = buf,
> + },
> + };
>
> mutex_lock(&rmi_i2c->page_mutex);
>
> if (RMI_I2C_PAGE(addr) != rmi_i2c->page) {
> retval = rmi_set_page(rmi_i2c, RMI_I2C_PAGE(addr));
> - if (retval < 0)
> + if (retval)
> goto exit;
> }
>
> - dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
> + retval = i2c_transfer(client->adapter, msgs, sizeof(msgs));
> + if (retval == sizeof(msgs))
> + retval = 0; /* success */
> + else if (retval >= 0)
> + retval = -EIO;
>
> - xport->stats.tx_count++;
> - xport->stats.tx_bytes += sizeof(txbuf);
> - retval = i2c_master_send(client, txbuf, sizeof(txbuf));
> - if (retval != sizeof(txbuf)) {
> - xport->stats.tx_errs++;
> - retval = (retval < 0) ? retval : -EIO;
> - goto exit;
> - }
> +exit:
> + dev_dbg(&client->dev,
> + "read %zd bytes at %#06x: %d (%*ph)\n",
> + len, addr, retval, (int)len, buf);
>
> xport->stats.rx_count++;
> - xport->stats.rx_bytes += len;
> -
> - retval = i2c_master_recv(client, buf, len);
> - if (retval < 0)
> + if (retval)
> xport->stats.rx_errs++;
> else
> - dev_dbg(&client->dev,
> - "read %zd bytes at %#06x: %*ph\n",
> - len, addr, (int)len, buf);
> + xport->stats.rx_bytes += len;
>
> -exit:
> mutex_unlock(&rmi_i2c->page_mutex);
> return retval;
> }
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists