[U-Boot] [PATCH 0/2] arm: lpc32xx: use driver model serial console drivers

[U-Boot] [PATCH 0/2] arm: lpc32xx: use driver model serial console drivers

[Vladimir Zapolskiy]

The change enables DM versions of NXP LPC32xx standard (NS16550)
and high-speed UART drivers for both ordinary and SPL U-boot images.

The change increases the size of SPL images over a maximum size limit
for a secondary bootloader, if it is downloaded from a small page NAND
flash, but this does not concern any maintained LPC32xx boards. Anyhow
this SPL_DM specific change is separated to a stand alone commit.

The change depends on high-speed UART driver switched to DM:
  https://patchwork.ozlabs.org/patch/556061/

Albert, I send the change to you before completion of DM HSUART review
in case if you have any comments and because the change touches one board,
which is maintained by you.

Vladimir Zapolskiy (2):
  arm: lpc32xx: switch serial console to driver model
  arm: lpc32xx: switch SPL builds to driver model

 arch/arm/cpu/arm926ejs/lpc32xx/devices.c   | 37 ++++++++++++++++++++++++++++--
 arch/arm/include/asm/arch-lpc32xx/config.h | 36 ++++++++++++-----------------
 configs/devkit3250_defconfig               |  2 ++
 configs/work_92105_defconfig               |  2 ++
 4 files changed, 54 insertions(+), 23 deletions(-)

-- 
2.1.4

[U-Boot] [PATCH 1/2] arm: lpc32xx: switch serial console to driver model

[Vladimir Zapolskiy]

On NXP LPC32xx platform for non-SPL builds the change adds
standard (NS16550) and high-speed UARTs to driver model.
Due to specific of DM NS16550 device description UART clock can not be
got in runtime and by default it is set to 13MHz, if board PERIPH_CLK
is different, this should be specified in board configuration file.

For SPL builds HSUARTs are disabled and non-DM NS16550 driver is
compiled, if needed.

The change also updates default configs of devkit3250 and work_92105
boards to reflect updates in platform files.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 arch/arm/cpu/arm926ejs/lpc32xx/devices.c   | 37 ++++++++++++++++++++++++++++--
 arch/arm/include/asm/arch-lpc32xx/config.h | 32 +++++++++++++++-----------
 configs/devkit3250_defconfig               |  1 +
 configs/work_92105_defconfig               |  1 +
 4 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
index b1c3f8f..447d0cd 100644
--- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
+++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
@@ -5,12 +5,14 @@
  */
 
 #include <common.h>
-#include <asm/arch/cpu.h>
+#include <dm.h>
+#include <dm/platform_data/lpc32xx_hsuart.h>
+#include <ns16550.h>
+
 #include <asm/arch/clk.h>
 #include <asm/arch/uart.h>
 #include <asm/arch/mux.h>
 #include <asm/io.h>
-#include <dm.h>
 
 static struct clk_pm_regs    *clk  = (struct clk_pm_regs *)CLK_PM_BASE;
 static struct uart_ctrl_regs *ctrl = (struct uart_ctrl_regs *)UART_CTRL_BASE;
@@ -41,6 +43,37 @@ void lpc32xx_uart_init(unsigned int uart_id)
 	       &clk->u3clk + (uart_id - 3));
 }
 
