Re: [PATCH 08/13] HID: ft260: uart: add modem pins control via ioctl

[Linus Walleij <linusw@kernel.org>]

Hi Michael,

On Fri, Aug 28, 2026 at 12:08 AM Michael Zaidman
<michael.zaidman@gmail.com> wrote:

[Me]
> > On Kconfig, so that gpiolib is always available and you can use
> > the generic modem control helpers for modem control over GPIO.
>
> I looked into this, and it does not work for the FT260 without
> changing serial_mctrl_gpio.c first. Three blockers:
>
> - Every GPIO access on this chip is a USB transfer, so the
>   gpiochip has can_sleep = true. mctrl_gpio_set() calls
>   gpiod_set_array_value() and mctrl_gpio_get() calls
>   gpiod_get_value(), and gpiolib does WARN_ON(can_sleep) in both,
>   so every TIOCMGET/TIOCMSET would give a WARN backtrace.

Can't you just patch mctrl to use gpiod_set_array_value_cansleep()
and gpiod_get_value_cansleep()?

I don't think any of the users depend on call thing this
in atomic context.

> - mctrl_gpio_init() takes a struct uart_port and its IRQ handler
>   needs it: uart_port_lock_irqsave(), uart_handle_dcd_change(),
>   port->icount, delta_msr_wait. This UART is a plain tty_driver
>   with a tty_port, so only mctrl_gpio_init_noauto() is left - and
>   the FT260 GPIO lines have no interrupts anyway.

I don't understand this :D
But hopefully the TTY/serial maintainer does.

> - mctrl_gpio_init_noauto() only picks up lines that exist as
>   firmware properties: device_property_present(dev, "cts-gpios")
>   and friends. A gpiod_add_lookup_table() table is the machine
>   lookup path, so every line would be skipped, all descriptors
>   would stay NULL and both helpers would silently do nothing.
>   Software nodes could satisfy that check, but there is no
>   PROPERTY_ENTRY_GPIO in the tree to build them with.

Using software nodes is the way to go I think,
<linux/gpio/property.h> contains PROPERTY_ENTRY_GPIO.

> serial_mctrl_gpio.h is also private to drivers/tty/serial - all
> eleven users are serial_core drivers in that directory.

Well having serial drivers in drivers/hid and having all kinds
of misc drivers in drivers/hid has made it a dumping ground
for anything HID.

> Registering a uart_port instead was tried for this device and
> turned down. Daniel Beer's 2022 FT260 UART patch was built on
> serial_core and called uart_add_one_port(); Greg asked for
> usb-serial, and Johan Hovold answered that "neither USB-serial or
> serial (core) is a good fit for such a HID device", pointing at
> Christina Quast's tty driver as the right approach - which patch
> 1 of this series is a port of.
>
>   https://lore.kernel.org/lkml/638c51a2.170a0220.3af16.18f8@mx.google.com/
>   https://lore.kernel.org/lkml/Y6WNl6+ySy8zcSyg@hovoldconsulting.com/

Well if it absolutely has to live in drivers/hid then do the
ugly thing and

include "../tty/serial/serial_mctrl_gpio.h"

It's perhaps the lesser evil then?

Otherwise we just bite the bullet and move
drivers/tty/serial/serial_mctrl_gpio.h
to
include/linux/serial_mctrl_gpio.h
?

(Unless Greg want some new subdir such as
include/linux/serial/serial_mctrl_gpio.h)

> > The core idea is that the serial modem control should look
> > up the GPIOs from its own gpiochip and use the MCTRL
> > library helpers, then this should result in very little and
> > compact code that is easy to read.
>
> No argument with the goal - I would rather have that than my own
> TIOCM handling. But making it usable here means work inside the
> serial helpers: cansleep set/get, a path that does not require a
> uart_port, a lookup that works without firmware properties, and
> the header moved to include/linux. That is a serial subsystem
> series to agree with Greg and Jiri Slaby, so I propose keeping
> the ioctl implementation in this series and doing the conversion
> as a follow-up.

These things have a tendency to never happen and it's
not like we have a shortage of technical debt.

Yours,
Linus Walleij