Hi,

this patchset is based on Johan's patches [1] but dropped the need of
the special 'serial' of-node [2].

With the patches in place and the usb hierarchy described in properly we
can use serdev on-top of usb-serial. The below example adds the support
for the following hierarchy:
 - host->usb-hub->ftdi-usb-uart->bt/wlan-module:

&usb_dwc3_1 {
	dr_mode = "host";
	status = "okay";

	hub@1 {
		compatible = "usb424,2514";
		reg = <1>;

		vdd-supply = <&reg>;
		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>;

		#address-cells = <1>;
		#size-cells = <0>;

		device@1 {
			compatible = "usb403,6010";
			reg = <1>;

			#address-cells = <2>;
			#size-cells = <0>;

			interface@0 {
				compatible = "usbif403,6010.config1.0";
				reg = <0 1>;

				#address-cells = <1>;
				#size-cells = <0>;

				bluetooth {
					compatible = "nxp,88w8987-bt";
					fw-init-baudrate = <3000000>;
				};
			};
		};
	};
};

If no serdev node is found the usb-serial is exposed as usual and can be
accessed via /dev/ttyUSBx.

Regards,
  Marco

[1] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of
[2] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-serial-of&id=b19239022c92567a6a9ed40e8522e84972b0997f

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Marco Felsch (3):
      serdev: ttyport: make use of tty_kopen_exclusive
      USB: serial: cosmetic cleanup <space><tab> mix
      USB: serial: enable serdev support

 drivers/tty/serdev/serdev-ttyport.c |  9 ++++++---
 drivers/usb/serial/bus.c            | 10 ++++++----
 2 files changed, 12 insertions(+), 7 deletions(-)
---
base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd
change-id: 20240807-v6-10-topic-usb-serial-serdev-83a7f8f86432

Best regards,
-- 
Marco Felsch <m.felsch@pengutronix.de>

Hi,

gentle ping as this is series is two months old now.

Regards,
  Marco

On 24-08-07, Marco Felsch wrote:
> Hi,
> 
> this patchset is based on Johan's patches [1] but dropped the need of
> the special 'serial' of-node [2].
> 
> With the patches in place and the usb hierarchy described in properly we
> can use serdev on-top of usb-serial. The below example adds the support
> for the following hierarchy:
>  - host->usb-hub->ftdi-usb-uart->bt/wlan-module:
> 
> &usb_dwc3_1 {
> 	dr_mode = "host";
> 	status = "okay";
> 
> 	hub@1 {
> 		compatible = "usb424,2514";
> 		reg = <1>;
> 
> 		vdd-supply = <&reg>;
> 		reset-gpios = <&gpio4 5 GPIO_ACTIVE_LOW>;
> 
> 		#address-cells = <1>;
> 		#size-cells = <0>;
> 
> 		device@1 {
> 			compatible = "usb403,6010";
> 			reg = <1>;
> 
> 			#address-cells = <2>;
> 			#size-cells = <0>;
> 
> 			interface@0 {
> 				compatible = "usbif403,6010.config1.0";
> 				reg = <0 1>;
> 
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 
> 				bluetooth {
> 					compatible = "nxp,88w8987-bt";
> 					fw-init-baudrate = <3000000>;
> 				};
> 			};
> 		};
> 	};
> };
> 
> If no serdev node is found the usb-serial is exposed as usual and can be
> accessed via /dev/ttyUSBx.
> 
> Regards,
>   Marco
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-serial-of&id=b19239022c92567a6a9ed40e8522e84972b0997f
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
> Marco Felsch (3):
>       serdev: ttyport: make use of tty_kopen_exclusive
>       USB: serial: cosmetic cleanup <space><tab> mix
>       USB: serial: enable serdev support
> 
>  drivers/tty/serdev/serdev-ttyport.c |  9 ++++++---
>  drivers/usb/serial/bus.c            | 10 ++++++----
>  2 files changed, 12 insertions(+), 7 deletions(-)
> ---
> base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd
> change-id: 20240807-v6-10-topic-usb-serial-serdev-83a7f8f86432
> 
> Best regards,
> -- 
> Marco Felsch <m.felsch@pengutronix.de>
> 
>

