From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1nthVP-0001C3-Ph for mharc-grub-devel@gnu.org; Tue, 24 May 2022 23:16:15 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60586) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nthVN-00019P-PU for grub-devel@gnu.org; Tue, 24 May 2022 23:16:13 -0400 Received: from gate.crashing.org ([63.228.1.57]:41025) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nthVL-0007nI-C4 for grub-devel@gnu.org; Tue, 24 May 2022 23:16:13 -0400 Received: from ip6-localhost (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 24P3Duef012303; Tue, 24 May 2022 22:13:58 -0500 Message-ID: <07634cfd8ac9b7de6a72b96b7662cf1df2f52ee4.camel@kernel.crashing.org> Subject: Re: [PATCH 3/5] ns8250: Add base support for MMIO UARTs From: Benjamin Herrenschmidt To: Sven Anderson Cc: grub-devel@gnu.org, Matthias Lange Date: Wed, 25 May 2022 13:13:56 +1000 In-Reply-To: References: <20210318220728.495970-1-benh@kernel.crashing.org> <20210318220728.495970-4-benh@kernel.crashing.org> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5-0ubuntu1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=63.228.1.57; envelope-from=benh@kernel.crashing.org; helo=gate.crashing.org X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 May 2022 03:16:14 -0000 On Mon, 2022-05-23 at 19:11 +0200, Sven Anderson wrote: > Hi all, > > Am Do., 18. März 2021 um 23:08 Uhr schrieb Benjamin Herrenschmidt : > > This adds the ability for the driver to access UARTs via MMIO instead > > of PIO selectively at runtime, and exposes a new function to add an > > MMIO port. > > I had a couple of sleepless nights trying to find out why this didn't > work for my MMIO UART (Intel Cannon Lake PCH Intel C246), so I > thought I would share my findings with others in a similar situation. > (See below.) Thanks ! I'm overdue to re-submit those patches :-) > > diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c > > index 39809d042..183e14b3b 100644 > > --- a/grub-core/term/ns8250.c > > +++ b/grub-core/term/ns8250.c > > @@ -44,6 +44,24 @@ static int dead_ports = 0; > > #define DEFAULT_BASE_CLOCK 115200 > > #endif > > > > +static inline unsigned char > > +ns8250_reg_read (struct grub_serial_port *port, grub_addr_t reg) > > +{ > > + asm volatile("" : : : "memory"); > > + if (port->mmio) > > + return *((volatile unsigned char *)(port->mmio_base + reg)); > > + return grub_inb (port->port + reg); > > +} > > + > > +static inline void > > +ns8250_reg_write (struct grub_serial_port *port, unsigned char value, grub_addr_t reg) > > +{ > > + asm volatile("" : : : "memory"); > > + if (port->mmio) > > + *((volatile char *)(port->mmio_base + reg)) = value; > > + else > > + grub_outb (value, port->port + reg); > > +} > > This code assumes that the registers are only 8 bit wide. Apparently > for my chipset they are 32 bits wide, so it only (finally) worked > when I rewrote this code to address and write full grub_uint32_t > values, like this: Ah yes, there are all sort of "interesting" variations of MMIO UART layout and my patch didn't add >8 bits as I didn't need it. The information is available via SPCR though, so when I finally get a chance to respin this series, I'll try to add something and ask you to test if you don't mind ? PS. I'm quite swamped this week, in absence of news from me in a couple of weeks, feel free to poke for a heads up ! Cheers, Ben.