Hi Dudley, On Fri, Aug 22, 2014 at 04:41:07PM +0800, Dudley Du wrote: > Dmitry, > > Thanks for your feedback. > > Thanks, > Dudley > > > -----Original Message----- > > From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] > > Sent: Friday, August 22, 2014 2:21 AM > > To: Dudley Du > > Cc: Rafael J. Wysocki; Benson Leung; Patrik Fimml; linux-input@xxxxxxxxxxxxxxx; > > linux-kernel@xxxxxxxxxxxxxxx; Dudley Du > > Subject: Re: [PATCH v4 1/14] input: cyapa: re-architecture driver to support > > multi-trackpads in one driver > > > > On Thu, Jul 17, 2014 at 02:47:24PM +0800, Dudley Du wrote: > > > In order to support two different communication protocol based trackpad > > > device in one cyapa, the new cyapa driver is re-designed with > > > one cyapa driver core and two devices' functions component. > > > The cyapa driver core is contained in this patch, it supplies the basic > > > function with input and kernel system and also defined the interfaces > > > that the devices' functions component needs to apply and support. > > > Also, in order to speed up the system boot time, the device states > > > detecting and probing process is put into the async thread. > > > TEST=test on Chromebooks. > > > > > > Signed-off-by: Dudley Du <dudl@xxxxxxxxxxx> > > > --- > > > drivers/input/mouse/Makefile | 4 +- > > > drivers/input/mouse/cyapa.c | 1083 ++++++++++++++------------------------- > > --- > > > drivers/input/mouse/cyapa.h | 275 +++++++++++ > > > 3 files changed, 646 insertions(+), 716 deletions(-) > > > create mode 100644 drivers/input/mouse/cyapa.h > > > > > > diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile > > > index c25efdb..8608eb7 100644 > > > --- a/drivers/input/mouse/Makefile > > > +++ b/drivers/input/mouse/Makefile > > > @@ -8,7 +8,7 @@ obj-$(CONFIG_MOUSE_AMIGA) += amimouse.o > > > obj-$(CONFIG_MOUSE_APPLETOUCH) += appletouch.o > > > obj-$(CONFIG_MOUSE_ATARI) += atarimouse.o > > > obj-$(CONFIG_MOUSE_BCM5974) += bcm5974.o > > > -obj-$(CONFIG_MOUSE_CYAPA) += cyapa.o > > > +obj-$(CONFIG_MOUSE_CYAPA) += cyapatp.o > > > obj-$(CONFIG_MOUSE_GPIO) += gpio_mouse.o > > > obj-$(CONFIG_MOUSE_INPORT) += inport.o > > > obj-$(CONFIG_MOUSE_LOGIBM) += logibm.o > > > @@ -34,3 +34,5 @@ psmouse-$(CONFIG_MOUSE_PS2_SENTELIC) += sentelic.o > > > psmouse-$(CONFIG_MOUSE_PS2_TRACKPOINT) += trackpoint.o > > > psmouse-$(CONFIG_MOUSE_PS2_TOUCHKIT) += touchkit_ps2.o > > > psmouse-$(CONFIG_MOUSE_PS2_CYPRESS) += cypress_ps2.o > > > + > > > +cyapatp-y := cyapa.o > > > diff --git a/drivers/input/mouse/cyapa.c b/drivers/input/mouse/cyapa.c > > > index b409c3d..5fc8dbe 100644 > > > --- a/drivers/input/mouse/cyapa.c > > > +++ b/drivers/input/mouse/cyapa.c > > > @@ -6,7 +6,7 @@ > > > * Daniel Kurtz <djkurtz@xxxxxxxxxxxx> > > > * Benson Leung <bleung@xxxxxxxxxxxx> > > > * > > > - * Copyright (C) 2011-2012 Cypress Semiconductor, Inc. > > > + * Copyright (C) 2011-2014 Cypress Semiconductor, Inc. > > > * Copyright (C) 2011-2012 Google, Inc. > > > * > > > * This file is subject to the terms and conditions of the GNU General > > Public > > > @@ -14,731 +14,114 @@ > > > * more details. > > > */ > > > > > > +#include <linux/debugfs.h> > > > #include <linux/delay.h> > > > #include <linux/i2c.h> > > > #include <linux/input.h> > > > #include <linux/input/mt.h> > > > #include <linux/interrupt.h> > > > #include <linux/module.h> > > > +#include <linux/mutex.h> > > > #include <linux/slab.h> > > > +#include <linux/uaccess.h> > > > +#include <linux/pm_runtime.h> > > > +#include "cyapa.h" > > > > > > -/* APA trackpad firmware generation */ > > > -#define CYAPA_GEN3 0x03 /* support MT-protocol B with tracking ID. */ > > > - > > > -#define CYAPA_NAME "Cypress APA Trackpad (cyapa)" > > > - > > > -/* commands for read/write registers of Cypress trackpad */ > > > -#define CYAPA_CMD_SOFT_RESET 0x00 > > > -#define CYAPA_CMD_POWER_MODE 0x01 > > > -#define CYAPA_CMD_DEV_STATUS 0x02 > > > -#define CYAPA_CMD_GROUP_DATA 0x03 > > > -#define CYAPA_CMD_GROUP_CMD 0x04 > > > -#define CYAPA_CMD_GROUP_QUERY 0x05 > > > -#define CYAPA_CMD_BL_STATUS 0x06 > > > -#define CYAPA_CMD_BL_HEAD 0x07 > > > -#define CYAPA_CMD_BL_CMD 0x08 > > > -#define CYAPA_CMD_BL_DATA 0x09 > > > -#define CYAPA_CMD_BL_ALL 0x0a > > > -#define CYAPA_CMD_BLK_PRODUCT_ID 0x0b > > > -#define CYAPA_CMD_BLK_HEAD 0x0c > > > - > > > -/* report data start reg offset address. */ > > > -#define DATA_REG_START_OFFSET 0x0000 > > > - > > > -#define BL_HEAD_OFFSET 0x00 > > > -#define BL_DATA_OFFSET 0x10 > > > - > > > -/* > > > - * Operational Device Status Register > > > - * > > > - * bit 7: Valid interrupt source > > > - * bit 6 - 4: Reserved > > > - * bit 3 - 2: Power status > > > - * bit 1 - 0: Device status > > > - */ > > > -#define REG_OP_STATUS 0x00 > > > -#define OP_STATUS_SRC 0x80 > > > -#define OP_STATUS_POWER 0x0c > > > -#define OP_STATUS_DEV 0x03 > > > -#define OP_STATUS_MASK (OP_STATUS_SRC | OP_STATUS_POWER | OP_STATUS_DEV) > > > - > > > -/* > > > - * Operational Finger Count/Button Flags Register > > > - * > > > - * bit 7 - 4: Number of touched finger > > > - * bit 3: Valid data > > > - * bit 2: Middle Physical Button > > > - * bit 1: Right Physical Button > > > - * bit 0: Left physical Button > > > - */ > > > -#define REG_OP_DATA1 0x01 > > > -#define OP_DATA_VALID 0x08 > > > -#define OP_DATA_MIDDLE_BTN 0x04 > > > -#define OP_DATA_RIGHT_BTN 0x02 > > > -#define OP_DATA_LEFT_BTN 0x01 > > > -#define OP_DATA_BTN_MASK (OP_DATA_MIDDLE_BTN | OP_DATA_RIGHT_BTN | \ > > > - OP_DATA_LEFT_BTN) > > > - > > > -/* > > > - * Bootloader Status Register > > > - * > > > - * bit 7: Busy > > > - * bit 6 - 5: Reserved > > > - * bit 4: Bootloader running > > > - * bit 3 - 1: Reserved > > > - * bit 0: Checksum valid > > > - */ > > > -#define REG_BL_STATUS 0x01 > > > -#define BL_STATUS_BUSY 0x80 > > > -#define BL_STATUS_RUNNING 0x10 > > > -#define BL_STATUS_DATA_VALID 0x08 > > > -#define BL_STATUS_CSUM_VALID 0x01 > > > - > > > -/* > > > - * Bootloader Error Register > > > - * > > > - * bit 7: Invalid > > > - * bit 6: Invalid security key > > > - * bit 5: Bootloading > > > - * bit 4: Command checksum > > > - * bit 3: Flash protection error > > > - * bit 2: Flash checksum error > > > - * bit 1 - 0: Reserved > > > - */ > > > -#define REG_BL_ERROR 0x02 > > > -#define BL_ERROR_INVALID 0x80 > > > -#define BL_ERROR_INVALID_KEY 0x40 > > > -#define BL_ERROR_BOOTLOADING 0x20 > > > -#define BL_ERROR_CMD_CSUM 0x10 > > > -#define BL_ERROR_FLASH_PROT 0x08 > > > -#define BL_ERROR_FLASH_CSUM 0x04 > > > - > > > -#define BL_STATUS_SIZE 3 /* length of bootloader status registers */ > > > -#define BLK_HEAD_BYTES 32 > > > - > > > -#define PRODUCT_ID_SIZE 16 > > > -#define QUERY_DATA_SIZE 27 > > > -#define REG_PROTOCOL_GEN_QUERY_OFFSET 20 > > > - > > > -#define REG_OFFSET_DATA_BASE 0x0000 > > > -#define REG_OFFSET_COMMAND_BASE 0x0028 > > > -#define REG_OFFSET_QUERY_BASE 0x002a > > > - > > > -#define CAPABILITY_LEFT_BTN_MASK (0x01 << 3) > > > -#define CAPABILITY_RIGHT_BTN_MASK (0x01 << 4) > > > -#define CAPABILITY_MIDDLE_BTN_MASK (0x01 << 5) > > > -#define CAPABILITY_BTN_MASK (CAPABILITY_LEFT_BTN_MASK | \ > > > - CAPABILITY_RIGHT_BTN_MASK | \ > > > - CAPABILITY_MIDDLE_BTN_MASK) > > > - > > > -#define CYAPA_OFFSET_SOFT_RESET REG_OFFSET_COMMAND_BASE > > > - > > > -#define REG_OFFSET_POWER_MODE (REG_OFFSET_COMMAND_BASE + 1) > > > - > > > -#define PWR_MODE_MASK 0xfc > > > -#define PWR_MODE_FULL_ACTIVE (0x3f << 2) > > > -#define PWR_MODE_IDLE (0x05 << 2) /* default sleep time is 50 ms. */ > > > -#define PWR_MODE_OFF (0x00 << 2) > > > - > > > -#define PWR_STATUS_MASK 0x0c > > > -#define PWR_STATUS_ACTIVE (0x03 << 2) > > > -#define PWR_STATUS_IDLE (0x02 << 2) > > > -#define PWR_STATUS_OFF (0x00 << 2) > > > - > > > -/* > > > - * CYAPA trackpad device states. > > > - * Used in register 0x00, bit1-0, DeviceStatus field. > > > - * Other values indicate device is in an abnormal state and must be reset. > > > - */ > > > -#define CYAPA_DEV_NORMAL 0x03 > > > -#define CYAPA_DEV_BUSY 0x01 > > > - > > > -enum cyapa_state { > > > - CYAPA_STATE_OP, > > > - CYAPA_STATE_BL_IDLE, > > > - CYAPA_STATE_BL_ACTIVE, > > > - CYAPA_STATE_BL_BUSY, > > > - CYAPA_STATE_NO_DEVICE, > > > -}; > > > - > > > - > > > -struct cyapa_touch { > > > - /* > > > - * high bits or x/y position value > > > - * bit 7 - 4: high 4 bits of x position value > > > - * bit 3 - 0: high 4 bits of y position value > > > - */ > > > - u8 xy_hi; > > > - u8 x_lo; /* low 8 bits of x position value. */ > > > - u8 y_lo; /* low 8 bits of y position value. */ > > > - u8 pressure; > > > - /* id range is 1 - 15. It is incremented with every new touch. */ > > > - u8 id; > > > -} __packed; > > > - > > > -/* The touch.id is used as the MT slot id, thus max MT slot is 15 */ > > > -#define CYAPA_MAX_MT_SLOTS 15 > > > - > > > -struct cyapa_reg_data { > > > - /* > > > - * bit 0 - 1: device status > > > - * bit 3 - 2: power mode > > > - * bit 6 - 4: reserved > > > - * bit 7: interrupt valid bit > > > - */ > > > - u8 device_status; > > > - /* > > > - * bit 7 - 4: number of fingers currently touching pad > > > - * bit 3: valid data check bit > > > - * bit 2: middle mechanism button state if exists > > > - * bit 1: right mechanism button state if exists > > > - * bit 0: left mechanism button state if exists > > > - */ > > > - u8 finger_btn; > > > - /* CYAPA reports up to 5 touches per packet. */ > > > - struct cyapa_touch touches[5]; > > > -} __packed; > > > - > > > -/* The main device structure */ > > > -struct cyapa { > > > - enum cyapa_state state; > > > - > > > - struct i2c_client *client; > > > - struct input_dev *input; > > > - char phys[32]; /* device physical location */ > > > - int irq; > > > - bool irq_wake; /* irq wake is enabled */ > > > - bool smbus; > > > - > > > - /* read from query data region. */ > > > - char product_id[16]; > > > - u8 btn_capability; > > > - u8 gen; > > > - int max_abs_x; > > > - int max_abs_y; > > > - int physical_size_x; > > > - int physical_size_y; > > > -}; > > > - > > > -static const u8 bl_deactivate[] = { 0x00, 0xff, 0x3b, 0x00, 0x01, 0x02, > > 0x03, > > > - 0x04, 0x05, 0x06, 0x07 }; > > > -static const u8 bl_exit[] = { 0x00, 0xff, 0xa5, 0x00, 0x01, 0x02, 0x03, > > 0x04, > > > - 0x05, 0x06, 0x07 }; > > > - > > > -struct cyapa_cmd_len { > > > - u8 cmd; > > > - u8 len; > > > -}; > > > > > > #define CYAPA_ADAPTER_FUNC_NONE 0 > > > #define CYAPA_ADAPTER_FUNC_I2C 1 > > > #define CYAPA_ADAPTER_FUNC_SMBUS 2 > > > #define CYAPA_ADAPTER_FUNC_BOTH 3 > > > > > > -/* > > > - * macros for SMBus communication > > > - */ > > > -#define SMBUS_READ 0x01 > > > -#define SMBUS_WRITE 0x00 > > > -#define SMBUS_ENCODE_IDX(cmd, idx) ((cmd) | (((idx) & 0x03) << 1)) > > > -#define SMBUS_ENCODE_RW(cmd, rw) ((cmd) | ((rw) & 0x01)) > > > -#define SMBUS_BYTE_BLOCK_CMD_MASK 0x80 > > > -#define SMBUS_GROUP_BLOCK_CMD_MASK 0x40 > > > - > > > - /* for byte read/write command */ > > > -#define CMD_RESET 0 > > > -#define CMD_POWER_MODE 1 > > > -#define CMD_DEV_STATUS 2 > > > -#define SMBUS_BYTE_CMD(cmd) (((cmd) & 0x3f) << 1) > > > -#define CYAPA_SMBUS_RESET SMBUS_BYTE_CMD(CMD_RESET) > > > -#define CYAPA_SMBUS_POWER_MODE SMBUS_BYTE_CMD(CMD_POWER_MODE) > > > -#define CYAPA_SMBUS_DEV_STATUS SMBUS_BYTE_CMD(CMD_DEV_STATUS) > > > - > > > - /* for group registers read/write command */ > > > -#define REG_GROUP_DATA 0 > > > -#define REG_GROUP_CMD 2 > > > -#define REG_GROUP_QUERY 3 > > > -#define SMBUS_GROUP_CMD(grp) (0x80 | (((grp) & 0x07) << 3)) > > > -#define CYAPA_SMBUS_GROUP_DATA SMBUS_GROUP_CMD(REG_GROUP_DATA) > > > -#define CYAPA_SMBUS_GROUP_CMD SMBUS_GROUP_CMD(REG_GROUP_CMD) > > > -#define CYAPA_SMBUS_GROUP_QUERY SMBUS_GROUP_CMD(REG_GROUP_QUERY) > > > - > > > - /* for register block read/write command */ > > > -#define CMD_BL_STATUS 0 > > > -#define CMD_BL_HEAD 1 > > > -#define CMD_BL_CMD 2 > > > -#define CMD_BL_DATA 3 > > > -#define CMD_BL_ALL 4 > > > -#define CMD_BLK_PRODUCT_ID 5 > > > -#define CMD_BLK_HEAD 6 > > > -#define SMBUS_BLOCK_CMD(cmd) (0xc0 | (((cmd) & 0x1f) << 1)) > > > - > > > -/* register block read/write command in bootloader mode */ > > > -#define CYAPA_SMBUS_BL_STATUS SMBUS_BLOCK_CMD(CMD_BL_STATUS) > > > -#define CYAPA_SMBUS_BL_HEAD SMBUS_BLOCK_CMD(CMD_BL_HEAD) > > > -#define CYAPA_SMBUS_BL_CMD SMBUS_BLOCK_CMD(CMD_BL_CMD) > > > -#define CYAPA_SMBUS_BL_DATA SMBUS_BLOCK_CMD(CMD_BL_DATA) > > > -#define CYAPA_SMBUS_BL_ALL SMBUS_BLOCK_CMD(CMD_BL_ALL) > > > - > > > -/* register block read/write command in operational mode */ > > > -#define CYAPA_SMBUS_BLK_PRODUCT_ID SMBUS_BLOCK_CMD(CMD_BLK_PRODUCT_ID) > > > -#define CYAPA_SMBUS_BLK_HEAD SMBUS_BLOCK_CMD(CMD_BLK_HEAD) > > > - > > > -static const struct cyapa_cmd_len cyapa_i2c_cmds[] = { > > > - { CYAPA_OFFSET_SOFT_RESET, 1 }, > > > - { REG_OFFSET_COMMAND_BASE + 1, 1 }, > > > - { REG_OFFSET_DATA_BASE, 1 }, > > > - { REG_OFFSET_DATA_BASE, sizeof(struct cyapa_reg_data) }, > > > - { REG_OFFSET_COMMAND_BASE, 0 }, > > > - { REG_OFFSET_QUERY_BASE, QUERY_DATA_SIZE }, > > > - { BL_HEAD_OFFSET, 3 }, > > > - { BL_HEAD_OFFSET, 16 }, > > > - { BL_HEAD_OFFSET, 16 }, > > > - { BL_DATA_OFFSET, 16 }, > > > - { BL_HEAD_OFFSET, 32 }, > > > - { REG_OFFSET_QUERY_BASE, PRODUCT_ID_SIZE }, > > > - { REG_OFFSET_DATA_BASE, 32 } > > > -}; > > > +#define CYAPA_DEBUGFS_READ_FW "read_fw" > > > +#define CYAPA_DEBUGFS_RAW_DATA "raw_data" > > > +#define CYAPA_FW_NAME "cyapa.bin" > > > + > > > +const char unique_str[] = "CYTRA"; > > > + > > > > > > -static const struct cyapa_cmd_len cyapa_smbus_cmds[] = { > > > - { CYAPA_SMBUS_RESET, 1 }, > > > - { CYAPA_SMBUS_POWER_MODE, 1 }, > > > - { CYAPA_SMBUS_DEV_STATUS, 1 }, > > > - { CYAPA_SMBUS_GROUP_DATA, sizeof(struct cyapa_reg_data) }, > > > - { CYAPA_SMBUS_GROUP_CMD, 2 }, > > > - { CYAPA_SMBUS_GROUP_QUERY, QUERY_DATA_SIZE }, > > > - { CYAPA_SMBUS_BL_STATUS, 3 }, > > > - { CYAPA_SMBUS_BL_HEAD, 16 }, > > > - { CYAPA_SMBUS_BL_CMD, 16 }, > > > - { CYAPA_SMBUS_BL_DATA, 16 }, > > > - { CYAPA_SMBUS_BL_ALL, 32 }, > > > - { CYAPA_SMBUS_BLK_PRODUCT_ID, PRODUCT_ID_SIZE }, > > > - { CYAPA_SMBUS_BLK_HEAD, 16 }, > > > -}; > > > > > > -static ssize_t cyapa_i2c_reg_read_block(struct cyapa *cyapa, u8 reg, size_t > > len, > > > +ssize_t cyapa_i2c_reg_read_block(struct cyapa *cyapa, u8 reg, size_t len, > > > u8 *values) > > > { > > > return i2c_smbus_read_i2c_block_data(cyapa->client, reg, len, values); > > > } > > > > > > -static ssize_t cyapa_i2c_reg_write_block(struct cyapa *cyapa, u8 reg, > > > +ssize_t cyapa_i2c_reg_write_block(struct cyapa *cyapa, u8 reg, > > > size_t len, const u8 *values) > > > { > > > return i2c_smbus_write_i2c_block_data(cyapa->client, reg, len, values); > > > } > > > > > > -/* > > > - * cyapa_smbus_read_block - perform smbus block read command > > > - * @cyapa - private data structure of the driver > > > - * @cmd - the properly encoded smbus command > > > - * @len - expected length of smbus command result > > > - * @values - buffer to store smbus command result > > > - * > > > - * Returns negative errno, else the number of bytes written. > > > - * > > > - * Note: > > > - * In trackpad device, the memory block allocated for I2C register map > > > - * is 256 bytes, so the max read block for I2C bus is 256 bytes. > > > - */ > > > -static ssize_t cyapa_smbus_read_block(struct cyapa *cyapa, u8 cmd, size_t > > len, > > > - u8 *values) > > > -{ > > > - ssize_t ret; > > > - u8 index; > > > - u8 smbus_cmd; > > > - u8 *buf; > > > - struct i2c_client *client = cyapa->client; > > > - > > > - if (!(SMBUS_BYTE_BLOCK_CMD_MASK & cmd)) > > > - return -EINVAL; > > > - > > > - if (SMBUS_GROUP_BLOCK_CMD_MASK & cmd) { > > > - /* read specific block registers command. */ > > > - smbus_cmd = SMBUS_ENCODE_RW(cmd, SMBUS_READ); > > > - ret = i2c_smbus_read_block_data(client, smbus_cmd, values); > > > - goto out; > > > - } > > > - > > > - ret = 0; > > > - for (index = 0; index * I2C_SMBUS_BLOCK_MAX < len; index++) { > > > - smbus_cmd = SMBUS_ENCODE_IDX(cmd, index); > > > - smbus_cmd = SMBUS_ENCODE_RW(smbus_cmd, SMBUS_READ); > > > - buf = values + I2C_SMBUS_BLOCK_MAX * index; > > > - ret = i2c_smbus_read_block_data(client, smbus_cmd, buf); > > > - if (ret < 0) > > > - goto out; > > > - } > > > - > > > -out: > > > - return ret > 0 ? len : ret; > > > -} > > > - > > > -static s32 cyapa_read_byte(struct cyapa *cyapa, u8 cmd_idx) > > > -{ > > > - u8 cmd; > > > - > > > - if (cyapa->smbus) { > > > - cmd = cyapa_smbus_cmds[cmd_idx].cmd; > > > - cmd = SMBUS_ENCODE_RW(cmd, SMBUS_READ); > > > - } else { > > > - cmd = cyapa_i2c_cmds[cmd_idx].cmd; > > > - } > > > - return i2c_smbus_read_byte_data(cyapa->client, cmd); > > > -} > > > - > > > -static s32 cyapa_write_byte(struct cyapa *cyapa, u8 cmd_idx, u8 value) > > > -{ > > > - u8 cmd; > > > - > > > - if (cyapa->smbus) { > > > - cmd = cyapa_smbus_cmds[cmd_idx].cmd; > > > - cmd = SMBUS_ENCODE_RW(cmd, SMBUS_WRITE); > > > - } else { > > > - cmd = cyapa_i2c_cmds[cmd_idx].cmd; > > > - } > > > - return i2c_smbus_write_byte_data(cyapa->client, cmd, value); > > > -} > > > - > > > -static ssize_t cyapa_read_block(struct cyapa *cyapa, u8 cmd_idx, u8 *values) > > > -{ > > > - u8 cmd; > > > - size_t len; > > > - > > > - if (cyapa->smbus) { > > > - cmd = cyapa_smbus_cmds[cmd_idx].cmd; > > > - len = cyapa_smbus_cmds[cmd_idx].len; > > > - return cyapa_smbus_read_block(cyapa, cmd, len, values); > > > - } else { > > > - cmd = cyapa_i2c_cmds[cmd_idx].cmd; > > > - len = cyapa_i2c_cmds[cmd_idx].len; > > > - return cyapa_i2c_reg_read_block(cyapa, cmd, len, values); > > > - } > > > -} > > > - > > > -/* > > > - * Query device for its current operating state. > > > - * > > > - */ > > > -static int cyapa_get_state(struct cyapa *cyapa) > > > +/* Returns 0 on success, else negative errno on failure. */ > > > +ssize_t cyapa_i2c_read(struct cyapa *cyapa, u8 reg, size_t len, > > > + u8 *values) > > > { > > > int ret; > > > - u8 status[BL_STATUS_SIZE]; > > > - > > > - cyapa->state = CYAPA_STATE_NO_DEVICE; > > > - > > > - /* > > > - * Get trackpad status by reading 3 registers starting from 0. > > > - * If the device is in the bootloader, this will be BL_HEAD. > > > - * If the device is in operation mode, this will be the DATA regs. > > > - * > > > - */ > > > - ret = cyapa_i2c_reg_read_block(cyapa, BL_HEAD_OFFSET, BL_STATUS_SIZE, > > > - status); > > > - > > > - /* > > > - * On smbus systems in OP mode, the i2c_reg_read will fail with > > > - * -ETIMEDOUT. In this case, try again using the smbus equivalent > > > - * command. This should return a BL_HEAD indicating CYAPA_STATE_OP. > > > - */ > > > - if (cyapa->smbus && (ret == -ETIMEDOUT || ret == -ENXIO)) > > > - ret = cyapa_read_block(cyapa, CYAPA_CMD_BL_STATUS, status); > > > - > > > - if (ret != BL_STATUS_SIZE) > > > - goto error; > > > - > > > - if ((status[REG_OP_STATUS] & OP_STATUS_SRC) == OP_STATUS_SRC) { > > > - switch (status[REG_OP_STATUS] & OP_STATUS_DEV) { > > > - case CYAPA_DEV_NORMAL: > > > - case CYAPA_DEV_BUSY: > > > - cyapa->state = CYAPA_STATE_OP; > > > - break; > > > - default: > > > - ret = -EAGAIN; > > > - goto error; > > > - } > > > - } else { > > > - if (status[REG_BL_STATUS] & BL_STATUS_BUSY) > > > - cyapa->state = CYAPA_STATE_BL_BUSY; > > > - else if (status[REG_BL_ERROR] & BL_ERROR_BOOTLOADING) > > > - cyapa->state = CYAPA_STATE_BL_ACTIVE; > > > - else > > > - cyapa->state = CYAPA_STATE_BL_IDLE; > > > - } > > > - > > > - return 0; > > > -error: > > > - return (ret < 0) ? ret : -EAGAIN; > > > + struct i2c_client *client = cyapa->client; > > > + struct i2c_msg msgs[] = { > > > + { > > > + .addr = client->addr, > > > + .flags = 0, > > > + .len = 1, > > > + .buf = ®, > > > + }, > > > + { > > > + .addr = client->addr, > > > + .flags = I2C_M_RD, > > > + .len = len, > > > + .buf = values, > > > + }, > > > + }; > > > + > > > + ret = i2c_transfer(client->adapter, msgs, 2); > > > > You need to watch out for partial transfers. It should be: > > > > if (ret != ARRAY_SIZE(msgs)) > > return ret < 0 ? ret : -EIO; > > > > return 0; > > > > Thanks, I will update in next release. > > > > + > > > + return ret < 0 ? ret : 0; > > > } > > > > > > -/* > > > - * Poll device for its status in a loop, waiting up to timeout for a > > response. > > > - * > > > - * When the device switches state, it usually takes ~300 ms. > > > - * However, when running a new firmware image, the device must calibrate > > its > > > - * sensors, which can take as long as 2 seconds. > > > +/** > > > + * cyapa_i2c_write - Execute i2c block data write operation > > > + * @cyapa: Handle to this driver > > > + * @ret: Offset of the data to written in the register map > > > + * @len: number of bytes to write > > > + * @values: Data to be written > > > * > > > - * Note: The timeout has granularity of the polling rate, which is 100 ms. > > > - * > > > - * Returns: > > > - * 0 when the device eventually responds with a valid non-busy state. > > > - * -ETIMEDOUT if device never responds (too many -EAGAIN) > > > - * < 0 other errors > > > + * Return negative errno code on error; return zero when success. > > > */ > > > -static int cyapa_poll_state(struct cyapa *cyapa, unsigned int timeout) > > > +ssize_t cyapa_i2c_write(struct cyapa *cyapa, u8 reg, > > > + size_t len, const void *values) > > > { > > > int ret; > > > - int tries = timeout / 100; > > > - > > > - ret = cyapa_get_state(cyapa); > > > - while ((ret || cyapa->state >= CYAPA_STATE_BL_BUSY) && tries--) { > > > - msleep(100); > > > - ret = cyapa_get_state(cyapa); > > > - } > > > - return (ret == -EAGAIN || ret == -ETIMEDOUT) ? -ETIMEDOUT : ret; > > > -} > > > + struct i2c_client *client = cyapa->client; > > > + char data[32], *buf; > > > > > > -static int cyapa_bl_deactivate(struct cyapa *cyapa) > > > -{ > > > - int ret; > > > + if (len > 31) > > > + buf = kzalloc(len + 1, GFP_KERNEL); > > > + else > > > + buf = data; > > > > > > - ret = cyapa_i2c_reg_write_block(cyapa, 0, sizeof(bl_deactivate), > > > - bl_deactivate); > > > - if (ret < 0) > > > - return ret; > > > + buf[0] = reg; > > > + memcpy(&buf[1], values, len); > > > + ret = i2c_master_send(client, buf, len + 1); > > > > > > - /* wait for bootloader to switch to idle state; should take < 100ms */ > > > - msleep(100); > > > - ret = cyapa_poll_state(cyapa, 500); > > > - if (ret < 0) > > > - return ret; > > > - if (cyapa->state != CYAPA_STATE_BL_IDLE) > > > - return -EAGAIN; > > > - return 0; > > > + if (buf != data) > > > + kfree(buf); > > > + return (ret == (len + 1)) ? 0 : ((ret < 0) ? ret : -EIO); > > > } > > > > > > -/* > > > - * Exit bootloader > > > - * > > > - * Send bl_exit command, then wait 50 - 100 ms to let device transition to > > > - * operational mode. If this is the first time the device's firmware is > > > - * running, it can take up to 2 seconds to calibrate its sensors. So, poll > > > - * the device's new state for up to 2 seconds. > > > - * > > > - * Returns: > > > - * -EIO failure while reading from device > > > - * -EAGAIN device is stuck in bootloader, b/c it has invalid firmware > > > - * 0 device is supported and in operational mode > > > - */ > > > -static int cyapa_bl_exit(struct cyapa *cyapa) > > > +void cyapa_default_irq_handler(struct cyapa *cyapa) > > > { > > > - int ret; > > > - > > > - ret = cyapa_i2c_reg_write_block(cyapa, 0, sizeof(bl_exit), bl_exit); > > > - if (ret < 0) > > > - return ret; > > > - > > > - /* > > > - * Wait for bootloader to exit, and operation mode to start. > > > - * Normally, this takes at least 50 ms. > > > - */ > > > - usleep_range(50000, 100000); > > > /* > > > - * In addition, when a device boots for the first time after being > > > - * updated to new firmware, it must first calibrate its sensors, which > > > - * can take up to an additional 2 seconds. > > > + * Do redetecting when device states is still unknown and > > > + * interrupt envent is received from device. > > > */ > > > - ret = cyapa_poll_state(cyapa, 2000); > > > - if (ret < 0) > > > - return ret; > > > - if (cyapa->state != CYAPA_STATE_OP) > > > - return -EAGAIN; > > > - > > > - return 0; > > > -} > > > - > > > -/* > > > - * Set device power mode > > > - * > > > - */ > > > -static int cyapa_set_power_mode(struct cyapa *cyapa, u8 power_mode) > > > -{ > > > - struct device *dev = &cyapa->client->dev; > > > - int ret; > > > - u8 power; > > > - > > > - if (cyapa->state != CYAPA_STATE_OP) > > > - return 0; > > > - > > > - ret = cyapa_read_byte(cyapa, CYAPA_CMD_POWER_MODE); > > > - if (ret < 0) > > > - return ret; > > > - > > > - power = ret & ~PWR_MODE_MASK; > > > - power |= power_mode & PWR_MODE_MASK; > > > - ret = cyapa_write_byte(cyapa, CYAPA_CMD_POWER_MODE, power); > > > - if (ret < 0) > > > - dev_err(dev, "failed to set power_mode 0x%02x err = %d\n", > > > - power_mode, ret); > > > - return ret; > > > -} > > > - > > > -static int cyapa_get_query_data(struct cyapa *cyapa) > > > -{ > > > - u8 query_data[QUERY_DATA_SIZE]; > > > - int ret; > > > - > > > - if (cyapa->state != CYAPA_STATE_OP) > > > - return -EBUSY; > > > - > > > - ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_QUERY, query_data); > > > - if (ret < 0) > > > - return ret; > > > - if (ret != QUERY_DATA_SIZE) > > > - return -EIO; > > > - > > > - memcpy(&cyapa->product_id[0], &query_data[0], 5); > > > - cyapa->product_id[5] = '-'; > > > - memcpy(&cyapa->product_id[6], &query_data[5], 6); > > > - cyapa->product_id[12] = '-'; > > > - memcpy(&cyapa->product_id[13], &query_data[11], 2); > > > - cyapa->product_id[15] = '\0'; > > > - > > > - cyapa->btn_capability = query_data[19] & CAPABILITY_BTN_MASK; > > > - > > > - cyapa->gen = query_data[20] & 0x0f; > > > - > > > - cyapa->max_abs_x = ((query_data[21] & 0xf0) << 4) | query_data[22]; > > > - cyapa->max_abs_y = ((query_data[21] & 0x0f) << 8) | query_data[23]; > > > - > > > - cyapa->physical_size_x = > > > - ((query_data[24] & 0xf0) << 4) | query_data[25]; > > > - cyapa->physical_size_y = > > > - ((query_data[24] & 0x0f) << 8) | query_data[26]; > > > - > > > - return 0; > > > + async_schedule(cyapa_detect_async, cyapa); > > > > I think I already asked this before - why do we need to try and schedule async > > detect from interrupt handler. In what cases we will fail to initialize the > > device during normal probing, but then are fine when stray interrupt arrives? > > > > 1) Because gen5 TP devices use interrupt to sync the command and response, and > in detecting, some commands must be sent and executed in attached devices, so > the interrupt must be usable in detecting. > So if do not schedule async detect from interrupt handler, the interrupt > handler will be taken, and cannot enter again, which will cause the command to > the device failed, it will also cause long time detecting. > 2) detecting will fail when no device attached or the attached device is not > supported gen3 or gen5 TP devices or there is some issue with the device > that cannot enter into active working mode or stay in bootloader mode > for invalid firmware detected. > And it's tested that it's save when stray interrupt arrives in detecting. I am sorry, I have trouble parsing this. I understand that you may need interrupt to know when command response is ready, but I do not see how kicking asynchronous detect helps there. During probe you can figure out if you are talking to a Cypress device and whether it is operable. If it is not operable you can try flashing a new firmware and then kick off detection again after flash is successful. But I do not see any reason in trying to re-detect the device after random interrupt arrives in hopes that maybe this time stars will align and we'll be able to work with it. > > Thanks. > > > > > } > > > > > > -/* > > > - * Check if device is operational. > > > - * > > > - * An operational device is responding, has exited bootloader, and has > > > - * firmware supported by this driver. > > > - * > > > - * Returns: > > > - * -EBUSY no device or in bootloader > > > - * -EIO failure while reading from device > > > - * -EAGAIN device is still in bootloader > > > - * if ->state = CYAPA_STATE_BL_IDLE, device has invalid firmware > > > - * -EINVAL device is in operational mode, but not supported by this > > driver > > > - * 0 device is supported > > > - */ > > > -static int cyapa_check_is_operational(struct cyapa *cyapa) > > > -{ > > > - struct device *dev = &cyapa->client->dev; > > > - static const char unique_str[] = "CYTRA"; > > > - int ret; > > > - > > > - ret = cyapa_poll_state(cyapa, 2000); > > > - if (ret < 0) > > > - return ret; > > > - switch (cyapa->state) { > > > - case CYAPA_STATE_BL_ACTIVE: > > > - ret = cyapa_bl_deactivate(cyapa); > > > - if (ret) > > > - return ret; > > > - > > > - /* Fallthrough state */ > > > - case CYAPA_STATE_BL_IDLE: > > > - ret = cyapa_bl_exit(cyapa); > > > - if (ret) > > > - return ret; > > > - > > > - /* Fallthrough state */ > > > - case CYAPA_STATE_OP: > > > - ret = cyapa_get_query_data(cyapa); > > > - if (ret < 0) > > > - return ret; > > > - > > > - /* only support firmware protocol gen3 */ > > > - if (cyapa->gen != CYAPA_GEN3) { > > > - dev_err(dev, "unsupported protocol version (%d)", > > > - cyapa->gen); > > > - return -EINVAL; > > > - } > > > - > > > - /* only support product ID starting with CYTRA */ > > > - if (memcmp(cyapa->product_id, unique_str, > > > - sizeof(unique_str) - 1) != 0) { > > > - dev_err(dev, "unsupported product ID (%s)\n", > > > - cyapa->product_id); > > > - return -EINVAL; > > > - } > > > - return 0; > > > - > > > - default: > > > - return -EIO; > > > - } > > > - return 0; > > > -} > > > - > > > -static irqreturn_t cyapa_irq(int irq, void *dev_id) > > > -{ > > > - struct cyapa *cyapa = dev_id; > > > - struct device *dev = &cyapa->client->dev; > > > - struct input_dev *input = cyapa->input; > > > - struct cyapa_reg_data data; > > > - int i; > > > - int ret; > > > - int num_fingers; > > > - > > > - if (device_may_wakeup(dev)) > > > - pm_wakeup_event(dev, 0); > > > - > > > - ret = cyapa_read_block(cyapa, CYAPA_CMD_GROUP_DATA, (u8 *)&data); > > > - if (ret != sizeof(data)) > > > - goto out; > > > - > > > - if ((data.device_status & OP_STATUS_SRC) != OP_STATUS_SRC || > > > - (data.device_status & OP_STATUS_DEV) != CYAPA_DEV_NORMAL || > > > - (data.finger_btn & OP_DATA_VALID) != OP_DATA_VALID) { > > > - goto out; > > > - } > > > - > > > - num_fingers = (data.finger_btn >> 4) & 0x0f; > > > - for (i = 0; i < num_fingers; i++) { > > > - const struct cyapa_touch *touch = &data.touches[i]; > > > - /* Note: touch->id range is 1 to 15; slots are 0 to 14. */ > > > - int slot = touch->id - 1; > > > - > > > - input_mt_slot(input, slot); > > > - input_mt_report_slot_state(input, MT_TOOL_FINGER, true); > > > - input_report_abs(input, ABS_MT_POSITION_X, > > > - ((touch->xy_hi & 0xf0) << 4) | touch->x_lo); > > > - input_report_abs(input, ABS_MT_POSITION_Y, > > > - ((touch->xy_hi & 0x0f) << 8) | touch->y_lo); > > > - input_report_abs(input, ABS_MT_PRESSURE, touch->pressure); > > > - } > > > - > > > - input_mt_sync_frame(input); > > > - > > > - if (cyapa->btn_capability & CAPABILITY_LEFT_BTN_MASK) > > > - input_report_key(input, BTN_LEFT, > > > - data.finger_btn & OP_DATA_LEFT_BTN); > > > - > > > - if (cyapa->btn_capability & CAPABILITY_MIDDLE_BTN_MASK) > > > - input_report_key(input, BTN_MIDDLE, > > > - data.finger_btn & OP_DATA_MIDDLE_BTN); > > > - > > > - if (cyapa->btn_capability & CAPABILITY_RIGHT_BTN_MASK) > > > - input_report_key(input, BTN_RIGHT, > > > - data.finger_btn & OP_DATA_RIGHT_BTN); > > > - > > > - input_sync(input); > > > +const struct cyapa_dev_ops cyapa_default_ops = { > > > + .irq_handler = cyapa_default_irq_handler > > > +}; > > > > > > -out: > > > - return IRQ_HANDLED; > > > -} > > > > > > static u8 cyapa_check_adapter_functionality(struct i2c_client *client) > > > { > > > @@ -772,19 +155,40 @@ static int cyapa_create_input_dev(struct cyapa *cyapa) > > > input->phys = cyapa->phys; > > > input->id.bustype = BUS_I2C; > > > input->id.version = 1; > > > - input->id.product = 0; /* means any product in eventcomm. */ > > > + input->id.product = 0; /* Means any product in eventcomm. */ > > > input->dev.parent = &cyapa->client->dev; > > > > > > input_set_drvdata(input, cyapa); > > > > > > __set_bit(EV_ABS, input->evbit); > > > > > > - /* finger position */ > > > + /* Finger position */ > > > input_set_abs_params(input, ABS_MT_POSITION_X, 0, cyapa->max_abs_x, 0, > > > 0); > > > input_set_abs_params(input, ABS_MT_POSITION_Y, 0, cyapa->max_abs_y, 0, > > > 0); > > > - input_set_abs_params(input, ABS_MT_PRESSURE, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_MT_PRESSURE, 0, cyapa->max_z, 0, 0); > > > + if (cyapa->gen > CYAPA_GEN3) { > > > + input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 255, 0, 0); > > > + /* > > > + * Orientation is the angle between the vertical axis and > > > + * the major axis of the contact ellipse. > > > + * The range is -127 to 127. > > > + * the positive direction is clockwise form the vertical axis. > > > + * If the ellipse of contact degenerates into a circle, > > > + * orientation is reported as 0. > > > + * > > > + * Also, for Gen5 trackpad the accurate of this orientation > > > + * value is value + (-30 ~ 30). > > > + */ > > > + input_set_abs_params(input, ABS_MT_ORIENTATION, > > > + -127, 127, 0, 0); > > > + } > > > + if (cyapa->gen >= CYAPA_GEN5) { > > > + input_set_abs_params(input, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0); > > > + input_set_abs_params(input, ABS_MT_WIDTH_MINOR, 0, 255, 0, 0); > > > + } > > > > > > input_abs_set_res(input, ABS_MT_POSITION_X, > > > cyapa->max_abs_x / cyapa->physical_size_x); > > > @@ -801,7 +205,7 @@ static int cyapa_create_input_dev(struct cyapa *cyapa) > > > if (cyapa->btn_capability == CAPABILITY_LEFT_BTN_MASK) > > > __set_bit(INPUT_PROP_BUTTONPAD, input->propbit); > > > > > > - /* handle pointer emulation and unused slots in core */ > > > + /* Handle pointer emulation and unused slots in core */ > > > ret = input_mt_init_slots(input, CYAPA_MAX_MT_SLOTS, > > > INPUT_MT_POINTER | INPUT_MT_DROP_UNUSED); > > > if (ret) { > > > @@ -823,6 +227,224 @@ err_free_device: > > > return ret; > > > } > > > > > > +/* > > > + * Check if device is operational. > > > + * > > > + * An operational device is responding, has exited bootloader, and has > > > + * firmware supported by this driver. > > > + * > > > + * Returns: > > > + * -EBUSY no device or in bootloader > > > + * -EIO failure while reading from device > > > + * -EAGAIN device is still in bootloader > > > + * if ->state = CYAPA_STATE_BL_IDLE, device has invalid firmware > > > + * -EINVAL device is in operational mode, but not supported by this > > driver > > > + * 0 device is supported > > > + */ > > > +static int cyapa_check_is_operational(struct cyapa *cyapa) > > > +{ > > > + int ret; > > > + > > > + ret = cyapa_poll_state(cyapa, 4000); > > > + if (ret) > > > + return ret; > > > + > > > + switch (cyapa->gen) { > > > + default: > > > + cyapa->ops = &cyapa_default_ops; > > > + cyapa->gen = CYAPA_GEN_UNKNOWN; > > > + break; > > > + } > > > + > > > + if (cyapa->ops->operational_check) > > > + ret = cyapa->ops->operational_check(cyapa); > > > + else > > > + ret = -EBUSY; > > > + > > > + return ret; > > > +} > > > + > > > + > > > +static irqreturn_t cyapa_irq(int irq, void *dev_id) > > > +{ > > > + struct cyapa *cyapa = dev_id; > > > + struct input_dev *input = cyapa->input; > > > + bool cont; > > > + > > > + /* Interrupt event maybe cuased by host command to trackpad device. */ > > > + cont = true; > > > + if (cyapa->ops->irq_cmd_handler) > > > + cont = cyapa->ops->irq_cmd_handler(cyapa); > > > + > > > + /* Interrupt event maybe from trackpad device input reporting. */ > > > + if (cont && cyapa->ops->irq_handler) { > > > + if (!mutex_trylock(&cyapa->state_sync_lock)) { > > > + if (cyapa->ops->sort_empty_output_data) > > > + cyapa->ops->sort_empty_output_data(cyapa, > > > + NULL, NULL, NULL); > > > + goto out; > > > + } > > > + > > > + if (!input) { > > > + if (cyapa->ops->sort_empty_output_data) > > > + cyapa->ops->sort_empty_output_data(cyapa, > > > + NULL, NULL, NULL); > > > + } else > > > + cyapa->ops->irq_handler(cyapa); > > > + > > > + mutex_unlock(&cyapa->state_sync_lock); > > > + } > > > +out: > > > + return IRQ_HANDLED; > > > +} > > > + > > > +/* > > > + * Query device for its current operating state. > > > + */ > > > +static int cyapa_get_state(struct cyapa *cyapa) > > > +{ > > > + cyapa->state = CYAPA_STATE_NO_DEVICE; > > > + > > > + return -ENODEV; > > > +} > > > + > > > +/* > > > + * Poll device for its status in a loop, waiting up to timeout for a > > response. > > > + * > > > + * When the device switches state, it usually takes ~300 ms. > > > + * However, when running a new firmware image, the device must calibrate > > its > > > + * sensors, which can take as long as 2 seconds. > > > + * > > > + * Note: The timeout has granularity of the polling rate, which is 100 ms. > > > + * > > > + * Returns: > > > + * 0 when the device eventually responds with a valid non-busy state. > > > + * -ETIMEDOUT if device never responds (too many -EAGAIN) > > > + * < 0 other errors > > > + */ > > > +int cyapa_poll_state(struct cyapa *cyapa, unsigned int timeout) > > > +{ > > > + int ret; > > > + int tries = timeout / 100; > > > + > > > + ret = cyapa_get_state(cyapa); > > > + while ((ret || cyapa->state >= CYAPA_STATE_BL_BUSY) && tries--) { > > > + msleep(100); > > > + ret = cyapa_get_state(cyapa); > > > + } > > > + > > > + return (ret == -EAGAIN || ret == -ETIMEDOUT) ? -ETIMEDOUT : ret; > > > +} > > > + > > > +static void cyapa_detect(struct cyapa *cyapa) > > > +{ > > > + struct device *dev = &cyapa->client->dev; > > > + char *envp[2] = {"ERROR=1", NULL}; > > > + int ret; > > > + > > > + ret = cyapa_check_is_operational(cyapa); > > > + if (ret == -ETIMEDOUT) > > > + dev_err(dev, "no device detected, %d\n", ret); > > > + else if (ret) > > > + dev_err(dev, "device detected, but not operational, %d\n", ret); > > > + > > > + if (ret) { > > > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp); > > > + return; > > > + } > > > + > > > + if (!cyapa->input) { > > > + ret = cyapa_create_input_dev(cyapa); > > > + if (ret) > > > + dev_err(dev, "create input_dev instance failed, %d\n", > > > + ret); > > > + > > > + /* > > > + * On some systems, a system crash / warm boot does not reset > > > + * the device's current power mode to FULL_ACTIVE. > > > + * If such an event happens during suspend, after the device > > > + * has been put in a low power mode, the device will still be > > > + * in low power mode on a subsequent boot, since there was > > > + * never a matching resume(). > > > + * Handle this by always forcing full power here, when a > > > + * device is first detected to be in operational mode. > > > + */ > > > + if (cyapa->ops->set_power_mode) { > > > + ret = cyapa->ops->set_power_mode(cyapa, > > > + PWR_MODE_FULL_ACTIVE, 0); > > > + if (ret) > > > + dev_warn(dev, "set active power failed, %d\n", > > > + ret); > > > + } > > > + } > > > +} > > > + > > > +/* > > > + * Sysfs Interface. > > > + */ > > > + > > > +/* > > > + * cyapa_sleep_time_to_pwr_cmd and cyapa_pwr_cmd_to_sleep_time > > > + * > > > + * These are helper functions that convert to and from integer idle > > > + * times and register settings to write to the PowerMode register. > > > + * The trackpad supports between 20ms to 1000ms scan intervals. > > > + * The time will be increased in increments of 10ms from 20ms to 100ms. > > > + * From 100ms to 1000ms, time will be increased in increments of 20ms. > > > + * > > > + * When Idle_Time < 100, the format to convert Idle_Time to Idle_Command is: > > > + * Idle_Command = Idle Time / 10; > > > + * When Idle_Time >= 100, the format to convert Idle_Time to Idle_Command > > is: > > > + * Idle_Command = Idle Time / 20 + 5; > > > + */ > > > +u8 cyapa_sleep_time_to_pwr_cmd(u16 sleep_time) > > > +{ > > > + if (sleep_time < 20) > > > + sleep_time = 20; /* minimal sleep time. */ > > > + else if (sleep_time > 1000) > > > + sleep_time = 1000; /* maximal sleep time. */ > > > > clamp_val() > > Thanks. clamp_pwr_cmd_sleep_time() will be used to above code. No, we have a macro called clamp_val() which does exactly what you are doing here by hand. So please use sleep_time = clamp_val(sleep_time, 20, 1000); > > > > > > + > > > + if (sleep_time < 100) > > > + return ((sleep_time / 10) << 2) & PWR_MODE_MASK; > > > + else > > > + return ((sleep_time / 20 + 5) << 2) & PWR_MODE_MASK; > > > +} > > > + > > > +u16 cyapa_pwr_cmd_to_sleep_time(u8 pwr_mode) > > > +{ > > > + u8 encoded_time = pwr_mode >> 2; > > > + > > > + return (encoded_time < 10) ? encoded_time * 10 > > > + : (encoded_time - 5) * 20; > > > +} > > > + > > > +void cyapa_detect_async(void *data, async_cookie_t cookie) > > > +{ > > > + struct cyapa *cyapa = (struct cyapa *)data; > > > + > > > + mutex_lock(&cyapa->state_sync_lock); > > > + > > > + /* Keep synchronized with sys interface process threads. */ > > > + cyapa_detect(cyapa); > > > + > > > + mutex_unlock(&cyapa->state_sync_lock); > > > +} > > > + > > > +static void cyapa_detect_and_start(void *data, async_cookie_t cookie) > > > +{ > > > + cyapa_detect_async(data, cookie); > > > +} > > > + > > > +static int cyapa_tp_modules_init(struct cyapa *cyapa) > > > +{ > > > + return 0; > > > +} > > > + > > > +static int cyapa_tp_modules_uninit(struct cyapa *cyapa) > > > +{ > > > + return 0; > > > +} > > > + > > > static int cyapa_probe(struct i2c_client *client, > > > const struct i2c_device_id *dev_id) > > > { > > > @@ -830,6 +452,7 @@ static int cyapa_probe(struct i2c_client *client, > > > u8 adapter_func; > > > struct cyapa *cyapa; > > > struct device *dev = &client->dev; > > > + union i2c_smbus_data dummy; > > > > > > adapter_func = cyapa_check_adapter_functionality(client); > > > if (adapter_func == CYAPA_ADAPTER_FUNC_NONE) { > > > @@ -837,39 +460,43 @@ static int cyapa_probe(struct i2c_client *client, > > > return -EIO; > > > } > > > > > > + /* Make sure there is something at this address */ > > > + if (dev->of_node && i2c_smbus_xfer(client->adapter, client->addr, 0, > > > + I2C_SMBUS_READ, 0, I2C_SMBUS_BYTE, &dummy) < 0) > > > + return -ENODEV; > > > + > > > cyapa = kzalloc(sizeof(struct cyapa), GFP_KERNEL); > > > if (!cyapa) { > > > dev_err(dev, "allocate memory for cyapa failed\n"); > > > return -ENOMEM; > > > } > > > > > > - cyapa->gen = CYAPA_GEN3; > > > cyapa->client = client; > > > i2c_set_clientdata(client, cyapa); > > > sprintf(cyapa->phys, "i2c-%d-%04x/input0", client->adapter->nr, > > > client->addr); > > > > > > - /* i2c isn't supported, use smbus */ > > > - if (adapter_func == CYAPA_ADAPTER_FUNC_SMBUS) > > > - cyapa->smbus = true; > > > - cyapa->state = CYAPA_STATE_NO_DEVICE; > > > - ret = cyapa_check_is_operational(cyapa); > > > + ret = cyapa_tp_modules_init(cyapa); > > > if (ret) { > > > - dev_err(dev, "device not operational, %d\n", ret); > > > - goto err_mem_free; > > > + dev_err(dev, "initialize device modules failed.\n"); > > > + goto err_unregister_device; > > > } > > > > > > - ret = cyapa_create_input_dev(cyapa); > > > - if (ret) { > > > - dev_err(dev, "create input_dev instance failed, %d\n", ret); > > > - goto err_mem_free; > > > - } > > > + cyapa->gen = CYAPA_GEN_UNKNOWN; > > > + cyapa->ops = &cyapa_default_ops; > > > + mutex_init(&cyapa->state_sync_lock); > > > > > > - ret = cyapa_set_power_mode(cyapa, PWR_MODE_FULL_ACTIVE); > > > - if (ret) { > > > - dev_err(dev, "set active power failed, %d\n", ret); > > > - goto err_unregister_device; > > > - } > > > + /* i2c isn't supported, use smbus */ > > > + if (adapter_func == CYAPA_ADAPTER_FUNC_SMBUS) > > > + cyapa->smbus = true; > > > + cyapa->state = CYAPA_STATE_NO_DEVICE; > > > + /* > > > + * Set to hard code default, they will be updated with trackpad set > > > + * default values after probe and initialized. > > > + */ > > > + cyapa->suspend_power_mode = PWR_MODE_SLEEP; > > > + cyapa->suspend_sleep_time = > > > + cyapa_pwr_cmd_to_sleep_time(cyapa->suspend_power_mode); > > > > > > cyapa->irq = client->irq; > > > ret = request_threaded_irq(cyapa->irq, > > > @@ -880,14 +507,17 @@ static int cyapa_probe(struct i2c_client *client, > > > cyapa); > > > if (ret) { > > > dev_err(dev, "IRQ request failed: %d\n, ", ret); > > > - goto err_unregister_device; > > > + goto err_uninit_tp_modules; > > > } > > > > > > + async_schedule(cyapa_detect_and_start, cyapa); > > > return 0; > > > > > > +err_uninit_tp_modules: > > > + cyapa_tp_modules_uninit(cyapa); > > > err_unregister_device: > > > input_unregister_device(cyapa->input); > > > -err_mem_free: > > > + i2c_set_clientdata(client, NULL); > > > kfree(cyapa); > > > > > > return ret; > > > @@ -898,8 +528,12 @@ static int cyapa_remove(struct i2c_client *client) > > > struct cyapa *cyapa = i2c_get_clientdata(client); > > > > > > > You schedule detect asynchronously so you need to make sure it is not running > > (and will not be running) when you try to unbind the driver here. > > Thanks. I will update it in next release. > > To avoid this problem, a removed flag is introduced and is also synced by the > cyapa->state_sync_lock mutex lock. So when this driver is removing, it's sure > that the detecting asynchronous thread is not running, and if there's any event > cause the asynchronous thread is entered, it will do nothing and will exit immediately. Peeking at the other version of the patch you do: cyapa_remove() { ... cyapa->removed = true; ... kfree(cyapa); } and void cyapa_detect_async(void *data, async_cookie_t cookie) { ... if (cyapa->removed) { mutex_unlock(&cyapa->state_sync_lock); return; } ... } That's not going to work. Getting asynchronous behavior by individual driver is hard and I believe Luis Rordiguez is working on allowing to do that is the driver core. So I would recommend to switch driver to do synchronous probing, at least for now, and figure out other kinks first. Thanks. -- Dmitry -- 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
