Hi Sean, Am Fr., 18. März 2022 um 03:19 Uhr schrieb Sean Anderson <sean...@gmail.com>: > > On 3/17/22 3:14 PM, Heiko Thiery wrote: > > Hi Sean, > > > > Am Do., 17. März 2022 um 15:38 Uhr schrieb Sean Anderson > > <sean...@gmail.com>: > >> > >> Hi Heiko, > >> > >> On 3/17/22 8:41 AM, Heiko Thiery wrote: > >>> With the clock driver enabled for the imx8mq, it was noticed that the > >>> frequency used to calculate the baud rate is always taken from the root > >>> clock of UART1. This can cause problems if UART1 is not used as console > >>> and the settings are different from UART1. The result is that the console > >>> output is garbage. To do this correctly the UART frequency is taken from > >>> the used device. For the implementations that don't have the igp clock > >>> frequency written or can't return it the old way is tried. > >>> > >>> Signed-off-by: Heiko Thiery <heiko.thi...@gmail.com> > >>> --- > >>> drivers/serial/serial_mxc.c | 15 +++++++++++++-- > >>> 1 file changed, 13 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c > >>> index e4970a169b..6fdb2b2397 100644 > >>> --- a/drivers/serial/serial_mxc.c > >>> +++ b/drivers/serial/serial_mxc.c > >>> @@ -3,6 +3,7 @@ > >>> * (c) 2007 Sascha Hauer <s.ha...@pengutronix.de> > >>> */ > >>> > >>> +#include <clk.h> > >>> #include <common.h> > >>> #include <dm.h> > >>> #include <errno.h> > >>> @@ -266,9 +267,19 @@ __weak struct serial_device > >>> *default_serial_console(void) > >>> int mxc_serial_setbrg(struct udevice *dev, int baudrate) > >>> { > >>> struct mxc_serial_plat *plat = dev_get_plat(dev); > >>> - u32 clk = imx_get_uartclk(); > >>> + u32 rate = 0; > >>> + > >>> + if (IS_ENABLED(CONFIG_CLK)) { > >> > >> CONFIG_IS_ENABLED? > > > > This should be correct. The CONFIG_IS_ENABLED is a preprocessor macro > > and the IS_ENABLED can be used in c code. Or do you mean something > > else? > > I mean you should be using CONFIG_IS_ENABLED(CLK) so that this code is > correct for both SPL and U-Boot proper. But it is also fine to "try and > fail" (making this check unnecessary). > > >> > >> mx6ull at least does not have CONFIG_SPL_CLK enabled. > > > > I expect that in that case it will fallback to the old behavior ... not? > > Yes, if you handle -ENOSYS correctly. > > >> > >>> + struct clk clk; > >>> + if(!clk_get_by_name(dev, "ipg", &clk)) > >>> + rate = clk_get_rate(&clk); > > You may also need to enable this clock.
What would be the right place to enable the clock? in mxc_serial_probe()? > > >>> + } > >>> + > >>> + /* as fallback we try to get the clk rate that way */ > >>> + if (rate == 0) > >> > >> !rate || IS_ERR_VALUE(rate) > >> > >>> + rate = imx_get_uartclk(); > >>> > >>> - _mxc_serial_setbrg(plat->reg, clk, baudrate, plat->use_dte); > >>> + _mxc_serial_setbrg(plat->reg, rate, baudrate, plat->use_dte); > >>> > >>> return 0; > >>> } > >>> > >> --Sean > > > >