From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Thu, 29 Apr 2021 09:10:22 -0700 Subject: [PATCH 03/17] x86: Allow coreboot serial driver to guess the UART In-Reply-To: References: <20210407043228.2268429-1-sjg@chromium.org> <20210407163159.3.I967ea8c85e009f870c7aa944372d32c990f1b14a@changeid> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Bin, On Sun, 25 Apr 2021 at 18:21, Bin Meng wrote: > > Hi Simon, > > On Sun, Apr 25, 2021 at 10:10 AM Simon Glass wrote: > > > > Hi Bin, > > > > On Sun, 25 Apr 2021 at 13:49, Bin Meng wrote: > > > > > > Hi Simon, > > > > > > On Sat, Apr 24, 2021 at 12:56 PM Simon Glass wrote: > > > > > > > > Hi Bin, > > > > > > > > On Thu, 8 Apr 2021 at 14:23, Bin Meng wrote: > > > > > > > > > > Hi Simon, > > > > > > > > > > On Wed, Apr 7, 2021 at 12:32 PM Simon Glass 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. > > > > > > > > > > > > Signed-off-by: Simon Glass > > > > > > --- > > > > > > > > > > > > drivers/serial/serial_coreboot.c | 68 ++++++++++++++++++++++++++++---- > > > > > > include/pci_ids.h | 1 + > > > > > > 2 files changed, 61 insertions(+), 8 deletions(-) > > > > > > > > > > > > diff --git a/drivers/serial/serial_coreboot.c b/drivers/serial/serial_coreboot.c > > > > > > index de09c8681f5..4b4619432d8 100644 > > > > > > --- a/drivers/serial/serial_coreboot.c > > > > > > +++ b/drivers/serial/serial_coreboot.c > > > > > > @@ -11,19 +11,71 @@ > > > > > > #include > > > > > > #include > > > > > > > > > > > > +static const struct pci_device_id ids[] = { > > > > > > + { PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_APL_UART2) }, > > > > > > + {}, > > > > > > +}; > > > > > > + > > > > > > +/* > > > > > > + * Coreboot only sets up the UART if it uses it and doesn't bother to put the > > > > > > + * details in sysinfo if it doesn't. Try to guess in that case, using devices > > > > > > + * we know about > > > > > > + * > > > > > > + * @plat: Platform data to fill in > > > > > > + * @return 0 if found, -ve if no UART was found > > > > > > + */ > > > > > > +static int guess_uart(struct ns16550_plat *plat) > > > > > > > > > > This is really not a guess, but use a pre-configured platform data. > > > > > Also this only work for Apollo Lake board, and will break other boards > > > > > if they don't have cbinfo available. > > > > > > > > Which bit of it breaks other boards? > > > > > > I see it does not return the error code to the caller, that's okay ... > > > > > > > > > > > > > > > > > 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. What do you suggest? Regards, Simon