Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932345AbcLMGUM (ORCPT ); Tue, 13 Dec 2016 01:20:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33130 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751269AbcLMGUK (ORCPT ); Tue, 13 Dec 2016 01:20:10 -0500 Subject: Re: [PATCH] SPCR: check bit width for the 16550 UART To: Mark Salter , Duc Dang References: <20161205130534.11080-1-aleksey.makarov@linaro.org> <3a6193ef-ca86-c113-b09a-5d6c882137e2@redhat.com> <53421849-d031-77e1-9edb-53c9d673d462@redhat.com> <1481124199.4751.50.camel@redhat.com> Cc: Aleksey Makarov , "Rafael J . Wysocki" , linux-acpi@vger.kernel.org, linux-serial@vger.kernel.org, Linux Kernel Mailing List , Greg Kroah-Hartman , Russell King , Peter Hurley , Graeme Gregory , Len Brown From: Jon Masters Message-ID: <3a27e455-dc5f-4fee-aa72-4eab752e91d0@redhat.com> Date: Tue, 13 Dec 2016 01:20:05 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <1481124199.4751.50.camel@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 13 Dec 2016 06:20:09 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3910 Lines: 81 On 12/07/2016 10:23 AM, Mark Salter wrote: > On Tue, 2016-12-06 at 01:34 -0500, Jon Masters wrote: >> On 12/05/2016 10:55 PM, Duc Dang wrote: >>> On Mon, Dec 5, 2016 at 6:27 PM, Jon Masters wrote: >>>> HOWEVER while the console does come up, the use of "earlycon" on the >>>> command line (with no parameters) doesn't result in the early SPCR >>>> console coming up correctly. I see some garbled characters that >>>> suggest the baud (or register access width) is off somewhere. >>> My bad that I did not catch this in the morning. Yes, earlycon does >>> not seems to work as expected. I can see that earlycon parameters >>> seems to be correct, but the bootconsole message does not come out >>> (the following is from 'dmesg') >>> >>> [ 0.000000] ACPI: SPCR: console: uart,mmio32,0x1c020000,115200 >>> [ 0.000000] earlycon: uart0 at MMIO32 0x000000001c020000 (options '115200') >>> [ 0.000000] bootconsole [uart0] enabled >> The difference appears to be in the baud rate. When I explicitly specify >> "earlycon=uart8250,mmio32,0x1c021000" no baud is set. When booting with >> the SPCR, the baud is set to 9600 in my case or 115200 in yours. But we >> both know that the base clock on the X-Gene UART is weird. Maybe >> somehow we can explain this through the lack of setting a baud. > > BTW, this behavior is same with devicetree. At least it's consistent :) > If you specify a baudrate with earlycon=, the driver tries to set that > baudrate and if you have an 8250 with some non-standard baud clock, then > it will fail. Perhaps SPCR shouldn't pass baud option to setup_earlycon(). Yet they seem to explicitly want to do this...in my conversations with some others we agree that, in many cases, you really want to say "leave the baud whatever the firmware set it", which would work in this case, but might break some others. Then again, nobody on x86 Linux is really using the SPCR today due to it not having been something they used until now and due to the location of the COM ports being fairly well known ;) So who knows what folks will prefer, but we should at least get the spec to cover both situations by explicitly calling out Applied as special. > Then again, setup_earlycon() has this comment: > > * setup_earlycon - match and register earlycon console > * @buf: earlycon param string > * > * Registers the earlycon console matching the earlycon specified > * in the param string @buf. Acceptable param strings are of the form > * ,io|mmio|mmio32|mmio32be,, > * ,0x, > * , > * > * > * Only for the third form does the earlycon setup() method receive the > * string in the 'options' parameter; all other forms set > * the parameter to NULL. > > That part about the 3rd form doesn't seem to be true. I don't see where > options gets set to NULL for forms other than the third. I saw that also, and agree that it appears totally bogus. We'll want to get the documentation fixed as part of this cleanup. So I've been discussing some changes to the SPCR and the current proposal is that we have two new subtypes - one for 16550s that are non-standard register width/stride but use the typical base clock, and a specific additional type for SBSA level 0 compatible 16550 UARTs (Applied). I will followup when the specification document has been revised. Jon. P.S. I had a lot of fun over my birthday reading the original 8250 spec and learning about how the DLAB stuff works (I think those brain cells had died). Which - on a total tangent - finally helps with a long standing issue we have had. We (a small cabal) have wanted to sneak in an instruction into the ARM ISA or specs referring to DLAB (the initials of a certain person who owns the ARM ARM) and now we will get to do this into the SBBR ;) -- Computer Architect | Sent from my Fedora powered laptop