Input: atmel_mxt_ts - Make wait-after-reset period compatible with all chips
Subsystems (7)
PATCH v4 1/3 Input: atmel_mxt_ts - Make wait-after-reset period compatible with all chips
The delay before the chip can be accessed after reset varies between different chips in maXTouch family. Waiting for 200ms and then monitoring the CHG (chip is ready when the line is low) is guaranteed to work with all chips.
v4: Adjust delay depending on the family ID of the chip. Also add a readback
of command register after backup is issued, to make sure we are not proceeding too fast there.
v3: Add a check for NULL read_chg() function, and add the read_chg() to platform
files using this driver (currently only mach-goni.c)
v2: At Dmitry's suggestion, add a timeout so we are not stuck looping
endlessly in case the CHG is not going low.
--- arch/arm/mach-s5pv210/mach-goni.c | 6 ++++ drivers/input/touchscreen/atmel_mxt_ts.c | 45 ++++++++++++++++++++++++++++- include/linux/i2c/atmel_mxt_ts.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)
arch/arm/mach-s5pv210/mach-goni.c +6 -0
@@ -225,6 +225,11 @@ static void __init goni_radio_init(void)gpio_direction_output (gpio,1 ); } +static u8 read_chg (void ) +{ +return gpio_get_value (S5PV210_GPJ0 (5 )); +} +/* TSP */ static struct mxt_platform_data qt602240_platform_data = { .x_line =17 ,
@@ -236,6 +241,7 @@ static struct mxt_platform_data qt602240_platform_data = { .voltage = 2800000, /* 2.8V */ .orient = MXT_DIAGONAL, .irqflags = IRQF_TRIGGER_FALLING, + .read_chg = &read_chg, };static struct s3c2410_platform_i2c i2c2_data__initdata = {
drivers/input/touchscreen/atmel_mxt_ts.c +43 -2
74 lines not on this page · raw message
include/linux/i2c/atmel_mxt_ts.h +1 -0
@@ -39,6 +39,7 @@ struct mxt_platform_data {unsigned int voltage;unsigned char orient;unsigned long irqflags; + u8(*read_chg) (void); };#endif /* __LINUX_ATMEL_MXT_TS_H */
Signature · 1 line
1.7.0.4
Hi Iiro,
On one of our chipsets, we use mxt1386 and from the specification doc we found that the software reset delay for this chip is ~250ms. With your patch we observed the code to be looping in timeout_counter for sometime but with ~250ms change it avoids that loop. Please consider this change.
I also have couple of review comments, please check.
Quoted 81 lines · Iiro Valkonen
The delay before the chip can be accessed after reset varies between different chips in maXTouch family. Waiting for 200ms and then monitoring the CHG (chip is ready when the line is low) is guaranteed to work with all chips.
v4: Adjust delay depending on the family ID of the chip. Also add a readback
of command register after backup is issued, to make sure we are not proceeding too fast there.
v3: Add a check for NULLread_chg()function, and add theread_chg()to platform
files using this driver (currently onlymach-goni.c)
v2: At Dmitry's suggestion, add a timeout so we are not stuck looping
endlessly in case the CHG is not going low.Signed-off-by Iiro Valkonen<[email protected]>--- arch/arm/mach-s5pv210/mach-goni.c | 6 ++++ drivers/input/touchscreen/atmel_mxt_ts.c | 45 ++++++++++++++++++++++++++++- include/linux/i2c/atmel_mxt_ts.h | 1 + 3 files changed, 50 insertions(+), 2 deletions(-)arch/arm/mach-s5pv210/mach-goni.c +6
-0@@ -225,6 +225,11 @@ static void __init goni_radio_init(void)gpio_direction_output (gpio,1 ); } +static u8 read_chg (void ) +{ +return gpio_get_value (S5PV210_GPJ0 (5 )); +} +/* TSP */ static struct mxt_platform_data qt602240_platform_data = { .x_line =17 ,@@ -236,6 +241,7 @@ static struct mxt_platform_data qt602240_platform_data = { .voltage = 2800000, /* 2.8V */ .orient = MXT_DIAGONAL, .irqflags = IRQF_TRIGGER_FALLING, + .read_chg =&read_chg, };static struct s3c2410_platform_i2c i2c2_data__initdata = {drivers/input/touchscreen/atmel_mxt_ts.c +10
-1@@ -21,6 +21,10 @@#include <linux/interrupt.h> #include <linux/slab.h> +/* Family ID */ +#define MXT224_ID 0x80 +#define MXT1386_ID 0xA0 +/* Version */ #define MXT_VER_20 20#define MXT_VER_21 21@@ -176,7 +180,10 @@#define MXT_BOOT_VALUE 0xa5#define MXT_BACKUP_VALUE 0x55#define MXT_BACKUP_TIME 25/* msec */ -#define MXT_RESET_TIME 65/* msec */ +#define MXT224_RESET_TIME 65/* msec */ +#define MXT1386_RESET_TIME 200/* msec */ +#define MXT_RESET_TIME 200/* msec */ +#define MXT_RESET_NOCHGREAD 400/* msec */ #define MXT_FWRESET_TIME 175/* msec */ @@ -814,7 +821,9 @@ static int mxt_initialize(struct mxt_data *data)struct i2c_client *client = data->client;struct mxt_info *info =&data->info;int error; +int timeout_counter =0 ;u8 val; +u8 command_register; error =mxt_get_info (data);if (error)
drivers/input/touchscreen/atmel_mxt_ts.c static int mxt_initialize(struct mxt_data *data) · line 863
Quoted 10 lines
drivers/input/touchscreen/atmel_mxt_ts.c +7
-0@@ -845,11 +854,43 @@ static int mxt_initialize(struct mxt_data *data)MXT_COMMAND_BACKUPNV , MXT_BACKUP_VALUE);msleep (MXT_BACKUP_TIME ); + do { + error = mxt_read_object(data, MXT_GEN_COMMAND_T6, + MXT_COMMAND_BACKUPNV, + &command_register); + if (error) + return error; + msleep(2);
msleep(1~20) doesn't guarantee what we intend to do. Please check Documentation/timers/timers-howto.txt. This doc suggests us to use usleep_range() for these smaller delays.
drivers/input/touchscreen/atmel_mxt_ts.c static int mxt_initialize(struct mxt_data *data) · line 865
Quoted 2 lines
drivers/input/touchscreen/atmel_mxt_ts.c +2
-0+ }while ((command_register !=0 )&& (timeout_counter++<=100 )); +if (timeout_counter>=100 ) {
This condition has off-by-one error. If the loop succeeds at counter is 99 then as it is post-increment, the counter would be 100 by the time it comes to this condition. Though we are successful, it still returns error. Please check and I suggest to use ">" rather than ">=".
Quoted 14 lines
drivers/input/touchscreen/atmel_mxt_ts.c +9
-1+dev_err (&client->dev,"No response after backup!\n" ); +return -EIO ; + }/* Soft reset */ mxt_write_object (data,MXT_GEN_COMMAND_T6 ,MXT_COMMAND_RESET ,1 ); -msleep (MXT_RESET_TIME ); +if (data->pdata->read_chg ==NULL ) { +msleep (MXT_RESET_NOCHGREAD ); + }else { +switch (info->family_id) { +case MXT224_ID : +msleep (MXT224_RESET_TIME );
drivers/input/touchscreen/atmel_mxt_ts.c static int mxt_initialize(struct mxt_data *data) · line 888
Quoted 10 lines
drivers/input/touchscreen/atmel_mxt_ts.c +10
-0+break ; +case MXT1386_ID : +msleep (MXT1386_RESET_TIME ); +break ; +default : +msleep (MXT_RESET_TIME ); + } + timeout_counter =0 ; +while ((timeout_counter++<=100 )&& data->pdata->read_chg ()) +msleep (2 );
same here, use usleep_range.
drivers/input/touchscreen/atmel_mxt_ts.c static int mxt_initialize(struct mxt_data *data) · line 889
Quoted 1 line
drivers/input/touchscreen/atmel_mxt_ts.c +1
-0+if (timeout_counter>=100 ) {
Same here, one-by-off error.
Quoted 12 lines
drivers/input/touchscreen/atmel_mxt_ts.c +4
-0+dev_err (&client->dev,"No response after reset!\n" ); +return -EIO ; + } + }/* Update matrix size at info struct */ error =mxt_read_reg (client,MXT_MATRIX_X_SIZE ,&val);include/linux/i2c/atmel_mxt_ts.h +0
-0
include/linux/i2c/atmel_mxt_ts.h struct mxt_platform_data { · line 45
Quoted 7 lines
include/linux/i2c/atmel_mxt_ts.h +1
-0@@ -39,6 +39,7 @@ struct mxt_platform_data {unsigned int voltage;unsigned char orient;unsigned long irqflags; + u8(*read_chg) (void); };#endif /* __LINUX_ATMEL_MXT_TS_H */
--Mohan.
Signature · 3 lines
Sent by a consultant of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.