From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1oTJ6S-0007X8-C2 for mharc-grub-devel@gnu.org; Wed, 31 Aug 2022 04:29:40 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40202) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oTJ6Q-0007Wp-2N for grub-devel@gnu.org; Wed, 31 Aug 2022 04:29:38 -0400 Received: from gate.crashing.org ([63.228.1.57]:58907) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oTJ6O-0005dE-85 for grub-devel@gnu.org; Wed, 31 Aug 2022 04:29:37 -0400 Received: from [IPv6:::1] (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id 27V8RNqe013505; Wed, 31 Aug 2022 03:27:24 -0500 Message-ID: Subject: Re: [PATCH 3/5] ns8250: Add base support for MMIO UARTs From: Benjamin Herrenschmidt To: Sven Anderson Cc: Matthias Lange , The development of GNU GRUB Date: Wed, 31 Aug 2022 18:26:57 +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" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.44.1-0ubuntu1 MIME-Version: 1.0 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, 31 Aug 2022 08:29:38 -0000 On Wed, 2022-08-31 at 15:56 +1000, Benjamin Herrenschmidt wrote: > On Mon, 2022-05-23 at 19:11 +0200, Sven Anderson wrote: > >=20 > > I had a couple of sleepless nights trying to find out why this > > didn't > > work for my MMIO UART (Intel Cannon Lake PCH=C2=A0Intel=C2=A0C246), so = I > > thought I would share my findings with others in a > > similar=C2=A0situation. > > (See below.) > > =C2=A0 > =C2=A0.../... > >=20 > > 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=C2=A0and write full grub_uint32_t > > values, like this:=C2=A0 > >=20 > > ------ > > =C2=A0 volatile grub_uint32_t* p =3D (void*)(port->mmio_base); > > =C2=A0 *(p + reg) =3D (grub_uint32_t)(value); > > ------ >=20 > Sorry, I dopped the ball on this for a while. Getting back to it and > will re-submit the patches. >=20 > Can you send me a dump of your SPCR table ? I'd like to see if it > properly says "32-bit" there to auto-detect this. Does this help ? I'll resend the whole series after a bit more testing. >From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Benjamin Herrenschmidt Date: Wed, 31 Aug 2022 17:19:13 +1000 Subject: [PATCH] ns8250: Support more MMIO access sizes It is common for PCI based UARTs to use larger than one byte access sizes. This adds support for this and uses the information present in SPCR accordingly. Signed-off-by: Benjamin Herrenschmidt --- grub-core/term/ns8250-spcr.c | 3 ++- grub-core/term/ns8250.c | 45 ++++++++++++++++++++++++++++++++++++++++= +--- include/grub/serial.h | 4 +++- 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/grub-core/term/ns8250-spcr.c b/grub-core/term/ns8250-spcr.c index 0786aee1b..dd589af60 100644 --- a/grub-core/term/ns8250-spcr.c +++ b/grub-core/term/ns8250-spcr.c @@ -73,7 +73,8 @@ grub_ns8250_spcr_init (void) switch (spcr->base_addr.space_id) { case GRUB_ACPI_GENADDR_MEM_SPACE: - return grub_serial_ns8250_add_mmio(spcr->base_addr.addr, &config); + return grub_serial_ns8250_add_mmio(spcr->base_addr.addr, + spcr->base_addr.access_size, &c= onfig); case GRUB_ACPI_GENADDR_IO_SPACE: return grub_serial_ns8250_add_port(spcr->base_addr.addr, &config); default: diff --git a/grub-core/term/ns8250.c b/grub-core/term/ns8250.c index d783c2897..57d4d6d38 100644 --- a/grub-core/term/ns8250.c +++ b/grub-core/term/ns8250.c @@ -49,7 +49,26 @@ ns8250_reg_read (struct grub_serial_port *port, grub_add= r_t reg) { asm volatile("" : : : "memory"); if (port->mmio) - return *((volatile unsigned char *)(port->mmio_base + reg)); + { + /* Note: we assume MMIO UARTs are little endian. This is not true of= all + * embedded platforms but will do for now + */ + switch(port->access_size) + { + default: + case 1: + return *((volatile unsigned char *)(port->mmio_base + reg)); + case 2: + return grub_le_to_cpu16(*((volatile unsigned short *)(port->mmio= _base + (reg << 1)))); + case 3: + return grub_le_to_cpu32(*((volatile unsigned long *)(port->mmio_= base + (reg << 2)))); + case 4: + /* This will only work properly on 64-bit systems, thankfully it + * also probably doesn't exist + */ + return grub_le_to_cpu64(*((volatile unsigned long long *)(port->= mmio_base + (reg << 3)))); + } + } return grub_inb (port->port + reg); } =20 @@ -58,7 +77,24 @@ ns8250_reg_write (struct grub_serial_port *port, unsigne= d char value, grub_addr_ { asm volatile("" : : : "memory"); if (port->mmio) - *((volatile char *)(port->mmio_base + reg)) =3D value; + { + switch(port->access_size) + { + default: + case 1: + *((volatile unsigned char *)(port->mmio_base + reg)) =3D value; + break; + case 2: + *((volatile unsigned short *)(port->mmio_base + (reg << 1))) =3D= grub_cpu_to_le16(value); + break; + case 3: + *((volatile unsigned long *)(port->mmio_base + (reg << 2))) =3D = grub_cpu_to_le32(value); + break; + case 4: + *((volatile unsigned long long *)(port->mmio_base + (reg << 3)))= =3D grub_cpu_to_le64(value); + break; + } + } else grub_outb (value, port->port + reg); } @@ -283,6 +319,7 @@ grub_ns8250_init (void) grub_print_error (); =20 grub_serial_register (&com_ports[i]); + com_ports[i].access_size =3D 1; } } =20 @@ -333,6 +370,7 @@ grub_serial_ns8250_add_port (grub_port_t port, struct g= rub_serial_config *config p->driver =3D &grub_ns8250_driver; p->mmio =3D 0; p->port =3D port; + p->access_size =3D 1; if (config) grub_serial_port_configure (p, config); else @@ -343,7 +381,7 @@ grub_serial_ns8250_add_port (grub_port_t port, struct g= rub_serial_config *config } =20 char * -grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_config *c= onfig) +grub_serial_ns8250_add_mmio(grub_addr_t addr, unsigned int acc_size, struc= t grub_serial_config *config) { struct grub_serial_port *p; unsigned i; @@ -367,6 +405,7 @@ grub_serial_ns8250_add_mmio(grub_addr_t addr, struct gr= ub_serial_config *config) p->driver =3D &grub_ns8250_driver; p->mmio =3D 1; p->mmio_base =3D addr; + p->access_size =3D acc_size; if (config) grub_serial_port_configure (p, config); else diff --git a/include/grub/serial.h b/include/grub/serial.h index 588797768..56a80bb8b 100644 --- a/include/grub/serial.h +++ b/include/grub/serial.h @@ -94,6 +94,8 @@ struct grub_serial_port grub_port_t port; #endif grub_addr_t mmio_base; + /* Access size uses ACPI definition */ + unsigned int access_size; }; }; struct @@ -186,7 +188,7 @@ grub_serial_config_defaults (struct grub_serial_port *p= ort) void grub_ns8250_init (void); char * grub_ns8250_spcr_init (void); char *grub_serial_ns8250_add_port (grub_port_t port, struct grub_serial_co= nfig *config); -char *grub_serial_ns8250_add_mmio(grub_addr_t addr, struct grub_serial_con= fig *config); +char *grub_serial_ns8250_add_mmio(grub_addr_t addr, unsigned int acc_size,= struct grub_serial_config *config); #endif #ifdef GRUB_MACHINE_IEEE1275 void grub_ofserial_init (void);