On Fri, Mar 2, 2018 at 8:35 PM, Daniel Kurtz <djkurtz@xxxxxxxxxxxx> wrote: > On Thu, Mar 1, 2018 at 1:02 PM Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > wrote: >> On Thu, Mar 1, 2018 at 9:22 PM, Daniel Kurtz <djkurtz@xxxxxxxxxxxx> wrote: >> > On Thu, Mar 1, 2018 at 11:47 AM Andy Shevchenko < > andy.shevchenko@xxxxxxxxx> >> > wrote: > 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. > 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. 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. >> > 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. It does specify baudrate. It means it's _firmware_ responsibility to configure UART device properly. >> 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html