+#if !CONFIG_IS_ENABLED(OF_CONTROL) && !defined(CONFIG_SPL_BUILD)
+static const struct ns16550_platdata lpc32xx_uart[] = {
+	{ UART3_BASE, 2, CONFIG_SYS_NS16550_CLK },
+	{ UART4_BASE, 2, CONFIG_SYS_NS16550_CLK },
+	{ UART5_BASE, 2, CONFIG_SYS_NS16550_CLK },
+	{ UART6_BASE, 2, CONFIG_SYS_NS16550_CLK },
+};
+
+#if defined(CONFIG_LPC32XX_HSUART)
+static const struct lpc32xx_hsuart_platdata lpc32xx_hsuart[] = {
+	{ HS_UART1_BASE, },
+	{ HS_UART2_BASE, },
+	{ HS_UART7_BASE, },
+};
+#endif
+
+U_BOOT_DEVICES(lpc32xx_uarts) = {
+#if defined(CONFIG_LPC32XX_HSUART)
+	{ "lpc32xx_hsuart", &lpc32xx_hsuart[0], },
+	{ "lpc32xx_hsuart", &lpc32xx_hsuart[1], },
+#endif
+	{ "ns16550_serial", &lpc32xx_uart[0], },
+	{ "ns16550_serial", &lpc32xx_uart[1], },
+	{ "ns16550_serial", &lpc32xx_uart[2], },
+	{ "ns16550_serial", &lpc32xx_uart[3], },
+#if defined(CONFIG_LPC32XX_HSUART)
+	{ "lpc32xx_hsuart", &lpc32xx_hsuart[2], },
+#endif
+};
+#endif
+
 void lpc32xx_dma_init(void)
 {
 	/* Enable DMA interface */
diff --git a/arch/arm/include/asm/arch-lpc32xx/config.h b/arch/arm/include/asm/arch-lpc32xx/config.h
index 521bff1..27e60e1 100644
--- a/arch/arm/include/asm/arch-lpc32xx/config.h
+++ b/arch/arm/include/asm/arch-lpc32xx/config.h
@@ -16,16 +16,21 @@
 #define CONFIG_NR_DRAM_BANKS_MAX	2
 
 /* UART configuration */
-#if (CONFIG_SYS_LPC32XX_UART >= 3) && (CONFIG_SYS_LPC32XX_UART <= 6)
-#define CONFIG_SYS_NS16550_SERIAL
-#define CONFIG_CONS_INDEX		(CONFIG_SYS_LPC32XX_UART - 2)
-#elif	(CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
+#if	(CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
 	(CONFIG_SYS_LPC32XX_UART == 7)
+#if defined(CONFIG_SPL_BUILD)
+/* SPL images do not support LPC32xx HSUART, UART5 is selected for SPL */
+#undef CONFIG_SYS_LPC32XX_UART
+#define CONFIG_SYS_LPC32XX_UART		5
+#endif
+
+#if !defined(CONFIG_LPC32XX_HSUART)
 #define CONFIG_LPC32XX_HSUART
-#else
-#error "define CONFIG_SYS_LPC32XX_UART in the range from 1 to 7"
+#endif
 #endif
 
+#if defined(CONFIG_SPL_BUILD)
+#define CONFIG_SYS_NS16550_SERIAL
 #define CONFIG_SYS_NS16550_REG_SIZE	-4
 #define CONFIG_SYS_NS16550_CLK		get_serial_clock()
 
@@ -33,15 +38,16 @@
 #define CONFIG_SYS_NS16550_COM2		UART4_BASE
 #define CONFIG_SYS_NS16550_COM3		UART5_BASE
 #define CONFIG_SYS_NS16550_COM4		UART6_BASE
+#endif
 
-#if defined(CONFIG_LPC32XX_HSUART)
-#if	CONFIG_SYS_LPC32XX_UART == 1
-#define HS_UART_BASE			HS_UART1_BASE
-#elif	CONFIG_SYS_LPC32XX_UART == 2
-#define HS_UART_BASE			HS_UART2_BASE
-#else	/* CONFIG_SYS_LPC32XX_UART == 7 */
-#define HS_UART_BASE			HS_UART7_BASE
+#if !defined(CONFIG_SYS_NS16550_CLK)
+#define CONFIG_SYS_NS16550_CLK		13000000
 #endif
+
+#if !defined(CONFIG_LPC32XX_HSUART) || defined(CONFIG_SPL_BUILD)
+#define CONFIG_CONS_INDEX		(CONFIG_SYS_LPC32XX_UART - 2)
+#else
+#define CONFIG_CONS_INDEX		CONFIG_SYS_LPC32XX_UART
 #endif
 
 #define CONFIG_SYS_BAUDRATE_TABLE	\
diff --git a/configs/devkit3250_defconfig b/configs/devkit3250_defconfig
index 64a0fb0..0abb8e0 100644
--- a/configs/devkit3250_defconfig
+++ b/configs/devkit3250_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_TARGET_DEVKIT3250=y
+CONFIG_DM_SERIAL=y
 CONFIG_DM_GPIO=y
 CONFIG_SPL=y
 # CONFIG_CMD_FPGA is not set
diff --git a/configs/work_92105_defconfig b/configs/work_92105_defconfig
index 1cad3a2..a5a108e 100644
--- a/configs/work_92105_defconfig
+++ b/configs/work_92105_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_TARGET_WORK_92105=y
+CONFIG_DM_SERIAL=y
 CONFIG_DM_GPIO=y
 CONFIG_SPL=y
 # CONFIG_CMD_IMLS is not set
-- 
2.1.4

[U-Boot] [PATCH 2/2] arm: lpc32xx: switch SPL builds to driver model

[Vladimir Zapolskiy]

For NXP LPC32xx boards the change enables SPL_DM option, this allows
to use any driver model UART driver in SPL images, hence a restriction
on HSUART in SPL image is removed and well as definitions for non-DM
NS16550 driver, its DM version is used instead.

Note, CONFIG_SPL_DM option noticeably increases SPL image, if just
NAND SLC and DM version of NS16650 are included to the image, the size
of SPL image is increased almost in two times from 10672 bytes to
19704 bytes.
If SPL image is downloaded from a small page NAND device, then this
can cause a problem, according to the LPC32xx User's Manual the
maximum size of a secondary bootloader stored on small page NAND flash
should not exceed 15.5KB (maximum size of a secondary bootloader on a
large page NAND is 54KB).

Because SPL_DM requires malloc(), enable CONFIG_SYS_MALLOC_SIMPLE for
all LPC32xx boards in shared config.h file.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 arch/arm/cpu/arm926ejs/lpc32xx/devices.c   |  2 +-
 arch/arm/include/asm/arch-lpc32xx/config.h | 24 ++++++------------------
 configs/devkit3250_defconfig               |  1 +
 configs/work_92105_defconfig               |  1 +
 4 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
index 447d0cd..1fab875 100644
--- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
+++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
@@ -43,7 +43,7 @@ void lpc32xx_uart_init(unsigned int uart_id)
 	       &clk->u3clk + (uart_id - 3));
 }
 
