From patchwork Tue Nov 21 18:43:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eudean Sun X-Patchwork-Id: 10068741 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4B51360375 for ; Tue, 21 Nov 2017 18:43:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3E013298E1 for ; Tue, 21 Nov 2017 18:43:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 32EFD298E4; Tue, 21 Nov 2017 18:43:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CE188298E6 for ; Tue, 21 Nov 2017 18:43:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751235AbdKUSn2 (ORCPT ); Tue, 21 Nov 2017 13:43:28 -0500 Received: from mail-it0-f65.google.com ([209.85.214.65]:43507 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751184AbdKUSn1 (ORCPT ); Tue, 21 Nov 2017 13:43:27 -0500 Received: by mail-it0-f65.google.com with SMTP id m191so3166271itg.2 for ; Tue, 21 Nov 2017 10:43:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arista.com; s=google; h=date:from:to:cc:subject:message-id:mime-version:content-disposition :user-agent; bh=eTgg4XLwMRb67Xz1O0NP7BB9xpiV1yGAn82m7Qx+zxE=; b=hfq9htlfqwjh7LYozWQQZQ2qkkx+dUsTiAqYwVTQbQuffxQMADTAhlOfTxJnEcYS+g SShDW189rYvNTYMwbVe4eNH5s53kKpE1fgSB0+J7iemwtu+SiYBk+AWyraBofarZqXkS zcIlh6xtkmusTG33A0aLGAT2qm9NAOzJz+RSY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:mime-version :content-disposition:user-agent; bh=eTgg4XLwMRb67Xz1O0NP7BB9xpiV1yGAn82m7Qx+zxE=; b=DRUMiXLWRQpnHqXNA6IwzQb/CACFgNjMf09M8MqJflYHSjprhHavIRlOc61oKNX1Pr HbhNqz4zdjInkDzJgHPTr35x9wkTSlTQAez7NuLBjSiKbIuJzltOGJARG9UihlPb9WpD lVYiwb+JnCe/f49gBp9cV8arXHGc41rS/hgqsDQvVTYNVUM17MPVj/8Hk3W66XQVNsxu lk8pu90TfMKM5qoH7xGVRe+gfLHNYLB0WlH62VPaIpJ4N5MliyBf+EganO8VyR9liDO8 PopHZzvFVlA7x+zCWLSd06xqNCDKEcdgW5ywthgkUHU8K4/OP24R/9892xZdwz5coWK8 g2mA== X-Gm-Message-State: AJaThX4Q9jDCbs4H3ApyCX+glCDs0UXnNtAMr1CTJbjilaVTeeq+P0Ro JzOJn11ZNYuHPicytpPumHoYsQ== X-Google-Smtp-Source: AGs4zMbQogWnacNP/pk5st+vPxmA8e4egYt9HV6CFp2sRvCq8HrZAb/ls358cOzJNruKqFMEjra1pA== X-Received: by 10.36.17.5 with SMTP id 5mr209721itf.103.1511289807066; Tue, 21 Nov 2017 10:43:27 -0800 (PST) Received: from mail.arista.com ([50.232.156.226]) by smtp.gmail.com with ESMTPSA id k66sm939751ita.6.2017.11.21.10.43.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 21 Nov 2017 10:43:26 -0800 (PST) Date: Tue, 21 Nov 2017 10:43:24 -0800 From: Eudean Sun To: Jiri Kosina , Benjamin Tissoires Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2] HID: cp2112: Fix I2C_BLOCK_DATA transactions Message-ID: <20171121184324.GA7204@mail.arista.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The existing driver erroneously treats I2C_BLOCK_DATA and BLOCK_DATA commands the same. For I2C_BLOCK_DATA reads, the length of the read is provided in data->block[0], but the length itself should not be sent to the slave. In contrast, for BLOCK_DATA reads no length is specified since the length will be the first byte returned from the slave. When copying data back to the data buffer, for an I2C_BLOCK_DATA read we have to take care not to overwrite data->block[0] to avoid overwriting the length. A BLOCK_DATA read doesn't have this concern since the first byte returned by the device is the length and belongs in data->block[0]. For I2C_BLOCK_DATA writes, the length is also provided in data->block[0], but the length itself is not sent to the slave (in contrast to BLOCK_DATA writes where the length prefixes the data sent to the slave). This was tested on physical hardware using i2cdump with the i and s flags to test the behavior of I2C_BLOCK_DATA reads and BLOCK_DATA reads, respectively. Writes were not tested but the I2C_BLOCK_DATA write change is pretty simple to verify by inspection. Signed-off-by: Eudean Sun --- Changes in v2: - Explain the fix and testing in more detail in the commit message. --- drivers/hid/hid-cp2112.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 078026f..0adece2 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -692,8 +692,16 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr, (u8 *)&word, 2); break; case I2C_SMBUS_I2C_BLOCK_DATA: - size = I2C_SMBUS_BLOCK_DATA; - /* fallthrough */ + if (read_write == I2C_SMBUS_READ) { + read_length = data->block[0]; + count = cp2112_write_read_req(buf, addr, read_length, + command, NULL, 0); + } else { + count = cp2112_write_req(buf, addr, command, + data->block + 1, + data->block[0]); + } + break; case I2C_SMBUS_BLOCK_DATA: if (I2C_SMBUS_READ == read_write) { count = cp2112_write_read_req(buf, addr, @@ -781,6 +789,9 @@ static int cp2112_xfer(struct i2c_adapter *adap, u16 addr, case I2C_SMBUS_WORD_DATA: data->word = le16_to_cpup((__le16 *)buf); break; + case I2C_SMBUS_I2C_BLOCK_DATA: + memcpy(data->block + 1, buf, read_length); + break; case I2C_SMBUS_BLOCK_DATA: if (read_length > I2C_SMBUS_BLOCK_MAX) { ret = -EPROTO;