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

[Angus Ainslie <angus@akkea.ca>]

Hi Heiko,

On 2022-03-17 05:41, 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.thiery@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.hauer@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)) {
> +		struct clk clk;
> +		if(!clk_get_by_name(dev, "ipg", &clk))
> +			rate = clk_get_rate(&clk);

Is the "ipg" clock the correct name for all of the imx DM boards ?

> +	}
> +
> +	/* as fallback we try to get the clk rate that way */
> +	if (rate == 0)
> +		rate = imx_get_uartclk();

Would it be better to re-write imx_get_uartclk so that both the getting 
and setting of clocks was correct ?

With DM clocks enabled I don't even think it makes sense to call those 
older functions.

Angus
> 
> -	_mxc_serial_setbrg(plat->reg, clk, baudrate, plat->use_dte);
> +	_mxc_serial_setbrg(plat->reg, rate, baudrate, plat->use_dte);
> 
>  	return 0;
>  }