On Tue, Oct 01, 2024 at 09:24:53AM +0200, Marco Felsch wrote:
> Hi,
> 
> gentle ping as this is series is two months old now.

And it was rejected as serdev does not support hotplug which of course,
usb-serial does.

So until serdev is fixed up to handle that correctly, this is not going
anywhere, nor should you want it to as then you would be in charge of
code that does not work properly :)

thanks,

greg k-h

Hi Greg,

On 24-10-01, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2024 at 09:24:53AM +0200, Marco Felsch wrote:
> > Hi,
> > 
> > gentle ping as this is series is two months old now.
> 
> And it was rejected as serdev does not support hotplug which of course,
> usb-serial does.

I hoped to get some feedback on my answer [1]. Regarding hotplug
support: serdev _requires_ some sort of firmware like OF (not sure if it
does work with ACPI too). That said, if serdev finds no firmware a
fallback is provided to the standard serial handling.

The firmware could either be added directly by the platform OF file or
via OF-overlays. By making use of overlays we could gain some kind of
hotplug: Once a usb devices was detected and the driver has an
overlay, the overlay gets applied and the probe continues, like we do it
for PCIe devices now [2]. For devices which don't have a registered
overlay the standard usb-serial setup is done by exposing the serial
interface to the userspace.

> So until serdev is fixed up to handle that correctly, this is not going
> anywhere, nor should you want it to as then you would be in charge of
> code that does not work properly :)

Regards,
  Marco

[1] https://lore.kernel.org/all/20240917044948.i2eog4ondf7vna7q@pengutronix.de/
[2] https://lore.kernel.org/all/7512cbb7911b8395d926e9e9e390fbb55ce3aea9.camel@pengutronix.de/

> 
> thanks,
> 
> greg k-h
>

On Mon, Oct 28, 2024 at 11:57:02PM +0100, Marco Felsch wrote:

> On 24-10-01, Greg Kroah-Hartman wrote:
> > On Tue, Oct 01, 2024 at 09:24:53AM +0200, Marco Felsch wrote:

> > > gentle ping as this is series is two months old now.
> > 
> > And it was rejected as serdev does not support hotplug which of course,
> > usb-serial does.
> 
> I hoped to get some feedback on my answer [1]. Regarding hotplug
> support: serdev _requires_ some sort of firmware like OF (not sure if it
> does work with ACPI too). That said, if serdev finds no firmware a
> fallback is provided to the standard serial handling.

It's devices going away not being supported which is the main concern.
The serdev ttyport implementation does not implement hangup() which is
used for serial port tear down.

> The firmware could either be added directly by the platform OF file or
> via OF-overlays. By making use of overlays we could gain some kind of
> hotplug: Once a usb devices was detected and the driver has an
> overlay, the overlay gets applied and the probe continues, like we do it
> for PCIe devices now [2]. For devices which don't have a registered
> overlay the standard usb-serial setup is done by exposing the serial
> interface to the userspace.

Then it would also be nice to have a way to describe hotplugged devices
on the fly, and overlays could indeed be used for that. But that's a
separate story.

Johan

Hi,

On 24-10-28, Marco Felsch wrote:
> Hi Greg,
> 
> On 24-10-01, Greg Kroah-Hartman wrote:
> > On Tue, Oct 01, 2024 at 09:24:53AM +0200, Marco Felsch wrote:
> > > Hi,
> > > 
> > > gentle ping as this is series is two months old now.
> > 
> > And it was rejected as serdev does not support hotplug which of course,
> > usb-serial does.
> 
> I hoped to get some feedback on my answer [1]. Regarding hotplug
> support: serdev _requires_ some sort of firmware like OF (not sure if it
> does work with ACPI too). That said, if serdev finds no firmware a
> fallback is provided to the standard serial handling.
> 
> The firmware could either be added directly by the platform OF file or
> via OF-overlays. By making use of overlays we could gain some kind of
> hotplug: Once a usb devices was detected and the driver has an
> overlay, the overlay gets applied and the probe continues, like we do it
> for PCIe devices now [2]. For devices which don't have a registered
> overlay the standard usb-serial setup is done by exposing the serial
> interface to the userspace.

