From patchwork Sun Oct 30 20:33:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620963 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7685DECAAA1 for ; Sun, 30 Oct 2022 20:34:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229761AbiJ3Ue0 (ORCPT ); Sun, 30 Oct 2022 16:34:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45288 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229587AbiJ3UeZ (ORCPT ); Sun, 30 Oct 2022 16:34:25 -0400 Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F394EB1E8; Sun, 30 Oct 2022 13:34:23 -0700 (PDT) Received: by mail-wr1-x436.google.com with SMTP id l14so13466439wrw.2; Sun, 30 Oct 2022 13:34:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ZSJlpADL6FxqQLVTkCQJu3ikcoUIWPyVlc8ef31tV88=; b=HB38NIcXWQdMiWE5240TE0Z0Eqj+ulKlHtjPFfNZihIzZMBdVTWGNxrTbU6ypSJDDS nkQYRLh11gDbPf4KBA52u+cKFdrXFQR513d6mcCiVV9mGr54FxpjVKaSOmeGKsbQZuhs AbfTzuRXxGPxYDeI89APAN3lu2jDaYxUQ3ROynLvDiIlBj6WppPut4IYEB9Sm/sU+c+w voO1luE48P2vMxEnIRu/0grETuVpUqMsPW1IPnvZ7qpxDTmRSu6QhwkTsye90boPOrtd Gu/+9qaps9itld+YsaR+FbKH7k207UvsL2R34okxJZacZ5eKzfnxHTDo7GqetAgCumwv 7VNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ZSJlpADL6FxqQLVTkCQJu3ikcoUIWPyVlc8ef31tV88=; b=sKwve+etjVdEyMxD4+2dyEpn7aN2LGKPeyaFwKj3r9C/5mdc/lYlqn2mAa9J+2KCJm oekjs26narbDIfNkQZZ2qNu3eacxJQWBR+lDqJCxhlNZMfuLUMI9ZRQLuz4Gf0khgSLI KaUiEbPEodQhZm2Vv3vKySmfBrJpN5TcCbR4YNCwiUMnSSt7PWM22I7JfS0xUo43VO68 jSm+I/ZclX4tNeURcQaM3bx/KAb4FxaaNrv3j7hVAQR/vqIV8KTDn3MzL78k/4M4JimE ihz565NjleFenWtS9DLa+uM8skZqcVJfhU3tpJSw25enVRg+cQ/xSGCFtPI+5OwvwCkz 4ZYA== X-Gm-Message-State: ACrzQf1aq1l99NWxxHQiKecQ8vvq8uo2mxZLX2Z+ENoqdTeoGRiHVZBH Z8/Fo4QmORfJeilZHXQ6SDA= X-Google-Smtp-Source: AMsMyM7QwNYThr9kT+OgTNBOSKxORRJ5xnHIHt/kXI5crESQlSllpCty1iLxbI5vnOW6ogbIHJtq4g== X-Received: by 2002:a05:6000:168f:b0:22e:4c3:de09 with SMTP id y15-20020a056000168f00b0022e04c3de09mr5822079wrd.40.1667162061603; Sun, 30 Oct 2022 13:34:21 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:21 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman , Guillaume Champagne Subject: [PATCH v3 01/12] HID: ft260: ft260_xfer_status routine cleanup Date: Sun, 30 Oct 2022 22:33:52 +0200 Message-Id: <20221030203403.4637-2-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org After clarifying with FTDI's support, it turned out that the error condition (bit 1) in byte 1 of the i2c status HID report is a status bit reflecting all error conditions. When bits 2, 3, or 4 are raised to 1, bit 1 is set to 1 also. Since the ft260_xfer_status routine tests the error condition bit and exits in the case of an error, the program flow never reaches the conditional expressions for 2, 3, and 4 bits when any of them indicates an error state. Though these expressions are never evaluated to true, they are checked several times per IO, increasing the ft260_xfer_status polling cycle duration. The patch removes the conditional expressions for 2, 3, and 4 bits in byte 1 of the i2c status HID report. Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne --- drivers/hid/hid-ft260.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 79505c64dbfe..a35201d68b15 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -313,27 +313,17 @@ static int ft260_xfer_status(struct ft260_device *dev) if (report.bus_status & FT260_I2C_STATUS_CTRL_BUSY) return -EAGAIN; - if (report.bus_status & FT260_I2C_STATUS_BUS_BUSY) - return -EBUSY; - - if (report.bus_status & FT260_I2C_STATUS_ERROR) + /* + * The error condition (bit 1) is a status bit reflecting any + * error conditions. When any of the bits 2, 3, or 4 are raised + * to 1, bit 1 is also set to 1. + */ + if (report.bus_status & FT260_I2C_STATUS_ERROR) { + hid_err(hdev, "i2c bus error: %#02x\n", report.bus_status); return -EIO; + } - ret = -EIO; - - if (report.bus_status & FT260_I2C_STATUS_ADDR_NO_ACK) - ft260_dbg("unacknowledged address\n"); - - if (report.bus_status & FT260_I2C_STATUS_DATA_NO_ACK) - ft260_dbg("unacknowledged data\n"); - - if (report.bus_status & FT260_I2C_STATUS_ARBITR_LOST) - ft260_dbg("arbitration loss\n"); - - if (report.bus_status & FT260_I2C_STATUS_CTRL_IDLE) - ret = 0; - - return ret; + return 0; } static int ft260_hid_output_report(struct hid_device *hdev, u8 *data, @@ -376,7 +366,7 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, break; } while (--try); - if (ret == 0 || ret == -EBUSY) + if (ret == 0) return 0; ft260_i2c_reset(hdev); From patchwork Sun Oct 30 20:33:53 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620130 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D4378C38A02 for ; Sun, 30 Oct 2022 20:34:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229802AbiJ3Ueg (ORCPT ); Sun, 30 Oct 2022 16:34:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45546 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229779AbiJ3Uee (ORCPT ); Sun, 30 Oct 2022 16:34:34 -0400 Received: from mail-wm1-x330.google.com (mail-wm1-x330.google.com [IPv6:2a00:1450:4864:20::330]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4C31DB85D; Sun, 30 Oct 2022 13:34:31 -0700 (PDT) Received: by mail-wm1-x330.google.com with SMTP id v7so371478wmn.0; Sun, 30 Oct 2022 13:34:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=RqowWq5NnPX1JsllkPtBTllVoOrDwgpWdvHnLuJ47J8=; b=CwgpuJYre/63XIBX6gYSnLWmoIcHc/GAG2FALtWpMOzm6AMEpmIV8HzK+7V2TFQHHm x0Ca06DfSF061J8aZNShZy0/X9k4rPtN4/TrxB2m1Xg/SRgb54N6gqgC8JYVNE6TgOcq oBmMc63VCyEAFYbrBxMcu5zraMvcOEgtFTA7wfx5T/NEJ1jzmrglS/OwNAIqkqkXym+T hJLTWp++zerbt/sBZbtQnQqc3Ypq9l4cBiFh01XYSpF04KJNyS/MtoPISaIXNSefyH9Q cc4UTXe6Q+OIw13YzymHtSn5HhqeYwW75MkUSNLD1iTtDegAz9k1UqYjwnOefiSjDjfF bC2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=RqowWq5NnPX1JsllkPtBTllVoOrDwgpWdvHnLuJ47J8=; b=HjH3AvxRx1UoH70qw/X9jMDfc1aSMEF4lxWB+QzE+H+SWStJ/QrTEKKUwl3oyPtdfy kMp9tZGOvlTnkpo8oRENZc5Jz3hKLWoQy8Dm8D88zayd/2z0QVnME/XeIuaWdYgXRrwp aQAKFHf7VyDMp9i9+roIBDzMO8No5KsE1Vhk1+HJBFZ8QG5PW+w2jSJkwpCIhutcerKL km17mGk54ePYbKgw8Pzz8XxUDP2kJJOvF9MqfSrbtP2oxSuYcFVmiTekUpuHj97MbGJ7 rSFq2uYPdojrF/xteIlwSVluMbnb+e5xAq887exE2uL5pYoylZLd2kfLcG/DhSjggQ7d kSng== X-Gm-Message-State: ACrzQf3imhpqZjZ7HGwwCT+nHZrB72vr29AbRqTRsH+zPI2LE2wDLFS7 DknYvaQOJm+wJBAknYEQmGE= X-Google-Smtp-Source: AMsMyM6fhNK8oWuf2HkHiUZ1S709zu84zs3Bk8/FHNlQEW8t/pMnaOe0Ma1+klh5H0fMkNRQ7Ty27Q== X-Received: by 2002:a1c:25c1:0:b0:3cf:4dc4:5a97 with SMTP id l184-20020a1c25c1000000b003cf4dc45a97mr14208076wml.147.1667162069673; Sun, 30 Oct 2022 13:34:29 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:29 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman , Guillaume Champagne Subject: [PATCH v3 02/12] HID: ft260: improve i2c write performance Date: Sun, 30 Oct 2022 22:33:53 +0200 Message-Id: <20221030203403.4637-3-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The patch improves the I2C write performance by 20 - 30 percent by revising the sleep time in the ft260_hid_output_report_check_status() in the following ways: 1. Reduce the wait time and start to poll earlier. Sending a large amount of data at a low I2C clock rate saturates the internal FT260 buffer and causes hiccups in status readiness, as shown below in the log fragment. Aligning the status check wait time to the worst case significantly reduces the write performance. [Oct22 10:28] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.005296] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.013460] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003244] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000190] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.015324] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003491] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000202] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.016047] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.002768] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000150] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011389] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003467] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000191] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000172] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000131] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000241] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000233] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000190] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000196] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011314] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 [ +0.003334] ft260_hid_output_report_check_status: wait 1920 usec, len 38 [ +0.000227] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000204] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000198] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000147] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.011060] ft260_i2c_write: rep 0xd8 addr 0x51 off 0 len 34 d[0] 0x0 Before: $ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 40510 80 256 8 32 After: $ sudo ./i2cperf -f 2 -o 2 -s 32 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 52584 80 256 8 32 2. Do not sleep if the estimated I2C transfer time is below 2 ms since the first xfer status query frequently takes around 1.5 ms, and the following status queries take about 200us on average. So we usually return from the routine after the first 1 - 3 status checks. [Oct22 11:14] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.004270] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.013889] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.000856] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000138] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.013352] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.001501] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000177] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.014477] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 [ +0.001377] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000233] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000191] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.013197] ft260_i2c_write: rep 0xd4 addr 0x51 off 0 len 18 d[0] 0x0 Before: $ sudo ./i2cperf -f 2 -o 2 -s 16 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 28826 73 256 16 16 After: $ sudo ./i2cperf -f 2 -o 2 -s 16 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 45138 73 256 16 16 Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne --- drivers/hid/hid-ft260.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index a35201d68b15..44106cadd746 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -345,7 +345,7 @@ static int ft260_hid_output_report(struct hid_device *hdev, u8 *data, static int ft260_hid_output_report_check_status(struct ft260_device *dev, u8 *data, int len) { - int ret, usec, try = 3; + int ret, usec, try = 100; struct hid_device *hdev = dev->hdev; ret = ft260_hid_output_report(hdev, data, len); @@ -356,10 +356,14 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, return ret; } - /* transfer time = 1 / clock(KHz) * 10 bits * bytes */ - usec = 10000 / dev->clock * len; - usleep_range(usec, usec + 100); - ft260_dbg("wait %d usec, len %d\n", usec, len); + /* transfer time = 1 / clock(KHz) * 9 bits * bytes */ + usec = len * 9000 / dev->clock; + if (usec > 2000) { + usec -= 1500; + usleep_range(usec, usec + 100); + ft260_dbg("wait %d usec, len %d\n", usec, len); + } + do { ret = ft260_xfer_status(dev); if (ret != -EAGAIN) From patchwork Sun Oct 30 20:33:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620962 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A82AFA3743 for ; Sun, 30 Oct 2022 20:34:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229786AbiJ3Ueh (ORCPT ); Sun, 30 Oct 2022 16:34:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45558 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229686AbiJ3Uee (ORCPT ); Sun, 30 Oct 2022 16:34:34 -0400 Received: from mail-wr1-x42e.google.com (mail-wr1-x42e.google.com [IPv6:2a00:1450:4864:20::42e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8F715B85F; Sun, 30 Oct 2022 13:34:31 -0700 (PDT) Received: by mail-wr1-x42e.google.com with SMTP id y16so13408739wrt.12; Sun, 30 Oct 2022 13:34:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=DNp683CGj+Z6itDhM79bg7tLFjPTOBd5dr9kEuZxIG0=; b=J+146yPeeqATBVRWMjweqpFAnEut2Emhc1ifayU5sYCeklrHz0D8RLneGUR3Mi3wpS uz6Zxnt4jz00CPRLJdp1RKKwzq0sO+HxbrFPB/GbQytn0TOivMpOe4H7iyVF/5kOvqcZ 7xtNto1uR8EpRY1JtRY2KVlSDFY4Awxe+kWuAPTobcAT4KUpCWoMadTmSktmoFiSJ4aN QxsAz1jBFhk9ftY8Lv/7lUN8YXN8YwNG9tQpWIcRFFYscU2OMSDVD4NXGn6EWWrkRNX2 XOfG1f8WDUWptO4MV1qhphsnwRK0prejFVxW9y+5lDn0mRETzdZRkL1RPnmdr4j6PVZ4 mg4g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=DNp683CGj+Z6itDhM79bg7tLFjPTOBd5dr9kEuZxIG0=; b=qLinXwCg1UdAPqi0GdznvxCKlReJbHtKlCpxrEKCM+AKCgRX1UkUVhxTM+5I7rfVSQ DkYSTCcqJwV173uw0yxhRlQHXBN2f/uGmuXIdvIIlfjckaNSmz1KxR1YkdznrNvW5RCx BNOMiE/kspNgXkDBHju5AmBWsUnfG65hqfXKGiwUGZe/4uJRH38cvAJluTmoJBET0IJy 37h2wE1gdjxkuCMMZY4CYSXWmTZYiRUhMEfsKuEeRv7DguCEuiwtZkRetH3hq/ZZcJlf y2+VIP4nGCHowGldXseF9MlZRhyZghNOxckU7nNNKDTGzVnofEfDuS6xLN3yxtDT5apM vALA== X-Gm-Message-State: ACrzQf3ryrzUw/XCUymTs43gGm8rdI4dA58C9S1JEMGFFlXzmYeONapD fboFBD7O/cUcoTinke2IAskDwqW7gAN8Yw== X-Google-Smtp-Source: AMsMyM5h+1XjQ+SoOEve5s2lUBHBKESIzzYp6JWSIslcb9uznXxX7zKc1BQvYjdCD58ulSyP7sRZmw== X-Received: by 2002:a5d:564c:0:b0:236:6089:cc50 with SMTP id j12-20020a5d564c000000b002366089cc50mr5632337wrw.520.1667162071077; Sun, 30 Oct 2022 13:34:31 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:30 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman , Guillaume Champagne Subject: [PATCH v3 03/12] HID: ft260: support i2c writes larger than HID report size Date: Sun, 30 Oct 2022 22:33:54 +0200 Message-Id: <20221030203403.4637-4-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org To support longer than one HID report size write, the driver splits a single i2c message data payload into multiple i2c messages of HID report size. However, it does not replicate the offset bytes within the EEPROM chip in every consequent HID report because it is not and should not be aware of the EEPROM type. It breaks the i2c write message integrity and causes the EEPROM device not to acknowledge the second HID report keeping the i2c bus busy until the ft260 controller reports failure. This patch preserves the i2c write message integrity by manipulating the i2c flag bits across multiple HID reports to be seen by the EEPROM device as a single i2c write transfer. Before: $ sudo ./i2cperf -f 2 -o 2 -s 64 -r 0-0xff 13 0x51 -S Error: Sending messages failed: Input/output error [ +3.667741] ft260_i2c_write: rep 0xde addr 0x51 off 0 len 60 d[0] 0x0 [ +0.007330] ft260_hid_output_report_check_status: wait 6400 usec, len 64 [ +0.000203] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_write: rep 0xd1 addr 0x51 off 60 len 6 d[0] 0x0 [ +0.002337] ft260_hid_output_report_check_status: wait 1000 usec, len 10 [ +0.000157] ft260_xfer_status: bus_status 0x2e, clock 100 [ +0.000241] ft260_i2c_reset: done [ +0.000003] ft260_i2c_write: failed to start transfer, ret -5 After: $ sudo ./i2cperf -f 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Fill block with increment via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 71260 86 256 2 128 Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne --- drivers/hid/hid-ft260.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 44106cadd746..cec83f69ebdc 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -378,41 +378,46 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, } static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, - int data_len, u8 flag) + int len, u8 flag) { - int len, ret, idx = 0; + int ret, wr_len, idx = 0; struct hid_device *hdev = dev->hdev; struct ft260_i2c_write_request_report *rep = (struct ft260_i2c_write_request_report *)dev->write_buf; + rep->flag = FT260_FLAG_START; + do { - if (data_len <= FT260_WR_DATA_MAX) - len = data_len; - else - len = FT260_WR_DATA_MAX; + if (len <= FT260_WR_DATA_MAX) { + wr_len = len; + if (flag == FT260_FLAG_START_STOP) + rep->flag |= FT260_FLAG_STOP; + } else { + wr_len = FT260_WR_DATA_MAX; + } - rep->report = FT260_I2C_DATA_REPORT_ID(len); + rep->report = FT260_I2C_DATA_REPORT_ID(wr_len); rep->address = addr; - rep->length = len; - rep->flag = flag; + rep->length = wr_len; - memcpy(rep->data, &data[idx], len); + memcpy(rep->data, &data[idx], wr_len); - ft260_dbg("rep %#02x addr %#02x off %d len %d d[0] %#02x\n", - rep->report, addr, idx, len, data[0]); + ft260_dbg("rep %#02x addr %#02x off %d len %d wlen %d flag %#x d[0] %#02x\n", + rep->report, addr, idx, len, wr_len, + rep->flag, data[0]); ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, - len + 4); + wr_len + 4); if (ret < 0) { - hid_err(hdev, "%s: failed to start transfer, ret %d\n", - __func__, ret); + hid_err(hdev, "%s: failed with %d\n", __func__, ret); return ret; } - data_len -= len; - idx += len; + len -= wr_len; + idx += wr_len; + rep->flag = 0; - } while (data_len > 0); + } while (len > 0); return 0; } From patchwork Sun Oct 30 20:33:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620129 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BDBD8FA3745 for ; Sun, 30 Oct 2022 20:34:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229832AbiJ3Uei (ORCPT ); Sun, 30 Oct 2022 16:34:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45596 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229772AbiJ3Ueg (ORCPT ); Sun, 30 Oct 2022 16:34:36 -0400 Received: from mail-wm1-x336.google.com (mail-wm1-x336.google.com [IPv6:2a00:1450:4864:20::336]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 15C61BC12; Sun, 30 Oct 2022 13:34:34 -0700 (PDT) Received: by mail-wm1-x336.google.com with SMTP id a11-20020a05600c2d4b00b003cf6f5fd9f1so774722wmg.2; Sun, 30 Oct 2022 13:34:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ALhJBetIf5X/SyaoXHlGdZsad0OG/1yQx1wQzt9ePhk=; b=qbY0Tl3+bXS2sSGNqVjdke0NooRIxBZtZNoVqbGH1GP2unMRmKhx2di7rzJYcmvPkY z44SzbkorqFS6odepQhc73eeJRrm3ROUhc1687dp/BhFpIc6FfoVwfEguoK4tZ92t+KY wwPLlIyL1yX+NeGnAtGHVLQfgVudnRBTAIO9wtBD+QjT6dGdLeSFA817bjJDQ/vw/769 klrweYpwFKLNdiPrKwGD8gctHJHpY488RLVyuSENvD7z8XyYlpunqH1eYkWLagF/+dMH ixADUhxjTrXWE9Fqy1CZB771psduc9byncqppJAMCfuaNInRAQbel458vieHlwdE23zf XNUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ALhJBetIf5X/SyaoXHlGdZsad0OG/1yQx1wQzt9ePhk=; b=s2K7CllR5HP+hfq/NHKc5I1cdrtAYiklgLJrs1I1KMuays1GB7k6ozeuUixoXEdSd+ bmCrFwPtk7LIPllKzSsJO09r/AGJN+CqdXhLwT1M5Y9ZUOUpoK8T78s9N47hvoR05eT4 rkXazOHG3k2Yhg4lD9CYMLYAWvXsm/3o0XwWWqMThGo3tWp2/i7q1qpKMoFR0gx2BuW1 gP40a7ts5EACUxdfBr5YWf/QQvlfLDwh0Rtrs6XllYJqib7wSruta/pA4StDSgRUXdX3 Xbooel97eB63mr4h9NmpQtiKmtG1Fu1ROZUfL4BL75JyOaScOlZo5cU1/QPvjtcx03OF z4Bw== X-Gm-Message-State: ACrzQf1PveQRU92ey0uow2iyHb1dJYsCx/wrRmUQvxWgiUHkywkGE2yB hx6bTK914cvuAn6OzkqhdPM= X-Google-Smtp-Source: AMsMyM4fJtooSmhksopxZCiWlxu6V5XShAi1peWVehD790HUqpBFGCIYDONDci5MZhoGheuby0jjlA== X-Received: by 2002:a1c:27c6:0:b0:3c2:e6df:c79b with SMTP id n189-20020a1c27c6000000b003c2e6dfc79bmr15539151wmn.14.1667162072572; Sun, 30 Oct 2022 13:34:32 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:32 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman , Guillaume Champagne Subject: [PATCH v3 04/12] HID: ft260: support i2c reads greater than HID report size Date: Sun, 30 Oct 2022 22:33:55 +0200 Message-Id: <20221030203403.4637-5-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org A random i2c read operation in EEPROM devices is implemented as a dummy write operation, followed by a current address read operation. The dummy write operation is used to load the target byte or word address (a.k.a offset) into the offset counter, from which the subsequent read operation then reads. To support longer than one HID report size random read, the ft260 driver issues multiple pairs of i2c write offset + read data transactions of HID report size so that the EEPROM device sees many i2c random read requests from different offsets. Two issues with the current implementation: - This approach suffers from extra overhead caused by writing offset requests. - Necessity to handle offset per HID report in big-endian representation as EEPROM devices expect. The current implementation does not do it and correctly handles the reads up to 60 bytes only. This patch addresses both issues by implementing more efficient approach. It issues a single i2c read request of up to the EEPROM page size and then waits for the data to arrive in multiple HID reports. For example, to read the 256 bytes from a 24LC512 chip, which has 128 bytes page size, the old method performs six ft260_i2c_write_read transactions while the new - two only. Before: $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 40803 85 256 2 128 Kernel log of a single 128 bytes read request: [ +2.376308] ft260_i2c_write_read: read_off 0x0 left_len 128 len 60 [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.000707] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000173] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 [ +0.008660] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000156] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_write_read: read_off 0x3c left_len 68 len 60 [ +0.000001] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x3c [ +0.001034] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000191] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 [ +0.008614] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000203] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_write_read: read_off 0x78 left_len 8 len 8 [ +0.000001] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x78 [ +0.000987] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000192] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 8 [ +0.002614] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.000200] ft260_xfer_status: bus_status 0x20, clock 100 After: $ sudo ./i2cperf -d 2 -o 2 -s 128 -r 0-0xff 13 0x51 -S Read block via i2ctransfer by chunks ------------------------------------------------------------------- data rate(bps) efficiency(%) data size(B) total IOs IO size(B) ------------------------------------------------------------------- 43990 85 256 2 128 Kernel log of a single 128 bytes read request: [ +1.464346] ft260_i2c_write_read: off 0x0 rlen 128 wlen 2 [ +0.000002] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.001653] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000188] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 128 rlen 60 flag 0x3 [ +0.008609] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000157] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 68 rlen 60 flag 0x0 [ +0.008840] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000203] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 8 rlen 8 flag 0x4 [ +0.002794] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.000201] ft260_xfer_status: bus_status 0x20, clock 100 Signed-off-by: Michael Zaidman Tested-by: Guillaume Champagne --- drivers/hid/hid-ft260.c | 127 +++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 61 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index cec83f69ebdc..a354089bb747 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -456,49 +456,62 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd, static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, u16 len, u8 flag) { + u16 rd_len; + int timeout, ret; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; - int timeout; - int ret; - if (len > FT260_RD_DATA_MAX) { - hid_err(hdev, "%s: unsupported rd len: %d\n", __func__, len); - return -EINVAL; - } + if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED) + flag = FT260_FLAG_START_REPEATED; + else + flag = FT260_FLAG_START; + do { + if (len <= FT260_RD_DATA_MAX) { + rd_len = len; + flag |= FT260_FLAG_STOP; + } else { + rd_len = FT260_RD_DATA_MAX; + } - dev->read_idx = 0; - dev->read_buf = data; - dev->read_len = len; + dev->read_idx = 0; + dev->read_buf = data; + dev->read_len = rd_len; - rep.report = FT260_I2C_READ_REQ; - rep.length = cpu_to_le16(len); - rep.address = addr; - rep.flag = flag; + rep.report = FT260_I2C_READ_REQ; + rep.length = cpu_to_le16(rd_len); + rep.address = addr; + rep.flag = flag; - ft260_dbg("rep %#02x addr %#02x len %d\n", rep.report, rep.address, - rep.length); + ft260_dbg("rep %#02x addr %#02x len %d rlen %d flag %#x\n", + rep.report, rep.address, len, rd_len, flag); - reinit_completion(&dev->wait); + reinit_completion(&dev->wait); - ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep)); - if (ret < 0) { - hid_err(hdev, "%s: failed to start transaction, ret %d\n", - __func__, ret); - return ret; - } + ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep)); + if (ret < 0) { + hid_err(hdev, "%s: failed with %d\n", __func__, ret); + return ret; + } - timeout = msecs_to_jiffies(5000); - if (!wait_for_completion_timeout(&dev->wait, timeout)) { - ft260_i2c_reset(hdev); - return -ETIMEDOUT; - } + timeout = msecs_to_jiffies(5000); + if (!wait_for_completion_timeout(&dev->wait, timeout)) { + ft260_i2c_reset(hdev); + return -ETIMEDOUT; + } - ret = ft260_xfer_status(dev); - if (ret == 0) - return 0; + ret = ft260_xfer_status(dev); + if (ret < 0) { + ft260_i2c_reset(hdev); + return -EIO; + } - ft260_i2c_reset(hdev); - return -EIO; + len -= rd_len; + data += rd_len; + flag = 0; + + } while (len > 0); + + return 0; } /* @@ -509,45 +522,37 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, */ static int ft260_i2c_write_read(struct ft260_device *dev, struct i2c_msg *msgs) { - int len, ret; - u16 left_len = msgs[1].len; - u8 *read_buf = msgs[1].buf; + int ret; + int wr_len = msgs[0].len; + int rd_len = msgs[1].len; + struct hid_device *hdev = dev->hdev; u8 addr = msgs[0].addr; u16 read_off = 0; - struct hid_device *hdev = dev->hdev; - if (msgs[0].len > 2) { - hid_err(hdev, "%s: unsupported wr len: %d\n", __func__, - msgs[0].len); + if (wr_len > 2) { + hid_err(hdev, "%s: invalid wr_len: %d\n", __func__, wr_len); return -EOPNOTSUPP; } - memcpy(&read_off, msgs[0].buf, msgs[0].len); - - do { - if (left_len <= FT260_RD_DATA_MAX) - len = left_len; + if (ft260_debug) { + if (wr_len == 2) + read_off = be16_to_cpu(*(u16 *)msgs[0].buf); else - len = FT260_RD_DATA_MAX; + read_off = *msgs[0].buf; - ft260_dbg("read_off %#x left_len %d len %d\n", read_off, - left_len, len); - - ret = ft260_i2c_write(dev, addr, (u8 *)&read_off, msgs[0].len, - FT260_FLAG_START); - if (ret < 0) - return ret; - - ret = ft260_i2c_read(dev, addr, read_buf, len, - FT260_FLAG_START_STOP); - if (ret < 0) - return ret; + pr_info("%s: off %#x rlen %d wlen %d\n", __func__, + read_off, rd_len, wr_len); + } - left_len -= len; - read_buf += len; - read_off += len; + ret = ft260_i2c_write(dev, addr, msgs[0].buf, wr_len, + FT260_FLAG_START); + if (ret < 0) + return ret; - } while (left_len > 0); + ret = ft260_i2c_read(dev, addr, msgs[1].buf, rd_len, + FT260_FLAG_START_STOP_REPEATED); + if (ret < 0) + return ret; return 0; } From patchwork Sun Oct 30 20:33:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620127 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D16C6C38A02 for ; Sun, 30 Oct 2022 20:34:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229822AbiJ3Ueo (ORCPT ); Sun, 30 Oct 2022 16:34:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45562 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229798AbiJ3Ueg (ORCPT ); Sun, 30 Oct 2022 16:34:36 -0400 Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B20FCB1E8; Sun, 30 Oct 2022 13:34:35 -0700 (PDT) Received: by mail-wr1-x42c.google.com with SMTP id bs21so13454278wrb.4; Sun, 30 Oct 2022 13:34:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=YSSvluQm3NGC5J6X42oGu/YUSo51/OoBhNLx5zGbzEw=; b=VsvBHYDiL+FDN9DKjUQwmovPnjmlpOfI/8bv6P5mWEioGjsozIiiVVcFhxPCGCP9D5 O5cG4jet50RlXXSxlw6c+hF+qnJ+VRmrdk3CNstRNmKddAKEy+vBq6iPeMeRcZQYKVa+ 39qSpX/f1Bo+xTb6CPwl9VahH57WB6Tks03XNWCIkQm4W7QKeI0whC+LvshAk4XpPbGr 1KAaVI8GxnFGbXdhAR4Wd+9assORv6r8tU0A+CcTKwL6ZItei2ONxOoQSRQGImkITV4x RMDmkJcBF3RxbKzY+23Z0zZYlTDyG8iFIf/eSy1rOpTHavtXirPvKaeaYfFmUnZ9JucR fBtQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YSSvluQm3NGC5J6X42oGu/YUSo51/OoBhNLx5zGbzEw=; b=zh3qc3nhGYs9ySlJKW6h6+9FdZDnDKLKfA7KeOZV2TJHovJGHZ3irfrajRMtnJH9d5 GeZ0QQyzuYQvxGQ9h+dNXRQlwaOKFxQAtFYT94DlWr+gUl4A4jFzxRalZGmIDNXIdbGb 8e1A7m31r+QjzyDWrIIS3KKeEvqqgjyvjwRsSxcEcboVtRLDqTODvXpAQ24vOK4f9Cp6 eZfJCTQArHx9E5p51LCvxunij/ChBr2yueCpMZZgcKWCmPC8VvrnEL6JNLZFrOGkCw97 RB9UGHMEP8c4YuMPN+aBD4F8NNn794TJDCnSzmZe1JqsdBebIu5fyBJrINMZoa9lwzU4 hXyQ== X-Gm-Message-State: ACrzQf286G0nBXF8CgWTHHcMXEpSCGrc5bankHF3D8fC33utSRET7m3S 6/58QxsuCToRRrmQgiawsvM= X-Google-Smtp-Source: AMsMyM6hCI+tWbEhji8iIg5DuxmfJQxC/JVelrtGfB2kVWhzw1ZEzAiDs+DSWJIqn/NmN6zCjXRSXA== X-Received: by 2002:a5d:5f04:0:b0:236:cae9:2991 with SMTP id cl4-20020a5d5f04000000b00236cae92991mr1321897wrb.120.1667162073985; Sun, 30 Oct 2022 13:34:33 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:33 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman Subject: [PATCH v3 05/12] HID: ft260: improve i2c large reads performance Date: Sun, 30 Oct 2022 22:33:56 +0200 Message-Id: <20221030203403.4637-6-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The patch increases the read buffer size to 180 bytes. It reduces the number of ft260_i2c_read() calls by three, improving the big reads performance. $ sudo i2ctransfer -y -f 13 w2@0x51 0x0 0x0 r180 Before: [ +4.071878] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2 [ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.001097] ft260_xfer_status: bus_status 0x41, clock 100 [ +0.000175] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000004] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 60 flag 0x3 [ +0.008579] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000208] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 120 rlen 60 flag 0x0 [ +0.008794] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000181] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000002] ft260_i2c_read: rep 0xc2 addr 0x51 len 60 rlen 60 flag 0x4 [ +0.008817] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000223] ft260_xfer_status: bus_status 0x20, clock 100 After: [ +11.611642] ft260_i2c_write_read: off 0x0 rlen 180 wlen 2 [ +0.000005] ft260_i2c_write: rep 0xd0 addr 0x51 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.008001] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000001] ft260_i2c_read: rep 0xc2 addr 0x51 len 180 rlen 180 flag 0x7 [ +0.008994] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.007987] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.007992] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.000206] ft260_xfer_status: bus_status 0x20, clock 100 Suggested-by: Enrik Berkhan Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index a354089bb747..91f9087e49dc 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -30,12 +30,19 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages"); #define FT260_REPORT_MAX_LENGTH (64) #define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4) + /* - * The input report format assigns 62 bytes for the data payload, but ft260 - * returns 60 and 2 in two separate transactions. To minimize transfer time - * in reading chunks mode, set the maximum read payload length to 60 bytes. - */ -#define FT260_RD_DATA_MAX (60) + * The ft260 input report format defines 62 bytes for the data payload, but + * when requested 62 bytes, the controller returns 60 and 2 in separate input + * reports. To achieve better performance with the multi-report read data + * transfers, we set the maximum read payload length to a multiple of 60. + * With a 100 kHz I2C clock, one 240 bytes read takes about 1/27 second, + * which is excessive; On the other hand, some higher layer drivers like at24 + * or optoe limit the i2c reads to 128 bytes. To not block other drivers out + * of I2C for potentially troublesome amounts of time, we select the maximum + * read payload length to be 180 bytes. +*/ +#define FT260_RD_DATA_MAX (180) #define FT260_WR_DATA_MAX (60) /* From patchwork Sun Oct 30 20:33:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620961 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5EA66C38A02 for ; Sun, 30 Oct 2022 20:34:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229827AbiJ3Uek (ORCPT ); Sun, 30 Oct 2022 16:34:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45548 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229828AbiJ3Uei (ORCPT ); Sun, 30 Oct 2022 16:34:38 -0400 Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CCA0EB7CF; Sun, 30 Oct 2022 13:34:36 -0700 (PDT) Received: by mail-wr1-x431.google.com with SMTP id g12so13430987wrs.10; Sun, 30 Oct 2022 13:34:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=l/pgm5Us2Yq+S8ZFCwq+NZ3vvwHGv8IPDUFxkuklnRI=; b=iW9FK1s7jeYS8FugmovRAodcRRrFtv6uJZOAKP8ZobyyvpnbTmg8lqdrVbkZ9je5i8 biqVeGVOseZwJuFgTide2IE67TulaYq6JTFc9YZt/DyLHaB65zhyMaIfXOFfWMJWz28U yE7MelMUVBrnFNBcxPeFt9706lWuXexFLn5kk5kMAWF+4hPjnbIFmDmv0lp0o4BtaC08 ZgoucQG0dYA9wSVVlUjl8qPQxTNLGw6GMjePJOZBleFJyUI6loTkSwInQvFItLc6codI /p8wbifXhlWQTYwtsvOihkVCpQolqPfH/CiXqDwHwD0i/oT+iLGp99niK+Bzd96/717H Oytw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=l/pgm5Us2Yq+S8ZFCwq+NZ3vvwHGv8IPDUFxkuklnRI=; b=JYDYGijt9mM5c78pqiz+98CSr1W+4Isauc9SRyMrgV0xhCMhxHNq7OxnQm/CUDjvJm MyWc4E2xwWf77bqqmTiDdXKkrOmdHH/r9+CUVMj3fd4OZ6We40IC+S0gTviFXD9E9DMy tlLMWsVta5BAV4+pj77YGW2Uu5j1b57b5ryjHeLMPN0651o5p3seLYHEq8+rNXV578QM pSrdFZ9KTvtmS03xb7LiDRhoVV/4k4ESMoMUD5RSvHbx2YPQqgD7U4MNlerYS1OMZITI H1Q6aJtRB8X0+hiOgnNHpkfr940n+o+xRY66P/jK8tSmNOhriJP58272J8Y5OSFhCC09 /xiQ== X-Gm-Message-State: ACrzQf2zDgwsutzPB4HR+7RwkPzYKgnpDRKZ9vuGC0GfsROC2lMNqDyV KTeaWgpcD3YOAKeU9utqKW4= X-Google-Smtp-Source: AMsMyM5xBmH9rpx2lt8kPqHmeJ3xhg6PE9aOYB2zutgKiILKug3qf76RstBfVmbCoM7DRY5U24cErQ== X-Received: by 2002:adf:e54a:0:b0:236:bf8a:4782 with SMTP id z10-20020adfe54a000000b00236bf8a4782mr2733716wrm.442.1667162075367; Sun, 30 Oct 2022 13:34:35 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:34 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman Subject: [PATCH v3 06/12] HID: ft260: do not populate /dev/hidraw device Date: Sun, 30 Oct 2022 22:33:57 +0200 Message-Id: <20221030203403.4637-7-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org Do not populate the /dev/hidraw on ft260 interfaces when the hid-ft260 driver is loaded. $ sudo insmod hid-ft260.ko $ ls /dev/hidraw* /dev/hidraw0 $ sudo rmmod hid-ft260.ko $ ls /dev/hidraw* /dev/hidraw0 /dev/hidraw1 /dev/hidraw2 Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 91f9087e49dc..8d6d2a19b9ed 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -939,7 +939,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) return ret; } - ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW); + ret = hid_hw_start(hdev, 0); if (ret) { hid_err(hdev, "failed to start HID HW\n"); return ret; @@ -966,6 +966,10 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) if (ret <= 0) goto err_hid_close; + hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", + hdev->version >> 8, hdev->version & 0xff, hdev->name, + hdev->phys); + hid_set_drvdata(hdev, dev); dev->hdev = hdev; dev->adap.owner = THIS_MODULE; @@ -974,8 +978,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) dev->adap.quirks = &ft260_i2c_quirks; dev->adap.dev.parent = &hdev->dev; snprintf(dev->adap.name, sizeof(dev->adap.name), - "FT260 usb-i2c bridge on hidraw%d", - ((struct hidraw *)hdev->hidraw)->minor); + "FT260 usb-i2c bridge"); mutex_init(&dev->lock); init_completion(&dev->wait); From patchwork Sun Oct 30 20:33:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620128 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D7FE4FA3744 for ; Sun, 30 Oct 2022 20:34:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229851AbiJ3Uem (ORCPT ); Sun, 30 Oct 2022 16:34:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45658 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229750AbiJ3Uej (ORCPT ); Sun, 30 Oct 2022 16:34:39 -0400 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9F623A1BB; Sun, 30 Oct 2022 13:34:38 -0700 (PDT) Received: by mail-wm1-x32a.google.com with SMTP id m29-20020a05600c3b1d00b003c6bf423c71so9723699wms.0; Sun, 30 Oct 2022 13:34:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=47oIaMFA8Eg3AOKmQgHp46WYx0yoJU5i2kodk4hvPko=; b=pilNXscJ9s6usj1pxwynivWIPr7JfKzl/Alrws79R1/YB/vgCLZeb4Y+8htmPlCFiu TR5nOrZEkYU7BcYZoFS6azjAatYPaCfR55USMKczXC+0WLEMxpavLnVLlPjI3UWTbdDH z4wXhmeqGmZjWHezIZllQ1WTNlbaYwUgQWEWqeGsxGlFvzy9rOoD+HHaUOTulGDBH9fN vc5RlkYwfK5F8cXrKpvvAG+tugabhbQBJln2SY2j29hNz+IM8OrGELmGxVGTOR3muba2 AmbWITELFnQJc3KB4FXfjvuXs0SQLAtZM9UeBtsV/rpnDmmGQt/GDkjzNrstD2Wn59i3 hl6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=47oIaMFA8Eg3AOKmQgHp46WYx0yoJU5i2kodk4hvPko=; b=uHs3YonakTc2F6PsSeU9+Kv3lzFMf35MHdwbsbEnk4onRbxluPFYukbk2H4mNAQuUr 6oMZaJsWA8Bd9vIShN4L/V8uydjqZfUzTS9R7JH5JY/CI56NDg013QkWEH/1X47u/vYl WzvHYhfE71Z3EXq/15otbWh5yefvBcfqkUuEf9HWCpv+rx60DS1mGqS64Kqw8ARAFMq1 MbFBO5X7pR5llzFghfdbHrFSW9GkCJP7ANUJm9I+qu/7GVkhIThV4YjuVA2K1vLRsHKy Q+CxglPuToFcYM4Ck3O02PLLKm/x+TThnevy9ejiyNXv/sbIDh1fYBjD8DhLO97FPLIC 0FVA== X-Gm-Message-State: ACrzQf3WqBxiouWYHnpdaUi5gBCuZaz4mYQCTzkasUQByVwOZ8CHeZJ+ bN4XFMd3LC1gYDtoHS9obBo= X-Google-Smtp-Source: AMsMyM7xVKy+uRshU7d09XCHKwJzC+eXYxWtE3kvvr/8W6rVgCYH9A/DBRZVkcdxIr6qvZ45rXNcWQ== X-Received: by 2002:a05:600c:44d5:b0:3cf:6749:afe3 with SMTP id f21-20020a05600c44d500b003cf6749afe3mr4916090wmo.90.1667162076777; Sun, 30 Oct 2022 13:34:36 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:36 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman Subject: [PATCH v3 07/12] HID: ft260: skip unexpected HID input reports Date: Sun, 30 Oct 2022 22:33:58 +0200 Message-Id: <20221030203403.4637-8-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The FT260 is not supposed to generate unexpected HID reports. However, in theory, the unsolicited HID Input reports can be issued by a specially crafted malicious USB device masquerading as FT260 when the attacker has physical access to the USB port. In this case, the read_buf pointer points to the final data portion of the previous I2C Read transfer, and the memcpy invoked in the ft260_raw_event() will try copying the content of the unexpected report into the wrong location. This commit sets the Read buffer pointer to NULL on the I2C Read transaction completion and checks it in the ft260_raw_event() to detect and skip the unsolicited Input report. Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 8d6d2a19b9ed..8b6ebc5228eb 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -464,7 +464,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, u16 len, u8 flag) { u16 rd_len; - int timeout, ret; + int timeout, ret = 0; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; @@ -480,10 +480,6 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, rd_len = FT260_RD_DATA_MAX; } - dev->read_idx = 0; - dev->read_buf = data; - dev->read_len = rd_len; - rep.report = FT260_I2C_READ_REQ; rep.length = cpu_to_le16(rd_len); rep.address = addr; @@ -494,22 +490,30 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, reinit_completion(&dev->wait); + dev->read_idx = 0; + dev->read_buf = data; + dev->read_len = rd_len; + ret = ft260_hid_output_report(hdev, (u8 *)&rep, sizeof(rep)); if (ret < 0) { hid_err(hdev, "%s: failed with %d\n", __func__, ret); - return ret; + goto ft260_i2c_read_exit; } timeout = msecs_to_jiffies(5000); if (!wait_for_completion_timeout(&dev->wait, timeout)) { + ret = -ETIMEDOUT; ft260_i2c_reset(hdev); - return -ETIMEDOUT; + goto ft260_i2c_read_exit; } + dev->read_buf = NULL; + ret = ft260_xfer_status(dev); if (ret < 0) { + ret = -EIO; ft260_i2c_reset(hdev); - return -EIO; + goto ft260_i2c_read_exit; } len -= rd_len; @@ -518,7 +522,9 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, } while (len > 0); - return 0; +ft260_i2c_read_exit: + dev->read_buf = NULL; + return ret; } /* @@ -1036,6 +1042,13 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report, ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report, xfer->length); + if ((dev->read_buf == NULL) || + (xfer->length > dev->read_len - dev->read_idx)) { + hid_err(hdev, "unexpected report %#02x, length %d\n", + xfer->report, xfer->length); + return -1; + } + memcpy(&dev->read_buf[dev->read_idx], &xfer->data, xfer->length); dev->read_idx += xfer->length; @@ -1044,10 +1057,9 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report, complete(&dev->wait); } else { - hid_err(hdev, "unknown report: %#02x\n", xfer->report); - return 0; + hid_err(hdev, "unhandled report %#02x\n", xfer->report); } - return 1; + return 0; } static struct hid_driver ft260_driver = { From patchwork Sun Oct 30 20:33:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620960 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7C48EFA3746 for ; Sun, 30 Oct 2022 20:34:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229782AbiJ3Uen (ORCPT ); Sun, 30 Oct 2022 16:34:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45704 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229835AbiJ3Uek (ORCPT ); Sun, 30 Oct 2022 16:34:40 -0400 Received: from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com [IPv6:2a00:1450:4864:20::32d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E2E58B1FE; Sun, 30 Oct 2022 13:34:39 -0700 (PDT) Received: by mail-wm1-x32d.google.com with SMTP id c3-20020a1c3503000000b003bd21e3dd7aso9696544wma.1; Sun, 30 Oct 2022 13:34:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=qGr1gPz8zB0Ciz7kPlieGQfpDM11emH9iwgNzYsPKO4=; b=hdSxL/jp05gPlg7e/8Z5vI/+iGjPCAkTJ1evp/DDgm5VbtZm9rHIdGasoFiJrwe6Uv K3q/NMLi1l+Le7fJI1/icK4qsUAXRWyb9vCCRkaBBahI0nn/BP+68IuKoP8se88xaI+n mcMTnhwiZ3u2nu3ESu9IKIpyn1zYxVg9V8H1c5wuknX0EP83UU9YcCH+slEykxjroOLr S8ajmYL3HoWs7+DghMz6zMy0hbNlUhgV8crMq7L2TDlj26H9QdYTWJoyoAQeJegz+5IA n69dDFb4Nnw2Wn9XoUiul/jPsA/XFcSWTPRTy/3+vf1Ct1QzcOolnVnDr/jKp3riMiB4 CWJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=qGr1gPz8zB0Ciz7kPlieGQfpDM11emH9iwgNzYsPKO4=; b=aEUcSRZloTT3tF9B6xDEIKuEfWbxmr5yBGrGRxqMugoYIWBB9AIPRJgHRT0FzBBYqm njxTEZ2IulJLVHGvyx2D5n6+cnOvuQZwff/C+hFL3Ck/8F2vPXqoT+5v0e4fliZR1ATF 9ogTriu+cDGNZnyoqmQx5Sr+rbbd4OaWoDNu7nyQvx9e7WwG7mzNdMx6OBV6oKLzV3NX RjKZ/gRjHvdrMlLdg1OTcx1MLicEIaqM3eom1/A1STB2YnoNugmdrRk2xACoXs4T5XWN HMLZmeL4EhdezQ66t2o5I1FaCri1Ln+Hmf1C/aQcCGI2Kl33gGjLuIPEeN9WoJsoyQkl 06TQ== X-Gm-Message-State: ACrzQf0wLRaKEdIXJc940TgA6SF3egKgAIGMYC9l7wecZmHq4jf6L+Af p0XT0/ML9K29w9oU5iTUm5bTJIMbavlaUQ== X-Google-Smtp-Source: AMsMyM6iK4kumvV6kjVG5x+UBZysodHXQQp6lmoKNDwSZLL0A4bDc3wDTtn6rSzFl1fTGsPDbmfbbw== X-Received: by 2002:a1c:7405:0:b0:3cf:55ea:6520 with SMTP id p5-20020a1c7405000000b003cf55ea6520mr5972572wmc.46.1667162078327; Sun, 30 Oct 2022 13:34:38 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:37 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman , Vince Asbridge , Stephen Shirron Subject: [PATCH v3 08/12] HID: ft260: remove SMBus Quick command support Date: Sun, 30 Oct 2022 22:33:59 +0200 Message-Id: <20221030203403.4637-9-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The i2cdetect uses the SMBus Quick command by default to scan devices on the I2C bus. The FT260 implements an I2C bus controller. The SMBus is derived from I2C, but there are several differences between the specifications of the two buses in the areas of timing, protocols, operation modes, and electrical characteristics. One of the differences is that the I2C devices allow the slave not to ACK its slave address, but SMBus requires it to always ACK it as a mechanism to detect a detachable device’s presence on the bus. Since FT260 is the I2C bus controller, it does not acknowledge the SMBus Quick write command, which sends a single bit to the device at the place of the RD/WR bit. The ft260 driver attempted to mimic the SMBus Quick Write functionality by writing a single byte as the SMBus Byte Write command does. Usually, one byte in the SMBus Quick Write will be fine. However, it may cause problems with devices with a control register at offset 0, like i2c muxes, for example, when scanned with the i2cdetect utility. The i2cdetect with the "-r" option uses the SMBus Read Byte command, which is a reasonable workaround. To prevent the I2C bus from locking at write-only devices (most notably clock chips at address 0x69), use the "-r" option in conjunction with scanning range parameters. This patch removes the SMBus Quick command support. $ sudo i2cdetect -y 13 Warning: Can't use SMBus Quick Write command, will skip some addresses 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: 10: 20: 30: -- -- -- -- -- -- -- -- 40: 50: 50 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: 70: $ sudo i2cdetect -y -r 13 0 1 2 3 4 5 6 7 8 9 a b c d e f 00: -- -- -- -- -- -- -- -- 10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 50: 50 51 -- -- -- -- -- -- -- -- -- -- -- -- -- -- 60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- 70: -- -- -- -- -- -- -- -- Reported-by: Vince Asbridge Reported-by: Stephen Shirron Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 8b6ebc5228eb..d186aa5a8e73 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -630,14 +630,6 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags, } switch (size) { - case I2C_SMBUS_QUICK: - if (read_write == I2C_SMBUS_READ) - ret = ft260_i2c_read(dev, addr, &data->byte, 0, - FT260_FLAG_START_STOP); - else - ret = ft260_smbus_write(dev, addr, cmd, NULL, 0, - FT260_FLAG_START_STOP); - break; case I2C_SMBUS_BYTE: if (read_write == I2C_SMBUS_READ) ret = ft260_i2c_read(dev, addr, &data->byte, 1, @@ -720,7 +712,7 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags, static u32 ft260_functionality(struct i2c_adapter *adap) { - return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_QUICK | + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_I2C_BLOCK; } From patchwork Sun Oct 30 20:34:00 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620959 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A67EFC38A02 for ; Sun, 30 Oct 2022 20:35:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229865AbiJ3UfB (ORCPT ); Sun, 30 Oct 2022 16:35:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45726 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229866AbiJ3Uem (ORCPT ); Sun, 30 Oct 2022 16:34:42 -0400 Received: from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com [IPv6:2a00:1450:4864:20::32d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6DF8FA1A9; Sun, 30 Oct 2022 13:34:41 -0700 (PDT) Received: by mail-wm1-x32d.google.com with SMTP id n39-20020a05600c3ba700b003cf71011cddso335010wms.1; Sun, 30 Oct 2022 13:34:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=yAkHqN6oZd2dovWsENCSlIqD0VO6aehIZ8ArRH69xz8=; b=F4MXqGE9UQW3kaFZow314uGnPwlkkcUy93RM28Ql+tuVGTEOUkyx7/bPhZC1Z3tZiO Qf2MtRxiM/eZwtIZRysh7/GKoqsGPI4pSP9ZmVfQrgr9mWhCUXNFOIZ74oCOQ3Xv/EPD o+pT1Xn+t+eVENM/NYyfK6DlEOFk0PB0aKfx1nnEWjwmlqwLcYuqIA+fLKhndHg4mL6S ywQWsYOPu0og1RqppJWUFJu9uzLe+h2usjCJXILCK4kmElSpS5HUhB5X7VR/sLNg/50m X8POPgYHaajoDl5yqVXtnkfNH8TBIE/iRhtd0R7ZS5wvjOKbx2Ozq6Bm18O6xezE3jqh TmnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yAkHqN6oZd2dovWsENCSlIqD0VO6aehIZ8ArRH69xz8=; b=JSPLxeqaSLPBzfWbXlkTIMKUcfSwWFS5AgOAYu4CfEKMOGpWb/VwjiqsqF37xd0KkH P5f7ytNs6MEX3Z+rhp/CJZn41rZ2WN7JtIuJwGDna88rxqo/aXndjCwBsoha7op6OKka C6uVVNpW9I0i1RJnndzgvCxp8wVHjPKNx9vLdyxJfw++Cbq1km6UJwNHFJ0amvUmFKgJ 41wnBDqBn31kbay0JB7wq4DQbRaXLCT+aogsaQKmXo7seYC2Q1JvH2+awPxEaxQnMjTh c+AESTIiS/MNt/Q3/3Pw7S+WvxCnfCyKKhsd1iUlevM4qRwstBHgd1W8TrV9fSdAy0L6 HKbw== X-Gm-Message-State: ACrzQf320BVvy6YCS8NrQLXWGTaQmr1F6AXZG/ikvdRVgKAc+2PUErpK GcyTVm5xdD1iGmObQ3U318o= X-Google-Smtp-Source: AMsMyM4ivnUkI4EN2HuHFheEejNYCvrW4t0H4IEqgyPxJC+eoMDkNVxgWwp4nP98UG5iILMUWZ77bA== X-Received: by 2002:a05:600c:14c6:b0:3cf:6a83:c2a6 with SMTP id i6-20020a05600c14c600b003cf6a83c2a6mr3445866wmh.52.1667162079779; Sun, 30 Oct 2022 13:34:39 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:39 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman Subject: [PATCH v3 09/12] HID: ft260: missed NACK from big i2c read Date: Sun, 30 Oct 2022 22:34:00 +0200 Message-Id: <20221030203403.4637-10-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The FT260 controller does not return NACK when performing a big read (of multiple hid reports size) from a non-existing device or from the device responding with NACK when it is not ready to serve the request. However, it responds correctly with NACK to a read of up to a single hid report size. To overcome this issue, we split the muli-report read request into a read of a single HID report of 60 bytes size and a multi-report read. Big read of 256 bytes with first read of 60 bytes: $ sudo ./i2cperf -d 2 -o 2 -s 256 -r 0-0xff 1 0x50 -S [ +5.633280] ft260_i2c_write_read: off 0x0 rlen 255 wlen 2 [ +0.000006] ft260_i2c_write: rep 0xd0 addr 0x50 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.013205] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000007] ft260_i2c_read: rep 0xc2 addr 0x50 len 255 rlen 60 flag 0x3 [ +0.010932] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.004733] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000006] ft260_i2c_read: rep 0xc2 addr 0x50 len 195 rlen 128 flag 0x0 [ +0.012572] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.005789] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.003189] ft260_raw_event: i2c resp: rep 0xd1 len 8 [ +0.004092] ft260_xfer_status: bus_status 0x40, clock 100 [ +0.000010] ft260_i2c_read: rep 0xc2 addr 0x50 len 67 rlen 67 flag 0x4 [ +0.011688] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.004700] ft260_raw_event: i2c resp: rep 0xd1 len 7 [ +0.004858] ft260_xfer_status: bus_status 0x20, clock 100 Read from non-existing device at address 8. The first 60 read responded with NACK. $ sudo ./i2cperf -d 2 -o 2 -s 256 -r 0-0xff 1 0x8 -S [Oct19 15:37] ft260_i2c_write_read: off 0x0 rlen 255 wlen 2 [ +0.000007] ft260_i2c_write: rep 0xd0 addr 0x8 off 0 len 2 wlen 2 flag 0x2 d[0] 0x0 [ +0.022820] ft260_xfer_status: bus_status 0x20, clock 100 [ +0.000007] ft260_i2c_read: rep 0xc2 addr 0x8 len 255 rlen 60 flag 0x3 [ +0.010658] ft260_raw_event: i2c resp: rep 0xde len 60 [ +0.005965] ft260_xfer_status: bus_status 0x46, clock 100 <-- NACK [ +0.000009] ft260 0003:0403:6030.0004: i2c bus error: 0x46 [ +0.007784] ft260_i2c_reset: done Co-developed-by: Enrik Berkhan Signed-off-by: Enrik Berkhan Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index d186aa5a8e73..40fae81386e3 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -464,6 +464,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, u16 len, u8 flag) { u16 rd_len; + u16 rd_data_max = 60; int timeout, ret = 0; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; @@ -473,12 +474,13 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, else flag = FT260_FLAG_START; do { - if (len <= FT260_RD_DATA_MAX) { + if (len <= rd_data_max) { rd_len = len; flag |= FT260_FLAG_STOP; } else { - rd_len = FT260_RD_DATA_MAX; + rd_len = rd_data_max; } + rd_data_max = FT260_RD_DATA_MAX; rep.report = FT260_I2C_READ_REQ; rep.length = cpu_to_le16(rd_len); From patchwork Sun Oct 30 20:34:01 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620126 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A17D4C38A02 for ; Sun, 30 Oct 2022 20:35:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229917AbiJ3UfK (ORCPT ); Sun, 30 Oct 2022 16:35:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45792 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229824AbiJ3Ueo (ORCPT ); Sun, 30 Oct 2022 16:34:44 -0400 Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 88071B859; Sun, 30 Oct 2022 13:34:42 -0700 (PDT) Received: by mail-wm1-x32f.google.com with SMTP id b20-20020a05600c4e1400b003cc28585e2fso6976331wmq.1; Sun, 30 Oct 2022 13:34:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=j8YVAuzbfJpfqbUz/i1mMiECgUpENTS2gXYifLvndPY=; b=nhZFx0fMnfBA/bYVNDsfLk8FSEqSmIDg+n823lgrWpAmbTaSEC4F2iW/D4zWZQ4cBB joKT8ZzRTMfAqtMlmpVMeuuW3hcmsqDyJ0+e5K2APeJUAoRiw3ieYSmX3dn2iIspWc9/ umu+Om46pcrN7htlWomI/QaDG2IQEWIeDS1OX4ga+VJfiLKyPwHjjndr3hNZNSFUtkMd PRt32YmYunYO+eeXwP6F7zRlJmgXLVbbWdGfm90C03XG+VbptCMy3FshpkiGpCjLxTDQ z935YYys9fPzc1eCSM8syAw+dyknAs7eUJPv2blHPaPwlAG7E3IjJifUte2/PVr6RdZh +eLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=j8YVAuzbfJpfqbUz/i1mMiECgUpENTS2gXYifLvndPY=; b=PXzqhSY09TfoGTOBQv6qcq580Cp2ErqLCPypWRiwWs+AwHBmasL4lpaG/1bTsVmHM0 wRia+kLu7sTkslRQ/s3imYZ7eD7ua3UV7RQrr3NJzzrBOQaCAB4TV1r+gt+M3Lw1xRZ5 p4lewvfPMLxwkcWlCKSmGczi8ApTbQ0i4yGrv8HdNmbFukIG31nSpDUoYod3yhyTYJOy PkXRy1jhDxAUReI30noUwnibyzypQnXvsdPmluntjr5XinZpT9JawvQWun0hkSL6oPCb pJ93yBRhcmHMakzFzS2lVA9QFxCWgKBxsbzBqb/44zH1OyxQ2f34Q466xHPwmzj1UMTS fV+A== X-Gm-Message-State: ACrzQf3J0Qu1pwV5e6g3E/230fr/Q+9J3UIF3GAmb2rYqT0CBq23CDI3 PLaYAI3to6HfuHfCVVG/tBQ= X-Google-Smtp-Source: AMsMyM6cbf1clV5GonDt1YvNrWhuvr0GfPS8r3IcnhhFD6UhpNUDYyg3Zbp4uXql0tRSFZ81SMOZag== X-Received: by 2002:a05:600c:4ed2:b0:3c6:c1ff:222 with SMTP id g18-20020a05600c4ed200b003c6c1ff0222mr5769621wmq.163.1667162081112; Sun, 30 Oct 2022 13:34:41 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:40 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman Subject: [PATCH v3 10/12] HID: ft260: wake up device from power saving mode Date: Sun, 30 Oct 2022 22:34:01 +0200 Message-Id: <20221030203403.4637-11-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The FT260 can enter a power saving mode after being idle for longer than 5 seconds. When being woken up from power saving mode by an I2C write request, it looks like a possible NACK will not be correctly reported back. As a workaround, the driver will request an I2C status report before starting the next I2C transfer if the FT260 has been idle for longer than 4800ms. Signed-off-by: Enrik Berkhan Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 40fae81386e3..00cbe7693ba0 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -31,6 +31,8 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages"); #define FT260_REPORT_MAX_LENGTH (64) #define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4) +#define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */ + /* * The ft260 input report format defines 62 bytes for the data payload, but * when requested 62 bytes, the controller returns 60 and 2 in separate input @@ -237,6 +239,7 @@ struct ft260_device { struct completion wait; struct mutex lock; u8 write_buf[FT260_REPORT_MAX_LENGTH]; + unsigned long need_wakeup_at; u8 *read_buf; u16 read_idx; u16 read_len; @@ -392,6 +395,12 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, struct ft260_i2c_write_request_report *rep = (struct ft260_i2c_write_request_report *)dev->write_buf; + + if (time_is_before_jiffies(dev->need_wakeup_at)) { + (void)ft260_xfer_status(dev); + ft260_dbg("device wakeup"); + } + rep->flag = FT260_FLAG_START; do { @@ -441,6 +450,11 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd, if (data_len >= sizeof(rep->data)) return -EINVAL; + if (time_is_before_jiffies(dev->need_wakeup_at)) { + (void)ft260_xfer_status(dev); + ft260_dbg("device wakeup"); + } + rep->address = addr; rep->data[0] = cmd; rep->length = data_len + 1; @@ -607,6 +621,8 @@ static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, ret = num; i2c_exit: + dev->need_wakeup_at = + jiffies + msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS); hid_hw_power(hdev, PM_HINT_NORMAL); mutex_unlock(&dev->lock); return ret; @@ -707,6 +723,8 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags, } smbus_exit: + dev->need_wakeup_at = + jiffies + msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS); hid_hw_power(hdev, PM_HINT_NORMAL); mutex_unlock(&dev->lock); return ret; From patchwork Sun Oct 30 20:34:02 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620125 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 583C4FA3744 for ; Sun, 30 Oct 2022 20:35:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229947AbiJ3UfV (ORCPT ); Sun, 30 Oct 2022 16:35:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46460 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229909AbiJ3UfJ (ORCPT ); Sun, 30 Oct 2022 16:35:09 -0400 Received: from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com [IPv6:2a00:1450:4864:20::32d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D3DF7BC0C; Sun, 30 Oct 2022 13:34:43 -0700 (PDT) Received: by mail-wm1-x32d.google.com with SMTP id f16-20020a05600c491000b003cf66a2e7c0so2311329wmp.5; Sun, 30 Oct 2022 13:34:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ITcMCaKfMZfpT5FNI3zHkV64gyDtL5X1yd5OeMk6v78=; b=ZXKYgW2OmUN+mOeH+tEszk3tevmdmmSH8NV6DLpJ8vDP//0HHJC6J/EuFOsCjnlWmv kWknIQTJL5wfogjwX9ZxA2adFQQkVUfARYye1syIM+QUqfKuuixvoZE4oGZ2MH3Z37IT 5fCrl34VpUmzFHMzuklsoWg45dAxmh//WAmgPbFfsKzR4Vn3Vf8BjRp7dYU06p+sjsAj dP/7yzJJfYVi0pS+KznAD/OiG+G3nGrxXzoWL4TbvchSHPwH8kRCSsglyCe2opMsh2l/ FJadjV9UVYbzdwbS+Dv+w4G6+PrPmNzeWviB4GwLHsDyETI5zcDjxO2w6i5Jj6WILS9T dUFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ITcMCaKfMZfpT5FNI3zHkV64gyDtL5X1yd5OeMk6v78=; b=QUTihgeS5nyu8Mpi5UcrOtptnK4dpkYDdYvAXu7ReUhMyVIta3+u8wrQsdIEAoF6aq bzdXE1FCrlZs1qEEtpReQ+NAUI+P24IXBJzFehG1p4ZnAuKrnp8R5qSrnCsEpMknMBPJ nNR63f1Pb4kSPOIP+6txTV8gtSsCYZJaAte+DpYxQqFZqnQBCulHby1Yo5kQZKxfKJIC 9+hZJrJpXaFvYqCNtL8SZGWipHduIO9trxRaUD1AlwFkWYfya8Q1ofzHpDL99wi2rD9Q Ylgp8OtuZW9SDtHvl63bp5djN/asNM3nG/CJRzAsuXFDt4RvOpv0jdbKZNhot+OiYiKq 8PPg== X-Gm-Message-State: ACrzQf0fG5bJddaBDR6aTA6ovpQlmyT4w+0Kr5KmLvWLzC1KO69WgjLp UHoxE3AkVbqeiTsp+IBx748= X-Google-Smtp-Source: AMsMyM5cS1XXi9FhT0S9YX0QH41H8184CWU4WzeFAJo5pztK7u4CONZRIZbxMTaeIhc1ZvzukelhSQ== X-Received: by 2002:a05:600c:5486:b0:3b4:7e47:e19 with SMTP id iv6-20020a05600c548600b003b47e470e19mr5929744wmb.12.1667162082460; Sun, 30 Oct 2022 13:34:42 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:42 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman Subject: [PATCH v3 11/12] HID: ft260: fix a NULL pointer dereference in ft260_i2c_write Date: Sun, 30 Oct 2022 22:34:02 +0200 Message-Id: <20221030203403.4637-12-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org The len=0 passed into the ft260_i2c_write() triggered the NULL pointer dereference in the debug message on access to data[0]. Since a Write without data makes little sense in this context, do not allow it. Before: $ sudo i2ctransfer -y 13 w0@0x51 Killed After: $ sudo i2ctransfer -y 13 w0@0x51 Error: Sending messages failed: Invalid argument Reported-by: Enrik Berkhan Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index 00cbe7693ba0..b3f715f6ea86 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -395,6 +395,8 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, struct ft260_i2c_write_request_report *rep = (struct ft260_i2c_write_request_report *)dev->write_buf; + if (len < 1) + return -EINVAL; if (time_is_before_jiffies(dev->need_wakeup_at)) { (void)ft260_xfer_status(dev); From patchwork Sun Oct 30 20:34:03 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Zaidman X-Patchwork-Id: 620958 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A98ECC38A02 for ; Sun, 30 Oct 2022 20:35:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229941AbiJ3UfU (ORCPT ); Sun, 30 Oct 2022 16:35:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45792 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229781AbiJ3UfJ (ORCPT ); Sun, 30 Oct 2022 16:35:09 -0400 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5FFE7B7D1; Sun, 30 Oct 2022 13:34:44 -0700 (PDT) Received: by mail-wm1-x32a.google.com with SMTP id m29-20020a05600c3b1d00b003c6bf423c71so9723798wms.0; Sun, 30 Oct 2022 13:34:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=/70yde/ER3LkJXc+fZGhAb/0UY1KoaXhbuLb7z7ORbE=; b=RNgslzFLXcYc/7qkNDCxsyGq6dFUmnXok01RYIzUOiVlZFkGHXhuoYC9umrs8QLFDe tf9jXn1lXQD4tOIqOxBm1aNFzBDHKLf48HO0khZHJ1q3dI2WqW8QDXNEIsheehVuYbGM yiLiNnxtOYDg1HeM+mz0iq15zgCOpDsr1sQzuD09u0n+2vOWFabwZgYnT2UchWKGTXSY HQYR2eTHx9Z4UtxUKhcni2+4rSG3M937Wo3KER5/qNf+ZVImtopt8SLsMz0ciU4nh8mp 7Ja4+FkXEVOqrQBOh6kByn7eU4eCsgp8ZXGkRfOG0xmt00Zl5obPsXWTpCzVE/VeDGIA ZPWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/70yde/ER3LkJXc+fZGhAb/0UY1KoaXhbuLb7z7ORbE=; b=dnrWsmV0+7YTUfolftMf9IjkHFkEn1XO7iqWdypv2PNC8gBPbXKJ1t7u7CviarvxDN k/uZkll2RGqNR+A3dDYFHjJsGbd0YFrgnxzDzO+oAE1tfcKjzrC7tB0RE5/G8yYyy+bD GDQHWyxmYle1HxDEnlp/uoyPmhG7lkSxb2HGfsY+S++/6nq7rgIs9gaDRBKhuEPI7Zw3 ybRlCglM3T+QNVVOk5tUKxDlAaEMJkg3fhXJnyC3zCuo1McAK7oRIzfMj8ykobuslkBU svT/Lp8SH+WPOWjhq4+fAwo8GznyQVoYYoRxqgG/Bq+3iC/x4p0zFDDnkIZSeCB5Ho+e p5eg== X-Gm-Message-State: ACrzQf1/CuGbqY77SgIkeTiB41m8sBWgbxW8U3ccCkHyfYBitXV+07vH apNgytJ+qaLkDU8z6b45RCs= X-Google-Smtp-Source: AMsMyM5doZuOSQ1JQWji6UVhz7Su2MPqADODQ2pHsNUYyaap8MSi3d2lE9RG8kdSHQhOWICZaC+8bA== X-Received: by 2002:a1c:f008:0:b0:3b4:fd2e:3ede with SMTP id a8-20020a1cf008000000b003b4fd2e3edemr16281717wmb.133.1667162083882; Sun, 30 Oct 2022 13:34:43 -0700 (PDT) Received: from michael-VirtualBox.. (89-139-44-91.bb.netvision.net.il. [89.139.44.91]) by smtp.googlemail.com with ESMTPSA id n25-20020a05600c181900b003b95ed78275sm4939385wmp.20.2022.10.30.13.34.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 13:34:43 -0700 (PDT) From: Michael Zaidman To: jikos@kernel.org Cc: linux-kernel@vger.kernel.org, linux-input@vger.kernel.org, linux-i2c@vger.kernel.org, germain.hebert@ca.abb.com, Enrik.Berkhan@inka.de, Michael Zaidman Subject: [PATCH v3 12/12] HID: ft260: missed NACK from busy device Date: Sun, 30 Oct 2022 22:34:03 +0200 Message-Id: <20221030203403.4637-13-michael.zaidman@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030203403.4637-1-michael.zaidman@gmail.com> References: <20221030203403.4637-1-michael.zaidman@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org When writing into a slow device like an EEPROM chip, the controller may exit the busy state before the device releases the bus. In this case, the ft260_xfer_status returns success before the data transfer completion. The patch fixes it by returning from the ft260_xfer_status() with the "-EAGAIN" on both controller and bus busy status when appropriate. It does not apply to the i2c combined transactions when after the write IO, the controller keeps the bus busy until the read IO and then between reading IOs to ensure an atomic operation. Co-developed-by: Germain Hebert Signed-off-by: Germain Hebert Signed-off-by: Michael Zaidman --- drivers/hid/hid-ft260.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c index b3f715f6ea86..da8ea0d16059 100644 --- a/drivers/hid/hid-ft260.c +++ b/drivers/hid/hid-ft260.c @@ -303,7 +303,7 @@ static int ft260_i2c_reset(struct hid_device *hdev) return ret; } -static int ft260_xfer_status(struct ft260_device *dev) +static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy) { struct hid_device *hdev = dev->hdev; struct ft260_get_i2c_status_report report; @@ -320,7 +320,7 @@ static int ft260_xfer_status(struct ft260_device *dev) ft260_dbg("bus_status %#02x, clock %u\n", report.bus_status, dev->clock); - if (report.bus_status & FT260_I2C_STATUS_CTRL_BUSY) + if (report.bus_status & (FT260_I2C_STATUS_CTRL_BUSY | bus_busy)) return -EAGAIN; /* @@ -355,8 +355,11 @@ static int ft260_hid_output_report(struct hid_device *hdev, u8 *data, static int ft260_hid_output_report_check_status(struct ft260_device *dev, u8 *data, int len) { + u8 bus_busy; int ret, usec, try = 100; struct hid_device *hdev = dev->hdev; + struct ft260_i2c_write_request_report *rep = + (struct ft260_i2c_write_request_report *)data; ret = ft260_hid_output_report(hdev, data, len); if (ret < 0) { @@ -374,8 +377,18 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev, ft260_dbg("wait %d usec, len %d\n", usec, len); } + /* + * Do not check the busy bit for combined transactions + * since the controller keeps the bus busy between writing + * and reading IOs to ensure an atomic operation. + */ + if (rep->flag == FT260_FLAG_START) + bus_busy = 0; + else + bus_busy = FT260_I2C_STATUS_BUS_BUSY; + do { - ret = ft260_xfer_status(dev); + ret = ft260_xfer_status(dev, bus_busy); if (ret != -EAGAIN) break; } while (--try); @@ -399,7 +412,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data, return -EINVAL; if (time_is_before_jiffies(dev->need_wakeup_at)) { - (void)ft260_xfer_status(dev); + (void)ft260_xfer_status(dev, 0); ft260_dbg("device wakeup"); } @@ -453,7 +466,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd, return -EINVAL; if (time_is_before_jiffies(dev->need_wakeup_at)) { - (void)ft260_xfer_status(dev); + (void)ft260_xfer_status(dev, 0); ft260_dbg("device wakeup"); } @@ -484,6 +497,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, int timeout, ret = 0; struct ft260_i2c_read_request_report rep; struct hid_device *hdev = dev->hdev; + u8 bus_busy = 0; if ((flag & FT260_FLAG_START_REPEATED) == FT260_FLAG_START_REPEATED) flag = FT260_FLAG_START_REPEATED; @@ -527,7 +541,10 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data, dev->read_buf = NULL; - ret = ft260_xfer_status(dev); + if (flag & FT260_FLAG_STOP) + bus_busy = FT260_I2C_STATUS_BUS_BUSY; + + ret = ft260_xfer_status(dev, bus_busy); if (ret < 0) { ret = -EIO; ft260_i2c_reset(hdev); @@ -1003,7 +1020,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id) mutex_init(&dev->lock); init_completion(&dev->wait); - ret = ft260_xfer_status(dev); + ret = ft260_xfer_status(dev, FT260_I2C_STATUS_BUS_BUSY); if (ret) ft260_i2c_reset(hdev);