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

[Heiko Thiery]

Hi Angus,

Am Do., 17. März 2022 um 14:19 Uhr schrieb Angus Ainslie <angus at 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 at 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 at 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 ?

I checked the dtsi files for all the boards that use the compatible
strings from the serial_mxc and see that there are always 2 clocks:
'ipg' and 'per'.

The imx8 dtsi files describe ipg and per always the same clock.
The imx7 dtsi files descibe ipg and per always the same clock.
The imx6 dtsi files describe ipg and per are 2 different clocks
The imx51 dtsi files describeipg and per are 2 different clocks

So I'm not sure if the ipg clock is the right one for the boards that
has different clock for ipg and per.

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

I do not understand what you mean with that.

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

You mean when DM clocks are available the "new" method should be used
and no fallback to the old mechanism?

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