is this idea worth to give it a try for a v2 or do you have something
different in mind?

I'm happy for any input.

Regards,
  Marco

> > So until serdev is fixed up to handle that correctly, this is not going
> > anywhere, nor should you want it to as then you would be in charge of
> > code that does not work properly :)
> 
> Regards,
>   Marco
> 
> [1] https://lore.kernel.org/all/20240917044948.i2eog4ondf7vna7q@pengutronix.de/
> [2] https://lore.kernel.org/all/7512cbb7911b8395d926e9e9e390fbb55ce3aea9.camel@pengutronix.de/
> 
> > 
> > thanks,
> > 
> > greg k-h
> >

On 24-10-01, Greg Kroah-Hartman wrote:
> On Tue, Oct 01, 2024 at 09:24:53AM +0200, Marco Felsch wrote:
> > Hi,
> > 
> > gentle ping as this is series is two months old now.
> 
> And it was rejected as serdev does not support hotplug which of course,
> usb-serial does.

I just read concerns which I tried to explain/argue but didn't saw it as
rejected.

> So until serdev is fixed up to handle that correctly, this is not going
> anywhere, nor should you want it to as then you would be in charge of
> code that does not work properly :)

Sure as always :)

Regards,
  Marco

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.

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.


> [1] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-serial-of&id=b19239022c92567a6a9ed40e8522e84972b0997f

Johan

Hi Johan,

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.

I'm glad for your input.

> 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

> 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.

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.

Regards,
  Marco

> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of
> > [2] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-serial-of&id=b19239022c92567a6a9ed40e8522e84972b0997f
> 
> Johan
>

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).

> > 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.

> 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.

> > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of
> > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-serial-of&id=b19239022c92567a6a9ed40e8522e84972b0997f

Johan

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:

/:  Bus 001.Port 001: Dev 001, Class=root_hub, Driver=xhci-hcd/1p, 480M
    ID 1d6b:0002 Linux Foundation 2.0 root hub
    |__ Port 001: Dev 002, If 0, Class=Hub, Driver=hub/4p, 480M
        ID 0424:2514 Microchip Technology, Inc. (formerly SMSC) USB 2.0 Hub
        |__ Port 001: Dev 003, If 0, Class=Vendor Specific Class, Driver=ftdi_sio, 480M
            ID 0403:6010 Future Technology Devices International, Ltd FT2232C/D/H Dual UART/FIFO IC
        |__ Port 001: Dev 003, If 1, Class=Vendor Specific Class, Driver=ftdi_sio, 480M
            ID 0403:6010 Future Technology Devices International, Ltd FT2232C/D/H Dual UART/FIFO IC

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?

> > > 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.

8<----------------------------------------------------------------

## The test system before the "unplug"

root@test:~# ls -al /sys/class/bluetooth/
total 0
drwxr-xr-x  2 root root 0 Jan  8 18:31 .
drwxr-xr-x 62 root root 0 Jan  8 18:31 ..
lrwxrwxrwx  1 root root 0 Jan  8 18:31 hci0 -> ../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0/bluetooth/hci0

root@test:~# ls -al /sys/bus/serial/devices/
total 0
drwxr-xr-x 2 root root 0 Jan  8 18:31 .
drwxr-xr-x 4 root root 0 Jan  8 18:28 ..
lrwxrwxrwx 1 root root 0 Jan  8 18:31 serial0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0
lrwxrwxrwx 1 root root 0 Jan  8 18:31 serial0-0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0

## The "unplug" event and the system after the event

root@test:~# echo 1 > /sys/bus/usb/devices/usb1/1-1/1-1\:1.0/1-1-port1/disable

