Re: [RFC] serial: mxc: get the clock frequency from the used clock for the device

[Sean Anderson]

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.

+     }
+
+     /* 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