linux-kernel - Re: [PATCH v2] earlycon: Allow specifying a uartclk in options

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE2855vbQyMwu8pmH-Sq1kA+=h5TJPWddTWPQoVdEqOn6sAyBw@mail.gmail.com>
Date:   Thu, 1 Mar 2018 14:24:08 -0700
From:   Aaron Durbin <adurbin@...omium.org>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Daniel Kurtz <djkurtz@...omium.org>,
        Aaron Durbin <adurbin@...omium.org>,
        Brian Norris <briannorris@...omium.org>,
        Jonathan Corbet <corbet@....net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jslaby@...e.com>, Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Christoffer Dall <cdall@...aro.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        Marc Zyngier <marc.zyngier@....com>,
        Frederic Weisbecker <frederic@...nel.org>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Tom Saeger <tom.saeger@...cle.com>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        "Levin, Alexander (Sasha Levin)" <alexander.levin@...izon.com>,
        Linux Documentation List <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:SERIAL DRIVERS" <linux-serial@...r.kernel.org>
Subject: Re: [PATCH v2] earlycon: Allow specifying a uartclk in options

On Thu, Mar 1, 2018 at 1:02 PM, Andy Shevchenko
<andy.shevchenko@...il.com> wrote:
> On Thu, Mar 1, 2018 at 9:22 PM, Daniel Kurtz <djkurtz@...omium.org> wrote:
>> On Thu, Mar 1, 2018 at 11:47 AM Andy Shevchenko <andy.shevchenko@...il.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 would look to this later. It's late here. I need to do a bit of
> research for the answer.
>
>> I see that support was also added recently to earlycon to let it use ACPI
>> SPCR to choose a console and configure its parameters... but AFAICT, this
>> path also doesn't allow specifying the uart clock.
>
> Fix your firmware then. It should set console to 115200 like (almost)
> everyone does.

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

> 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.

>
> --
> With Best Regards,
> Andy Shevchenko