Hi Nick, Thanks for this patch. Comments inline. On Fri, Feb 22, 2013 at 9:57 AM, Nick Dyer <nick.dyer@xxxxxxxxxxx> wrote: > From: Iiro Valkonen <iiro.valkonen@xxxxxxxxx> > > 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. > > Signed-off-by: Nick Dyer <nick.dyer@xxxxxxxxxxx> > --- > drivers/input/touchscreen/atmel_mxt_ts.c | 91 ++++++++++++++++++++++++------ > include/linux/i2c/atmel_mxt_ts.h | 1 + > 2 files changed, 76 insertions(+), 16 deletions(-) > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index d04f810..b4bf946 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -2,6 +2,7 @@ > * Atmel maXTouch Touchscreen driver > * > * Copyright (C) 2010 Samsung Electronics Co.Ltd > + * Copyright (C) 2011 Atmel Corporation > * Author: Joonyoung Shim <jy0922.shim@xxxxxxxxxxx> > * > * This program is free software; you can redistribute it and/or modify it > @@ -175,11 +176,14 @@ > > /* Define for MXT_GEN_COMMAND_T6 */ > #define MXT_BOOT_VALUE 0xa5 > +#define MXT_RESET_VALUE 0x01 > #define MXT_BACKUP_VALUE 0x55 > -#define MXT_BACKUP_TIME 25 /* msec */ > -#define MXT_RESET_TIME 65 /* msec */ > > -#define MXT_FWRESET_TIME 175 /* msec */ > +/* Delay times */ > +#define MXT_BACKUP_TIME 25 /* msec */ > +#define MXT_RESET_TIME 200 /* msec */ > +#define MXT_RESET_NOCHGREAD 400 /* msec */ > +#define MXT_FWRESET_TIME 1000 /* msec */ > > /* Command to unlock bootloader */ > #define MXT_UNLOCK_CMD_MSB 0xaa > @@ -249,6 +253,7 @@ struct mxt_data { > > /* Cached parameters from object table */ > u8 T6_reportid; > + u16 T6_address; > u8 T9_reportid_min; > u8 T9_reportid_max; > }; > @@ -599,6 +604,66 @@ end: > return IRQ_HANDLED; > } > > +static int mxt_t6_command(struct mxt_data *data, u16 cmd_offset, u8 value, bool wait) > +{ > + u16 reg; > + u8 command_register; > + int ret; > + int timeout_counter = 0; > + > + reg = data->T6_address + cmd_offset; > + > + ret = mxt_write_reg(data->client, reg, value); > + if (ret) > + return ret; > + > + if (!wait) > + return 0; > + > + do { > + msleep(20); > + ret = __mxt_read_reg(data->client, reg, 1, &command_register); > + if (ret) > + return ret; > + } while ((command_register != 0) && (timeout_counter++ <= 100)); > + > + if (timeout_counter > 100) { > + dev_err(&data->client->dev, "Command failed!\n"); > + return -EIO; > + } > + > + return 0; > +} > + > +static int mxt_soft_reset(struct mxt_data *data, u8 value) > +{ > + int ret; > + int timeout_counter = 0; > + struct device *dev = &data->client->dev; > + > + dev_info(dev, "Resetting chip\n"); > + > + ret = mxt_t6_command(data, MXT_COMMAND_RESET, value, false); > + if (ret) > + return ret; > + > + if (data->pdata->read_chg == NULL) { Is it possible for data->pdata to be NULL here? For our chromiumos projects, we operate with a NULL pdata. There may be a bunch of other places where this is a problem, actually. > + msleep(MXT_RESET_NOCHGREAD); > + } else { > + msleep(MXT_RESET_TIME); > + > + timeout_counter = 0; > + while ((timeout_counter++ <= 100) && data->pdata->read_chg()) > + msleep(20); > + if (timeout_counter > 100) { > + dev_err(dev, "No response after reset!\n"); > + return -EIO; > + } > + } > + > + return 0; > +} > + > static int mxt_check_reg_init(struct mxt_data *data) > { > const struct mxt_platform_data *pdata = data->pdata; > @@ -759,6 +824,7 @@ static int mxt_get_object_table(struct mxt_data *data) > switch (object->type) { > case MXT_GEN_COMMAND_T6: > data->T6_reportid = min_id; > + data->T6_address = object->start_address; > break; > case MXT_TOUCH_MULTI_T9: > data->T9_reportid_min = min_id; > @@ -811,16 +877,9 @@ static int mxt_initialize(struct mxt_data *data) > > mxt_handle_pdata(data); > > - /* Backup to memory */ > - mxt_write_object(data, MXT_GEN_COMMAND_T6, > - MXT_COMMAND_BACKUPNV, > - MXT_BACKUP_VALUE); > - msleep(MXT_BACKUP_TIME); > - > - /* Soft reset */ > - mxt_write_object(data, MXT_GEN_COMMAND_T6, > - MXT_COMMAND_RESET, 1); > - msleep(MXT_RESET_TIME); > + error = mxt_t6_command(data, MXT_COMMAND_BACKUPNV, MXT_BACKUP_VALUE, false); > + if (!error) > + mxt_soft_reset(data, MXT_RESET_VALUE); > > /* Update matrix size at info struct */ > error = mxt_read_reg(client, MXT_MATRIX_X_SIZE, &val); > @@ -960,9 +1019,9 @@ static int mxt_load_fw(struct device *dev, const char *fn) > } > > /* Change to the bootloader mode */ > - mxt_write_object(data, MXT_GEN_COMMAND_T6, > - MXT_COMMAND_RESET, MXT_BOOT_VALUE); > - msleep(MXT_RESET_TIME); > + ret = mxt_soft_reset(data, MXT_BOOT_VALUE); > + if (ret) > + return ret; > > /* Change to slave address of bootloader */ > if (client->addr == MXT_APP_LOW) > diff --git a/include/linux/i2c/atmel_mxt_ts.h b/include/linux/i2c/atmel_mxt_ts.h > index f027f7a..ef59c22 100644 > --- a/include/linux/i2c/atmel_mxt_ts.h > +++ b/include/linux/i2c/atmel_mxt_ts.h > @@ -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 */ > -- > 1.7.10.4 > -- Benson Leung Software Engineer, Chrom* OS bleung@xxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html