Message-ID: <20251023134828.2dzq2rhtjplqyyaj@pengutronix.de>
Date: Thu, 23 Oct 2025 15:48:28 +0200
From: Marco Felsch <m.felsch@...gutronix.de>
To: Johan Hovold <johan@...nel.org>
Cc: Rob Herring <robh@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>, linux-serial@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH 0/3] USB-Serial serdev support

On 25-10-23, Johan Hovold wrote:
> On Thu, Mar 13, 2025 at 08:40:44PM +0100, Marco Felsch wrote:
> > On 25-03-11, Johan Hovold wrote:
> > > On Tue, Sep 17, 2024 at 06:49:48AM +0200, Marco Felsch wrote:
> > > > On 24-09-09, Johan Hovold wrote:
> > > > > On Wed, Aug 07, 2024 at 04:08:47PM +0200, Marco Felsch wrote:
> > > > > > this patchset is based on Johan's patches [1] but dropped the need of
> > > > > > the special 'serial' of-node [2].
> > > > > 
> > > > > That's great that you found and referenced my proof-of-concept patches,
> > > > > but it doesn't seem like you tried to understand why this hasn't been
> > > > > merged yet.
> > > 
> > > > > First, as the commit message you refer to below explain, we need some
> > > > > way to describe multiport controllers. Just dropping the 'serial' node
> > > > > does not make that issue go away.
> > > > 
> > > > Sorry for asking but isn't the current OF abstraction [1] enough? As far
> > > > as I understood we can describe the whole USB tree within OF. I used [1]
> > > > and the this patchset to describe the following hierarchy:
> > > > 
> > > >  usb-root -> usb-hub port-1 -> usb-serial interface-0 -> serial
> > > >                                                          bt-module
> > > > 
> > > > [1] Documentation/devicetree/bindings/usb/usb-device.yaml
> > > 
> > > Again, you still need to consider devices with multiple serial ports
> > > (and they do not always map neatly to one port per interface either).
> > 
> > We use a dual-port FTDI and our USB tree looks as followed:
> 
> > interface-0 is used for the bt-module which is served by the serdev
> > driver.
> > 
> > interface-1 is used by an userspace driver which makes use of the
> > /dev/ttyUSB1 port.
> >
> > So we do have the multiple serial ports use-case already. Can you please
> > explain what I miss?
> 
> There are other USB serial devices that support multiple ports over a
> single USB interface. The DT bindings need to account for that case as
> well, and that probably means we should not be describing the interfaces
> at all but rather the physical ports.

Ah okay, I wasn't even aware that this possible too. However this is DT
description and another topic.

> > > > > Second, and more importantly, you do not address the main obstacle for
> > > > > enabling serdev for USB serial which is that the serdev cannot handle
> > > > > hotplugging.
> > > > 
> > > > Hotplugging is a good point but out-of-scope IMHO (at least for now)
> > > > since the current serdev implementation rely on additional firmware
> > > > information e.g OF node to be present. E.g. if the above mentioned setup
> > > > would connect the "serial bt-module" directly to the UART port you still
> > > > need an OF node to bind the serdev driver. If the node isn't present
> > > > user-space would need to do the hci handling.
> > > 
> > > There's nothing preventing you from adding a devicetree node for a USB
> > > device that can be unplugged.
> > 
> > I see and I have to admit that I didn't test this :/ But since you
> > pointed it out I tested it now!
> > 
> > So as explained, our USB tree looks as above and our DTS looks like the
> > one in the cover letter. Of course I run on an embedded system but the
> > USB FTDI based module is powered by the VBUS of the hub. Therefore I
> > ran the test by disabling the downstream port which in turn disabled the
> > VBUS supply. This should come very close to a physical unplug event.
> 
> You will also see the following kind of warnings in the logs:
> 
> ttyUSB ttyUSB0: tty_hangup: tty->count(1) != (#fd's(0) + #kopen's(0))
> ttyUSB ttyUSB0: tty_port_close_start: tty->count = 1 port count = 0
> 
> which are due to the fact that serdev does not support hangups which are
> used during teardown of USB serial ports.

IIRC I added the following patch to solve this:

 - [PATCH 1/3] serdev: ttyport: make use of tty_kopen_exclusive

Sorry for not remembering the details since this conversation/patchset
is quite old but still one of our top prios.

> > > > So from my POV the serdev abstraction is for manufacturers which make
> > > > use of "onboard" usb-devices which are always present at the same USB
> > > > tree location. Serdev is not made for general purpose USB ports (yet)
> > > > where a user can plug-in all types of USB devices.
> > > 
> > > Right, but someone need to make sure that serdev can handle devices
> > > going away first as nothing is currently preventing that from happening.
> > 
> > Can you please check my above tests? Maybe I do miss something but for
> > me it looks like it's working. Looking forwards for your input.
> 
> I skimmed the code to verify that the issue is still there, and even
> forward ported my patches to confirm that that you would still see the
> port count warnings that indicate that something is amiss.

As said, patch-1 should address this issue.

Regards,
  Marco


> Johan
> 

Powered by blists - more mailing lists