diff mbox series

[03/15] imx: imx8mm_mx8menlo: Enable DM_SERIAL

Message ID 20220430124317.17382-4-peng.fan@oss.nxp.com
State Superseded
Delegated to: Stefano Babic
Headers show
Series imx8m: convert to DM_SERIAL | expand

Commit Message

Peng Fan (OSS) April 30, 2022, 12:43 p.m. UTC 
From: Peng Fan <peng.fan@nxp.com>

Enable CONFIG_DM_SERIAL. uart2 and its pinmux was already
marked with u-boot,dm-spl.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 board/menlo/mx8menlo/mx8menlo.c   | 9 ---------
 configs/imx8mm-mx8menlo_defconfig | 1 +
 include/configs/imx8mm-mx8menlo.h | 3 ---
 3 files changed, 1 insertion(+), 12 deletions(-)

Comments

Marek Vasut April 30, 2022, 5 p.m. UTC | #1 
On 4/30/22 14:43, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Enable CONFIG_DM_SERIAL. uart2 and its pinmux was already
> marked with u-boot,dm-spl.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>   board/menlo/mx8menlo/mx8menlo.c   | 9 ---------
>   configs/imx8mm-mx8menlo_defconfig | 1 +
>   include/configs/imx8mm-mx8menlo.h | 3 ---
>   3 files changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/board/menlo/mx8menlo/mx8menlo.c b/board/menlo/mx8menlo/mx8menlo.c
> index a4d0becdcc8..95ff95ad360 100644
> --- a/board/menlo/mx8menlo/mx8menlo.c
> +++ b/board/menlo/mx8menlo/mx8menlo.c
> @@ -12,15 +12,8 @@
>   #include <asm/mach-imx/iomux-v3.h>
>   #include <spl.h>
>   
> -#define UART_PAD_CTRL	(PAD_CTL_PUE | PAD_CTL_PE | PAD_CTL_DSE4)
>   #define WDOG_PAD_CTRL	(PAD_CTL_DSE6 | PAD_CTL_ODE | PAD_CTL_PUE | PAD_CTL_PE)
>   
> -/* Verdin UART_3, Console/Debug UART */
> -static iomux_v3_cfg_t const uart_pads[] = {
> -	IMX8MM_PAD_SAI3_TXFS_UART2_RX | MUX_PAD_CTRL(UART_PAD_CTRL),
> -	IMX8MM_PAD_SAI3_TXC_UART2_TX | MUX_PAD_CTRL(UART_PAD_CTRL),
> -};
> -
>   static iomux_v3_cfg_t const wdog_pads[] = {
>   	IMX8MM_PAD_GPIO1_IO02_WDOG1_WDOG_B  | MUX_PAD_CTRL(WDOG_PAD_CTRL),
>   };
> @@ -48,8 +41,6 @@ void board_early_init(void)
>   
>   	set_wdog_reset(wdog);
>   
> -	imx_iomux_v3_setup_multiple_pads(uart_pads, ARRAY_SIZE(uart_pads));
> -
>   	init_uart_clk(1);

But that means the UART is available much later in SPL ?

