On Fri, Mar 2, 2018 at 8:35 PM, Daniel Kurtz [off-list ref] wrote:

On Thu, Mar 1, 2018 at 1:02 PM Andy Shevchenko [off-list ref]
wrote:

quoted

On Thu, Mar 1, 2018 at 9:22 PM, Daniel Kurtz [off-list ref] wrote:

quoted

On Thu, Mar 1, 2018 at 11:47 AM Andy Shevchenko <

andy.shevchenko@gmail.com>

quoted

quoted

wrote:

the UART bitclock
is

quoted

quoted

always "BASE_BAUD * 16" (1843200).  While this may be true for many

UARTs,

quoted

quoted

it isn't true for AMD's CZ/ST which has a 8250_dw and uses a fixed 48

MHz

quoted

quoted

clock.  The main 8250_dw driver uses devm_clk_get to get the "baudclk"

and

quoted

quoted

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",

quoted

quoted

with a rate read from the .fixed_clk_rate param of the corresponding
apd_device_desc.

As

quoted

quoted

noted above, the information is actually already in the kernel and used

by

quoted

quoted

8250_dw - I would happy be to hear recommendations for wiring this data
into earlycon that doesn't require adding another command line arg.

Brief look at the code shows that ->setup() call back is executed
after setting initial (which is hardcoded) clock.

What you need is to either create another type of earlycon for your
device with accompanied ->setup() callback, or patch 8250_early.c.

quoted

quoted

I see that support was also added recently to earlycon to let it use

ACPI

quoted

quoted

SPCR to choose a console and configure its parameters... but AFAICT,

this

quoted

quoted

path also doesn't allow specifying the uart clock.

It does specify baudrate. It means it's _firmware_ responsibility to
configure UART device properly.

quoted

Fix your firmware then. It should set console to 115200 like (almost)
everyone does.
Okay, configures a necessary IPs to feed UART with expected 1.8432M clock.

The console is 115200 when it is enabled.  However, the firmware does not
always enable it by default.

Another firmware bug.

The problem is that the UART IP block has a fixed 48 MHz input clock, but
earlycon assumes this clock is always 1843200.

I looked a bit further, and I think this patch (or something similar) is
still required to teach generic earlycon how to handle an explicit
port->uartclk (ie, one that is not 1843200).
The extended string can then be explicitly set on the kernel command line
for this kind of hardware.

No.

In addition, we can add another patch with a new quirk detector in
drivers/acpi/spcr.c:acpi_parse_spcr() to handle this hardware.
acpi_parse_spcr() can then use the extended option string to pass in the
appropriate UART clock to setup_eralycon().

Definitely no. It's not defined in SPCR spec.

This would again allow a user to just use the simple command line parameter
"earlycon" if the device's firmware has a correctly confiured ACPI SPCR
table.

NAK to the patch, see above alternatives.

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html