Re: [PATCH v2 3/9] xue: add support for selecting specific xhci

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.07.2022 17:32, Marek Marczykowski-Górecki wrote:
> Handle parameters similar to dbgp=ehci.
> 
> Implement this by not resettting xhc_cf8 again in xue_init_xhc(), but
> using a value found there if non-zero. Additionally, add xue->xhc_num to
> select n-th controller.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
preferably with two adjustments:

> @@ -238,24 +239,35 @@ static bool __init xue_init_xhc(struct xue *xue)
>      uint64_t bar1;
>      uint64_t devfn;
>  
> -    /*
> -     * Search PCI bus 0 for the xHC. All the host controllers supported so 
> far
> -     * are part of the chipset and are on bus 0.
> -     */
> -    for ( devfn = 0; devfn < 256; devfn++ )
> +    if ( xue->sbdf.sbdf == 0 )
>      {
> -        pci_sbdf_t sbdf = PCI_SBDF(0, 0, devfn);
> -        uint32_t hdr = pci_conf_read8(sbdf, PCI_HEADER_TYPE);
> -
> -        if ( hdr == 0 || hdr == 0x80 )
> +        /*
> +         * Search PCI bus 0 for the xHC. All the host controllers supported 
> so far
> +         * are part of the chipset and are on bus 0.
> +         */
> +        for ( devfn = 0; devfn < 256; devfn++ )
>          {
> -            if ( (pci_conf_read32(sbdf, PCI_CLASS_REVISION) >> 8) == 
> XUE_XHC_CLASSC )
> +            pci_sbdf_t sbdf = PCI_SBDF(0, 0, devfn);
> +            uint32_t hdr = pci_conf_read8(sbdf, PCI_HEADER_TYPE);

Already in the original code: Why uint32_t? If anything, uint8_t, but
according to ./CODING_STYLE unsigned int might be even better.

> @@ -955,12 +967,29 @@ void __init xue_uart_init(void)
>  {
>      struct xue_uart *uart = &xue_uart;
>      struct xue *xue = &uart->xue;
> +    const char *e;
>  
>      if ( strncmp(opt_dbgp, "xue", 3) )
>          return;
>  
>      memset(xue, 0, sizeof(*xue));
>  
> +    if ( isdigit(opt_dbgp[3]) || !opt_dbgp[3] )

I don't think you need the right side here, nor ...

> +    {
> +        if ( opt_dbgp[3] )

... this inner conditional.

Jan

> +            xue->xhc_num = simple_strtoul(opt_dbgp + 3, &e, 10);
> +    }
> +    else if ( strncmp(opt_dbgp + 3, "@pci", 4) == 0 )
> +    {
> +        unsigned int bus, slot, func;
> +
> +        e = parse_pci(opt_dbgp + 7, NULL, &bus, &slot, &func);
> +        if ( !e || *e )
> +            return;
> +
> +        xue->sbdf = PCI_SBDF(0, bus, slot, func);
> +    }
> +
>      xue->dbc_ctx = &ctx;
>      xue->dbc_erst = &erst;
>      xue->dbc_ering.trb = evt_trb;