Also, init_uart_clk(1) still hard-codes UART number , can the UART 
driver init those UART clock instead too ?
Adam Ford April 30, 2022, 5:32 p.m. UTC | #2 
On Sat, Apr 30, 2022 at 12:00 PM Marek Vasut <marex@denx.de> wrote:
>
> On 4/30/22 14:43, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Enable CONFIG_DM_SERIAL. uart2 and its pinmux was already
> > marked with u-boot,dm-spl.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >   board/menlo/mx8menlo/mx8menlo.c   | 9 ---------
> >   configs/imx8mm-mx8menlo_defconfig | 1 +
> >   include/configs/imx8mm-mx8menlo.h | 3 ---
> >   3 files changed, 1 insertion(+), 12 deletions(-)
> >
> > diff --git a/board/menlo/mx8menlo/mx8menlo.c b/board/menlo/mx8menlo/mx8menlo.c
> > index a4d0becdcc8..95ff95ad360 100644
> > --- a/board/menlo/mx8menlo/mx8menlo.c
> > +++ b/board/menlo/mx8menlo/mx8menlo.c
> > @@ -12,15 +12,8 @@
> >   #include <asm/mach-imx/iomux-v3.h>
> >   #include <spl.h>
> >
> > -#define UART_PAD_CTRL        (PAD_CTL_PUE | PAD_CTL_PE | PAD_CTL_DSE4)
> >   #define WDOG_PAD_CTRL       (PAD_CTL_DSE6 | PAD_CTL_ODE | PAD_CTL_PUE | PAD_CTL_PE)
> >
> > -/* Verdin UART_3, Console/Debug UART */
> > -static iomux_v3_cfg_t const uart_pads[] = {
> > -     IMX8MM_PAD_SAI3_TXFS_UART2_RX | MUX_PAD_CTRL(UART_PAD_CTRL),
> > -     IMX8MM_PAD_SAI3_TXC_UART2_TX | MUX_PAD_CTRL(UART_PAD_CTRL),
> > -};
> > -
> >   static iomux_v3_cfg_t const wdog_pads[] = {
> >       IMX8MM_PAD_GPIO1_IO02_WDOG1_WDOG_B  | MUX_PAD_CTRL(WDOG_PAD_CTRL),
> >   };
> > @@ -48,8 +41,6 @@ void board_early_init(void)
> >
> >       set_wdog_reset(wdog);
> >
> > -     imx_iomux_v3_setup_multiple_pads(uart_pads, ARRAY_SIZE(uart_pads));
> > -
> >       init_uart_clk(1);
>
> But that means the UART is available much later in SPL ?
>
> Also, init_uart_clk(1) still hard-codes UART number , can the UART
> driver init those UART clock instead too ?

I just submitted an RFC to address that [1].  The RTC is based on the
work Peng did.

If people are OK with my proposal or changing the serial driver, I can
work on porting the 8mn and 8mp clocks to let the drivers enable the
clocks.

[1] - https://patchwork.ozlabs.org/project/uboot/patch/20220430161422.558361-2-aford173@gmail.com/

adam

[1] - https://patchwork.ozlabs.org/project/uboot/patch/20220430161422.558361-2-aford173@gmail.com/
Marek Vasut April 30, 2022, 6:04 p.m. UTC | #3 
On 4/30/22 19:32, Adam Ford wrote:
> On Sat, Apr 30, 2022 at 12:00 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 4/30/22 14:43, Peng Fan (OSS) wrote:
>>> From: Peng Fan <peng.fan@nxp.com>
>>>
>>> Enable CONFIG_DM_SERIAL. uart2 and its pinmux was already
>>> marked with u-boot,dm-spl.
>>>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> ---
>>>    board/menlo/mx8menlo/mx8menlo.c   | 9 ---------
>>>    configs/imx8mm-mx8menlo_defconfig | 1 +
>>>    include/configs/imx8mm-mx8menlo.h | 3 ---
>>>    3 files changed, 1 insertion(+), 12 deletions(-)
>>>
>>> diff --git a/board/menlo/mx8menlo/mx8menlo.c b/board/menlo/mx8menlo/mx8menlo.c
>>> index a4d0becdcc8..95ff95ad360 100644
>>> --- a/board/menlo/mx8menlo/mx8menlo.c
>>> +++ b/board/menlo/mx8menlo/mx8menlo.c
>>> @@ -12,15 +12,8 @@
>>>    #include <asm/mach-imx/iomux-v3.h>
>>>    #include <spl.h>
>>>
>>> -#define UART_PAD_CTRL        (PAD_CTL_PUE | PAD_CTL_PE | PAD_CTL_DSE4)
>>>    #define WDOG_PAD_CTRL       (PAD_CTL_DSE6 | PAD_CTL_ODE | PAD_CTL_PUE | PAD_CTL_PE)
>>>
>>> -/* Verdin UART_3, Console/Debug UART */
>>> -static iomux_v3_cfg_t const uart_pads[] = {
>>> -     IMX8MM_PAD_SAI3_TXFS_UART2_RX | MUX_PAD_CTRL(UART_PAD_CTRL),
>>> -     IMX8MM_PAD_SAI3_TXC_UART2_TX | MUX_PAD_CTRL(UART_PAD_CTRL),
>>> -};
>>> -
>>>    static iomux_v3_cfg_t const wdog_pads[] = {
>>>        IMX8MM_PAD_GPIO1_IO02_WDOG1_WDOG_B  | MUX_PAD_CTRL(WDOG_PAD_CTRL),
>>>    };
>>> @@ -48,8 +41,6 @@ void board_early_init(void)
>>>
>>>        set_wdog_reset(wdog);
>>>
>>> -     imx_iomux_v3_setup_multiple_pads(uart_pads, ARRAY_SIZE(uart_pads));
>>> -
>>>        init_uart_clk(1);
>>
>> But that means the UART is available much later in SPL ?
>>
>> Also, init_uart_clk(1) still hard-codes UART number , can the UART
>> driver init those UART clock instead too ?
> 
> I just submitted an RFC to address that [1].  The RTC is based on the
> work Peng did.
> 
> If people are OK with my proposal or changing the serial driver, I can
> work on porting the 8mn and 8mp clocks to let the drivers enable the
> clocks.
> 
> [1] - https://patchwork.ozlabs.org/project/uboot/patch/20220430161422.558361-2-aford173@gmail.com/

