[RFC 0/3] imx8m: Remove manual UART clock initialization

This series is an RFC focussing on only the i.MX8M Mini until feedback is received when the series can be expanded to other imx boards.
This series adds the UART clocks to the CCF, adds functions to the serial driver to fetch, enable, and query the clock(s), and finally removes the manual clock intialization from SPL.
Once feedback is received and final method is accepted, it can be expanded to other boards.
Adam Ford (3): clk: imx8mm: Add UART clocks serial: mxc: Enable getting and enabling clocks with CCF Composite imx8: imx8mm_beacon: Remove manual UART clock initialization
board/beacon/imx8mm/spl.c | 2 -- drivers/clk/imx/clk-imx8mm.c | 30 +++++++++++++++++++++ drivers/serial/serial_mxc.c | 39 ++++++++++++++++++++++++++- include/dm/platform_data/serial_mxc.h | 2 ++ 4 files changed, 70 insertions(+), 3 deletions(-)

There are four UART's with various clocks assoicated with each. Add them to the clock driver so they can be enabled and queried by the serial driver as needed.
Signed-off-by: Adam Ford aford173@gmail.com
diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c index 542aa31f7a..8566c3e4a0 100644 --- a/drivers/clk/imx/clk-imx8mm.c +++ b/drivers/clk/imx/clk-imx8mm.c @@ -78,6 +78,18 @@ static const char *imx8mm_pwm3_sels[] = {"clock-osc-24m", "sys_pll2_100m", "sys_ static const char *imx8mm_pwm4_sels[] = {"clock-osc-24m", "sys_pll2_100m", "sys_pll1_160m", "sys_pll1_40m", "sys_pll3_out", "clk_ext2", "sys_pll1_80m", "video_pll1_out", };
+static const char *imx8mm_uart1_sels[] = {"clock-osc-24m", "sys_pll1_80m", "sys_pll2_200m", "sys_pll2_100m", + "sys_pll3_out", "clk_ext2", "clk_ext4", "audio_pll2_out", }; + +static const char *imx8mm_uart2_sels[] = {"clock-osc-24m", "sys_pll1_80m", "sys_pll2_200m", "sys_pll2_100m", + "sys_pll3_out", "clk_ext2", "clk_ext3", "audio_pll2_out", }; + +static const char *imx8mm_uart3_sels[] = {"clock-osc-24m", "sys_pll1_80m", "sys_pll2_200m", "sys_pll2_100m", + "sys_pll3_out", "clk_ext2", "clk_ext4", "audio_pll2_out", }; + +static const char *imx8mm_uart4_sels[] = {"clock-osc-24m", "sys_pll1_80m", "sys_pll2_200m", "sys_pll2_100m", + "sys_pll3_out", "clk_ext2", "clk_ext3", "audio_pll2_out", }; + static const char *imx8mm_wdog_sels[] = {"clock-osc-24m", "sys_pll1_133m", "sys_pll1_160m", "vpu_pll_out", "sys_pll2_125m", "sys_pll3_out", "sys_pll1_80m", "sys_pll2_166m", };
@@ -267,6 +279,14 @@ static int imx8mm_clk_probe(struct udevice *dev) imx8m_clk_composite("i2c3", imx8mm_i2c3_sels, base + 0xae00)); clk_dm(IMX8MM_CLK_I2C4, imx8m_clk_composite("i2c4", imx8mm_i2c4_sels, base + 0xae80)); + clk_dm(IMX8MM_CLK_UART1, + imx8m_clk_composite("uart1", imx8mm_uart1_sels, base + 0xaf00)); + clk_dm(IMX8MM_CLK_UART2, + imx8m_clk_composite("uart2", imx8mm_uart2_sels, base + 0xaf80)); + clk_dm(IMX8MM_CLK_UART3, + imx8m_clk_composite("uart3", imx8mm_uart3_sels, base + 0xb000)); + clk_dm(IMX8MM_CLK_UART4, + imx8m_clk_composite("uart4", imx8mm_uart4_sels, base + 0xb080)); clk_dm(IMX8MM_CLK_PWM1, imx8m_clk_composite("pwm1", imx8mm_pwm1_sels, base + 0xb380)); clk_dm(IMX8MM_CLK_PWM2, @@ -307,6 +327,16 @@ static int imx8mm_clk_probe(struct udevice *dev) imx_clk_gate4("i2c3_root_clk", "i2c3", base + 0x4190, 0)); clk_dm(IMX8MM_CLK_I2C4_ROOT, imx_clk_gate4("i2c4_root_clk", "i2c4", base + 0x41a0, 0)); + + clk_dm(IMX8MM_CLK_UART1_ROOT, + imx_clk_gate4("uart1_root_clk", "uart1", base + 0x4490, 0)); + clk_dm(IMX8MM_CLK_UART2_ROOT, + imx_clk_gate4("uart2_root_clk", "uart2", base + 0x44a0, 0)); + clk_dm(IMX8MM_CLK_UART3_ROOT, + imx_clk_gate4("uart3_root_clk", "uart3", base + 0x44b0, 0)); + clk_dm(IMX8MM_CLK_UART4_ROOT, + imx_clk_gate4("uart4_root_clk", "uart4", base + 0x44c0, 0)); + clk_dm(IMX8MM_CLK_OCOTP_ROOT, imx_clk_gate4("ocotp_root_clk", "ipg_root", base + 0x4220, 0)); clk_dm(IMX8MM_CLK_PWM1_ROOT,

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@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) + clk = clk_get_rate(&plat->ipg_clk); +#else + clk = imx_get_uartclk; +#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) + 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); + 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"); + return ret; + } +#endif 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

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@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.
clk = clk_get_rate(&plat->ipg_clk);
+#else
- clk = imx_get_uartclk;
"()" should not be dropped.
+#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?
- 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");
return ret;
- }
+#endif
Regards, Peng.
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

On Sun, May 1, 2022 at 4:48 AM Peng Fan (OSS) peng.fan@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@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

SPL has a manual call to init_uart_clk in order to start the clocks. If the CCF has the UART clocks available, the serial driver can enable and query the clock when needed, so the manual call to init_uart_clk can be dropped.
Signed-off-by: Adam Ford aford173@gmail.com
diff --git a/board/beacon/imx8mm/spl.c b/board/beacon/imx8mm/spl.c index f92b4c3ed0..fc0935bc97 100644 --- a/board/beacon/imx8mm/spl.c +++ b/board/beacon/imx8mm/spl.c @@ -114,8 +114,6 @@ void board_init_f(ulong dummy)
arch_cpu_init();
- init_uart_clk(1); - board_early_init_f();
timer_init();
participants (2)
-
Adam Ford
-
Peng Fan (OSS)