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@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