[U-Boot] [PATCH v2 5/5] x86: acpi: Generate SPCR table

Andy Shevchenko andy.shevchenko at gmail.com
Thu Nov 15 19:43:21 UTC 2018


On Thu, Nov 15, 2018 at 8:27 PM Alexander Graf <agraf at suse.de> wrote:
> On 15.11.18 18:58, Andy Shevchenko wrote:
> > Microsoft specifies a SPCR (Serial Port Console Redirection Table) [1].
> > Let's provide it in U-Boot.
> >
> > [1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx

> > +     ret = serial_getinfo(&info);
> > +     if (ret)
> > +             spcr->interface_type = ACPI_DBG2_UNKNOWN;
> > +     else
> > +             spcr->interface_type = ACPI_DBG2_16550_COMPATIBLE;
>
> This sounds like pretty subtle semantics. Could you just include the
> interface type in &info?

It might be another step if you provide a cool way to do so. Otherwise
we need first to de-x86 acpi stuff. I won't do it right now since it
so-o out of the scope of this series.

> The main problem I'm seeing is that your UART might be a PL011
> compatible which could still implement getinfo() but then wouldn't be
> 16660 compatible.

Yes, I know. Perhaps I forgot to mark it in "Known issues" section.

Have you seen, btw, PL011 on x86 environment?

> Are any higher (and lower) ones specified too? If so, you may want to
> add them as well.

There is a link to a specification. Care to read?

> > +     default:
> > +             spcr->baud_rate = 0;
> > +             break;

Going ahead.
This part actually something to develop through specifications, but I
need to ping Microsoft and our internal ACPI people for that.

> > +             break;
> > +     default:
> > +             access_size = ACPI_ACCESS_SIZE_UNDEFINED;
> > +             break;
> > +     }
> > +
> > +     spcr->serial_port.space_id = ACPI_ADDRESS_SPACE_MEMORY;
>
> Can you guarantee that? Should probably be part of &info as well if addr is.

Not now.
As per moving to info, see my comment about de-x86 of ACPI in U-Boot.

> > +     /* REVISIT: Hard coded values for now */
> > +     spcr->parity = 0;
> > +     spcr->stop_bits = 1;
>
> IMHO those should be part of &info as well. If you have to hard code
> them, better hard code them in the driver rather than the generic ACPI
> generation code.

Care to read cover letter carefully?

-- 
With Best Regards,
Andy Shevchenko


More information about the U-Boot mailing list