[PATCH 03/17] x86: Allow coreboot serial driver to guess the UART

[Simon Glass]

Hi Bin,

On Tue, 4 May 2021 at 08:26, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin and Andy,
>
> On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko <andy.shevchenko at gmail.com> wrote:
> >
> > On Fri, Apr 30, 2021 at 9:14 PM Simon Glass <sjg at chromium.org> wrote:
> > > On Thu, 29 Apr 2021 at 17:01, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > On Fri, Apr 30, 2021 at 12:10 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > On Sun, 25 Apr 2021 at 18:21, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > On Sun, Apr 25, 2021 at 10:10 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > On Sun, 25 Apr 2021 at 13:49, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > At present this driver relies on coreboot to provide information about
> > > > > > > > > > > the console UART. However if coreboot is not compiled with the UART
> > > > > > > > > > > enabled, the information is left out. This configuration is quite
> > > > > > > > > > > common, e.g. with shipping x86-based Chrome OS Chromebooks.
> > > > > > > > > > >
> > > > > > > > > > > Add a way to determine the UART settings in this case, using a
> > > > > > > > > > > hard-coded list of PCI IDs.
> >
> > I don't like this either, so -1 from me.
> >
> > What coreboot should do is either provide serial information or SPCR ACPI table.
> > Otherwise if it does not provide it, I think it's on purpose, and we
> > have to respect this.
> >
> > The patch is ugly hack in my opinion. Sorry.
> >
> > You may make it less ugly by checking PCI class rather than IDs.
>
> I have not been able to distinguish a pattern on Intel SoCs yet.
> Perhaps you can help with that as there must be a way...
>
>
> >
> > > > > > > > > > Why not just simply put a serial node in the device tree and we are all done?
> > > > > > > > >
> > > > > > > > > See my other email...I am trying to make this boot on any board that
> > > > > > > > > coreboot supports.
> > > > > > > >
> > > > > > > > But this solution does not scale. One has to put all known UARTs into
> > > > > > > > serial_coreboot.c.
> > > > > > >
> > > > > > > Yes that's right...but this is only for when coreboot does not enable
> > > > > > > serial. Also the number of new platforms is not that great.
> > > > > > >
> > > > > > > >
> > > > > > > > Why not patch coreboot instead? Why coreboot does not provide a cbinfo
> > > > > > > > with serial?
> > > > > > >
> > > > > > > Because it does not even set up the serial device in that case, so
> > > > > > > doesn't know the details. The driver is completely missing.
> > > > > >
> > > > > > Sigh. Is it possible to upstream a patch to coreboot to enable that?
> > > > >
> > > > > Well I'm not even sure upstream coreboot boots on the various
> > > > > Chromebooks I am targetting. If it does, then serial is probablt
> > > > > enabled. But certainly for chromebooks, it is not. My goal is to have
> > > > > U-Boot boot on a chromebook in altfw mode with serial enabled.
> > > > >
> > > > > >
> > > > > > I don't like the current approach because it ends up duplicating all
> > > > > > UART IDs/info in C.
> > > > >
> > > > > Yes. Do you think it would be better to put it in the devicetree? I
> > > > > suppose we could add some more stuff to the compatible string,
> > > > > although U-Boot does not support the PCI compatible strings at
> > > > > present.
> > > >
> > > > Putting it in the device tree also looks odd, because it only matters
> > > > on a dedicated board.
> > >
> > > Right, but that is the nature of trying to run the same image on
> > > different hardware.
> > >
> > > >
> > > > > What do you suggest?
> > > >
> > > > Or parse the ACPI table coreboot has set up? But that might be another
> > > > huge monster :(
> > >
> > > Yes, even worse...
>
> All of the suggestions so far do not solve the problem, so we are left
> without serial output, or something else...?

In the interested of getting the patches applied, shall we just drop
this one? I will revisit it when I try the next platform and see if I
can find a different pattern.

Regards,
Simon