That still works only after spl_early_init() is called, right ?
Adam Ford April 30, 2022, 6:40 p.m. UTC | #4 
On Sat, Apr 30, 2022 at 1:04 PM Marek Vasut <marex@denx.de> wrote:
>
> On 4/30/22 19:32, Adam Ford wrote:
> > On Sat, Apr 30, 2022 at 12:00 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 4/30/22 14:43, Peng Fan (OSS) wrote:
> >>> From: Peng Fan <peng.fan@nxp.com>
> >>>
> >>> Enable CONFIG_DM_SERIAL. uart2 and its pinmux was already
> >>> marked with u-boot,dm-spl.
> >>>
> >>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> >>> ---
> >>>    board/menlo/mx8menlo/mx8menlo.c   | 9 ---------
> >>>    configs/imx8mm-mx8menlo_defconfig | 1 +
> >>>    include/configs/imx8mm-mx8menlo.h | 3 ---
> >>>    3 files changed, 1 insertion(+), 12 deletions(-)
> >>>
> >>> diff --git a/board/menlo/mx8menlo/mx8menlo.c b/board/menlo/mx8menlo/mx8menlo.c
> >>> index a4d0becdcc8..95ff95ad360 100644
> >>> --- a/board/menlo/mx8menlo/mx8menlo.c
> >>> +++ b/board/menlo/mx8menlo/mx8menlo.c
> >>> @@ -12,15 +12,8 @@
> >>>    #include <asm/mach-imx/iomux-v3.h>
> >>>    #include <spl.h>
> >>>
> >>> -#define UART_PAD_CTRL        (PAD_CTL_PUE | PAD_CTL_PE | PAD_CTL_DSE4)
> >>>    #define WDOG_PAD_CTRL       (PAD_CTL_DSE6 | PAD_CTL_ODE | PAD_CTL_PUE | PAD_CTL_PE)
> >>>
> >>> -/* Verdin UART_3, Console/Debug UART */
> >>> -static iomux_v3_cfg_t const uart_pads[] = {
> >>> -     IMX8MM_PAD_SAI3_TXFS_UART2_RX | MUX_PAD_CTRL(UART_PAD_CTRL),
> >>> -     IMX8MM_PAD_SAI3_TXC_UART2_TX | MUX_PAD_CTRL(UART_PAD_CTRL),
> >>> -};
> >>> -
> >>>    static iomux_v3_cfg_t const wdog_pads[] = {
> >>>        IMX8MM_PAD_GPIO1_IO02_WDOG1_WDOG_B  | MUX_PAD_CTRL(WDOG_PAD_CTRL),
> >>>    };
> >>> @@ -48,8 +41,6 @@ void board_early_init(void)
> >>>
> >>>        set_wdog_reset(wdog);
> >>>
> >>> -     imx_iomux_v3_setup_multiple_pads(uart_pads, ARRAY_SIZE(uart_pads));
> >>> -
> >>>        init_uart_clk(1);
> >>
> >> But that means the UART is available much later in SPL ?
> >>
> >> Also, init_uart_clk(1) still hard-codes UART number , can the UART
> >> driver init those UART clock instead too ?
> >
> > I just submitted an RFC to address that [1].  The RTC is based on the
> > work Peng did.
> >
> > If people are OK with my proposal or changing the serial driver, I can
> > work on porting the 8mn and 8mp clocks to let the drivers enable the
> > clocks.
> >
> > [1] - https://patchwork.ozlabs.org/project/uboot/patch/20220430161422.558361-2-aford173@gmail.com/
>
> That still works only after spl_early_init() is called, right ?

