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.ha...@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+3...@mail.gmail.com/
