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
>
