Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752402AbaLOOLr (ORCPT ); Mon, 15 Dec 2014 09:11:47 -0500 Received: from mail-pd0-f169.google.com ([209.85.192.169]:43391 "EHLO mail-pd0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752366AbaLOOLn (ORCPT ); Mon, 15 Dec 2014 09:11:43 -0500 Date: Mon, 15 Dec 2014 06:11:38 -0800 From: Jeremiah Mahler To: Dudley Du Cc: dmitry.torokhov@gmail.com, rydberg@euromail.se, bleung@google.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v15 06/12] input: cyapa: add gen3 trackpad device firmware update function support Message-ID: <20141215141138.GE918@newt.localdomain> Mail-Followup-To: Jeremiah Mahler , Dudley Du , dmitry.torokhov@gmail.com, rydberg@euromail.se, bleung@google.com, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org References: <1418624603-19054-1-git-send-email-dudley.dulixin@gmail.com> <1418624603-19054-7-git-send-email-dudley.dulixin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418624603-19054-7-git-send-email-dudley.dulixin@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dudley, On Mon, Dec 15, 2014 at 02:23:17PM +0800, Dudley Du wrote: > Add firmware image update function supported for gen3 trackpad device, > it can be used through sysfs update_fw interface. > TEST=test on Chromebooks. > > Signed-off-by: Dudley Du > --- > drivers/input/mouse/cyapa_gen3.c | 284 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 284 insertions(+) > > diff --git a/drivers/input/mouse/cyapa_gen3.c b/drivers/input/mouse/cyapa_gen3.c > index 433c5b1..9851337 100644 > --- a/drivers/input/mouse/cyapa_gen3.c > +++ b/drivers/input/mouse/cyapa_gen3.c > @@ -416,6 +416,72 @@ static int cyapa_gen3_state_parse(struct cyapa *cyapa, u8 *reg_data, int len) > return -EAGAIN; > } > > +/* > + * Enter bootloader by soft resetting the device. > + * > + * If device is already in the bootloader, the function just returns. > + * Otherwise, reset the device; after reset, device enters bootloader idle > + * state immediately. > + * > + * Also, if device was unregister device from input core. Device will > + * re-register after it is detected following resumption of operational mode. > + * I don't quite understand this. Tell me if the following re-wording is correct. + * Also, if the device was unregistered from the input core, the device + * will re-register after it is detected following resumption of + * operational mode. > + * Returns: > + * 0 on success > + * -EAGAIN device was reset, but is not now in bootloader idle state > + * < 0 if the device never responds within the timeout Could be formatted nicer. Tell me if the re-wording makes sense. + * 0 on success + * -EAGAIN if device was reset, but it is not in bootloader idle state + * < 0 if the device never responds within the timeout > + */ > +static int cyapa_gen3_bl_enter(struct cyapa *cyapa) > +{ > + int error; > + > + error = cyapa_poll_state(cyapa, 500); > + if (error) > + return error; > + if (cyapa->state == CYAPA_STATE_BL_IDLE) { > + /* Already in BL_IDLE. Skipping reset. */ > + return 0; > + } > + > + if (cyapa->state != CYAPA_STATE_OP) > + return -EAGAIN; > + > + cyapa->state = CYAPA_STATE_NO_DEVICE; > + error = cyapa_write_byte(cyapa, CYAPA_CMD_SOFT_RESET, 0x01); > + if (error) > + return -EIO; > + > + usleep_range(25000, 50000); > + error = cyapa_poll_state(cyapa, 500); > + if (error) > + return error; > + if ((cyapa->state != CYAPA_STATE_BL_IDLE) || > + (cyapa->status[REG_BL_STATUS] & BL_STATUS_WATCHDOG)) > + return -EAGAIN; > + > + return 0; > +} > + > +static int cyapa_gen3_bl_activate(struct cyapa *cyapa) > +{ > + int error; > + > + error = cyapa_i2c_reg_write_block(cyapa, 0, sizeof(bl_activate), > + bl_activate); > + if (error) > + return error; > + > + /* Wait for bootloader to activate; takes between 2 and 12 seconds */ > + msleep(2000); > + error = cyapa_poll_state(cyapa, 11000); > + if (error) > + return error; > + if (cyapa->state != CYAPA_STATE_BL_ACTIVE) > + return -EAGAIN; > + > + return 0; > +} > + > static int cyapa_gen3_bl_deactivate(struct cyapa *cyapa) > { > int error; > @@ -476,6 +542,218 @@ static int cyapa_gen3_bl_exit(struct cyapa *cyapa) > return 0; > } > > +/* Used in gen3 bootloader commands. */ We should be able to figure out where it is used. Unnecessary comment? > +static u16 cyapa_gen3_csum(const u8 *buf, size_t count) > +{ > + int i; > + u16 csum = 0; > + > + for (i = 0; i < count; i++) > + csum += buf[i]; > + > + return csum; > +} > + > +/* > + * Verify the integrity of a CYAPA firmware image file. > + * > + * The firmware image file is 30848 bytes, composed of 482 64-byte blocks. > + * > + * The first 2 blocks are the firmware header. > + * The next 480 blocks are the firmware image. > + * > + * The first two bytes of the header hold the header checksum, computed by > + * summing the other 126 bytes of the header. > + * The last two bytes of the header hold the firmware image checksum, computed > + * by summing the 30720 bytes of the image modulo 0xffff. > + * > + * Both checksums are stored little-endian. > + */ > +static int cyapa_gen3_check_fw(struct cyapa *cyapa, const struct firmware *fw) > +{ > + struct device *dev = &cyapa->client->dev; > + u16 csum; > + u16 csum_expected; > + > + /* Firmware must match exact 30848 bytes = 482 64-byte blocks. */ > + if (fw->size != CYAPA_FW_SIZE) { > + dev_err(dev, "invalid firmware size = %zu, expected %u.\n", > + fw->size, CYAPA_FW_SIZE); > + return -EINVAL; > + } > + > + /* Verify header block */ > + csum_expected = (fw->data[0] << 8) | fw->data[1]; > + csum = cyapa_gen3_csum(&fw->data[2], CYAPA_FW_HDR_SIZE - 2); > + if (csum != csum_expected) { > + dev_err(dev, "%s %04x, expected: %04x\n", > + "invalid firmware header checksum = ", > + csum, csum_expected); > + return -EINVAL; > + } > + > + /* Verify firmware image */ > + csum_expected = (fw->data[CYAPA_FW_HDR_SIZE - 2] << 8) | > + fw->data[CYAPA_FW_HDR_SIZE - 1]; > + csum = cyapa_gen3_csum(&fw->data[CYAPA_FW_HDR_SIZE], > + CYAPA_FW_DATA_SIZE); > + if (csum != csum_expected) { > + dev_err(dev, "%s %04x, expected: %04x\n", > + "invalid firmware header checksum = ", > + csum, csum_expected); > + return -EINVAL; > + } > + return 0; > +} > + > +/* > + * Write a |len| byte long buffer |buf| to the device, by chopping it up into a > + * sequence of smaller |CYAPA_CMD_LEN|-length write commands. > + * > + * The data bytes for a write command are prepended with the 1-byte offset > + * of the data relative to the start of |buf|. > + */ > +static int cyapa_gen3_write_buffer(struct cyapa *cyapa, > + const u8 *buf, size_t len) > +{ > + int error; > + size_t i; > + unsigned char cmd[CYAPA_CMD_LEN + 1]; > + size_t cmd_len; > + > + for (i = 0; i < len; i += CYAPA_CMD_LEN) { > + const u8 *payload = &buf[i]; > + > + cmd_len = (len - i >= CYAPA_CMD_LEN) ? CYAPA_CMD_LEN : len - i; > + cmd[0] = i; > + memcpy(&cmd[1], payload, cmd_len); > + > + error = cyapa_i2c_reg_write_block(cyapa, 0, cmd_len + 1, cmd); > + if (error) > + return error; > + } > + return 0; > +} > + > +/* > + * A firmware block write command writes 64 bytes of data to a single flash > + * page in the device. The 78-byte block write command has the format: > + * <0xff> > + * > + * <0xff> - every command starts with 0xff > + * - the write command value is 0x39 > + * - write commands include an 8-byte key: { 00 01 02 03 04 05 06 07 } > + * - Memory Block number (address / 64) (16-bit, big-endian) > + * - 64 bytes of firmware image data > + * - sum of 64 bytes, modulo 0xff > + * - sum of 77 bytes, from 0xff to > + * > + * Each write command is split into 5 i2c write transactions of up to 16 bytes. > + * Each transaction starts with an i2c register offset: (00, 10, 20, 30, 40). > + */ > +static int cyapa_gen3_write_fw_block(struct cyapa *cyapa, > + u16 block, const u8 *data) > +{ > + int ret; > + u8 cmd[78]; > + u8 status[BL_STATUS_SIZE]; > + /* Programming for one block can take about 100ms. */ > + int tries = 11; > + u8 bl_status, bl_error; > + > + /* Set write command and security key bytes. */ > + cmd[0] = 0xff; > + cmd[1] = 0x39; > + cmd[2] = 0x00; > + cmd[3] = 0x01; > + cmd[4] = 0x02; > + cmd[5] = 0x03; > + cmd[6] = 0x04; > + cmd[7] = 0x05; > + cmd[8] = 0x06; > + cmd[9] = 0x07; > + cmd[10] = block >> 8; > + cmd[11] = block; > + memcpy(&cmd[12], data, CYAPA_FW_BLOCK_SIZE); > + cmd[76] = cyapa_gen3_csum(data, CYAPA_FW_BLOCK_SIZE); > + cmd[77] = cyapa_gen3_csum(cmd, sizeof(cmd) - 1); > + Sequences like these are used all over this code. I wish there was a cleaner and more descriptive way to describe what is being constructed. Perhaps using a struct? > + ret = cyapa_gen3_write_buffer(cyapa, cmd, sizeof(cmd)); > + if (ret) > + return ret; > + > + /* Wait for write to finish */ > + do { > + usleep_range(10000, 20000); > + > + /* Check block write command result status. */ > + ret = cyapa_i2c_reg_read_block(cyapa, BL_HEAD_OFFSET, > + BL_STATUS_SIZE, status); > + if (ret != BL_STATUS_SIZE) > + return (ret < 0) ? ret : -EIO; > + } while ((status[REG_BL_STATUS] & BL_STATUS_BUSY) && --tries); > + > + /* Ignore WATCHDOG bit and reserved bits. */ > + bl_status = status[REG_BL_STATUS] & ~BL_STATUS_REV_MASK; > + bl_error = status[REG_BL_ERROR] & ~BL_ERROR_RESERVED; > + > + if (bl_status & BL_STATUS_BUSY) > + ret = -ETIMEDOUT; > + else if (bl_status != BL_STATUS_RUNNING || > + bl_error != BL_ERROR_BOOTLOADING) > + ret = -EIO; > + else > + ret = 0; > + > + return ret; > +} > + > +static int cyapa_gen3_write_blocks(struct cyapa *cyapa, > + size_t start_block, size_t block_count, > + const u8 *image_data) > +{ > + int error; > + int i; > + > + for (i = 0; i < block_count; i++) { > + size_t block = start_block + i; > + size_t addr = i * CYAPA_FW_BLOCK_SIZE; > + const u8 *data = &image_data[addr]; > + > + error = cyapa_gen3_write_fw_block(cyapa, block, data); > + if (error) > + return error; > + } > + return 0; > +} > + > +static int cyapa_gen3_do_fw_update(struct cyapa *cyapa, > + const struct firmware *fw) > +{ > + struct device *dev = &cyapa->client->dev; > + int error; > + > + /* First write data, starting at byte 128 of fw->data */ ^ extra space > + error = cyapa_gen3_write_blocks(cyapa, > + CYAPA_FW_DATA_BLOCK_START, CYAPA_FW_DATA_BLOCK_COUNT, > + &fw->data[CYAPA_FW_HDR_BLOCK_COUNT * CYAPA_FW_BLOCK_SIZE]); > + if (error) { > + dev_err(dev, "FW update aborted, write image: %d\n", error); > + return error; > + } > + > + /* Then write checksum */ > + error = cyapa_gen3_write_blocks(cyapa, > + CYAPA_FW_HDR_BLOCK_START, CYAPA_FW_HDR_BLOCK_COUNT, > + &fw->data[0]); > + if (error) { > + dev_err(dev, "FW update aborted, write checksum: %d\n", error); > + return error; > + } > + > + return 0; > +} > + > /* > * cyapa_get_wait_time_for_pwr_cmd > * > @@ -783,6 +1061,12 @@ static int cyapa_gen3_irq_handler(struct cyapa *cyapa) > } > > const struct cyapa_dev_ops cyapa_gen3_ops = { > + .check_fw = cyapa_gen3_check_fw, > + .bl_enter = cyapa_gen3_bl_enter, > + .bl_activate = cyapa_gen3_bl_activate, > + .update_fw = cyapa_gen3_do_fw_update, > + .bl_deactivate = cyapa_gen3_bl_deactivate, > + > .state_parse = cyapa_gen3_state_parse, > .operational_check = cyapa_gen3_do_operational_check, > > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- - Jeremiah Mahler -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/