Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <CAE2855vbQyMwu8pmH-Sq1kA+=h5TJPWddTWPQoVdEqOn6sAyBw@mail.gmail.com>
References: <20180301184335.248378-1-djkurtz@chromium.org> <CAHp75VfHuNHnjMqsvLZPSNeP57YX1CDZgf=EH-JNSh5bAVdUBA@mail.gmail.com>
 <CAGS+omCfVUPb0yq3XOEqO_zQ0_uppkKaGhqtB0OqXmQDK9xH1Q@mail.gmail.com>
 <CAHp75VfL82ujwBa1h0B8nEgBtFcBJPmS9ir7H5YWByDgxFGskA@mail.gmail.com> <CAE2855vbQyMwu8pmH-Sq1kA+=h5TJPWddTWPQoVdEqOn6sAyBw@mail.gmail.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Sat, 3 Mar 2018 17:38:43 +0200
Message-ID: <CAHp75Vcnuc48xb167hu5nz3iv9uSp2jRw+M-w=yBihex6RTXYQ@mail.gmail.com>
Subject: Re: [PATCH v2] earlycon: Allow specifying a uartclk in options
To:     Aaron Durbin <adurbin@chromium.org>
Cc:     Daniel Kurtz <djkurtz@chromium.org>,
        Brian Norris <briannorris@chromium.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Jiri Slaby <jslaby@suse.com>, Ingo Molnar <mingo@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Christoffer Dall <cdall@linaro.org>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        Marc Zyngier <marc.zyngier@arm.com>,
        Frederic Weisbecker <frederic@kernel.org>,
        David Woodhouse <dwmw@amazon.co.uk>,
        Tom Saeger <tom.saeger@oracle.com>,
        Mimi Zohar <zohar@linux.vnet.ibm.com>,
        "Levin, Alexander (Sasha Levin)" <alexander.levin@verizon.com>,
        Linux Documentation List <linux-doc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Mar 1, 2018 at 11:24 PM, Aaron Durbin <adurbin@chromium.org> wrote:
> On Thu, Mar 1, 2018 at 1:02 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Thu, Mar 1, 2018 at 9:22 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>>> On Thu, Mar 1, 2018 at 11:47 AM Andy Shevchenko <andy.shevchenko@gmail.com>
>>> wrote:
>>
>>> "earlycon simply does not utilize the information".
>>>
>>> earlycon parses iotype, mapbase and baud (from options).  However, it is
>>> hard-coded to assume that the clock used to generate the UART bitclock is
>>> always "BASE_BAUD * 16" (1843200).  While this may be true for many UARTs,
>>> it isn't true for AMD's CZ/ST which has a 8250_dw and uses a fixed 48 MHz
>>> clock.  The main 8250_dw driver uses devm_clk_get to get the "baudclk" and
>>> uses its rate to initialize uartclk.  For AMD CZ/ST, this "baudclk" is
>>> actually a set up in acpi_apd.c when there is an acpi match for "AMD0020",
>>> with a rate read from the .fixed_clk_rate param of the corresponding
>>> apd_device_desc.
>>>
>>> This patch attempts to add a way to inform earlycon about this clock.  As
>>> noted above, the information is actually already in the kernel and used by
>>> 8250_dw - I would happy be to hear recommendations for wiring this data
>>> into earlycon that doesn't require adding another command line arg.
>>
>> And it should not require that for sure!
>
> But it does require that. There's an input clock to the uart ip block.
> That is a design constraint by the hardware and is required to make
> baud calculation work.

I mean it should not be user's headache to provide this information to
the system.

> It's not a firmware problem.

If it's ACPI, then it's definitely firmware issue, since SPCR provides
a baudrate.

> Its the driver's problem in that it
> assumes an input clock to the uart block that does not reflect
> reality.

So, driver can't get this info from device tree or what?

>> Okay, configures a necessary IPs to feed UART with expected 1.8432M clock.
>
> That's only possible if there is a clock divider on the front end of
> the uart block. For this hardware that's not the case. I actually did
> this very thing on intel chromebook devices, but it was only possible
> because there was a hardware divider that could be tuned to reach the
> assumed clock that the code currently assumes.

OK.

-- 
With Best Regards,
Andy Shevchenko