Hi Bin, On Fri, 7 May 2021 at 18:48, Bin Meng <[email protected]> wrote: > > Hi Simon, > > On Sat, May 8, 2021 at 9:42 AM Simon Glass <[email protected]> wrote: > > > > Hi Bin, > > > > On Tue, 4 May 2021 at 08:26, Simon Glass <[email protected]> wrote: > > > > > > Hi Bin and Andy, > > > > > > On Fri, 30 Apr 2021 at 12:41, Andy Shevchenko <[email protected]> > > > wrote: > > > > > > > > On Fri, Apr 30, 2021 at 9:14 PM Simon Glass <[email protected]> wrote: > > > > > On Thu, 29 Apr 2021 at 17:01, Bin Meng <[email protected]> wrote: > > > > > > On Fri, Apr 30, 2021 at 12:10 AM Simon Glass <[email protected]> > > > > > > wrote: > > > > > > > On Sun, 25 Apr 2021 at 18:21, Bin Meng <[email protected]> wrote: > > > > > > > > On Sun, Apr 25, 2021 at 10:10 AM Simon Glass > > > > > > > > <[email protected]> wrote: > > > > > > > > > On Sun, 25 Apr 2021 at 13:49, Bin Meng <[email protected]> > > > > > > > > > wrote: > > > > > > > > > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass > > > > > > > > > > <[email protected]> wrote: > > > > > > > > > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng > > > > > > > > > > > <[email protected]> wrote: > > > > > > > > > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass > > > > > > > > > > > > <[email protected]> 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. > > I think we should drop this patch for now, and apply other patches in > this series. But I suspect the rest patches cannot be applied cleanly > if dropping this patch.
That's OK with me. I did a little test and it applies cleanly without this patch. If you have trouble I can resend it. Regards, Simon