As far as I can tell, we need spl init to run to get the device tree
stuff functional in order to enable either the clocks or the serial
port.  However, I have noticed the README states that board_init_f
should not call board_init_r directly and BSS isn't not available.  It
seems like there might be more work to do on cleaning up the various
board_init_f functions.  It seems like everyone clears BSS and calls
board_init_r directly.

I wonder if we could consolidate all these boards files into a
platform specific startup function and let people who have
board-specific stuff they need done to do it in early init or
something.

adam
diff mbox series

Patch

diff --git a/board/menlo/mx8menlo/mx8menlo.c b/board/menlo/mx8menlo/mx8menlo.c
index a4d0becdcc8..95ff95ad360 100644
--- a/board/menlo/mx8menlo/mx8menlo.c
+++ b/board/menlo/mx8menlo/mx8menlo.c
@@ -12,15 +12,8 @@ 
 #include <asm/mach-imx/iomux-v3.h>
 #include <spl.h>
 
-#define UART_PAD_CTRL	(PAD_CTL_PUE | PAD_CTL_PE | PAD_CTL_DSE4)
 #define WDOG_PAD_CTRL	(PAD_CTL_DSE6 | PAD_CTL_ODE | PAD_CTL_PUE | PAD_CTL_PE)
 
-/* Verdin UART_3, Console/Debug UART */
-static iomux_v3_cfg_t const uart_pads[] = {
-	IMX8MM_PAD_SAI3_TXFS_UART2_RX | MUX_PAD_CTRL(UART_PAD_CTRL),
-	IMX8MM_PAD_SAI3_TXC_UART2_TX | MUX_PAD_CTRL(UART_PAD_CTRL),
-};
-
 static iomux_v3_cfg_t const wdog_pads[] = {
 	IMX8MM_PAD_GPIO1_IO02_WDOG1_WDOG_B  | MUX_PAD_CTRL(WDOG_PAD_CTRL),
 };
@@ -48,8 +41,6 @@  void board_early_init(void)
 
 	set_wdog_reset(wdog);
 
-	imx_iomux_v3_setup_multiple_pads(uart_pads, ARRAY_SIZE(uart_pads));
-
 	init_uart_clk(1);
 
 	setup_snvs();
diff --git a/configs/imx8mm-mx8menlo_defconfig b/configs/imx8mm-mx8menlo_defconfig
index 0d3e19e5e31..a4164951de0 100644
--- a/configs/imx8mm-mx8menlo_defconfig
+++ b/configs/imx8mm-mx8menlo_defconfig
@@ -105,6 +105,7 @@  CONFIG_DM_PMIC_PFUZE100=y
 CONFIG_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
+CONFIG_DM_SERIAL=y
 CONFIG_MXC_UART=y
 CONFIG_SYSRESET=y
 CONFIG_SPL_SYSRESET=y
diff --git a/include/configs/imx8mm-mx8menlo.h b/include/configs/imx8mm-mx8menlo.h
index fd1831622f0..530ecd1d460 100644
--- a/include/configs/imx8mm-mx8menlo.h
+++ b/include/configs/imx8mm-mx8menlo.h
@@ -30,7 +30,4 @@ 
 	"initrd_addr=0x43800000\0"					\
 	"kernel_image=fitImage\0"
 
-#undef CONFIG_MXC_UART_BASE
-#define CONFIG_MXC_UART_BASE		UART2_BASE_ADDR
-
 #endif /* __IMX8MM_MX8MENLO_H */