-#if !CONFIG_IS_ENABLED(OF_CONTROL) && !defined(CONFIG_SPL_BUILD)
+#if !CONFIG_IS_ENABLED(OF_CONTROL)
 static const struct ns16550_platdata lpc32xx_uart[] = {
 	{ UART3_BASE, 2, CONFIG_SYS_NS16550_CLK },
 	{ UART4_BASE, 2, CONFIG_SYS_NS16550_CLK },
diff --git a/arch/arm/include/asm/arch-lpc32xx/config.h b/arch/arm/include/asm/arch-lpc32xx/config.h
index 27e60e1..a9f057e 100644
--- a/arch/arm/include/asm/arch-lpc32xx/config.h
+++ b/arch/arm/include/asm/arch-lpc32xx/config.h
@@ -15,36 +15,24 @@
 
 #define CONFIG_NR_DRAM_BANKS_MAX	2
 
-/* UART configuration */
-#if	(CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
-	(CONFIG_SYS_LPC32XX_UART == 7)
+/* SPL build configuration */
 #if defined(CONFIG_SPL_BUILD)
-/* SPL images do not support LPC32xx HSUART, UART5 is selected for SPL */
-#undef CONFIG_SYS_LPC32XX_UART
-#define CONFIG_SYS_LPC32XX_UART		5
+#define CONFIG_SYS_MALLOC_SIMPLE
 #endif
 
+/* UART configuration */
+#if	(CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
+	(CONFIG_SYS_LPC32XX_UART == 7)
 #if !defined(CONFIG_LPC32XX_HSUART)
 #define CONFIG_LPC32XX_HSUART
 #endif
 #endif
 
-#if defined(CONFIG_SPL_BUILD)
-#define CONFIG_SYS_NS16550_SERIAL
-#define CONFIG_SYS_NS16550_REG_SIZE	-4
-#define CONFIG_SYS_NS16550_CLK		get_serial_clock()
-
-#define CONFIG_SYS_NS16550_COM1		UART3_BASE
-#define CONFIG_SYS_NS16550_COM2		UART4_BASE
-#define CONFIG_SYS_NS16550_COM3		UART5_BASE
-#define CONFIG_SYS_NS16550_COM4		UART6_BASE
-#endif
-
 #if !defined(CONFIG_SYS_NS16550_CLK)
 #define CONFIG_SYS_NS16550_CLK		13000000
 #endif
 
-#if !defined(CONFIG_LPC32XX_HSUART) || defined(CONFIG_SPL_BUILD)
+#if !defined(CONFIG_LPC32XX_HSUART)
 #define CONFIG_CONS_INDEX		(CONFIG_SYS_LPC32XX_UART - 2)
 #else
 #define CONFIG_CONS_INDEX		CONFIG_SYS_LPC32XX_UART
diff --git a/configs/devkit3250_defconfig b/configs/devkit3250_defconfig
index 0abb8e0..87c137d 100644
--- a/configs/devkit3250_defconfig
+++ b/configs/devkit3250_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_TARGET_DEVKIT3250=y
+CONFIG_SPL_DM=y
 CONFIG_DM_SERIAL=y
 CONFIG_DM_GPIO=y
 CONFIG_SPL=y
diff --git a/configs/work_92105_defconfig b/configs/work_92105_defconfig
index a5a108e..db69345 100644
--- a/configs/work_92105_defconfig
+++ b/configs/work_92105_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_TARGET_WORK_92105=y
+CONFIG_SPL_DM=y
 CONFIG_DM_SERIAL=y
 CONFIG_DM_GPIO=y
 CONFIG_SPL=y
-- 
2.1.4

[U-Boot] [PATCH 2/2] arm: lpc32xx: switch SPL builds to driver model

[Simon Glass]

Hi Vladimir,

On 12 December 2015 at 17:48, Vladimir Zapolskiy <vz@mleia.com> wrote:
> For NXP LPC32xx boards the change enables SPL_DM option, this allows
> to use any driver model UART driver in SPL images, hence a restriction
> on HSUART in SPL image is removed and well as definitions for non-DM
> NS16550 driver, its DM version is used instead.
>
> Note, CONFIG_SPL_DM option noticeably increases SPL image, if just
> NAND SLC and DM version of NS16650 are included to the image, the size
> of SPL image is increased almost in two times from 10672 bytes to
> 19704 bytes.
> If SPL image is downloaded from a small page NAND device, then this
> can cause a problem, according to the LPC32xx User's Manual the
> maximum size of a secondary bootloader stored on small page NAND flash
> should not exceed 15.5KB (maximum size of a secondary bootloader on a
> large page NAND is 54KB).
>
> Because SPL_DM requires malloc(), enable CONFIG_SYS_MALLOC_SIMPLE for
> all LPC32xx boards in shared config.h file.
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
>  arch/arm/cpu/arm926ejs/lpc32xx/devices.c   |  2 +-
>  arch/arm/include/asm/arch-lpc32xx/config.h | 24 ++++++------------------
>  configs/devkit3250_defconfig               |  1 +
>  configs/work_92105_defconfig               |  1 +
>  4 files changed, 9 insertions(+), 19 deletions(-)

You are bringing in the full malloc(). Try adding:

CONFIG_SPL_SYS_MALLOC_SIMPLE=y

to your defconfig. The should cut the size increment to just under
5KB. That is still a lot. Are you able to use Thumb on your board?

Regards,
Simon

[U-Boot] [PATCH 2/2] arm: lpc32xx: switch SPL builds to driver model

[Vladimir Zapolskiy]

Hi Simon,

On 15.12.2015 20:57, Simon Glass wrote:
> Hi Vladimir,
> 
> On 12 December 2015 at 17:48, Vladimir Zapolskiy <vz@mleia.com> wrote:
>> For NXP LPC32xx boards the change enables SPL_DM option, this allows
>> to use any driver model UART driver in SPL images, hence a restriction
>> on HSUART in SPL image is removed and well as definitions for non-DM
>> NS16550 driver, its DM version is used instead.
>>
>> Note, CONFIG_SPL_DM option noticeably increases SPL image, if just
>> NAND SLC and DM version of NS16650 are included to the image, the size
>> of SPL image is increased almost in two times from 10672 bytes to
>> 19704 bytes.
>> If SPL image is downloaded from a small page NAND device, then this
>> can cause a problem, according to the LPC32xx User's Manual the
>> maximum size of a secondary bootloader stored on small page NAND flash
>> should not exceed 15.5KB (maximum size of a secondary bootloader on a
>> large page NAND is 54KB).
>>
>> Because SPL_DM requires malloc(), enable CONFIG_SYS_MALLOC_SIMPLE for
>> all LPC32xx boards in shared config.h file.
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>> ---
>>  arch/arm/cpu/arm926ejs/lpc32xx/devices.c   |  2 +-
>>  arch/arm/include/asm/arch-lpc32xx/config.h | 24 ++++++------------------
>>  configs/devkit3250_defconfig               |  1 +
>>  configs/work_92105_defconfig               |  1 +
>>  4 files changed, 9 insertions(+), 19 deletions(-)
> 
> You are bringing in the full malloc(). Try adding:
> 
> CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> 
> to your defconfig. The should cut the size increment to just under
> 5KB. 

thank you for the hint, I was not aware of this option.

The size of SPL image dropped from 19704 down to 15608 bytes for
devkit3250, and I verified that the new image works.

I checked the difference with objdump (10672 -> 15608):

* +3236 bytes of alloc/DM/uclass functions,
* +320 bytes of static data,
* +360 bytes of difference in non-DM/DM NS16550 drivers
* +160 bytes memcpy(), strcmp(),
* +144 bytes reset_cpu(), do_reset(), reset_misc()
* etc, nothing outstanding.

I think that the total size of DM/uclass functions about 3KB is pretty
tight, let it remain.

> That is still a lot. Are you able to use Thumb on your board?
> 

Yes, ARM core of LPC32xx has ARMv5TE architecture.

My board has a large page NAND device (Albert's board also has some
large page NAND device I believe), so generally it is not a problem for
me, if new SPL image is a bit larger, however I want to emphasize this
in a commit message.

Albert, what do you think about the change? Do be you prefer to add
CONFIG_SPL_SYS_MALLOC_SIMPLE=y to defconfigs?

I think it is better to do it in v2, plus I have to remove

diff --git a/arch/arm/include/asm/arch-lpc32xx/config.h
b/arch/arm/include/asm/arch-lpc32xx/config.h
index a9f057e..3b7f6bd 100644
--- a/arch/arm/include/asm/arch-lpc32xx/config.h
+++ b/arch/arm/include/asm/arch-lpc32xx/config.h
@@ -15,11 +15,6 @@

 #define CONFIG_NR_DRAM_BANKS_MAX       2

-/* SPL build configuration */
-#if defined(CONFIG_SPL_BUILD)
-#define CONFIG_SYS_MALLOC_SIMPLE
-#endif
-
 /* UART configuration */
 #if	(CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
 	(CONFIG_SYS_LPC32XX_UART == 7)

--
With best wishes,
Vladimir

[U-Boot] [PATCH 1/2] arm: lpc32xx: switch serial console to driver model

[Simon Glass]

Hi Vladimir,

On 12 December 2015 at 17:48, Vladimir Zapolskiy <vz@mleia.com> wrote:
> On NXP LPC32xx platform for non-SPL builds the change adds
> standard (NS16550) and high-speed UARTs to driver model.
> Due to specific of DM NS16550 device description UART clock can not be
> got in runtime and by default it is set to 13MHz, if board PERIPH_CLK
> is different, this should be specified in board configuration file.
>
> For SPL builds HSUARTs are disabled and non-DM NS16550 driver is
> compiled, if needed.
>
> The change also updates default configs of devkit3250 and work_92105
> boards to reflect updates in platform files.
>
> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
> ---
>  arch/arm/cpu/arm926ejs/lpc32xx/devices.c   | 37 ++++++++++++++++++++++++++++--
>  arch/arm/include/asm/arch-lpc32xx/config.h | 32 +++++++++++++++-----------
>  configs/devkit3250_defconfig               |  1 +
>  configs/work_92105_defconfig               |  1 +
>  4 files changed, 56 insertions(+), 15 deletions(-)

 Reviewed-by: Simon Glass <sjg@chromium.org>

Nits below.

>
> diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
> index b1c3f8f..447d0cd 100644
> --- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
> +++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
> @@ -5,12 +5,14 @@
>   */
>
>  #include <common.h>
> -#include <asm/arch/cpu.h>
> +#include <dm.h>
> +#include <dm/platform_data/lpc32xx_hsuart.h>
> +#include <ns16550.h>

nit: please put this before the dm/ include. Sub-directory include
should go at the end. See:

http://www.denx.de/wiki/U-Boot/CodingStyle

> +
>  #include <asm/arch/clk.h>
>  #include <asm/arch/uart.h>
>  #include <asm/arch/mux.h>
>  #include <asm/io.h>
> -#include <dm.h>
>
>  static struct clk_pm_regs    *clk  = (struct clk_pm_regs *)CLK_PM_BASE;
>  static struct uart_ctrl_regs *ctrl = (struct uart_ctrl_regs *)UART_CTRL_BASE;
> @@ -41,6 +43,37 @@ void lpc32xx_uart_init(unsigned int uart_id)
>                &clk->u3clk + (uart_id - 3));
>  }
>
> +#if !CONFIG_IS_ENABLED(OF_CONTROL) && !defined(CONFIG_SPL_BUILD)

