* re: Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices
@ 2013-07-02 21:47 Dan Carpenter
2013-07-03 13:24 ` Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2013-07-02 21:47 UTC (permalink / raw)
To: fery; +Cc: linux-input
Hello Ferruh Yigit,
The patch 67bf12ca50d5: "Input: cyttsp4 - SPI driver for Cypress
TMA4XX touchscreen devices" from Jun 30, 2013, leads to the following
Smatch warning:
"drivers/input/touchscreen/cyttsp4_spi.c:66 cyttsp_spi_xfer()
warn: impossible condition '(reg > 255) => (0-255 > 255)'"
drivers/input/touchscreen/cyttsp4_spi.c
66 if (reg > 255)
^^^^^^^^^
"reg" is unsigned char so 255 is the max it can be.
67 wr_buf[0] = op + CY_SPI_A8_BIT;
68 else
69 wr_buf[0] = op;
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices
2013-07-02 21:47 Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices Dan Carpenter
@ 2013-07-03 13:24 ` Ferruh Yigit
2013-07-03 13:52 ` Dan Carpenter
0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2013-07-03 13:24 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-input
On 07/03/2013 12:47 AM, Dan Carpenter wrote:
> Hello Ferruh Yigit,
>
> The patch 67bf12ca50d5: "Input: cyttsp4 - SPI driver for Cypress
> TMA4XX touchscreen devices" from Jun 30, 2013, leads to the following
> Smatch warning:
> "drivers/input/touchscreen/cyttsp4_spi.c:66 cyttsp_spi_xfer()
> warn: impossible condition '(reg > 255) => (0-255 > 255)'"
>
> drivers/input/touchscreen/cyttsp4_spi.c
> 66 if (reg > 255)
> ^^^^^^^^^
> "reg" is unsigned char so 255 is the max it can be.
> 67 wr_buf[0] = op + CY_SPI_A8_BIT;
> 68 else
> 69 wr_buf[0] = op;
>
> regards,
> dan carpenter
>
Hi Dan,
Shortly, check is correct and address should be 16 bit, parameter size
is wrong.
In TSG4, register map is 512bytes long and to access all of it, one bit
from address byte is used (which bit to use differs for I2C and SPI),
the code you marked is doing that check.
Since common code used TSG3 and TSG4 for I2C, this parameter wrongly
used as u8. TSG3 does not access beyond 255 bytes but TSG4 may.
I have a patch for this, but should I create it against previous patch I
have sent or against latest next tree?
--
Thanks,
ferruh
This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices
2013-07-03 13:24 ` Ferruh Yigit
@ 2013-07-03 13:52 ` Dan Carpenter
2013-07-03 20:36 ` [PATCH] Input: cyttsp4 - use 16bit address for I2C/SPI communication Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2013-07-03 13:52 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: linux-input
On Wed, Jul 03, 2013 at 04:24:18PM +0300, Ferruh Yigit wrote:
> I have a patch for this, but should I create it against previous patch I
> have sent or against latest next tree?
>
Latest linux-next.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] Input: cyttsp4 - use 16bit address for I2C/SPI communication
2013-07-03 13:52 ` Dan Carpenter
@ 2013-07-03 20:36 ` Ferruh Yigit
2013-07-04 12:25 ` Javier Martinez Canillas
2013-07-07 5:08 ` Dmitry Torokhov
0 siblings, 2 replies; 8+ messages in thread
From: Ferruh Yigit @ 2013-07-03 20:36 UTC (permalink / raw)
To: Dan Carpenter, Dmitry Torokhov
Cc: ttdrivers, Javier Martinez Canillas, linux-input, Ferruh Yigit
In TSG4, register map is 512bytes long and to access all of it,
one bit from address byte is used (which bit to use differs for
I2C and SPI);
Since common code used for TSG3 and TSG4 for I2C, this parameter
wrongly used as u8. TSG3 does not access beyond 255 bytes
but TSG4 may.
Signed-off-by: Ferruh Yigit <fery@cypress.com>
Tested-on: TMA3XX DVB && TMA4XX DVB
---
drivers/input/touchscreen/cyttsp4_core.h | 12 +++++-----
drivers/input/touchscreen/cyttsp4_spi.c | 20 ++++++++---------
drivers/input/touchscreen/cyttsp_core.h | 8 +++----
drivers/input/touchscreen/cyttsp_i2c_common.c | 30 ++++++++++++++++++-------
drivers/input/touchscreen/cyttsp_spi.c | 6 ++---
5 files changed, 44 insertions(+), 32 deletions(-)
diff --git a/drivers/input/touchscreen/cyttsp4_core.h b/drivers/input/touchscreen/cyttsp4_core.h
index 86a2543..8e0d4d4 100644
--- a/drivers/input/touchscreen/cyttsp4_core.h
+++ b/drivers/input/touchscreen/cyttsp4_core.h
@@ -369,9 +369,9 @@ struct cyttsp4 {
struct cyttsp4_bus_ops {
u16 bustype;
- int (*write)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
+ int (*write)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
const void *values);
- int (*read)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
+ int (*read)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
void *values);
};
@@ -448,13 +448,13 @@ enum cyttsp4_event_id {
/* y-axis, 0:origin is on top side of panel, 1: bottom */
#define CY_PCFG_ORIGIN_Y_MASK 0x80
-static inline int cyttsp4_adap_read(struct cyttsp4 *ts, u8 addr, int size,
+static inline int cyttsp4_adap_read(struct cyttsp4 *ts, u16 addr, int size,
void *buf)
{
return ts->bus_ops->read(ts->dev, ts->xfer_buf, addr, size, buf);
}
-static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u8 addr, int size,
+static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u16 addr, int size,
const void *buf)
{
return ts->bus_ops->write(ts->dev, ts->xfer_buf, addr, size, buf);
@@ -463,9 +463,9 @@ static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u8 addr, int size,
extern struct cyttsp4 *cyttsp4_probe(const struct cyttsp4_bus_ops *ops,
struct device *dev, u16 irq, size_t xfer_buf_size);
extern int cyttsp4_remove(struct cyttsp4 *ts);
-int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
+int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
u8 length, const void *values);
-int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
+int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
u8 length, void *values);
extern const struct dev_pm_ops cyttsp4_pm_ops;
diff --git a/drivers/input/touchscreen/cyttsp4_spi.c b/drivers/input/touchscreen/cyttsp4_spi.c
index f8f891b..a71e114 100644
--- a/drivers/input/touchscreen/cyttsp4_spi.c
+++ b/drivers/input/touchscreen/cyttsp4_spi.c
@@ -44,7 +44,7 @@
#define CY_SPI_DATA_BUF_SIZE (CY_SPI_CMD_BYTES + CY_SPI_DATA_SIZE)
static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
- u8 op, u8 reg, u8 *buf, int length)
+ u8 op, u16 reg, u8 *buf, int length)
{
struct spi_device *spi = to_spi_device(dev);
struct spi_message msg;
@@ -63,14 +63,12 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
memset(wr_buf, 0, CY_SPI_DATA_BUF_SIZE);
memset(rd_buf, 0, CY_SPI_CMD_BYTES);
- if (reg > 255)
- wr_buf[0] = op + CY_SPI_A8_BIT;
- else
- wr_buf[0] = op;
- if (op == CY_SPI_WR_OP)
- wr_buf[1] = reg % 256;
- if (op == CY_SPI_WR_OP && length > 0)
- memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
+ wr_buf[0] = op + (((reg >> 8) & 0x1) ? CY_SPI_A8_BIT : 0);
+ if (op == CY_SPI_WR_OP) {
+ wr_buf[1] = reg & 0xFF;
+ if (length > 0)
+ memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
+ }
memset(xfer, 0, sizeof(xfer));
spi_message_init(&msg);
@@ -130,7 +128,7 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
}
static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
- u8 addr, u8 length, void *data)
+ u16 addr, u8 length, void *data)
{
int rc;
@@ -143,7 +141,7 @@ static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
}
static int cyttsp_spi_write_block_data(struct device *dev, u8 *xfer_buf,
- u8 addr, u8 length, const void *data)
+ u16 addr, u8 length, const void *data)
{
return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_WR_OP, addr, (void *)data,
length);
diff --git a/drivers/input/touchscreen/cyttsp_core.h b/drivers/input/touchscreen/cyttsp_core.h
index 0cf564a..0707411 100644
--- a/drivers/input/touchscreen/cyttsp_core.h
+++ b/drivers/input/touchscreen/cyttsp_core.h
@@ -112,9 +112,9 @@ struct cyttsp;
struct cyttsp_bus_ops {
u16 bustype;
- int (*write)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
+ int (*write)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
const void *values);
- int (*read)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
+ int (*read)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
void *values);
};
@@ -145,9 +145,9 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
struct device *dev, int irq, size_t xfer_buf_size);
void cyttsp_remove(struct cyttsp *ts);
-int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
+int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
u8 length, const void *values);
-int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
+int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
u8 length, void *values);
extern const struct dev_pm_ops cyttsp_pm_ops;
diff --git a/drivers/input/touchscreen/cyttsp_i2c_common.c b/drivers/input/touchscreen/cyttsp_i2c_common.c
index 07c553f..1d7b6f1 100644
--- a/drivers/input/touchscreen/cyttsp_i2c_common.c
+++ b/drivers/input/touchscreen/cyttsp_i2c_common.c
@@ -32,18 +32,20 @@
#include <linux/types.h>
int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf,
- u8 addr, u8 length, void *values)
+ u16 addr, u8 length, void *values)
{
struct i2c_client *client = to_i2c_client(dev);
+ u8 client_addr = client->addr | ((addr >> 8) & 0x1);
+ u8 addr_lo = addr & 0xFF;
struct i2c_msg msgs[] = {
{
- .addr = client->addr,
+ .addr = client_addr,
.flags = 0,
.len = 1,
- .buf = &addr,
+ .buf = &addr_lo,
},
{
- .addr = client->addr,
+ .addr = client_addr,
.flags = I2C_M_RD,
.len = length,
.buf = values,
@@ -60,17 +62,29 @@ int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf,
EXPORT_SYMBOL_GPL(cyttsp_i2c_read_block_data);
int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf,
- u8 addr, u8 length, const void *values)
+ u16 addr, u8 length, const void *values)
{
struct i2c_client *client = to_i2c_client(dev);
+ u8 client_addr = client->addr | ((addr >> 8) & 0x1);
+ u8 addr_lo = addr & 0xFF;
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client_addr,
+ .flags = 0,
+ .len = length + 1,
+ .buf = xfer_buf,
+ },
+ };
int retval;
- xfer_buf[0] = addr;
+ xfer_buf[0] = addr_lo;
memcpy(&xfer_buf[1], values, length);
- retval = i2c_master_send(client, xfer_buf, length + 1);
+ retval = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+ if (retval < 0)
+ return retval;
- return retval < 0 ? retval : 0;
+ return retval != ARRAY_SIZE(msgs) ? -EIO : 0;
}
EXPORT_SYMBOL_GPL(cyttsp_i2c_write_block_data);
diff --git a/drivers/input/touchscreen/cyttsp_spi.c b/drivers/input/touchscreen/cyttsp_spi.c
index 1df6253..4728bcb 100644
--- a/drivers/input/touchscreen/cyttsp_spi.c
+++ b/drivers/input/touchscreen/cyttsp_spi.c
@@ -41,7 +41,7 @@
#define CY_SPI_BITS_PER_WORD 8
static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
- u8 op, u8 reg, u8 *buf, int length)
+ u8 op, u16 reg, u8 *buf, int length)
{
struct spi_device *spi = to_spi_device(dev);
struct spi_message msg;
@@ -126,14 +126,14 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
}
static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
- u8 addr, u8 length, void *data)
+ u16 addr, u8 length, void *data)
{
return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_RD_OP, addr, data,
length);
}
static int cyttsp_spi_write_block_data(struct device *dev, u8 *xfer_buf,
- u8 addr, u8 length, const void *data)
+ u16 addr, u8 length, const void *data)
{
return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_WR_OP, addr, (void *)data,
length);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] Input: cyttsp4 - use 16bit address for I2C/SPI communication
2013-07-03 20:36 ` [PATCH] Input: cyttsp4 - use 16bit address for I2C/SPI communication Ferruh Yigit
@ 2013-07-04 12:25 ` Javier Martinez Canillas
2013-07-07 5:08 ` Dmitry Torokhov
1 sibling, 0 replies; 8+ messages in thread
From: Javier Martinez Canillas @ 2013-07-04 12:25 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Dan Carpenter, Dmitry Torokhov, ttdrivers, linux-input
On Wed, Jul 3, 2013 at 10:36 PM, Ferruh Yigit <fery@cypress.com> wrote:
> In TSG4, register map is 512bytes long and to access all of it,
> one bit from address byte is used (which bit to use differs for
> I2C and SPI);
>
> Since common code used for TSG3 and TSG4 for I2C, this parameter
> wrongly used as u8. TSG3 does not access beyond 255 bytes
> but TSG4 may.
>
> Signed-off-by: Ferruh Yigit <fery@cypress.com>
> Tested-on: TMA3XX DVB && TMA4XX DVB
> ---
> drivers/input/touchscreen/cyttsp4_core.h | 12 +++++-----
> drivers/input/touchscreen/cyttsp4_spi.c | 20 ++++++++---------
> drivers/input/touchscreen/cyttsp_core.h | 8 +++----
> drivers/input/touchscreen/cyttsp_i2c_common.c | 30 ++++++++++++++++++-------
> drivers/input/touchscreen/cyttsp_spi.c | 6 ++---
> 5 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/input/touchscreen/cyttsp4_core.h b/drivers/input/touchscreen/cyttsp4_core.h
> index 86a2543..8e0d4d4 100644
> --- a/drivers/input/touchscreen/cyttsp4_core.h
> +++ b/drivers/input/touchscreen/cyttsp4_core.h
> @@ -369,9 +369,9 @@ struct cyttsp4 {
>
> struct cyttsp4_bus_ops {
> u16 bustype;
> - int (*write)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*write)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> const void *values);
> - int (*read)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*read)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> void *values);
> };
>
> @@ -448,13 +448,13 @@ enum cyttsp4_event_id {
> /* y-axis, 0:origin is on top side of panel, 1: bottom */
> #define CY_PCFG_ORIGIN_Y_MASK 0x80
>
> -static inline int cyttsp4_adap_read(struct cyttsp4 *ts, u8 addr, int size,
> +static inline int cyttsp4_adap_read(struct cyttsp4 *ts, u16 addr, int size,
> void *buf)
> {
> return ts->bus_ops->read(ts->dev, ts->xfer_buf, addr, size, buf);
> }
>
> -static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u8 addr, int size,
> +static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u16 addr, int size,
> const void *buf)
> {
> return ts->bus_ops->write(ts->dev, ts->xfer_buf, addr, size, buf);
> @@ -463,9 +463,9 @@ static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u8 addr, int size,
> extern struct cyttsp4 *cyttsp4_probe(const struct cyttsp4_bus_ops *ops,
> struct device *dev, u16 irq, size_t xfer_buf_size);
> extern int cyttsp4_remove(struct cyttsp4 *ts);
> -int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, const void *values);
> -int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, void *values);
> extern const struct dev_pm_ops cyttsp4_pm_ops;
>
> diff --git a/drivers/input/touchscreen/cyttsp4_spi.c b/drivers/input/touchscreen/cyttsp4_spi.c
> index f8f891b..a71e114 100644
> --- a/drivers/input/touchscreen/cyttsp4_spi.c
> +++ b/drivers/input/touchscreen/cyttsp4_spi.c
> @@ -44,7 +44,7 @@
> #define CY_SPI_DATA_BUF_SIZE (CY_SPI_CMD_BYTES + CY_SPI_DATA_SIZE)
>
> static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> - u8 op, u8 reg, u8 *buf, int length)
> + u8 op, u16 reg, u8 *buf, int length)
> {
> struct spi_device *spi = to_spi_device(dev);
> struct spi_message msg;
> @@ -63,14 +63,12 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> memset(wr_buf, 0, CY_SPI_DATA_BUF_SIZE);
> memset(rd_buf, 0, CY_SPI_CMD_BYTES);
>
> - if (reg > 255)
> - wr_buf[0] = op + CY_SPI_A8_BIT;
> - else
> - wr_buf[0] = op;
> - if (op == CY_SPI_WR_OP)
> - wr_buf[1] = reg % 256;
> - if (op == CY_SPI_WR_OP && length > 0)
> - memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
> + wr_buf[0] = op + (((reg >> 8) & 0x1) ? CY_SPI_A8_BIT : 0);
> + if (op == CY_SPI_WR_OP) {
> + wr_buf[1] = reg & 0xFF;
> + if (length > 0)
> + memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
> + }
>
> memset(xfer, 0, sizeof(xfer));
> spi_message_init(&msg);
> @@ -130,7 +128,7 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> }
>
> static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, void *data)
> + u16 addr, u8 length, void *data)
> {
> int rc;
>
> @@ -143,7 +141,7 @@ static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
> }
>
> static int cyttsp_spi_write_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, const void *data)
> + u16 addr, u8 length, const void *data)
> {
> return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_WR_OP, addr, (void *)data,
> length);
> diff --git a/drivers/input/touchscreen/cyttsp_core.h b/drivers/input/touchscreen/cyttsp_core.h
> index 0cf564a..0707411 100644
> --- a/drivers/input/touchscreen/cyttsp_core.h
> +++ b/drivers/input/touchscreen/cyttsp_core.h
> @@ -112,9 +112,9 @@ struct cyttsp;
>
> struct cyttsp_bus_ops {
> u16 bustype;
> - int (*write)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*write)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> const void *values);
> - int (*read)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*read)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> void *values);
> };
>
> @@ -145,9 +145,9 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
> struct device *dev, int irq, size_t xfer_buf_size);
> void cyttsp_remove(struct cyttsp *ts);
>
> -int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, const void *values);
> -int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, void *values);
> extern const struct dev_pm_ops cyttsp_pm_ops;
>
> diff --git a/drivers/input/touchscreen/cyttsp_i2c_common.c b/drivers/input/touchscreen/cyttsp_i2c_common.c
> index 07c553f..1d7b6f1 100644
> --- a/drivers/input/touchscreen/cyttsp_i2c_common.c
> +++ b/drivers/input/touchscreen/cyttsp_i2c_common.c
> @@ -32,18 +32,20 @@
> #include <linux/types.h>
>
> int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, void *values)
> + u16 addr, u8 length, void *values)
> {
> struct i2c_client *client = to_i2c_client(dev);
> + u8 client_addr = client->addr | ((addr >> 8) & 0x1);
> + u8 addr_lo = addr & 0xFF;
> struct i2c_msg msgs[] = {
> {
> - .addr = client->addr,
> + .addr = client_addr,
> .flags = 0,
> .len = 1,
> - .buf = &addr,
> + .buf = &addr_lo,
> },
> {
> - .addr = client->addr,
> + .addr = client_addr,
> .flags = I2C_M_RD,
> .len = length,
> .buf = values,
> @@ -60,17 +62,29 @@ int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf,
> EXPORT_SYMBOL_GPL(cyttsp_i2c_read_block_data);
>
> int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, const void *values)
> + u16 addr, u8 length, const void *values)
> {
> struct i2c_client *client = to_i2c_client(dev);
> + u8 client_addr = client->addr | ((addr >> 8) & 0x1);
> + u8 addr_lo = addr & 0xFF;
> + struct i2c_msg msgs[] = {
> + {
> + .addr = client_addr,
> + .flags = 0,
> + .len = length + 1,
> + .buf = xfer_buf,
> + },
> + };
> int retval;
>
> - xfer_buf[0] = addr;
> + xfer_buf[0] = addr_lo;
> memcpy(&xfer_buf[1], values, length);
>
> - retval = i2c_master_send(client, xfer_buf, length + 1);
> + retval = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (retval < 0)
> + return retval;
>
> - return retval < 0 ? retval : 0;
> + return retval != ARRAY_SIZE(msgs) ? -EIO : 0;
> }
> EXPORT_SYMBOL_GPL(cyttsp_i2c_write_block_data);
>
> diff --git a/drivers/input/touchscreen/cyttsp_spi.c b/drivers/input/touchscreen/cyttsp_spi.c
> index 1df6253..4728bcb 100644
> --- a/drivers/input/touchscreen/cyttsp_spi.c
> +++ b/drivers/input/touchscreen/cyttsp_spi.c
> @@ -41,7 +41,7 @@
> #define CY_SPI_BITS_PER_WORD 8
>
> static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> - u8 op, u8 reg, u8 *buf, int length)
> + u8 op, u16 reg, u8 *buf, int length)
> {
> struct spi_device *spi = to_spi_device(dev);
> struct spi_message msg;
> @@ -126,14 +126,14 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> }
>
> static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, void *data)
> + u16 addr, u8 length, void *data)
> {
> return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_RD_OP, addr, data,
> length);
> }
>
> static int cyttsp_spi_write_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, const void *data)
> + u16 addr, u8 length, const void *data)
> {
> return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_WR_OP, addr, (void *)data,
> length);
> --
> 1.7.9.5
>
Acked-by: Javier Martinez Canillas <javier@dowhile0.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Input: cyttsp4 - use 16bit address for I2C/SPI communication
2013-07-03 20:36 ` [PATCH] Input: cyttsp4 - use 16bit address for I2C/SPI communication Ferruh Yigit
2013-07-04 12:25 ` Javier Martinez Canillas
@ 2013-07-07 5:08 ` Dmitry Torokhov
1 sibling, 0 replies; 8+ messages in thread
From: Dmitry Torokhov @ 2013-07-07 5:08 UTC (permalink / raw)
To: Ferruh Yigit
Cc: Dan Carpenter, ttdrivers, Javier Martinez Canillas, linux-input
On Wed, Jul 03, 2013 at 11:36:20PM +0300, Ferruh Yigit wrote:
> In TSG4, register map is 512bytes long and to access all of it,
> one bit from address byte is used (which bit to use differs for
> I2C and SPI);
>
> Since common code used for TSG3 and TSG4 for I2C, this parameter
> wrongly used as u8. TSG3 does not access beyond 255 bytes
> but TSG4 may.
>
> Signed-off-by: Ferruh Yigit <fery@cypress.com>
> Tested-on: TMA3XX DVB && TMA4XX DVB
Applied, thank you.
> ---
> drivers/input/touchscreen/cyttsp4_core.h | 12 +++++-----
> drivers/input/touchscreen/cyttsp4_spi.c | 20 ++++++++---------
> drivers/input/touchscreen/cyttsp_core.h | 8 +++----
> drivers/input/touchscreen/cyttsp_i2c_common.c | 30 ++++++++++++++++++-------
> drivers/input/touchscreen/cyttsp_spi.c | 6 ++---
> 5 files changed, 44 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/input/touchscreen/cyttsp4_core.h b/drivers/input/touchscreen/cyttsp4_core.h
> index 86a2543..8e0d4d4 100644
> --- a/drivers/input/touchscreen/cyttsp4_core.h
> +++ b/drivers/input/touchscreen/cyttsp4_core.h
> @@ -369,9 +369,9 @@ struct cyttsp4 {
>
> struct cyttsp4_bus_ops {
> u16 bustype;
> - int (*write)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*write)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> const void *values);
> - int (*read)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*read)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> void *values);
> };
>
> @@ -448,13 +448,13 @@ enum cyttsp4_event_id {
> /* y-axis, 0:origin is on top side of panel, 1: bottom */
> #define CY_PCFG_ORIGIN_Y_MASK 0x80
>
> -static inline int cyttsp4_adap_read(struct cyttsp4 *ts, u8 addr, int size,
> +static inline int cyttsp4_adap_read(struct cyttsp4 *ts, u16 addr, int size,
> void *buf)
> {
> return ts->bus_ops->read(ts->dev, ts->xfer_buf, addr, size, buf);
> }
>
> -static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u8 addr, int size,
> +static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u16 addr, int size,
> const void *buf)
> {
> return ts->bus_ops->write(ts->dev, ts->xfer_buf, addr, size, buf);
> @@ -463,9 +463,9 @@ static inline int cyttsp4_adap_write(struct cyttsp4 *ts, u8 addr, int size,
> extern struct cyttsp4 *cyttsp4_probe(const struct cyttsp4_bus_ops *ops,
> struct device *dev, u16 irq, size_t xfer_buf_size);
> extern int cyttsp4_remove(struct cyttsp4 *ts);
> -int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, const void *values);
> -int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, void *values);
> extern const struct dev_pm_ops cyttsp4_pm_ops;
>
> diff --git a/drivers/input/touchscreen/cyttsp4_spi.c b/drivers/input/touchscreen/cyttsp4_spi.c
> index f8f891b..a71e114 100644
> --- a/drivers/input/touchscreen/cyttsp4_spi.c
> +++ b/drivers/input/touchscreen/cyttsp4_spi.c
> @@ -44,7 +44,7 @@
> #define CY_SPI_DATA_BUF_SIZE (CY_SPI_CMD_BYTES + CY_SPI_DATA_SIZE)
>
> static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> - u8 op, u8 reg, u8 *buf, int length)
> + u8 op, u16 reg, u8 *buf, int length)
> {
> struct spi_device *spi = to_spi_device(dev);
> struct spi_message msg;
> @@ -63,14 +63,12 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> memset(wr_buf, 0, CY_SPI_DATA_BUF_SIZE);
> memset(rd_buf, 0, CY_SPI_CMD_BYTES);
>
> - if (reg > 255)
> - wr_buf[0] = op + CY_SPI_A8_BIT;
> - else
> - wr_buf[0] = op;
> - if (op == CY_SPI_WR_OP)
> - wr_buf[1] = reg % 256;
> - if (op == CY_SPI_WR_OP && length > 0)
> - memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
> + wr_buf[0] = op + (((reg >> 8) & 0x1) ? CY_SPI_A8_BIT : 0);
> + if (op == CY_SPI_WR_OP) {
> + wr_buf[1] = reg & 0xFF;
> + if (length > 0)
> + memcpy(wr_buf + CY_SPI_CMD_BYTES, buf, length);
> + }
>
> memset(xfer, 0, sizeof(xfer));
> spi_message_init(&msg);
> @@ -130,7 +128,7 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> }
>
> static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, void *data)
> + u16 addr, u8 length, void *data)
> {
> int rc;
>
> @@ -143,7 +141,7 @@ static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
> }
>
> static int cyttsp_spi_write_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, const void *data)
> + u16 addr, u8 length, const void *data)
> {
> return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_WR_OP, addr, (void *)data,
> length);
> diff --git a/drivers/input/touchscreen/cyttsp_core.h b/drivers/input/touchscreen/cyttsp_core.h
> index 0cf564a..0707411 100644
> --- a/drivers/input/touchscreen/cyttsp_core.h
> +++ b/drivers/input/touchscreen/cyttsp_core.h
> @@ -112,9 +112,9 @@ struct cyttsp;
>
> struct cyttsp_bus_ops {
> u16 bustype;
> - int (*write)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*write)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> const void *values);
> - int (*read)(struct device *dev, u8 *xfer_buf, u8 addr, u8 length,
> + int (*read)(struct device *dev, u8 *xfer_buf, u16 addr, u8 length,
> void *values);
> };
>
> @@ -145,9 +145,9 @@ struct cyttsp *cyttsp_probe(const struct cyttsp_bus_ops *bus_ops,
> struct device *dev, int irq, size_t xfer_buf_size);
> void cyttsp_remove(struct cyttsp *ts);
>
> -int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, const void *values);
> -int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u8 addr,
> +int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf, u16 addr,
> u8 length, void *values);
> extern const struct dev_pm_ops cyttsp_pm_ops;
>
> diff --git a/drivers/input/touchscreen/cyttsp_i2c_common.c b/drivers/input/touchscreen/cyttsp_i2c_common.c
> index 07c553f..1d7b6f1 100644
> --- a/drivers/input/touchscreen/cyttsp_i2c_common.c
> +++ b/drivers/input/touchscreen/cyttsp_i2c_common.c
> @@ -32,18 +32,20 @@
> #include <linux/types.h>
>
> int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, void *values)
> + u16 addr, u8 length, void *values)
> {
> struct i2c_client *client = to_i2c_client(dev);
> + u8 client_addr = client->addr | ((addr >> 8) & 0x1);
> + u8 addr_lo = addr & 0xFF;
> struct i2c_msg msgs[] = {
> {
> - .addr = client->addr,
> + .addr = client_addr,
> .flags = 0,
> .len = 1,
> - .buf = &addr,
> + .buf = &addr_lo,
> },
> {
> - .addr = client->addr,
> + .addr = client_addr,
> .flags = I2C_M_RD,
> .len = length,
> .buf = values,
> @@ -60,17 +62,29 @@ int cyttsp_i2c_read_block_data(struct device *dev, u8 *xfer_buf,
> EXPORT_SYMBOL_GPL(cyttsp_i2c_read_block_data);
>
> int cyttsp_i2c_write_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, const void *values)
> + u16 addr, u8 length, const void *values)
> {
> struct i2c_client *client = to_i2c_client(dev);
> + u8 client_addr = client->addr | ((addr >> 8) & 0x1);
> + u8 addr_lo = addr & 0xFF;
> + struct i2c_msg msgs[] = {
> + {
> + .addr = client_addr,
> + .flags = 0,
> + .len = length + 1,
> + .buf = xfer_buf,
> + },
> + };
> int retval;
>
> - xfer_buf[0] = addr;
> + xfer_buf[0] = addr_lo;
> memcpy(&xfer_buf[1], values, length);
>
> - retval = i2c_master_send(client, xfer_buf, length + 1);
> + retval = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (retval < 0)
> + return retval;
>
> - return retval < 0 ? retval : 0;
> + return retval != ARRAY_SIZE(msgs) ? -EIO : 0;
> }
> EXPORT_SYMBOL_GPL(cyttsp_i2c_write_block_data);
>
> diff --git a/drivers/input/touchscreen/cyttsp_spi.c b/drivers/input/touchscreen/cyttsp_spi.c
> index 1df6253..4728bcb 100644
> --- a/drivers/input/touchscreen/cyttsp_spi.c
> +++ b/drivers/input/touchscreen/cyttsp_spi.c
> @@ -41,7 +41,7 @@
> #define CY_SPI_BITS_PER_WORD 8
>
> static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> - u8 op, u8 reg, u8 *buf, int length)
> + u8 op, u16 reg, u8 *buf, int length)
> {
> struct spi_device *spi = to_spi_device(dev);
> struct spi_message msg;
> @@ -126,14 +126,14 @@ static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> }
>
> static int cyttsp_spi_read_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, void *data)
> + u16 addr, u8 length, void *data)
> {
> return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_RD_OP, addr, data,
> length);
> }
>
> static int cyttsp_spi_write_block_data(struct device *dev, u8 *xfer_buf,
> - u8 addr, u8 length, const void *data)
> + u16 addr, u8 length, const void *data)
> {
> return cyttsp_spi_xfer(dev, xfer_buf, CY_SPI_WR_OP, addr, (void *)data,
> length);
> --
> 1.7.9.5
>
--
Dmitry
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices
[not found] <20130704235117.733726609A1@gitolite.kernel.org>
@ 2013-07-07 19:15 ` Geert Uytterhoeven
2013-07-09 7:12 ` Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2013-07-07 19:15 UTC (permalink / raw)
To: Ferruh Yigit, Javier Martinez Canillas, Dmitry Torokhov
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
On Fri, Jul 5, 2013 at 1:51 AM, Linux Kernel Mailing List
<linux-kernel@vger.kernel.org> wrote:
> +++ b/drivers/input/touchscreen/cyttsp4_spi.c
> +static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
> + u8 op, u8 reg, u8 *buf, int length)
> +{
> + if (reg > 255)
As "reg" is "u8", this is never true:
drivers/input/touchscreen/cyttsp4_spi.c: In function ‘cyttsp_spi_xfer’:
drivers/input/touchscreen/cyttsp4_spi.c:66: warning: comparison is
always false due to limited range of data type
> + wr_buf[0] = op + CY_SPI_A8_BIT;
> + else
> + wr_buf[0] = op;
Can the if-clause and the first branch just be removed, or is there a real bug
involved (e.g. wrong type for "reg")?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices
2013-07-07 19:15 ` Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices Geert Uytterhoeven
@ 2013-07-09 7:12 ` Ferruh Yigit
0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2013-07-09 7:12 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Javier Martinez Canillas, Dmitry Torokhov,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
On 07/07/2013 10:15 PM, Geert Uytterhoeven wrote:
> On Fri, Jul 5, 2013 at 1:51 AM, Linux Kernel Mailing List
> <linux-kernel@vger.kernel.org> wrote:
>> +++ b/drivers/input/touchscreen/cyttsp4_spi.c
>
>> +static int cyttsp_spi_xfer(struct device *dev, u8 *xfer_buf,
>> + u8 op, u8 reg, u8 *buf, int length)
>> +{
>
>> + if (reg > 255)
>
> As "reg" is "u8", this is never true:
>
> drivers/input/touchscreen/cyttsp4_spi.c: In function ‘cyttsp_spi_xfer’:
> drivers/input/touchscreen/cyttsp4_spi.c:66: warning: comparison is
> always false due to limited range of data type
>
>> + wr_buf[0] = op + CY_SPI_A8_BIT;
>> + else
>> + wr_buf[0] = op;
>
> Can the if-clause and the first branch just be removed, or is there a real bug
> involved (e.g. wrong type for "reg")?
Yes there was a bug here, and already sent a patch for this, please
check https://patchwork.kernel.org/patch/2820561/
thanks,
ferruh
This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-07-09 7:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-02 21:47 Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices Dan Carpenter
2013-07-03 13:24 ` Ferruh Yigit
2013-07-03 13:52 ` Dan Carpenter
2013-07-03 20:36 ` [PATCH] Input: cyttsp4 - use 16bit address for I2C/SPI communication Ferruh Yigit
2013-07-04 12:25 ` Javier Martinez Canillas
2013-07-07 5:08 ` Dmitry Torokhov
[not found] <20130704235117.733726609A1@gitolite.kernel.org>
2013-07-07 19:15 ` Input: cyttsp4 - SPI driver for Cypress TMA4XX touchscreen devices Geert Uytterhoeven
2013-07-09 7:12 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).