Re: [SPCR] mmio32 iotype access requirements for X-Gene 8250(_dw) UART

[Mark Salter <msalter@redhat.com>]

On Sat, 2016-12-03 at 05:06 -0500, Jon Masters wrote:
> Hi Duc, all,
> 
> (and changing the subject and trimming/adjusting the CC)
> 
> On 12/02/2016 02:39 PM, Duc Dang wrote:
> > 
> > On Fri, Dec 2, 2016 at 12:11 AM, Jon Masters <jcm@redhat.com> wrote:
> > > 
> > > You're welcome.
> > > 
> > > (Unrelated) Note that I added a console= and earlycon in my test (and
> > > got the baud rate wrong for the console but nevermind...was ssh'd in
> > > after the earlycon output I cared about anyway) because of some other
> > > cleanup work for the SPCR parsing that apparently is still not quite
> > > fixed for upstream, or rather, there is a need to match on the 32-bit
> > > access required for the UART and that isn't happening so it's not
> > > getting setup. Folks are tracking that one and fixing it though.
> > 
> > I don't see this console issue on X-Gene1 (Mustang board). I tried
> > with X-Gene 2 as well. I used both console=ttyS0,115200 and
> > earlycon=uart8250,mmio32,0x1c020000. Are you setting baudrate to
> > 115200 or something else?
> Let me clarify. What I meant above is that when I generated the boot
> log, I had specified earlycon and console parameters, but had fat
> fingered the baud rate (m400 uses 9600, mustang uses 115200 baud).
> That's what I was referring to in the original text above.
> 
> HOWEVER...
> 
> There is a broader problem with X-Gene SPCR support. The problem is
> that the 16550 UART in X-Gene requires the 32-bit access quirk (the
> iotype is set to UPIO_MEM32 for the APMC0D08 device). This means
> that when univ8250_console_match runs later, it will compare the
> iotype (MEM32) with the type previously registered with the
> kernel when the earlycon setup the preferred console.

Linaro has a kernel patch which looks at the bit_width field of the
port address:

Author: Aleksey Makarov <aleksey.makarov@linaro.org>
Date:   Thu Apr 28 19:52:38 2016 +0300

    serial: SPCR: check bit width for the 16550 UART

The SPCR in 3.06.25 firmware has a bit_width field set to 32 and with the
above patch, I don't need to use console=. But HP firmware on m400 sets
bit width to 8 so that needs a firmware fix to work with above.


> 
> The earlycon preferred console will parse the SPCR and find:
> 
>         iotype = table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY ?
>                         "mmio" : "io";
> 
> Which sets it to "mmio" (not "mmio32"). There is a DBG2 (the table
> referenced by the SPCR that provides the actual port types for all
> modern revisions of the SPCR with revision2+, as required by Linux)
> port subtype for non-compliant SBSA ARM Generic UARTs that require
> 32-bit accesses, and this is ACPI_DBG2_ARM_SBSA_32BIT (type 0xd).
> 
> However that only applies for "pl011" devices, and doesn't provide
> for 16550 UARTs that require 32-bit accessors.
> 
> So you'll end up with Linux thinking it's registering a non-32-bit
> mmio preferred console during earlycon setup and then never match
> against that later in the 8250_core/8250_dw match function by
> virtue of the fact that these differ.
> 
> There are a couple of possibilities:
> 
> 1). Perhaps (for some reason) the IP actually does support sub-32-bit
>     access and the iotype simply needs to be changed to reflect this.
>     That would be the easiest option. But it's been this way for a
>     long time in various codebases, so I would be pleasantly
>     surprised if this were the case. Let me/us know :)
> 
> 2). We find some way during SPCR setup to quirk for X-Gene as a non
>     standard 16550 and set it up as an mmio32 iotype.
> 
> 3). We get the DBG2 table updated to add a subtype of the 16650
>     called something like "(deprecated) 16550 subset supporting
>     only 32-bit accesses". Then we add this to Linux and get
>     the firmware updated on systems to switch to this type. We
>     would probably still want to quirk for existing machines.
> 
> Perhaps I'm missing something. I would love for that to be true,
> but I don't think it is. I think we need a subtype of the 16550
> defined that encapsulates this mmio32 requirement properly. To
> that end, I've preemptively asked some friends at MS for a
> favor to look into adding a new subtype for this.
> 
> Let me know what you think is the best path... :)
> 
> Thanks,
> 
> Jon.
>