Re: [PATCH] usb: xhci: dbc: get rid of global pointer

[Johan Hovold <johan@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2683 bytes --]

On Mon, Jun 17, 2019 at 09:43:19AM +0300, Felipe Balbi wrote:
> 
> Hi,
> 
> Johan Hovold <johan@kernel.org> writes:
> > On Tue, Jun 11, 2019 at 08:24:16PM +0300, Felipe Balbi wrote:
> >> If we happen to have two XHCI controllers with DbC capability, then
> >> there's no hope this will ever work as the global pointer will be
> >> overwritten by the controller that probes last.
> >> 
> >> Avoid this problem by keeping the tty_driver struct pointer inside
> >> struct xhci_dbc.
> >
> > How did you test this patch?
> 
> by running it on a machine that actually has two DbCs
> 
> >> @@ -279,52 +279,52 @@ static const struct tty_operations dbc_tty_ops = {
> >>  	.unthrottle		= dbc_tty_unthrottle,
> >>  };
> >>  
> >> -static struct tty_driver *dbc_tty_driver;
> >> -
> >>  int xhci_dbc_tty_register_driver(struct xhci_hcd *xhci)
> >>  {
> >>  	int			status;
> >>  	struct xhci_dbc		*dbc = xhci->dbc;
> >>  
> >> -	dbc_tty_driver = tty_alloc_driver(1, TTY_DRIVER_REAL_RAW |
> >> +	dbc->tty_driver = tty_alloc_driver(1, TTY_DRIVER_REAL_RAW |
> >>  					  TTY_DRIVER_DYNAMIC_DEV);
> >> -	if (IS_ERR(dbc_tty_driver)) {
> >> -		status = PTR_ERR(dbc_tty_driver);
> >> -		dbc_tty_driver = NULL;
> >> +	if (IS_ERR(dbc->tty_driver)) {
> >> +		status = PTR_ERR(dbc->tty_driver);
> >> +		dbc->tty_driver = NULL;
> >>  		return status;
> >>  	}
> >>  
> >> -	dbc_tty_driver->driver_name = "dbc_serial";
> >> -	dbc_tty_driver->name = "ttyDBC";
> >> +	dbc->tty_driver->driver_name = "dbc_serial";
> >> +	dbc->tty_driver->name = "ttyDBC";
> >
> > You're now registering multiple drivers for the same thing (and wasting
> > a major number for each) and specifically using the same name, which
> > should lead to name clashes when registering the second port.
> 
> No warnings were printed while running this, actually. Odd

Odd indeed. I get the expected warning from sysfs when trying to
register a second tty using an already registered name:

[  643.360555] sysfs: cannot create duplicate filename '/class/tty/ttyS0'
[  643.360637] CPU: 1 PID: 2383 Comm: modprobe Not tainted 5.2.0-rc1 #2
[  643.360702] Hardware name:  /D34010WYK, BIOS WYLPT10H.86A.0051.2019.0322.1320 03/22/2019
[  643.360784] Call Trace:
[  643.360823]  dump_stack+0x46/0x60
[  643.360865]  sysfs_warn_dup.cold.3+0x17/0x2f
[  643.360914]  sysfs_do_create_link_sd.isra.2+0xa6/0xc0
[  643.360961]  device_add+0x30d/0x660
[  643.360987]  tty_register_device_attr+0xdd/0x1d0
[  643.361018]  ? sysfs_create_file_ns+0x5d/0x90
[  643.361049]  usb_serial_device_probe+0x72/0xf0 [usbserial]
...

Are you sure you actually did register two xhci debug ttys?

Johan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]