[RFC 2/3] serial: mxc: Enable getting and enabling clocks with CCF Composite

[Adam Ford]

On Sun, May 1, 2022 at 4:48 AM Peng Fan (OSS) <peng.fan at oss.nxp.com> wrote:
>
>
>
> On 2022/5/1 0:14, Adam Ford wrote:
> > Certain platforms have the UART clocks exposed through the CCF.
> > When they are, it's possible to enable and query the clock(s)
> > for a given UART. Add support getting, enabling, and querying
> > the clocks.
> >
> > Signed-off-by: Adam Ford <aford173 at gmail.com>
> >
> > diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
> > index e4970a169b..c68040ba74 100644
> > --- a/drivers/serial/serial_mxc.c
> > +++ b/drivers/serial/serial_mxc.c
> > @@ -10,6 +10,7 @@
> >   #include <asm/arch/imx-regs.h>
> >   #include <asm/arch/clock.h>
> >   #include <asm/global_data.h>
> > +#include <clk.h>
> >   #include <dm/platform_data/serial_mxc.h>
> >   #include <serial.h>
> >   #include <linux/compiler.h>
> > @@ -266,8 +267,13 @@ __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 clk;
> >
> > +#if CONFIG_IS_ENABLED(CLK_COMPOSITE_CCF)
>
> CONFIG_IS_ENABLED(CLK) should be fine.

I chose the composite because I knew it was limited to the imx8m
family and there is one imx6 and an imxrt with CLK enabled.  I can
look into the imx6, but the imxrt isn't hardware i have.
I'll try to add the clocks for both imx6q and the imxrt.

>
> > +      clk = clk_get_rate(&plat->ipg_clk);
> > +#else
> > +     clk = imx_get_uartclk;
>
> "()" should not be dropped.

oops.  I'll fix that.


>
> > +#endif
> >       _mxc_serial_setbrg(plat->reg, clk, baudrate, plat->use_dte);
> >
> >       return 0;
> > @@ -277,6 +283,22 @@ static int mxc_serial_probe(struct udevice *dev)
> >   {
> >       struct mxc_serial_plat *plat = dev_get_plat(dev);
> >
> > +#if CONFIG_IS_ENABLED(CLK_COMPOSITE_CCF)
>
> Ditto
>
> > +     int ret = 0;
> > +
> > +     ret = clk_enable(&plat->per_clk);
> > +     if (ret) {
> > +             printf("Failed to enable per_clk\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = clk_enable(&plat->per_clk);
>
> ipg_clk?

Opps.  Copy-paste error.  I'll fix that.
>
> > +     if (ret) {
> > +             printf("Failed to enable ipg_clk\n");
> > +             return ret;
> > +     }
> > +
> > +#endif
> >       _mxc_serial_init(plat->reg, plat->use_dte);
> >
> >       return 0;
> > @@ -339,6 +361,21 @@ static int mxc_serial_of_to_plat(struct udevice *dev)
> >
> >       plat->use_dte = fdtdec_get_bool(gd->fdt_blob, dev_of_offset(dev),
> >                                       "fsl,dte-mode");
> > +#if CONFIG_IS_ENABLED(CLK_COMPOSITE_CCF)
> > +     int ret = 0;
> > +
> > +     ret = clk_get_by_name(dev, "per", &plat->per_clk);
> > +     if (ret) {
> > +             printf("Failed to get per_clk\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = clk_get_by_name(dev, "ipg", &plat->ipg_clk);
> > +     if (ret) {
> > +             printf("Failed to get per_clk\n");

There is a copy-paste error here too that I'll fix.

> > +             return ret;
> > +     }
> > +#endif
>
> Regards,
> Peng.

Thanks for the review  I'll try to get a more formal patch with the
additional clocks added for the other SoC's, then fix the typos in
here.  I'm on vacation out of the country on May 8-23, so I'm going to
try to get it submitted before I leave.

adam
>
> >       return 0;
> >   }
> >
> > diff --git a/include/dm/platform_data/serial_mxc.h b/include/dm/platform_data/serial_mxc.h
> > index cc59eeb1dd..9a8c8498bf 100644
> > --- a/include/dm/platform_data/serial_mxc.h
> > +++ b/include/dm/platform_data/serial_mxc.h
> > @@ -10,6 +10,8 @@
> >   struct mxc_serial_plat {
> >       struct mxc_uart *reg;  /* address of registers in physical memory */
> >       bool use_dte;
> > +     struct clk per_clk;
> > +     struct clk ipg_clk;
> >   };
> >
> >   #endif
> >