From f326fb05b564b0975a9870b2831f9372539202db Mon Sep 17 00:00:00 2001 From: Chun-Ta Lin Date: Wed, 21 Jun 2017 15:14:37 +0800 Subject: hammer: enable large block reading on usb i2c passthru. Originally, i2c passthru is limited to use I2C_XFER_SINGLE flag where it can only read at most 255 bytes at a time. For application that requires larger i2c bus reading, we change the flag setting and the command protocol. TEST=old ./touchpad_updater still works (previous protocol) TEST=new ./touchpad_updater can get more than 500 bytes per transaction TEST=Debug message only print when -d assigned. ./touchpad_updater -d TEST=Manually change #define CONFIG_USB_I2C_MAX_READ_COUNT (1024 - 6) to #define CONFIG_USB_I2C_MAX_READ_COUNT (1024 - 4) and trigger POWER_OF_TWO assertion. BRANCH=none BUG=b:35587174, b:63993891 Change-Id: Id75b11ea49ba89bab8e18af24d47219030c778c5 Signed-off-by: Chun-Ta Lin Reviewed-on: https://chromium-review.googlesource.com/542716 Commit-Ready: Chun-ta Lin Tested-by: Chun-ta Lin Reviewed-by: Nicolas Boichat --- board/hammer/board.h | 12 +++- common/usb_i2c.c | 51 +++++++++++++---- extra/touchpad_updater/touchpad_updater.c | 73 +++++++++++++++++++----- include/config.h | 3 +- include/usb_i2c.h | 92 ++++++++++++++++++++++--------- 5 files changed, 177 insertions(+), 54 deletions(-) diff --git a/board/hammer/board.h b/board/hammer/board.h index 7b713d05f2..c150ac40bd 100644 --- a/board/hammer/board.h +++ b/board/hammer/board.h @@ -130,9 +130,17 @@ #define CONFIG_BOARD_PRE_INIT #define CONFIG_WATCHDOG_HELP -/* Enlarge the allowed write count */ +/* + * Enlarge the allowed write / read count for trackpad debug + * In the extended I2C reading over I2C ( >= 128 bytes ), the header size + * have to be 6 bytes instead of 4 bytes for receiving packets. Moreover, + * buffer size have to be power of two. + */ #undef CONFIG_USB_I2C_MAX_WRITE_COUNT -#define CONFIG_USB_I2C_MAX_WRITE_COUNT 124 +#define CONFIG_USB_I2C_MAX_WRITE_COUNT (128 - 4) /* 4 is maximum header size */ + +#undef CONFIG_USB_I2C_MAX_READ_COUNT +#define CONFIG_USB_I2C_MAX_READ_COUNT (1024 - 6) /* 6 is maximum header size */ /* No lid switch */ #undef CONFIG_LID_SWITCH diff --git a/common/usb_i2c.c b/common/usb_i2c.c index a6f5acc99c..de8314f8fa 100644 --- a/common/usb_i2c.c +++ b/common/usb_i2c.c @@ -22,6 +22,8 @@ #define CPRINTS(format, args...) cprints(CC_I2C, format, ## args) +#define MAX_BYTES_IN_ONE_READING 254 + USB_I2C_CONFIG(i2c, USB_IFACE_I2C, @@ -45,7 +47,7 @@ static uint8_t usb_i2c_read_packet(struct usb_i2c_config const *config) } static void usb_i2c_write_packet(struct usb_i2c_config const *config, - uint8_t count) + size_t count) { QUEUE_ADD_UNITS(config->tx_queue, config->buffer, count); } @@ -76,26 +78,37 @@ static uint8_t usb_i2c_executable(struct usb_i2c_config const *config) void usb_i2c_execute(struct usb_i2c_config const *config) { /* Payload is ready to execute. */ - uint8_t count = usb_i2c_read_packet(config); + uint8_t count = usb_i2c_read_packet(config); int portindex = (config->buffer[0] >> 0) & 0xff; /* Convert 7-bit slave address to chromium EC 8-bit address. */ uint8_t slave_addr = (config->buffer[0] >> 7) & 0xfe; int write_count = (config->buffer[1] >> 0) & 0xff; int read_count = (config->buffer[1] >> 8) & 0xff; - int port; + uint8_t *payload = (uint8_t *)(config->buffer + 2); + int xfer_flag = I2C_XFER_START; + int offset = 0; /* Offset for extended reading header. */ + int rv = 0; + int complete_bytes = 0; + int bytes_to_read, port; config->buffer[0] = 0; config->buffer[1] = 0; + if (read_count & 0x80) { + read_count = ((config->buffer[2] & 0xff) << 7) | + (read_count & 0x7f); + offset = 2; + } + if (!count || (!read_count && !write_count)) return; if (!usb_i2c_board_is_enabled()) { config->buffer[0] = USB_I2C_DISABLED; } else if (write_count > CONFIG_USB_I2C_MAX_WRITE_COUNT || - write_count != (count - 4)) { + write_count != (count - 4 - offset)) { config->buffer[0] = USB_I2C_WRITE_COUNT_INVALID; - } else if (read_count > USB_I2C_MAX_READ_COUNT) { + } else if (read_count > CONFIG_USB_I2C_MAX_READ_COUNT) { config->buffer[0] = USB_I2C_READ_COUNT_INVALID; } else if (portindex >= i2c_ports_used) { config->buffer[0] = USB_I2C_PORT_INVALID; @@ -107,12 +120,28 @@ void usb_i2c_execute(struct usb_i2c_config const *config) * EC_CMD_I2C_PASSTHRU, which can protect ports and ranges. */ port = i2c_ports[portindex].port; - config->buffer[0] = usb_i2c_map_error( - i2c_xfer(port, slave_addr, - (uint8_t *)(config->buffer + 2), - write_count, - (uint8_t *)(config->buffer + 2), - read_count, I2C_XFER_SINGLE)); + /* + * Because The I2C_XFER_SINGLE might limit the reading to be + * less than 255 bytes (For example, register design of + * STM32_I2C_CR2 in i2c-stm32f0.c), we hold the STOP bits for + * reading request larger than 255 bytes. + */ + do { + bytes_to_read = read_count - complete_bytes; + if (bytes_to_read > MAX_BYTES_IN_ONE_READING) + bytes_to_read = MAX_BYTES_IN_ONE_READING; + else + xfer_flag |= I2C_XFER_STOP; + rv = i2c_xfer( + port, slave_addr, + complete_bytes ? NULL : payload + offset, + complete_bytes ? 0 : write_count, + payload + complete_bytes, + bytes_to_read, xfer_flag); + complete_bytes += bytes_to_read; + xfer_flag = 0; + } while (complete_bytes < read_count && !rv); + config->buffer[0] = usb_i2c_map_error(rv); } usb_i2c_write_packet(config, read_count + 4); } diff --git a/extra/touchpad_updater/touchpad_updater.c b/extra/touchpad_updater/touchpad_updater.c index b54f348c2b..059ddb0933 100644 --- a/extra/touchpad_updater/touchpad_updater.c +++ b/extra/touchpad_updater/touchpad_updater.c @@ -21,6 +21,7 @@ static uint16_t vid = 0x18d1; /* Google */ static uint16_t pid = 0x5022; /* Hammer */ static uint8_t ep_num = 4; /* console endpoint */ +static uint8_t extended_i2c_exercise; /* non-zero to exercise */ static char *firmware_binary = "144.0_2.0.bin"; /* firmware blob */ /* Firmware binary blob related */ @@ -40,7 +41,7 @@ static int le_bytes_to_int(uint8_t *buf) /* Command line parsing related */ static char *progname; -static char *short_opts = ":f:v:p:e:h"; +static char *short_opts = ":f:v:p:e:hd"; static const struct option long_opts[] = { /* name hasarg *flag val */ {"file", 1, NULL, 'f'}, @@ -48,6 +49,7 @@ static const struct option long_opts[] = { {"pid", 1, NULL, 'p'}, {"ep", 1, NULL, 'e'}, {"help", 0, NULL, 'h'}, + {"debug", 0, NULL, 'd'}, {NULL, 0, NULL, 0}, }; @@ -63,6 +65,8 @@ static void usage(int errs) " -v,--vid HEXVAL Vendor ID (default %04x)\n" " -p,--pid HEXVAL Product ID (default %04x)\n" " -e,--ep NUM Endpoint (default %d)\n" + " -d,--debug Exercise extended read I2C over USB\n" + " and print verbose debug messages.\n" " -h,--help Show this message\n" "\n", progname, firmware_binary, vid, pid, ep_num); @@ -107,6 +111,9 @@ static void parse_cmdline(int argc, char *argv[]) errorcnt++; } break; + case 'd': + extended_i2c_exercise = 1; + break; case 'h': usage(errorcnt); break; @@ -136,8 +143,8 @@ static void parse_cmdline(int argc, char *argv[]) } /* USB transfer related */ -static uint8_t rx_buf[128]; -static uint8_t tx_buf[128]; +static uint8_t rx_buf[1024]; +static uint8_t tx_buf[1024]; static struct libusb_device_handle *devh; static struct libusb_transfer *rx_transfer; @@ -272,8 +279,8 @@ static int check_read_status(int r, int expected, int actual) printf("Warning: Defined error code (%d) returned.\n", r); } - if (r) { - printf("Dumping the receive buffer:\n"); + if (r || extended_i2c_exercise) { + printf("\nDumping the receive buffer:\n"); printf(" Recv %d bytes from USB hosts.\n", actual); for (i = 0; i < actual; ++i) printf(" [%2d]bytes: 0x%0x\n", i, rx_buf[i]); @@ -282,26 +289,40 @@ static int check_read_status(int r, int expected, int actual) } #define MAX_USB_PACKET_SIZE 64 +#define PRIMITIVE_READING_SIZE 60 static int libusb_single_write_and_read( uint8_t *to_write, uint16_t write_length, - uint8_t *to_read, uint8_t read_length) + uint8_t *to_read, uint16_t read_length) { int r; int tx_ready; int remains; int sent_bytes = 0; int actual_length = -1; + int offset = read_length > PRIMITIVE_READING_SIZE ? 6 : 4; tx_transfer = rx_transfer = 0; - memmove(tx_buf + 4, to_write, write_length); + memmove(tx_buf + offset, to_write, write_length); tx_buf[0] = I2C_PORT_ON_HAMMER; tx_buf[1] = I2C_ADDRESS_ON_HAMMER; tx_buf[2] = write_length; - tx_buf[3] = read_length; + if (read_length > PRIMITIVE_READING_SIZE) { + tx_buf[3] = (read_length & 0x7f) | (1 << 7); + tx_buf[4] = read_length >> 7; + if (extended_i2c_exercise) { + printf("Triggering extended reading." + "rc:%0x, rc1:%0x\n", + tx_buf[3], tx_buf[4]); + printf("Expecting %d Bytes.\n", + (tx_buf[3] & 0x7f) | (tx_buf[4] << 7)); + } + } else { + tx_buf[3] = read_length; + } - while (sent_bytes < (4 + write_length)) { - tx_ready = remains = (4 + write_length) - sent_bytes; + while (sent_bytes < (offset + write_length)) { + tx_ready = remains = (offset + write_length) - sent_bytes; if (tx_ready > MAX_USB_PACKET_SIZE) tx_ready = MAX_USB_PACKET_SIZE; @@ -312,7 +333,8 @@ static int libusb_single_write_and_read( if (r == 0 && actual_length == tx_ready) { r = libusb_bulk_transfer(devh, (ep_num | LIBUSB_ENDPOINT_IN), - rx_buf, 128, &actual_length, 0); + rx_buf, sizeof(rx_buf), + &actual_length, 0); } r = check_read_status( r, (remains == tx_ready) ? read_length : 0, @@ -526,6 +548,16 @@ static uint16_t elan_update_firmware(void) return checksum; } +static void pretty_print_buffer(uint8_t *buf, int len) +{ + int i; + + printf("Buffer = 0x"); + for (i = 0; i < len; ++i) + printf("%02X", buf[i]); + printf("\n"); +} + int main(int argc, char *argv[]) { uint16_t local_checksum; @@ -537,7 +569,7 @@ int main(int argc, char *argv[]) /* * Judge IC type and get page count first. - *Then check the FW file. + * Then check the FW file. */ fw_page_count = elan_get_ic_page_count(); fw_size = fw_page_count * FW_PAGE_SIZE; @@ -545,15 +577,26 @@ int main(int argc, char *argv[]) /* Read the FW file */ FILE *f = fopen(firmware_binary, "rb"); - - if (!(f && fread(fw_data, 1, fw_size, f) == (unsigned int)fw_size)) - printf("Failed to read firmware from %s\n", firmware_binary); + if (!f) + request_exit("Cannot find binary: %s\n", firmware_binary); + if (fread(fw_data, 1, fw_size, f) != (unsigned int)fw_size) + request_exit("binary size mismatch, expect %d\n", fw_size); /* * It is possible that you are not able to get firmware info. This * might due to an incomplete update last time */ elan_get_fw_info(); + + if (extended_i2c_exercise) { + /* + * Trigger an I2C transaction of expecting reading > 60 bytes. + * source: https://goo.gl/pSxESS + */ + elan_write_and_read(0x0002, rx_buf, 118, 0, 0); + pretty_print_buffer(rx_buf, 118); + } + /* Get the trackpad ready for receiving update */ elan_prepare_for_update(); diff --git a/include/config.h b/include/config.h index dba87924db..45e144484d 100644 --- a/include/config.h +++ b/include/config.h @@ -2873,8 +2873,9 @@ /* USB I2C config */ #undef CONFIG_USB_I2C -/* Allowed write count for USB over I2C */ +/* Allowed read/write count for USB over I2C */ #define CONFIG_USB_I2C_MAX_WRITE_COUNT 60 +#define CONFIG_USB_I2C_MAX_READ_COUNT 60 /*****************************************************************************/ /* USB Power monitoring interface config */ diff --git a/include/usb_i2c.h b/include/usb_i2c.h index 3e8eb3e7e5..c135066e26 100644 --- a/include/usb_i2c.h +++ b/include/usb_i2c.h @@ -14,27 +14,63 @@ #define __CROS_USB_I2C_H /* - * Command: - * +----------+-----------+---------------+---------------+---------------+ - * | port: 1B | addr: 1B | wr count : 1B | rd count : 1B | data : <= 60B | - * +----------+-----------+---------------+---------------+---------------+ + * 2 forms of command are supported: + * - When write payload + header is larger than 64 bytes, which exceed the + * common USB packet (64 bytes), remaining payload should send without + * header. * - * port address: 1 byte, i2c interface index + * - CONFIG_USB_I2C_MAX_WRITE_COUNT / CONFIG_USB_I2C_MAX_READ_COUNT have to + * be defined properly based on the use cases. * - * slave address: 1 byte, i2c 7-bit bus address + * - Read less than 128 (0x80) bytes. + * +------+------+----+----+---------------+ + * | port | addr | wc | rc | write payload | + * +------+------+----+----+---------------+ + * | 1B | 1B | 1B | 1B | < 256 bytes | + * +------+------+----+----+---------------+ * - * write count: 1 byte, zero based count of bytes to write. If write - * count exceed 60 bytes, following packets are expected - * to continue the payload without header. + * - Read less than 32768 (0x8000) bytes. + * +------+------+----+----+-----+----------+---------------+ + * | port | addr | wc | rc | rc1 | reserved | write payload | + * +------+------+----+----+----------------+---------------+ + * | 1B | 1B | 1B | 1B | 1B | 1B | < 256 bytes | + * +------+------+----+----+----------------+---------------+ * - * read count: 1 byte, zero based count of bytes to read + * - Special notes for rc and rc1: + * If the most significant bit in rc is set (rc >= 0x80), this indicates + * that we want to read back more than 127 bytes, so the first byte of + * data contains rc1 (read count continuation), and the final read count + * will be (rc1 << 7) | (rc & 0x7F). * - * data: payload of data to write. + * Fields: + * - port: port address, 1 byte, i2c interface index. + * + * - addr: slave address, 1 byte, i2c 7-bit bus address. + * + * - wc: write count, 1 byte, zero based count of bytes to write. If the + * indicated write count cause the payload + header exceeds 64 bytes, + * Following packets are expected to continue the payload without + * header. + * + * - rc: read count, 1 byte, zero based count of bytes to read. To read more + * than 127 (0x7F) bytes please see the special notes above. + * + * - data: payload of data to write. See wc above for more information. + * + * - rc1: extended read count, 1 byte. An extended version indicates we want + * to read more data. While the most significant bits is set in read + * count (rc >= 0x80), rc1 will concatenate with rc together. See the + * special notes above for concatenating details. + * + * - reserved: reserved byte, 1 byte. * * Response: - * +-------------+---+---+-----------------------+ - * | status : 2B | 0 | 0 | read payload : <= 60B | - * +-------------+---+---+-----------------------+ + * +-------------+---+---+--------------+ + * | status : 2B | 0 | 0 | read payload | + * +-------------+---+---+--------------+ + * + * - read payload might not fit into a single USB packets. Remaining will be + * transimitted witout header. Receiving side should concatenate them. * * status: 2 byte status * 0x0000: Success @@ -43,15 +79,15 @@ * This can happen if someone else has acquired the shared memory * buffer that the I2C driver uses as /dev/null * 0x0003: Write count invalid (mismatch with merged payload) - * 0x0004: Read count invalid (> 60 bytes) + * 0x0004: Read count invalid (e.g. larger than available buffer) * 0x0005: The port specified is invalid. * 0x0006: The I2C interface is disabled. * 0x8000: Unknown error mask * The bottom 15 bits will contain the bottom 15 bits from the EC * error code. * - * read payload: up to 60 bytes of data read from I2C, length will match - * requested read count + * read payload: Depends on the buffer size and implementation. Length will + * match requested read count */ enum usb_i2c_error { @@ -66,12 +102,18 @@ enum usb_i2c_error { }; -#define USB_I2C_MAX_READ_COUNT 60 -#define USB_I2C_CONFIG_BUFFER_SIZE \ - ((CONFIG_USB_I2C_MAX_WRITE_COUNT+4) > USB_MAX_PACKET_SIZE ? \ - (CONFIG_USB_I2C_MAX_WRITE_COUNT+4) : USB_MAX_PACKET_SIZE) +#define USB_I2C_WRITE_BUFFER (CONFIG_USB_I2C_MAX_WRITE_COUNT + 4) +/* If read payload is larger or equal to 128 bytes, header contains rc1 */ +#define USB_I2C_READ_BUFFER ((CONFIG_USB_I2C_MAX_READ_COUNT < 128) ? \ + (CONFIG_USB_I2C_MAX_READ_COUNT + 4) : \ + (CONFIG_USB_I2C_MAX_READ_COUNT + 6)) + +#define USB_I2C_BUFFER_SIZE \ + (USB_I2C_READ_BUFFER > USB_I2C_WRITE_BUFFER ? \ + USB_I2C_READ_BUFFER : USB_I2C_WRITE_BUFFER) -BUILD_ASSERT(USB_MAX_PACKET_SIZE == (2 + 1 + 1 + USB_I2C_MAX_READ_COUNT)); +BUILD_ASSERT(POWER_OF_TWO(USB_I2C_READ_BUFFER)); +BUILD_ASSERT(POWER_OF_TWO(USB_I2C_WRITE_BUFFER)); /* * Compile time Per-USB gpio configuration stored in flash. Instances of this @@ -110,7 +152,7 @@ extern struct consumer_ops const usb_i2c_consumer_ops; ENDPOINT) \ static uint16_t \ CONCAT2(NAME, _buffer_) \ - [USB_I2C_CONFIG_BUFFER_SIZE / 2]; \ + [USB_I2C_BUFFER_SIZE / 2]; \ static void CONCAT2(NAME, _deferred_)(void); \ DECLARE_DEFERRED(CONCAT2(NAME, _deferred_)); \ static struct queue const CONCAT2(NAME, _to_usb_); \ @@ -136,10 +178,10 @@ extern struct consumer_ops const usb_i2c_consumer_ops; .tx_queue = &CONCAT2(NAME, _to_usb_), \ }; \ static struct queue const CONCAT2(NAME, _to_usb_) = \ - QUEUE_DIRECT(USB_MAX_PACKET_SIZE, uint8_t, \ + QUEUE_DIRECT(USB_I2C_READ_BUFFER, uint8_t, \ null_producer, CONCAT2(NAME, _usb_).consumer); \ static struct queue const CONCAT3(usb_to_, NAME, _) = \ - QUEUE_DIRECT(CONFIG_USB_I2C_MAX_WRITE_COUNT+4, uint8_t, \ + QUEUE_DIRECT(USB_I2C_WRITE_BUFFER, uint8_t, \ CONCAT2(NAME, _usb_).producer, NAME.consumer); \ static void CONCAT2(NAME, _deferred_)(void) \ { usb_i2c_deferred(&NAME); } -- cgit v1.2.1