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

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

[Adam Ford]

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(-)

-- 
2.34.1

[RFC 1/3] clk: imx8mm: Add UART clocks

[Adam Ford]

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,
-- 
2.34.1

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

[Adam Ford]

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

[RFC 3/3] imx8: imx8mm_beacon: Remove manual UART clock initialization

[Adam Ford]

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();
-- 
2.34.1

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

[Peng Fan (OSS)]

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
> 

Re: [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@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
> >