root@test:~# ls -al /sys/class/bluetooth/
total 0
drwxr-xr-x  2 root root 0 Jan  8 18:40 .
drwxr-xr-x 62 root root 0 Jan  8 18:31 ..

root@test:~# ls -al /sys/bus/serial/devices/
total 0
drwxr-xr-x 2 root root 0 Jan  8 18:40 .
drwxr-xr-x 4 root root 0 Jan  8 18:28 ..

## The "plug" event and the system after the event

root@test:~# echo 0 > /sys/bus/usb/devices/usb1/1-1/1-1\:1.0/1-1-port1/disable
root@test:~# [ 1121.297918] btnxpuart serial0-0: supply vcc not found, using dummy regulator

root@test:~# ls -al /sys/class/bluetooth/
total 0
drwxr-xr-x  2 root root 0 Jan  8 18:41 .
drwxr-xr-x 62 root root 0 Jan  8 18:31 ..
lrwxrwxrwx  1 root root 0 Jan  8 18:41 hci0 -> ../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0/bluetooth/hci0

root@test:~# ls -al /sys/bus/serial/devices/
total 0
drwxr-xr-x 2 root root 0 Jan  8 18:41 .
drwxr-xr-x 4 root root 0 Jan  8 18:28 ..
lrwxrwxrwx 1 root root 0 Jan  8 18:41 serial0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0
lrwxrwxrwx 1 root root 0 Jan  8 18:41 serial0-0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0

8<----------------------------------------------------------------

> > 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.

Regards,
  Marco


> > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of
> > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-serial-of&id=b19239022c92567a6a9ed40e8522e84972b0997f
> 
> Johan
>

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.

> > > > 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.

> > > 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.

Johan

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
>

On Thu, Oct 23, 2025 at 03:48:28PM +0200, Marco Felsch wrote:
> 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:

> > > > > > 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.

> > 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.

It's still one of the issues that need to addressed.
 
> > > > > > 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.

> > 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.

That suppresses the first warning but doesn't address the underlying
issue (that hangups are built around file handles which serdev does not
use). And you will still see the second one when the serdev driver tries
to close the already hung up port during deregistration.

Also, that commit message needs to more work since you don't really
motivate why you think it's needed (e.g. as serdev ports can't be shared
with user space).

If it's just about suppressing the warning you could possibly just have
set that new flag.

Johan

On 25-10-24, Johan Hovold wrote:
> On Thu, Oct 23, 2025 at 03:48:28PM +0200, Marco Felsch wrote:
> > 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:
> 
> > > > > > > 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.
> 
> > > 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.
> 
> It's still one of the issues that need to addressed.

Yes but this shouldn't be an issue with this patchset. So far the
smallest DT-describale USB entities are the interfaces.

Additional support needs to be added if there are devices which need a
more fine-grained description. I can't implement this since I don't have
access to such devices. That beeing said, my patchset doesn't break such
devices because in that case these devices simply can't be described as
serdev device within the DT and the ttyUSBx devices are exposed to
userspace.

If one wants to add the support for it, the support can surely be added
afterwards too in a backward compatible manner.

> > > > > > > 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.
> 
> > > 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.
> 
> That suppresses the first warning but doesn't address the underlying
> issue (that hangups are built around file handles which serdev does not
> use). And you will still see the second one when the serdev driver tries
> to close the already hung up port during deregistration.

Can you please elaborate how I can check this? I'm not aware of any
warning yet, but I only tested the hot-(un)plug. If I got your right, I
should see the issue once I unload the serdev driver, right?

> Also, that commit message needs to more work since you don't really
> motivate why you think it's needed (e.g. as serdev ports can't be shared
> with user space).

Maybe it needs some adaptions but:

| The purpose of serdev is to provide kernel drivers for particular serial
| device, serdev-ttyport is no exception here. Make use of the
| tty_kopen_exclusive() funciton to mark this tty device as kernel
| internal device.