#ifndef CONFIG_OF_CONTROL

should be equivalent to this.

> +static const struct ns16550_platdata lpc32xx_uart[] = {
> +       { UART3_BASE, 2, CONFIG_SYS_NS16550_CLK },
> +       { UART4_BASE, 2, CONFIG_SYS_NS16550_CLK },
> +       { UART5_BASE, 2, CONFIG_SYS_NS16550_CLK },
> +       { UART6_BASE, 2, CONFIG_SYS_NS16550_CLK },
> +};
> +
> +#if defined(CONFIG_LPC32XX_HSUART)
> +static const struct lpc32xx_hsuart_platdata lpc32xx_hsuart[] = {
> +       { HS_UART1_BASE, },
> +       { HS_UART2_BASE, },
> +       { HS_UART7_BASE, },
> +};
> +#endif
> +
> +U_BOOT_DEVICES(lpc32xx_uarts) = {
> +#if defined(CONFIG_LPC32XX_HSUART)
> +       { "lpc32xx_hsuart", &lpc32xx_hsuart[0], },
> +       { "lpc32xx_hsuart", &lpc32xx_hsuart[1], },
> +#endif
> +       { "ns16550_serial", &lpc32xx_uart[0], },
> +       { "ns16550_serial", &lpc32xx_uart[1], },
> +       { "ns16550_serial", &lpc32xx_uart[2], },
> +       { "ns16550_serial", &lpc32xx_uart[3], },
> +#if defined(CONFIG_LPC32XX_HSUART)
> +       { "lpc32xx_hsuart", &lpc32xx_hsuart[2], },
> +#endif
> +};
> +#endif
> +
>  void lpc32xx_dma_init(void)
>  {
>         /* Enable DMA interface */
> diff --git a/arch/arm/include/asm/arch-lpc32xx/config.h b/arch/arm/include/asm/arch-lpc32xx/config.h
> index 521bff1..27e60e1 100644
> --- a/arch/arm/include/asm/arch-lpc32xx/config.h
> +++ b/arch/arm/include/asm/arch-lpc32xx/config.h
> @@ -16,16 +16,21 @@
>  #define CONFIG_NR_DRAM_BANKS_MAX       2
>
>  /* UART configuration */
> -#if (CONFIG_SYS_LPC32XX_UART >= 3) && (CONFIG_SYS_LPC32XX_UART <= 6)
> -#define CONFIG_SYS_NS16550_SERIAL
> -#define CONFIG_CONS_INDEX              (CONFIG_SYS_LPC32XX_UART - 2)
> -#elif  (CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
> +#if    (CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
>         (CONFIG_SYS_LPC32XX_UART == 7)
> +#if defined(CONFIG_SPL_BUILD)
> +/* SPL images do not support LPC32xx HSUART, UART5 is selected for SPL */
> +#undef CONFIG_SYS_LPC32XX_UART
> +#define CONFIG_SYS_LPC32XX_UART                5
> +#endif
> +
> +#if !defined(CONFIG_LPC32XX_HSUART)
>  #define CONFIG_LPC32XX_HSUART
> -#else
> -#error "define CONFIG_SYS_LPC32XX_UART in the range from 1 to 7"
> +#endif
>  #endif
>
> +#if defined(CONFIG_SPL_BUILD)
> +#define CONFIG_SYS_NS16550_SERIAL
>  #define CONFIG_SYS_NS16550_REG_SIZE    -4
>  #define CONFIG_SYS_NS16550_CLK         get_serial_clock()
>
> @@ -33,15 +38,16 @@
>  #define CONFIG_SYS_NS16550_COM2                UART4_BASE
>  #define CONFIG_SYS_NS16550_COM3                UART5_BASE
>  #define CONFIG_SYS_NS16550_COM4                UART6_BASE
> +#endif
>
> -#if defined(CONFIG_LPC32XX_HSUART)
> -#if    CONFIG_SYS_LPC32XX_UART == 1
> -#define HS_UART_BASE                   HS_UART1_BASE
> -#elif  CONFIG_SYS_LPC32XX_UART == 2
> -#define HS_UART_BASE                   HS_UART2_BASE
> -#else  /* CONFIG_SYS_LPC32XX_UART == 7 */
> -#define HS_UART_BASE                   HS_UART7_BASE
> +#if !defined(CONFIG_SYS_NS16550_CLK)
> +#define CONFIG_SYS_NS16550_CLK         13000000
>  #endif
> +
> +#if !defined(CONFIG_LPC32XX_HSUART) || defined(CONFIG_SPL_BUILD)
> +#define CONFIG_CONS_INDEX              (CONFIG_SYS_LPC32XX_UART - 2)
> +#else
> +#define CONFIG_CONS_INDEX              CONFIG_SYS_LPC32XX_UART
>  #endif
>
>  #define CONFIG_SYS_BAUDRATE_TABLE      \
> diff --git a/configs/devkit3250_defconfig b/configs/devkit3250_defconfig
> index 64a0fb0..0abb8e0 100644
> --- a/configs/devkit3250_defconfig
> +++ b/configs/devkit3250_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_TARGET_DEVKIT3250=y
> +CONFIG_DM_SERIAL=y
>  CONFIG_DM_GPIO=y
>  CONFIG_SPL=y
>  # CONFIG_CMD_FPGA is not set
> diff --git a/configs/work_92105_defconfig b/configs/work_92105_defconfig
> index 1cad3a2..a5a108e 100644
> --- a/configs/work_92105_defconfig
> +++ b/configs/work_92105_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_TARGET_WORK_92105=y
> +CONFIG_DM_SERIAL=y
>  CONFIG_DM_GPIO=y
>  CONFIG_SPL=y
>  # CONFIG_CMD_IMLS is not set
> --
> 2.1.4
>

BTW you should be able to adjust it to work in SPL also.

Regards,
Simon

[U-Boot] [PATCH 1/2] arm: lpc32xx: switch serial console to driver model

[Vladimir Zapolskiy]

Hi Simon,

On 19.12.2015 22:30, Simon Glass wrote:
> Hi Vladimir,
> 
> On 12 December 2015 at 17:48, Vladimir Zapolskiy <vz@mleia.com> wrote:
>> On NXP LPC32xx platform for non-SPL builds the change adds
>> standard (NS16550) and high-speed UARTs to driver model.
>> Due to specific of DM NS16550 device description UART clock can not be
>> got in runtime and by default it is set to 13MHz, if board PERIPH_CLK
>> is different, this should be specified in board configuration file.
>>
>> For SPL builds HSUARTs are disabled and non-DM NS16550 driver is
>> compiled, if needed.
>>
>> The change also updates default configs of devkit3250 and work_92105
>> boards to reflect updates in platform files.
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>> ---
>>  arch/arm/cpu/arm926ejs/lpc32xx/devices.c   | 37 ++++++++++++++++++++++++++++--
>>  arch/arm/include/asm/arch-lpc32xx/config.h | 32 +++++++++++++++-----------
>>  configs/devkit3250_defconfig               |  1 +
>>  configs/work_92105_defconfig               |  1 +
>>  4 files changed, 56 insertions(+), 15 deletions(-)
> 
>  Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Nits below.
> 
>>
>> diff --git a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> index b1c3f8f..447d0cd 100644
>> --- a/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> +++ b/arch/arm/cpu/arm926ejs/lpc32xx/devices.c
>> @@ -5,12 +5,14 @@
>>   */
>>
>>  #include <common.h>
>> -#include <asm/arch/cpu.h>
>> +#include <dm.h>
>> +#include <dm/platform_data/lpc32xx_hsuart.h>
>> +#include <ns16550.h>
> 
> nit: please put this before the dm/ include. Sub-directory include
> should go at the end. See:
> 
> http://www.denx.de/wiki/U-Boot/CodingStyle

Ok, thanks for the hint, I relied only on coding style description from
README.

>> +
>>  #include <asm/arch/clk.h>
>>  #include <asm/arch/uart.h>
>>  #include <asm/arch/mux.h>
>>  #include <asm/io.h>
>> -#include <dm.h>
>>
>>  static struct clk_pm_regs    *clk  = (struct clk_pm_regs *)CLK_PM_BASE;
>>  static struct uart_ctrl_regs *ctrl = (struct uart_ctrl_regs *)UART_CTRL_BASE;
>> @@ -41,6 +43,37 @@ void lpc32xx_uart_init(unsigned int uart_id)
>>                &clk->u3clk + (uart_id - 3));
>>  }
>>
>> +#if !CONFIG_IS_ENABLED(OF_CONTROL) && !defined(CONFIG_SPL_BUILD)
> 
> #ifndef CONFIG_OF_CONTROL
> 
> should be equivalent to this.

