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

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: u-boot@lists.denx.de
Subject: [PATCH 03/17] x86: Allow coreboot serial driver to guess the UART
Date: Tue, 4 May 2021 19:48:35 +0300	[thread overview]
Message-ID: <YJF64x3Rng1qO3n2@smile.fi.intel.com> (raw)
In-Reply-To: <CAPnjgZ2-iEjiy8v0Vn7=56KoaBjN0x-tHWpegaOwj5U8bEM86Q@mail.gmail.com>

On Tue, May 04, 2021 at 09:26:19AM -0600, Simon Glass wrote:
> On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Apr 30, 2021 at 9:14 PM Simon Glass <sjg@chromium.org> wrote:
> > > On Thu, 29 Apr 2021 at 17:01, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > On Fri, Apr 30, 2021 at 12:10 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > On Sun, 25 Apr 2021 at 18:21, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > On Sun, Apr 25, 2021 at 10:10 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > On Sun, 25 Apr 2021 at 13:49, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass <sjg@chromium.org> wrote:
> > > > > > > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass <sjg@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...?

You are truing to solve Coreboot issue in U-Boot, doesn't seem to be quite
right.

Yes, I prefer no serial than some "smart" hack. It will encourage people who
want to have serial out of Coreboot to whine to coreboot project and see what
they will do about it.

-- 
With Best Regards,
Andy Shevchenko