the last sentence should address your point that serdev ports can't be
shared with user-space.

> If it's just about suppressing the warning you could possibly just have
> set that new flag.

Which new flag? As I have written in my commit message: "Make use of ...
to mark this tty device as kernel internal device". I thought this was
the purpose of tty_kopen_exclusive().

Regards,
  Marco


> 
> Johan
> 

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

On Fri, Oct 24, 2025 at 11:27:38AM +0200, Marco Felsch wrote:
> On 25-10-24, Johan Hovold wrote:
> > On Thu, Oct 23, 2025 at 03:48:28PM +0200, Marco Felsch wrote:
> > > 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:

> > It's still one of the issues that need to addressed.
> 
> Yes but this shouldn't be an issue with this patchset. So far the
> smallest DT-describale USB entities are the interfaces.

It is an issue with this patchset since any binding for USB serdev will
need to take both kind of devices into account. Period.

> > > > > > > > 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.
> > 
> > > > 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.
> > 
> > That suppresses the first warning but doesn't address the underlying
> > issue (that hangups are built around file handles which serdev does not
> > use). And you will still see the second one when the serdev driver tries
> > to close the already hung up port during deregistration.
> 
> Can you please elaborate how I can check this? I'm not aware of any
> warning yet, but I only tested the hot-(un)plug. If I got your right, I
> should see the issue once I unload the serdev driver, right?

You should see it in your test setup as well. Unless the bluetooth
driver you use is doing something funky (e.g. not closing the port).

I'm testing with a mock gnss device here.

> > Also, that commit message needs to more work since you don't really
> > motivate why you think it's needed (e.g. as serdev ports can't be shared
> > with user space).
> 
> Maybe it needs some adaptions but:
> 
> | The purpose of serdev is to provide kernel drivers for particular serial
> | device, serdev-ttyport is no exception here. Make use of the
> | tty_kopen_exclusive() funciton to mark this tty device as kernel
> | internal device.
> 
> the last sentence should address your point that serdev ports can't be
> shared with user-space.A

No, my point was that serdev devices *are* not shared with user space,
you don't need to use that new kopen helper for that.

> > If it's just about suppressing the warning you could possibly just have
> > set that new flag.
> 
> Which new flag? As I have written in my commit message: "Make use of ...
> to mark this tty device as kernel internal device". I thought this was
> the purpose of tty_kopen_exclusive().

That helper sets the new TTY_PORT_KOPENED flag which suppresses the
warning on hangups.

Johan

On 25-10-24, Johan Hovold wrote:
> On Fri, Oct 24, 2025 at 11:27:38AM +0200, Marco Felsch wrote:
> > On 25-10-24, Johan Hovold wrote:
> > > On Thu, Oct 23, 2025 at 03:48:28PM +0200, Marco Felsch wrote:
> > > > 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:
> 
> > > It's still one of the issues that need to addressed.
> > 
> > Yes but this shouldn't be an issue with this patchset. So far the
> > smallest DT-describale USB entities are the interfaces.
> 
> It is an issue with this patchset since any binding for USB serdev will
> need to take both kind of devices into account. Period.

Sorry but I really don't see the issue. As of now DT abstractions
supports all my use-cases. If $another_developer has an USB device which
actually exposes multiple serial ports behind a single usb-interface,
fine. But in that case $another_developer needs to add the
support/extend the support for it if he wants to use it in combination
with serdev.

I actually have no such USB device and also my customer doesn't use such
a device. Therefore I'm afraid that I can't add support for something I
can't actually test.

What is your suggestion how the DT abstraction should look like in 2025,
e.g. given the current DT abstraction?

> > > > > > > > > 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.
> > > 
> > > > > 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.
> > > 
> > > That suppresses the first warning but doesn't address the underlying
> > > issue (that hangups are built around file handles which serdev does not
> > > use). And you will still see the second one when the serdev driver tries
> > > to close the already hung up port during deregistration.
> > 
> > Can you please elaborate how I can check this? I'm not aware of any
> > warning yet, but I only tested the hot-(un)plug. If I got your right, I
> > should see the issue once I unload the serdev driver, right?
> 
> You should see it in your test setup as well. Unless the bluetooth
> driver you use is doing something funky (e.g. not closing the port).
> 
> I'm testing with a mock gnss device here.

