* [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes
@ 2026-11-14 23:07 Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 01/16] HID: ft260: add serial driver Michael Zaidman
` (15 more replies)
0 siblings, 16 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
Mainline supports only the FT260's I2C bridge. This series adds the
UART/TTY interface and GPIO on top of it, and carries the I2C fixes
made alongside them [1].
Patch 1 is Christina Quast's v4 serial driver, ported to the current
I2C code. Patch 2 holds the fixes it needed on hardware and the fixes
for the review findings in the patch 1 code. As agreed with Jiri, the
series keeps the order in which the code was developed, and each patch
keeps its own author. So the UART work is split: patches 7 and 8 add
flow control and the modem pins after the GPIO and I2C patches.
The v1 and v2 threads:
https://lore.kernel.org/linux-input/20260822213941.98882-1-michael.zaidman@gmail.com/T/#t
https://lore.kernel.org/linux-input/20261030223637.86907-1-michael.zaidman@gmail.com/T/#t
Changes in v3:
All 21 issues Sashiko reported on v2 are fixed. The v3 note of each
patch lists its changes, with the same words as below. A patch with
no v3 note is unchanged from v2.
Patch 1:
Unchanged. From v3 on, review fixes for the serial driver go into
patch 2 (the v3 note of patch 1 says why).
Patch 2:
Fixes for the issues reported by Sashiko:
- Wakeup timer shut down and its work cancelled in
ft260_uart_port_remove(), so a tty call during removal cannot
re-arm it; the probe error path also cancels the work.
- -ENODEV for an unknown chip mode, so probe does not return 0 after
it frees dev.
- Port list sorted by index, so a re-added port does not get an index
that is in use.
- TX from a work item, because write() may not sleep; no shared
buffer, no endless loop on a FIFO reset; disabled before the FIFO is
freed.
- TTY registered last, so userspace cannot open it before the UART is
configured.
- UART reports passed to the tty code only on the UART interface; the
I2C interface has no tty port.
- TX report limited to 60 bytes, so 61 or 62 bytes do not give the
undefined report ID 0xFF.
- Unneeded ft260_uart_wakeup() prototype removed, changelog text
corrected.
- port_activate() sets the wakeup workaround from the baud rate and
arms the timer only when it is on, as ft260_uart_change_speed() does.
Patch 3:
Fixes for the issues reported by Sashiko:
- The gpiochip is registered without devm and removed in
ft260_remove(), so a GPIO write that races an unbind does not use
the freed dev.
- The uart_dcd_ri setting is kept across UART mode changes, so a mode
change does not give GPIO4 and GPIO5 back to the GPIO function while
they are DCD and RI.
Struct gpio_chip embedded in ft260_device, label from
devm_kasprintf(), the "initialize gpio chip" messages dropped and
"enabled GPIOs" moved to debug output (Bartosz Golaszewski).
Patch 5:
Subject reworded after the original out-of-tree commit, "Get rid of
sysfs store big numbers failure notice". The v2 subject read as if the
patch hid big values. No i2c: prefix, because FT260_ATTR_STORE serves
all store attributes, not only the I2C ones.
Patch 8:
Fixes for the issues reported by Sashiko:
- gpio_en checked under port->lock in tiocmget() and tiocmset(), so a
termios or sysfs change cannot switch the pins between the check and
the GPIO report.
- tiocmget() and tiocmset() return -ENODEV after ft260_remove(), so
they do not use hdev after hid_hw_stop().
- tiocmget() cannot run before dev->gc is set, because
ft260_uart_probe() registers the TTY last since patch 2.
Patch 9:
The err_register_tty path, which patch 3 adds to ft260_uart_probe(),
removes the UART attribute group of patch 9.
Patch 11:
Fixes for the issues reported by Sashiko:
- One reset per chip: struct ft260_chip, shared by the two interfaces,
is added here, so a reset queued by the second interface does not
tear down the chip that the first reset has recovered.
- Interfaces unbound before the USB reset, not after, so the reset
does not run while a failing probe stops usbhid and frees its URBs.
- -ENOMEM from usb_control_msg() returned as it is, not turned into
-ENODEV and a reset.
Patch 15:
Recovery info per adapter in struct ft260_device, not one static
copy, because i2c_init_recovery() writes into it for each adapter.
Fix for the issue reported by Sashiko:
- GPIO report lock in the ft260_chip, shared by the I2C and UART
interfaces of one chip, so a GPIO write from one interface is not
lost by a write from the other; tiocmset() reads the report.
Patch 16:
New patch (suggested by Bartosz Golaszewski). It is the last patch,
so the out-of-tree driver for kernels older than 6.5, which do not
have linux/cleanup.h, can leave it out.
Testing:
The kernel is built from hid/for-next 720806515cb6 with KASAN,
PROVE_LOCKING and DEBUG_ATOMIC_SLEEP, and booted with
kasan_multi_shot. The driver is built as a module from the last patch.
The pytest suite [2] of 206 tests passed, with no KASAN or lockdep
report. The v2 driver on the same kernel fails the new gpiochip unbind
test with the KASAN report that patch 3 fixes.
For the TN_189 workaround, a HW setup that resets the FT260 chip by
its RESETN pin ran 7,781 cycles. 101 of them hit the endpoint STALL,
and each of these recovered with one USB reset (details in patch 11).
Every patch builds with W=1 without warnings. checkpatch --strict
reports no errors. It warns once, in patch 15, for a quoted kernel log
line.
[1] https://github.com/MichaelZaidman/hid-ft260
[2] https://github.com/MichaelZaidman/hid-ft260/tree/main/tests
Christina Quast (1):
HID: ft260: add serial driver
Michael Zaidman (15):
HID: ft260: uart: fix the serial driver
HID: ft260: add GPIO support
HID: ft260: i2c: reduce driver module loading time
HID: ft260: fix sysfs store failure notice for big numbers
HID: ft260: i2c: reduce bus-error message severity
HID: ft260: uart: enable flow control
HID: ft260: uart: add modem pins control via ioctl
HID: ft260: group sysfs attrs per HID interface
HID: ft260: i2c: fix large write transaction failure
HID: ft260: workaround for TN_189 errata endpoint STALL after
enumeration
HID: ft260: i2c: abort in-flight transfers with STOP before reset
HID: ft260: i2c: reject an I2C clock outside the supported range
HID: ft260: i2c: fix i2c_reset sysfs store return value
HID: ft260: i2c: recover a bus held by a slave
HID: ft260: use lock guards
drivers/hid/Kconfig | 10 +-
drivers/hid/hid-ft260.c | 2579 +++++++++++++++++++++++++++++++++++----
2 files changed, 2319 insertions(+), 270 deletions(-)
base-commit: 720806515cb6921ab05caaa6f470f7054b56385e
--
2.43.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 01/16] HID: ft260: add serial driver
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:39 ` sashiko-bot
2026-11-14 23:07 ` [PATCH v3 02/16] HID: ft260: uart: fix the " Michael Zaidman
` (14 subsequent siblings)
15 siblings, 1 reply; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Daniel Beer, Michael Zaidman
From: Christina Quast <contact@christina-quast.de>
Add a TTY serial interface (/dev/ttyFTx) for FT260 USB HID devices,
providing baud rate configuration, data transfer, and termios settings
without requiring userspace HID report formatting.
The FT260 exposes I2C and UART through separate HID interfaces. Only
I2C was supported previously; UART required FTDI hidraw example code.
This is the v4 patch:
https://lore.kernel.org/all/20231218093153.192268-1-contact@christina-quast.de/
The follow-up v5 series stalled without being applied:
https://lore.kernel.org/linux-input/20240216-ft260_review5-v5-1-36db44673ac7@christina-quast.de/
Michael Zaidman ported it to the current driver:
- Adapt it to the I2C hardening merged upstream since v4: input-report
length validation, the read_buf/raw_event race fix with read_lock,
and SMBus block-read START/STOP handling.
- Make HID_FT260 depend on TTY, because the UART code is part of this
driver.
- Stop the report dispatch after an I2C input report is consumed.
Routing UART reports through ft260_raw_event() made v4 also report
every I2C input report as unhandled.
- Correct the UART probe/remove lifetime, the TTY install, index and
write return, the baud endianness, and the lock initialisation
before hid_hw_open().
Signed-off-by: Christina Quast <contact@christina-quast.de>
Co-developed-by: Daniel Beer <daniel.beer@igorinstitute.com>
Signed-off-by: Daniel Beer <daniel.beer@igorinstitute.com>
Co-developed-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fixes for the issues reported by Sashiko, folded into this patch:
- UART probe/remove lifetime.
- TTY install, index and write return.
- Baud endianness.
- Lock initialisation before hid_hw_open().
v3:
Unchanged. This patch is Christina Quast's v4, ported to the
current driver. From v3 on, fixes for review findings go into patch 2,
which holds Michael Zaidman's fixes to this driver, so that this patch
does not collect more of his changes under her authorship. The issues
Sashiko reported on v2 of this patch are fixed there; the v3 note of
patch 2 lists them.
---
drivers/hid/Kconfig | 10 +-
drivers/hid/hid-ft260.c | 853 +++++++++++++++++++++++++++++++++++++---
2 files changed, 802 insertions(+), 61 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 2481bc01c435..670bce481c42 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -386,12 +386,12 @@ config HID_EZKEY
Support for Ezkey BTC 8193 keyboard.
config HID_FT260
- tristate "FTDI FT260 USB HID to I2C host support"
- depends on USB_HID && HIDRAW && I2C
+ tristate "FTDI FT260 USB HID to I2C/UART host support"
+ depends on USB_HID && HIDRAW && I2C && TTY
help
- Provides I2C host adapter functionality over USB-HID through FT260
- device. The customizable USB descriptor fields are exposed as sysfs
- attributes.
+ Provides I2C host adapter and UART/TTY functionality over USB-HID
+ through the FT260 device. The customizable USB descriptor fields
+ are exposed as sysfs attributes.
To compile this driver as a module, choose M here: the module
will be called hid-ft260.
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 64c9c940db46..982e2ca7a2de 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -13,6 +13,16 @@
#include <linux/i2c.h>
#include <linux/module.h>
#include <linux/usb.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/kfifo.h>
+#include <linux/tty_flip.h>
+#include <linux/minmax.h>
+#include <linux/unaligned.h>
+
+#define UART_COUNT_MAX 4 /* Number of UARTs this driver can handle */
+#define FIFO_SIZE 256
+#define TTY_WAKEUP_WATERMARK (FIFO_SIZE / 2)
#ifdef DEBUG
static int ft260_debug = 1;
@@ -30,6 +40,7 @@ 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_UART_DATA_REPORT_ID(len) (FT260_UART_REPORT_MIN + (len - 1) / 4)
#define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */
@@ -43,7 +54,7 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
* 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)
@@ -81,7 +92,8 @@ enum {
FT260_UART_INTERRUPT_STATUS = 0xB1,
FT260_UART_STATUS = 0xE0,
FT260_UART_RI_DCD_STATUS = 0xE1,
- FT260_UART_REPORT = 0xF0,
+ FT260_UART_REPORT_MIN = 0xF0,
+ FT260_UART_REPORT_MAX = 0xFE,
};
/* Feature Out */
@@ -132,6 +144,13 @@ enum {
FT260_FLAG_START_STOP_REPEATED = 0x07,
};
+/* Return values for ft260_get_interface_type func */
+enum {
+ FT260_IFACE_NONE,
+ FT260_IFACE_I2C,
+ FT260_IFACE_UART
+};
+
#define FT260_SET_REQUEST_VALUE(report_id) ((FT260_FEATURE << 8) | report_id)
/* Feature In reports */
@@ -220,12 +239,71 @@ struct ft260_i2c_read_request_report {
__le16 length; /* data payload length */
} __packed;
-struct ft260_i2c_input_report {
- u8 report; /* FT260_I2C_REPORT */
+struct ft260_input_report {
+ u8 report; /* FT260_I2C_REPORT or FT260_UART_REPORT */
u8 length; /* data payload length */
u8 data[2]; /* data payload */
} __packed;
+/* UART reports */
+struct ft260_uart_write_request_report {
+ u8 report; /* FT260_UART_REPORT */
+ u8 length; /* data payload length */
+ u8 data[] __counted_by(length); /* variable data payload */
+} __packed;
+
+struct ft260_configure_uart_request {
+ u8 report; /* FT260_SYSTEM_SETTINGS */
+ u8 request; /* FT260_SET_UART_CONFIG */
+ u8 flow_ctrl; /* 0: OFF, 1: RTS_CTS, 2: DTR_DSR */
+ /* 3: XON_XOFF, 4: No flow ctrl */
+ /* The baudrate field is unaligned: */
+ __le32 baudrate; /* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
+ u8 data_bit; /* 7 or 8 */
+ u8 parity; /* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
+ u8 stop_bit; /* 0: one stop bit, 2: 2 stop bits */
+ u8 breaking; /* 0: no break */
+} __packed;
+
+/* UART flow control */
+enum {
+ FT260_UART_CFG_FLOW_CTRL_OFF = 0x00,
+ FT260_UART_CFG_FLOW_CTRL_RTS_CTS = 0x01,
+ FT260_UART_CFG_FLOW_CTRL_DTR_DSR = 0x02,
+ FT260_UART_CFG_FLOW_CTRL_XON_XOFF = 0x03,
+ FT260_UART_CFG_FLOW_CTRL_NONE = 0x04,
+};
+
+/* UART data bits */
+enum {
+ FT260_UART_CFG_DATA_BITS_7 = 0x07,
+ FT260_UART_CFG_DATA_BITS_8 = 0x08,
+};
+
+/* UART parity */
+enum {
+ FT260_UART_CFG_PAR_NO = 0x00,
+ FT260_UART_CFG_PAR_ODD = 0x01,
+ FT260_UART_CFG_PAR_EVEN = 0x02,
+ FT260_UART_CFG_PAR_HIGH = 0x03,
+ FT260_UART_CFG_PAR_LOW = 0x04,
+};
+
+/* UART stop bits */
+enum {
+ FT260_UART_CFG_STOP_ONE_BIT = 0x00,
+ FT260_UART_CFG_STOP_TWO_BIT = 0x02,
+};
+
+/* UART break */
+enum {
+ FT260_UART_CFG_BREAKING_NO = 0x00,
+ FT260_UART_CFG_BREAKING_YES = 0x01,
+};
+
+#define FT260_UART_CFG_BAUD_MIN 1200
+#define FT260_UART_CFG_BAUD_MAX 12000000
+
static const struct hid_device_id ft260_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
USB_DEVICE_ID_FT260) },
@@ -236,6 +314,22 @@ MODULE_DEVICE_TABLE(hid, ft260_devices);
struct ft260_device {
struct i2c_adapter adap;
struct hid_device *hdev;
+
+ bool ft260_is_serial;
+ struct list_head device_list;
+
+ /* tty_port lifetime is equal to device lifetime */
+ struct tty_port port;
+ unsigned int index;
+ struct kfifo xmit_fifo;
+ /* write_lock: lock to serialize access to xmit fifo */
+ spinlock_t write_lock;
+ struct uart_icount icount;
+
+ struct timer_list wakeup_timer;
+ struct work_struct wakeup_work;
+ bool reschedule_work;
+
struct completion wait;
struct mutex lock;
u8 write_buf[FT260_REPORT_MAX_LENGTH];
@@ -379,7 +473,7 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
ret = ft260_hid_output_report(hdev, data, len);
if (ret < 0) {
- hid_err(hdev, "%s: failed to start transfer, ret %d\n",
+ hid_dbg(hdev, "%s: failed to start transfer, ret %d\n",
__func__, ret);
ft260_i2c_reset(hdev);
return ret;
@@ -609,7 +703,7 @@ static int ft260_i2c_write_read(struct ft260_device *dev, struct i2c_msg *msgs)
else
read_off = *msgs[0].buf;
- pr_info("%s: off %#x rlen %d wlen %d\n", __func__,
+ ft260_dbg("%s: off %#x rlen %d wlen %d\n", __func__,
read_off, rd_len, wr_len);
}
@@ -826,7 +920,7 @@ static int ft260_get_system_config(struct hid_device *hdev,
return 0;
}
-static int ft260_is_interface_enabled(struct hid_device *hdev)
+static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device *dev)
{
struct ft260_get_system_status_report cfg;
struct usb_interface *usbif = to_usb_interface(hdev->dev.parent);
@@ -843,21 +937,27 @@ static int ft260_is_interface_enabled(struct hid_device *hdev)
ft260_dbg("i2c_enable: 0x%02x\n", cfg.i2c_enable);
ft260_dbg("uart_mode: 0x%02x\n", cfg.uart_mode);
+ dev->ft260_is_serial = false;
+
switch (cfg.chip_mode) {
case FT260_MODE_ALL:
case FT260_MODE_BOTH:
- if (interface == 1)
- hid_info(hdev, "uart interface is not supported\n");
- else
- ret = 1;
+ if (interface == 1) {
+ ret = FT260_IFACE_UART;
+ dev->ft260_is_serial = true;
+ } else {
+ ret = FT260_IFACE_I2C;
+ }
break;
case FT260_MODE_UART:
- hid_info(hdev, "uart interface is not supported\n");
+ ret = FT260_IFACE_UART;
+ dev->ft260_is_serial = true;
break;
case FT260_MODE_I2C:
- ret = 1;
+ ret = FT260_IFACE_I2C;
break;
}
+
return ret;
}
@@ -1001,51 +1101,478 @@ static const struct attribute_group ft260_attr_group = {
}
};
-static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
+/***
+ * START Serial dev part
+ */
+static DEFINE_MUTEX(ft260_uart_list_lock);
+static LIST_HEAD(ft260_uart_device_list);
+
+static struct ft260_device *ft260_dev_by_index(int index)
+{
+ struct ft260_device *port;
+
+ list_for_each_entry(port, &ft260_uart_device_list, device_list) {
+ if (index == port->index)
+ return port;
+ }
+ return NULL;
+}
+
+static int ft260_uart_add_port(struct ft260_device *port)
{
+ int index = 0, ret = 0;
struct ft260_device *dev;
- struct ft260_get_chip_version_report version;
+
+ spin_lock_init(&port->write_lock);
+ if (kfifo_alloc(&port->xmit_fifo, FIFO_SIZE, GFP_KERNEL))
+ return -ENOMEM;
+
+ mutex_lock(&ft260_uart_list_lock);
+ list_for_each_entry(dev, &ft260_uart_device_list, device_list) {
+ if (dev->index != index)
+ break;
+ index++;
+ }
+
+ port->index = index;
+ list_add_tail(&port->device_list, &ft260_uart_device_list);
+ mutex_unlock(&ft260_uart_list_lock);
+
+ return ret;
+}
+
+static void ft260_uart_port_put(struct ft260_device *port)
+{
+ tty_port_put(&port->port);
+}
+
+static void ft260_uart_port_remove(struct ft260_device *port)
+{
+ timer_delete_sync(&port->wakeup_timer);
+
+ mutex_lock(&ft260_uart_list_lock);
+ list_del(&port->device_list);
+ mutex_unlock(&ft260_uart_list_lock);
+
+ tty_port_tty_vhangup(&port->port);
+
+ spin_lock(&port->write_lock);
+ kfifo_free(&port->xmit_fifo);
+ spin_unlock(&port->write_lock);
+}
+
+static struct ft260_device *ft260_uart_port_get(unsigned int index)
+{
+ struct ft260_device *port;
+
+ if (index >= UART_COUNT_MAX)
+ return NULL;
+
+ mutex_lock(&ft260_uart_list_lock);
+ port = ft260_dev_by_index(index);
+ if (port)
+ tty_port_get(&port->port);
+ mutex_unlock(&ft260_uart_list_lock);
+
+ return port;
+}
+
+static int ft260_uart_open(struct tty_struct *tty, struct file *filp)
+{
int ret;
+ struct ft260_device *port = tty->driver_data;
- if (!hid_is_usb(hdev))
- return -EINVAL;
+ ret = tty_port_open(&port->port, tty, filp);
- dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
- if (!dev)
- return -ENOMEM;
+ return ret;
+}
- ret = hid_parse(hdev);
- if (ret) {
- hid_err(hdev, "failed to parse HID\n");
- return ret;
+static void ft260_uart_close(struct tty_struct *tty, struct file *filp)
+{
+ struct ft260_device *port = tty->driver_data;
+
+ tty_port_close(&port->port, tty, filp);
+}
+
+static void ft260_uart_hangup(struct tty_struct *tty)
+{
+ struct ft260_device *port = tty->driver_data;
+
+ tty_port_hangup(&port->port);
+}
+
+static int ft260_uart_transmit_chars(struct ft260_device *port)
+{
+ struct hid_device *hdev = port->hdev;
+ struct kfifo *xmit = &port->xmit_fifo;
+ struct tty_struct *tty;
+ struct ft260_uart_write_request_report *rep;
+ int len, data_len, ret = 0;
+
+ tty = tty_port_tty_get(&port->port);
+
+ data_len = kfifo_len(xmit);
+ if (!tty || !data_len) {
+ ret = -EINVAL;
+ goto tty_out;
}
- ret = hid_hw_start(hdev, 0);
+ rep = (struct ft260_uart_write_request_report *)port->write_buf;
+
+ do {
+ len = min(data_len, FT260_WR_DATA_MAX);
+
+ rep->report = FT260_UART_DATA_REPORT_ID(len);
+ rep->length = len;
+
+ len = kfifo_out_locked(xmit, rep->data, len, &port->write_lock);
+
+ ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
+ if (ret < 0) {
+ hid_err(hdev, "Failed to start transfer, ret %d\n", ret);
+ goto tty_out;
+ }
+
+ data_len -= len;
+ port->icount.tx += len;
+ } while (data_len > 0);
+
+ len = kfifo_len(xmit);
+ if ((FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
+ tty_wakeup(tty);
+
+ ret = 0;
+
+tty_out:
+ tty_kref_put(tty);
+ return ret;
+}
+
+static int ft260_uart_receive_chars(struct ft260_device *port,
+ u8 *data, u8 length)
+{
+ struct hid_device *hdev = port->hdev;
+ int ret = 0;
+
+ if (length > FT260_RD_DATA_MAX) {
+ hid_err(hdev, "Received too much data (%d)\n", length);
+ return -EBADR;
+ }
+
+ ret = tty_insert_flip_string(&port->port, data, length);
+ if (ret != length)
+ hid_err(hdev, "%d char not inserted to flip buffer\n", length - ret);
+ port->icount.rx += ret;
+
+ if (ret)
+ tty_flip_buffer_push(&port->port);
+
+ return ret;
+}
+
+static ssize_t ft260_uart_write(struct tty_struct *tty, const u8 *buf,
+ size_t count)
+{
+ struct ft260_device *port = tty->driver_data;
+ struct hid_device *hdev = port->hdev;
+ int len, ret;
+
+ len = kfifo_in_locked(&port->xmit_fifo, buf, count, &port->write_lock);
+ ft260_dbg("count: %zu, len: %d", count, len);
+
+ ret = ft260_uart_transmit_chars(port);
+ if (ret < 0)
+ hid_dbg(hdev, "Failed to transmit chars: %d\n", ret);
+
+ return len;
+}
+
+static unsigned int ft260_uart_write_room(struct tty_struct *tty)
+{
+ struct ft260_device *port = tty->driver_data;
+
+ return FIFO_SIZE - kfifo_len(&port->xmit_fifo);
+}
+
+static unsigned int ft260_uart_chars_in_buffer(struct tty_struct *tty)
+{
+ struct ft260_device *port = tty->driver_data;
+
+ return kfifo_len(&port->xmit_fifo);
+}
+
+static int ft260_uart_change_speed(struct ft260_device *port,
+ struct ktermios *termios,
+ struct ktermios *old)
+{
+ struct hid_device *hdev = port->hdev;
+ unsigned int baud;
+ struct ft260_configure_uart_request req;
+ int ret;
+
+ memset(&req, 0, sizeof(req));
+
+ req.report = FT260_SYSTEM_SETTINGS;
+ req.request = FT260_SET_UART_CONFIG;
+
+ switch (termios->c_cflag & CSIZE) {
+ case CS7:
+ req.data_bit = FT260_UART_CFG_DATA_BITS_7;
+ break;
+ case CS5:
+ case CS6:
+ hid_err(hdev, "Invalid data bit size, setting to default (8 bit)\n");
+ req.data_bit = FT260_UART_CFG_DATA_BITS_8;
+ termios->c_cflag &= ~CSIZE;
+ termios->c_cflag |= CS8;
+ break;
+ default:
+ case CS8:
+ req.data_bit = FT260_UART_CFG_DATA_BITS_8;
+ break;
+ }
+
+ req.stop_bit = (termios->c_cflag & CSTOPB) ?
+ FT260_UART_CFG_STOP_TWO_BIT : FT260_UART_CFG_STOP_ONE_BIT;
+
+ if (termios->c_cflag & PARENB) {
+ req.parity = (termios->c_cflag & PARODD) ?
+ FT260_UART_CFG_PAR_ODD : FT260_UART_CFG_PAR_EVEN;
+ } else {
+ req.parity = FT260_UART_CFG_PAR_NO;
+ }
+
+ baud = tty_termios_baud_rate(termios);
+ if (baud == 0 || baud < FT260_UART_CFG_BAUD_MIN || baud > FT260_UART_CFG_BAUD_MAX) {
+ struct tty_struct *tty = tty_port_tty_get(&port->port);
+
+ hid_err(hdev, "Invalid baud rate %d\n", baud);
+ baud = 9600;
+ if (tty) {
+ tty_encode_baud_rate(tty, baud, baud);
+ tty_kref_put(tty);
+ }
+ }
+ put_unaligned_le32(baud, &req.baudrate);
+
+ if (termios->c_cflag & CRTSCTS)
+ req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_RTS_CTS;
+ else
+ req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_OFF;
+
+ ft260_dbg("Configured termios: flow control: %d, baudrate: %d, ",
+ req.flow_ctrl, baud);
+ ft260_dbg("data_bit: %d, parity: %d, stop_bit: %d, breaking: %d\n",
+ req.data_bit, req.parity,
+ req.stop_bit, req.breaking);
+
+ req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_NONE;
+ req.breaking = FT260_UART_CFG_BREAKING_NO;
+
+ ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
+ if (ret < 0)
+ hid_err(hdev, "ft260_hid_feature_report_set failed: %d\n", ret);
+
+ return ret;
+}
+
+static int ft260_uart_get_icount(struct tty_struct *tty,
+ struct serial_icounter_struct *icount)
+{
+ struct ft260_device *port = tty->driver_data;
+
+ memcpy(icount, &port->icount, sizeof(struct uart_icount));
+
+ return 0;
+}
+
+static void ft260_uart_set_termios(struct tty_struct *tty,
+ const struct ktermios *old_termios)
+{
+ struct ft260_device *port = tty->driver_data;
+
+ ft260_uart_change_speed(port, &tty->termios, NULL);
+}
+
+static int ft260_uart_install(struct tty_driver *driver, struct tty_struct *tty)
+{
+ int idx = tty->index;
+ struct ft260_device *port = ft260_uart_port_get(idx);
+ int ret;
+
+ if (!port)
+ return -ENODEV;
+
+ ret = tty_standard_install(driver, tty);
if (ret) {
- hid_err(hdev, "failed to start HID HW\n");
+ ft260_uart_port_put(port);
return ret;
}
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "failed to open HID HW\n");
- goto err_hid_stop;
+ tty->driver_data = port;
+ return 0;
+}
+
+static void ft260_uart_cleanup(struct tty_struct *tty)
+{
+ struct ft260_device *port = tty->driver_data;
+
+ tty->driver_data = NULL; /* Bug trap */
+ ft260_uart_port_put(port);
+}
+
+static int ft260_uart_proc_show(struct seq_file *m, void *v)
+{
+ int i;
+
+ seq_printf(m, "ft260 info:1.0 driver%s%s revision:%s\n",
+ "", "", "");
+ for (i = 0; i < UART_COUNT_MAX; i++) {
+ struct ft260_device *port = ft260_uart_port_get(i);
+
+ if (port) {
+ seq_printf(m, "%d: uart:FT260", i);
+ if (capable(CAP_SYS_ADMIN)) {
+ seq_printf(m, " tx:%d rx:%d",
+ port->icount.tx, port->icount.rx);
+ if (port->icount.frame)
+ seq_printf(m, " fe:%d",
+ port->icount.frame);
+ if (port->icount.parity)
+ seq_printf(m, " pe:%d",
+ port->icount.parity);
+ if (port->icount.brk)
+ seq_printf(m, " brk:%d",
+ port->icount.brk);
+ if (port->icount.overrun)
+ seq_printf(m, " oe:%d",
+ port->icount.overrun);
+ if (port->icount.cts)
+ seq_printf(m, " cts:%d",
+ port->icount.cts);
+ if (port->icount.dsr)
+ seq_printf(m, " dsr:%d",
+ port->icount.dsr);
+ if (port->icount.rng)
+ seq_printf(m, " rng:%d",
+ port->icount.rng);
+ if (port->icount.dcd)
+ seq_printf(m, " dcd:%d",
+ port->icount.dcd);
+ }
+ ft260_uart_port_put(port);
+ seq_putc(m, '\n');
+ }
}
+ return 0;
+}
- ret = ft260_hid_feature_report_get(hdev, FT260_CHIP_VERSION,
- (u8 *)&version, sizeof(version));
- if (ret < 0) {
- hid_err(hdev, "failed to retrieve chip version\n");
- goto err_hid_close;
+static const struct tty_operations ft260_uart_ops = {
+ .open = ft260_uart_open,
+ .close = ft260_uart_close,
+ .write = ft260_uart_write,
+ .write_room = ft260_uart_write_room,
+ .chars_in_buffer = ft260_uart_chars_in_buffer,
+ .set_termios = ft260_uart_set_termios,
+ .hangup = ft260_uart_hangup,
+ .install = ft260_uart_install,
+ .cleanup = ft260_uart_cleanup,
+ .proc_show = ft260_uart_proc_show,
+ .get_icount = ft260_uart_get_icount,
+};
+
+/* The FT260 has a "power saving mode" that causes the device to switch
+ * to a 30 kHz oscillator if there's no activity for 5 seconds.
+ * Unfortunately this mode can only be disabled by reprogramming
+ * internal fuses, which requires an additional programming voltage.
+ *
+ * One effect of this mode is to cause data loss on a fast UART that
+ * transmits after being idle for longer than 5 seconds. We work around
+ * this by sending a dummy report at least once per 4 seconds if the
+ * UART is in use.
+ */
+static void ft260_uart_start_wakeup(struct timer_list *t)
+{
+ struct ft260_device *dev =
+ container_of(t, struct ft260_device, wakeup_timer);
+
+ if (dev->reschedule_work) {
+ schedule_work(&dev->wakeup_work);
+ mod_timer(&dev->wakeup_timer, jiffies +
+ msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
}
+}
- hid_info(hdev, "chip code: %02x%02x %02x%02x\n",
- version.chip_code[0], version.chip_code[1],
- version.chip_code[2], version.chip_code[3]);
+static void ft260_uart_do_wakeup(struct work_struct *work)
+{
+ struct ft260_device *dev =
+ container_of(work, struct ft260_device, wakeup_work);
+ struct ft260_get_chip_version_report version;
+ int ret;
- ret = ft260_is_interface_enabled(hdev);
- if (ret <= 0)
- goto err_hid_close;
+ if (dev->reschedule_work) {
+ ret = ft260_hid_feature_report_get(dev->hdev, FT260_CHIP_VERSION,
+ (u8 *)&version, sizeof(version));
+ if (ret < 0)
+ hid_err(dev->hdev,
+ "%s: failed to start transfer, ret %d\n",
+ __func__, ret);
+ }
+}
+
+static void ft260_uart_shutdown(struct tty_port *tport)
+{
+ struct ft260_device *port =
+ container_of(tport, struct ft260_device, port);
+
+ port->reschedule_work = false;
+}
+
+static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
+{
+ struct ft260_device *port =
+ container_of(tport, struct ft260_device, port);
+
+ /*
+ * Set the TTY IO error marker - we will only clear this
+ * once we have successfully opened the port.
+ */
+ set_bit(TTY_IO_ERROR, &tty->flags);
+
+ spin_lock(&port->write_lock);
+ kfifo_reset(&port->xmit_fifo);
+ spin_unlock(&port->write_lock);
+
+ ft260_uart_change_speed(port, &tty->termios, NULL);
+ clear_bit(TTY_IO_ERROR, &tty->flags);
+
+ if (port->reschedule_work) {
+ mod_timer(&port->wakeup_timer, jiffies +
+ msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
+ }
+
+ return 0;
+}
+
+static void ft260_uart_port_destroy(struct tty_port *tport)
+{
+ struct ft260_device *port =
+ container_of(tport, struct ft260_device, port);
+
+ kfree(port);
+}
+
+static const struct tty_port_operations ft260_uart_port_ops = {
+ .shutdown = ft260_uart_shutdown,
+ .activate = ft260_uart_activate,
+ .destruct = ft260_uart_port_destroy,
+};
+
+static struct tty_driver *ft260_tty_driver;
+
+static int ft260_i2c_probe(struct hid_device *hdev, struct ft260_device *dev)
+{
+ int ret;
hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n",
hdev->version >> 8, hdev->version & 0xff, hdev->name,
@@ -1061,10 +1588,6 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
snprintf(dev->adap.name, sizeof(dev->adap.name),
"FT260 usb-i2c bridge");
- mutex_init(&dev->lock);
- spin_lock_init(&dev->read_lock);
- init_completion(&dev->wait);
-
ret = ft260_xfer_status(dev, FT260_I2C_STATUS_BUS_BUSY);
if (ret)
ft260_i2c_reset(hdev);
@@ -1073,7 +1596,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = i2c_add_adapter(&dev->adap);
if (ret) {
hid_err(hdev, "failed to add i2c adapter\n");
- goto err_hid_close;
+ return ret;
}
ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
@@ -1081,15 +1604,157 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_err(hdev, "failed to create sysfs attrs\n");
goto err_i2c_free;
}
-
return 0;
err_i2c_free:
i2c_del_adapter(&dev->adap);
+ return ret;
+}
+
+static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
+{
+ struct ft260_configure_uart_request req;
+ int ret;
+ struct device *devt;
+
+ INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
+ // FIXME: Do I need that if I have cancel_work_sync?
+ // FIXME: are all kfifo access secured by lock? with irq or not?
+ dev->reschedule_work = false;
+ /* Work not started at this point */
+ timer_setup(&dev->wakeup_timer, ft260_uart_start_wakeup, 0);
+
+ tty_port_init(&dev->port);
+ dev->port.ops = &ft260_uart_port_ops;
+
+ ret = ft260_uart_add_port(dev);
+ if (ret) {
+ hid_err(hdev, "failed to add port\n");
+ goto err_port;
+ }
+ devt = tty_port_register_device_attr(&dev->port,
+ ft260_tty_driver,
+ dev->index, &hdev->dev,
+ dev, NULL);
+ if (IS_ERR(devt)) {
+ hid_err(hdev, "failed to register tty port\n");
+ ret = PTR_ERR(devt);
+ goto err_register_tty;
+ }
+ hid_info(hdev, "Registering device /dev/%s%d\n",
+ ft260_tty_driver->name, dev->index);
+
+ /* Send Feature Report to Configure FT260 as UART 9600-8-N-1 */
+ req.report = FT260_SYSTEM_SETTINGS;
+ req.request = FT260_SET_UART_CONFIG;
+ req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_NONE;
+ put_unaligned_le32(9600, &req.baudrate);
+ req.data_bit = FT260_UART_CFG_DATA_BITS_8;
+ req.parity = FT260_UART_CFG_PAR_NO;
+ req.stop_bit = FT260_UART_CFG_STOP_ONE_BIT;
+ req.breaking = FT260_UART_CFG_BREAKING_NO;
+
+ ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
+ if (ret < 0) {
+ hid_err(hdev, "ft260_hid_feature_report_set failed: %d\n",
+ ret);
+ goto err_hid_report;
+ }
+
+ return 0;
+
+err_hid_report:
+ tty_port_unregister_device(&dev->port, ft260_tty_driver, dev->index);
+err_register_tty:
+ ft260_uart_port_remove(dev);
+err_port:
+ timer_delete_sync(&dev->wakeup_timer);
+ return ret;
+}
+
+static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct ft260_device *dev;
+ struct ft260_get_chip_version_report version;
+ int ret;
+
+ if (!hid_is_usb(hdev))
+ return -EINVAL;
+
+ /* We cannot used devm_kzalloc here, because port has to survive until
+ * destroy function call
+ */
+ dev = kzalloc_obj(*dev, GFP_KERNEL);
+ if (!dev) {
+ ret = -ENOMEM;
+ goto alloc_fail;
+ }
+ hid_set_drvdata(hdev, dev);
+ mutex_init(&dev->lock);
+ spin_lock_init(&dev->read_lock);
+ init_completion(&dev->wait);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "failed to parse HID\n");
+ goto hid_fail;
+ }
+
+ ret = hid_hw_start(hdev, 0);
+ if (ret) {
+ hid_err(hdev, "failed to start HID HW\n");
+ goto hid_fail;
+ }
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "failed to open HID HW\n");
+ goto err_hid_stop;
+ }
+
+ ret = ft260_hid_feature_report_get(hdev, FT260_CHIP_VERSION,
+ (u8 *)&version, sizeof(version));
+ if (ret < 0) {
+ hid_err(hdev, "failed to retrieve chip version\n");
+ goto err_hid_close;
+ }
+
+ hid_info(hdev, "chip code: %02x%02x %02x%02x\n",
+ version.chip_code[0], version.chip_code[1],
+ version.chip_code[2], version.chip_code[3]);
+
+ ret = ft260_get_interface_type(hdev, dev);
+ if (ret <= FT260_IFACE_NONE)
+ goto err_hid_close;
+
+ hid_set_drvdata(hdev, dev);
+ dev->hdev = hdev;
+
+ if (!dev->ft260_is_serial) {
+ ret = ft260_i2c_probe(hdev, dev);
+ if (ret)
+ goto err_hid_close;
+ } else {
+ ret = ft260_uart_probe(hdev, dev);
+ if (ret)
+ goto err_uart;
+ }
+
+ return 0;
+
+err_uart:
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+ tty_port_put(&dev->port);
+ return ret;
+
err_hid_close:
hid_hw_close(hdev);
err_hid_stop:
hid_hw_stop(hdev);
+hid_fail:
+ kfree(dev);
+alloc_fail:
return ret;
}
@@ -1100,21 +1765,34 @@ static void ft260_remove(struct hid_device *hdev)
if (!dev)
return;
- sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
- i2c_del_adapter(&dev->adap);
+ if (dev->ft260_is_serial) {
+ timer_delete_sync(&dev->wakeup_timer);
+ cancel_work_sync(&dev->wakeup_work);
+ tty_port_unregister_device(&dev->port, ft260_tty_driver,
+ dev->index);
+ ft260_uart_port_remove(dev);
+ } else {
+ sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
+ i2c_del_adapter(&dev->adap);
+ }
hid_hw_close(hdev);
hid_hw_stop(hdev);
+
+ if (dev->ft260_is_serial)
+ ft260_uart_port_put(dev);
+ else
+ kfree(dev);
}
static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
u8 *data, int size)
{
struct ft260_device *dev = hid_get_drvdata(hdev);
- struct ft260_i2c_input_report *xfer = (void *)data;
+ struct ft260_input_report *xfer = (void *)data;
unsigned long irqflags;
- if (size < offsetof(struct ft260_i2c_input_report, data)) {
+ if (size < offsetof(struct ft260_input_report, data)) {
hid_err(hdev, "short report %d\n", size);
return -1;
}
@@ -1127,7 +1805,7 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
xfer->report, xfer->length, size);
if (xfer->length > size -
- offsetof(struct ft260_i2c_input_report, data)) {
+ offsetof(struct ft260_input_report, data)) {
hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
xfer->report, xfer->length);
return -1;
@@ -1157,9 +1835,17 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
if (complete_read)
complete(&dev->wait);
- } else {
- hid_err(hdev, "unhandled report %#02x\n", xfer->report);
+ return 0;
+
+ } else if (xfer->length > FT260_RD_DATA_MAX) {
+ hid_err(hdev, "received data too long (%d)\n", xfer->length);
+ return -EBADR;
+ } else if (xfer->report >= FT260_UART_REPORT_MIN &&
+ xfer->report <= FT260_UART_REPORT_MAX) {
+ return ft260_uart_receive_chars(dev, xfer->data, xfer->length);
}
+ hid_err(hdev, "unhandled report %#02x\n", xfer->report);
+
return 0;
}
@@ -1171,7 +1857,62 @@ static struct hid_driver ft260_driver = {
.raw_event = ft260_raw_event,
};
-module_hid_driver(ft260_driver);
-MODULE_DESCRIPTION("FTDI FT260 USB HID to I2C host bridge");
+static int __init ft260_driver_init(void)
+{
+ int ret;
+
+ ft260_tty_driver = tty_alloc_driver(UART_COUNT_MAX,
+ TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV);
+ if (IS_ERR(ft260_tty_driver)) {
+ pr_err("tty_alloc_driver failed: %d\n",
+ (int)PTR_ERR(ft260_tty_driver));
+ return PTR_ERR(ft260_tty_driver);
+ }
+
+ ft260_tty_driver->driver_name = "ft260_ser";
+ ft260_tty_driver->name = "ttyFT";
+ ft260_tty_driver->major = 0;
+ ft260_tty_driver->minor_start = 0;
+ ft260_tty_driver->type = TTY_DRIVER_TYPE_SERIAL;
+ ft260_tty_driver->subtype = SERIAL_TYPE_NORMAL;
+ ft260_tty_driver->init_termios = tty_std_termios;
+ ft260_tty_driver->init_termios.c_cflag = B9600 | CS8 | CREAD | HUPCL | CLOCAL;
+ ft260_tty_driver->init_termios.c_ispeed = 9600;
+ ft260_tty_driver->init_termios.c_ospeed = 9600;
+ tty_set_operations(ft260_tty_driver, &ft260_uart_ops);
+
+ ret = tty_register_driver(ft260_tty_driver);
+ if (ret) {
+ pr_err("tty_register_driver failed: %d\n", ret);
+ goto err_reg_driver;
+ }
+
+ ret = hid_register_driver(&(ft260_driver));
+ if (ret) {
+ pr_err("hid_register_driver failed: %d\n", ret);
+ goto err_reg_hid;
+ }
+
+ return 0;
+
+err_reg_hid:
+ tty_unregister_driver(ft260_tty_driver);
+err_reg_driver:
+ tty_driver_kref_put(ft260_tty_driver);
+
+ return ret;
+}
+
+static void __exit ft260_driver_exit(void)
+{
+ hid_unregister_driver(&(ft260_driver));
+ tty_unregister_driver(ft260_tty_driver);
+ tty_driver_kref_put(ft260_tty_driver);
+}
+
+module_init(ft260_driver_init);
+module_exit(ft260_driver_exit);
+
+MODULE_DESCRIPTION("FTDI FT260 USB HID to I2C host bridge and TTY driver");
MODULE_AUTHOR("Michael Zaidman <michael.zaidman@gmail.com>");
MODULE_LICENSE("GPL v2");
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 02/16] HID: ft260: uart: fix the serial driver
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 01/16] HID: ft260: add serial driver Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:37 ` sashiko-bot
2026-11-14 23:07 ` [PATCH v3 03/16] HID: ft260: add GPIO support Michael Zaidman
` (13 subsequent siblings)
15 siblings, 1 reply; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
Fix the serial driver added by the previous patch, a port of
Christina Quast's v4, so that it works reliably on hardware.
The UART and I2C paths shared write_buf without proper separation,
so concurrent use could corrupt transfers on both channels. Give
UART its own TX buffer. The write request report is now built in
that fixed per-port buffer, so its payload member becomes a fixed
FT260_WR_UART_DATA_MAX (60) array rather than a __counted_by()
flexible array, which describes a runtime-counted allocation this
path no longer makes.
The FT260 enters power-save after ~5s idle, which drops RX data at
higher baud rates. Keep a timed dummy wakeup, but only when needed:
skip it at 4800 baud or less and when EEPROM disables power-save;
avoid configuring baud or disabling the workaround twice. Arm the timer
when a baud change or the start of a tty session enables the
workaround.
Otherwise the chip can enter power-save before the first RX character
arrives, and that character is lost.
The received report length was validated against the actual HID report
size only on the I2C path. The UART path instead bounded the
device-supplied length against FT260_RD_DATA_MAX (180), which is the
maximum I2C read payload, not a bound on the report buffer. As a UART
payload cannot exceed FT260_WR_UART_DATA_MAX (60) bytes, a report
claiming more would make ft260_uart_receive_chars() push data past the
end of the report buffer into the tty flip buffer. Validate the length
against the received report size once, before dispatching on the report
type, and drop the two now-redundant FT260_RD_DATA_MAX checks.
Also fix RX data loss after reopening the device, improve TX
throughput, simplify HID interface-type detection, use kfifo_avail
for write-room, and ignore spurious UART interrupt status (0xb1)
reports instead of treating them as errors.
Also includes UART cleanup and refactoring to match the rest of the
driver style: replace the kfifo_*_locked aliases with
kfifo_*_spinlocked, and drop the parentheses around single-value
defines, which coding-style asks for only when the value is an
expression.
This replaces the UART bring-up fix series:
https://lore.kernel.org/all/20240210215147.77629-1-michael.zaidman@gmail.com/
Do not treat UART interrupt status (0xB1) as a payload length. When
re-enabling the power-save wakeup workaround after a baud change,
re-arm the timer.
Shut the wakeup timer down and cancel its work in
ft260_uart_port_remove(), before the tty hangup. A tty call during
removal can then not re-arm the timer, and the probe error path also
cancels the work. Return -ENODEV for an unknown chip mode, so that
probe does not return 0 after it frees dev. Keep the port list sorted
by index, so that a re-added port does not get an index that is in use.
Send UART data from a work item. The tty write() callback may not
sleep, and sending a USB report sleeps. The work item is the only user
of the TX buffer. It takes data out of the FIFO before it builds a
report and stops when the FIFO is empty, so a FIFO reset cannot make
it loop. When it is done, it wakes up the writers. In
ft260_uart_port_remove(), the work is disabled after the tty hangup,
which waits for a running write(), and before the FIFO is freed, so
neither of them uses the FIFO after it is freed.
Register the tty device last in ft260_uart_probe(), so that userspace
cannot open it before the UART is configured.
Pass UART reports to the tty code only on the UART interface. The I2C
interface has no initialized tty port.
A UART report carries at most 60 data bytes after a 2-byte header, and
the UART report IDs end at 0xFE (AN_394, sections 4.3 and 4.6.2). Limit
a TX report to 60 bytes, so that 61 or 62 bytes do not give the
undefined report ID 0xFF.
Report 0xE1 is defined as FT260_UART_RI_DCD_STATUS, but AN_394 section
4.3 lists 0xE2 for "UART RI and DCD Status". Correct the value. The
macro has no user.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fixes for the issues reported by Sashiko: 0xB1 length check,
wakeup timer re-arm.
v3:
Fixes for the issues reported by Sashiko:
- Wakeup timer shut down and its work cancelled in
ft260_uart_port_remove(), so a tty call during removal cannot
re-arm it; the probe error path also cancels the work.
- -ENODEV for an unknown chip mode, so probe does not return 0 after
it frees dev.
- Port list sorted by index, so a re-added port does not get an index
that is in use.
- TX from a work item, because write() may not sleep; no shared
buffer, no endless loop on a FIFO reset; disabled before the FIFO is
freed.
- TTY registered last, so userspace cannot open it before the UART is
configured.
- UART reports passed to the tty code only on the UART interface; the
I2C interface has no tty port.
- TX report limited to 60 bytes, so 61 or 62 bytes do not give the
undefined report ID 0xFF.
- Unneeded ft260_uart_wakeup() prototype removed, changelog text
corrected.
- port_activate() sets the wakeup workaround from the baud rate and
arms the timer only when it is on, as ft260_uart_change_speed() does.
---
drivers/hid/hid-ft260.c | 409 +++++++++++++++++++++++-----------------
1 file changed, 233 insertions(+), 176 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 982e2ca7a2de..8d72a22d7a07 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * hid-ft260.c - FTDI FT260 USB HID to I2C host bridge
+ * FTDI FT260 USB HID to I2C/UART host bridge
*
* Copyright (c) 2021, Michael Zaidman <michaelz@xsightlabs.com>
*
@@ -20,10 +20,6 @@
#include <linux/minmax.h>
#include <linux/unaligned.h>
-#define UART_COUNT_MAX 4 /* Number of UARTs this driver can handle */
-#define FIFO_SIZE 256
-#define TTY_WAKEUP_WATERMARK (FIFO_SIZE / 2)
-
#ifdef DEBUG
static int ft260_debug = 1;
#else
@@ -38,11 +34,14 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
pr_info("%s: " format, __func__, ##arg); \
} while (0)
-#define FT260_REPORT_MAX_LENGTH (64)
-#define FT260_I2C_DATA_REPORT_ID(len) (FT260_I2C_REPORT_MIN + (len - 1) / 4)
-#define FT260_UART_DATA_REPORT_ID(len) (FT260_UART_REPORT_MIN + (len - 1) / 4)
+#define FT260_REPORT_MAX_LEN 64
+#define FT260_DATA_REPORT_ID(min, len) (min + (len - 1) / 4)
+#define FT260_I2C_DATA_REPORT_ID(len) \
+ FT260_DATA_REPORT_ID(FT260_I2C_REPORT_MIN, len)
+#define FT260_UART_DATA_REPORT_ID(len) \
+ FT260_DATA_REPORT_ID(FT260_UART_REPORT_MIN, len)
-#define FT260_WAKEUP_NEEDED_AFTER_MS (4800) /* 5s minus 200ms margin */
+#define FT260_WAKEUP_NEEDED_AFTER_MS 4800 /* 5s minus 200ms margin */
/*
* The ft260 input report format defines 62 bytes for the data payload, but
@@ -55,8 +54,9 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
* 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)
+#define FT260_RD_DATA_MAX 180
+#define FT260_WR_I2C_DATA_MAX 60
+#define FT260_WR_UART_DATA_MAX 60
/*
* Device interface configuration.
@@ -90,8 +90,8 @@ enum {
FT260_I2C_REPORT_MAX = 0xDE,
FT260_GPIO = 0xB0,
FT260_UART_INTERRUPT_STATUS = 0xB1,
- FT260_UART_STATUS = 0xE0,
- FT260_UART_RI_DCD_STATUS = 0xE1,
+ FT260_UART_SETTINGS = 0xE0,
+ FT260_UART_RI_DCD_STATUS = 0xE2,
FT260_UART_REPORT_MIN = 0xF0,
FT260_UART_REPORT_MAX = 0xFE,
};
@@ -144,7 +144,7 @@ enum {
FT260_FLAG_START_STOP_REPEATED = 0x07,
};
-/* Return values for ft260_get_interface_type func */
+/* USB interface type values */
enum {
FT260_IFACE_NONE,
FT260_IFACE_I2C,
@@ -190,6 +190,18 @@ struct ft260_get_i2c_status_report {
u8 reserved;
} __packed;
+struct ft260_get_uart_settings_report {
+ u8 report; /* FT260_UART_SETTINGS */
+ u8 flow_ctrl; /* 0 - OFF; 1 - RTS_CTS, 2 - DTR_DSR, */
+ /* 3 - XON_XOFF, 4 - No flow control */
+ /* The baudrate field is unaligned */
+ __le32 baudrate; /* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
+ u8 data_bit; /* 7 or 8 */
+ u8 parity; /* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
+ u8 stop_bit; /* 0: one stop bit, 2: 2 stop bits */
+ u8 breaking; /* 0: no break */
+} __packed;
+
/* Feature Out reports */
struct ft260_set_system_clock_report {
@@ -229,7 +241,7 @@ struct ft260_i2c_write_request_report {
u8 address; /* 7-bit I2C address */
u8 flag; /* I2C transaction condition */
u8 length; /* data payload length */
- u8 data[FT260_WR_DATA_MAX]; /* data payload */
+ u8 data[FT260_WR_I2C_DATA_MAX]; /* data payload */
} __packed;
struct ft260_i2c_read_request_report {
@@ -246,18 +258,19 @@ struct ft260_input_report {
} __packed;
/* UART reports */
+
struct ft260_uart_write_request_report {
u8 report; /* FT260_UART_REPORT */
u8 length; /* data payload length */
- u8 data[] __counted_by(length); /* variable data payload */
+ u8 data[FT260_WR_UART_DATA_MAX]; /* data payload */
} __packed;
-struct ft260_configure_uart_request {
+struct ft260_configure_uart_request_report {
u8 report; /* FT260_SYSTEM_SETTINGS */
u8 request; /* FT260_SET_UART_CONFIG */
u8 flow_ctrl; /* 0: OFF, 1: RTS_CTS, 2: DTR_DSR */
/* 3: XON_XOFF, 4: No flow ctrl */
- /* The baudrate field is unaligned: */
+ /* The baudrate field is unaligned */
__le32 baudrate; /* little endian, 9600 = 0x2580, 19200 = 0x4B00 */
u8 data_bit; /* 7 or 8 */
u8 parity; /* 0: no parity, 1: odd, 2: even, 3: high, 4: low */
@@ -304,6 +317,11 @@ enum {
#define FT260_UART_CFG_BAUD_MIN 1200
#define FT260_UART_CFG_BAUD_MAX 12000000
+#define FT260_UART_EN_PW_SAVE_BAUD 4800
+
+#define UART_COUNT_MAX 4 /* Number of supported UARTs */
+#define XMIT_FIFO_SIZE PAGE_SIZE
+
static const struct hid_device_id ft260_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_FUTURE_TECHNOLOGY,
USB_DEVICE_ID_FT260) },
@@ -314,25 +332,24 @@ MODULE_DEVICE_TABLE(hid, ft260_devices);
struct ft260_device {
struct i2c_adapter adap;
struct hid_device *hdev;
-
- bool ft260_is_serial;
+ int iface_type;
struct list_head device_list;
-
- /* tty_port lifetime is equal to device lifetime */
struct tty_port port;
+ /* tty port index */
unsigned int index;
struct kfifo xmit_fifo;
- /* write_lock: lock to serialize access to xmit fifo */
- spinlock_t write_lock;
+ /* Protects xmit_fifo */
+ spinlock_t xmit_fifo_lock;
struct uart_icount icount;
-
struct timer_list wakeup_timer;
struct work_struct wakeup_work;
+ struct work_struct tx_work;
bool reschedule_work;
-
+ bool power_saving_en;
struct completion wait;
struct mutex lock;
- u8 write_buf[FT260_REPORT_MAX_LENGTH];
+ u8 i2c_wr_buf[FT260_REPORT_MAX_LEN];
+ u8 uart_wr_buf[FT260_REPORT_MAX_LEN];
unsigned long need_wakeup_at;
/* Protects read_buf, read_idx and read_len against ft260_raw_event() */
spinlock_t read_lock;
@@ -343,8 +360,7 @@ struct ft260_device {
};
static int ft260_hid_feature_report_get(struct hid_device *hdev,
- unsigned char report_id, u8 *data,
- size_t len)
+ u8 report_id, u8 *data, size_t len)
{
u8 *buf;
int ret;
@@ -473,8 +489,6 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
ret = ft260_hid_output_report(hdev, data, len);
if (ret < 0) {
- hid_dbg(hdev, "%s: failed to start transfer, ret %d\n",
- __func__, ret);
ft260_i2c_reset(hdev);
return ret;
}
@@ -516,7 +530,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
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;
+ (struct ft260_i2c_write_request_report *)dev->i2c_wr_buf;
if (len < 1)
return -EINVAL;
@@ -524,12 +538,12 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
rep->flag = FT260_FLAG_START;
do {
- if (len <= FT260_WR_DATA_MAX) {
+ if (len <= FT260_WR_I2C_DATA_MAX) {
wr_len = len;
if (flag == FT260_FLAG_START_STOP)
rep->flag |= FT260_FLAG_STOP;
} else {
- wr_len = FT260_WR_DATA_MAX;
+ wr_len = FT260_WR_I2C_DATA_MAX;
}
rep->report = FT260_I2C_DATA_REPORT_ID(wr_len);
@@ -565,7 +579,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
int len = 4;
struct ft260_i2c_write_request_report *rep =
- (struct ft260_i2c_write_request_report *)dev->write_buf;
+ (struct ft260_i2c_write_request_report *)dev->i2c_wr_buf;
if (data_len >= sizeof(rep->data))
return -EINVAL;
@@ -585,6 +599,8 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
rep->report, addr, cmd, rep->length, len);
ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len);
+ if (ret < 0)
+ hid_err(dev->hdev, "%s: failed with %d\n", __func__, ret);
return ret;
}
@@ -703,8 +719,7 @@ static int ft260_i2c_write_read(struct ft260_device *dev, struct i2c_msg *msgs)
else
read_off = *msgs[0].buf;
- ft260_dbg("%s: off %#x rlen %d wlen %d\n", __func__,
- read_off, rd_len, wr_len);
+ ft260_dbg("off %#x rlen %d wlen %d\n", read_off, rd_len, wr_len);
}
ret = ft260_i2c_write(dev, addr, msgs[0].buf, wr_len,
@@ -937,27 +952,28 @@ static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device
ft260_dbg("i2c_enable: 0x%02x\n", cfg.i2c_enable);
ft260_dbg("uart_mode: 0x%02x\n", cfg.uart_mode);
- dev->ft260_is_serial = false;
+ dev->power_saving_en = cfg.power_saving_en;
switch (cfg.chip_mode) {
case FT260_MODE_ALL:
case FT260_MODE_BOTH:
- if (interface == 1) {
+ if (interface == 1)
ret = FT260_IFACE_UART;
- dev->ft260_is_serial = true;
- } else {
+ else
ret = FT260_IFACE_I2C;
- }
break;
case FT260_MODE_UART:
ret = FT260_IFACE_UART;
- dev->ft260_is_serial = true;
break;
case FT260_MODE_I2C:
ret = FT260_IFACE_I2C;
break;
+ default:
+ hid_err(hdev, "unknown chip mode: %d\n", cfg.chip_mode);
+ return -ENODEV;
}
+ dev->iface_type = ret;
return ret;
}
@@ -1101,12 +1117,34 @@ static const struct attribute_group ft260_attr_group = {
}
};
-/***
- * START Serial dev part
- */
static DEFINE_MUTEX(ft260_uart_list_lock);
static LIST_HEAD(ft260_uart_device_list);
+static int ft260_get_uart_settings(struct hid_device *hdev,
+ struct ft260_get_uart_settings_report *cfg)
+{
+ int ret;
+ int len = sizeof(struct ft260_get_uart_settings_report);
+
+ ret = ft260_hid_feature_report_get(hdev, FT260_UART_SETTINGS,
+ (u8 *)cfg, len);
+ if (ret < 0) {
+ hid_err(hdev, "failed to retrieve uart settings\n");
+ return ret;
+ }
+ return 0;
+}
+
+static void ft260_uart_wakeup_workaraund_enable(struct ft260_device *port,
+ bool enable)
+{
+ if (port->power_saving_en) {
+ port->reschedule_work = enable;
+ ft260_dbg("%s wakeup workaround",
+ enable ? "activate" : "deactivate");
+ }
+}
+
static struct ft260_device *ft260_dev_by_index(int index)
{
struct ft260_device *port;
@@ -1123,8 +1161,8 @@ static int ft260_uart_add_port(struct ft260_device *port)
int index = 0, ret = 0;
struct ft260_device *dev;
- spin_lock_init(&port->write_lock);
- if (kfifo_alloc(&port->xmit_fifo, FIFO_SIZE, GFP_KERNEL))
+ spin_lock_init(&port->xmit_fifo_lock);
+ if (kfifo_alloc(&port->xmit_fifo, XMIT_FIFO_SIZE, GFP_KERNEL))
return -ENOMEM;
mutex_lock(&ft260_uart_list_lock);
@@ -1135,7 +1173,8 @@ static int ft260_uart_add_port(struct ft260_device *port)
}
port->index = index;
- list_add_tail(&port->device_list, &ft260_uart_device_list);
+ /* Before the first gap, or at the end: the list stays sorted */
+ list_add_tail(&port->device_list, &dev->device_list);
mutex_unlock(&ft260_uart_list_lock);
return ret;
@@ -1148,7 +1187,12 @@ static void ft260_uart_port_put(struct ft260_device *port)
static void ft260_uart_port_remove(struct ft260_device *port)
{
- timer_delete_sync(&port->wakeup_timer);
+ /*
+ * Shut down, not delete: a tty call before the hangup below
+ * must not re-arm the timer.
+ */
+ timer_shutdown_sync(&port->wakeup_timer);
+ cancel_work_sync(&port->wakeup_work);
mutex_lock(&ft260_uart_list_lock);
list_del(&port->device_list);
@@ -1156,12 +1200,15 @@ static void ft260_uart_port_remove(struct ft260_device *port)
tty_port_tty_vhangup(&port->port);
- spin_lock(&port->write_lock);
+ /* Disable, not cancel: a racing write must not queue it again */
+ disable_work_sync(&port->tx_work);
+
+ spin_lock(&port->xmit_fifo_lock);
kfifo_free(&port->xmit_fifo);
- spin_unlock(&port->write_lock);
+ spin_unlock(&port->xmit_fifo_lock);
}
-static struct ft260_device *ft260_uart_port_get(unsigned int index)
+static struct ft260_device *ft260_uart_port_get(int index)
{
struct ft260_device *port;
@@ -1207,61 +1254,51 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
struct kfifo *xmit = &port->xmit_fifo;
struct tty_struct *tty;
struct ft260_uart_write_request_report *rep;
- int len, data_len, ret = 0;
+ int len, ret = 0;
tty = tty_port_tty_get(&port->port);
+ if (!tty)
+ return -EINVAL;
- data_len = kfifo_len(xmit);
- if (!tty || !data_len) {
- ret = -EINVAL;
- goto tty_out;
- }
-
- rep = (struct ft260_uart_write_request_report *)port->write_buf;
-
- do {
- len = min(data_len, FT260_WR_DATA_MAX);
+ rep = (struct ft260_uart_write_request_report *)port->uart_wr_buf;
+ while ((len = kfifo_out_spinlocked(xmit, rep->data,
+ FT260_WR_UART_DATA_MAX,
+ &port->xmit_fifo_lock)) > 0) {
rep->report = FT260_UART_DATA_REPORT_ID(len);
rep->length = len;
- len = kfifo_out_locked(xmit, rep->data, len, &port->write_lock);
-
- ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
- if (ret < 0) {
- hid_err(hdev, "Failed to start transfer, ret %d\n", ret);
- goto tty_out;
- }
+ ret = ft260_hid_output_report(hdev, (u8 *)rep, len + 2);
+ if (ret < 0)
+ break;
- data_len -= len;
port->icount.tx += len;
- } while (data_len > 0);
-
- len = kfifo_len(xmit);
- if ((FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
- tty_wakeup(tty);
-
- ret = 0;
+ }
-tty_out:
+ tty_wakeup(tty);
tty_kref_put(tty);
- return ret;
+ return ret < 0 ? ret : 0;
}
-static int ft260_uart_receive_chars(struct ft260_device *port,
- u8 *data, u8 length)
+static void ft260_uart_tx_work(struct work_struct *work)
{
- struct hid_device *hdev = port->hdev;
- int ret = 0;
+ struct ft260_device *port =
+ container_of(work, struct ft260_device, tx_work);
+ int ret;
- if (length > FT260_RD_DATA_MAX) {
- hid_err(hdev, "Received too much data (%d)\n", length);
- return -EBADR;
- }
+ ret = ft260_uart_transmit_chars(port);
+ if (ret < 0)
+ ft260_dbg("failed to transmit %d\n", ret);
+}
+
+static int ft260_uart_receive_chars(struct ft260_device *port, u8 *data, u8 length)
+{
+ int ret;
ret = tty_insert_flip_string(&port->port, data, length);
if (ret != length)
- hid_err(hdev, "%d char not inserted to flip buffer\n", length - ret);
+ ft260_dbg("%d char not inserted to flip buf\n", length - ret);
+
port->icount.rx += ret;
if (ret)
@@ -1271,18 +1308,16 @@ static int ft260_uart_receive_chars(struct ft260_device *port,
}
static ssize_t ft260_uart_write(struct tty_struct *tty, const u8 *buf,
- size_t count)
+ size_t cnt)
{
struct ft260_device *port = tty->driver_data;
- struct hid_device *hdev = port->hdev;
- int len, ret;
+ ssize_t len;
- len = kfifo_in_locked(&port->xmit_fifo, buf, count, &port->write_lock);
- ft260_dbg("count: %zu, len: %d", count, len);
+ len = kfifo_in_spinlocked(&port->xmit_fifo, buf, cnt,
+ &port->xmit_fifo_lock);
+ ft260_dbg("count: %zu, len: %zd", cnt, len);
- ret = ft260_uart_transmit_chars(port);
- if (ret < 0)
- hid_dbg(hdev, "Failed to transmit chars: %d\n", ret);
+ schedule_work(&port->tx_work);
return len;
}
@@ -1291,7 +1326,7 @@ static unsigned int ft260_uart_write_room(struct tty_struct *tty)
{
struct ft260_device *port = tty->driver_data;
- return FIFO_SIZE - kfifo_len(&port->xmit_fifo);
+ return kfifo_avail(&port->xmit_fifo);
}
static unsigned int ft260_uart_chars_in_buffer(struct tty_struct *tty)
@@ -1307,7 +1342,8 @@ static int ft260_uart_change_speed(struct ft260_device *port,
{
struct hid_device *hdev = port->hdev;
unsigned int baud;
- struct ft260_configure_uart_request req;
+ struct ft260_configure_uart_request_report req;
+ bool wakeup_workaraund = false;
int ret;
memset(&req, 0, sizeof(req));
@@ -1321,7 +1357,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
break;
case CS5:
case CS6:
- hid_err(hdev, "Invalid data bit size, setting to default (8 bit)\n");
+ hid_err(hdev, "invalid data bit size, setting a default\n");
req.data_bit = FT260_UART_CFG_DATA_BITS_8;
termios->c_cflag &= ~CSIZE;
termios->c_cflag |= CS8;
@@ -1346,13 +1382,21 @@ static int ft260_uart_change_speed(struct ft260_device *port,
if (baud == 0 || baud < FT260_UART_CFG_BAUD_MIN || baud > FT260_UART_CFG_BAUD_MAX) {
struct tty_struct *tty = tty_port_tty_get(&port->port);
- hid_err(hdev, "Invalid baud rate %d\n", baud);
+ hid_err(hdev, "invalid baud rate %d\n", baud);
baud = 9600;
if (tty) {
tty_encode_baud_rate(tty, baud, baud);
tty_kref_put(tty);
}
}
+ if (baud > FT260_UART_EN_PW_SAVE_BAUD)
+ wakeup_workaraund = true;
+
+ ft260_uart_wakeup_workaraund_enable(port, wakeup_workaraund);
+ if (port->reschedule_work)
+ mod_timer(&port->wakeup_timer, jiffies +
+ msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
+
put_unaligned_le32(baud, &req.baudrate);
if (termios->c_cflag & CRTSCTS)
@@ -1360,7 +1404,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
else
req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_OFF;
- ft260_dbg("Configured termios: flow control: %d, baudrate: %d, ",
+ ft260_dbg("configured termios: flow control: %d, baudrate: %d, ",
req.flow_ctrl, baud);
ft260_dbg("data_bit: %d, parity: %d, stop_bit: %d, breaking: %d\n",
req.data_bit, req.parity,
@@ -1371,7 +1415,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
if (ret < 0)
- hid_err(hdev, "ft260_hid_feature_report_set failed: %d\n", ret);
+ hid_err(hdev, "failed to change termios: %d\n", ret);
return ret;
}
@@ -1425,8 +1469,8 @@ static int ft260_uart_proc_show(struct seq_file *m, void *v)
{
int i;
- seq_printf(m, "ft260 info:1.0 driver%s%s revision:%s\n",
- "", "", "");
+ seq_printf(m, "ft260 info:1.0 driver%s%s revision:%s\n", "", "", "");
+
for (i = 0; i < UART_COUNT_MAX; i++) {
struct ft260_device *port = ft260_uart_port_get(i);
@@ -1481,15 +1525,16 @@ static const struct tty_operations ft260_uart_ops = {
.get_icount = ft260_uart_get_icount,
};
-/* The FT260 has a "power saving mode" that causes the device to switch
+/*
+ * The FT260 has a "power saving mode" that causes the device to switch
* to a 30 kHz oscillator if there's no activity for 5 seconds.
- * Unfortunately this mode can only be disabled by reprogramming
+ * Unfortunately, this mode can only be disabled by reprogramming
* internal fuses, which requires an additional programming voltage.
*
- * One effect of this mode is to cause data loss on a fast UART that
- * transmits after being idle for longer than 5 seconds. We work around
- * this by sending a dummy report at least once per 4 seconds if the
- * UART is in use.
+ * One effect of this mode is to cause data loss on an Rx line at baud
+ * rates higher than 4800 after being idle for longer than 5 seconds.
+ * We work around this by sending a dummy report at least once per 4.8
+ * seconds if the UART is in use.
*/
static void ft260_uart_start_wakeup(struct timer_list *t)
{
@@ -1503,53 +1548,69 @@ static void ft260_uart_start_wakeup(struct timer_list *t)
}
}
-static void ft260_uart_do_wakeup(struct work_struct *work)
+static void ft260_uart_wakeup(struct ft260_device *dev)
{
- struct ft260_device *dev =
- container_of(work, struct ft260_device, wakeup_work);
- struct ft260_get_chip_version_report version;
+ struct ft260_get_chip_version_report ver;
int ret;
if (dev->reschedule_work) {
ret = ft260_hid_feature_report_get(dev->hdev, FT260_CHIP_VERSION,
- (u8 *)&version, sizeof(version));
+ (u8 *)&ver, sizeof(ver));
if (ret < 0)
- hid_err(dev->hdev,
- "%s: failed to start transfer, ret %d\n",
- __func__, ret);
+ hid_err(dev->hdev, "%s: failed with %d\n", __func__, ret);
}
}
-static void ft260_uart_shutdown(struct tty_port *tport)
+static void ft260_uart_do_wakeup(struct work_struct *work)
{
- struct ft260_device *port =
- container_of(tport, struct ft260_device, port);
+ struct ft260_device *dev =
+ container_of(work, struct ft260_device, wakeup_work);
- port->reschedule_work = false;
+ ft260_uart_wakeup(dev);
}
-static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
+static void ft260_uart_port_shutdown(struct tty_port *tport)
{
struct ft260_device *port =
container_of(tport, struct ft260_device, port);
- /*
- * Set the TTY IO error marker - we will only clear this
- * once we have successfully opened the port.
- */
+ ft260_uart_wakeup_workaraund_enable(port, false);
+}
+
+static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *tty)
+{
+ int ret;
+ int baudrate;
+ struct ft260_get_uart_settings_report cfg;
+ struct ft260_device *port = container_of(tport, struct ft260_device, port);
+
set_bit(TTY_IO_ERROR, &tty->flags);
- spin_lock(&port->write_lock);
+ spin_lock(&port->xmit_fifo_lock);
kfifo_reset(&port->xmit_fifo);
- spin_unlock(&port->write_lock);
+ spin_unlock(&port->xmit_fifo_lock);
- ft260_uart_change_speed(port, &tty->termios, NULL);
clear_bit(TTY_IO_ERROR, &tty->flags);
- if (port->reschedule_work) {
+ /*
+ * The port setting may remain intact after session termination.
+ * Then, when reopening the port without configuring the port
+ * setting, we need to retrieve the baud rate from the device to
+ * reactivate the wakeup workaround if needed.
+ */
+ ret = ft260_get_uart_settings(port->hdev, &cfg);
+ if (ret)
+ return ret;
+
+ baudrate = get_unaligned_le32(&cfg.baudrate);
+ ft260_uart_wakeup_workaraund_enable(port,
+ baudrate > FT260_UART_EN_PW_SAVE_BAUD);
+
+ ft260_dbg("configured baudrate = %d", baudrate);
+
+ if (port->reschedule_work)
mod_timer(&port->wakeup_timer, jiffies +
msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
- }
return 0;
}
@@ -1563,8 +1624,8 @@ static void ft260_uart_port_destroy(struct tty_port *tport)
}
static const struct tty_port_operations ft260_uart_port_ops = {
- .shutdown = ft260_uart_shutdown,
- .activate = ft260_uart_activate,
+ .shutdown = ft260_uart_port_shutdown,
+ .activate = ft260_uart_port_activate,
.destruct = ft260_uart_port_destroy,
};
@@ -1613,14 +1674,14 @@ static int ft260_i2c_probe(struct hid_device *hdev, struct ft260_device *dev)
static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
{
- struct ft260_configure_uart_request req;
+ struct ft260_configure_uart_request_report req;
int ret;
struct device *devt;
INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
- // FIXME: Do I need that if I have cancel_work_sync?
+ INIT_WORK(&dev->tx_work, ft260_uart_tx_work);
// FIXME: are all kfifo access secured by lock? with irq or not?
- dev->reschedule_work = false;
+ ft260_uart_wakeup_workaraund_enable(dev, true);
/* Work not started at this point */
timer_setup(&dev->wakeup_timer, ft260_uart_start_wakeup, 0);
@@ -1632,19 +1693,8 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
hid_err(hdev, "failed to add port\n");
goto err_port;
}
- devt = tty_port_register_device_attr(&dev->port,
- ft260_tty_driver,
- dev->index, &hdev->dev,
- dev, NULL);
- if (IS_ERR(devt)) {
- hid_err(hdev, "failed to register tty port\n");
- ret = PTR_ERR(devt);
- goto err_register_tty;
- }
- hid_info(hdev, "Registering device /dev/%s%d\n",
- ft260_tty_driver->name, dev->index);
- /* Send Feature Report to Configure FT260 as UART 9600-8-N-1 */
+ /* Configure UART to 9600n8 */
req.report = FT260_SYSTEM_SETTINGS;
req.request = FT260_SET_UART_CONFIG;
req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_NONE;
@@ -1656,16 +1706,25 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
if (ret < 0) {
- hid_err(hdev, "ft260_hid_feature_report_set failed: %d\n",
- ret);
+ hid_err(hdev, "failed to configure uart: %d\n", ret);
+ goto err_hid_report;
+ }
+
+ devt = tty_port_register_device_attr(&dev->port,
+ ft260_tty_driver,
+ dev->index, &hdev->dev,
+ dev, NULL);
+ if (IS_ERR(devt)) {
+ hid_err(hdev, "failed to register tty port\n");
+ ret = PTR_ERR(devt);
goto err_hid_report;
}
+ hid_info(hdev, "registering device /dev/%s%d\n",
+ ft260_tty_driver->name, dev->index);
return 0;
err_hid_report:
- tty_port_unregister_device(&dev->port, ft260_tty_driver, dev->index);
-err_register_tty:
ft260_uart_port_remove(dev);
err_port:
timer_delete_sync(&dev->wakeup_timer);
@@ -1680,9 +1739,9 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (!hid_is_usb(hdev))
return -EINVAL;
-
- /* We cannot used devm_kzalloc here, because port has to survive until
- * destroy function call
+ /*
+ * We cannot use devm_kzalloc here because the port has to survive
+ * until destroy function call.
*/
dev = kzalloc_obj(*dev, GFP_KERNEL);
if (!dev) {
@@ -1730,7 +1789,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_set_drvdata(hdev, dev);
dev->hdev = hdev;
- if (!dev->ft260_is_serial) {
+ if (dev->iface_type == FT260_IFACE_I2C) {
ret = ft260_i2c_probe(hdev, dev);
if (ret)
goto err_hid_close;
@@ -1765,9 +1824,7 @@ static void ft260_remove(struct hid_device *hdev)
if (!dev)
return;
- if (dev->ft260_is_serial) {
- timer_delete_sync(&dev->wakeup_timer);
- cancel_work_sync(&dev->wakeup_work);
+ if (dev->iface_type == FT260_IFACE_UART) {
tty_port_unregister_device(&dev->port, ft260_tty_driver,
dev->index);
ft260_uart_port_remove(dev);
@@ -1779,7 +1836,7 @@ static void ft260_remove(struct hid_device *hdev)
hid_hw_close(hdev);
hid_hw_stop(hdev);
- if (dev->ft260_is_serial)
+ if (dev->iface_type == FT260_IFACE_UART)
ft260_uart_port_put(dev);
else
kfree(dev);
@@ -1797,6 +1854,13 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
return -1;
}
+ if (xfer->report != FT260_UART_INTERRUPT_STATUS &&
+ xfer->length > size - offsetof(struct ft260_input_report, data)) {
+ hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
+ xfer->report, xfer->length);
+ return -1;
+ }
+
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
bool complete_read;
@@ -1804,13 +1868,6 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
xfer->report, xfer->length, size);
- if (xfer->length > size -
- offsetof(struct ft260_input_report, data)) {
- hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
- xfer->report, xfer->length);
- return -1;
- }
-
/*
* Hold read_lock so a timed-out ft260_i2c_read() cannot
* clear read_buf between the NULL check and the memcpy.
@@ -1837,12 +1894,12 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
return 0;
- } else if (xfer->length > FT260_RD_DATA_MAX) {
- hid_err(hdev, "received data too long (%d)\n", xfer->length);
- return -EBADR;
- } else if (xfer->report >= FT260_UART_REPORT_MIN &&
+ } else if (dev->iface_type == FT260_IFACE_UART &&
+ xfer->report >= FT260_UART_REPORT_MIN &&
xfer->report <= FT260_UART_REPORT_MAX) {
return ft260_uart_receive_chars(dev, xfer->data, xfer->length);
+ } else if (xfer->report == FT260_UART_INTERRUPT_STATUS) {
+ return 0;
}
hid_err(hdev, "unhandled report %#02x\n", xfer->report);
@@ -1887,7 +1944,7 @@ static int __init ft260_driver_init(void)
goto err_reg_driver;
}
- ret = hid_register_driver(&(ft260_driver));
+ ret = hid_register_driver(&ft260_driver);
if (ret) {
pr_err("hid_register_driver failed: %d\n", ret);
goto err_reg_hid;
@@ -1905,7 +1962,7 @@ static int __init ft260_driver_init(void)
static void __exit ft260_driver_exit(void)
{
- hid_unregister_driver(&(ft260_driver));
+ hid_unregister_driver(&ft260_driver);
tty_unregister_driver(ft260_tty_driver);
tty_driver_kref_put(ft260_tty_driver);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 03/16] HID: ft260: add GPIO support
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 01/16] HID: ft260: add serial driver Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 02/16] HID: ft260: uart: fix the " Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 04/16] HID: ft260: i2c: reduce driver module loading time Michael Zaidman
` (12 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
Add gpiochip support for the FT260 multifunctional pins, including
GPIO2/GPIOA/GPIOG function selection via sysfs, and enable the
available GPIOs according to the active UART mode.
This supersedes the earlier pre-UART GPIO series:
https://lore.kernel.org/lkml/20230211115752.26276-2-michael.zaidman@gmail.com/
The change of the gpio_chip.set callback to return int, as the current
gpiolib API requires, is based on a patch by Rio Liu <rio@r26.me>.
A pin set or direction change reads the FT260_GPIO report from the
chip, changes the bits of that one pin and writes the report back. The
other pins keep the state that the chip has, not a cached copy that
can be zero. A change to output writes the direction and the value in
the same report, and the error of a failed write goes back to gpiolib.
The driver keeps in gpio_en the pins that work as GPIOs. A uart_mode
change removes from gpio_en the pins that the new flow control mode
uses and adds the UART pins that it leaves free. Pins outside the UART
do not change, and an unknown mode value is ignored. While uart_dcd_ri
selects DCD and RI, GPIO4 and GPIO5 stay out of gpio_en, also after a
uart_mode change.
The gpio_chip is embedded in ft260_device. The enabled-GPIO mask is
printed only when the "debug" module parameter is set.
The gpiochip is not devm: ft260_remove() frees dev before the devm
actions run, so it removes the chip itself before it stops the HID
device.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fixes for the issues reported by Sashiko:
- The GPIO state is read before a write, so a set does not clobber
other pins with a zero shadow.
- Direction and value in one feature report.
- UART mode OFF clears only UART-owned bits; an out-of-range uart_mode
index is rejected.
v3:
Fixes for the issues reported by Sashiko:
- The gpiochip is registered without devm and removed in
ft260_remove(), so a GPIO write that races an unbind does not use
the freed dev.
- The uart_dcd_ri setting is kept across UART mode changes, so a mode
change does not give GPIO4 and GPIO5 back to the GPIO function while
they are DCD and RI.
A new test, test_gpio_unbind.py, races writes to gpioN/value against
unbind of the gpiochip owner. With the v2 code, on a 7.3.0 KASAN
kernel (hid/for-next 720806515cb6 plus this series,
CONFIG_KASAN_GENERIC and PROVE_LOCKING), it failed in the first cycle:
KASAN reported a slab-use-after-free write of size 4 in
ft260_gpio_set(), on memory that kfree() in ft260_uart_port_destroy()
had freed, called from ft260_remove(). With this patch, 20 cycles pass
with no report.
Struct gpio_chip embedded in ft260_device, label from
devm_kasprintf(), the "initialize gpio chip" messages dropped and
"enabled GPIOs" moved to debug output (Bartosz Golaszewski).
---
drivers/hid/Kconfig | 10 +-
drivers/hid/hid-ft260.c | 619 +++++++++++++++++++++++++++++++++++++---
2 files changed, 579 insertions(+), 50 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 670bce481c42..122a5b816d2c 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -386,12 +386,12 @@ config HID_EZKEY
Support for Ezkey BTC 8193 keyboard.
config HID_FT260
- tristate "FTDI FT260 USB HID to I2C/UART host support"
- depends on USB_HID && HIDRAW && I2C && TTY
+ tristate "FTDI FT260 USB HID to I2C/UART/GPIO host support"
+ depends on USB_HID && HIDRAW && I2C && TTY && GPIOLIB
help
- Provides I2C host adapter and UART/TTY functionality over USB-HID
- through the FT260 device. The customizable USB descriptor fields
- are exposed as sysfs attributes.
+ Provides I2C host adapter, UART/TTY, and GPIO functionality over
+ USB-HID through the FT260 device. The customizable USB descriptor
+ fields are exposed as sysfs attributes.
To compile this driver as a module, choose M here: the module
will be called hid-ft260.
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 8d72a22d7a07..a924ca2eef88 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -19,6 +19,8 @@
#include <linux/tty_flip.h>
#include <linux/minmax.h>
#include <linux/unaligned.h>
+#include <linux/bits.h>
+#include <linux/gpio/driver.h>
#ifdef DEBUG
static int ft260_debug = 1;
@@ -57,6 +59,11 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
#define FT260_RD_DATA_MAX 180
#define FT260_WR_I2C_DATA_MAX 60
#define FT260_WR_UART_DATA_MAX 60
+#define FT260_GPIOCHIP "ft260_gpio"
+#define FT260_GPIO_0_5_COUNT 6
+#define FT260_GPIO_A_H_COUNT 8
+#define FT260_GPIO_TOTAL (FT260_GPIO_0_5_COUNT + FT260_GPIO_A_H_COUNT)
+#define FT260_GPIO_MASK GENMASK(FT260_GPIO_TOTAL - 1, 0)
/*
* Device interface configuration.
@@ -151,7 +158,65 @@ enum {
FT260_IFACE_UART
};
-#define FT260_SET_REQUEST_VALUE(report_id) ((FT260_FEATURE << 8) | report_id)
+/* Multi-function pin functions */
+enum {
+ FT260_MFPIN_GPIO = 0x00,
+ FT260_MFPIN_SUSPOUT = 0x01,
+ FT260_MFPIN_PWREN = 0x02,
+ FT260_MFPIN_TX_ACTIVE = 0x03,
+ FT260_MFPIN_TX_LED = 0x04,
+ FT260_MFPIN_RX_LED = 0x05,
+ FT260_MFPIN_BCD_DET = 0x06,
+};
+
+enum {
+ FT260_GPIO_VALUE = 0x00,
+ FT260_GPIO_DIRECTION = 0x01,
+ FT260_GPIO_DIR_INPUT = 0x00,
+ FT260_GPIO_DIR_OUTPUT = 0x01,
+};
+
+/* GPIO offsets */
+#define FT260_GPIO_0 BIT(0)
+#define FT260_GPIO_1 BIT(1)
+#define FT260_GPIO_2 BIT(2)
+#define FT260_GPIO_3 BIT(3)
+#define FT260_GPIO_4 BIT(4)
+#define FT260_GPIO_5 BIT(5)
+#define FT260_GPIO_A BIT(FT260_GPIO_0_5_COUNT + 0)
+#define FT260_GPIO_B BIT(FT260_GPIO_0_5_COUNT + 1)
+#define FT260_GPIO_C BIT(FT260_GPIO_0_5_COUNT + 2)
+#define FT260_GPIO_D BIT(FT260_GPIO_0_5_COUNT + 3)
+#define FT260_GPIO_E BIT(FT260_GPIO_0_5_COUNT + 4)
+#define FT260_GPIO_F BIT(FT260_GPIO_0_5_COUNT + 5)
+#define FT260_GPIO_G BIT(FT260_GPIO_0_5_COUNT + 6)
+#define FT260_GPIO_H BIT(FT260_GPIO_0_5_COUNT + 7)
+
+/* GPIO groups */
+#define FT260_GPIO_WAKEUP FT260_GPIO_3
+#define FT260_GPIO_I2C_DEFAULT (FT260_GPIO_0 | FT260_GPIO_1)
+#define FT260_GPIO_UART_RX_TX (FT260_GPIO_C | FT260_GPIO_D)
+#define FT260_GPIO_UART_DCD_RI (FT260_GPIO_4 | FT260_GPIO_5)
+#define FT260_GPIO_UART_RTS_CTS (FT260_GPIO_B | FT260_GPIO_E)
+#define FT260_GPIO_UART_DTR_DSR (FT260_GPIO_F | FT260_GPIO_H)
+#define FT260_GPIO_UART_MODE_0_SET (FT260_GPIO_UART_RX_TX | \
+ FT260_GPIO_UART_DCD_RI | \
+ FT260_GPIO_UART_RTS_CTS | \
+ FT260_GPIO_UART_DTR_DSR)
+#define FT260_GPIO_UART_MODE_1_SET FT260_GPIO_UART_DTR_DSR
+#define FT260_GPIO_UART_MODE_2_SET FT260_GPIO_UART_RTS_CTS
+#define FT260_GPIO_UART_MODE_3_SET (FT260_GPIO_UART_RTS_CTS | \
+ FT260_GPIO_UART_DTR_DSR)
+#define FT260_GPIO_UART_MODE_4_SET FT260_GPIO_UART_MODE_3_SET
+#define FT260_GPIO_UART_DEFAULT FT260_GPIO_UART_MODE_0_SET
+#define FT260_GPIO_UART_MODE_1_CLR (FT260_GPIO_UART_RX_TX | \
+ FT260_GPIO_UART_RTS_CTS)
+#define FT260_GPIO_UART_MODE_2_CLR (FT260_GPIO_UART_RX_TX | \
+ FT260_GPIO_UART_DTR_DSR)
+#define FT260_GPIO_UART_MODE_3_CLR FT260_GPIO_UART_RX_TX
+#define FT260_GPIO_UART_MODES 5
+
+#define FT260_SET_REQUEST_VALUE(report_id) ((FT260_FEATURE << 8) | (report_id))
/* Feature In reports */
@@ -171,10 +236,10 @@ struct ft260_get_system_status_report {
u8 uart_mode; /* 0 - OFF; 1 - RTS_CTS, 2 - DTR_DSR, */
/* 3 - XON_XOFF, 4 - No flow control */
u8 hid_over_i2c_en; /* 0 - disabled, 1 - enabled */
- u8 gpio2_function; /* 0 - GPIO, 1 - SUSPOUT, */
+ u8 gpio2_func; /* 0 - GPIO, 1 - SUSPOUT, */
/* 2 - PWREN, 4 - TX_LED */
- u8 gpioA_function; /* 0 - GPIO, 3 - TX_ACTIVE, 4 - TX_LED */
- u8 gpioG_function; /* 0 - GPIO, 2 - PWREN, */
+ u8 gpioa_func; /* 0 - GPIO, 3 - TX_ACTIVE, 4 - TX_LED */
+ u8 gpiog_func; /* 0 - GPIO, 2 - PWREN, */
/* 5 - RX_LED, 6 - BCD_DET */
u8 suspend_out_pol; /* 0 - active-high, 1 - active-low */
u8 enable_wakeup_int; /* 0 - disabled, 1 - enabled */
@@ -202,6 +267,18 @@ struct ft260_get_uart_settings_report {
u8 breaking; /* 0: no break */
} __packed;
+struct ft260_gpio_state {
+ u8 vals; /* GPIO[0-5] values in bits 0 - 5 */
+ u8 dirs; /* GPIO[0-5] directions, 0 - in, 1 - out */
+ u8 ex_vals; /* GPIO[A-H] values in bits 0 - 7 */
+ u8 ex_dirs; /* GPIO[A-H] directions, 0 - in, 1 - out */
+} __packed;
+
+struct ft260_gpio_read_request_report {
+ u8 report; /* FT260_GPIO */
+ struct ft260_gpio_state gpio;
+} __packed;
+
/* Feature Out reports */
struct ft260_set_system_clock_report {
@@ -223,6 +300,12 @@ struct ft260_set_uart_mode_report {
/* 3 - XON_XOFF, 4 - No flow control */
} __packed;
+struct ft260_set_uart_dcd_ri_report {
+ u8 report; /* FT260_SYSTEM_SETTINGS */
+ u8 request; /* FT260_ENABLE_UART_DCD_RI */
+ u8 uart_dcd_ri; /* Pins func: 0 - GPIO4,GPIO5, 1 - DCD,RI */
+} __packed;
+
struct ft260_set_i2c_reset_report {
u8 report; /* FT260_SYSTEM_SETTINGS */
u8 request; /* FT260_SET_I2C_RESET */
@@ -234,6 +317,33 @@ struct ft260_set_i2c_speed_report {
__le16 clock; /* I2C bus clock in range 60-3400 KHz */
} __packed;
+struct ft260_set_gpio2_func_report {
+ u8 report; /* FT260_SYSTEM_SETTINGS */
+ u8 request; /* FT260_SELECT_GPIO2_FUNC */
+ u8 gpio2_func; /* Pin func: 0 - GPIO, 1 - SUSPOUT, */
+ /* 2 - PWREN# (active-low), 4 - TX_LED */
+} __packed;
+
+struct ft260_set_gpioa_func_report {
+ u8 report; /* FT260_SYSTEM_SETTINGS */
+ u8 request; /* FT260_SELECT_GPIOA_FUNC */
+ u8 gpioa_func; /* Pin func: 0 - GPIO, */
+ /* 3 - TX_ACTIVE, 4 - TX_LED */
+} __packed;
+
+struct ft260_set_gpiog_func_report {
+ u8 report; /* FT260_SYSTEM_SETTINGS */
+ u8 request; /* FT260_SELECT_GPIOG_FUNC */
+ u8 gpiog_func; /* Pin func: 0 - GPIO, */
+ /* 2 - PWREN# (active-low), */
+ /* 5 - RX_LED, 6 - BCD_DET */
+} __packed;
+
+struct ft260_gpio_write_request_report {
+ u8 report; /* FT260_GPIO */
+ struct ft260_gpio_state gpio;
+} __packed;
+
/* Data transfer reports */
struct ft260_i2c_write_request_report {
@@ -333,6 +443,7 @@ struct ft260_device {
struct i2c_adapter adap;
struct hid_device *hdev;
int iface_type;
+ int iface_id;
struct list_head device_list;
struct tty_port port;
/* tty port index */
@@ -357,6 +468,11 @@ struct ft260_device {
u16 read_idx;
u16 read_len;
u16 clock;
+ u16 gpio_en;
+ struct gpio_chip gc;
+ struct ft260_gpio_state gpio;
+ u16 gpio_uart_mode[FT260_GPIO_UART_MODES];
+ bool uart_dcd_ri;
};
static int ft260_hid_feature_report_get(struct hid_device *hdev,
@@ -389,8 +505,6 @@ static int ft260_hid_feature_report_set(struct hid_device *hdev, u8 *data,
if (!buf)
return -ENOMEM;
- buf[0] = FT260_SYSTEM_SETTINGS;
-
ret = hid_hw_raw_request(hdev, buf[0], buf, len, HID_FEATURE_REPORT,
HID_REQ_SET_REPORT);
@@ -403,6 +517,7 @@ static int ft260_i2c_reset(struct hid_device *hdev)
struct ft260_set_i2c_reset_report report;
int ret;
+ report.report = FT260_SYSTEM_SETTINGS;
report.request = FT260_SET_I2C_RESET;
ret = ft260_hid_feature_report_set(hdev, (u8 *)&report, sizeof(report));
@@ -920,6 +1035,341 @@ static const struct i2c_algorithm ft260_i2c_algo = {
.functionality = ft260_functionality,
};
+static void ft260_gpio_en_set(struct ft260_device *dev, u16 bitmap)
+{
+ dev->gpio_en |= bitmap & FT260_GPIO_MASK;
+}
+
+static void ft260_gpio_en_clr(struct ft260_device *dev, u16 bitmap)
+{
+ dev->gpio_en &= ~bitmap & FT260_GPIO_MASK;
+}
+
+static void ft260_gpio_en_update(struct hid_device *hdev, u8 req, u8 value)
+{
+ u16 bitmap;
+ struct ft260_device *dev = hid_get_drvdata(hdev);
+
+ switch (req) {
+ case FT260_SET_I2C_MODE:
+ bitmap = FT260_GPIO_I2C_DEFAULT;
+ break;
+ case FT260_SET_UART_MODE:
+ switch (value) {
+ case FT260_UART_CFG_FLOW_CTRL_OFF:
+ bitmap = FT260_GPIO_UART_MODE_0_SET;
+ break;
+ case FT260_UART_CFG_FLOW_CTRL_RTS_CTS:
+ bitmap = FT260_GPIO_UART_MODE_1_CLR;
+ break;
+ case FT260_UART_CFG_FLOW_CTRL_DTR_DSR:
+ bitmap = FT260_GPIO_UART_MODE_2_CLR;
+ break;
+ case FT260_UART_CFG_FLOW_CTRL_XON_XOFF:
+ case FT260_UART_CFG_FLOW_CTRL_NONE:
+ bitmap = FT260_GPIO_UART_MODE_3_CLR;
+ break;
+ default:
+ return;
+ }
+ ft260_gpio_en_clr(dev, bitmap);
+ bitmap = dev->gpio_uart_mode[value];
+ if (dev->uart_dcd_ri)
+ bitmap &= ~FT260_GPIO_UART_DCD_RI;
+ ft260_gpio_en_set(dev, bitmap);
+ goto exit;
+
+ case FT260_ENABLE_UART_DCD_RI:
+ dev->uart_dcd_ri = value != FT260_MFPIN_GPIO;
+ bitmap = FT260_GPIO_UART_DCD_RI;
+ break;
+
+ case FT260_SELECT_GPIO2_FUNC:
+ bitmap = FT260_GPIO_2;
+ break;
+ case FT260_SELECT_GPIOA_FUNC:
+ bitmap = FT260_GPIO_A;
+ break;
+ case FT260_SELECT_GPIOG_FUNC:
+ bitmap = FT260_GPIO_G;
+ break;
+ default:
+ return;
+ }
+
+ if (value == FT260_MFPIN_GPIO)
+ ft260_gpio_en_set(dev, bitmap);
+ else
+ ft260_gpio_en_clr(dev, bitmap);
+exit:
+ ft260_dbg("%s: enabled GPIOs: %04x\n", dev_name(&hdev->dev),
+ dev->gpio_en);
+}
+
+static int ft260_gpio_set(struct gpio_chip *gc, u32 offset, int value)
+{
+ int ret = 0;
+ struct ft260_gpio_read_request_report buf;
+ struct ft260_gpio_write_request_report rep;
+ struct ft260_device *dev = gpiochip_get_data(gc);
+ struct hid_device *hdev = dev->hdev;
+
+ if (offset >= FT260_GPIO_TOTAL) {
+ hid_err(hdev, "%s: invalid offset %d\n", __func__, offset);
+ return -EINVAL;
+ }
+
+ ft260_dbg("offset %d val %d\n", offset, value);
+
+ mutex_lock(&dev->lock);
+
+ if (!(dev->gpio_en & BIT(offset))) {
+ hid_err(hdev, "%s: wrong pin function %d\n", __func__, offset);
+ ret = -EINVAL;
+ goto exit;
+ }
+
+ ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf,
+ sizeof(buf));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
+ goto exit;
+ }
+
+ rep.report = FT260_GPIO;
+ rep.gpio = buf.gpio;
+
+ if (offset < FT260_GPIO_0_5_COUNT) {
+ if (value)
+ rep.gpio.vals |= !!value << offset;
+ else
+ rep.gpio.vals &= ~BIT(offset);
+ } else {
+ offset = offset - FT260_GPIO_0_5_COUNT;
+ if (value)
+ rep.gpio.ex_vals |= !!value << offset;
+ else
+ rep.gpio.ex_vals &= ~BIT(offset);
+ }
+
+ ft260_dbg("dirs %#02x vals %#02x ex_dir %#02x ex_vals %#02x\n",
+ rep.gpio.dirs, rep.gpio.vals,
+ rep.gpio.ex_dirs, rep.gpio.ex_vals);
+
+ ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, sizeof(rep));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
+ goto exit;
+ }
+
+ dev->gpio = rep.gpio;
+ ret = 0;
+exit:
+ mutex_unlock(&dev->lock);
+ return ret;
+}
+
+static int ft260_gpio_direction_set(struct gpio_chip *gc, u32 offset,
+ int value, int direction)
+{
+ int ret;
+ struct ft260_gpio_read_request_report buf;
+ struct ft260_gpio_write_request_report *rep;
+ struct ft260_device *dev = gpiochip_get_data(gc);
+ struct hid_device *hdev = dev->hdev;
+
+ if (offset >= FT260_GPIO_TOTAL) {
+ hid_err(hdev, "%s: invalid offset %d\n", __func__, offset);
+ return -EINVAL;
+ }
+
+ ft260_dbg("offset %d val %d direction %d\n", offset, value, direction);
+
+ mutex_lock(&dev->lock);
+
+ if (!(dev->gpio_en & BIT(offset))) {
+ hid_err(hdev, "%s: wrong pin function %d\n", __func__, offset);
+ ret = -EIO;
+ goto exit;
+ }
+
+ ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf, sizeof(buf));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
+ goto exit;
+ }
+
+ rep = (struct ft260_gpio_write_request_report *)&buf;
+
+ if (direction == FT260_GPIO_DIR_OUTPUT) {
+ if (offset < FT260_GPIO_0_5_COUNT) {
+ rep->gpio.dirs |= BIT(offset);
+ if (value)
+ rep->gpio.vals |= BIT(offset);
+ else
+ rep->gpio.vals &= ~BIT(offset);
+ } else {
+ u32 ex = offset - FT260_GPIO_0_5_COUNT;
+
+ rep->gpio.ex_dirs |= BIT(ex);
+ if (value)
+ rep->gpio.ex_vals |= BIT(ex);
+ else
+ rep->gpio.ex_vals &= ~BIT(ex);
+ }
+ } else if (offset < FT260_GPIO_0_5_COUNT) {
+ rep->gpio.dirs &= ~BIT(offset);
+ } else {
+ rep->gpio.ex_dirs &= ~BIT(offset - FT260_GPIO_0_5_COUNT);
+ }
+
+ ft260_dbg("dirs %#02x val %#02x ex_dirs %#02x ex_vals %#02x\n",
+ rep->gpio.dirs, rep->gpio.vals,
+ rep->gpio.ex_dirs, rep->gpio.ex_vals);
+
+ ret = ft260_hid_feature_report_set(hdev, (u8 *)rep, sizeof(*rep));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
+ goto exit;
+ }
+
+ dev->gpio = rep->gpio;
+ ret = 0;
+exit:
+ mutex_unlock(&dev->lock);
+ return ret;
+}
+
+static int ft260_gpio_direction_output(struct gpio_chip *gc,
+ u32 offset, int value)
+{
+ return ft260_gpio_direction_set(gc, offset, value,
+ FT260_GPIO_DIR_OUTPUT);
+}
+
+static int ft260_gpio_direction_input(struct gpio_chip *gc, u32 offset)
+{
+ return ft260_gpio_direction_set(gc, offset, 0,
+ FT260_GPIO_DIR_INPUT);
+}
+
+static int ft260_gpio_get_all(struct gpio_chip *gc, int item)
+{
+ int ret;
+ struct ft260_gpio_read_request_report rep;
+ struct ft260_device *dev = gpiochip_get_data(gc);
+ struct hid_device *hdev = dev->hdev;
+
+ ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&rep, sizeof(rep));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
+ goto exit;
+ }
+
+ if (item == FT260_GPIO_VALUE)
+ ret = (rep.gpio.ex_vals << FT260_GPIO_0_5_COUNT) | rep.gpio.vals;
+ else
+ ret = (rep.gpio.ex_dirs << FT260_GPIO_0_5_COUNT) | rep.gpio.dirs;
+exit:
+ return ret;
+}
+
+static int ft260_gpio_get_direction(struct gpio_chip *gc, u32 offset)
+{
+ int ret = ft260_gpio_get_all(gc, FT260_GPIO_DIRECTION);
+
+ if (ret < 0)
+ return ret;
+ return !((ret >> offset) & 1);
+}
+
+static int ft260_gpio_get(struct gpio_chip *gc, u32 offset)
+{
+ int ret = ft260_gpio_get_all(gc, FT260_GPIO_VALUE);
+
+ if (ret < 0)
+ return ret;
+ return (ret >> offset) & 1;
+}
+
+static int ft260_gpio_init(struct ft260_device *dev,
+ struct ft260_get_system_status_report *cfg)
+{
+ int ret;
+ struct ft260_get_chip_version_report ver;
+ struct ft260_gpio_read_request_report gbuf;
+ struct hid_device *hdev = dev->hdev;
+ u8 mode = cfg->chip_mode;
+
+ dev->gpio_uart_mode[0] = (u16)FT260_GPIO_UART_MODE_0_SET;
+ dev->gpio_uart_mode[1] = (u16)FT260_GPIO_UART_MODE_1_SET;
+ dev->gpio_uart_mode[2] = (u16)FT260_GPIO_UART_MODE_2_SET;
+ dev->gpio_uart_mode[3] = (u16)FT260_GPIO_UART_MODE_3_SET;
+ dev->gpio_uart_mode[4] = (u16)FT260_GPIO_UART_MODE_4_SET;
+
+ if (mode == FT260_MODE_ALL)
+ mode = FT260_MODE_BOTH;
+
+ if (mode & FT260_MODE_UART) {
+ if (cfg->uart_mode < FT260_GPIO_UART_MODES)
+ dev->gpio_en |= dev->gpio_uart_mode[cfg->uart_mode];
+ else
+ dev->gpio_en |= FT260_GPIO_UART_DEFAULT;
+ } else {
+ dev->gpio_en |= FT260_GPIO_UART_DEFAULT;
+ }
+
+ if (!(mode & FT260_MODE_I2C))
+ dev->gpio_en |= FT260_GPIO_I2C_DEFAULT;
+
+ if (cfg->gpio2_func == FT260_MFPIN_GPIO)
+ dev->gpio_en |= FT260_GPIO_2;
+ if (cfg->enable_wakeup_int == FT260_MFPIN_GPIO)
+ dev->gpio_en |= FT260_GPIO_3;
+ if (cfg->gpioa_func == FT260_MFPIN_GPIO)
+ dev->gpio_en |= FT260_GPIO_A;
+ if (cfg->gpiog_func == FT260_MFPIN_GPIO)
+ dev->gpio_en |= FT260_GPIO_G;
+
+ ft260_dbg("%s: enabled GPIOs: %04x\n", dev_name(&hdev->dev),
+ dev->gpio_en);
+
+ dev->gc.label = devm_kasprintf(&hdev->dev, GFP_KERNEL, "ft260_%s",
+ dev_name(&hdev->dev));
+ if (!dev->gc.label)
+ return -ENOMEM;
+
+ dev->gc.direction_input = ft260_gpio_direction_input;
+ dev->gc.direction_output = ft260_gpio_direction_output;
+ dev->gc.get_direction = ft260_gpio_get_direction;
+ dev->gc.set = ft260_gpio_set;
+ dev->gc.get = ft260_gpio_get;
+ dev->gc.base = -1;
+ dev->gc.ngpio = FT260_GPIO_TOTAL;
+ dev->gc.can_sleep = true;
+ dev->gc.parent = &hdev->dev;
+
+ /* Wakeup chip */
+ (void)ft260_hid_feature_report_get(dev->hdev, FT260_CHIP_VERSION,
+ (u8 *)&ver, sizeof(ver));
+
+ ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&gbuf,
+ sizeof(gbuf));
+ if (ret < 0)
+ hid_warn(hdev, "cannot read GPIO state: %d\n", ret);
+ else
+ dev->gpio = gbuf.gpio;
+
+ /*
+ * Not devm: ft260_remove() must remove the gpiochip before it
+ * stops the HID device and frees dev.
+ */
+ ret = gpiochip_add_data(&dev->gc, dev);
+ if (ret < 0)
+ hid_err(hdev, "cannot add GPIO chip %d\n", ret);
+ return ret;
+}
+
static int ft260_get_system_config(struct hid_device *hdev,
struct ft260_get_system_status_report *cfg)
{
@@ -935,29 +1385,36 @@ static int ft260_get_system_config(struct hid_device *hdev,
return 0;
}
-static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device *dev)
+static int ft260_get_interface_type(struct ft260_device *dev,
+ struct ft260_get_system_status_report *cfg)
+
{
- struct ft260_get_system_status_report cfg;
- struct usb_interface *usbif = to_usb_interface(hdev->dev.parent);
- int interface = usbif->cur_altsetting->desc.bInterfaceNumber;
int ret;
+ struct hid_device *hdev = dev->hdev;
+ struct usb_interface *usbif = to_usb_interface(hdev->dev.parent);
+
+ dev->iface_id = usbif->cur_altsetting->desc.bInterfaceNumber;
- ret = ft260_get_system_config(hdev, &cfg);
+ ret = ft260_get_system_config(hdev, cfg);
if (ret < 0)
return ret;
- ft260_dbg("interface: 0x%02x\n", interface);
- ft260_dbg("chip mode: 0x%02x\n", cfg.chip_mode);
- ft260_dbg("clock_ctl: 0x%02x\n", cfg.clock_ctl);
- ft260_dbg("i2c_enable: 0x%02x\n", cfg.i2c_enable);
- ft260_dbg("uart_mode: 0x%02x\n", cfg.uart_mode);
+ ft260_dbg("interface: 0x%02x\n", dev->iface_id);
+ ft260_dbg("chip mode: 0x%02x\n", cfg->chip_mode);
+ ft260_dbg("clock_ctl: 0x%02x\n", cfg->clock_ctl);
+ ft260_dbg("i2c_enable: 0x%02x\n", cfg->i2c_enable);
+ ft260_dbg("uart_mode: 0x%02x\n", cfg->uart_mode);
+ ft260_dbg("gpio2_func: 0x%02x\n", cfg->gpio2_func);
+ ft260_dbg("gpioA_func: 0x%02x\n", cfg->gpioa_func);
+ ft260_dbg("gpioG_func: 0x%02x\n", cfg->gpiog_func);
+ ft260_dbg("wakeup_int: 0x%02x\n", cfg->enable_wakeup_int);
- dev->power_saving_en = cfg.power_saving_en;
+ dev->power_saving_en = cfg->power_saving_en;
- switch (cfg.chip_mode) {
+ switch (cfg->chip_mode) {
case FT260_MODE_ALL:
case FT260_MODE_BOTH:
- if (interface == 1)
+ if (dev->iface_id == 1)
ret = FT260_IFACE_UART;
else
ret = FT260_IFACE_I2C;
@@ -969,7 +1426,7 @@ static int ft260_get_interface_type(struct hid_device *hdev, struct ft260_device
ret = FT260_IFACE_I2C;
break;
default:
- hid_err(hdev, "unknown chip mode: %d\n", cfg.chip_mode);
+ hid_err(hdev, "unknown chip mode: %d\n", cfg->chip_mode);
return -ENODEV;
}
@@ -1001,6 +1458,10 @@ static int ft260_word_show(struct hid_device *hdev, int id, u8 *cfg, int len,
return scnprintf(buf, PAGE_SIZE, "%d\n", le16_to_cpu(*field));
}
+static void ft260_attr_dummy_func(struct hid_device *hdev, u8 req, u16 value)
+{
+}
+
#define FT260_ATTR_SHOW(name, reptype, id, type, func) \
static ssize_t name##_show(struct device *kdev, \
struct device_attribute *attr, char *buf) \
@@ -1021,37 +1482,42 @@ static int ft260_word_show(struct hid_device *hdev, int id, u8 *cfg, int len,
FT260_ATTR_SHOW(name, ft260_get_i2c_status_report, \
FT260_I2C_STATUS, __le16, ft260_word_show)
-#define FT260_ATTR_STORE(name, reptype, id, req, type, ctype, func) \
+#define FT260_ATTR_STORE(name, reptype, id, req, type, ctype, strtou, func) \
static ssize_t name##_store(struct device *kdev, \
struct device_attribute *attr, \
const char *buf, size_t count) \
{ \
struct reptype rep; \
struct hid_device *hdev = to_hid_device(kdev); \
+ struct ft260_device *dev = hid_get_drvdata(hdev); \
type name; \
int ret; \
\
- if (!func(buf, 10, (ctype *)&name)) { \
+ if (!strtou(buf, 10, (ctype *)&name)) { \
rep.name = name; \
rep.report = id; \
rep.request = req; \
+ mutex_lock(&dev->lock); \
ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, \
sizeof(rep)); \
- if (!ret) \
- ret = count; \
+ if (ret < 0) \
+ hid_err(hdev, "%s: failed!\n", __func__); \
+ else \
+ func(hdev, req, name); \
+ mutex_unlock(&dev->lock); \
} else { \
ret = -EINVAL; \
} \
return ret; \
}
-#define FT260_BYTE_ATTR_STORE(name, reptype, req) \
+#define FT260_BYTE_ATTR_STORE(name, reptype, req, func) \
FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \
- u8, u8, kstrtou8)
+ u8, u8, kstrtou8, func)
-#define FT260_WORD_ATTR_STORE(name, reptype, req) \
+#define FT260_WORD_ATTR_STORE(name, reptype, req, func) \
FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \
- __le16, u16, kstrtou16)
+ __le16, u16, kstrtou16, func)
FT260_SSTAT_ATTR_SHOW(chip_mode);
static DEVICE_ATTR_RO(chip_mode);
@@ -1065,27 +1531,46 @@ static DEVICE_ATTR_RO(suspend_status);
FT260_SSTAT_ATTR_SHOW(hid_over_i2c_en);
static DEVICE_ATTR_RO(hid_over_i2c_en);
+FT260_SSTAT_ATTR_SHOW(gpio2_func);
+FT260_BYTE_ATTR_STORE(gpio2_func, ft260_set_gpio2_func_report,
+ FT260_SELECT_GPIO2_FUNC, ft260_gpio_en_update);
+static DEVICE_ATTR_RW(gpio2_func);
+
+FT260_SSTAT_ATTR_SHOW(gpioa_func);
+FT260_BYTE_ATTR_STORE(gpioa_func, ft260_set_gpioa_func_report,
+ FT260_SELECT_GPIOA_FUNC, ft260_gpio_en_update);
+static DEVICE_ATTR_RW(gpioa_func);
+
+FT260_SSTAT_ATTR_SHOW(gpiog_func);
+FT260_BYTE_ATTR_STORE(gpiog_func, ft260_set_gpiog_func_report,
+ FT260_SELECT_GPIOG_FUNC, ft260_gpio_en_update);
+static DEVICE_ATTR_RW(gpiog_func);
+
FT260_SSTAT_ATTR_SHOW(power_saving_en);
static DEVICE_ATTR_RO(power_saving_en);
FT260_SSTAT_ATTR_SHOW(i2c_enable);
FT260_BYTE_ATTR_STORE(i2c_enable, ft260_set_i2c_mode_report,
- FT260_SET_I2C_MODE);
+ FT260_SET_I2C_MODE, ft260_gpio_en_update);
static DEVICE_ATTR_RW(i2c_enable);
FT260_SSTAT_ATTR_SHOW(uart_mode);
FT260_BYTE_ATTR_STORE(uart_mode, ft260_set_uart_mode_report,
- FT260_SET_UART_MODE);
+ FT260_SET_UART_MODE, ft260_gpio_en_update);
static DEVICE_ATTR_RW(uart_mode);
+FT260_BYTE_ATTR_STORE(uart_dcd_ri, ft260_set_uart_dcd_ri_report,
+ FT260_ENABLE_UART_DCD_RI, ft260_gpio_en_update);
+static DEVICE_ATTR_WO(uart_dcd_ri);
+
FT260_SSTAT_ATTR_SHOW(clock_ctl);
FT260_BYTE_ATTR_STORE(clock_ctl, ft260_set_system_clock_report,
- FT260_SET_CLOCK);
+ FT260_SET_CLOCK, ft260_attr_dummy_func);
static DEVICE_ATTR_RW(clock_ctl);
FT260_I2CST_ATTR_SHOW(clock);
FT260_WORD_ATTR_STORE(clock, ft260_set_i2c_speed_report,
- FT260_SET_I2C_CLOCK_SPEED);
+ FT260_SET_I2C_CLOCK_SPEED, ft260_attr_dummy_func);
static DEVICE_ATTR_RW(clock);
static ssize_t i2c_reset_store(struct device *kdev,
@@ -1109,7 +1594,11 @@ static const struct attribute_group ft260_attr_group = {
&dev_attr_hid_over_i2c_en.attr,
&dev_attr_power_saving_en.attr,
&dev_attr_i2c_enable.attr,
+ &dev_attr_gpio2_func.attr,
+ &dev_attr_gpioa_func.attr,
+ &dev_attr_gpiog_func.attr,
&dev_attr_uart_mode.attr,
+ &dev_attr_uart_dcd_ri.attr,
&dev_attr_clock_ctl.attr,
&dev_attr_i2c_reset.attr,
&dev_attr_clock.attr,
@@ -1413,9 +1902,15 @@ static int ft260_uart_change_speed(struct ft260_device *port,
req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_NONE;
req.breaking = FT260_UART_CFG_BREAKING_NO;
+ mutex_lock(&port->lock);
+
ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
if (ret < 0)
hid_err(hdev, "failed to change termios: %d\n", ret);
+ else
+ ft260_gpio_en_update(hdev, FT260_SET_UART_MODE, req.flow_ctrl);
+
+ mutex_unlock(&port->lock);
return ret;
}
@@ -1631,9 +2126,11 @@ static const struct tty_port_operations ft260_uart_port_ops = {
static struct tty_driver *ft260_tty_driver;
-static int ft260_i2c_probe(struct hid_device *hdev, struct ft260_device *dev)
+static int ft260_i2c_probe(struct ft260_device *dev,
+ struct ft260_get_system_status_report *cfg)
{
int ret;
+ struct hid_device *hdev = dev->hdev;
hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n",
hdev->version >> 8, hdev->version & 0xff, hdev->name,
@@ -1660,27 +2157,35 @@ static int ft260_i2c_probe(struct hid_device *hdev, struct ft260_device *dev)
return ret;
}
+ ret = ft260_gpio_init(dev, cfg);
+ if (ret)
+ goto err_i2c_free;
+
ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
if (ret < 0) {
hid_err(hdev, "failed to create sysfs attrs\n");
- goto err_i2c_free;
+ goto err_gpio_remove;
}
+
return 0;
+err_gpio_remove:
+ gpiochip_remove(&dev->gc);
err_i2c_free:
i2c_del_adapter(&dev->adap);
return ret;
}
-static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
+static int ft260_uart_probe(struct ft260_device *dev,
+ struct ft260_get_system_status_report *cfg)
{
struct ft260_configure_uart_request_report req;
- int ret;
+ struct hid_device *hdev = dev->hdev;
struct device *devt;
+ int ret;
INIT_WORK(&dev->wakeup_work, ft260_uart_do_wakeup);
INIT_WORK(&dev->tx_work, ft260_uart_tx_work);
- // FIXME: are all kfifo access secured by lock? with irq or not?
ft260_uart_wakeup_workaraund_enable(dev, true);
/* Work not started at this point */
timer_setup(&dev->wakeup_timer, ft260_uart_start_wakeup, 0);
@@ -1710,6 +2215,20 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
goto err_hid_report;
}
+ cfg->uart_mode = FT260_UART_CFG_FLOW_CTRL_NONE;
+
+ if (dev->iface_id == 0) {
+ ret = ft260_gpio_init(dev, cfg);
+ if (ret)
+ goto err_hid_report;
+
+ ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
+ if (ret < 0) {
+ hid_err(hdev, "failed to create sysfs attrs\n");
+ goto err_gpio_remove;
+ }
+ }
+
devt = tty_port_register_device_attr(&dev->port,
ft260_tty_driver,
dev->index, &hdev->dev,
@@ -1717,13 +2236,19 @@ static int ft260_uart_probe(struct hid_device *hdev, struct ft260_device *dev)
if (IS_ERR(devt)) {
hid_err(hdev, "failed to register tty port\n");
ret = PTR_ERR(devt);
- goto err_hid_report;
+ goto err_register_tty;
}
hid_info(hdev, "registering device /dev/%s%d\n",
ft260_tty_driver->name, dev->index);
return 0;
+err_register_tty:
+ if (dev->gc.parent)
+ sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
+err_gpio_remove:
+ if (dev->gc.parent)
+ gpiochip_remove(&dev->gc);
err_hid_report:
ft260_uart_port_remove(dev);
err_port:
@@ -1735,6 +2260,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct ft260_device *dev;
struct ft260_get_chip_version_report version;
+ struct ft260_get_system_status_report cfg;
int ret;
if (!hid_is_usb(hdev))
@@ -1749,6 +2275,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto alloc_fail;
}
hid_set_drvdata(hdev, dev);
+ dev->hdev = hdev;
mutex_init(&dev->lock);
spin_lock_init(&dev->read_lock);
init_completion(&dev->wait);
@@ -1782,19 +2309,16 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
version.chip_code[0], version.chip_code[1],
version.chip_code[2], version.chip_code[3]);
- ret = ft260_get_interface_type(hdev, dev);
+ ret = ft260_get_interface_type(dev, &cfg);
if (ret <= FT260_IFACE_NONE)
goto err_hid_close;
- hid_set_drvdata(hdev, dev);
- dev->hdev = hdev;
-
- if (dev->iface_type == FT260_IFACE_I2C) {
- ret = ft260_i2c_probe(hdev, dev);
+ if (ret == FT260_IFACE_I2C) {
+ ret = ft260_i2c_probe(dev, &cfg);
if (ret)
goto err_hid_close;
} else {
- ret = ft260_uart_probe(hdev, dev);
+ ret = ft260_uart_probe(dev, &cfg);
if (ret)
goto err_uart;
}
@@ -1828,11 +2352,16 @@ static void ft260_remove(struct hid_device *hdev)
tty_port_unregister_device(&dev->port, ft260_tty_driver,
dev->index);
ft260_uart_port_remove(dev);
+ if (dev->iface_id == 0)
+ sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
} else {
sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
i2c_del_adapter(&dev->adap);
}
+ if (dev->gc.parent)
+ gpiochip_remove(&dev->gc);
+
hid_hw_close(hdev);
hid_hw_stop(hdev);
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 04/16] HID: ft260: i2c: reduce driver module loading time
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (2 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 03/16] HID: ft260: add GPIO support Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 05/16] HID: ft260: fix sysfs store failure notice for big numbers Michael Zaidman
` (11 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
The HWMON class I2C client drivers register the address range to scan
during the I2C master module probing. For example, the lm75 module
registers eight bus addresses scanned during the ft260 module loading.
In the case of ft260, the bus is scanned by ft260_i2c_read, but due to
an excessive read completion timeout, it took about 40 seconds for the
ft260 driver to scan the lm75 address range before exiting the probe
function.
Adjust the i2c_read timeout so it depends on the amount of data to
read, reducing the lm75 address range scan time to about 200ms.
The 25 ms per-report budget was checked on a UMFT260EV1A with a 24LC512
EEPROM by shrinking the timeout until reads began to fail: a 60-byte
read still completes at 9 ms and times out at 7 ms, against 5.4 ms of
wire time at 100 kHz, so the fixed USB, HID and scheduling overhead
costs a few milliseconds. At the chip's slowest 60 kHz clock that chunk
needs about 9 ms on the wire and a 180-byte chunk about 27 ms. Scale
the wait by 25 ms per 60-byte HID payload so a last chunk of 61..180
bytes gets 50..75 ms, not the single-report 25 ms budget.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fix for the issue reported by Sashiko: the timeout of the last I2C
read chunk is computed from rd_len, so a chunk of 61..180 bytes gets
50..75 ms, not 25 ms.
---
drivers/hid/hid-ft260.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index a924ca2eef88..50389b3e79d8 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -17,6 +17,7 @@
#include <linux/serial_core.h>
#include <linux/kfifo.h>
#include <linux/tty_flip.h>
+#include <linux/math.h>
#include <linux/minmax.h>
#include <linux/unaligned.h>
#include <linux/bits.h>
@@ -49,14 +50,20 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
* 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.
+ * transfers, we set the maximum read payload length to a multiple of
+ * FT260_RD_ONE_REPORT_MAX.
* 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_ONE_REPORT_MAX 60
#define FT260_RD_DATA_MAX 180
+
+/* Time in ms to wait for a single HID input report at ONE_REPORT_MAX */
+#define FT260_RD_ONE_REPORT_TO 25
+
#define FT260_WR_I2C_DATA_MAX 60
#define FT260_WR_UART_DATA_MAX 60
#define FT260_GPIOCHIP "ft260_gpio"
@@ -724,8 +731,8 @@ 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;
+ u16 rd_data_max = FT260_RD_ONE_REPORT_MAX;
+ int timeout, timeout_jiffies, ret = 0;
struct ft260_i2c_read_request_report rep;
struct hid_device *hdev = dev->hdev;
unsigned long irqflags;
@@ -750,6 +757,8 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
} else {
rd_len = rd_data_max;
}
+ timeout = FT260_RD_ONE_REPORT_TO *
+ DIV_ROUND_UP(rd_len, FT260_RD_ONE_REPORT_MAX);
rd_data_max = FT260_RD_DATA_MAX;
rep.report = FT260_I2C_READ_REQ;
@@ -774,8 +783,8 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
goto ft260_i2c_read_exit;
}
- timeout = msecs_to_jiffies(5000);
- if (!wait_for_completion_timeout(&dev->wait, timeout)) {
+ timeout_jiffies = msecs_to_jiffies(timeout);
+ if (!wait_for_completion_timeout(&dev->wait, timeout_jiffies)) {
ret = -ETIMEDOUT;
ft260_i2c_reset(hdev);
goto ft260_i2c_read_exit;
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 05/16] HID: ft260: fix sysfs store failure notice for big numbers
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (3 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 04/16] HID: ft260: i2c: reduce driver module loading time Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 06/16] HID: ft260: i2c: reduce bus-error message severity Michael Zaidman
` (10 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
FT260_ATTR_STORE returned the feature-report transfer length from
ft260_hid_feature_report_set() instead of the sysfs write size. For
values like 1000, userspace writes more bytes than the HID report
length, so the VFS retries the remainder and reports "Invalid argument"
even though the attribute was applied correctly:
$ echo 1000 > .../clock
bash: line 1: echo: write error: Invalid argument
$ cat .../clock
1000
Return count when the feature report succeeds, so the full write is
consumed, and the error when it fails.
Parse a 16-bit attribute, such as clock, into a u16 and convert it with
cpu_to_le16(), because the report field is little endian. Before, the
value was parsed straight into the __le16 field, which is wrong on a
big-endian CPU.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fixes for the issues reported by Sashiko:
- A failed feature report is not reported as success.
- A 16-bit attribute is converted with cpu_to_le16(), so it is correct
on a big-endian CPU.
v3:
Subject reworded after the original out-of-tree commit, "Get rid of
sysfs store big numbers failure notice". The v2 subject read as if the
patch hid big values. No i2c: prefix, because FT260_ATTR_STORE serves
all store attributes, not only the I2C ones.
---
drivers/hid/hid-ft260.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 50389b3e79d8..13dbff661fab 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1484,56 +1484,60 @@ static void ft260_attr_dummy_func(struct hid_device *hdev, u8 req, u16 value)
}
#define FT260_SSTAT_ATTR_SHOW(name) \
FT260_ATTR_SHOW(name, ft260_get_system_status_report, \
FT260_SYSTEM_SETTINGS, u8, ft260_byte_show)
#define FT260_I2CST_ATTR_SHOW(name) \
FT260_ATTR_SHOW(name, ft260_get_i2c_status_report, \
FT260_I2C_STATUS, __le16, ft260_word_show)
-#define FT260_ATTR_STORE(name, reptype, id, req, type, ctype, strtou, func) \
+#define FT260_STORE_NATIVE(v) (v)
+
+#define FT260_ATTR_STORE(name, reptype, id, req, type, strtou, func, conv) \
static ssize_t name##_store(struct device *kdev, \
struct device_attribute *attr, \
const char *buf, size_t count) \
{ \
struct reptype rep; \
struct hid_device *hdev = to_hid_device(kdev); \
struct ft260_device *dev = hid_get_drvdata(hdev); \
type name; \
int ret; \
\
- if (!strtou(buf, 10, (ctype *)&name)) { \
- rep.name = name; \
+ if (!strtou(buf, 10, &name)) { \
+ rep.name = conv(name); \
rep.report = id; \
rep.request = req; \
mutex_lock(&dev->lock); \
ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, \
sizeof(rep)); \
if (ret < 0) \
hid_err(hdev, "%s: failed!\n", __func__); \
- else \
+ else { \
func(hdev, req, name); \
+ ret = count; \
+ } \
mutex_unlock(&dev->lock); \
} else { \
ret = -EINVAL; \
} \
return ret; \
}
#define FT260_BYTE_ATTR_STORE(name, reptype, req, func) \
FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \
- u8, u8, kstrtou8, func)
+ u8, kstrtou8, func, FT260_STORE_NATIVE)
#define FT260_WORD_ATTR_STORE(name, reptype, req, func) \
FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \
- __le16, u16, kstrtou16, func)
+ u16, kstrtou16, func, cpu_to_le16)
FT260_SSTAT_ATTR_SHOW(chip_mode);
static DEVICE_ATTR_RO(chip_mode);
FT260_SSTAT_ATTR_SHOW(pwren_status);
static DEVICE_ATTR_RO(pwren_status);
FT260_SSTAT_ATTR_SHOW(suspend_status);
static DEVICE_ATTR_RO(suspend_status);
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 06/16] HID: ft260: i2c: reduce bus-error message severity
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (4 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 05/16] HID: ft260: fix sysfs store failure notice for big numbers Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 07/16] HID: ft260: uart: enable flow control Michael Zaidman
` (9 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
The driver logged errors when the FT260 reported an I2C bus error or
when ft260_i2c_write / ft260_smbus_write failed after such a status.
That was meant to flag unexpected failures, but bus scans (i2cdetect,
HWMON discovery, transceiver monitoring, and similar) hit missing
devices routinely and should not fill dmesg with lines such as:
ft260 0003:0403:6030.0001: ft260_i2c_write: failed with -5
Keep returning -EIO, but print those via ft260_dbg. Still use hid_err in
ft260_hid_output_report_check_status for actual HID/USB transport
failures, which are genuine errors.
Drop the extra __func__ from the write-path ft260_dbg calls; the helper
already prints the function name.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fix for the issue reported by Sashiko: the write-path messages
printed the function name twice, because ft260_dbg() already prints it.
---
drivers/hid/hid-ft260.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 13dbff661fab..270481766e2c 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -577,7 +577,7 @@ static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy)
* 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);
+ ft260_dbg("i2c bus error: %#02x\n", report.bus_status);
return -EIO;
}
@@ -611,6 +611,7 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
ret = ft260_hid_output_report(hdev, data, len);
if (ret < 0) {
+ hid_err(hdev, "%s: failed with %d\n", __func__, ret);
ft260_i2c_reset(hdev);
return ret;
}
@@ -650,7 +651,6 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
int len, u8 flag)
{
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->i2c_wr_buf;
@@ -681,7 +681,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
ret = ft260_hid_output_report_check_status(dev, (u8 *)rep,
wr_len + 4);
if (ret < 0) {
- hid_err(hdev, "%s: failed with %d\n", __func__, ret);
+ ft260_dbg("failed with %d\n", ret);
return ret;
}
@@ -697,7 +697,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
u8 *data, u8 data_len, u8 flag)
{
- int ret = 0;
+ int ret;
int len = 4;
struct ft260_i2c_write_request_report *rep =
@@ -722,7 +722,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len);
if (ret < 0)
- hid_err(dev->hdev, "%s: failed with %d\n", __func__, ret);
+ ft260_dbg("failed with %d\n", ret);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 07/16] HID: ft260: uart: enable flow control
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (5 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 06/16] HID: ft260: i2c: reduce bus-error message severity Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 08/16] HID: ft260: uart: add modem pins control via ioctl Michael Zaidman
` (8 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
The FT260 is a full-speed USB device. Interrupt transfers poll at 1 ms
with a 64-byte max packet size, so the theoretical UART payload ceiling
is about 60 kB/s (~600 kbaud in 8N1). The chip advertises baud rates
above 1M and small transfers can work there, but large transfers are not
reliable above 460800 baud without flow control.
Stop forcing FT260_UART_CFG_FLOW_CTRL_NONE after termios setup. Honor
CRTSCTS (RTS/CTS) and IXON/IXOFF (XON/XOFF) when configuring the UART.
Tested at 1500000 baud both directions against an FT2232H with XON/XOFF
and with RTS/CTS (~2.9 MB transfers, no data corruption).
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 270481766e2c..75ba68e2e8fc 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1903,8 +1903,12 @@ static int ft260_uart_change_speed(struct ft260_device *port,
if (termios->c_cflag & CRTSCTS)
req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_RTS_CTS;
+ else if (termios->c_iflag & (IXON | IXOFF))
+ req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_XON_XOFF;
else
- req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_OFF;
+ req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_NONE;
+
+ req.breaking = FT260_UART_CFG_BREAKING_NO;
ft260_dbg("configured termios: flow control: %d, baudrate: %d, ",
req.flow_ctrl, baud);
@@ -1912,9 +1916,6 @@ static int ft260_uart_change_speed(struct ft260_device *port,
req.data_bit, req.parity,
req.stop_bit, req.breaking);
- req.flow_ctrl = FT260_UART_CFG_FLOW_CTRL_NONE;
- req.breaking = FT260_UART_CFG_BREAKING_NO;
-
mutex_lock(&port->lock);
ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 08/16] HID: ft260: uart: add modem pins control via ioctl
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (6 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 07/16] HID: ft260: uart: enable flow control Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 09/16] HID: ft260: group sysfs attrs per HID interface Michael Zaidman
` (7 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
Add TIOCMGET/TIOCMSET (and thus TIOCMBIS/TIOCMBIC) support so userspace
can drive DTR/RTS when the modem pins are in GPIO mode. This enables
programming an ESP32 over the FT260 with esptool, which resets the chip
via those lines.
Set up the gpiochip and the sysfs attributes on the UART interface when
the chip mode has UART, and on the I2C interface only in the I2C-only
mode. Before, the choice depended on USB interface number 0, which is
the I2C interface in mode 3, but TIOCMGET and TIOCMSET run on the UART
interface and need the GPIO state there. Add DCD and RI (GPIO4 and
GPIO5) to the GPIO set of UART modes 3 and 4, so the mask that TIOCMGET
and TIOCMSET check covers the two lines that TIOCMGET reports.
The RTS, CTS, DTR and DSR pins are active-low, so TIOCMGET inverts the
sense of those four lines and TIOCMSET drives 0 to assert them. DCD and
RI are not inverted. Rio Liu found and fixed the polarity.
Keep the errno from ft260_get_interface_type(), so that a failed
status read does not let probe succeed after the teardown.
TIOCMGET and TIOCMSET work only while the modem pins are GPIOs, and
return -EINVAL otherwise. They check gpio_en under port->lock, so that
a termios or sysfs change cannot switch the pins to flow control
between the check and the GPIO report. TIOCMGET returns the error of
the GPIO read. TIOCMSET builds its report from the cached GPIO state,
which it copies under the same lock.
The tty hangup does not wait for a TIOCMGET or TIOCMSET that already
runs, because the tty core handles them without a line discipline
reference. ft260_remove() sets a flag under port->lock after the
hangup, so it waits for such a call, and a later one returns -ENODEV
instead of using hdev after hid_hw_stop().
Co-developed-by: Rio Liu <rio@r26.me>
Signed-off-by: Rio Liu <rio@r26.me>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Polarity fix folded in (Rio Liu).
Fixes for the issues reported by Sashiko:
- The errno of ft260_get_interface_type() is kept, so probe does not
succeed after the teardown.
- TIOCMGET and TIOCMSET return -EINVAL when the modem pins are not
GPIOs, and TIOCMGET returns the error of the GPIO read.
- TIOCMSET copies the cached GPIO state under port->lock.
v3:
Fixes for the issues reported by Sashiko:
- gpio_en checked under port->lock in tiocmget() and tiocmset(), so a
termios or sysfs change cannot switch the pins between the check and
the GPIO report.
- tiocmget() and tiocmset() return -ENODEV after ft260_remove(), so
they do not use hdev after hid_hw_stop().
- tiocmget() cannot run before dev->gc is set, because
ft260_uart_probe() registers the TTY last since patch 2.
---
drivers/hid/hid-ft260.c | 168 ++++++++++++++++++++++++++++++++++++----
1 file changed, 151 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 75ba68e2e8fc..d795355fc26b 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -16,6 +16,7 @@
#include <linux/serial.h>
#include <linux/serial_core.h>
#include <linux/kfifo.h>
+#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <linux/math.h>
#include <linux/minmax.h>
@@ -183,7 +184,7 @@ enum {
FT260_GPIO_DIR_OUTPUT = 0x01,
};
-/* GPIO offsets */
+/* GPIO bit masks */
#define FT260_GPIO_0 BIT(0)
#define FT260_GPIO_1 BIT(1)
#define FT260_GPIO_2 BIT(2)
@@ -213,7 +214,8 @@ enum {
#define FT260_GPIO_UART_MODE_1_SET FT260_GPIO_UART_DTR_DSR
#define FT260_GPIO_UART_MODE_2_SET FT260_GPIO_UART_RTS_CTS
#define FT260_GPIO_UART_MODE_3_SET (FT260_GPIO_UART_RTS_CTS | \
- FT260_GPIO_UART_DTR_DSR)
+ FT260_GPIO_UART_DTR_DSR | \
+ FT260_GPIO_UART_DCD_RI)
#define FT260_GPIO_UART_MODE_4_SET FT260_GPIO_UART_MODE_3_SET
#define FT260_GPIO_UART_DEFAULT FT260_GPIO_UART_MODE_0_SET
#define FT260_GPIO_UART_MODE_1_CLR (FT260_GPIO_UART_RX_TX | \
@@ -223,6 +225,16 @@ enum {
#define FT260_GPIO_UART_MODE_3_CLR FT260_GPIO_UART_RX_TX
#define FT260_GPIO_UART_MODES 5
+/* UART modem control GPIO offsets */
+enum {
+ FT260_GPIO_UART_DCD = (4),
+ FT260_GPIO_UART_RI = (5),
+ FT260_GPIO_UART_RTS = (7),
+ FT260_GPIO_UART_CTS = (10),
+ FT260_GPIO_UART_DTR = (11),
+ FT260_GPIO_UART_DSR = (13),
+};
+
#define FT260_SET_REQUEST_VALUE(report_id) ((FT260_FEATURE << 8) | (report_id))
/* Feature In reports */
@@ -449,6 +461,7 @@ MODULE_DEVICE_TABLE(hid, ft260_devices);
struct ft260_device {
struct i2c_adapter adap;
struct hid_device *hdev;
+ int chip_mode;
int iface_type;
int iface_id;
struct list_head device_list;
@@ -480,6 +493,8 @@ struct ft260_device {
struct ft260_gpio_state gpio;
u16 gpio_uart_mode[FT260_GPIO_UART_MODES];
bool uart_dcd_ri;
+ /* Set under lock by ft260_remove(); tiocm calls then stop */
+ bool removed;
};
static int ft260_hid_feature_report_get(struct hid_device *hdev,
@@ -1111,8 +1126,27 @@ static void ft260_gpio_en_update(struct hid_device *hdev, u8 req, u8 value)
else
ft260_gpio_en_clr(dev, bitmap);
exit:
- ft260_dbg("%s: enabled GPIOs: %04x\n", dev_name(&hdev->dev),
- dev->gpio_en);
+ ft260_dbg("%s: enabled GPIOs: %04x, bitmap %04x\n",
+ dev_name(&hdev->dev), dev->gpio_en, bitmap);
+}
+
+static void ft260_gpio_output_cfg(struct ft260_gpio_state *gpio,
+ u32 offset, int value)
+{
+ if (offset < FT260_GPIO_0_5_COUNT) {
+ gpio->dirs |= BIT(offset);
+ if (value)
+ gpio->vals |= !!value << offset;
+ else
+ gpio->vals &= ~BIT(offset);
+ } else {
+ offset = offset - FT260_GPIO_0_5_COUNT;
+ gpio->ex_dirs |= BIT(offset);
+ if (value)
+ gpio->ex_vals |= !!value << offset;
+ else
+ gpio->ex_vals &= ~BIT(offset);
+ }
}
static int ft260_gpio_set(struct gpio_chip *gc, u32 offset, int value)
@@ -1161,7 +1195,7 @@ static int ft260_gpio_set(struct gpio_chip *gc, u32 offset, int value)
rep.gpio.ex_vals &= ~BIT(offset);
}
- ft260_dbg("dirs %#02x vals %#02x ex_dir %#02x ex_vals %#02x\n",
+ ft260_dbg("dirs %#02x vals %#02x ex_dirs %#02x ex_vals %#02x\n",
rep.gpio.dirs, rep.gpio.vals,
rep.gpio.ex_dirs, rep.gpio.ex_vals);
@@ -1419,6 +1453,7 @@ static int ft260_get_interface_type(struct ft260_device *dev,
ft260_dbg("wakeup_int: 0x%02x\n", cfg->enable_wakeup_int);
dev->power_saving_en = cfg->power_saving_en;
+ dev->chip_mode = cfg->chip_mode;
switch (cfg->chip_mode) {
case FT260_MODE_ALL:
@@ -1439,7 +1474,6 @@ static int ft260_get_interface_type(struct ft260_device *dev,
return -ENODEV;
}
- dev->iface_type = ret;
return ret;
}
@@ -1947,6 +1981,91 @@ static void ft260_uart_set_termios(struct tty_struct *tty,
ft260_uart_change_speed(port, &tty->termios, NULL);
}
+static int ft260_uart_tiocmget(struct tty_struct *tty)
+{
+ struct ft260_device *port = tty->driver_data;
+ int gpios;
+
+ /*
+ * The modem lines control via ioctl is allowed only
+ * when the related pins are in the gpio mode.
+ */
+ int mask = FT260_GPIO_UART_MODE_4_SET;
+
+ mutex_lock(&port->lock);
+ if (port->removed) {
+ mutex_unlock(&port->lock);
+ return -ENODEV;
+ }
+ if ((port->gpio_en & mask) != mask) {
+ ft260_dbg("Illegal gpio config %#02x != mask %#04x\n",
+ port->gpio_en, mask);
+ mutex_unlock(&port->lock);
+ return -EINVAL;
+ }
+
+ gpios = ft260_gpio_get_all(&port->gc, FT260_GPIO_VALUE);
+ mutex_unlock(&port->lock);
+ if (gpios < 0)
+ return gpios;
+
+ return ((!(gpios & FT260_GPIO_B) ? TIOCM_RTS : 0) |
+ (!(gpios & FT260_GPIO_E) ? TIOCM_CTS : 0) |
+ (!(gpios & FT260_GPIO_F) ? TIOCM_DTR : 0) |
+ (!(gpios & FT260_GPIO_H) ? TIOCM_DSR : 0) |
+ ((gpios & FT260_GPIO_4) ? TIOCM_CAR : 0) |
+ ((gpios & FT260_GPIO_5) ? TIOCM_RNG : 0));
+}
+
+static int ft260_uart_tiocmset(struct tty_struct *tty,
+ unsigned int set, unsigned int clear)
+{
+ int ret;
+ struct ft260_device *port = tty->driver_data;
+ struct ft260_gpio_write_request_report rep;
+ struct hid_device *hdev = port->hdev;
+ int mask = FT260_GPIO_UART_MODE_4_SET;
+
+ mutex_lock(&port->lock);
+ if (port->removed) {
+ mutex_unlock(&port->lock);
+ return -ENODEV;
+ }
+ if ((port->gpio_en & mask) != mask) {
+ ft260_dbg("Illegal gpio config %#02x != mask %#04x\n",
+ port->gpio_en, mask);
+ mutex_unlock(&port->lock);
+ return -EINVAL;
+ }
+ rep.report = FT260_GPIO;
+ rep.gpio = port->gpio;
+
+ if (set & TIOCM_RTS)
+ ft260_gpio_output_cfg(&rep.gpio, FT260_GPIO_UART_RTS, 0);
+ if (set & TIOCM_DTR)
+ ft260_gpio_output_cfg(&rep.gpio, FT260_GPIO_UART_DTR, 0);
+ if (clear & TIOCM_RTS)
+ ft260_gpio_output_cfg(&rep.gpio, FT260_GPIO_UART_RTS, 1);
+ if (clear & TIOCM_DTR)
+ ft260_gpio_output_cfg(&rep.gpio, FT260_GPIO_UART_DTR, 1);
+
+ ft260_dbg("dirs %#02x vals %#02x ex_dirs %#02x ex_vals %#02x\n",
+ rep.gpio.dirs, rep.gpio.vals,
+ rep.gpio.ex_dirs, rep.gpio.ex_vals);
+
+ ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, sizeof(rep));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
+ mutex_unlock(&port->lock);
+ return -EIO;
+ }
+
+ port->gpio = rep.gpio;
+ mutex_unlock(&port->lock);
+
+ return 0;
+}
+
static int ft260_uart_install(struct tty_driver *driver, struct tty_struct *tty)
{
int idx = tty->index;
@@ -2027,6 +2146,8 @@ static const struct tty_operations ft260_uart_ops = {
.write_room = ft260_uart_write_room,
.chars_in_buffer = ft260_uart_chars_in_buffer,
.set_termios = ft260_uart_set_termios,
+ .tiocmget = ft260_uart_tiocmget,
+ .tiocmset = ft260_uart_tiocmset,
.hangup = ft260_uart_hangup,
.install = ft260_uart_install,
.cleanup = ft260_uart_cleanup,
@@ -2171,14 +2292,16 @@ static int ft260_i2c_probe(struct ft260_device *dev,
return ret;
}
- ret = ft260_gpio_init(dev, cfg);
- if (ret)
- goto err_i2c_free;
+ if (cfg->chip_mode == FT260_MODE_I2C) {
+ ret = ft260_gpio_init(dev, cfg);
+ if (ret)
+ goto err_i2c_free;
- ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
- if (ret < 0) {
- hid_err(hdev, "failed to create sysfs attrs\n");
- goto err_gpio_remove;
+ ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
+ if (ret < 0) {
+ hid_err(hdev, "failed to create sysfs attrs\n");
+ goto err_gpio_remove;
+ }
}
return 0;
@@ -2231,7 +2354,8 @@ static int ft260_uart_probe(struct ft260_device *dev,
cfg->uart_mode = FT260_UART_CFG_FLOW_CTRL_NONE;
- if (dev->iface_id == 0) {
+ if (dev->chip_mode & FT260_MODE_UART ||
+ dev->chip_mode == FT260_MODE_ALL) {
ret = ft260_gpio_init(dev, cfg);
if (ret)
goto err_hid_report;
@@ -2326,8 +2450,9 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = ft260_get_interface_type(dev, &cfg);
if (ret <= FT260_IFACE_NONE)
goto err_hid_close;
+ dev->iface_type = ret;
- if (ret == FT260_IFACE_I2C) {
+ if (dev->iface_type == FT260_IFACE_I2C) {
ret = ft260_i2c_probe(dev, &cfg);
if (ret)
goto err_hid_close;
@@ -2366,10 +2491,19 @@ static void ft260_remove(struct hid_device *hdev)
tty_port_unregister_device(&dev->port, ft260_tty_driver,
dev->index);
ft260_uart_port_remove(dev);
- if (dev->iface_id == 0)
+ /*
+ * The hangup does not wait for a TIOCMGET or TIOCMSET
+ * that is running; they hold the lock while they use hdev.
+ */
+ mutex_lock(&dev->lock);
+ dev->removed = true;
+ mutex_unlock(&dev->lock);
+ if (dev->chip_mode & FT260_MODE_UART ||
+ dev->chip_mode == FT260_MODE_ALL)
sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
} else {
- sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
+ if (dev->chip_mode == FT260_MODE_I2C)
+ sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
i2c_del_adapter(&dev->adap);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 09/16] HID: ft260: group sysfs attrs per HID interface
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (7 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 08/16] HID: ft260: uart: add modem pins control via ioctl Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 10/16] HID: ft260: i2c: fix large write transaction failure Michael Zaidman
` (6 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
Split the single sysfs attribute group into per-interface groups based
on chip mode (DCNF0/DCNF1):
- I2C-only (mode 1): I2C and GPIO attributes on the I2C HID interface
- Dual mode (0/3): hid_over_i2c_en, i2c_reset and clock on the I2C
interface; GPIO and UART attrs on the UART interface
- UART-only (mode 2): GPIO and UART attrs on the UART interface
i2c_enable is GPIO-related too: it switches GPIO0 and GPIO1 between I2C
and GPIO, and its store updates gpio_en of its own interface. So it is
on the interface with the gpiochip, the UART interface in modes 0 and 3.
i2c_reset is an I2C attribute, so it is only on the I2C interface and
not in the UART group, which holds the GPIO and UART attributes.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fixes for the issues reported by Sashiko:
- The i2c_enable attribute was missing in modes 0 and 3; it is now in
the UART group, with the gpiochip.
- The i2c_reset attribute, an I2C attribute, is no longer in the UART
group.
v3:
The err_register_tty path, which patch 3 adds to ft260_uart_probe(),
removes the UART attribute group of this patch.
---
drivers/hid/hid-ft260.c | 63 ++++++++++++++++++++++++++++++++++-------
1 file changed, 53 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index d795355fc26b..b68cb0748a23 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1633,7 +1633,7 @@ static ssize_t i2c_reset_store(struct device *kdev,
}
static DEVICE_ATTR_WO(i2c_reset);
-static const struct attribute_group ft260_attr_group = {
+static const struct attribute_group ft260_i2c_chip_mode_1_attr_group = {
.attrs = (struct attribute *[]) {
&dev_attr_chip_mode.attr,
&dev_attr_pwren_status.attr,
@@ -1644,8 +1644,6 @@ static const struct attribute_group ft260_attr_group = {
&dev_attr_gpio2_func.attr,
&dev_attr_gpioa_func.attr,
&dev_attr_gpiog_func.attr,
- &dev_attr_uart_mode.attr,
- &dev_attr_uart_dcd_ri.attr,
&dev_attr_clock_ctl.attr,
&dev_attr_i2c_reset.attr,
&dev_attr_clock.attr,
@@ -1653,6 +1651,32 @@ static const struct attribute_group ft260_attr_group = {
}
};
+static const struct attribute_group ft260_i2c_chip_mode_0_3_attr_group = {
+ .attrs = (struct attribute *[]) {
+ &dev_attr_hid_over_i2c_en.attr,
+ &dev_attr_i2c_reset.attr,
+ &dev_attr_clock.attr,
+ NULL
+ }
+};
+
+static const struct attribute_group ft260_uart_chip_mode_0_2_3_attr_group = {
+ .attrs = (struct attribute *[]) {
+ &dev_attr_chip_mode.attr,
+ &dev_attr_pwren_status.attr,
+ &dev_attr_suspend_status.attr,
+ &dev_attr_power_saving_en.attr,
+ &dev_attr_i2c_enable.attr,
+ &dev_attr_gpio2_func.attr,
+ &dev_attr_gpioa_func.attr,
+ &dev_attr_gpiog_func.attr,
+ &dev_attr_uart_mode.attr,
+ &dev_attr_uart_dcd_ri.attr,
+ &dev_attr_clock_ctl.attr,
+ NULL
+ }
+};
+
static DEFINE_MUTEX(ft260_uart_list_lock);
static LIST_HEAD(ft260_uart_device_list);
@@ -2297,11 +2321,20 @@ static int ft260_i2c_probe(struct ft260_device *dev,
if (ret)
goto err_i2c_free;
- ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
+ ret = sysfs_create_group(&hdev->dev.kobj,
+ &ft260_i2c_chip_mode_1_attr_group);
if (ret < 0) {
hid_err(hdev, "failed to create sysfs attrs\n");
goto err_gpio_remove;
}
+ } else if (cfg->chip_mode == FT260_MODE_ALL ||
+ cfg->chip_mode == FT260_MODE_BOTH) {
+ ret = sysfs_create_group(&hdev->dev.kobj,
+ &ft260_i2c_chip_mode_0_3_attr_group);
+ if (ret < 0) {
+ hid_err(hdev, "failed to create sysfs attrs\n");
+ goto err_i2c_free;
+ }
}
return 0;
@@ -2355,12 +2388,14 @@ static int ft260_uart_probe(struct ft260_device *dev,
cfg->uart_mode = FT260_UART_CFG_FLOW_CTRL_NONE;
if (dev->chip_mode & FT260_MODE_UART ||
- dev->chip_mode == FT260_MODE_ALL) {
+ dev->chip_mode == FT260_MODE_ALL ||
+ dev->chip_mode == FT260_MODE_BOTH) {
ret = ft260_gpio_init(dev, cfg);
if (ret)
goto err_hid_report;
- ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
+ ret = sysfs_create_group(&hdev->dev.kobj,
+ &ft260_uart_chip_mode_0_2_3_attr_group);
if (ret < 0) {
hid_err(hdev, "failed to create sysfs attrs\n");
goto err_gpio_remove;
@@ -2383,7 +2418,8 @@ static int ft260_uart_probe(struct ft260_device *dev,
err_register_tty:
if (dev->gc.parent)
- sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
+ sysfs_remove_group(&hdev->dev.kobj,
+ &ft260_uart_chip_mode_0_2_3_attr_group);
err_gpio_remove:
if (dev->gc.parent)
gpiochip_remove(&dev->gc);
@@ -2499,11 +2535,18 @@ static void ft260_remove(struct hid_device *hdev)
dev->removed = true;
mutex_unlock(&dev->lock);
if (dev->chip_mode & FT260_MODE_UART ||
- dev->chip_mode == FT260_MODE_ALL)
- sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
+ dev->chip_mode == FT260_MODE_ALL ||
+ dev->chip_mode == FT260_MODE_BOTH)
+ sysfs_remove_group(&hdev->dev.kobj,
+ &ft260_uart_chip_mode_0_2_3_attr_group);
} else {
if (dev->chip_mode == FT260_MODE_I2C)
- sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
+ sysfs_remove_group(&hdev->dev.kobj,
+ &ft260_i2c_chip_mode_1_attr_group);
+ else if (dev->chip_mode == FT260_MODE_ALL ||
+ dev->chip_mode == FT260_MODE_BOTH)
+ sysfs_remove_group(&hdev->dev.kobj,
+ &ft260_i2c_chip_mode_0_3_attr_group);
i2c_del_adapter(&dev->adap);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 10/16] HID: ft260: i2c: fix large write transaction failure
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (8 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 09/16] HID: ft260: group sysfs attrs per HID interface Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 11/16] HID: ft260: workaround for TN_189 errata endpoint STALL after enumeration Michael Zaidman
` (5 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman, Chris Keeser
Fixes a regression from commit 5afac727defa ("HID: ft260: missed NACK
from busy device"), which waited for bus-idle after every HID write
report except when the report flag was exactly FT260_FLAG_START.
Multi-report I2C writes keep the bus busy between chunks for atomicity.
Treating those middle reports like a final STOP caused ft260_xfer_status()
to spin on -EAGAIN after the second HID report and fail large writes.
Write-then-read combined transfers still worked.
Pass the bus-busy check policy from the caller: ft260_i2c_write() and
ft260_smbus_write() wait for idle only when the report they send carries
STOP; intermediate chunks and the command phase of a combined transfer
do not.
The SMBus side matters for register reads. The command phase is written
with FT260_FLAG_START and no STOP, because a repeated START read follows
and the controller deliberately keeps the bus busy to keep the
transaction atomic. Checking the busy bit there makes
ft260_xfer_status() return -EAGAIN on every retry and the command write
fail with -EIO, which breaks probing of clients that read registers:
ads1x19 6-0040: Failed to read config register: -5
leds-pca963x: probe of 6-0008 failed with error -5
Fixes: 5afac727defa ("HID: ft260: missed NACK from busy device")
Reported-by: Chris Keeser <chriskeeser@meta.com>
Closes: https://github.com/MichaelZaidman/hid-ft260/issues/35
Tested-by: Chris Keeser <chriskeeser@meta.com>
Reported-by: Bruno Giacomazzi <brunoceg1@gmail.com>
Closes: https://github.com/MichaelZaidman/hid-ft260/issues/42
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 35 ++++++++++++++++-------------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index b68cb0748a23..f160b5360f89 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -580,9 +580,14 @@ static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy)
}
dev->clock = le16_to_cpu(report.clock);
- ft260_dbg("bus_status %#02x, clock %u\n", report.bus_status,
- dev->clock);
+ ft260_dbg("bus_status %#02x, clock %u, bus_busy %#02x\n",
+ report.bus_status, dev->clock, bus_busy);
+ /*
+ * 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 (report.bus_status & (FT260_I2C_STATUS_CTRL_BUSY | bus_busy))
return -EAGAIN;
@@ -616,13 +621,10 @@ 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 *data, int len, u8 bus_busy)
{
- 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) {
@@ -639,16 +641,6 @@ 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, bus_busy);
if (ret != -EAGAIN)
@@ -665,6 +657,7 @@ 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 len, u8 flag)
{
+ u8 bus_busy = 0;
int ret, wr_len, idx = 0;
struct ft260_i2c_write_request_report *rep =
(struct ft260_i2c_write_request_report *)dev->i2c_wr_buf;
@@ -677,8 +670,10 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
do {
if (len <= FT260_WR_I2C_DATA_MAX) {
wr_len = len;
- if (flag == FT260_FLAG_START_STOP)
+ if (flag == FT260_FLAG_START_STOP) {
rep->flag |= FT260_FLAG_STOP;
+ bus_busy = FT260_I2C_STATUS_BUS_BUSY;
+ }
} else {
wr_len = FT260_WR_I2C_DATA_MAX;
}
@@ -694,7 +689,7 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
rep->flag, data[0]);
ret = ft260_hid_output_report_check_status(dev, (u8 *)rep,
- wr_len + 4);
+ wr_len + 4, bus_busy);
if (ret < 0) {
ft260_dbg("failed with %d\n", ret);
return ret;
@@ -735,7 +730,9 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
ft260_dbg("rep %#02x addr %#02x cmd %#02x datlen %d replen %d\n",
rep->report, addr, cmd, rep->length, len);
- ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len);
+ ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len,
+ (flag & FT260_FLAG_STOP) ?
+ FT260_I2C_STATUS_BUS_BUSY : 0);
if (ret < 0)
ft260_dbg("failed with %d\n", ret);
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 11/16] HID: ft260: workaround for TN_189 errata endpoint STALL after enumeration
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (9 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 10/16] HID: ft260: i2c: fix large write transaction failure Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 12/16] HID: ft260: i2c: abort in-flight transfers with STOP before reset Michael Zaidman
` (4 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman, Andreas Boose
FTDI errata TN_189 (Section 2.1) documents a silicon bug where the
FT260's USB interrupt endpoints are occasionally halted right after
enumeration. When this happens, Clear-Feature ENDPOINT_HALT does not
recover the endpoint and the only known recovery is a USB device reset.
Implement a workaround:
1. ft260_check_intr_ep_health() observes the STALL by attempting an
actual interrupt IN transfer. The FT260 does not honestly report
its halt state via USB_REQ_GET_STATUS (returns 0 even when
STALLed; confirmed separately by FTDI engineering [1] with a
USB analyzer trace), so we cannot rely on it; instead we let the
host controller return -EPIPE when it sees the STALL handshake.
2. ft260_check_dev_responsive() catches the broader broken state
where the interrupt endpoint may look healthy but the device
still fails to respond to control transfers. A USB_REQ_GET_STATUS
to the device with a short 500 ms timeout fails fast on a broken
device, preventing later probe stages from hanging on usbhid's
default 10 s timeouts and starving the usb_hub_wq workqueue.
3. When either check fails, probe schedules a deferred work item
and returns -ENODEV so that hub_event releases the device lock
quickly. Both interfaces of a stalled chip fail their probe, so
they share a per-chip struct ft260_chip, and only the first
failing probe queues the work. The work item retries
usb_lock_device_for_reset() up to 10 times (~10 s; each attempt
already polls for up to one second) before giving up, then
unbinds all USB interfaces, calls usb_reset_device() and binds
them again, to force usbhid to recreate the HID devices and
trigger a fresh ft260_probe(). The unbind+rebind step is needed
because usbhid's pre_reset and post_reset both return 0, so
usb_reset_device() alone keeps usbhid bound to stale HID device
state. The work clears the pending flag after the unbind, so a
probe that fails after the reset can queue the next one.
An earlier version of this workaround, which reset the chip once per
interface, was tested by FTDI engineering [1] on a Raspberry Pi 4
Model B Rev 1.5 running Linux 6.12.62-v8+ on an xhci_hcd host, with the
FT260 connected at full-speed through a downstream USB 2.0 hub. Across
28,684 re-enumeration cycles, 350 cycles triggered the recovery path.
Two of those required two consecutive USB resets before the device
returned. All 28,684 cycles recovered to a fully functional state with
I2C and UART working end-to-end. This version, with the fixes below,
was tested on an Intel 9 Series xHCI host running Linux 7.3 with KASAN,
the FT260 in mode 3 at full speed behind a USB 2.0 hub, reset by its
RESETN pin. 101 of 7,781 cycles triggered the recovery path. In each of
them both endpoints were halted and the chip was reset once. All cycles
recovered with I2C and UART working, and KASAN reported nothing.
Queue the work on a driver-owned ordered workqueue flushed on module
exit. Do not treat a GFP failure, in the driver or in usb_control_msg(),
as a TN_189 stall. Queue one reset per chip: a reset queued by the
second interface would tear down the chip that the first reset has
recovered, and a chip that never recovers would add two work items per
cycle. Do not cap the resets: a chip that stalls again after a reset
gets the next one. Unbind before the reset: the unbind waits for a
probe that still runs on either interface, so the reset does not run
while that probe stops usbhid and frees its URBs.
Reported-by: Andreas Boose <Andreas.Boose@almex.de>
Closes: https://github.com/MichaelZaidman/hid-ft260/issues/40 [1]
Link: https://ftdichip.com/wp-content/uploads/2026/05/TN_189-FT260-Errata-Technical-Note.pdf
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v2:
Fixes for the issues reported by Sashiko:
- Reset work queued on a driver-owned workqueue that is flushed on
module exit, so the work does not run after the module is unloaded.
- No USB reset on -ENOMEM, so an allocation failure in probe is not
taken as a TN_189 stall.
Left as is, reported by Sashiko:
- The number of resets in a row is not capped, because a chip that
stalls again after a reset needs the next one; in the FTDI test,
two of the 350 recoveries needed two resets in a row.
v3:
Fixes for the issues reported by Sashiko:
- One reset per chip: struct ft260_chip, shared by the two interfaces,
is added here, so a reset queued by the second interface does not
tear down the chip that the first reset has recovered.
- Interfaces unbound before the USB reset, not after, so the reset
does not run while a failing probe stops usbhid and frees its URBs.
- -ENOMEM from usb_control_msg() returned as it is, not turned into
-ENODEV and a reset.
---
drivers/hid/hid-ft260.c | 366 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 364 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index f160b5360f89..f51ba3df8890 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -12,10 +12,12 @@
#include <linux/hidraw.h>
#include <linux/i2c.h>
#include <linux/module.h>
+#include <linux/workqueue.h>
#include <linux/usb.h>
#include <linux/serial.h>
#include <linux/serial_core.h>
#include <linux/kfifo.h>
+#include <linux/kref.h>
#include <linux/tty.h>
#include <linux/tty_flip.h>
#include <linux/math.h>
@@ -495,8 +497,67 @@ struct ft260_device {
bool uart_dcd_ri;
/* Set under lock by ft260_remove(); tiocm calls then stop */
bool removed;
+ struct ft260_chip *chip;
};
+/*
+ * The I2C and the UART interface of one chip are two HID devices, each
+ * with its own ft260_device. They share one ft260_chip, so that only
+ * one of them queues the TN_189 reset.
+ */
+struct ft260_chip {
+ struct list_head list;
+ struct kref kref;
+ struct usb_device *udev;
+ /* A TN_189 reset is queued; protected by ft260_chip_list_lock */
+ bool reset_pending;
+};
+
+static LIST_HEAD(ft260_chip_list);
+static DEFINE_MUTEX(ft260_chip_list_lock);
+
+static struct ft260_chip *ft260_chip_get(struct usb_device *udev)
+{
+ struct ft260_chip *chip;
+
+ mutex_lock(&ft260_chip_list_lock);
+ list_for_each_entry(chip, &ft260_chip_list, list) {
+ if (chip->udev == udev) {
+ kref_get(&chip->kref);
+ goto out;
+ }
+ }
+
+ chip = kzalloc_obj(*chip, GFP_KERNEL);
+ if (chip) {
+ kref_init(&chip->kref);
+ chip->udev = usb_get_dev(udev);
+ list_add_tail(&chip->list, &ft260_chip_list);
+ }
+out:
+ mutex_unlock(&ft260_chip_list_lock);
+ return chip;
+}
+
+static void ft260_chip_release(struct kref *kref)
+{
+ struct ft260_chip *chip = container_of(kref, struct ft260_chip, kref);
+
+ list_del(&chip->list);
+ usb_put_dev(chip->udev);
+ kfree(chip);
+}
+
+static void ft260_chip_put(struct ft260_chip *chip)
+{
+ if (!chip)
+ return;
+
+ mutex_lock(&ft260_chip_list_lock);
+ kref_put(&chip->kref, ft260_chip_release);
+ mutex_unlock(&ft260_chip_list_lock);
+}
+
static int ft260_hid_feature_report_get(struct hid_device *hdev,
u8 report_id, u8 *data, size_t len)
{
@@ -2271,6 +2332,7 @@ static void ft260_uart_port_destroy(struct tty_port *tport)
struct ft260_device *port =
container_of(tport, struct ft260_device, port);
+ ft260_chip_put(port->chip);
kfree(port);
}
@@ -2427,15 +2489,276 @@ static int ft260_uart_probe(struct ft260_device *dev,
return ret;
}
+/*
+ * FT260 errata TN_189 Section 2.1: the USB interrupt endpoints are
+ * occasionally halted right after enumeration. When this happens:
+ * - Standard Clear-Feature ENDPOINT_HALT does not recover the endpoint
+ * - Subsequent communication with the device is dead
+ * - The only known recovery is a USB device reset
+ *
+ * A separate finding from FTDI engineering (confirmed by USB analyzer
+ * trace while testing this workaround) is that the FT260 does NOT
+ * honestly report the halt state via USB_REQ_GET_STATUS: it returns 0
+ * even when the endpoint is STALLed. Detection must therefore observe
+ * the STALL handshake at the host controller level rather than ask
+ * the device.
+ *
+ * Recovery is performed by a deferred work item that unbinds all
+ * interfaces, resets the USB device and binds the interfaces again to
+ * force usbhid to destroy stale HID devices and create fresh ones,
+ * which triggers a new ft260_probe() that succeeds.
+ *
+ * https://ftdichip.com/wp-content/uploads/2026/05/TN_189-FT260-Errata-Technical-Note.pdf
+ */
+struct ft260_reset_work {
+ struct work_struct work;
+ struct usb_interface *usbif;
+ struct ft260_chip *chip;
+};
+
+static struct workqueue_struct *ft260_reset_wq;
+
+static void ft260_chip_reset_done(struct ft260_chip *chip)
+{
+ mutex_lock(&ft260_chip_list_lock);
+ chip->reset_pending = false;
+ mutex_unlock(&ft260_chip_list_lock);
+}
+
+static void ft260_reset_and_rebind(struct work_struct *ws)
+{
+ struct ft260_reset_work *rw =
+ container_of(ws, struct ft260_reset_work, work);
+ struct usb_interface *usbif = rw->usbif;
+ struct usb_device *usbdev = interface_to_usbdev(usbif);
+ struct usb_host_config *actconfig;
+ int ret, i, attempt;
+
+ /*
+ * Retry the device lock for up to ~10 seconds. The lock is held
+ * by hub_event for the duration of device enumeration; with the
+ * fast-fail responsiveness check in probe, both interfaces should
+ * abort within ~1-2 seconds, after which the lock becomes free.
+ * Each usb_lock_device_for_reset() attempt already polls for up to
+ * one second internally.
+ */
+ for (attempt = 0; attempt < 10; attempt++) {
+ ret = usb_lock_device_for_reset(usbdev, NULL);
+ if (ret >= 0)
+ break;
+ if (ret == -ENODEV || ret == -EHOSTUNREACH) {
+ dev_dbg(&usbif->dev,
+ "device gone before reset (%d), abort\n", ret);
+ ft260_chip_reset_done(rw->chip);
+ goto out;
+ }
+ /* -EBUSY: someone else holds the lock; retry. */
+ }
+ if (ret < 0) {
+ dev_err(&usbif->dev,
+ "failed to acquire USB device lock for reset after %d attempts: %d\n",
+ attempt, ret);
+ ft260_chip_reset_done(rw->chip);
+ goto out;
+ }
+
+ /*
+ * usb_reset_device() keeps usbhid bound (its pre_reset/post_reset
+ * both return 0) and does not re-trigger HID-level driver probing.
+ * Unbind all USB interfaces, reset, and bind them again to force
+ * usbhid to destroy stale HID devices and create new ones, which
+ * triggers fresh ft260_probe() calls. Unbind before the reset: it
+ * waits for a probe still running on either interface, so the
+ * reset does not run while that probe stops usbhid.
+ */
+ actconfig = usbdev->actconfig;
+ for (i = 0; actconfig && i < actconfig->desc.bNumInterfaces; i++) {
+ struct usb_interface *intf = actconfig->interface[i];
+
+ if (intf && intf->dev.driver)
+ device_release_driver(&intf->dev);
+ }
+
+ /*
+ * No probe of the old enumeration runs now. A probe that fails
+ * after this reset may queue the next one.
+ */
+ ft260_chip_reset_done(rw->chip);
+
+ ret = usb_reset_device(usbdev);
+ if (ret < 0) {
+ dev_err(&usbif->dev, "USB reset failed: %d\n", ret);
+ usb_unlock_device(usbdev);
+ goto out;
+ }
+
+ actconfig = usbdev->actconfig;
+ for (i = 0; actconfig && i < actconfig->desc.bNumInterfaces; i++) {
+ struct usb_interface *intf = actconfig->interface[i];
+
+ if (!intf)
+ continue;
+ ret = device_attach(&intf->dev);
+ if (ret < 0)
+ dev_err(&intf->dev,
+ "failed to rebind USB interface: %d\n", ret);
+ }
+
+ usb_unlock_device(usbdev);
+out:
+ ft260_chip_put(rw->chip);
+ usb_put_intf(usbif);
+ kfree(rw);
+}
+
+/*
+ * Both interfaces of a stalled chip fail their probe. Queue one reset
+ * per chip: a second one would tear down the chip that the first one
+ * has recovered.
+ */
+static int ft260_schedule_reset(struct usb_interface *usbif)
+{
+ struct ft260_reset_work *rw;
+ struct ft260_chip *chip;
+ bool pending;
+
+ rw = kmalloc_obj(*rw, GFP_KERNEL);
+ if (!rw)
+ return -ENOMEM;
+
+ chip = ft260_chip_get(interface_to_usbdev(usbif));
+ if (!chip) {
+ kfree(rw);
+ return -ENOMEM;
+ }
+
+ mutex_lock(&ft260_chip_list_lock);
+ pending = chip->reset_pending;
+ chip->reset_pending = true;
+ mutex_unlock(&ft260_chip_list_lock);
+ if (pending) {
+ ft260_chip_put(chip);
+ kfree(rw);
+ return 0;
+ }
+
+ usb_get_intf(usbif);
+ rw->usbif = usbif;
+ rw->chip = chip;
+ INIT_WORK(&rw->work, ft260_reset_and_rebind);
+ queue_work(ft260_reset_wq, &rw->work);
+
+ return 0;
+}
+
+/*
+ * Detect whether the device's interrupt IN endpoint is in the STALL
+ * state described by TN_189. GET_STATUS is unreliable on the FT260
+ * (returns 0 even when halted, confirmed by FTDI with a USB analyzer
+ * trace), so observe the STALL handshake by attempting an actual
+ * interrupt IN transfer. The host controller returns -EPIPE when it
+ * receives a STALL handshake.
+ *
+ * Must be called before hid_hw_open() so it does not race against
+ * usbhid's own interrupt IN URB.
+ */
+static int ft260_check_intr_ep_health(struct hid_device *hdev)
+{
+ struct usb_interface *usbif = to_usb_interface(hdev->dev.parent);
+ struct usb_device *usbdev = interface_to_usbdev(usbif);
+ struct usb_host_interface *iface_desc = usbif->cur_altsetting;
+ struct usb_endpoint_descriptor *ep = NULL;
+ unsigned int pipe;
+ u8 *buf;
+ int ret, actual_length, i;
+
+ for (i = 0; i < iface_desc->desc.bNumEndpoints; i++) {
+ if (usb_endpoint_is_int_in(&iface_desc->endpoint[i].desc)) {
+ ep = &iface_desc->endpoint[i].desc;
+ break;
+ }
+ }
+ if (!ep)
+ return 0;
+
+ buf = kmalloc(FT260_REPORT_MAX_LEN, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ pipe = usb_rcvintpipe(usbdev, ep->bEndpointAddress);
+ ret = usb_interrupt_msg(usbdev, pipe, buf, FT260_REPORT_MAX_LEN,
+ &actual_length, 100);
+ kfree(buf);
+
+ if (ret == -EPIPE) {
+ hid_warn(hdev,
+ "interrupt IN ep %#x halted (TN_189 errata), scheduling USB reset and rebind\n",
+ ep->bEndpointAddress);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+/*
+ * Quick check that the device responds to a standard control transfer.
+ * When the FT260 is in the buggy post-enumeration state, control
+ * transfers initiated by later probe stages (chip version retrieval,
+ * UART/I2C configuration, etc.) can hang for very long periods,
+ * starving the usb_hub_wq workqueue and preventing the reset work
+ * from acquiring the device lock.
+ *
+ * Issue USB_REQ_GET_STATUS to the device (any compliant USB device
+ * must answer immediately) with a short explicit timeout. If it
+ * fails, treat the device as broken and bail out before reaching
+ * anything that can block.
+ *
+ * The interrupt-endpoint health check above only catches STALLs on
+ * the interrupt IN path; this check catches the broader broken state
+ * that affects the other interface even when its interrupt endpoint
+ * happens to look healthy.
+ */
+static int ft260_check_dev_responsive(struct hid_device *hdev)
+{
+ struct usb_interface *usbif = to_usb_interface(hdev->dev.parent);
+ struct usb_device *usbdev = interface_to_usbdev(usbif);
+ __le16 *status;
+ int ret;
+
+ status = kmalloc_obj(*status, GFP_KERNEL);
+ if (!status)
+ return -ENOMEM;
+
+ ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
+ USB_REQ_GET_STATUS,
+ USB_DIR_IN | USB_RECIP_DEVICE,
+ 0, 0, status, sizeof(*status), 500);
+ kfree(status);
+ if (ret == -ENOMEM)
+ return ret;
+
+ if (ret < 0) {
+ hid_warn(hdev,
+ "device unresponsive to GET_STATUS (%d), suspected TN_189 errata, scheduling USB reset and rebind\n",
+ ret);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
struct ft260_device *dev;
+ struct usb_interface *usbif;
struct ft260_get_chip_version_report version;
struct ft260_get_system_status_report cfg;
int ret;
if (!hid_is_usb(hdev))
return -EINVAL;
+
+ usbif = to_usb_interface(hdev->dev.parent);
/*
* We cannot use devm_kzalloc here because the port has to survive
* until destroy function call.
@@ -2451,6 +2774,12 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
spin_lock_init(&dev->read_lock);
init_completion(&dev->wait);
+ dev->chip = ft260_chip_get(interface_to_usbdev(usbif));
+ if (!dev->chip) {
+ ret = -ENOMEM;
+ goto hid_fail;
+ }
+
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "failed to parse HID\n");
@@ -2463,6 +2792,27 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto hid_fail;
}
+ /*
+ * TN_189 errata workaround: bail out fast on a broken device so
+ * that hub_event releases the device lock quickly, allowing the
+ * scheduled reset work to acquire it and recover the device.
+ */
+ ret = ft260_check_intr_ep_health(hdev);
+ if (ret == -ENODEV) {
+ ft260_schedule_reset(usbif);
+ goto err_hid_stop;
+ }
+ if (ret)
+ goto err_hid_stop;
+
+ ret = ft260_check_dev_responsive(hdev);
+ if (ret == -ENODEV) {
+ ft260_schedule_reset(usbif);
+ goto err_hid_stop;
+ }
+ if (ret)
+ goto err_hid_stop;
+
ret = hid_hw_open(hdev);
if (ret) {
hid_err(hdev, "failed to open HID HW\n");
@@ -2508,6 +2858,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
err_hid_stop:
hid_hw_stop(hdev);
hid_fail:
+ ft260_chip_put(dev->chip);
kfree(dev);
alloc_fail:
return ret;
@@ -2553,10 +2904,12 @@ static void ft260_remove(struct hid_device *hdev)
hid_hw_close(hdev);
hid_hw_stop(hdev);
- if (dev->iface_type == FT260_IFACE_UART)
+ if (dev->iface_type == FT260_IFACE_UART) {
ft260_uart_port_put(dev);
- else
+ } else {
+ ft260_chip_put(dev->chip);
kfree(dev);
+ }
}
static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
@@ -2661,6 +3014,12 @@ static int __init ft260_driver_init(void)
goto err_reg_driver;
}
+ ft260_reset_wq = alloc_ordered_workqueue("hid-ft260-reset", 0);
+ if (!ft260_reset_wq) {
+ ret = -ENOMEM;
+ goto err_reg_driver_unreg;
+ }
+
ret = hid_register_driver(&ft260_driver);
if (ret) {
pr_err("hid_register_driver failed: %d\n", ret);
@@ -2670,6 +3029,8 @@ static int __init ft260_driver_init(void)
return 0;
err_reg_hid:
+ destroy_workqueue(ft260_reset_wq);
+err_reg_driver_unreg:
tty_unregister_driver(ft260_tty_driver);
err_reg_driver:
tty_driver_kref_put(ft260_tty_driver);
@@ -2680,6 +3041,7 @@ static int __init ft260_driver_init(void)
static void __exit ft260_driver_exit(void)
{
hid_unregister_driver(&ft260_driver);
+ destroy_workqueue(ft260_reset_wq);
tty_unregister_driver(ft260_tty_driver);
tty_driver_kref_put(ft260_tty_driver);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 12/16] HID: ft260: i2c: abort in-flight transfers with STOP before reset
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (10 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 11/16] HID: ft260: workaround for TN_189 errata endpoint STALL after enumeration Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 13/16] HID: ft260: i2c: reject an I2C clock outside the supported range Michael Zaidman
` (3 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
A transfer that ends without STOP leaves the FT260 I2C controller
with BUS_BUSY set. I2C reset (AN_394 section 4.4.15) restarts the
master but, with the driver still bound, status stays 0x60
(CTRL_IDLE | BUS_BUSY) even when SCL and SDA are pulled up. The
next START then fails with 0x72 (arbitration lost + error) and
further resets do not recover; only a USB replug did.
Add ft260_i2c_abort(): a STOP-only write (flag 0x04, zero payload)
then FT260_SET_I2C_RESET. Use it wherever the host tears down an
open transaction:
- read wait timeout
- read status check failure after the HID report arrived
- write status poll failure after the report was sent
- SMBus block read with an invalid count (count phase has no STOP)
Leave ft260_i2c_reset() alone for HID output failure (the request
may never have reached the controller), sysfs i2c_reset, and probe.
Tested on a UMFT260EV1A with a 24LC512 at 0x51. Five 1 ms read
timeouts each followed by STOP+reset left bus_status 0x20; raising
the timeout to 25 ms on the same loaded module then read 16 bytes
successfully.
This matters more with the 25/75 ms completion timeout from
"HID: ft260: i2c: reduce driver module loading time", which makes
read timeouts more likely than mainline's 5000 ms wait.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 43 ++++++++++++++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index f51ba3df8890..5564ed62cc48 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -613,6 +613,33 @@ static int ft260_i2c_reset(struct hid_device *hdev)
return ret;
}
+static int ft260_hid_output_report(struct hid_device *hdev, u8 *data,
+ size_t len);
+
+/*
+ * STOP with no START and no payload. Used by ft260_i2c_abort() when
+ * tearing down an in-flight transfer: I2C reset alone can leave
+ * BUS_BUSY set even when SCL/SDA are idle, and the next START then
+ * fails with arbitration lost.
+ */
+static int ft260_i2c_stop(struct hid_device *hdev, u8 addr)
+{
+ u8 buf[4] = {
+ FT260_I2C_REPORT_MIN,
+ addr,
+ FT260_FLAG_STOP,
+ 0,
+ };
+
+ return ft260_hid_output_report(hdev, buf, sizeof(buf));
+}
+
+static int ft260_i2c_abort(struct hid_device *hdev, u8 addr)
+{
+ ft260_i2c_stop(hdev, addr);
+ return ft260_i2c_reset(hdev);
+}
+
static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy)
{
struct hid_device *hdev = dev->hdev;
@@ -682,7 +709,8 @@ 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)
+ u8 *data, int len, u8 addr,
+ u8 bus_busy)
{
int ret, usec, try = 100;
struct hid_device *hdev = dev->hdev;
@@ -711,7 +739,7 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
if (ret == 0)
return 0;
- ft260_i2c_reset(hdev);
+ ft260_i2c_abort(hdev, addr);
return -EIO;
}
@@ -750,7 +778,8 @@ static int ft260_i2c_write(struct ft260_device *dev, u8 addr, u8 *data,
rep->flag, data[0]);
ret = ft260_hid_output_report_check_status(dev, (u8 *)rep,
- wr_len + 4, bus_busy);
+ wr_len + 4, addr,
+ bus_busy);
if (ret < 0) {
ft260_dbg("failed with %d\n", ret);
return ret;
@@ -791,7 +820,7 @@ static int ft260_smbus_write(struct ft260_device *dev, u8 addr, u8 cmd,
ft260_dbg("rep %#02x addr %#02x cmd %#02x datlen %d replen %d\n",
rep->report, addr, cmd, rep->length, len);
- ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len,
+ ret = ft260_hid_output_report_check_status(dev, (u8 *)rep, len, addr,
(flag & FT260_FLAG_STOP) ?
FT260_I2C_STATUS_BUS_BUSY : 0);
if (ret < 0)
@@ -859,7 +888,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
timeout_jiffies = msecs_to_jiffies(timeout);
if (!wait_for_completion_timeout(&dev->wait, timeout_jiffies)) {
ret = -ETIMEDOUT;
- ft260_i2c_reset(hdev);
+ ft260_i2c_abort(hdev, addr);
goto ft260_i2c_read_exit;
}
@@ -873,7 +902,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
ret = ft260_xfer_status(dev, bus_busy);
if (ret < 0) {
ret = -EIO;
- ft260_i2c_reset(hdev);
+ ft260_i2c_abort(hdev, addr);
goto ft260_i2c_read_exit;
}
@@ -1057,7 +1086,7 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
hid_warn(hdev,
"smbus block read: invalid count %u from slave 0x%02x\n",
count, addr);
- ft260_i2c_reset(hdev);
+ ft260_i2c_abort(hdev, addr);
ret = -EPROTO;
goto smbus_exit;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 13/16] HID: ft260: i2c: reject an I2C clock outside the supported range
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (11 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 12/16] HID: ft260: i2c: abort in-flight transfers with STOP before reset Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 14/16] HID: ft260: i2c: fix i2c_reset sysfs store return value Michael Zaidman
` (2 subsequent siblings)
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
The clock sysfs attribute passes any 16-bit value to the chip. The chip
accepts it, reports it back in the I2C status report that clock_show
reads, and runs the bus at the 100 kHz default. A read of 2048 bytes
takes the same 0.298 s after writing 50, 3500 or 65535 as it does after
writing 100, while 60 and 3400 give 0.387 s and 0.058 s. Userspace has
no way to see that the value was not applied.
Refuse a value outside the 60-3400 kHz range named in the datasheet
section 4.4.16. ft260_uart_change_speed() checks the baud rate against
FT260_UART_CFG_BAUD_MIN and FT260_UART_CFG_BAUD_MAX, but termios has no
error return, so it falls back to 9600. A sysfs store can return an
error, so the clock store refuses the value.
FT260_WORD_ATTR_STORE() takes the parse function as a parameter now, so
a word attribute with no range of its own keeps passing kstrtou16.
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 5564ed62cc48..391128e4bb35 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -69,6 +69,11 @@ MODULE_PARM_DESC(debug, "Toggle FT260 debugging messages");
#define FT260_WR_I2C_DATA_MAX 60
#define FT260_WR_UART_DATA_MAX 60
+
+/* I2C bus clock range in kHz, see the FT260 datasheet section 4.4.16 */
+#define FT260_I2C_CLOCK_MIN 60
+#define FT260_I2C_CLOCK_MAX 3400
+
#define FT260_GPIOCHIP "ft260_gpio"
#define FT260_GPIO_0_5_COUNT 6
#define FT260_GPIO_A_H_COUNT 8
@@ -1592,6 +1597,25 @@ static void ft260_attr_dummy_func(struct hid_device *hdev, u8 req, u16 value)
{
}
+/*
+ * The chip takes any 16-bit value and reports it back in the I2C status
+ * report, but runs the bus at the 100 kHz default when the value is
+ * outside the documented range. The report gives no way to tell the two
+ * cases apart, so refuse the value here.
+ */
+static int ft260_i2c_clock_parse(const char *buf, unsigned int base, u16 *clock)
+{
+ int ret = kstrtou16(buf, base, clock);
+
+ if (ret)
+ return ret;
+
+ if (*clock < FT260_I2C_CLOCK_MIN || *clock > FT260_I2C_CLOCK_MAX)
+ return -EINVAL;
+
+ return 0;
+}
+
#define FT260_ATTR_SHOW(name, reptype, id, type, func) \
static ssize_t name##_show(struct device *kdev, \
struct device_attribute *attr, char *buf) \
@@ -1649,9 +1673,9 @@ static void ft260_attr_dummy_func(struct hid_device *hdev, u8 req, u16 value)
FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \
u8, kstrtou8, func, FT260_STORE_NATIVE)
-#define FT260_WORD_ATTR_STORE(name, reptype, req, func) \
+#define FT260_WORD_ATTR_STORE(name, reptype, req, strtou, func) \
FT260_ATTR_STORE(name, reptype, FT260_SYSTEM_SETTINGS, req, \
- u16, kstrtou16, func, cpu_to_le16)
+ u16, strtou, func, cpu_to_le16)
FT260_SSTAT_ATTR_SHOW(chip_mode);
static DEVICE_ATTR_RO(chip_mode);
@@ -1704,7 +1728,8 @@ static DEVICE_ATTR_RW(clock_ctl);
FT260_I2CST_ATTR_SHOW(clock);
FT260_WORD_ATTR_STORE(clock, ft260_set_i2c_speed_report,
- FT260_SET_I2C_CLOCK_SPEED, ft260_attr_dummy_func);
+ FT260_SET_I2C_CLOCK_SPEED, ft260_i2c_clock_parse,
+ ft260_attr_dummy_func);
static DEVICE_ATTR_RW(clock);
static ssize_t i2c_reset_store(struct device *kdev,
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 14/16] HID: ft260: i2c: fix i2c_reset sysfs store return value
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (12 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 13/16] HID: ft260: i2c: reject an I2C clock outside the supported range Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 15/16] HID: ft260: i2c: recover a bus held by a slave Michael Zaidman
2026-11-14 23:08 ` [PATCH v3 16/16] HID: ft260: use lock guards Michael Zaidman
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
A sysfs store must return the number of bytes it consumed. kernfs
hands that value straight to write(2). i2c_reset_store() returns
whatever ft260_i2c_reset() returns, which is the length of the
feature report it sent, 2, not 0. The "return count" below it never
runs, so every write to i2c_reset returns 2 whatever its size.
A write of two bytes is right by coincidence, because the report is
two bytes long. A caller that leaves out the newline, as C and Python
code usually does, writes one byte and gets 2 back, which is more
than it asked for:
echo -n 1 > i2c_reset 1 byte, write() returns 2, bash ignores it
printf 1 > i2c_reset 1 byte, dash reports an I/O error
echo 1 > i2c_reset 2 bytes, correct by coincidence
Return 0 from ft260_i2c_reset() on success. The value is discarded by
its other callers, ft260_hid_output_report_check_status(),
ft260_i2c_probe() and ft260_i2c_abort(), whose own four callers
discard it as well.
Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver")
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
drivers/hid/hid-ft260.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 391128e4bb35..80b300893658 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -615,7 +615,7 @@ static int ft260_i2c_reset(struct hid_device *hdev)
}
ft260_dbg("done\n");
- return ret;
+ return 0;
}
static int ft260_hid_output_report(struct hid_device *hdev, u8 *data,
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 15/16] HID: ft260: i2c: recover a bus held by a slave
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (13 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 14/16] HID: ft260: i2c: fix i2c_reset sysfs store return value Michael Zaidman
@ 2026-11-14 23:07 ` Michael Zaidman
2026-11-14 23:08 ` [PATCH v3 16/16] HID: ft260: use lock guards Michael Zaidman
15 siblings, 0 replies; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:07 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
An SMBus block read takes the first byte the slave sends as the block
length. An EEPROM has no length byte, it answers with stored data, and
ft260_smbus_xfer() rejects a value above I2C_SMBUS_BLOCK_MAX. That
count phase carries no STOP, so the slave keeps SDA low while it waits
for the next clock. FT260_SET_I2C_RESET restarts the controller but
does not clock the bus, the chip cannot put a STOP on the line, and
every later START ends in arbitration lost.
A 24c02 holds 0x68 at offset 0x80, so on that bus
i2cget -y 0 0x50 0x80 s
ft260 0003:0403:6030.0011: smbus block read: invalid count 104 from slave 0x50
killed the bus: every transfer after it failed with bus_status 0x72
(bus busy, controller idle, arbitration lost, error). The sysfs
i2c_reset, unbind and bind of the HID interface, and rmmod followed by
insmod all left the status at 0x72.
The fix is the I2C bus recovery: nine SCL pulses let the slave clock
out the rest of its byte and release SDA. The I2C controller of the
FT260 cannot send them, but SCL and SDA are GPIO0 and GPIO1 of the
chip. ft260_i2c_prepare_recovery() switches the I2C controller off,
which turns the two pins into GPIOs, and i2c_generic_scl_recovery()
drives them to make the pulses and a STOP.
ft260_i2c_unprepare_recovery() switches the controller on again.
ft260_i2c_abort() starts the recovery after the controller reset if
the bus status still has BUS_BUSY set. It does not test the error bit,
because the reset clears it: right after the reset the chip reports
0x60 (bus busy, controller idle).
Split ft260_gpio_pin_set() out of ft260_gpio_direction_set() and
ft260_gpio_read_all() out of ft260_gpio_get_all(). The recovery needs
them without the gpio_en check and without dev->lock: SCL and SDA are
not GPIOs while I2C runs, and the abort path already holds dev->lock.
In chip modes 0 and 3 the recovery runs on the I2C interface, and the
gpiochip and the modem lines run on the UART interface. Each interface
has its own ft260_device and lock, and both read, change and write the
chip-wide FT260_GPIO report. The two interfaces of one chip already
share an ft260_chip for the TN_189 reset; it now also holds a lock for
that report. The recovery changes the report from the I2C interface, so
tiocmset() reads it under this lock and no longer starts from its
cached copy.
With the patch, the same i2cget still fails with -EPROTO, because the
count is invalid, but the bus is free again: a plain read of the same
EEPROM, i2ctransfer -y 0 w1@0x50 0x00 r4, succeeds. A read from an
address that no slave answers (an address NAK) does not start the
recovery: a full i2cdetect scan of 112 addresses logs no "recovering"
line.
Fixes: 2d68d67151eb ("HID: ft260: fix SMBus block read protocol handling")
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v3:
Recovery info per adapter in struct ft260_device, not one static
copy, because i2c_init_recovery() writes into it for each adapter.
Fix for the issue reported by Sashiko:
- GPIO report lock in the ft260_chip, shared by the I2C and UART
interfaces of one chip, so a GPIO write from one interface is not
lost by a write from the other; tiocmset() reads the report.
---
drivers/hid/hid-ft260.c | 233 +++++++++++++++++++++++++++++++++-------
1 file changed, 196 insertions(+), 37 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 80b300893658..53321c8d70fd 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -189,6 +189,8 @@ enum {
FT260_GPIO_DIRECTION = 0x01,
FT260_GPIO_DIR_INPUT = 0x00,
FT260_GPIO_DIR_OUTPUT = 0x01,
+ FT260_GPIO_SCL_OFFSET = 0, /* GPIO0 */
+ FT260_GPIO_SDA_OFFSET = 1, /* GPIO1 */
};
/* GPIO bit masks */
@@ -467,6 +469,8 @@ MODULE_DEVICE_TABLE(hid, ft260_devices);
struct ft260_device {
struct i2c_adapter adap;
+ /* Per adapter: i2c_init_recovery() writes into it */
+ struct i2c_bus_recovery_info rinfo;
struct hid_device *hdev;
int chip_mode;
int iface_type;
@@ -508,7 +512,8 @@ struct ft260_device {
/*
* The I2C and the UART interface of one chip are two HID devices, each
* with its own ft260_device. They share one ft260_chip, so that only
- * one of them queues the TN_189 reset.
+ * one of them queues the TN_189 reset. Both also read, change and write
+ * the chip-wide FT260_GPIO report.
*/
struct ft260_chip {
struct list_head list;
@@ -516,6 +521,8 @@ struct ft260_chip {
struct usb_device *udev;
/* A TN_189 reset is queued; protected by ft260_chip_list_lock */
bool reset_pending;
+ /* Serializes the read-modify-write of the FT260_GPIO report */
+ struct mutex gpio_lock;
};
static LIST_HEAD(ft260_chip_list);
@@ -537,6 +544,7 @@ static struct ft260_chip *ft260_chip_get(struct usb_device *udev)
if (chip) {
kref_init(&chip->kref);
chip->udev = usb_get_dev(udev);
+ mutex_init(&chip->gpio_lock);
list_add_tail(&chip->list, &ft260_chip_list);
}
out:
@@ -550,6 +558,7 @@ static void ft260_chip_release(struct kref *kref)
list_del(&chip->list);
usb_put_dev(chip->udev);
+ mutex_destroy(&chip->gpio_lock);
kfree(chip);
}
@@ -618,6 +627,22 @@ static int ft260_i2c_reset(struct hid_device *hdev)
return 0;
}
+/*
+ * Enable or disable the I2C controller. With the controller disabled
+ * GPIO0 and GPIO1 carry their GPIO function instead of SCL and SDA.
+ */
+static int ft260_i2c_set_mode(struct hid_device *hdev, u8 enable)
+{
+ struct ft260_set_i2c_mode_report report;
+
+ report.report = FT260_SYSTEM_SETTINGS;
+ report.request = FT260_SET_I2C_MODE;
+ report.i2c_enable = enable;
+
+ return ft260_hid_feature_report_set(hdev, (u8 *)&report,
+ sizeof(report));
+}
+
static int ft260_hid_output_report(struct hid_device *hdev, u8 *data,
size_t len);
@@ -639,10 +664,46 @@ static int ft260_i2c_stop(struct hid_device *hdev, u8 addr)
return ft260_hid_output_report(hdev, buf, sizeof(buf));
}
-static int ft260_i2c_abort(struct hid_device *hdev, u8 addr)
+static int ft260_i2c_bus_status(struct ft260_device *dev)
+{
+ struct ft260_get_i2c_status_report report;
+ int ret;
+
+ ret = ft260_hid_feature_report_get(dev->hdev, FT260_I2C_STATUS,
+ (u8 *)&report, sizeof(report));
+ if (ret < 0)
+ return ret;
+
+ return report.bus_status;
+}
+
+/*
+ * I2C reset does not clock the bus. A slave left in the middle of a
+ * byte keeps SDA low, the chip cannot put a STOP on the bus, and every
+ * later START fails with arbitration lost. The chip reports this as
+ * BUS_BUSY once the reset is done; only driving SCL as a GPIO frees the
+ * slave, so fall back to bus recovery.
+ */
+static int ft260_i2c_abort(struct ft260_device *dev, u8 addr)
{
- ft260_i2c_stop(hdev, addr);
- return ft260_i2c_reset(hdev);
+ int ret;
+
+ ft260_i2c_stop(dev->hdev, addr);
+
+ ret = ft260_i2c_reset(dev->hdev);
+ if (ret < 0)
+ return ret;
+
+ ret = ft260_i2c_bus_status(dev);
+ if (ret < 0)
+ return ret;
+
+ if (ret & FT260_I2C_STATUS_BUS_BUSY) {
+ ft260_dbg("bus_status %#02x, recovering\n", ret);
+ return i2c_recover_bus(&dev->adap);
+ }
+
+ return 0;
}
static int ft260_xfer_status(struct ft260_device *dev, u8 bus_busy)
@@ -744,7 +805,7 @@ static int ft260_hid_output_report_check_status(struct ft260_device *dev,
if (ret == 0)
return 0;
- ft260_i2c_abort(hdev, addr);
+ ft260_i2c_abort(dev, addr);
return -EIO;
}
@@ -893,7 +954,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
timeout_jiffies = msecs_to_jiffies(timeout);
if (!wait_for_completion_timeout(&dev->wait, timeout_jiffies)) {
ret = -ETIMEDOUT;
- ft260_i2c_abort(hdev, addr);
+ ft260_i2c_abort(dev, addr);
goto ft260_i2c_read_exit;
}
@@ -907,7 +968,7 @@ static int ft260_i2c_read(struct ft260_device *dev, u8 addr, u8 *data,
ret = ft260_xfer_status(dev, bus_busy);
if (ret < 0) {
ret = -EIO;
- ft260_i2c_abort(hdev, addr);
+ ft260_i2c_abort(dev, addr);
goto ft260_i2c_read_exit;
}
@@ -1091,7 +1152,7 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
hid_warn(hdev,
"smbus block read: invalid count %u from slave 0x%02x\n",
count, addr);
- ft260_i2c_abort(hdev, addr);
+ ft260_i2c_abort(dev, addr);
ret = -EPROTO;
goto smbus_exit;
}
@@ -1264,11 +1325,13 @@ static int ft260_gpio_set(struct gpio_chip *gc, u32 offset, int value)
goto exit;
}
+ mutex_lock(&dev->chip->gpio_lock);
+
ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf,
sizeof(buf));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
- goto exit;
+ goto unlock;
}
rep.report = FT260_GPIO;
@@ -1294,44 +1357,38 @@ static int ft260_gpio_set(struct gpio_chip *gc, u32 offset, int value)
ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, sizeof(rep));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
- goto exit;
+ goto unlock;
}
dev->gpio = rep.gpio;
ret = 0;
+unlock:
+ mutex_unlock(&dev->chip->gpio_lock);
exit:
mutex_unlock(&dev->lock);
return ret;
}
-static int ft260_gpio_direction_set(struct gpio_chip *gc, u32 offset,
- int value, int direction)
+/*
+ * Set the direction and the value of one pin. Takes only
+ * chip->gpio_lock and does not look at gpio_en, so the I2C bus recovery
+ * can drive SCL and SDA while it holds dev->lock and while the pins are
+ * not GPIOs.
+ */
+static int ft260_gpio_pin_set(struct ft260_device *dev, u32 offset,
+ int value, int direction)
{
int ret;
struct ft260_gpio_read_request_report buf;
struct ft260_gpio_write_request_report *rep;
- struct ft260_device *dev = gpiochip_get_data(gc);
struct hid_device *hdev = dev->hdev;
- if (offset >= FT260_GPIO_TOTAL) {
- hid_err(hdev, "%s: invalid offset %d\n", __func__, offset);
- return -EINVAL;
- }
-
- ft260_dbg("offset %d val %d direction %d\n", offset, value, direction);
-
- mutex_lock(&dev->lock);
-
- if (!(dev->gpio_en & BIT(offset))) {
- hid_err(hdev, "%s: wrong pin function %d\n", __func__, offset);
- ret = -EIO;
- goto exit;
- }
+ mutex_lock(&dev->chip->gpio_lock);
ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf, sizeof(buf));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
- goto exit;
+ goto unlock;
}
rep = (struct ft260_gpio_write_request_report *)&buf;
@@ -1365,12 +1422,39 @@ static int ft260_gpio_direction_set(struct gpio_chip *gc, u32 offset,
ret = ft260_hid_feature_report_set(hdev, (u8 *)rep, sizeof(*rep));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
- goto exit;
+ goto unlock;
}
dev->gpio = rep->gpio;
ret = 0;
-exit:
+unlock:
+ mutex_unlock(&dev->chip->gpio_lock);
+ return ret;
+}
+
+static int ft260_gpio_direction_set(struct gpio_chip *gc, u32 offset,
+ int value, int direction)
+{
+ int ret;
+ struct ft260_device *dev = gpiochip_get_data(gc);
+ struct hid_device *hdev = dev->hdev;
+
+ if (offset >= FT260_GPIO_TOTAL) {
+ hid_err(hdev, "%s: invalid offset %d\n", __func__, offset);
+ return -EINVAL;
+ }
+
+ ft260_dbg("offset %d val %d direction %d\n", offset, value, direction);
+
+ mutex_lock(&dev->lock);
+
+ if (!(dev->gpio_en & BIT(offset))) {
+ hid_err(hdev, "%s: wrong pin function %d\n", __func__, offset);
+ ret = -EIO;
+ } else {
+ ret = ft260_gpio_pin_set(dev, offset, value, direction);
+ }
+
mutex_unlock(&dev->lock);
return ret;
}
@@ -1388,11 +1472,10 @@ static int ft260_gpio_direction_input(struct gpio_chip *gc, u32 offset)
FT260_GPIO_DIR_INPUT);
}
-static int ft260_gpio_get_all(struct gpio_chip *gc, int item)
+static int ft260_gpio_read_all(struct ft260_device *dev, int item)
{
int ret;
struct ft260_gpio_read_request_report rep;
- struct ft260_device *dev = gpiochip_get_data(gc);
struct hid_device *hdev = dev->hdev;
ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&rep, sizeof(rep));
@@ -1409,6 +1492,11 @@ static int ft260_gpio_get_all(struct gpio_chip *gc, int item)
return ret;
}
+static int ft260_gpio_get_all(struct gpio_chip *gc, int item)
+{
+ return ft260_gpio_read_all(gpiochip_get_data(gc), item);
+}
+
static int ft260_gpio_get_direction(struct gpio_chip *gc, u32 offset)
{
int ret = ft260_gpio_get_all(gc, FT260_GPIO_DIRECTION);
@@ -1427,6 +1515,56 @@ static int ft260_gpio_get(struct gpio_chip *gc, u32 offset)
return (ret >> offset) & 1;
}
+/*
+ * I2C bus recovery. Turning the I2C controller off gives GPIO0 and
+ * GPIO1 back their GPIO function, so i2c_generic_scl_recovery() can
+ * clock the slave out of the byte it holds and put a STOP on the bus.
+ * A pin is released by making it an input, the bus pull-ups then drive
+ * it high.
+ */
+static void ft260_i2c_set_pin(struct i2c_adapter *adap, u32 offset, int val)
+{
+ struct ft260_device *dev = i2c_get_adapdata(adap);
+
+ ft260_gpio_pin_set(dev, offset, 0,
+ val ? FT260_GPIO_DIR_INPUT : FT260_GPIO_DIR_OUTPUT);
+}
+
+static void ft260_i2c_set_scl(struct i2c_adapter *adap, int val)
+{
+ ft260_i2c_set_pin(adap, FT260_GPIO_SCL_OFFSET, val);
+}
+
+static void ft260_i2c_set_sda(struct i2c_adapter *adap, int val)
+{
+ ft260_i2c_set_pin(adap, FT260_GPIO_SDA_OFFSET, val);
+}
+
+static int ft260_i2c_get_scl(struct i2c_adapter *adap)
+{
+ struct ft260_device *dev = i2c_get_adapdata(adap);
+ int ret = ft260_gpio_read_all(dev, FT260_GPIO_VALUE);
+
+ if (ret < 0)
+ return ret;
+
+ return (ret >> FT260_GPIO_SCL_OFFSET) & 1;
+}
+
+static void ft260_i2c_prepare_recovery(struct i2c_adapter *adap)
+{
+ struct ft260_device *dev = i2c_get_adapdata(adap);
+
+ ft260_i2c_set_mode(dev->hdev, 0);
+}
+
+static void ft260_i2c_unprepare_recovery(struct i2c_adapter *adap)
+{
+ struct ft260_device *dev = i2c_get_adapdata(adap);
+
+ ft260_i2c_set_mode(dev->hdev, 1);
+}
+
static int ft260_gpio_init(struct ft260_device *dev,
struct ft260_get_system_status_report *cfg)
{
@@ -2158,6 +2296,7 @@ static int ft260_uart_tiocmset(struct tty_struct *tty,
{
int ret;
struct ft260_device *port = tty->driver_data;
+ struct ft260_gpio_read_request_report buf;
struct ft260_gpio_write_request_report rep;
struct hid_device *hdev = port->hdev;
int mask = FT260_GPIO_UART_MODE_4_SET;
@@ -2173,8 +2312,19 @@ static int ft260_uart_tiocmset(struct tty_struct *tty,
mutex_unlock(&port->lock);
return -EINVAL;
}
+
+ mutex_lock(&port->chip->gpio_lock);
+
+ ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf,
+ sizeof(buf));
+ if (unlikely(ret < 0)) {
+ hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
+ ret = -EIO;
+ goto unlock;
+ }
+
rep.report = FT260_GPIO;
- rep.gpio = port->gpio;
+ rep.gpio = buf.gpio;
if (set & TIOCM_RTS)
ft260_gpio_output_cfg(&rep.gpio, FT260_GPIO_UART_RTS, 0);
@@ -2192,14 +2342,16 @@ static int ft260_uart_tiocmset(struct tty_struct *tty,
ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, sizeof(rep));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
- mutex_unlock(&port->lock);
- return -EIO;
+ ret = -EIO;
+ goto unlock;
}
port->gpio = rep.gpio;
+ ret = 0;
+unlock:
+ mutex_unlock(&port->chip->gpio_lock);
mutex_unlock(&port->lock);
-
- return 0;
+ return ret;
}
static int ft260_uart_install(struct tty_driver *driver, struct tty_struct *tty)
@@ -2414,6 +2566,13 @@ static int ft260_i2c_probe(struct ft260_device *dev,
dev->adap.class = I2C_CLASS_HWMON;
dev->adap.algo = &ft260_i2c_algo;
dev->adap.quirks = &ft260_i2c_quirks;
+ dev->rinfo.recover_bus = i2c_generic_scl_recovery;
+ dev->rinfo.set_scl = ft260_i2c_set_scl;
+ dev->rinfo.get_scl = ft260_i2c_get_scl;
+ dev->rinfo.set_sda = ft260_i2c_set_sda;
+ dev->rinfo.prepare_recovery = ft260_i2c_prepare_recovery;
+ dev->rinfo.unprepare_recovery = ft260_i2c_unprepare_recovery;
+ dev->adap.bus_recovery_info = &dev->rinfo;
dev->adap.dev.parent = &hdev->dev;
snprintf(dev->adap.name, sizeof(dev->adap.name),
"FT260 usb-i2c bridge");
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v3 16/16] HID: ft260: use lock guards
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
` (14 preceding siblings ...)
2026-11-14 23:07 ` [PATCH v3 15/16] HID: ft260: i2c: recover a bus held by a slave Michael Zaidman
@ 2026-11-14 23:08 ` Michael Zaidman
2026-11-14 23:21 ` sashiko-bot
15 siblings, 1 reply; 20+ messages in thread
From: Michael Zaidman @ 2026-11-14 23:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linus Walleij, Bartosz Golaszewski, Greg Kroah-Hartman,
Jiri Slaby, Raman Varabets, Germain Hebert, Christina Quast,
Rio Liu, Bruno Giacomazzi, linux-input, linux-serial, linux-gpio,
linux-i2c, linux-kernel, Michael Zaidman
Convert the mutex_lock() and mutex_unlock() pairs to guard(mutex) from
linux/cleanup.h. The lock is released on each return, so the goto
labels that only unlock a mutex are removed, and the error paths
return directly.
ft260_uart_port_remove(), ft260_remove() and ft260_schedule_reset()
take a lock for one or two statements in the middle of the function.
They use scoped_guard(), which releases the lock at the end of the
block. The two remove functions do not hold the lock over the tty
hangup and the rest of the removal. ft260_schedule_reset() must not
hold it when it calls ft260_chip_put(), which takes the same lock.
The i2c_exit and smbus_exit labels stay, because they also set the
normal power mode with hid_hw_power().
No functional change.
Suggested-by: Bartosz Golaszewski <brgl@kernel.org>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
v3:
New patch (suggested by Bartosz Golaszewski). It is the last patch,
so the out-of-tree driver for kernels older than 6.5, which do not
have linux/cleanup.h, can leave it out.
---
drivers/hid/hid-ft260.c | 119 +++++++++++++---------------------------
1 file changed, 39 insertions(+), 80 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 53321c8d70fd..716b37cead7d 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -532,11 +532,11 @@ static struct ft260_chip *ft260_chip_get(struct usb_device *udev)
{
struct ft260_chip *chip;
- mutex_lock(&ft260_chip_list_lock);
+ guard(mutex)(&ft260_chip_list_lock);
list_for_each_entry(chip, &ft260_chip_list, list) {
if (chip->udev == udev) {
kref_get(&chip->kref);
- goto out;
+ return chip;
}
}
@@ -547,8 +547,6 @@ static struct ft260_chip *ft260_chip_get(struct usb_device *udev)
mutex_init(&chip->gpio_lock);
list_add_tail(&chip->list, &ft260_chip_list);
}
-out:
- mutex_unlock(&ft260_chip_list_lock);
return chip;
}
@@ -567,9 +565,8 @@ static void ft260_chip_put(struct ft260_chip *chip)
if (!chip)
return;
- mutex_lock(&ft260_chip_list_lock);
+ guard(mutex)(&ft260_chip_list_lock);
kref_put(&chip->kref, ft260_chip_release);
- mutex_unlock(&ft260_chip_list_lock);
}
static int ft260_hid_feature_report_get(struct hid_device *hdev,
@@ -1034,12 +1031,11 @@ static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
struct ft260_device *dev = i2c_get_adapdata(adapter);
struct hid_device *hdev = dev->hdev;
- mutex_lock(&dev->lock);
+ guard(mutex)(&dev->lock);
ret = hid_hw_power(hdev, PM_HINT_FULLON);
if (ret < 0) {
hid_err(hdev, "failed to enter FULLON power mode: %d\n", ret);
- mutex_unlock(&dev->lock);
return ret;
}
@@ -1063,7 +1059,6 @@ static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
ret = num;
i2c_exit:
hid_hw_power(hdev, PM_HINT_NORMAL);
- mutex_unlock(&dev->lock);
return ret;
}
@@ -1077,12 +1072,11 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
ft260_dbg("smbus size %d\n", size);
- mutex_lock(&dev->lock);
+ guard(mutex)(&dev->lock);
ret = hid_hw_power(hdev, PM_HINT_FULLON);
if (ret < 0) {
hid_err(hdev, "power management error: %d\n", ret);
- mutex_unlock(&dev->lock);
return ret;
}
@@ -1190,7 +1184,6 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
smbus_exit:
hid_hw_power(hdev, PM_HINT_NORMAL);
- mutex_unlock(&dev->lock);
return ret;
}
@@ -1317,21 +1310,20 @@ static int ft260_gpio_set(struct gpio_chip *gc, u32 offset, int value)
ft260_dbg("offset %d val %d\n", offset, value);
- mutex_lock(&dev->lock);
+ guard(mutex)(&dev->lock);
if (!(dev->gpio_en & BIT(offset))) {
hid_err(hdev, "%s: wrong pin function %d\n", __func__, offset);
- ret = -EINVAL;
- goto exit;
+ return -EINVAL;
}
- mutex_lock(&dev->chip->gpio_lock);
+ guard(mutex)(&dev->chip->gpio_lock);
ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf,
sizeof(buf));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
- goto unlock;
+ return ret;
}
rep.report = FT260_GPIO;
@@ -1357,16 +1349,11 @@ static int ft260_gpio_set(struct gpio_chip *gc, u32 offset, int value)
ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, sizeof(rep));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
- goto unlock;
+ return ret;
}
dev->gpio = rep.gpio;
- ret = 0;
-unlock:
- mutex_unlock(&dev->chip->gpio_lock);
-exit:
- mutex_unlock(&dev->lock);
- return ret;
+ return 0;
}
/*
@@ -1383,12 +1370,12 @@ static int ft260_gpio_pin_set(struct ft260_device *dev, u32 offset,
struct ft260_gpio_write_request_report *rep;
struct hid_device *hdev = dev->hdev;
- mutex_lock(&dev->chip->gpio_lock);
+ guard(mutex)(&dev->chip->gpio_lock);
ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf, sizeof(buf));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
- goto unlock;
+ return ret;
}
rep = (struct ft260_gpio_write_request_report *)&buf;
@@ -1422,20 +1409,16 @@ static int ft260_gpio_pin_set(struct ft260_device *dev, u32 offset,
ret = ft260_hid_feature_report_set(hdev, (u8 *)rep, sizeof(*rep));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
- goto unlock;
+ return ret;
}
dev->gpio = rep->gpio;
- ret = 0;
-unlock:
- mutex_unlock(&dev->chip->gpio_lock);
- return ret;
+ return 0;
}
static int ft260_gpio_direction_set(struct gpio_chip *gc, u32 offset,
int value, int direction)
{
- int ret;
struct ft260_device *dev = gpiochip_get_data(gc);
struct hid_device *hdev = dev->hdev;
@@ -1446,17 +1429,14 @@ static int ft260_gpio_direction_set(struct gpio_chip *gc, u32 offset,
ft260_dbg("offset %d val %d direction %d\n", offset, value, direction);
- mutex_lock(&dev->lock);
+ guard(mutex)(&dev->lock);
if (!(dev->gpio_en & BIT(offset))) {
hid_err(hdev, "%s: wrong pin function %d\n", __func__, offset);
- ret = -EIO;
- } else {
- ret = ft260_gpio_pin_set(dev, offset, value, direction);
+ return -EIO;
}
- mutex_unlock(&dev->lock);
- return ret;
+ return ft260_gpio_pin_set(dev, offset, value, direction);
}
static int ft260_gpio_direction_output(struct gpio_chip *gc,
@@ -1791,7 +1771,7 @@ static int ft260_i2c_clock_parse(const char *buf, unsigned int base, u16 *clock)
rep.name = conv(name); \
rep.report = id; \
rep.request = req; \
- mutex_lock(&dev->lock); \
+ guard(mutex)(&dev->lock); \
ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, \
sizeof(rep)); \
if (ret < 0) \
@@ -1800,7 +1780,6 @@ static int ft260_i2c_clock_parse(const char *buf, unsigned int base, u16 *clock)
func(hdev, req, name); \
ret = count; \
} \
- mutex_unlock(&dev->lock); \
} else { \
ret = -EINVAL; \
} \
@@ -1975,7 +1954,7 @@ static int ft260_uart_add_port(struct ft260_device *port)
if (kfifo_alloc(&port->xmit_fifo, XMIT_FIFO_SIZE, GFP_KERNEL))
return -ENOMEM;
- mutex_lock(&ft260_uart_list_lock);
+ guard(mutex)(&ft260_uart_list_lock);
list_for_each_entry(dev, &ft260_uart_device_list, device_list) {
if (dev->index != index)
break;
@@ -1985,7 +1964,6 @@ static int ft260_uart_add_port(struct ft260_device *port)
port->index = index;
/* Before the first gap, or at the end: the list stays sorted */
list_add_tail(&port->device_list, &dev->device_list);
- mutex_unlock(&ft260_uart_list_lock);
return ret;
}
@@ -2004,9 +1982,8 @@ static void ft260_uart_port_remove(struct ft260_device *port)
timer_shutdown_sync(&port->wakeup_timer);
cancel_work_sync(&port->wakeup_work);
- mutex_lock(&ft260_uart_list_lock);
- list_del(&port->device_list);
- mutex_unlock(&ft260_uart_list_lock);
+ scoped_guard(mutex, &ft260_uart_list_lock)
+ list_del(&port->device_list);
tty_port_tty_vhangup(&port->port);
@@ -2025,11 +2002,10 @@ static struct ft260_device *ft260_uart_port_get(int index)
if (index >= UART_COUNT_MAX)
return NULL;
- mutex_lock(&ft260_uart_list_lock);
+ guard(mutex)(&ft260_uart_list_lock);
port = ft260_dev_by_index(index);
if (port)
tty_port_get(&port->port);
- mutex_unlock(&ft260_uart_list_lock);
return port;
}
@@ -2224,7 +2200,7 @@ static int ft260_uart_change_speed(struct ft260_device *port,
req.data_bit, req.parity,
req.stop_bit, req.breaking);
- mutex_lock(&port->lock);
+ guard(mutex)(&port->lock);
ret = ft260_hid_feature_report_set(hdev, (u8 *)&req, sizeof(req));
if (ret < 0)
@@ -2232,8 +2208,6 @@ static int ft260_uart_change_speed(struct ft260_device *port,
else
ft260_gpio_en_update(hdev, FT260_SET_UART_MODE, req.flow_ctrl);
- mutex_unlock(&port->lock);
-
return ret;
}
@@ -2266,20 +2240,16 @@ static int ft260_uart_tiocmget(struct tty_struct *tty)
*/
int mask = FT260_GPIO_UART_MODE_4_SET;
- mutex_lock(&port->lock);
- if (port->removed) {
- mutex_unlock(&port->lock);
+ guard(mutex)(&port->lock);
+ if (port->removed)
return -ENODEV;
- }
if ((port->gpio_en & mask) != mask) {
ft260_dbg("Illegal gpio config %#02x != mask %#04x\n",
port->gpio_en, mask);
- mutex_unlock(&port->lock);
return -EINVAL;
}
gpios = ft260_gpio_get_all(&port->gc, FT260_GPIO_VALUE);
- mutex_unlock(&port->lock);
if (gpios < 0)
return gpios;
@@ -2301,26 +2271,22 @@ static int ft260_uart_tiocmset(struct tty_struct *tty,
struct hid_device *hdev = port->hdev;
int mask = FT260_GPIO_UART_MODE_4_SET;
- mutex_lock(&port->lock);
- if (port->removed) {
- mutex_unlock(&port->lock);
+ guard(mutex)(&port->lock);
+ if (port->removed)
return -ENODEV;
- }
if ((port->gpio_en & mask) != mask) {
ft260_dbg("Illegal gpio config %#02x != mask %#04x\n",
port->gpio_en, mask);
- mutex_unlock(&port->lock);
return -EINVAL;
}
- mutex_lock(&port->chip->gpio_lock);
+ guard(mutex)(&port->chip->gpio_lock);
ret = ft260_hid_feature_report_get(hdev, FT260_GPIO, (u8 *)&buf,
sizeof(buf));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot get GPIO: %d\n", __func__, ret);
- ret = -EIO;
- goto unlock;
+ return -EIO;
}
rep.report = FT260_GPIO;
@@ -2342,16 +2308,11 @@ static int ft260_uart_tiocmset(struct tty_struct *tty,
ret = ft260_hid_feature_report_set(hdev, (u8 *)&rep, sizeof(rep));
if (unlikely(ret < 0)) {
hid_err(hdev, "%s: cannot set GPIO: %d\n", __func__, ret);
- ret = -EIO;
- goto unlock;
+ return -EIO;
}
port->gpio = rep.gpio;
- ret = 0;
-unlock:
- mutex_unlock(&port->chip->gpio_lock);
- mutex_unlock(&port->lock);
- return ret;
+ return 0;
}
static int ft260_uart_install(struct tty_driver *driver, struct tty_struct *tty)
@@ -2733,9 +2694,8 @@ static struct workqueue_struct *ft260_reset_wq;
static void ft260_chip_reset_done(struct ft260_chip *chip)
{
- mutex_lock(&ft260_chip_list_lock);
+ guard(mutex)(&ft260_chip_list_lock);
chip->reset_pending = false;
- mutex_unlock(&ft260_chip_list_lock);
}
static void ft260_reset_and_rebind(struct work_struct *ws)
@@ -2845,10 +2805,10 @@ static int ft260_schedule_reset(struct usb_interface *usbif)
return -ENOMEM;
}
- mutex_lock(&ft260_chip_list_lock);
- pending = chip->reset_pending;
- chip->reset_pending = true;
- mutex_unlock(&ft260_chip_list_lock);
+ scoped_guard(mutex, &ft260_chip_list_lock) {
+ pending = chip->reset_pending;
+ chip->reset_pending = true;
+ }
if (pending) {
ft260_chip_put(chip);
kfree(rw);
@@ -3092,9 +3052,8 @@ static void ft260_remove(struct hid_device *hdev)
* The hangup does not wait for a TIOCMGET or TIOCMSET
* that is running; they hold the lock while they use hdev.
*/
- mutex_lock(&dev->lock);
- dev->removed = true;
- mutex_unlock(&dev->lock);
+ scoped_guard(mutex, &dev->lock)
+ dev->removed = true;
if (dev->chip_mode & FT260_MODE_UART ||
dev->chip_mode == FT260_MODE_ALL ||
dev->chip_mode == FT260_MODE_BOTH)
--
2.43.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 16/16] HID: ft260: use lock guards
2026-11-14 23:08 ` [PATCH v3 16/16] HID: ft260: use lock guards Michael Zaidman
@ 2026-11-14 23:21 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-11-14 23:21 UTC (permalink / raw)
To: Michael Zaidman; +Cc: linux-input, linux-serial, linux-gpio, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] HID: ft260: mixing guard(mutex) with goto cleanup in xfer
functions
--
commit 27addf69822143f9d183be5b9b500ff0df6b356b
Author: Michael Zaidman <michael.zaidman@gmail.com>
HID: ft260: use lock guards
This commit converts mutex_lock() and mutex_unlock() pairs in hid-ft260 to
guard(mutex) and scoped_guard(mutex, ...).
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 53321c8d70fd0..716b37cead7dc 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
[ ... ]
> @@ -1034,12 +1031,11 @@ static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
> struct ft260_device *dev = i2c_get_adapdata(adapter);
> struct hid_device *hdev = dev->hdev;
>
> - mutex_lock(&dev->lock);
> + guard(mutex)(&dev->lock);
>
> ret = hid_hw_power(hdev, PM_HINT_FULLON);
> if (ret < 0) {
> hid_err(hdev, "failed to enter FULLON power mode: %d\n", ret);
> - mutex_unlock(&dev->lock);
> return ret;
> }
>
> @@ -1063,7 +1059,6 @@ static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
> ret = num;
> i2c_exit:
> hid_hw_power(hdev, PM_HINT_NORMAL);
> - mutex_unlock(&dev->lock);
> return ret;
> }
>
> @@ -1077,12 +1072,11 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
>
> ft260_dbg("smbus size %d\n", size);
>
> - mutex_lock(&dev->lock);
> + guard(mutex)(&dev->lock);
>
> ret = hid_hw_power(hdev, PM_HINT_FULLON);
> if (ret < 0) {
> hid_err(hdev, "power management error: %d\n", ret);
> - mutex_unlock(&dev->lock);
> return ret;
> }
>
> @@ -1190,7 +1184,6 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
>
> smbus_exit:
> hid_hw_power(hdev, PM_HINT_NORMAL);
> - mutex_unlock(&dev->lock);
> return ret;
> }
[Severity: Low]
This isn't a bug, but ft260_i2c_xfer() and ft260_smbus_xfer() now mix
guard(mutex) with goto-based cleanup at the i2c_exit and smbus_exit labels,
contrary to the cleanup.h convention of not mixing scope-based cleanup with
goto unwinding in the same function.
In ft260_i2c_xfer(), the i2c_exit label can be dropped by calling
hid_hw_power(hdev, PM_HINT_NORMAL) after the if/else block and returning
ret < 0 ? ret : num, while ft260_smbus_xfer() could either factor the
transfer switch into a helper or keep explicit mutex_lock() and
mutex_unlock() alongside smbus_exit.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261114230800.1344401-1-michael.zaidman@gmail.com?part=16
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 02/16] HID: ft260: uart: fix the serial driver
2026-11-14 23:07 ` [PATCH v3 02/16] HID: ft260: uart: fix the " Michael Zaidman
@ 2026-11-14 23:37 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-11-14 23:37 UTC (permalink / raw)
To: Michael Zaidman; +Cc: linux-input, linux-serial, linux-gpio, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [High] HID: ft260: undrained xmit_fifo on TX error or remove hangs tty
drain and vhangup
- [High] HID: ft260: race between tty open/reopen and ft260_remove()
causes hdev UAF
- [High] HID: ft260: premature list_del() in ft260_uart_port_remove()
causes index reuse
- [Medium] HID: ft260: missing UART_COUNT_MAX check in
ft260_uart_add_port()
--
commit 4759b9dbc3d5fe28cb8dc19ab0c19093471e2b92
Author: Michael Zaidman <michael.zaidman@gmail.com>
HID: ft260: uart: fix the serial driver
This commit fixes buffer sharing, power-save wakeup handling, input report
validation, TX work queueing, and port lifecycle management in the FT260
UART driver.
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 982e2ca7a2de6..8d72a22d7a079 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
[ ... ]
> @@ -1123,8 +1161,8 @@ static int ft260_uart_add_port(struct ft260_device *port)
> int index = 0, ret = 0;
> struct ft260_device *dev;
>
> - spin_lock_init(&port->write_lock);
> - if (kfifo_alloc(&port->xmit_fifo, FIFO_SIZE, GFP_KERNEL))
> + spin_lock_init(&port->xmit_fifo_lock);
> + if (kfifo_alloc(&port->xmit_fifo, XMIT_FIFO_SIZE, GFP_KERNEL))
> return -ENOMEM;
>
> mutex_lock(&ft260_uart_list_lock);
> @@ -1135,7 +1173,8 @@ static int ft260_uart_add_port(struct ft260_device *port)
> }
>
> port->index = index;
> - list_add_tail(&port->device_list, &ft260_uart_device_list);
> + /* Before the first gap, or at the end: the list stays sorted */
> + list_add_tail(&port->device_list, &dev->device_list);
> mutex_unlock(&ft260_uart_list_lock);
>
> return ret;
[Severity: Medium]
Can ft260_uart_add_port() assign port->index >= UART_COUNT_MAX when more
than four FT260 UART interfaces are probed?
ft260_tty_driver is allocated with UART_COUNT_MAX (4) ports, but
ft260_uart_add_port() never checks index against UART_COUNT_MAX before
adding the port and returning 0. ft260_uart_probe() then passes
dev->index to tty_port_register_device_attr() -> tty_port_link_device(),
which hits WARN_ON(index >= driver->num).
> @@ -1148,7 +1187,12 @@ static void ft260_uart_port_put(struct ft260_device *port)
>
> static void ft260_uart_port_remove(struct ft260_device *port)
> {
> - timer_delete_sync(&port->wakeup_timer);
> + /*
> + * Shut down, not delete: a tty call before the hangup below
> + * must not re-arm the timer.
> + */
> + timer_shutdown_sync(&port->wakeup_timer);
> + cancel_work_sync(&port->wakeup_work);
>
> mutex_lock(&ft260_uart_list_lock);
> list_del(&port->device_list);
> mutex_unlock(&ft260_uart_list_lock);
[Severity: High]
Does removing port->device_list in ft260_uart_port_remove() allow a newly
probed FT260 to reuse port->index while an open file descriptor still
keeps the old tty_struct in ft260_tty_driver->ttys[index]?
Until the last file descriptor is closed and release_tty() runs,
ft260_tty_driver->ttys[index] still points to the old hung-up tty_struct.
If the device is re-plugged before that close, ft260_uart_add_port() sees
the index as free and assigns it to the new device. Opening the new
/dev/ttyFTx then finds the old tty_struct via tty_driver_lookup_tty() and
calls tty_reopen() -> ft260_uart_open() -> ft260_uart_port_activate() on
the old removed ft260_device, dereferencing its stopped and freed
port->hdev.
Moving list_del(&port->device_list) to ft260_uart_port_destroy() would keep
the index reserved until ft260_tty_driver->ttys[index] has been cleared.
[ ... ]
> @@ -1207,61 +1254,51 @@ static int ft260_uart_transmit_chars(struct ft260_device *port)
> struct kfifo *xmit = &port->xmit_fifo;
> struct tty_struct *tty;
> struct ft260_uart_write_request_report *rep;
> - int len, data_len, ret = 0;
> + int len, ret = 0;
>
> tty = tty_port_tty_get(&port->port);
> + if (!tty)
> + return -EINVAL;
>
> - data_len = kfifo_len(xmit);
> - if (!tty || !data_len) {
> - ret = -EINVAL;
> - goto tty_out;
> - }
> -
> - rep = (struct ft260_uart_write_request_report *)port->write_buf;
> -
> - do {
> - len = min(data_len, FT260_WR_DATA_MAX);
> + rep = (struct ft260_uart_write_request_report *)port->uart_wr_buf;
>
> + while ((len = kfifo_out_spinlocked(xmit, rep->data,
> + FT260_WR_UART_DATA_MAX,
> + &port->xmit_fifo_lock)) > 0) {
> rep->report = FT260_UART_DATA_REPORT_ID(len);
> rep->length = len;
>
> - len = kfifo_out_locked(xmit, rep->data, len, &port->write_lock);
> -
> - ret = ft260_hid_output_report(hdev, (u8 *)rep, len + sizeof(*rep));
> - if (ret < 0) {
> - hid_err(hdev, "Failed to start transfer, ret %d\n", ret);
> - goto tty_out;
> - }
> + ret = ft260_hid_output_report(hdev, (u8 *)rep, len + 2);
> + if (ret < 0)
> + break;
>
> - data_len -= len;
> port->icount.tx += len;
> - } while (data_len > 0);
> -
> - len = kfifo_len(xmit);
> - if ((FIFO_SIZE - len) > TTY_WAKEUP_WATERMARK)
> - tty_wakeup(tty);
> -
> - ret = 0;
> + }
>
> -tty_out:
> + tty_wakeup(tty);
> tty_kref_put(tty);
> - return ret;
> + return ret < 0 ? ret : 0;
> }
[Severity: High]
If ft260_hid_output_report() fails when more than 60 bytes are queued in
xmit_fifo, does breaking out of this loop leave xmit_fifo non-empty with
nothing to drain or flush it?
ft260_uart_chars_in_buffer() then continues returning a non-zero
kfifo_len(&port->xmit_fifo) forever, and ft260_uart_ops has no
.flush_buffer callback. As a result, tcdrain() and
set_termios(TCSADRAIN/TCSAFLUSH) sleep indefinitely in
wait_event_interruptible(tty->write_wait, !tty_chars_in_buffer(tty)), and
if the device is unplugged while set_termios() is waiting with an ldisc
reference held, ft260_uart_port_remove() -> tty_port_tty_vhangup() ->
tty_ldisc_hangup() blocks forever in tty_ldisc_lock() before reaching
kfifo_free(&port->xmit_fifo):
CPU0 CPU1
tty_ioctl() [TCSETSW]
ld = tty_ldisc_ref_wait(tty)
...
set_termios()
wait_event_interruptible(
tty->write_wait,
!tty_chars_in_buffer(tty))
ft260_uart_port_remove()
tty_port_tty_vhangup(&port->port)
...
tty_ldisc_hangup()
tty_driver_flush_buffer(tty)
wake_up_interruptible_poll(
&tty->write_wait, EPOLLOUT)
/* xmit_fifo still non-empty,
goes back to sleep */
tty_ldisc_lock(tty,
MAX_SCHEDULE_TIMEOUT)
<-- hangs forever
[ ... ]
> @@ -1503,53 +1548,69 @@ static void ft260_uart_start_wakeup(struct timer_list *t)
[ ... ]
> -static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
> +static int ft260_uart_port_activate(struct tty_port *tport, struct tty_struct *tty)
> {
> + int ret;
> + int baudrate;
> + struct ft260_get_uart_settings_report cfg;
> + struct ft260_device *port = container_of(tport, struct ft260_device, port);
> +
> set_bit(TTY_IO_ERROR, &tty->flags);
>
> - spin_lock(&port->write_lock);
> + spin_lock(&port->xmit_fifo_lock);
> kfifo_reset(&port->xmit_fifo);
> - spin_unlock(&port->write_lock);
> + spin_unlock(&port->xmit_fifo_lock);
>
> - ft260_uart_change_speed(port, &tty->termios, NULL);
> clear_bit(TTY_IO_ERROR, &tty->flags);
>
> - if (port->reschedule_work) {
> + /*
> + * The port setting may remain intact after session termination.
> + * Then, when reopening the port without configuring the port
> + * setting, we need to retrieve the baud rate from the device to
> + * reactivate the wakeup workaround if needed.
> + */
> + ret = ft260_get_uart_settings(port->hdev, &cfg);
[Severity: High]
Can ft260_uart_port_activate() race with ft260_remove() and dereference
port->hdev after it has been stopped and freed?
CPU0 CPU1
tty_open()
tty_open_by_driver()
tty_init_dev()
ft260_uart_install()
ft260_uart_port_get()
mutex_unlock(&tty_mutex)
ft260_remove()
...
ft260_uart_port_remove()
list_del(&port->device_list)
tty_port_tty_vhangup(&port->port)
/* port.tty is still NULL */
kfifo_free(&port->xmit_fifo)
hid_hw_close(hdev)
hid_hw_stop(hdev)
/* hdev freed by HID/USB core */
ft260_uart_open()
tty_port_open()
ft260_uart_port_activate()
ft260_get_uart_settings(port->hdev, ...)
<-- use-after-free on port->hdev
The same race also happens on reopen when a second opener waits on
tty_lock_interruptible(tty) in tty_open_by_driver() while
tty_port_tty_vhangup() runs: vhangup clears the tty_port's initialized
flag, and tty_reopen() then proceeds to ft260_uart_open() ->
tty_port_open() -> ft260_uart_port_activate() after ft260_remove() has
finished.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261114230800.1344401-1-michael.zaidman@gmail.com?part=2
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 01/16] HID: ft260: add serial driver
2026-11-14 23:07 ` [PATCH v3 01/16] HID: ft260: add serial driver Michael Zaidman
@ 2026-11-14 23:39 ` sashiko-bot
0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-11-14 23:39 UTC (permalink / raw)
To: Michael Zaidman; +Cc: linux-input, linux-serial, linux-gpio, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 3 potential
issues to consider:
- [High] HID: ft260: UAF on port->hdev and premature xmit_fifo free on
disconnect
- [High] HID: ft260: use-after-free in ft260_uart_activate() on remove
race
- [Medium] HID: ft260: missing UART_COUNT_MAX check in
ft260_uart_add_port()
--
commit 0485244a5cc12377d97264627f49ff95e6a060c7
Author: Christina Quast <contact@christina-quast.de>
HID: ft260: add serial driver
This commit adds a TTY serial driver (/dev/ttyFTx) for the UART interface
of FT260 USB HID devices.
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 64c9c940db46e..982e2ca7a2de6 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
[ ... ]
> @@ -1001,51 +1101,478 @@ static const struct attribute_group ft260_attr_group = {
> }
> };
>
> +static int ft260_uart_add_port(struct ft260_device *port)
> {
> + int index = 0, ret = 0;
> struct ft260_device *dev;
> +
> + spin_lock_init(&port->write_lock);
> + if (kfifo_alloc(&port->xmit_fifo, FIFO_SIZE, GFP_KERNEL))
> + return -ENOMEM;
> +
> + mutex_lock(&ft260_uart_list_lock);
> + list_for_each_entry(dev, &ft260_uart_device_list, device_list) {
> + if (dev->index != index)
> + break;
> + index++;
> + }
> +
> + port->index = index;
> + list_add_tail(&port->device_list, &ft260_uart_device_list);
> + mutex_unlock(&ft260_uart_list_lock);
> +
> + return ret;
> +}
[Severity: Medium]
Does ft260_uart_add_port() need to check if index >= UART_COUNT_MAX before
adding the port? When a fifth FT260 UART interface is probed,
ft260_uart_probe() passes dev->index == 4 to
tty_port_register_device_attr(), which triggers a WARN_ON(index >=
driver->num) in tty_port_link_device() because ft260_tty_driver was
allocated with UART_COUNT_MAX (4) lines.
While commit "HID: ft260: uart: fix the serial driver" fixes the unsorted
list insertion here, the missing UART_COUNT_MAX bounds check remains at
the end of the series.
> +
> +static void ft260_uart_port_put(struct ft260_device *port)
> +{
> + tty_port_put(&port->port);
> +}
> +
> +static void ft260_uart_port_remove(struct ft260_device *port)
> +{
> + timer_delete_sync(&port->wakeup_timer);
> +
> + mutex_lock(&ft260_uart_list_lock);
> + list_del(&port->device_list);
> + mutex_unlock(&ft260_uart_list_lock);
> +
> + tty_port_tty_vhangup(&port->port);
> +
> + spin_lock(&port->write_lock);
> + kfifo_free(&port->xmit_fifo);
> + spin_unlock(&port->write_lock);
> +}
[Severity: High]
Can freeing port->xmit_fifo in ft260_uart_port_remove() and leaving
port->hdev accessible after ft260_remove() cause a use-after-free?
struct ft260_device stays alive until the final tty_port reference is
dropped in ft260_uart_cleanup() -> ft260_uart_port_destroy(), whereas
ft260_remove() calls ft260_uart_port_remove(), hid_hw_close(hdev), and
hid_hw_stop(hdev), after which usbhid_disconnect() -> hid_destroy_device()
frees hdev. If a process keeps an open file descriptor across
ft260_remove() and reopens it (for example via /proc/self/fd/<fd>),
tty_open_by_driver() takes the tty_reopen() path into tty_port_open() ->
ft260_uart_activate(), which resets the already freed port->xmit_fifo and
calls ft260_uart_change_speed() -> ft260_hid_feature_report_set() on the
freed port->hdev. Because ft260_uart_activate() also ignores the return
value of ft260_uart_change_speed() and clears TTY_IO_ERROR, the reopen
succeeds and subsequent ft260_uart_write() and ft260_uart_set_termios()
calls operate on the freed FIFO and freed hdev as well.
Commit "HID: ft260: uart: fix the serial driver" and commit "HID: ft260:
uart: add modem pins control via ioctl" add a port->removed flag for the
modem control ioctls, but ft260_uart_port_activate() and
ft260_uart_change_speed() still do not check port->removed and
port->xmit_fifo is still freed in ft260_uart_port_remove() rather than
ft260_uart_port_destroy().
[ ... ]
> +static int ft260_uart_install(struct tty_driver *driver, struct tty_struct *tty)
> +{
> + int idx = tty->index;
> + struct ft260_device *port = ft260_uart_port_get(idx);
> + int ret;
> +
> + if (!port)
> + return -ENODEV;
> +
> + ret = tty_standard_install(driver, tty);
> if (ret) {
> + ft260_uart_port_put(port);
> return ret;
> }
>
> + tty->driver_data = port;
> + return 0;
> +}
[ ... ]
> +static int ft260_uart_activate(struct tty_port *tport, struct tty_struct *tty)
> +{
> + struct ft260_device *port =
> + container_of(tport, struct ft260_device, port);
> +
> + /*
> + * Set the TTY IO error marker - we will only clear this
> + * once we have successfully opened the port.
> + */
> + set_bit(TTY_IO_ERROR, &tty->flags);
> +
> + spin_lock(&port->write_lock);
> + kfifo_reset(&port->xmit_fifo);
> + spin_unlock(&port->write_lock);
> +
> + ft260_uart_change_speed(port, &tty->termios, NULL);
> + clear_bit(TTY_IO_ERROR, &tty->flags);
> +
> + if (port->reschedule_work) {
> + mod_timer(&port->wakeup_timer, jiffies +
> + msecs_to_jiffies(FT260_WAKEUP_NEEDED_AFTER_MS));
> + }
> +
> + return 0;
> +}
[Severity: High]
What prevents ft260_uart_activate() from running after ft260_remove() when
tty_open() races with disconnect?
CPU0 CPU1
tty_open()
tty_init_dev()
ft260_uart_install()
ft260_uart_port_get()
tty->driver_data = port
usbhid_disconnect()
hid_destroy_device()
ft260_remove()
ft260_uart_port_remove()
list_del(&port->device_list)
tty_port_tty_vhangup(&port->port)
kfifo_free(&port->xmit_fifo)
hid_hw_close(hdev)
hid_hw_stop(hdev)
kfree(hdev)
ft260_uart_open()
tty_port_open()
tty_port_tty_set()
ft260_uart_activate()
kfifo_reset(&port->xmit_fifo)
ft260_uart_change_speed()
ft260_hid_feature_report_set(port->hdev, ...)
<-- use-after-free on port->hdev
Between ft260_uart_install() and tty_port_open(), port->port.tty is still
NULL, so tty_port_tty_vhangup() in ft260_uart_port_remove() sees no tty to
hang up. tty_open() then enters ft260_uart_activate(), resets the freed
port->xmit_fifo, dereferences the freed port->hdev, and clears
TTY_IO_ERROR so the open succeeds on the removed device. Even after commit
"HID: ft260: uart: fix the serial driver" and commit "HID: ft260: uart:
add modem pins control via ioctl", ft260_uart_port_activate() still does
not check whether the device was removed under a lock synchronized with
disconnect.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261114230800.1344401-1-michael.zaidman@gmail.com?part=1
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2026-11-14 23:39 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-14 23:07 [PATCH v3 00/16] HID: ft260: add UART and GPIO support, plus I2C fixes Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 01/16] HID: ft260: add serial driver Michael Zaidman
2026-11-14 23:39 ` sashiko-bot
2026-11-14 23:07 ` [PATCH v3 02/16] HID: ft260: uart: fix the " Michael Zaidman
2026-11-14 23:37 ` sashiko-bot
2026-11-14 23:07 ` [PATCH v3 03/16] HID: ft260: add GPIO support Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 04/16] HID: ft260: i2c: reduce driver module loading time Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 05/16] HID: ft260: fix sysfs store failure notice for big numbers Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 06/16] HID: ft260: i2c: reduce bus-error message severity Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 07/16] HID: ft260: uart: enable flow control Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 08/16] HID: ft260: uart: add modem pins control via ioctl Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 09/16] HID: ft260: group sysfs attrs per HID interface Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 10/16] HID: ft260: i2c: fix large write transaction failure Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 11/16] HID: ft260: workaround for TN_189 errata endpoint STALL after enumeration Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 12/16] HID: ft260: i2c: abort in-flight transfers with STOP before reset Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 13/16] HID: ft260: i2c: reject an I2C clock outside the supported range Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 14/16] HID: ft260: i2c: fix i2c_reset sysfs store return value Michael Zaidman
2026-11-14 23:07 ` [PATCH v3 15/16] HID: ft260: i2c: recover a bus held by a slave Michael Zaidman
2026-11-14 23:08 ` [PATCH v3 16/16] HID: ft260: use lock guards Michael Zaidman
2026-11-14 23:21 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox