[PATCH 10/31] serial: mtk: add support for using dynamic baud clock souce

[Weijie Gao]

On Thu, 2022-08-04 at 07:56 -0600, Simon Glass wrote:
> Hi Weijie,
> 
> On Wed, 3 Aug 2022 at 21:36, Weijie Gao <weijie.gao at mediatek.com>
> wrote:
> > 
> > The baud clock on some platform may change due to assigned-clock-
> > parent
> > set in DT. In current flow the baud clock is only retrieved during
> > probe
> > stage. If the parent of the source clock changes after probe stage,
> > the
> > setbrg will set wrong baudrate.
> > 
> > To get the right clock rate, this patch records the baud clk struct
> > to the
> > driver's priv, and changes the driver's flow to get the clock rate
> > before
> > calling _mtk_serial_setbrg().
> > 
> > Signed-off-by: Weijie Gao <weijie.gao at mediatek.com>
> > ---
> >  drivers/serial/serial_mtk.c | 72 ++++++++++++++++++++-------------
> > ----
> >  1 file changed, 39 insertions(+), 33 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg at chromium.org>
> 
> please see below
> 
> > 
> > diff --git a/drivers/serial/serial_mtk.c
> > b/drivers/serial/serial_mtk.c
> > index a84f39b3fa..99cf62b8d9 100644
> > --- a/drivers/serial/serial_mtk.c
> > +++ b/drivers/serial/serial_mtk.c
> > @@ -10,6 +10,7 @@
> >  #include <common.h>
> >  #include <div64.h>
> >  #include <dm.h>
> > +#include <dm/device_compat.h>
> >  #include <errno.h>
> >  #include <log.h>
> >  #include <serial.h>
> > @@ -72,25 +73,27 @@ struct mtk_serial_regs {
> > 
> >  struct mtk_serial_priv {
> 
> please add a full comment for this struct

OK.

> 
> >         struct mtk_serial_regs __iomem *regs;
> > -       u32 clock;
> > +       struct clk clk;
> > +       u32 fixed_clk_rate;
> >         bool force_highspeed;
> >  };
> > 
> > -static void _mtk_serial_setbrg(struct mtk_serial_priv *priv, int
> > baud)
> > +static void _mtk_serial_setbrg(struct mtk_serial_priv *priv, int
> > baud,
> > +                              u32 clk_rate)
> 
> Why u32? Can you use uint? Generally it is better for parameters to
> use natural types unless there is a good reason.

In fact u32 is identical to uint.

<asm-generic/int-ll64.h>: typedef __u32 u32;
<asm-generic/int-ll64.h>: typedef unsigned int __u32;

<linux/types.h>: typedef unsigned int uint;

> 
> [..]
> 
> Regards,
> Simon