Okay, let me test this. Just that we're on the same page: The test is to
remove the serdev (bluetooth, gnss, ...) driver, right?

> > > Also, that commit message needs to more work since you don't really
> > > motivate why you think it's needed (e.g. as serdev ports can't be shared
> > > with user space).
> > 
> > Maybe it needs some adaptions but:
> > 
> > | The purpose of serdev is to provide kernel drivers for particular serial
> > | device, serdev-ttyport is no exception here. Make use of the
> > | tty_kopen_exclusive() funciton to mark this tty device as kernel
> > | internal device.
> > 
> > the last sentence should address your point that serdev ports can't be
> > shared with user-space.A
> 
> No, my point was that serdev devices *are* not shared with user space,
> you don't need to use that new kopen helper for that.
> 
> > > If it's just about suppressing the warning you could possibly just have
> > > set that new flag.
> > 
> > Which new flag? As I have written in my commit message: "Make use of ...
> > to mark this tty device as kernel internal device". I thought this was
> > the purpose of tty_kopen_exclusive().
> 
> That helper sets the new TTY_PORT_KOPENED flag which suppresses the
> warning on hangups.

Okay, so you meant the TTY_PORT_KOPENED flag. According the
documentation of tty_kopen_exclusive():

| tty_kopen_exclusive - open a tty device for kernel

isn't that exactly what serdev-ttyport should do to "not share it with
user space"? IMHO it's an implementation detail if the logic behind
"open a tty device for kernel" is only built around a flag to suppress
the warning.

Regards,
  Marco

On Fri, Oct 24, 2025 at 02:40:47PM +0200, Marco Felsch wrote:
> On 25-10-24, Johan Hovold wrote:
> > On Fri, Oct 24, 2025 at 11:27:38AM +0200, Marco Felsch wrote:
> > > On 25-10-24, Johan Hovold wrote:
> > > > On Thu, Oct 23, 2025 at 03:48:28PM +0200, Marco Felsch wrote:
> > > > > 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:
> > 
> > > > It's still one of the issues that need to addressed.
> > > 
> > > Yes but this shouldn't be an issue with this patchset. So far the
> > > smallest DT-describale USB entities are the interfaces.
> > 
> > It is an issue with this patchset since any binding for USB serdev will
> > need to take both kind of devices into account. Period.
> 
> Sorry but I really don't see the issue. As of now DT abstractions
> supports all my use-cases. If $another_developer has an USB device which
> actually exposes multiple serial ports behind a single usb-interface,
> fine. But in that case $another_developer needs to add the
> support/extend the support for it if he wants to use it in combination
> with serdev.

Fine, but if you only care about your use case then you can keep your
implementation out-of-tree until someone comes with along with enough
time to solve this properly.

> > > > > > 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.

> > You should see it in your test setup as well. Unless the bluetooth
> > driver you use is doing something funky (e.g. not closing the port).
> > 
> > I'm testing with a mock gnss device here.
> 
> Okay, let me test this. Just that we're on the same page: The test is to
> remove the serdev (bluetooth, gnss, ...) driver, right?

No, trigger a disconnect like you did before, or do a physical
disconnect, by wiring up a regular USB port.
 
> > > > Also, that commit message needs to more work since you don't really
> > > > motivate why you think it's needed (e.g. as serdev ports can't be shared
> > > > with user space).

> > No, my point was that serdev devices *are* not shared with user space,
> > you don't need to use that new kopen helper for that.

