Hi Dmitry: 1. In this function elan_get_fwinfo(). +static int elan_get_fwinfo(u16 ic_type, u8 iap_version, u8 pattern, + u16 *validpage_count, u32 *signature_address, + u16 *page_size) { - switch (ic_type) { + u16 type = pattern >= 0x01 ? ic_type : iap_version; + + switch (type) { This iap_version in pattern0 is read from this command ETP_I2C_IAP_VERSION_CMD_OLD ,it is not from this command ETP_I2C_IAP_VERSION. So u16 type = pattern >= 0x01 ? ic_type : iap_version; <- wrong 2. In this "static int elan_i2c_prepare_fw_update(struct i2c_client *client, u16 ic_type, u8 iap_version)" function. The ic is old pattern must be modify correct ic_type. (cmd is ETP_I2C_IAP_VERSION) THANKS -----Original Message----- From: jingle [mailto:jingle.wu@xxxxxxxxxx] Sent: Friday, July 17, 2020 4:20 PM To: 'Dmitry Torokhov' Cc: 'linux-kernel'; 'linux-input'; 'phoenix'; 'josh.chen'; 'kai.heng.feng' Subject: RE: [PATCH 2/2] Input: elan_i2c - Modify the IAP related functio n for page sizes 128, 512 bytes. Hi Dmitry: In this "static int elan_i2c_prepare_fw_update(struct i2c_client *client, u16 ic_type, u8 iap_version)" function If IC is old_pattern, it must be modified to iap_version -> u16 type = pattern >= 0x01 ? ic_type : iap_version; Thanks -----Original Message----- From: Dmitry Torokhov [mailto:dmitry.torokhov@xxxxxxxxx] Sent: Friday, July 17, 2020 2:10 PM To: jingle.wu Cc: linux-kernel; linux-input; phoenix; josh.chen; kai.heng.feng Subject: Re: [PATCH 2/2] Input: elan_i2c - Modify the IAP related functio n for page sizes 128, 512 bytes. On Thu, Jul 16, 2020 at 06:27:19PM -0700, Dmitry Torokhov wrote: > Hi Jingle, > > On Thu, Jul 16, 2020 at 02:15:23PM +0800, jingle.wu wrote: > > HI Dmitry: > > > > Just to confirm, the older devices (I assume that pattern 0 means > > older) have version command that is numerically higher than the one > > for the newer (pattern >= 1) devices? > > > > >> Yes, Pattern 1, 2 are newer devices. > > > > > @@ -324,7 +342,14 @@ static int elan_i2c_get_sm_version(struct i2c_client *client, > > > return error; > > > } > > > *version = val[0]; > > > - *ic_type = val[1]; > > > + > > > + error = elan_i2c_read_cmd(client, ETP_I2C_IAP_VERSION_CMD, val); > > > + if (error) { > > > + dev_err(&client->dev, "failed to get ic type: %d\n", > > > + error); > > > + return error; > > > + } > > > > Could you please tell me why this chunk is needed? > > >> Modify the old pattern IC firmware read the correct ic_type. > > > > In the elan_i2c_core.c, move this code to elan_i2c_i2c.c. > > static int elan_query_device_info(struct elan_tp_data *data) { > > ..... > > if (data->pattern == 0x01) > > ic_type = data->ic_type; > > else > > ic_type = data->iap_version; > > ..... > > return 0; > > } > > I am concerned that unconditionally substituting iap_version for > ic_type for "pattern 0" devices will break check in > elan_check_ASUS_special_fw() as it operates on the ic_type returned by > ETP_I2C_OSM_VERSION_CMD and not iap_version. I split the firmware handling code into a few patches and uploaded it to a new elan-i2c branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git elan-i2c Please take a look and let me know if I messed it up or not. I will be looking at the new packet format next. Thanks. -- Dmitry