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

[Heiko Thiery]

Hi Adam,

[SNIP]

>
> I was thinking that we could expand the struct mxc_serial_plat to
> include both per and igp clocks to cover devices have clocks that are
> not the same.  The configuring of the platdata can happen using
> mxc_serial_of_to_plat to 'get' both igp and per clocks.  The probe
> would enable both per and igp to ensure they are operating, then the
> mxc_serial_setbrg could use clk_get_rate(plat->clk_igp) to determine
> the clock rate.

With your comment I have made the following, does this fit with your suggestion?

diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
index e4970a169b..7f9f7e8383 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,16 @@ __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 defined(CONFIG_CLK)
+       rate = clk_get_rate(&plat->per_clk);
+#else
+       /* as fallback we try to get the clk rate that way */
+       rate = imx_get_uartclk();
+#endif

-       _mxc_serial_setbrg(plat->reg, clk, baudrate, plat->use_dte);
+       _mxc_serial_setbrg(plat->reg, rate, baudrate, plat->use_dte);

        return 0;
 }
@@ -277,6 +285,11 @@ static int mxc_serial_probe(struct udevice *dev)
 {
        struct mxc_serial_plat *plat = dev_get_plat(dev);

+#if defined(CONFIG_CLK)
+       clk_enable(&plat->ipg_clk);
+       clk_enable(&plat->per_clk);
+#endif
+
        _mxc_serial_init(plat->reg, plat->use_dte);

        return 0;
@@ -339,6 +352,10 @@ 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 defined(CONFIG_CLK)
+       clk_get_by_name(dev, "ipg", &plat->ipg_clk);
+       clk_get_by_name(dev, "per", &plat->per_clk);
+#endif
        return 0;
 }

diff --git a/include/dm/platform_data/serial_mxc.h
b/include/dm/platform_data/serial_mxc.h
index cc59eeb1dd..330476f816 100644
--- a/include/dm/platform_data/serial_mxc.h
+++ b/include/dm/platform_data/serial_mxc.h
@@ -10,6 +10,10 @@
 struct mxc_serial_plat {
        struct mxc_uart *reg;  /* address of registers in physical memory */
        bool use_dte;
+#if defined(CONFIG_CLK)
+       struct clk ipg_clk;
+       struct clk per_clk;
+#endif
 };

 #endif

> I am guessing the clock composite would have to be expanded to include
> the UART clocks because from what I can see, they're not included.
> However, this could potentially eliminate the need to use some of the
> functions in arch/arm/mach-imx/imx8m/clock_imx8mm.

For the imx8mq I added this and asked Angus to integrate it into his
patchset. [1]

> The down-side is that a bunch of SoC's might need to be updated to
> support more and more clocks, so we could potentially use
> !CONFIG_IS_ENABLED to use the existing functions as a fall-back.
>
> adam
> >
> > Angus

-- 
Heiko

[1] https://lore.kernel.org/u-boot/CAEyMn7bC-p6o6VCu7YWQyXMm+51EcJ5OVXv_625cjDiaR+3TyQ@mail.gmail.com/