> > That helper sets the new TTY_PORT_KOPENED flag which suppresses the
> > warning on hangups.
> 
> Okay, so you meant the TTY_PORT_KOPENED flag. According the
> documentation of tty_kopen_exclusive():
> 
> | tty_kopen_exclusive - open a tty device for kernel
> 
> isn't that exactly what serdev-ttyport should do to "not share it with
> user space"? IMHO it's an implementation detail if the logic behind
> "open a tty device for kernel" is only built around a flag to suppress
> the warning.

I give up. I've already told you that serdev does not share anything
with user space.

Johan

On 25-10-24, Johan Hovold wrote:
> On Fri, Oct 24, 2025 at 02:40:47PM +0200, Marco Felsch wrote:
> > On 25-10-24, Johan Hovold wrote:
> > > On Fri, Oct 24, 2025 at 11:27:38AM +0200, Marco Felsch wrote:
> > > > On 25-10-24, Johan Hovold wrote:
> > > > > On Thu, Oct 23, 2025 at 03:48:28PM +0200, Marco Felsch wrote:
> > > > > > 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:
> > > 
> > > > > It's still one of the issues that need to addressed.
> > > > 
> > > > Yes but this shouldn't be an issue with this patchset. So far the
> > > > smallest DT-describale USB entities are the interfaces.
> > > 
> > > It is an issue with this patchset since any binding for USB serdev will
> > > need to take both kind of devices into account. Period.
> > 
> > Sorry but I really don't see the issue. As of now DT abstractions
> > supports all my use-cases. If $another_developer has an USB device which
> > actually exposes multiple serial ports behind a single usb-interface,
> > fine. But in that case $another_developer needs to add the
> > support/extend the support for it if he wants to use it in combination
> > with serdev.
> 
> Fine, but if you only care about your use case then you can keep your
> implementation out-of-tree until someone comes with along with enough
> time to solve this properly.

Fair enough, after checking your patchset once again I see your special
USB-UART converter. I get your point and will add the serial@ binding!

Since I don't have a MOS7840 USB-Serial to test it, it would be nice of
you if you could test it afterwards on your side as well.

> > > > > > > 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.
> 
> > > You should see it in your test setup as well. Unless the bluetooth
> > > driver you use is doing something funky (e.g. not closing the port).
> > > 
> > > I'm testing with a mock gnss device here.
> > 
> > Okay, let me test this. Just that we're on the same page: The test is to
> > remove the serdev (bluetooth, gnss, ...) driver, right?
> 
> No, trigger a disconnect like you did before, or do a physical
> disconnect, by wiring up a regular USB port.

I will test it again, thanks!

> > > > > Also, that commit message needs to more work since you don't really
> > > > > motivate why you think it's needed (e.g. as serdev ports can't be shared
> > > > > with user space).
> 
> > > No, my point was that serdev devices *are* not shared with user space,
> > > you don't need to use that new kopen helper for that.
> 
> > > That helper sets the new TTY_PORT_KOPENED flag which suppresses the
> > > warning on hangups.
> > 
> > Okay, so you meant the TTY_PORT_KOPENED flag. According the
> > documentation of tty_kopen_exclusive():
> > 
> > | tty_kopen_exclusive - open a tty device for kernel
> > 
> > isn't that exactly what serdev-ttyport should do to "not share it with
> > user space"? IMHO it's an implementation detail if the logic behind
> > "open a tty device for kernel" is only built around a flag to suppress
> > the warning.
> 
> I give up. I've already told you that serdev does not share anything
> with user space.

I really try to understand the issue you see which I don't.

To cycle back: You posted the following warnings:

| 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

As written earlier, the tty_kopen_exclusive() should mark the serial
port as kernel-only serial. Also as you said, this suppresses the first
warning.

Is your issue that serdev can't handle hangups (e.g. hot-plugs) yet and
that a serdev driver could try to access the tty while the .remove() is
in progress?

Regards,
  Marco

-- 
#gernperDu 
#CallMeByMyFirstName

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Hi,

gentle ping.

Regards,
  Marco

