grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/5] ns8250: Add base support for MMIO UARTs


From: Benjamin Herrenschmidt
Subject: Re: [PATCH 3/5] ns8250: Add base support for MMIO UARTs
Date: Wed, 25 May 2022 13:13:56 +1000
User-agent: Evolution 3.36.5-0ubuntu1

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 
> <benh@kernel.crashing.org>:
> > 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.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]