Here I want to emphasize that the change is non-SPL specific.

If this patch is applied, then SPL image still contains legacy NS16550
driver, and U-boot image is switched to its DM driver.

!defined(CONFIG_SPL_BUILD) is removed in the second change.

>> +static const struct ns16550_platdata lpc32xx_uart[] = {
>> +       { UART3_BASE, 2, CONFIG_SYS_NS16550_CLK },
>> +       { UART4_BASE, 2, CONFIG_SYS_NS16550_CLK },
>> +       { UART5_BASE, 2, CONFIG_SYS_NS16550_CLK },
>> +       { UART6_BASE, 2, CONFIG_SYS_NS16550_CLK },
>> +};
>> +
>> +#if defined(CONFIG_LPC32XX_HSUART)
>> +static const struct lpc32xx_hsuart_platdata lpc32xx_hsuart[] = {
>> +       { HS_UART1_BASE, },
>> +       { HS_UART2_BASE, },
>> +       { HS_UART7_BASE, },
>> +};
>> +#endif
>> +
>> +U_BOOT_DEVICES(lpc32xx_uarts) = {
>> +#if defined(CONFIG_LPC32XX_HSUART)
>> +       { "lpc32xx_hsuart", &lpc32xx_hsuart[0], },
>> +       { "lpc32xx_hsuart", &lpc32xx_hsuart[1], },
>> +#endif
>> +       { "ns16550_serial", &lpc32xx_uart[0], },
>> +       { "ns16550_serial", &lpc32xx_uart[1], },
>> +       { "ns16550_serial", &lpc32xx_uart[2], },
>> +       { "ns16550_serial", &lpc32xx_uart[3], },
>> +#if defined(CONFIG_LPC32XX_HSUART)
>> +       { "lpc32xx_hsuart", &lpc32xx_hsuart[2], },
>> +#endif
>> +};
>> +#endif
>> +
>>  void lpc32xx_dma_init(void)
>>  {
>>         /* Enable DMA interface */
>> diff --git a/arch/arm/include/asm/arch-lpc32xx/config.h b/arch/arm/include/asm/arch-lpc32xx/config.h
>> index 521bff1..27e60e1 100644
>> --- a/arch/arm/include/asm/arch-lpc32xx/config.h
>> +++ b/arch/arm/include/asm/arch-lpc32xx/config.h
>> @@ -16,16 +16,21 @@
>>  #define CONFIG_NR_DRAM_BANKS_MAX       2
>>
>>  /* UART configuration */
>> -#if (CONFIG_SYS_LPC32XX_UART >= 3) && (CONFIG_SYS_LPC32XX_UART <= 6)
>> -#define CONFIG_SYS_NS16550_SERIAL
>> -#define CONFIG_CONS_INDEX              (CONFIG_SYS_LPC32XX_UART - 2)
>> -#elif  (CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
>> +#if    (CONFIG_SYS_LPC32XX_UART == 1) || (CONFIG_SYS_LPC32XX_UART == 2) || \
>>         (CONFIG_SYS_LPC32XX_UART == 7)
>> +#if defined(CONFIG_SPL_BUILD)
>> +/* SPL images do not support LPC32xx HSUART, UART5 is selected for SPL */
>> +#undef CONFIG_SYS_LPC32XX_UART
>> +#define CONFIG_SYS_LPC32XX_UART                5
>> +#endif
>> +
>> +#if !defined(CONFIG_LPC32XX_HSUART)
>>  #define CONFIG_LPC32XX_HSUART
>> -#else
>> -#error "define CONFIG_SYS_LPC32XX_UART in the range from 1 to 7"
>> +#endif
>>  #endif
>>
>> +#if defined(CONFIG_SPL_BUILD)
>> +#define CONFIG_SYS_NS16550_SERIAL
>>  #define CONFIG_SYS_NS16550_REG_SIZE    -4
>>  #define CONFIG_SYS_NS16550_CLK         get_serial_clock()
>>
>> @@ -33,15 +38,16 @@
>>  #define CONFIG_SYS_NS16550_COM2                UART4_BASE
>>  #define CONFIG_SYS_NS16550_COM3                UART5_BASE
>>  #define CONFIG_SYS_NS16550_COM4                UART6_BASE
>> +#endif
>>
>> -#if defined(CONFIG_LPC32XX_HSUART)
>> -#if    CONFIG_SYS_LPC32XX_UART == 1
>> -#define HS_UART_BASE                   HS_UART1_BASE
>> -#elif  CONFIG_SYS_LPC32XX_UART == 2
>> -#define HS_UART_BASE                   HS_UART2_BASE
>> -#else  /* CONFIG_SYS_LPC32XX_UART == 7 */
>> -#define HS_UART_BASE                   HS_UART7_BASE
>> +#if !defined(CONFIG_SYS_NS16550_CLK)
>> +#define CONFIG_SYS_NS16550_CLK         13000000
>>  #endif
>> +
>> +#if !defined(CONFIG_LPC32XX_HSUART) || defined(CONFIG_SPL_BUILD)
>> +#define CONFIG_CONS_INDEX              (CONFIG_SYS_LPC32XX_UART - 2)
>> +#else
>> +#define CONFIG_CONS_INDEX              CONFIG_SYS_LPC32XX_UART
>>  #endif
>>
>>  #define CONFIG_SYS_BAUDRATE_TABLE      \
>> diff --git a/configs/devkit3250_defconfig b/configs/devkit3250_defconfig
>> index 64a0fb0..0abb8e0 100644
>> --- a/configs/devkit3250_defconfig
>> +++ b/configs/devkit3250_defconfig
>> @@ -1,5 +1,6 @@
>>  CONFIG_ARM=y
>>  CONFIG_TARGET_DEVKIT3250=y
>> +CONFIG_DM_SERIAL=y
>>  CONFIG_DM_GPIO=y
>>  CONFIG_SPL=y
>>  # CONFIG_CMD_FPGA is not set
>> diff --git a/configs/work_92105_defconfig b/configs/work_92105_defconfig
>> index 1cad3a2..a5a108e 100644
>> --- a/configs/work_92105_defconfig
>> +++ b/configs/work_92105_defconfig
>> @@ -1,5 +1,6 @@
>>  CONFIG_ARM=y
>>  CONFIG_TARGET_WORK_92105=y
>> +CONFIG_DM_SERIAL=y
>>  CONFIG_DM_GPIO=y
>>  CONFIG_SPL=y
>>  # CONFIG_CMD_IMLS is not set
>> --
>> 2.1.4
>>
> 
> BTW you should be able to adjust it to work in SPL also.
>

That is done in the second patch.

Do you think it makes sense to squash them?

With best wishes,
Vladimir