On 25-03-13, 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:
> 
> /:  Bus 001.Port 001: Dev 001, Class=root_hub, Driver=xhci-hcd/1p, 480M
>     ID 1d6b:0002 Linux Foundation 2.0 root hub
>     |__ Port 001: Dev 002, If 0, Class=Hub, Driver=hub/4p, 480M
>         ID 0424:2514 Microchip Technology, Inc. (formerly SMSC) USB 2.0 Hub
>         |__ Port 001: Dev 003, If 0, Class=Vendor Specific Class, Driver=ftdi_sio, 480M
>             ID 0403:6010 Future Technology Devices International, Ltd FT2232C/D/H Dual UART/FIFO IC
>         |__ Port 001: Dev 003, If 1, Class=Vendor Specific Class, Driver=ftdi_sio, 480M
>             ID 0403:6010 Future Technology Devices International, Ltd FT2232C/D/H Dual UART/FIFO IC
> 
> 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?
> 
> > > > 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.
> 
> 8<----------------------------------------------------------------
> 
> ## The test system before the "unplug"
> 
> root@test:~# ls -al /sys/class/bluetooth/
> total 0
> drwxr-xr-x  2 root root 0 Jan  8 18:31 .
> drwxr-xr-x 62 root root 0 Jan  8 18:31 ..
> lrwxrwxrwx  1 root root 0 Jan  8 18:31 hci0 -> ../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0/bluetooth/hci0
> 
> root@test:~# ls -al /sys/bus/serial/devices/
> total 0
> drwxr-xr-x 2 root root 0 Jan  8 18:31 .
> drwxr-xr-x 4 root root 0 Jan  8 18:28 ..
> lrwxrwxrwx 1 root root 0 Jan  8 18:31 serial0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0
> lrwxrwxrwx 1 root root 0 Jan  8 18:31 serial0-0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0
> 
> ## The "unplug" event and the system after the event
> 
> root@test:~# echo 1 > /sys/bus/usb/devices/usb1/1-1/1-1\:1.0/1-1-port1/disable
> 
> root@test:~# ls -al /sys/class/bluetooth/
> total 0
> drwxr-xr-x  2 root root 0 Jan  8 18:40 .
> drwxr-xr-x 62 root root 0 Jan  8 18:31 ..
> 
> root@test:~# ls -al /sys/bus/serial/devices/
> total 0
> drwxr-xr-x 2 root root 0 Jan  8 18:40 .
> drwxr-xr-x 4 root root 0 Jan  8 18:28 ..
> 
> ## The "plug" event and the system after the event
> 
> root@test:~# echo 0 > /sys/bus/usb/devices/usb1/1-1/1-1\:1.0/1-1-port1/disable
> root@test:~# [ 1121.297918] btnxpuart serial0-0: supply vcc not found, using dummy regulator
> 
> root@test:~# ls -al /sys/class/bluetooth/
> total 0
> drwxr-xr-x  2 root root 0 Jan  8 18:41 .
> drwxr-xr-x 62 root root 0 Jan  8 18:31 ..
> lrwxrwxrwx  1 root root 0 Jan  8 18:41 hci0 -> ../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0/bluetooth/hci0
> 
> root@test:~# ls -al /sys/bus/serial/devices/
> total 0
> drwxr-xr-x 2 root root 0 Jan  8 18:41 .
> drwxr-xr-x 4 root root 0 Jan  8 18:28 ..
> lrwxrwxrwx 1 root root 0 Jan  8 18:41 serial0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0
> lrwxrwxrwx 1 root root 0 Jan  8 18:41 serial0-0 -> ../../../devices/platform/soc@0/32f10108.usb/38200000.usb/xhci-hcd.1.auto/usb1/1-1/1-1.1/1-1.1:1.0/ttyUSB0/serial0/serial0-0
> 
> 8<----------------------------------------------------------------
> 
> > > 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.
> 
> Regards,
>   Marco
> 
> 
> > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/log/?h=usb-serial-of
> > > > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-serial-of&id=b19239022c92567a6a9ed40e8522e84972b0997f
> > 
> > Johan
> >