Re: [PATCH v5 1/9] drivers/char: separate dbgp=xhci to dbc=xhci option

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Thu, Aug 25, 2022 at 05:44:54PM +0200, Jan Beulich wrote:
> On 22.08.2022 17:27, Marek Marczykowski-Górecki wrote:
> > This allows configuring EHCI and XHCI consoles separately,
> > simultaneously.
> > 
> > Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> But was I maybe confused, and much less of a change would suffice? After
> all ...
> 
> > --- a/xen/drivers/char/xhci-dbc.c
> > +++ b/xen/drivers/char/xhci-dbc.c
> > @@ -1058,9 +1058,9 @@ static struct xhci_dbc_ctx ctx __aligned(16);
> >  static uint8_t out_wrk_buf[DBC_WORK_RING_CAP];
> >  static struct xhci_string_descriptor str_buf[DBC_STRINGS_COUNT];
> >  
> > -static char __initdata opt_dbgp[30];
> > +static char __initdata opt_dbc[30];
> >  
> > -string_param("dbgp", opt_dbgp);
> > +string_param("dbc", opt_dbc);
> >  
> >  void __init xhci_dbc_uart_init(void)
> >  {
> > @@ -1068,25 +1068,25 @@ void __init xhci_dbc_uart_init(void)
> >      struct dbc *dbc = &uart->dbc;
> >      const char *e;
> >  
> > -    if ( strncmp(opt_dbgp, "xhci", 4) )
> > +    if ( strncmp(opt_dbc, "xhci", 4) )
> >          return;
> 
> ... this already avoids mixing up who's going to parse what. So right
> now I think that ...
> 
> > @@ -1102,7 +1102,7 @@ void __init xhci_dbc_uart_init(void)
> >      dbc->dbc_str = str_buf;
> >  
> >      if ( dbc_open(dbc) )
> > -        serial_register_uart(SERHND_DBGP, &dbc_uart_driver, &dbc_uart);
> > +        serial_register_uart(SERHND_DBC, &dbc_uart_driver, &dbc_uart);
> >  }
> 
> ... this and other SERHND_* related changes are enough, and there's no
> need for a separate "dbc=" option.

But then you wouldn't be able to configure "dbgp=ehci dbgp=xhci" as
one would override the other, no?

> 
> > --- a/xen/include/xen/serial.h
> > +++ b/xen/include/xen/serial.h
> > @@ -95,6 +95,7 @@ struct uart_driver {
> >  # define SERHND_COM1    (0<<0)
> >  # define SERHND_COM2    (1<<0)
> >  # define SERHND_DBGP    (2<<0)
> > +# define SERHND_DBC     (3<<0)
> 
> Please also update the comment just out of context.
> 
> Jan

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature