Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Michael Zaidman <michael.zaidman@gmail.com>
To:     jikos@kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
        linux-i2c@vger.kernel.org, Enrik.Berkhan@inka.de,
        Michael Zaidman <michael.zaidman@gmail.com>,
        Vince Asbridge <VAsbridge@sanblaze.com>,
        Stephen Shirron <SShirron@sanblaze.com>
Subject: [PATCH v4 08/13] HID: ft260: remove SMBus Quick command support
Date:   Sat,  5 Nov 2022 23:11:46 +0200
Message-Id: <20221105211151.7094-9-michael.zaidman@gmail.com>
In-Reply-To: <20221105211151.7094-1-michael.zaidman@gmail.com>
References: <20221105211151.7094-1-michael.zaidman@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

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 <VAsbridge@sanblaze.com>
Reported-by: Stephen Shirron <SShirron@sanblaze.com>
Reported-by: Enrik Berkhan <Enrik.Berkhan@inka.de>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
 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;
 }
-- 
2.34.1