[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Bin Meng]

This series converts Freescale LS1021A-TWR board to driver model.
  - Enable ns16550 serial driver on ls1021atwr_nor configuration
  - Convert LPUART serial driver to driver model
  - Enable LPUART serial driver on ls1021atwr_nor_lpuart configuration


Bin Meng (8):
  fdt: Fix up stdout correctly in fdt_fixup_stdout()
  arm: ls1021atwr: Convert to driver model and enable serial support
  serial: lpuart: Move CONFIG_FSL_LPUART to Kconfig
  serial: lpuart: Fix several cosmetic issues
  serial: lpuart: Call local version of setbrg and putc directly
  serial: lpuart: Prepare the driver for DM conversion
  serial: lpuart: Add driver model serial support
  arm: ls1021atwr: Enable driver model lpuart serial driver

 arch/arm/dts/ls1021a-twr.dts                 |   8 +
 arch/arm/dts/ls1021a.dtsi                    |   4 -
 common/fdt_support.c                         |  16 +-
 configs/colibri_vf_defconfig                 |   1 +
 configs/colibri_vf_dtb_defconfig             |   1 +
 configs/ls1021aqds_ddr4_nor_lpuart_defconfig |   1 +
 configs/ls1021aqds_nor_lpuart_defconfig      |   1 +
 configs/ls1021atwr_nor_defconfig             |   4 +
 configs/ls1021atwr_nor_lpuart_defconfig      |   5 +
 configs/pcm052_defconfig                     |   1 +
 configs/vf610twr_defconfig                   |   1 +
 configs/vf610twr_nand_defconfig              |   1 +
 doc/driver-model/serial-howto.txt            |   1 -
 drivers/serial/Kconfig                       |   6 +
 drivers/serial/serial_lpuart.c               | 334 +++++++++++++++++++++------
 include/configs/colibri_vf.h                 |   1 -
 include/configs/ls1021aqds.h                 |   1 -
 include/configs/ls1021atwr.h                 |   3 +-
 include/configs/pcm052.h                     |   1 -
 include/configs/vf610twr.h                   |   1 -
 20 files changed, 298 insertions(+), 94 deletions(-)

-- 
1.8.2.1

[U-Boot] [PATCH 1/8] fdt: Fix up stdout correctly in fdt_fixup_stdout()

[Bin Meng]

When CONFIG_OF_STDOUT_VIA_ALIAS is on, always fix up kernel's stdout
string with hardcoded CONFIG_CONS_INDEX.

This actually reverts commit 3e303f748cf57fb23e8ec95ab7eac0074be50e2b
"fdt_support: Add multi-serial support for stdout fixup", as the fix
up in the /aliases node did not work under the following scenarios:
- Not every non-DM serial driver was written to have a driver name
  that conforms the format of "serial%d" or "eserial%d".
- With driver model serial, the stdio_devices[] stores the serial
  device node name in the device tree.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 common/fdt_support.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 66464db..4ce17d4 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -131,18 +131,6 @@ static int fdt_fixup_stdout(void *fdt, int chosenoff)
 			      OF_STDOUT_PATH, strlen(OF_STDOUT_PATH) + 1);
 }
 #elif defined(CONFIG_OF_STDOUT_VIA_ALIAS) && defined(CONFIG_CONS_INDEX)
-static void fdt_fill_multisername(char *sername, size_t maxlen)
-{
-	const char *outname = stdio_devices[stdout]->name;
-
-	if (strcmp(outname, "serial") > 0)
-		strncpy(sername, outname, maxlen);
-
-	/* eserial? */
-	if (strcmp(outname + 1, "serial") > 0)
-		strncpy(sername, outname + 1, maxlen);
-}
-
 static int fdt_fixup_stdout(void *fdt, int chosenoff)
 {
 	int err;
@@ -152,9 +140,7 @@ static int fdt_fixup_stdout(void *fdt, int chosenoff)
 	int len;
 	char tmp[256]; /* long enough */
 
-	fdt_fill_multisername(sername, sizeof(sername) - 1);
-	if (!sername[0])
-		sprintf(sername, "serial%d", CONFIG_CONS_INDEX - 1);
+	sprintf(sername, "serial%d", CONFIG_CONS_INDEX - 1);
 
 	aliasoff = fdt_path_offset(fdt, "/aliases");
 	if (aliasoff < 0) {
-- 
1.8.2.1

[U-Boot] [PATCH 1/8] fdt: Fix up stdout correctly in fdt_fixup_stdout()

[Simon Glass]

Hi Bin,

On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> When CONFIG_OF_STDOUT_VIA_ALIAS is on, always fix up kernel's stdout
> string with hardcoded CONFIG_CONS_INDEX.
>
> This actually reverts commit 3e303f748cf57fb23e8ec95ab7eac0074be50e2b
> "fdt_support: Add multi-serial support for stdout fixup", as the fix

In that case, could this be a revert, created with 'git revert'?

> up in the /aliases node did not work under the following scenarios:
> - Not every non-DM serial driver was written to have a driver name
>   that conforms the format of "serial%d" or "eserial%d".
> - With driver model serial, the stdio_devices[] stores the serial
>   device node name in the device tree.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  common/fdt_support.c | 16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)

Regards,
Simon

[U-Boot] [PATCH 1/8] fdt: Fix up stdout correctly in fdt_fixup_stdout()

[Bin Meng]

Hi Simon,

On Fri, Jan 8, 2016 at 11:34 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
>> When CONFIG_OF_STDOUT_VIA_ALIAS is on, always fix up kernel's stdout
>> string with hardcoded CONFIG_CONS_INDEX.
>>
>> This actually reverts commit 3e303f748cf57fb23e8ec95ab7eac0074be50e2b
>> "fdt_support: Add multi-serial support for stdout fixup", as the fix
>
> In that case, could this be a revert, created with 'git revert'?

I've never used 'git revert' command. Did you mean this commit/patch
should be recreated using 'git revert'? Does this matter?

>
>> up in the /aliases node did not work under the following scenarios:
>> - Not every non-DM serial driver was written to have a driver name
>>   that conforms the format of "serial%d" or "eserial%d".
>> - With driver model serial, the stdio_devices[] stores the serial
>>   device node name in the device tree.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  common/fdt_support.c | 16 +---------------
>>  1 file changed, 1 insertion(+), 15 deletions(-)
>

Regards,
Bin

[U-Boot] [PATCH 1/8] fdt: Fix up stdout correctly in fdt_fixup_stdout()

[Simon Glass]

Hi Bin,

On 10 January 2016 at 20:02, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Fri, Jan 8, 2016 at 11:34 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Bin,
>>
>> On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> When CONFIG_OF_STDOUT_VIA_ALIAS is on, always fix up kernel's stdout
>>> string with hardcoded CONFIG_CONS_INDEX.
>>>
>>> This actually reverts commit 3e303f748cf57fb23e8ec95ab7eac0074be50e2b
>>> "fdt_support: Add multi-serial support for stdout fixup", as the fix
>>
>> In that case, could this be a revert, created with 'git revert'?
>
> I've never used 'git revert' command. Did you mean this commit/patch
> should be recreated using 'git revert'? Does this matter?

Well it creates a commit with a particular subject and format, which
people are used to seeing for reverts. So if it is actually a revert,
then yes I think you should use 'git revert'. You can edit the commit
message to provide more detail.

>
>>
>>> up in the /aliases node did not work under the following scenarios:
>>> - Not every non-DM serial driver was written to have a driver name
>>>   that conforms the format of "serial%d" or "eserial%d".
>>> - With driver model serial, the stdio_devices[] stores the serial
>>>   device node name in the device tree.
>>>
>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>>> ---
>>>
>>>  common/fdt_support.c | 16 +---------------
>>>  1 file changed, 1 insertion(+), 15 deletions(-)
>>
>
> Regards,
> Bin

Regards,
Simon

[U-Boot] [PATCH 1/8] fdt: Fix up stdout correctly in fdt_fixup_stdout()

[Bin Meng]

Hi Simon,

On Tue, Jan 12, 2016 at 12:58 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 10 January 2016 at 20:02, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Simon,
>>
>> On Fri, Jan 8, 2016 at 11:34 AM, Simon Glass <sjg@chromium.org> wrote:
>>> Hi Bin,
>>>
>>> On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>> When CONFIG_OF_STDOUT_VIA_ALIAS is on, always fix up kernel's stdout
>>>> string with hardcoded CONFIG_CONS_INDEX.
>>>>
>>>> This actually reverts commit 3e303f748cf57fb23e8ec95ab7eac0074be50e2b
>>>> "fdt_support: Add multi-serial support for stdout fixup", as the fix
>>>
>>> In that case, could this be a revert, created with 'git revert'?
>>
>> I've never used 'git revert' command. Did you mean this commit/patch
>> should be recreated using 'git revert'? Does this matter?
>
> Well it creates a commit with a particular subject and format, which
> people are used to seeing for reverts. So if it is actually a revert,
> then yes I think you should use 'git revert'. You can edit the commit
> message to provide more detail.
>

When running 'git revert 3e303f748cf57fb23e8ec95ab7eac0074be50e2b', I got:

error: could not revert 3e303f7... fdt_support: Add multi-serial
support for stdout fixup
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

Looks it cannot be reverted cleanly using 'git revert' command. What's
the best practice here?

Regards,
Bin

[U-Boot] [PATCH 1/8] fdt: Fix up stdout correctly in fdt_fixup_stdout()

[Simon Glass]

Hi Bin,

On 13 January 2016 at 02:14, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Tue, Jan 12, 2016 at 12:58 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Bin,
>>
>> On 10 January 2016 at 20:02, Bin Meng <bmeng.cn@gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Fri, Jan 8, 2016 at 11:34 AM, Simon Glass <sjg@chromium.org> wrote:
>>>> Hi Bin,
>>>>
>>>> On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>>> When CONFIG_OF_STDOUT_VIA_ALIAS is on, always fix up kernel's stdout
>>>>> string with hardcoded CONFIG_CONS_INDEX.
>>>>>
>>>>> This actually reverts commit 3e303f748cf57fb23e8ec95ab7eac0074be50e2b
>>>>> "fdt_support: Add multi-serial support for stdout fixup", as the fix
>>>>
>>>> In that case, could this be a revert, created with 'git revert'?
>>>
>>> I've never used 'git revert' command. Did you mean this commit/patch
>>> should be recreated using 'git revert'? Does this matter?
>>
>> Well it creates a commit with a particular subject and format, which
>> people are used to seeing for reverts. So if it is actually a revert,
>> then yes I think you should use 'git revert'. You can edit the commit
>> message to provide more detail.
>>
>
> When running 'git revert 3e303f748cf57fb23e8ec95ab7eac0074be50e2b', I got:
>
> error: could not revert 3e303f7... fdt_support: Add multi-serial
> support for stdout fixup
> hint: after resolving the conflicts, mark the corrected paths
> hint: with 'git add <paths>' or 'git rm <paths>'
> hint: and commit the result with 'git commit'
>
> Looks it cannot be reverted cleanly using 'git revert' command. What's
> the best practice here?

I suggest using your original patch and updating the commit subject
and message to better match the output of 'git revert'. You can 'git
revert HEAD' to an example of see this.

Regards,
Simon

[U-Boot] [PATCH 2/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Bin Meng]

Convert ls1021atwr_nor to driver model support. As a start, enable
ns16550 serial port driver.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 arch/arm/dts/ls1021a-twr.dts     | 4 ++++
 arch/arm/dts/ls1021a.dtsi        | 4 ----
 configs/ls1021atwr_nor_defconfig | 4 ++++
 include/configs/ls1021atwr.h     | 2 ++
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/ls1021a-twr.dts b/arch/arm/dts/ls1021a-twr.dts
index 6ccd332..aead13f 100644
--- a/arch/arm/dts/ls1021a-twr.dts
+++ b/arch/arm/dts/ls1021a-twr.dts
@@ -19,6 +19,10 @@
 		spi0 = &qspi;
 		spi1 = &dspi1;
 	};
+
+	chosen {
+		stdout-path = &uart0;
+	};
 };
 
 &qspi {
diff --git a/arch/arm/dts/ls1021a.dtsi b/arch/arm/dts/ls1021a.dtsi
index 7fadd7c..ee0e554 100644
--- a/arch/arm/dts/ls1021a.dtsi
+++ b/arch/arm/dts/ls1021a.dtsi
@@ -218,7 +218,6 @@
 			compatible = "fsl,16550-FIFO64", "ns16550a";
 			reg = <0x21c0500 0x100>;
 			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
-			clock-frequency = <0>;
 			fifo-size = <15>;
 			status = "disabled";
 		};
@@ -227,7 +226,6 @@
 			compatible = "fsl,16550-FIFO64", "ns16550a";
 			reg = <0x21c0600 0x100>;
 			interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>;
-			clock-frequency = <0>;
 			fifo-size = <15>;
 			status = "disabled";
 		};
@@ -236,7 +234,6 @@
 			compatible = "fsl,16550-FIFO64", "ns16550a";
 			reg = <0x21d0500 0x100>;
 			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
-			clock-frequency = <0>;
 			fifo-size = <15>;
 			status = "disabled";
 		};
@@ -245,7 +242,6 @@
 			compatible = "fsl,16550-FIFO64", "ns16550a";
 			reg = <0x21d0600 0x100>;
 			interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
-			clock-frequency = <0>;
 			fifo-size = <15>;
 			status = "disabled";
 		};
diff --git a/configs/ls1021atwr_nor_defconfig b/configs/ls1021atwr_nor_defconfig
index aa874fd..2b79443 100644
--- a/configs/ls1021atwr_nor_defconfig
+++ b/configs/ls1021atwr_nor_defconfig
@@ -1,6 +1,10 @@
 CONFIG_ARM=y
 CONFIG_TARGET_LS1021ATWR=y
+CONFIG_DM_SERIAL=y
+CONFIG_DEFAULT_DEVICE_TREE="ls1021a-twr"
 # CONFIG_CMD_SETEXPR is not set
+CONFIG_OF_CONTROL=y
+CONFIG_DM=y
 CONFIG_NETDEVICES=y
 CONFIG_E1000=y
 CONFIG_SYS_NS16550=y
diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
index c12ba3a..bbef2a7 100644
--- a/include/configs/ls1021atwr.h
+++ b/include/configs/ls1021atwr.h
@@ -271,7 +271,9 @@
 #else
 #define CONFIG_CONS_INDEX		1
 #define CONFIG_SYS_NS16550_SERIAL
+#ifndef CONFIG_DM_SERIAL
 #define CONFIG_SYS_NS16550_REG_SIZE	1
+#endif
 #define CONFIG_SYS_NS16550_CLK		get_serial_clock()
 #endif
 
-- 
1.8.2.1

[U-Boot] [PATCH 3/8] serial: lpuart: Move CONFIG_FSL_LPUART to Kconfig

[Bin Meng]

LPUART is seen on Freescale VF610 and QorIQ Layerscape devices.
Create a Kconfig option and move it to defconfig for all boards
that have this serial driver.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 configs/colibri_vf_defconfig                 | 1 +
 configs/colibri_vf_dtb_defconfig             | 1 +
 configs/ls1021aqds_ddr4_nor_lpuart_defconfig | 1 +
 configs/ls1021aqds_nor_lpuart_defconfig      | 1 +
 configs/ls1021atwr_nor_lpuart_defconfig      | 1 +
 configs/pcm052_defconfig                     | 1 +
 configs/vf610twr_defconfig                   | 1 +
 configs/vf610twr_nand_defconfig              | 1 +
 drivers/serial/Kconfig                       | 6 ++++++
 include/configs/colibri_vf.h                 | 1 -
 include/configs/ls1021aqds.h                 | 1 -
 include/configs/ls1021atwr.h                 | 1 -
 include/configs/pcm052.h                     | 1 -
 include/configs/vf610twr.h                   | 1 -
 14 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/configs/colibri_vf_defconfig b/configs/colibri_vf_defconfig
index f8441e3..45917c8 100644
--- a/configs/colibri_vf_defconfig
+++ b/configs/colibri_vf_defconfig
@@ -8,3 +8,4 @@ CONFIG_CMD_GPIO=y
 CONFIG_DM=y
 CONFIG_NAND_VF610_NFC=y
 CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES=y
+CONFIG_FSL_LPUART=y
diff --git a/configs/colibri_vf_dtb_defconfig b/configs/colibri_vf_dtb_defconfig
index 3596cec..b1a843a 100644
--- a/configs/colibri_vf_dtb_defconfig
+++ b/configs/colibri_vf_dtb_defconfig
@@ -11,3 +11,4 @@ CONFIG_OF_CONTROL=y
 CONFIG_DM=y
 CONFIG_NAND_VF610_NFC=y
 CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES=y
+CONFIG_FSL_LPUART=y
diff --git a/configs/ls1021aqds_ddr4_nor_lpuart_defconfig b/configs/ls1021aqds_ddr4_nor_lpuart_defconfig
index 68bd117..44b2a0d 100644
--- a/configs/ls1021aqds_ddr4_nor_lpuart_defconfig
+++ b/configs/ls1021aqds_ddr4_nor_lpuart_defconfig
@@ -4,3 +4,4 @@ CONFIG_SYS_EXTRA_OPTIONS="SYS_FSL_DDR4,LPUART"
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_NETDEVICES=y
 CONFIG_E1000=y
+CONFIG_FSL_LPUART=y
diff --git a/configs/ls1021aqds_nor_lpuart_defconfig b/configs/ls1021aqds_nor_lpuart_defconfig
index b2f6832..1186af2 100644
--- a/configs/ls1021aqds_nor_lpuart_defconfig
+++ b/configs/ls1021aqds_nor_lpuart_defconfig
@@ -4,3 +4,4 @@ CONFIG_SYS_EXTRA_OPTIONS="LPUART"
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_NETDEVICES=y
 CONFIG_E1000=y
+CONFIG_FSL_LPUART=y
diff --git a/configs/ls1021atwr_nor_lpuart_defconfig b/configs/ls1021atwr_nor_lpuart_defconfig
index d7afca9..58cd61f 100644
--- a/configs/ls1021atwr_nor_lpuart_defconfig
+++ b/configs/ls1021atwr_nor_lpuart_defconfig
@@ -4,3 +4,4 @@ CONFIG_SYS_EXTRA_OPTIONS="LPUART"
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_NETDEVICES=y
 CONFIG_E1000=y
+CONFIG_FSL_LPUART=y
diff --git a/configs/pcm052_defconfig b/configs/pcm052_defconfig
index 9125645..26ab733 100644
--- a/configs/pcm052_defconfig
+++ b/configs/pcm052_defconfig
@@ -3,3 +3,4 @@ CONFIG_TARGET_PCM052=y
 CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/phytec/pcm052/imximage.cfg,ENV_IS_IN_NAND"
 CONFIG_NAND_VF610_NFC=y
 CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
+CONFIG_FSL_LPUART=y
diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig
index dc8df5c..d51c93b 100644
--- a/configs/vf610twr_defconfig
+++ b/configs/vf610twr_defconfig
@@ -6,3 +6,4 @@ CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_I
 CONFIG_NAND_VF610_NFC=y
 CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
 CONFIG_SPI_FLASH=y
+CONFIG_FSL_LPUART=y
diff --git a/configs/vf610twr_nand_defconfig b/configs/vf610twr_nand_defconfig
index 98880f3..299fa8f 100644
--- a/configs/vf610twr_nand_defconfig
+++ b/configs/vf610twr_nand_defconfig
@@ -6,3 +6,4 @@ CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_I
 CONFIG_NAND_VF610_NFC=y
 CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
 CONFIG_SPI_FLASH=y
+CONFIG_FSL_LPUART=y
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 1fc287e..3b54511 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -186,6 +186,12 @@ config ALTERA_UART
 	  Select this to enable an UART for Altera devices. Please find
 	  details on the "Embedded Peripherals IP User Guide" of Altera.
 
+config FSL_LPUART
+	bool "Freescale LPUART support"
+	help
+	  Select this to enable a Low Power UART for Freescale VF610 and
+	  QorIQ Layerscape devices.
+
 config SYS_NS16550
 	bool "NS16550 UART or compatible"
 	help
diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h
index 708c79a..5aed3a5 100644
--- a/include/configs/colibri_vf.h
+++ b/include/configs/colibri_vf.h
@@ -36,7 +36,6 @@
 
 #define CONFIG_BOARD_EARLY_INIT_F
 
-#define CONFIG_FSL_LPUART
 #define LPUART_BASE			UART0_BASE
 
 /* Allow to overwrite serial and ethaddr */
diff --git a/include/configs/ls1021aqds.h b/include/configs/ls1021aqds.h
index 2e8dbc7..e8b1eca 100644
--- a/include/configs/ls1021aqds.h
+++ b/include/configs/ls1021aqds.h
@@ -371,7 +371,6 @@ unsigned long get_board_ddr_clk(void);
  * Serial Port
  */
 #ifdef CONFIG_LPUART
-#define CONFIG_FSL_LPUART
 #define CONFIG_LPUART_32B_REG
 #else
 #define CONFIG_CONS_INDEX		1
diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
index bbef2a7..317ba62 100644
--- a/include/configs/ls1021atwr.h
+++ b/include/configs/ls1021atwr.h
@@ -266,7 +266,6 @@
  * Serial Port
  */
 #ifdef CONFIG_LPUART
-#define CONFIG_FSL_LPUART
 #define CONFIG_LPUART_32B_REG
 #else
 #define CONFIG_CONS_INDEX		1
diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
index b851bba..891bdb0 100644
--- a/include/configs/pcm052.h
+++ b/include/configs/pcm052.h
@@ -27,7 +27,6 @@
 
 #define CONFIG_BOARD_EARLY_INIT_F
 
-#define CONFIG_FSL_LPUART
 #define LPUART_BASE			UART1_BASE
 
 /* Allow to overwrite serial and ethaddr */
diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h
index 34df6f0..dcfafaf 100644
--- a/include/configs/vf610twr.h
+++ b/include/configs/vf610twr.h
@@ -34,7 +34,6 @@
 
 #define CONFIG_BOARD_EARLY_INIT_F
 
-#define CONFIG_FSL_LPUART
 #define LPUART_BASE			UART1_BASE
 
 /* Allow to overwrite serial and ethaddr */
-- 
1.8.2.1

[U-Boot] [PATCH 3/8] serial: lpuart: Move CONFIG_FSL_LPUART to Kconfig

[Simon Glass]

On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> LPUART is seen on Freescale VF610 and QorIQ Layerscape devices.
> Create a Kconfig option and move it to defconfig for all boards
> that have this serial driver.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  configs/colibri_vf_defconfig                 | 1 +
>  configs/colibri_vf_dtb_defconfig             | 1 +
>  configs/ls1021aqds_ddr4_nor_lpuart_defconfig | 1 +
>  configs/ls1021aqds_nor_lpuart_defconfig      | 1 +
>  configs/ls1021atwr_nor_lpuart_defconfig      | 1 +
>  configs/pcm052_defconfig                     | 1 +
>  configs/vf610twr_defconfig                   | 1 +
>  configs/vf610twr_nand_defconfig              | 1 +
>  drivers/serial/Kconfig                       | 6 ++++++
>  include/configs/colibri_vf.h                 | 1 -
>  include/configs/ls1021aqds.h                 | 1 -
>  include/configs/ls1021atwr.h                 | 1 -
>  include/configs/pcm052.h                     | 1 -
>  include/configs/vf610twr.h                   | 1 -
>  14 files changed, 14 insertions(+), 5 deletions(-)

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

[U-Boot] [PATCH 3/8] serial: lpuart: Move CONFIG_FSL_LPUART to Kconfig

[Stefan Agner]

Hi Bin,

On 2015-12-31 00:53, Bin Meng wrote:
> LPUART is seen on Freescale VF610 and QorIQ Layerscape devices.
> Create a Kconfig option and move it to defconfig for all boards
> that have this serial driver.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  configs/colibri_vf_defconfig                 | 1 +
>  configs/colibri_vf_dtb_defconfig             | 1 +
>  configs/ls1021aqds_ddr4_nor_lpuart_defconfig | 1 +
>  configs/ls1021aqds_nor_lpuart_defconfig      | 1 +
>  configs/ls1021atwr_nor_lpuart_defconfig      | 1 +
>  configs/pcm052_defconfig                     | 1 +
>  configs/vf610twr_defconfig                   | 1 +
>  configs/vf610twr_nand_defconfig              | 1 +
>  drivers/serial/Kconfig                       | 6 ++++++
>  include/configs/colibri_vf.h                 | 1 -

It would have been nice to CC the maintainers of these boards so they
are aware of the change.

As Bhuvan pointed out, this patchset as is breaks colibri_vf_defconfig
drivers/serial/serial_lpuart.c: In function ?lpuart_serial_tstc?:
drivers/serial/serial_lpuart.c:148:9: error: too few arguments to
function ?_lpuart_serial_tstc?
  return _lpuart_serial_tstc();


--
Stefan

>  include/configs/ls1021aqds.h                 | 1 -
>  include/configs/ls1021atwr.h                 | 1 -
>  include/configs/pcm052.h                     | 1 -
>  include/configs/vf610twr.h                   | 1 -
>  14 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/configs/colibri_vf_defconfig b/configs/colibri_vf_defconfig
> index f8441e3..45917c8 100644
> --- a/configs/colibri_vf_defconfig
> +++ b/configs/colibri_vf_defconfig
> @@ -8,3 +8,4 @@ CONFIG_CMD_GPIO=y
>  CONFIG_DM=y
>  CONFIG_NAND_VF610_NFC=y
>  CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES=y
> +CONFIG_FSL_LPUART=y
> diff --git a/configs/colibri_vf_dtb_defconfig b/configs/colibri_vf_dtb_defconfig
> index 3596cec..b1a843a 100644
> --- a/configs/colibri_vf_dtb_defconfig
> +++ b/configs/colibri_vf_dtb_defconfig
> @@ -11,3 +11,4 @@ CONFIG_OF_CONTROL=y
>  CONFIG_DM=y
>  CONFIG_NAND_VF610_NFC=y
>  CONFIG_SYS_NAND_VF610_NFC_60_ECC_BYTES=y
> +CONFIG_FSL_LPUART=y
> diff --git a/configs/ls1021aqds_ddr4_nor_lpuart_defconfig
> b/configs/ls1021aqds_ddr4_nor_lpuart_defconfig
> index 68bd117..44b2a0d 100644
> --- a/configs/ls1021aqds_ddr4_nor_lpuart_defconfig
> +++ b/configs/ls1021aqds_ddr4_nor_lpuart_defconfig
> @@ -4,3 +4,4 @@ CONFIG_SYS_EXTRA_OPTIONS="SYS_FSL_DDR4,LPUART"
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_NETDEVICES=y
>  CONFIG_E1000=y
> +CONFIG_FSL_LPUART=y
> diff --git a/configs/ls1021aqds_nor_lpuart_defconfig
> b/configs/ls1021aqds_nor_lpuart_defconfig
> index b2f6832..1186af2 100644
> --- a/configs/ls1021aqds_nor_lpuart_defconfig
> +++ b/configs/ls1021aqds_nor_lpuart_defconfig
> @@ -4,3 +4,4 @@ CONFIG_SYS_EXTRA_OPTIONS="LPUART"
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_NETDEVICES=y
>  CONFIG_E1000=y
> +CONFIG_FSL_LPUART=y
> diff --git a/configs/ls1021atwr_nor_lpuart_defconfig
> b/configs/ls1021atwr_nor_lpuart_defconfig
> index d7afca9..58cd61f 100644
> --- a/configs/ls1021atwr_nor_lpuart_defconfig
> +++ b/configs/ls1021atwr_nor_lpuart_defconfig
> @@ -4,3 +4,4 @@ CONFIG_SYS_EXTRA_OPTIONS="LPUART"
>  # CONFIG_CMD_SETEXPR is not set
>  CONFIG_NETDEVICES=y
>  CONFIG_E1000=y
> +CONFIG_FSL_LPUART=y
> diff --git a/configs/pcm052_defconfig b/configs/pcm052_defconfig
> index 9125645..26ab733 100644
> --- a/configs/pcm052_defconfig
> +++ b/configs/pcm052_defconfig
> @@ -3,3 +3,4 @@ CONFIG_TARGET_PCM052=y
>
> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/phytec/pcm052/imximage.cfg,ENV_IS_IN_NAND"
>  CONFIG_NAND_VF610_NFC=y
>  CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
> +CONFIG_FSL_LPUART=y
> diff --git a/configs/vf610twr_defconfig b/configs/vf610twr_defconfig
> index dc8df5c..d51c93b 100644
> --- a/configs/vf610twr_defconfig
> +++ b/configs/vf610twr_defconfig
> @@ -6,3 +6,4 @@
> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_I
>  CONFIG_NAND_VF610_NFC=y
>  CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
>  CONFIG_SPI_FLASH=y
> +CONFIG_FSL_LPUART=y
> diff --git a/configs/vf610twr_nand_defconfig b/configs/vf610twr_nand_defconfig
> index 98880f3..299fa8f 100644
> --- a/configs/vf610twr_nand_defconfig
> +++ b/configs/vf610twr_nand_defconfig
> @@ -6,3 +6,4 @@
> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/freescale/vf610twr/imximage.cfg,ENV_I
>  CONFIG_NAND_VF610_NFC=y
>  CONFIG_SYS_NAND_BUSWIDTH_16BIT=y
>  CONFIG_SPI_FLASH=y
> +CONFIG_FSL_LPUART=y
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 1fc287e..3b54511 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -186,6 +186,12 @@ config ALTERA_UART
>  	  Select this to enable an UART for Altera devices. Please find
>  	  details on the "Embedded Peripherals IP User Guide" of Altera.
>  
> +config FSL_LPUART
> +	bool "Freescale LPUART support"
> +	help
> +	  Select this to enable a Low Power UART for Freescale VF610 and
> +	  QorIQ Layerscape devices.
> +
>  config SYS_NS16550
>  	bool "NS16550 UART or compatible"
>  	help
> diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h
> index 708c79a..5aed3a5 100644
> --- a/include/configs/colibri_vf.h
> +++ b/include/configs/colibri_vf.h
> @@ -36,7 +36,6 @@
>  
>  #define CONFIG_BOARD_EARLY_INIT_F
>  
> -#define CONFIG_FSL_LPUART
>  #define LPUART_BASE			UART0_BASE
>  
>  /* Allow to overwrite serial and ethaddr */
> diff --git a/include/configs/ls1021aqds.h b/include/configs/ls1021aqds.h
> index 2e8dbc7..e8b1eca 100644
> --- a/include/configs/ls1021aqds.h
> +++ b/include/configs/ls1021aqds.h
> @@ -371,7 +371,6 @@ unsigned long get_board_ddr_clk(void);
>   * Serial Port
>   */
>  #ifdef CONFIG_LPUART
> -#define CONFIG_FSL_LPUART
>  #define CONFIG_LPUART_32B_REG
>  #else
>  #define CONFIG_CONS_INDEX		1
> diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
> index bbef2a7..317ba62 100644
> --- a/include/configs/ls1021atwr.h
> +++ b/include/configs/ls1021atwr.h
> @@ -266,7 +266,6 @@
>   * Serial Port
>   */
>  #ifdef CONFIG_LPUART
> -#define CONFIG_FSL_LPUART
>  #define CONFIG_LPUART_32B_REG
>  #else
>  #define CONFIG_CONS_INDEX		1
> diff --git a/include/configs/pcm052.h b/include/configs/pcm052.h
> index b851bba..891bdb0 100644
> --- a/include/configs/pcm052.h
> +++ b/include/configs/pcm052.h
> @@ -27,7 +27,6 @@
>  
>  #define CONFIG_BOARD_EARLY_INIT_F
>  
> -#define CONFIG_FSL_LPUART
>  #define LPUART_BASE			UART1_BASE
>  
>  /* Allow to overwrite serial and ethaddr */
> diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h
> index 34df6f0..dcfafaf 100644
> --- a/include/configs/vf610twr.h
> +++ b/include/configs/vf610twr.h
> @@ -34,7 +34,6 @@
>  
>  #define CONFIG_BOARD_EARLY_INIT_F
>  
> -#define CONFIG_FSL_LPUART
>  #define LPUART_BASE			UART1_BASE
>  
>  /* Allow to overwrite serial and ethaddr */

[U-Boot] [PATCH 4/8] serial: lpuart: Fix several cosmetic issues

[Bin Meng]

Clean up the driver codes a little bit, by:
- Use tab instead of space in the macro defines
- Use single line comment whenever possible
- Fix insertion of blank lines

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/serial/serial_lpuart.c | 35 +++++++++++++++--------------------
 1 file changed, 15 insertions(+), 20 deletions(-)

diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
index 63fc388..ae47183 100644
--- a/drivers/serial/serial_lpuart.c
+++ b/drivers/serial/serial_lpuart.c
@@ -12,15 +12,15 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/clock.h>
 
-#define US1_TDRE        (1 << 7)
-#define US1_RDRF        (1 << 5)
-#define US1_OR          (1 << 3)
-#define UC2_TE          (1 << 3)
-#define UC2_RE          (1 << 2)
-#define CFIFO_TXFLUSH   (1 << 7)
-#define CFIFO_RXFLUSH   (1 << 6)
-#define SFIFO_RXOF      (1 << 2)
-#define SFIFO_RXUF      (1 << 0)
+#define US1_TDRE	(1 << 7)
+#define US1_RDRF	(1 << 5)
+#define US1_OR		(1 << 3)
+#define UC2_TE		(1 << 3)
+#define UC2_RE		(1 << 2)
+#define CFIFO_TXFLUSH	(1 << 7)
+#define CFIFO_RXFLUSH	(1 << 6)
+#define SFIFO_RXOF	(1 << 2)
+#define SFIFO_RXUF	(1 << 0)
 
 #define STAT_LBKDIF	(1 << 31)
 #define STAT_RXEDGIF	(1 << 30)
@@ -34,7 +34,7 @@
 #define STAT_MA1F	(1 << 15)
 #define STAT_MA2F	(1 << 14)
 #define STAT_FLAGS	(STAT_LBKDIF | STAT_RXEDGIF | STAT_IDLE | STAT_OR | \
-			STAT_NF | STAT_FE | STAT_PF | STAT_MA1F | STAT_MA2F)
+			 STAT_NF | STAT_FE | STAT_PF | STAT_MA1F | STAT_MA2F)
 
 #define CTRL_TE		(1 << 19)
 #define CTRL_RE		(1 << 18)
@@ -59,8 +59,8 @@ static void lpuart_serial_setbrg(void)
 		gd->baudrate = CONFIG_BAUDRATE;
 
 	sbr = (u16)(clk / (16 * gd->baudrate));
-	/* place adjustment later - n/32 BRFA */
 
+	/* place adjustment later - n/32 BRFA */
 	__raw_writeb(sbr >> 8, &base->ubdh);
 	__raw_writeb(sbr & 0xff, &base->ubdl);
 }
@@ -86,9 +86,7 @@ static void lpuart_serial_putc(const char c)
 	__raw_writeb(c, &base->ud);
 }
 
-/*
- * Test whether a character is in the RX buffer
- */
+/* Test whether a character is in the RX buffer */
 static int lpuart_serial_tstc(void)
 {
 	if (__raw_readb(&base->urcfifo) == 0)
@@ -120,7 +118,6 @@ static int lpuart_serial_init(void)
 	__raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &base->ucfifo);
 
 	/* provide data bits, parity, stop bit, etc */
-
 	serial_setbrg();
 
 	__raw_writeb(UC2_RE | UC2_TE, &base->uc2);
@@ -148,8 +145,8 @@ static void lpuart32_serial_setbrg(void)
 		gd->baudrate = CONFIG_BAUDRATE;
 
 	sbr = (clk / (16 * gd->baudrate));
-	/* place adjustment later - n/32 BRFA */
 
+	/* place adjustment later - n/32 BRFA */
 	out_be32(&base->baud, sbr);
 }
 
@@ -176,9 +173,7 @@ static void lpuart32_serial_putc(const char c)
 	out_be32(&base->data, c);
 }
 
-/*
- * Test whether a character is in the RX buffer
- */
+/* Test whether a character is in the RX buffer */
 static int lpuart32_serial_tstc(void)
 {
 	if ((in_be32(&base->water) >> 24) == 0)
@@ -204,8 +199,8 @@ static int lpuart32_serial_init(void)
 	out_be32(&base->fifo, ~(FIFO_TXFE | FIFO_RXFE));
 
 	out_be32(&base->match, 0);
-	/* provide data bits, parity, stop bit, etc */
 
+	/* provide data bits, parity, stop bit, etc */
 	serial_setbrg();
 
 	out_be32(&base->ctrl, CTRL_RE | CTRL_TE);
-- 
1.8.2.1

[U-Boot] [PATCH 4/8] serial: lpuart: Fix several cosmetic issues

[Simon Glass]

On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> Clean up the driver codes a little bit, by:
> - Use tab instead of space in the macro defines
> - Use single line comment whenever possible
> - Fix insertion of blank lines
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/serial/serial_lpuart.c | 35 +++++++++++++++--------------------
>  1 file changed, 15 insertions(+), 20 deletions(-)

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

[U-Boot] [PATCH 5/8] serial: lpuart: Call local version of setbrg and putc directly

[Bin Meng]

There is no need to go through serial driver subsystem, instead
call the driver's setbrg and putc routines directly.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/serial/serial_lpuart.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
index ae47183..0c0ab87 100644
--- a/drivers/serial/serial_lpuart.c
+++ b/drivers/serial/serial_lpuart.c
@@ -78,7 +78,7 @@ static int lpuart_serial_getc(void)
 static void lpuart_serial_putc(const char c)
 {
 	if (c == '\n')
-		serial_putc('\r');
+		lpuart_serial_putc('\r');
 
 	while (!(__raw_readb(&base->us1) & US1_TDRE))
 		WATCHDOG_RESET();
@@ -118,7 +118,7 @@ static int lpuart_serial_init(void)
 	__raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &base->ucfifo);
 
 	/* provide data bits, parity, stop bit, etc */
-	serial_setbrg();
+	lpuart_serial_setbrg();
 
 	__raw_writeb(UC2_RE | UC2_TE, &base->uc2);
 
@@ -165,7 +165,7 @@ static int lpuart32_serial_getc(void)
 static void lpuart32_serial_putc(const char c)
 {
 	if (c == '\n')
-		serial_putc('\r');
+		lpuart32_serial_putc('\r');
 
 	while (!(in_be32(&base->stat) & STAT_TDRE))
 		WATCHDOG_RESET();
@@ -201,7 +201,7 @@ static int lpuart32_serial_init(void)
 	out_be32(&base->match, 0);
 
 	/* provide data bits, parity, stop bit, etc */
-	serial_setbrg();
+	lpuart32_serial_setbrg();
 
 	out_be32(&base->ctrl, CTRL_RE | CTRL_TE);
 
-- 
1.8.2.1

[U-Boot] [PATCH 5/8] serial: lpuart: Call local version of setbrg and putc directly

[Simon Glass]

On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> There is no need to go through serial driver subsystem, instead
> call the driver's setbrg and putc routines directly.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/serial/serial_lpuart.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

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

[U-Boot] [PATCH 6/8] serial: lpuart: Prepare the driver for DM conversion

[Bin Meng]

Create internal routines which take lpuart's register base as
a parameter, in preparation for driver model conversion.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 drivers/serial/serial_lpuart.c | 146 +++++++++++++++++++++++++++--------------
 1 file changed, 95 insertions(+), 51 deletions(-)

diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
index 0c0ab87..fed83a6 100644
--- a/drivers/serial/serial_lpuart.c
+++ b/drivers/serial/serial_lpuart.c
@@ -50,46 +50,43 @@ DECLARE_GLOBAL_DATA_PTR;
 struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
 
 #ifndef CONFIG_LPUART_32B_REG
-static void lpuart_serial_setbrg(void)
+static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
 {
 	u32 clk = mxc_get_clock(MXC_UART_CLK);
 	u16 sbr;
 
-	if (!gd->baudrate)
-		gd->baudrate = CONFIG_BAUDRATE;
-
-	sbr = (u16)(clk / (16 * gd->baudrate));
+	sbr = (u16)(clk / (16 * baudrate));
 
 	/* place adjustment later - n/32 BRFA */
-	__raw_writeb(sbr >> 8, &base->ubdh);
-	__raw_writeb(sbr & 0xff, &base->ubdl);
+	__raw_writeb(sbr >> 8, &reg->ubdh);
+	__raw_writeb(sbr & 0xff, &reg->ubdl);
 }
 
-static int lpuart_serial_getc(void)
+static int _lpuart_serial_getc(struct lpuart_fsl *reg)
 {
-	while (!(__raw_readb(&base->us1) & (US1_RDRF | US1_OR)))
+	while (!(__raw_readb(&reg->us1) & (US1_RDRF | US1_OR)))
 		WATCHDOG_RESET();
 
 	barrier();
 
-	return __raw_readb(&base->ud);
+	return __raw_readb(&reg->ud);
 }
 
-static void lpuart_serial_putc(const char c)
+static void _lpuart_serial_putc(struct lpuart_fsl *reg, const char c)
 {
 	if (c == '\n')
-		lpuart_serial_putc('\r');
+		_lpuart_serial_putc(reg, '\r');
 
-	while (!(__raw_readb(&base->us1) & US1_TDRE))
+	while (!(__raw_readb(&reg->us1) & US1_TDRE))
 		WATCHDOG_RESET();
 
-	__raw_writeb(c, &base->ud);
+	__raw_writeb(c, &reg->ud);
 }
 
 /* Test whether a character is in the RX buffer */
-static int lpuart_serial_tstc(void)
+static int _lpuart_serial_tstc(struct lpuart_fsl *reg)
 {
-	if (__raw_readb(&base->urcfifo) == 0)
+	if (__raw_readb(&reg->urcfifo) == 0)
 		return 0;
 
 	return 1;
@@ -99,32 +96,57 @@ static int lpuart_serial_tstc(void)
  * Initialise the serial port with the given baudrate. The settings
  * are always 8 data bits, no parity, 1 stop bit, no start bits.
  */
-static int lpuart_serial_init(void)
+static int _lpuart_serial_init(struct lpuart_fsl *reg)
 {
 	u8 ctrl;
 
-	ctrl = __raw_readb(&base->uc2);
+	ctrl = __raw_readb(&reg->uc2);
 	ctrl &= ~UC2_RE;
 	ctrl &= ~UC2_TE;
-	__raw_writeb(ctrl, &base->uc2);
+	__raw_writeb(ctrl, &reg->uc2);
 
-	__raw_writeb(0, &base->umodem);
-	__raw_writeb(0, &base->uc1);
+	__raw_writeb(0, &reg->umodem);
+	__raw_writeb(0, &reg->uc1);
 
 	/* Disable FIFO and flush buffer */
-	__raw_writeb(0x0, &base->upfifo);
-	__raw_writeb(0x0, &base->utwfifo);
-	__raw_writeb(0x1, &base->urwfifo);
-	__raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &base->ucfifo);
+	__raw_writeb(0x0, &reg->upfifo);
+	__raw_writeb(0x0, &reg->utwfifo);
+	__raw_writeb(0x1, &reg->urwfifo);
+	__raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &reg->ucfifo);
 
 	/* provide data bits, parity, stop bit, etc */
-	lpuart_serial_setbrg();
+	_lpuart_serial_setbrg(reg, gd->baudrate);
 
-	__raw_writeb(UC2_RE | UC2_TE, &base->uc2);
+	__raw_writeb(UC2_RE | UC2_TE, &reg->uc2);
 
 	return 0;
 }
 
+static void lpuart_serial_setbrg(void)
+{
+	_lpuart_serial_setbrg(base, gd->baudrate);
+}
+
+static int lpuart_serial_getc(void)
+{
+	return _lpuart_serial_getc(base);
+}
+
+static void lpuart_serial_putc(const char c)
+{
+	_lpuart_serial_putc(base, c);
+}
+
+static int lpuart_serial_tstc(void)
+{
+	return _lpuart_serial_tstc();
+}
+
+static int lpuart_serial_init(void)
+{
+	return _lpuart_serial_init(base);
+}
+
 static struct serial_device lpuart_serial_drv = {
 	.name = "lpuart_serial",
 	.start = lpuart_serial_init,
@@ -136,47 +158,44 @@ static struct serial_device lpuart_serial_drv = {
 	.tstc = lpuart_serial_tstc,
 };
 #else
-static void lpuart32_serial_setbrg(void)
+static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
 {
 	u32 clk = CONFIG_SYS_CLK_FREQ;
 	u32 sbr;
 
-	if (!gd->baudrate)
-		gd->baudrate = CONFIG_BAUDRATE;
-
-	sbr = (clk / (16 * gd->baudrate));
+	sbr = (clk / (16 * baudrate));
 
 	/* place adjustment later - n/32 BRFA */
-	out_be32(&base->baud, sbr);
+	out_be32(&reg->baud, sbr);
 }
 
-static int lpuart32_serial_getc(void)
+static int _lpuart32_serial_getc(struct lpuart_fsl *reg)
 {
 	u32 stat;
 
-	while (((stat = in_be32(&base->stat)) & STAT_RDRF) == 0) {
-		out_be32(&base->stat, STAT_FLAGS);
+	while (((stat = in_be32(&reg->stat)) & STAT_RDRF) == 0) {
+		out_be32(&reg->stat, STAT_FLAGS);
 		WATCHDOG_RESET();
 	}
 
-	return in_be32(&base->data) & 0x3ff;
+	return in_be32(&reg->data) & 0x3ff;
 }
 
-static void lpuart32_serial_putc(const char c)
+static void _lpuart32_serial_putc(struct lpuart_fsl *reg, const char c)
 {
 	if (c == '\n')
-		lpuart32_serial_putc('\r');
+		_lpuart32_serial_putc(reg, '\r');
 
-	while (!(in_be32(&base->stat) & STAT_TDRE))
+	while (!(in_be32(&reg->stat) & STAT_TDRE))
 		WATCHDOG_RESET();
 
-	out_be32(&base->data, c);
+	out_be32(&reg->data, c);
 }
 
 /* Test whether a character is in the RX buffer */
-static int lpuart32_serial_tstc(void)
+static int _lpuart32_serial_tstc(struct lpuart_fsl *reg)
 {
-	if ((in_be32(&base->water) >> 24) == 0)
+	if ((in_be32(&reg->water) >> 24) == 0)
 		return 0;
 
 	return 1;
@@ -186,28 +205,53 @@ static int lpuart32_serial_tstc(void)
  * Initialise the serial port with the given baudrate. The settings
  * are always 8 data bits, no parity, 1 stop bit, no start bits.
  */
-static int lpuart32_serial_init(void)
+static int _lpuart32_serial_init(struct lpuart_fsl *reg)
 {
 	u8 ctrl;
 
-	ctrl = in_be32(&base->ctrl);
+	ctrl = in_be32(&reg->ctrl);
 	ctrl &= ~CTRL_RE;
 	ctrl &= ~CTRL_TE;
-	out_be32(&base->ctrl, ctrl);
+	out_be32(&reg->ctrl, ctrl);
 
-	out_be32(&base->modir, 0);
-	out_be32(&base->fifo, ~(FIFO_TXFE | FIFO_RXFE));
+	out_be32(&reg->modir, 0);
+	out_be32(&reg->fifo, ~(FIFO_TXFE | FIFO_RXFE));
 
-	out_be32(&base->match, 0);
+	out_be32(&reg->match, 0);
 
 	/* provide data bits, parity, stop bit, etc */
-	lpuart32_serial_setbrg();
+	_lpuart32_serial_setbrg(reg, gd->baudrate);
 
-	out_be32(&base->ctrl, CTRL_RE | CTRL_TE);
+	out_be32(&reg->ctrl, CTRL_RE | CTRL_TE);
 
 	return 0;
 }
 
+static void lpuart32_serial_setbrg(void)
+{
+	_lpuart32_serial_setbrg(base, gd->baudrate);
+}
+
+static int lpuart32_serial_getc(void)
+{
+	return _lpuart32_serial_getc(base);
+}
+
+static void lpuart32_serial_putc(const char c)
+{
+	_lpuart32_serial_putc(base, c);
+}
+
+static int lpuart32_serial_tstc(void)
+{
+	return _lpuart32_serial_tstc(base);
+}
+
+static int lpuart32_serial_init(void)
+{
+	return _lpuart32_serial_init(base);
+}
+
 static struct serial_device lpuart32_serial_drv = {
 	.name = "lpuart32_serial",
 	.start = lpuart32_serial_init,
-- 
1.8.2.1

[U-Boot] [PATCH 6/8] serial: lpuart: Prepare the driver for DM conversion

[Simon Glass]

On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> Create internal routines which take lpuart's register base as
> a parameter, in preparation for driver model conversion.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  drivers/serial/serial_lpuart.c | 146 +++++++++++++++++++++++++++--------------
>  1 file changed, 95 insertions(+), 51 deletions(-)

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

[U-Boot] [PATCH 6/8] serial: lpuart: Prepare the driver for DM conversion

[Stefan Agner]

On 2015-12-31 00:53, Bin Meng wrote:
> Create internal routines which take lpuart's register base as
> a parameter, in preparation for driver model conversion.
> 
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
> 
>  drivers/serial/serial_lpuart.c | 146 +++++++++++++++++++++++++++--------------
>  1 file changed, 95 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
> index 0c0ab87..fed83a6 100644
> --- a/drivers/serial/serial_lpuart.c
> +++ b/drivers/serial/serial_lpuart.c
> @@ -50,46 +50,43 @@ DECLARE_GLOBAL_DATA_PTR;
>  struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
>  
>  #ifndef CONFIG_LPUART_32B_REG
> -static void lpuart_serial_setbrg(void)
> +static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
>  	u32 clk = mxc_get_clock(MXC_UART_CLK);
>  	u16 sbr;
>  
> -	if (!gd->baudrate)
> -		gd->baudrate = CONFIG_BAUDRATE;
> -
> -	sbr = (u16)(clk / (16 * gd->baudrate));
> +	sbr = (u16)(clk / (16 * baudrate));
>  
>  	/* place adjustment later - n/32 BRFA */
> -	__raw_writeb(sbr >> 8, &base->ubdh);
> -	__raw_writeb(sbr & 0xff, &base->ubdl);
> +	__raw_writeb(sbr >> 8, &reg->ubdh);
> +	__raw_writeb(sbr & 0xff, &reg->ubdl);
>  }
>  
> -static int lpuart_serial_getc(void)
> +static int _lpuart_serial_getc(struct lpuart_fsl *reg)
>  {
> -	while (!(__raw_readb(&base->us1) & (US1_RDRF | US1_OR)))
> +	while (!(__raw_readb(&reg->us1) & (US1_RDRF | US1_OR)))
>  		WATCHDOG_RESET();
>  
>  	barrier();
>  
> -	return __raw_readb(&base->ud);
> +	return __raw_readb(&reg->ud);
>  }
>  
> -static void lpuart_serial_putc(const char c)
> +static void _lpuart_serial_putc(struct lpuart_fsl *reg, const char c)
>  {
>  	if (c == '\n')
> -		lpuart_serial_putc('\r');
> +		_lpuart_serial_putc(reg, '\r');
>  
> -	while (!(__raw_readb(&base->us1) & US1_TDRE))
> +	while (!(__raw_readb(&reg->us1) & US1_TDRE))
>  		WATCHDOG_RESET();
>  
> -	__raw_writeb(c, &base->ud);
> +	__raw_writeb(c, &reg->ud);
>  }
>  
>  /* Test whether a character is in the RX buffer */
> -static int lpuart_serial_tstc(void)
> +static int _lpuart_serial_tstc(struct lpuart_fsl *reg)
>  {
> -	if (__raw_readb(&base->urcfifo) == 0)
> +	if (__raw_readb(&reg->urcfifo) == 0)
>  		return 0;
>  
>  	return 1;
> @@ -99,32 +96,57 @@ static int lpuart_serial_tstc(void)
>   * Initialise the serial port with the given baudrate. The settings
>   * are always 8 data bits, no parity, 1 stop bit, no start bits.
>   */
> -static int lpuart_serial_init(void)
> +static int _lpuart_serial_init(struct lpuart_fsl *reg)

Couldn't you just name the parameter base and get rid of the changes
below? IMHO, renaming variables is only really necessary if the current
name is misleading, which I don't think it is...

>  {
>  	u8 ctrl;
>  
> -	ctrl = __raw_readb(&base->uc2);
> +	ctrl = __raw_readb(&reg->uc2);
>  	ctrl &= ~UC2_RE;
>  	ctrl &= ~UC2_TE;
> -	__raw_writeb(ctrl, &base->uc2);
> +	__raw_writeb(ctrl, &reg->uc2);
>  
> -	__raw_writeb(0, &base->umodem);
> -	__raw_writeb(0, &base->uc1);
> +	__raw_writeb(0, &reg->umodem);
> +	__raw_writeb(0, &reg->uc1);
>  
>  	/* Disable FIFO and flush buffer */
> -	__raw_writeb(0x0, &base->upfifo);
> -	__raw_writeb(0x0, &base->utwfifo);
> -	__raw_writeb(0x1, &base->urwfifo);
> -	__raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &base->ucfifo);
> +	__raw_writeb(0x0, &reg->upfifo);
> +	__raw_writeb(0x0, &reg->utwfifo);
> +	__raw_writeb(0x1, &reg->urwfifo);
> +	__raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &reg->ucfifo);
>  
>  	/* provide data bits, parity, stop bit, etc */
> -	lpuart_serial_setbrg();
> +	_lpuart_serial_setbrg(reg, gd->baudrate);
>  
> -	__raw_writeb(UC2_RE | UC2_TE, &base->uc2);
> +	__raw_writeb(UC2_RE | UC2_TE, &reg->uc2);
>  
>  	return 0;
>  }
>  
> +static void lpuart_serial_setbrg(void)
> +{
> +	_lpuart_serial_setbrg(base, gd->baudrate);
> +}
> +
> +static int lpuart_serial_getc(void)
> +{
> +	return _lpuart_serial_getc(base);
> +}
> +
> +static void lpuart_serial_putc(const char c)
> +{
> +	_lpuart_serial_putc(base, c);
> +}
> +
> +static int lpuart_serial_tstc(void)
> +{
> +	return _lpuart_serial_tstc();
> +}
> +
> +static int lpuart_serial_init(void)
> +{
> +	return _lpuart_serial_init(base);
> +}
> +
>  static struct serial_device lpuart_serial_drv = {
>  	.name = "lpuart_serial",
>  	.start = lpuart_serial_init,
> @@ -136,47 +158,44 @@ static struct serial_device lpuart_serial_drv = {
>  	.tstc = lpuart_serial_tstc,
>  };
>  #else
> -static void lpuart32_serial_setbrg(void)
> +static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
>  	u32 clk = CONFIG_SYS_CLK_FREQ;
>  	u32 sbr;
>  
> -	if (!gd->baudrate)
> -		gd->baudrate = CONFIG_BAUDRATE;
> -
> -	sbr = (clk / (16 * gd->baudrate));
> +	sbr = (clk / (16 * baudrate));
>  
>  	/* place adjustment later - n/32 BRFA */
> -	out_be32(&base->baud, sbr);
> +	out_be32(&reg->baud, sbr);
>  }
>  
> -static int lpuart32_serial_getc(void)
> +static int _lpuart32_serial_getc(struct lpuart_fsl *reg)
>  {
>  	u32 stat;
>  
> -	while (((stat = in_be32(&base->stat)) & STAT_RDRF) == 0) {
> -		out_be32(&base->stat, STAT_FLAGS);
> +	while (((stat = in_be32(&reg->stat)) & STAT_RDRF) == 0) {
> +		out_be32(&reg->stat, STAT_FLAGS);
>  		WATCHDOG_RESET();
>  	}
>  
> -	return in_be32(&base->data) & 0x3ff;
> +	return in_be32(&reg->data) & 0x3ff;
>  }
>  
> -static void lpuart32_serial_putc(const char c)
> +static void _lpuart32_serial_putc(struct lpuart_fsl *reg, const char c)
>  {
>  	if (c == '\n')
> -		lpuart32_serial_putc('\r');
> +		_lpuart32_serial_putc(reg, '\r');
>  
> -	while (!(in_be32(&base->stat) & STAT_TDRE))
> +	while (!(in_be32(&reg->stat) & STAT_TDRE))
>  		WATCHDOG_RESET();
>  
> -	out_be32(&base->data, c);
> +	out_be32(&reg->data, c);
>  }
>  
>  /* Test whether a character is in the RX buffer */
> -static int lpuart32_serial_tstc(void)
> +static int _lpuart32_serial_tstc(struct lpuart_fsl *reg)
>  {
> -	if ((in_be32(&base->water) >> 24) == 0)
> +	if ((in_be32(&reg->water) >> 24) == 0)
>  		return 0;
>  
>  	return 1;
> @@ -186,28 +205,53 @@ static int lpuart32_serial_tstc(void)
>   * Initialise the serial port with the given baudrate. The settings
>   * are always 8 data bits, no parity, 1 stop bit, no start bits.
>   */
> -static int lpuart32_serial_init(void)
> +static int _lpuart32_serial_init(struct lpuart_fsl *reg)
>  {
>  	u8 ctrl;
>  
> -	ctrl = in_be32(&base->ctrl);
> +	ctrl = in_be32(&reg->ctrl);
>  	ctrl &= ~CTRL_RE;
>  	ctrl &= ~CTRL_TE;
> -	out_be32(&base->ctrl, ctrl);
> +	out_be32(&reg->ctrl, ctrl);
>  
> -	out_be32(&base->modir, 0);
> -	out_be32(&base->fifo, ~(FIFO_TXFE | FIFO_RXFE));
> +	out_be32(&reg->modir, 0);
> +	out_be32(&reg->fifo, ~(FIFO_TXFE | FIFO_RXFE));
>  
> -	out_be32(&base->match, 0);
> +	out_be32(&reg->match, 0);
>  
>  	/* provide data bits, parity, stop bit, etc */
> -	lpuart32_serial_setbrg();
> +	_lpuart32_serial_setbrg(reg, gd->baudrate);
>  
> -	out_be32(&base->ctrl, CTRL_RE | CTRL_TE);
> +	out_be32(&reg->ctrl, CTRL_RE | CTRL_TE);
>  
>  	return 0;
>  }
>  
> +static void lpuart32_serial_setbrg(void)
> +{
> +	_lpuart32_serial_setbrg(base, gd->baudrate);
> +}
> +
> +static int lpuart32_serial_getc(void)
> +{
> +	return _lpuart32_serial_getc(base);
> +}
> +
> +static void lpuart32_serial_putc(const char c)
> +{
> +	_lpuart32_serial_putc(base, c);
> +}
> +
> +static int lpuart32_serial_tstc(void)
> +{
> +	return _lpuart32_serial_tstc(base);
> +}
> +
> +static int lpuart32_serial_init(void)
> +{
> +	return _lpuart32_serial_init(base);
> +}
> +
>  static struct serial_device lpuart32_serial_drv = {
>  	.name = "lpuart32_serial",
>  	.start = lpuart32_serial_init,

[U-Boot] [PATCH 6/8] serial: lpuart: Prepare the driver for DM conversion

[Bin Meng]

Hi Stefan,

On Thu, Jan 14, 2016 at 2:51 AM, Stefan Agner <stefan@agner.ch> wrote:
> On 2015-12-31 00:53, Bin Meng wrote:
>> Create internal routines which take lpuart's register base as
>> a parameter, in preparation for driver model conversion.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  drivers/serial/serial_lpuart.c | 146 +++++++++++++++++++++++++++--------------
>>  1 file changed, 95 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
>> index 0c0ab87..fed83a6 100644
>> --- a/drivers/serial/serial_lpuart.c
>> +++ b/drivers/serial/serial_lpuart.c
>> @@ -50,46 +50,43 @@ DECLARE_GLOBAL_DATA_PTR;
>>  struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
>>
>>  #ifndef CONFIG_LPUART_32B_REG
>> -static void lpuart_serial_setbrg(void)
>> +static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>>  {
>>       u32 clk = mxc_get_clock(MXC_UART_CLK);
>>       u16 sbr;
>>
>> -     if (!gd->baudrate)
>> -             gd->baudrate = CONFIG_BAUDRATE;
>> -
>> -     sbr = (u16)(clk / (16 * gd->baudrate));
>> +     sbr = (u16)(clk / (16 * baudrate));
>>
>>       /* place adjustment later - n/32 BRFA */
>> -     __raw_writeb(sbr >> 8, &base->ubdh);
>> -     __raw_writeb(sbr & 0xff, &base->ubdl);
>> +     __raw_writeb(sbr >> 8, &reg->ubdh);
>> +     __raw_writeb(sbr & 0xff, &reg->ubdl);
>>  }
>>
>> -static int lpuart_serial_getc(void)
>> +static int _lpuart_serial_getc(struct lpuart_fsl *reg)
>>  {
>> -     while (!(__raw_readb(&base->us1) & (US1_RDRF | US1_OR)))
>> +     while (!(__raw_readb(&reg->us1) & (US1_RDRF | US1_OR)))
>>               WATCHDOG_RESET();
>>
>>       barrier();
>>
>> -     return __raw_readb(&base->ud);
>> +     return __raw_readb(&reg->ud);
>>  }
>>
>> -static void lpuart_serial_putc(const char c)
>> +static void _lpuart_serial_putc(struct lpuart_fsl *reg, const char c)
>>  {
>>       if (c == '\n')
>> -             lpuart_serial_putc('\r');
>> +             _lpuart_serial_putc(reg, '\r');
>>
>> -     while (!(__raw_readb(&base->us1) & US1_TDRE))
>> +     while (!(__raw_readb(&reg->us1) & US1_TDRE))
>>               WATCHDOG_RESET();
>>
>> -     __raw_writeb(c, &base->ud);
>> +     __raw_writeb(c, &reg->ud);
>>  }
>>
>>  /* Test whether a character is in the RX buffer */
>> -static int lpuart_serial_tstc(void)
>> +static int _lpuart_serial_tstc(struct lpuart_fsl *reg)
>>  {
>> -     if (__raw_readb(&base->urcfifo) == 0)
>> +     if (__raw_readb(&reg->urcfifo) == 0)
>>               return 0;
>>
>>       return 1;
>> @@ -99,32 +96,57 @@ static int lpuart_serial_tstc(void)
>>   * Initialise the serial port with the given baudrate. The settings
>>   * are always 8 data bits, no parity, 1 stop bit, no start bits.
>>   */
>> -static int lpuart_serial_init(void)
>> +static int _lpuart_serial_init(struct lpuart_fsl *reg)
>
> Couldn't you just name the parameter base and get rid of the changes
> below? IMHO, renaming variables is only really necessary if the current
> name is misleading, which I don't think it is...
>

Will change 'reg' to 'base' in v2.

>>  {
>>       u8 ctrl;
>>
>> -     ctrl = __raw_readb(&base->uc2);
>> +     ctrl = __raw_readb(&reg->uc2);
>>       ctrl &= ~UC2_RE;
>>       ctrl &= ~UC2_TE;
>> -     __raw_writeb(ctrl, &base->uc2);
>> +     __raw_writeb(ctrl, &reg->uc2);
>>
>> -     __raw_writeb(0, &base->umodem);
>> -     __raw_writeb(0, &base->uc1);
>> +     __raw_writeb(0, &reg->umodem);
>> +     __raw_writeb(0, &reg->uc1);
>>
>>       /* Disable FIFO and flush buffer */
>> -     __raw_writeb(0x0, &base->upfifo);
>> -     __raw_writeb(0x0, &base->utwfifo);
>> -     __raw_writeb(0x1, &base->urwfifo);
>> -     __raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &base->ucfifo);
>> +     __raw_writeb(0x0, &reg->upfifo);
>> +     __raw_writeb(0x0, &reg->utwfifo);
>> +     __raw_writeb(0x1, &reg->urwfifo);
>> +     __raw_writeb(CFIFO_TXFLUSH | CFIFO_RXFLUSH, &reg->ucfifo);
>>
>>       /* provide data bits, parity, stop bit, etc */
>> -     lpuart_serial_setbrg();
>> +     _lpuart_serial_setbrg(reg, gd->baudrate);
>>
>> -     __raw_writeb(UC2_RE | UC2_TE, &base->uc2);
>> +     __raw_writeb(UC2_RE | UC2_TE, &reg->uc2);
>>
>>       return 0;
>>  }
>>
>> +static void lpuart_serial_setbrg(void)
>> +{
>> +     _lpuart_serial_setbrg(base, gd->baudrate);
>> +}
>> +
>> +static int lpuart_serial_getc(void)
>> +{
>> +     return _lpuart_serial_getc(base);
>> +}
>> +
>> +static void lpuart_serial_putc(const char c)
>> +{
>> +     _lpuart_serial_putc(base, c);
>> +}
>> +
>> +static int lpuart_serial_tstc(void)
>> +{
>> +     return _lpuart_serial_tstc();
>> +}
>> +
>> +static int lpuart_serial_init(void)
>> +{
>> +     return _lpuart_serial_init(base);
>> +}
>> +
>>  static struct serial_device lpuart_serial_drv = {
>>       .name = "lpuart_serial",
>>       .start = lpuart_serial_init,
>> @@ -136,47 +158,44 @@ static struct serial_device lpuart_serial_drv = {
>>       .tstc = lpuart_serial_tstc,
>>  };
>>  #else
>> -static void lpuart32_serial_setbrg(void)
>> +static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>>  {
>>       u32 clk = CONFIG_SYS_CLK_FREQ;
>>       u32 sbr;
>>
>> -     if (!gd->baudrate)
>> -             gd->baudrate = CONFIG_BAUDRATE;
>> -
>> -     sbr = (clk / (16 * gd->baudrate));
>> +     sbr = (clk / (16 * baudrate));
>>
>>       /* place adjustment later - n/32 BRFA */
>> -     out_be32(&base->baud, sbr);
>> +     out_be32(&reg->baud, sbr);
>>  }
>>
>> -static int lpuart32_serial_getc(void)
>> +static int _lpuart32_serial_getc(struct lpuart_fsl *reg)
>>  {
>>       u32 stat;
>>
>> -     while (((stat = in_be32(&base->stat)) & STAT_RDRF) == 0) {
>> -             out_be32(&base->stat, STAT_FLAGS);
>> +     while (((stat = in_be32(&reg->stat)) & STAT_RDRF) == 0) {
>> +             out_be32(&reg->stat, STAT_FLAGS);
>>               WATCHDOG_RESET();
>>       }
>>
>> -     return in_be32(&base->data) & 0x3ff;
>> +     return in_be32(&reg->data) & 0x3ff;
>>  }
>>
>> -static void lpuart32_serial_putc(const char c)
>> +static void _lpuart32_serial_putc(struct lpuart_fsl *reg, const char c)
>>  {
>>       if (c == '\n')
>> -             lpuart32_serial_putc('\r');
>> +             _lpuart32_serial_putc(reg, '\r');
>>
>> -     while (!(in_be32(&base->stat) & STAT_TDRE))
>> +     while (!(in_be32(&reg->stat) & STAT_TDRE))
>>               WATCHDOG_RESET();
>>
>> -     out_be32(&base->data, c);
>> +     out_be32(&reg->data, c);
>>  }
>>
>>  /* Test whether a character is in the RX buffer */
>> -static int lpuart32_serial_tstc(void)
>> +static int _lpuart32_serial_tstc(struct lpuart_fsl *reg)
>>  {
>> -     if ((in_be32(&base->water) >> 24) == 0)
>> +     if ((in_be32(&reg->water) >> 24) == 0)
>>               return 0;
>>
>>       return 1;
>> @@ -186,28 +205,53 @@ static int lpuart32_serial_tstc(void)
>>   * Initialise the serial port with the given baudrate. The settings
>>   * are always 8 data bits, no parity, 1 stop bit, no start bits.
>>   */
>> -static int lpuart32_serial_init(void)
>> +static int _lpuart32_serial_init(struct lpuart_fsl *reg)
>>  {
>>       u8 ctrl;
>>
>> -     ctrl = in_be32(&base->ctrl);
>> +     ctrl = in_be32(&reg->ctrl);
>>       ctrl &= ~CTRL_RE;
>>       ctrl &= ~CTRL_TE;
>> -     out_be32(&base->ctrl, ctrl);
>> +     out_be32(&reg->ctrl, ctrl);
>>
>> -     out_be32(&base->modir, 0);
>> -     out_be32(&base->fifo, ~(FIFO_TXFE | FIFO_RXFE));
>> +     out_be32(&reg->modir, 0);
>> +     out_be32(&reg->fifo, ~(FIFO_TXFE | FIFO_RXFE));
>>
>> -     out_be32(&base->match, 0);
>> +     out_be32(&reg->match, 0);
>>
>>       /* provide data bits, parity, stop bit, etc */
>> -     lpuart32_serial_setbrg();
>> +     _lpuart32_serial_setbrg(reg, gd->baudrate);
>>
>> -     out_be32(&base->ctrl, CTRL_RE | CTRL_TE);
>> +     out_be32(&reg->ctrl, CTRL_RE | CTRL_TE);
>>
>>       return 0;
>>  }
>>
>> +static void lpuart32_serial_setbrg(void)
>> +{
>> +     _lpuart32_serial_setbrg(base, gd->baudrate);
>> +}
>> +
>> +static int lpuart32_serial_getc(void)
>> +{
>> +     return _lpuart32_serial_getc(base);
>> +}
>> +
>> +static void lpuart32_serial_putc(const char c)
>> +{
>> +     _lpuart32_serial_putc(base, c);
>> +}
>> +
>> +static int lpuart32_serial_tstc(void)
>> +{
>> +     return _lpuart32_serial_tstc(base);
>> +}
>> +
>> +static int lpuart32_serial_init(void)
>> +{
>> +     return _lpuart32_serial_init(base);
>> +}
>> +
>>  static struct serial_device lpuart32_serial_drv = {
>>       .name = "lpuart32_serial",
>>       .start = lpuart32_serial_init,

Regards,
Bin

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bin Meng]

This adds driver model support to lpuart serial driver.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 doc/driver-model/serial-howto.txt |   1 -
 drivers/serial/serial_lpuart.c    | 157 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 157 insertions(+), 1 deletion(-)

diff --git a/doc/driver-model/serial-howto.txt b/doc/driver-model/serial-howto.txt
index 76ad629..b933836 100644
--- a/doc/driver-model/serial-howto.txt
+++ b/doc/driver-model/serial-howto.txt
@@ -13,7 +13,6 @@ is time for maintainers to start converting over the remaining serial drivers:
    opencores_yanu.c
    serial_bfin.c
    serial_imx.c
-   serial_lpuart.c
    serial_max3100.c
    serial_pxa.c
    serial_s3c24x0.c
diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
index fed83a6..b1ba1bc 100644
--- a/drivers/serial/serial_lpuart.c
+++ b/drivers/serial/serial_lpuart.c
@@ -5,6 +5,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <watchdog.h>
 #include <asm/io.h>
 #include <serial.h>
@@ -49,6 +50,10 @@ DECLARE_GLOBAL_DATA_PTR;
 
 struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
 
+struct lpuart_serial_platdata {
+	struct lpuart_fsl *reg;
+};
+
 #ifndef CONFIG_LPUART_32B_REG
 static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
 {
@@ -122,6 +127,7 @@ static int _lpuart_serial_init(struct lpuart_fsl *reg)
 	return 0;
 }
 
+#ifndef CONFIG_DM_SERIAL
 static void lpuart_serial_setbrg(void)
 {
 	_lpuart_serial_setbrg(base, gd->baudrate);
@@ -157,6 +163,54 @@ static struct serial_device lpuart_serial_drv = {
 	.getc = lpuart_serial_getc,
 	.tstc = lpuart_serial_tstc,
 };
+#else /* CONFIG_DM_SERIAL */
+static int lpuart_serial_setbrg(struct udevice *dev, int baudrate)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	_lpuart_serial_setbrg(reg, baudrate);
+
+	return 0;
+}
+
+static int lpuart_serial_getc(struct udevice *dev)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	return _lpuart_serial_getc(reg);
+}
+
+static int lpuart_serial_putc(struct udevice *dev, const char c)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	_lpuart_serial_putc(reg, c);
+
+	return 0;
+}
+
+static int lpuart_serial_pending(struct udevice *dev, bool input)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	if (input)
+		return _lpuart_serial_tstc(reg);
+	else
+		return __raw_readb(&reg->us1) & US1_TDRE ? 0 : 1;
+}
+
+static int lpuart_serial_probe(struct udevice *dev)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	return _lpuart_serial_init(reg);
+}
+#endif /* CONFIG_DM_SERIAL */
 #else
 static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
 {
@@ -227,6 +281,7 @@ static int _lpuart32_serial_init(struct lpuart_fsl *reg)
 	return 0;
 }
 
+#ifndef CONFIG_DM_SERIAL
 static void lpuart32_serial_setbrg(void)
 {
 	_lpuart32_serial_setbrg(base, gd->baudrate);
@@ -262,8 +317,57 @@ static struct serial_device lpuart32_serial_drv = {
 	.getc = lpuart32_serial_getc,
 	.tstc = lpuart32_serial_tstc,
 };
+#else /* CONFIG_DM_SERIAL */
+static int lpuart32_serial_setbrg(struct udevice *dev, int baudrate)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	_lpuart32_serial_setbrg(reg, baudrate);
+
+	return 0;
+}
+
+static int lpuart32_serial_getc(struct udevice *dev)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	return _lpuart32_serial_getc(reg);
+}
+
+static int lpuart32_serial_putc(struct udevice *dev, const char c)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	_lpuart32_serial_putc(reg, c);
+
+	return 0;
+}
+
+static int lpuart32_serial_pending(struct udevice *dev, bool input)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	if (input)
+		return _lpuart32_serial_tstc(reg);
+	else
+		return in_be32(&reg->stat) & STAT_TDRE ? 0 : 1;
+}
+
+static int lpuart32_serial_probe(struct udevice *dev)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	struct lpuart_fsl *reg = plat->reg;
+
+	return _lpuart32_serial_init(reg);
+}
+#endif /* CONFIG_DM_SERIAL */
 #endif
 
+#ifndef CONFIG_DM_SERIAL
 void lpuart_serial_initialize(void)
 {
 #ifdef CONFIG_LPUART_32B_REG
@@ -281,3 +385,56 @@ __weak struct serial_device *default_serial_console(void)
 	return &lpuart_serial_drv;
 #endif
 }
+#else /* CONFIG_DM_SERIAL */
+static int lpuart_serial_ofdata_to_platdata(struct udevice *dev)
+{
+	struct lpuart_serial_platdata *plat = dev->platdata;
+	fdt_addr_t addr;
+
+	addr = dev_get_addr(dev);
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	plat->reg = (struct lpuart_fsl *)addr;
+
+	return 0;
+}
+
+static const struct dm_serial_ops lpuart_serial_ops = {
+#ifdef CONFIG_LPUART_32B_REG
+	.putc = lpuart32_serial_putc,
+	.pending = lpuart32_serial_pending,
+	.getc = lpuart32_serial_getc,
+	.setbrg = lpuart32_serial_setbrg,
+#else
+	.putc = lpuart_serial_putc,
+	.pending = lpuart_serial_pending,
+	.getc = lpuart_serial_getc,
+	.setbrg = lpuart_serial_setbrg,
+#endif
+};
+
+static const struct udevice_id lpuart_serial_ids[] = {
+#ifdef CONFIG_LPUART_32B_REG
+	{ .compatible = "fsl,ls1021a-lpuart" },
+#else
+	{ .compatible = "fsl,vf610-lpuart" },
+#endif
+	{ }
+};
+
+U_BOOT_DRIVER(serial_lpuart) = {
+	.name	= "serial_lpuart",
+	.id	= UCLASS_SERIAL,
+	.of_match = lpuart_serial_ids,
+	.ofdata_to_platdata = lpuart_serial_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct lpuart_serial_platdata),
+#ifdef CONFIG_LPUART_32B_REG
+	.probe = lpuart32_serial_probe,
+#else
+	.probe = lpuart_serial_probe,
+#endif
+	.ops	= &lpuart_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
+#endif /* CONFIG_DM_SERIAL */
-- 
1.8.2.1

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Simon Glass]

Hi Bin,

On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> This adds driver model support to lpuart serial driver.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  doc/driver-model/serial-howto.txt |   1 -
>  drivers/serial/serial_lpuart.c    | 157 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 157 insertions(+), 1 deletion(-)

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

But please see below.

>
> diff --git a/doc/driver-model/serial-howto.txt b/doc/driver-model/serial-howto.txt
> index 76ad629..b933836 100644
> --- a/doc/driver-model/serial-howto.txt
> +++ b/doc/driver-model/serial-howto.txt
> @@ -13,7 +13,6 @@ is time for maintainers to start converting over the remaining serial drivers:
>     opencores_yanu.c
>     serial_bfin.c
>     serial_imx.c
> -   serial_lpuart.c
>     serial_max3100.c
>     serial_pxa.c
>     serial_s3c24x0.c
> diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
> index fed83a6..b1ba1bc 100644
> --- a/drivers/serial/serial_lpuart.c
> +++ b/drivers/serial/serial_lpuart.c
> @@ -5,6 +5,7 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
>  #include <watchdog.h>
>  #include <asm/io.h>
>  #include <serial.h>
> @@ -49,6 +50,10 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
>
> +struct lpuart_serial_platdata {
> +       struct lpuart_fsl *reg;
> +};
> +
>  #ifndef CONFIG_LPUART_32B_REG
>  static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
> @@ -122,6 +127,7 @@ static int _lpuart_serial_init(struct lpuart_fsl *reg)
>         return 0;
>  }
>
> +#ifndef CONFIG_DM_SERIAL
>  static void lpuart_serial_setbrg(void)
>  {
>         _lpuart_serial_setbrg(base, gd->baudrate);
> @@ -157,6 +163,54 @@ static struct serial_device lpuart_serial_drv = {
>         .getc = lpuart_serial_getc,
>         .tstc = lpuart_serial_tstc,
>  };
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart_serial_setbrg(reg, baudrate);
> +
> +       return 0;
> +}
> +
> +static int lpuart_serial_getc(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart_serial_getc(reg);
> +}
> +
> +static int lpuart_serial_putc(struct udevice *dev, const char c)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart_serial_putc(reg, c);
> +
> +       return 0;
> +}
> +
> +static int lpuart_serial_pending(struct udevice *dev, bool input)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       if (input)
> +               return _lpuart_serial_tstc(reg);
> +       else
> +               return __raw_readb(&reg->us1) & US1_TDRE ? 0 : 1;
> +}
> +
> +static int lpuart_serial_probe(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart_serial_init(reg);
> +}
> +#endif /* CONFIG_DM_SERIAL */
>  #else
>  static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
> @@ -227,6 +281,7 @@ static int _lpuart32_serial_init(struct lpuart_fsl *reg)
>         return 0;
>  }
>
> +#ifndef CONFIG_DM_SERIAL
>  static void lpuart32_serial_setbrg(void)
>  {
>         _lpuart32_serial_setbrg(base, gd->baudrate);
> @@ -262,8 +317,57 @@ static struct serial_device lpuart32_serial_drv = {
>         .getc = lpuart32_serial_getc,
>         .tstc = lpuart32_serial_tstc,
>  };
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart32_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart32_serial_setbrg(reg, baudrate);
> +
> +       return 0;
> +}
> +
> +static int lpuart32_serial_getc(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart32_serial_getc(reg);
> +}
> +
> +static int lpuart32_serial_putc(struct udevice *dev, const char c)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart32_serial_putc(reg, c);
> +
> +       return 0;
> +}
> +
> +static int lpuart32_serial_pending(struct udevice *dev, bool input)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       if (input)
> +               return _lpuart32_serial_tstc(reg);
> +       else
> +               return in_be32(&reg->stat) & STAT_TDRE ? 0 : 1;
> +}
> +
> +static int lpuart32_serial_probe(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart32_serial_init(reg);
> +}
> +#endif /* CONFIG_DM_SERIAL */
>  #endif
>
> +#ifndef CONFIG_DM_SERIAL
>  void lpuart_serial_initialize(void)
>  {
>  #ifdef CONFIG_LPUART_32B_REG
> @@ -281,3 +385,56 @@ __weak struct serial_device *default_serial_console(void)
>         return &lpuart_serial_drv;
>  #endif
>  }
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart_serial_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       fdt_addr_t addr;
> +
> +       addr = dev_get_addr(dev);
> +       if (addr == FDT_ADDR_T_NONE)
> +               return -EINVAL;
> +
> +       plat->reg = (struct lpuart_fsl *)addr;
> +
> +       return 0;
> +}
> +
> +static const struct dm_serial_ops lpuart_serial_ops = {
> +#ifdef CONFIG_LPUART_32B_REG
> +       .putc = lpuart32_serial_putc,
> +       .pending = lpuart32_serial_pending,
> +       .getc = lpuart32_serial_getc,
> +       .setbrg = lpuart32_serial_setbrg,
> +#else
> +       .putc = lpuart_serial_putc,
> +       .pending = lpuart_serial_pending,
> +       .getc = lpuart_serial_getc,
> +       .setbrg = lpuart_serial_setbrg,
> +#endif
> +};
> +
> +static const struct udevice_id lpuart_serial_ids[] = {
> +#ifdef CONFIG_LPUART_32B_REG
> +       { .compatible = "fsl,ls1021a-lpuart" },
> +#else
> +       { .compatible = "fsl,vf610-lpuart" },
> +#endif

Shouldn't we have both of these in there, with a .data member to
distinguish between them? Or two separate U_BOOT_DRIVER() entries each
with their own operations.

> +       { }
> +};
> +
> +U_BOOT_DRIVER(serial_lpuart) = {
> +       .name   = "serial_lpuart",
> +       .id     = UCLASS_SERIAL,
> +       .of_match = lpuart_serial_ids,
> +       .ofdata_to_platdata = lpuart_serial_ofdata_to_platdata,
> +       .platdata_auto_alloc_size = sizeof(struct lpuart_serial_platdata),
> +#ifdef CONFIG_LPUART_32B_REG
> +       .probe = lpuart32_serial_probe,
> +#else
> +       .probe = lpuart_serial_probe,
> +#endif
> +       .ops    = &lpuart_serial_ops,
> +       .flags = DM_FLAG_PRE_RELOC,
> +};
> +#endif /* CONFIG_DM_SERIAL */
> --
> 1.8.2.1
>

Regards,
Simon

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bin Meng]

Hi Simon,

On Wed, Jan 6, 2016 at 8:25 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
>> This adds driver model support to lpuart serial driver.
>>
>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>>
>>  doc/driver-model/serial-howto.txt |   1 -
>>  drivers/serial/serial_lpuart.c    | 157 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 157 insertions(+), 1 deletion(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> But please see below.
>
>>

[snip]

>> +#else /* CONFIG_DM_SERIAL */
>> +static int lpuart_serial_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct lpuart_serial_platdata *plat = dev->platdata;
>> +       fdt_addr_t addr;
>> +
>> +       addr = dev_get_addr(dev);
>> +       if (addr == FDT_ADDR_T_NONE)
>> +               return -EINVAL;
>> +
>> +       plat->reg = (struct lpuart_fsl *)addr;
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct dm_serial_ops lpuart_serial_ops = {
>> +#ifdef CONFIG_LPUART_32B_REG
>> +       .putc = lpuart32_serial_putc,
>> +       .pending = lpuart32_serial_pending,
>> +       .getc = lpuart32_serial_getc,
>> +       .setbrg = lpuart32_serial_setbrg,
>> +#else
>> +       .putc = lpuart_serial_putc,
>> +       .pending = lpuart_serial_pending,
>> +       .getc = lpuart_serial_getc,
>> +       .setbrg = lpuart_serial_setbrg,
>> +#endif
>> +};
>> +
>> +static const struct udevice_id lpuart_serial_ids[] = {
>> +#ifdef CONFIG_LPUART_32B_REG
>> +       { .compatible = "fsl,ls1021a-lpuart" },
>> +#else
>> +       { .compatible = "fsl,vf610-lpuart" },
>> +#endif
>
> Shouldn't we have both of these in there, with a .data member to
> distinguish between them? Or two separate U_BOOT_DRIVER() entries each
> with their own operations.

Sure, will do in v2.

>
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(serial_lpuart) = {
>> +       .name   = "serial_lpuart",
>> +       .id     = UCLASS_SERIAL,
>> +       .of_match = lpuart_serial_ids,
>> +       .ofdata_to_platdata = lpuart_serial_ofdata_to_platdata,
>> +       .platdata_auto_alloc_size = sizeof(struct lpuart_serial_platdata),
>> +#ifdef CONFIG_LPUART_32B_REG
>> +       .probe = lpuart32_serial_probe,
>> +#else
>> +       .probe = lpuart_serial_probe,
>> +#endif
>> +       .ops    = &lpuart_serial_ops,
>> +       .flags = DM_FLAG_PRE_RELOC,
>> +};
>> +#endif /* CONFIG_DM_SERIAL */
>> --

Regards,
Bin

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bhuvanchandra DV]

Hi Bin,

With reference to the discussion here[1].

Unfortunately the lpuart driver is now broken for legacy code and also
the driver doesn't
work with serial driver model enabled on Toradex Colibri VF50/VF61,
Freescale VF610twr
and Phytec pcm052 boards. Did some one tested this patchset on these boards ?

[1] http://lists.denx.de/pipermail/u-boot/2016-January/240781.html

Best regards,
Bhuvan

On Thu, Dec 31, 2015 at 2:23 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> This adds driver model support to lpuart serial driver.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  doc/driver-model/serial-howto.txt |   1 -
>  drivers/serial/serial_lpuart.c    | 157 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 157 insertions(+), 1 deletion(-)
>
> diff --git a/doc/driver-model/serial-howto.txt b/doc/driver-model/serial-howto.txt
> index 76ad629..b933836 100644
> --- a/doc/driver-model/serial-howto.txt
> +++ b/doc/driver-model/serial-howto.txt
> @@ -13,7 +13,6 @@ is time for maintainers to start converting over the remaining serial drivers:
>     opencores_yanu.c
>     serial_bfin.c
>     serial_imx.c
> -   serial_lpuart.c
>     serial_max3100.c
>     serial_pxa.c
>     serial_s3c24x0.c
> diff --git a/drivers/serial/serial_lpuart.c b/drivers/serial/serial_lpuart.c
> index fed83a6..b1ba1bc 100644
> --- a/drivers/serial/serial_lpuart.c
> +++ b/drivers/serial/serial_lpuart.c
> @@ -5,6 +5,7 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
>  #include <watchdog.h>
>  #include <asm/io.h>
>  #include <serial.h>
> @@ -49,6 +50,10 @@ DECLARE_GLOBAL_DATA_PTR;
>
>  struct lpuart_fsl *base = (struct lpuart_fsl *)LPUART_BASE;
>
> +struct lpuart_serial_platdata {
> +       struct lpuart_fsl *reg;
> +};
> +
>  #ifndef CONFIG_LPUART_32B_REG
>  static void _lpuart_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
> @@ -122,6 +127,7 @@ static int _lpuart_serial_init(struct lpuart_fsl *reg)
>         return 0;
>  }
>
> +#ifndef CONFIG_DM_SERIAL
>  static void lpuart_serial_setbrg(void)
>  {
>         _lpuart_serial_setbrg(base, gd->baudrate);
> @@ -157,6 +163,54 @@ static struct serial_device lpuart_serial_drv = {
>         .getc = lpuart_serial_getc,
>         .tstc = lpuart_serial_tstc,
>  };
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart_serial_setbrg(reg, baudrate);
> +
> +       return 0;
> +}
> +
> +static int lpuart_serial_getc(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart_serial_getc(reg);
> +}
> +
> +static int lpuart_serial_putc(struct udevice *dev, const char c)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart_serial_putc(reg, c);
> +
> +       return 0;
> +}
> +
> +static int lpuart_serial_pending(struct udevice *dev, bool input)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       if (input)
> +               return _lpuart_serial_tstc(reg);
> +       else
> +               return __raw_readb(&reg->us1) & US1_TDRE ? 0 : 1;
> +}
> +
> +static int lpuart_serial_probe(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart_serial_init(reg);
> +}
> +#endif /* CONFIG_DM_SERIAL */
>  #else
>  static void _lpuart32_serial_setbrg(struct lpuart_fsl *reg, int baudrate)
>  {
> @@ -227,6 +281,7 @@ static int _lpuart32_serial_init(struct lpuart_fsl *reg)
>         return 0;
>  }
>
> +#ifndef CONFIG_DM_SERIAL
>  static void lpuart32_serial_setbrg(void)
>  {
>         _lpuart32_serial_setbrg(base, gd->baudrate);
> @@ -262,8 +317,57 @@ static struct serial_device lpuart32_serial_drv = {
>         .getc = lpuart32_serial_getc,
>         .tstc = lpuart32_serial_tstc,
>  };
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart32_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart32_serial_setbrg(reg, baudrate);
> +
> +       return 0;
> +}
> +
> +static int lpuart32_serial_getc(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart32_serial_getc(reg);
> +}
> +
> +static int lpuart32_serial_putc(struct udevice *dev, const char c)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       _lpuart32_serial_putc(reg, c);
> +
> +       return 0;
> +}
> +
> +static int lpuart32_serial_pending(struct udevice *dev, bool input)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       if (input)
> +               return _lpuart32_serial_tstc(reg);
> +       else
> +               return in_be32(&reg->stat) & STAT_TDRE ? 0 : 1;
> +}
> +
> +static int lpuart32_serial_probe(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       struct lpuart_fsl *reg = plat->reg;
> +
> +       return _lpuart32_serial_init(reg);
> +}
> +#endif /* CONFIG_DM_SERIAL */
>  #endif
>
> +#ifndef CONFIG_DM_SERIAL
>  void lpuart_serial_initialize(void)
>  {
>  #ifdef CONFIG_LPUART_32B_REG
> @@ -281,3 +385,56 @@ __weak struct serial_device *default_serial_console(void)
>         return &lpuart_serial_drv;
>  #endif
>  }
> +#else /* CONFIG_DM_SERIAL */
> +static int lpuart_serial_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct lpuart_serial_platdata *plat = dev->platdata;
> +       fdt_addr_t addr;
> +
> +       addr = dev_get_addr(dev);
> +       if (addr == FDT_ADDR_T_NONE)
> +               return -EINVAL;
> +
> +       plat->reg = (struct lpuart_fsl *)addr;
> +
> +       return 0;
> +}
> +
> +static const struct dm_serial_ops lpuart_serial_ops = {
> +#ifdef CONFIG_LPUART_32B_REG
> +       .putc = lpuart32_serial_putc,
> +       .pending = lpuart32_serial_pending,
> +       .getc = lpuart32_serial_getc,
> +       .setbrg = lpuart32_serial_setbrg,
> +#else
> +       .putc = lpuart_serial_putc,
> +       .pending = lpuart_serial_pending,
> +       .getc = lpuart_serial_getc,
> +       .setbrg = lpuart_serial_setbrg,
> +#endif
> +};
> +
> +static const struct udevice_id lpuart_serial_ids[] = {
> +#ifdef CONFIG_LPUART_32B_REG
> +       { .compatible = "fsl,ls1021a-lpuart" },
> +#else
> +       { .compatible = "fsl,vf610-lpuart" },
> +#endif
> +       { }
> +};
> +
> +U_BOOT_DRIVER(serial_lpuart) = {
> +       .name   = "serial_lpuart",
> +       .id     = UCLASS_SERIAL,
> +       .of_match = lpuart_serial_ids,
> +       .ofdata_to_platdata = lpuart_serial_ofdata_to_platdata,
> +       .platdata_auto_alloc_size = sizeof(struct lpuart_serial_platdata),
> +#ifdef CONFIG_LPUART_32B_REG
> +       .probe = lpuart32_serial_probe,
> +#else
> +       .probe = lpuart_serial_probe,
> +#endif
> +       .ops    = &lpuart_serial_ops,
> +       .flags = DM_FLAG_PRE_RELOC,
> +};
> +#endif /* CONFIG_DM_SERIAL */
> --
> 1.8.2.1
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bin Meng]

Hi Bhuvan,

On Wed, Jan 13, 2016 at 1:49 PM, Bhuvanchandra DV
<bhuvanchandra.dv@gmail.com> wrote:
> Hi Bin,
>
> With reference to the discussion here[1].
>
> Unfortunately the lpuart driver is now broken for legacy code and also
> the driver doesn't
> work with serial driver model enabled on Toradex Colibri VF50/VF61,
> Freescale VF610twr
> and Phytec pcm052 boards. Did some one tested this patchset on these boards ?

I will fix the legacy code build in v2. About serial driver model not
working on these boards, is that the caused by no device tree of these
boards?

>
> [1] http://lists.denx.de/pipermail/u-boot/2016-January/240781.html
>
> Best regards,
> Bhuvan
>

Regards,
Bin

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bhuvanchandra DV]

Hi Bin,

On 01/13/2016 11:43 AM, Bin Meng wrote:
> Hi Bhuvan,
>
> On Wed, Jan 13, 2016 at 1:49 PM, Bhuvanchandra DV
> <bhuvanchandra.dv@gmail.com> wrote:
>> Hi Bin,
>>
>> With reference to the discussion here[1].
>>
>> Unfortunately the lpuart driver is now broken for legacy code and also
>> the driver doesn't
>> work with serial driver model enabled on Toradex Colibri VF50/VF61,
>> Freescale VF610twr
>> and Phytec pcm052 boards. Did some one tested this patchset on these boards ?
>
> I will fix the legacy code build in v2. About serial driver model not
> working on these boards, is that the caused by no device tree of these
> boards?

Yes, i tested on Colibri VF50/VF61 with device tree and it works fine.
I think it would be nice to have the support for both platform data and 
device tree so that we can use it with platform data via board files and 
device tree too.

Since only few boards are using lpuart driver we can update the driver 
completly to driver model, drop the legacy code and update the boards.

>
>>
>> [1] http://lists.denx.de/pipermail/u-boot/2016-January/240781.html
>>
>> Best regards,
>> Bhuvan
>>
>
> Regards,
> Bin
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

-- 
Best regards,
Bhuvan

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bin Meng]

+Simon

Hi Bhuvan,

On Wed, Jan 13, 2016 at 4:07 PM, Bhuvanchandra DV
<bhuvanchandra.dv@toradex.com> wrote:
> Hi Bin,
>
> On 01/13/2016 11:43 AM, Bin Meng wrote:
>>
>> Hi Bhuvan,
>>
>> On Wed, Jan 13, 2016 at 1:49 PM, Bhuvanchandra DV
>> <bhuvanchandra.dv@gmail.com> wrote:
>>>
>>> Hi Bin,
>>>
>>> With reference to the discussion here[1].
>>>
>>> Unfortunately the lpuart driver is now broken for legacy code and also
>>> the driver doesn't
>>> work with serial driver model enabled on Toradex Colibri VF50/VF61,
>>> Freescale VF610twr
>>> and Phytec pcm052 boards. Did some one tested this patchset on these
>>> boards ?
>>
>>
>> I will fix the legacy code build in v2. About serial driver model not
>> working on these boards, is that the caused by no device tree of these
>> boards?
>
>
> Yes, i tested on Colibri VF50/VF61 with device tree and it works fine.

Great to know!

> I think it would be nice to have the support for both platform data and
> device tree so that we can use it with platform data via board files and
> device tree too.

I believe we should introduce device tree support on these boards. The
configuration data (like in your patches the reg base for LPUART)
should really be put into device tree. I adapted the comments from
platdata.h below:

31/**
32 * NOTE: Avoid using these except in extreme circumstances, where device tree
33 * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is
34 * available). U-Boot's driver model uses device tree for configuration.
35 */
36#define U_BOOT_DEVICE(__name)                                           \
37        ll_entry_declare(struct driver_info, __name, driver_info)

>
> Since only few boards are using lpuart driver we can update the driver
> completly to driver model, drop the legacy code and update the boards.
>

Since in my patches I only updated ls1021atwr board to use driver
model serial, and I don't have those other boards (like Colibri
VF50/VF61) to test this lpuart dm driver. I chose to leave the legacy
codes there. On top of my series, you can prepare a patch to
completely drop those legacy codes after you switch to use driver
model lpuart driver on those boards in your series. Then we get a
legacy-free driver for lpuart boards :)

Regards,
Bin

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Stefan Agner]

On 2016-01-13 00:19, Bin Meng wrote:
> +Simon
> 
> Hi Bhuvan,
> 
> On Wed, Jan 13, 2016 at 4:07 PM, Bhuvanchandra DV
> <bhuvanchandra.dv@toradex.com> wrote:
>> Hi Bin,
>>
>> On 01/13/2016 11:43 AM, Bin Meng wrote:
>>>
>>> Hi Bhuvan,
>>>
>>> On Wed, Jan 13, 2016 at 1:49 PM, Bhuvanchandra DV
>>> <bhuvanchandra.dv@gmail.com> wrote:
>>>>
>>>> Hi Bin,
>>>>
>>>> With reference to the discussion here[1].
>>>>
>>>> Unfortunately the lpuart driver is now broken for legacy code and also
>>>> the driver doesn't
>>>> work with serial driver model enabled on Toradex Colibri VF50/VF61,
>>>> Freescale VF610twr
>>>> and Phytec pcm052 boards. Did some one tested this patchset on these
>>>> boards ?
>>>
>>>
>>> I will fix the legacy code build in v2. About serial driver model not
>>> working on these boards, is that the caused by no device tree of these
>>> boards?
>>
>>
>> Yes, i tested on Colibri VF50/VF61 with device tree and it works fine.
> 
> Great to know!
> 
>> I think it would be nice to have the support for both platform data and
>> device tree so that we can use it with platform data via board files and
>> device tree too.
> 
> I believe we should introduce device tree support on these boards. The
> configuration data (like in your patches the reg base for LPUART)
> should really be put into device tree. I adapted the comments from
> platdata.h below:

Currently colibri_vf has both, a DT and a non-DT config. There has been
only one driver (SPI I think?) which required DT so far, and since most
user do not use that driver, we created two configs.

However, if something like UART requires DT, then we can as well drop
the non-DT config for colibri_vf. 

> 
> 31/**
> 32 * NOTE: Avoid using these except in extreme circumstances, where device tree
> 33 * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is
> 34 * available). U-Boot's driver model uses device tree for configuration.
> 35 */
> 36#define U_BOOT_DEVICE(__name)                                           \
> 37        ll_entry_declare(struct driver_info, __name, driver_info)
> 

Since Vybrid has so large internal SRAM, there has been no need for SPL
at all so far. Not sure about LS1021a/other LPUART SoCs...

>>
>> Since only few boards are using lpuart driver we can update the driver
>> completly to driver model, drop the legacy code and update the boards.
>>
> 
> Since in my patches I only updated ls1021atwr board to use driver
> model serial, and I don't have those other boards (like Colibri
> VF50/VF61) to test this lpuart dm driver. I chose to leave the legacy
> codes there. On top of my series, you can prepare a patch to
> completely drop those legacy codes after you switch to use driver
> model lpuart driver on those boards in your series. Then we get a
> legacy-free driver for lpuart boards :)

I guess nobody has all this boards, we should nontheless try to find a
solution for all of them.

I see three options:
- Leave legacy code and the other boards as is (pcm052/vf610twr)
- Drop legacy code, and add platform data support and the corresponding
platform data for pcm052/vf610twr (in this case, we could also keep the
colibri_vf non-DT config)
- Drop legacy code, add device tree for pcm052/vf610twr, extend
colibri_vf device tree and drop non-DT config for colibri_vf.

I am inclined to say lets go for the pure DT solution, since that is
where U-Boot is evolving to long term anyway. Not sure how much work is
required to make that happen. I guess the lpuart only DT for
pcm052/vf610twr should be fairly easy to create...?

Other opinions?

--
Stefan

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bin Meng]

Hi Stefan,

On Thu, Jan 14, 2016 at 3:20 AM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-01-13 00:19, Bin Meng wrote:
>> +Simon
>>
>> Hi Bhuvan,
>>
>> On Wed, Jan 13, 2016 at 4:07 PM, Bhuvanchandra DV
>> <bhuvanchandra.dv@toradex.com> wrote:
>>> Hi Bin,
>>>
>>> On 01/13/2016 11:43 AM, Bin Meng wrote:
>>>>
>>>> Hi Bhuvan,
>>>>
>>>> On Wed, Jan 13, 2016 at 1:49 PM, Bhuvanchandra DV
>>>> <bhuvanchandra.dv@gmail.com> wrote:
>>>>>
>>>>> Hi Bin,
>>>>>
>>>>> With reference to the discussion here[1].
>>>>>
>>>>> Unfortunately the lpuart driver is now broken for legacy code and also
>>>>> the driver doesn't
>>>>> work with serial driver model enabled on Toradex Colibri VF50/VF61,
>>>>> Freescale VF610twr
>>>>> and Phytec pcm052 boards. Did some one tested this patchset on these
>>>>> boards ?
>>>>
>>>>
>>>> I will fix the legacy code build in v2. About serial driver model not
>>>> working on these boards, is that the caused by no device tree of these
>>>> boards?
>>>
>>>
>>> Yes, i tested on Colibri VF50/VF61 with device tree and it works fine.
>>
>> Great to know!
>>
>>> I think it would be nice to have the support for both platform data and
>>> device tree so that we can use it with platform data via board files and
>>> device tree too.
>>
>> I believe we should introduce device tree support on these boards. The
>> configuration data (like in your patches the reg base for LPUART)
>> should really be put into device tree. I adapted the comments from
>> platdata.h below:
>
> Currently colibri_vf has both, a DT and a non-DT config. There has been
> only one driver (SPI I think?) which required DT so far, and since most
> user do not use that driver, we created two configs.
>
> However, if something like UART requires DT, then we can as well drop
> the non-DT config for colibri_vf.
>

I vote for dropping the non-DT config.

>>
>> 31/**
>> 32 * NOTE: Avoid using these except in extreme circumstances, where device tree
>> 33 * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is
>> 34 * available). U-Boot's driver model uses device tree for configuration.
>> 35 */
>> 36#define U_BOOT_DEVICE(__name)                                           \
>> 37        ll_entry_declare(struct driver_info, __name, driver_info)
>>
>
> Since Vybrid has so large internal SRAM, there has been no need for SPL
> at all so far. Not sure about LS1021a/other LPUART SoCs...
>

LS1021 has 128KB SRAM, and current lp1021atwr_nor_lpuart does not use
SPL. It boots from NOR flash.

>>>
>>> Since only few boards are using lpuart driver we can update the driver
>>> completly to driver model, drop the legacy code and update the boards.
>>>
>>
>> Since in my patches I only updated ls1021atwr board to use driver
>> model serial, and I don't have those other boards (like Colibri
>> VF50/VF61) to test this lpuart dm driver. I chose to leave the legacy
>> codes there. On top of my series, you can prepare a patch to
>> completely drop those legacy codes after you switch to use driver
>> model lpuart driver on those boards in your series. Then we get a
>> legacy-free driver for lpuart boards :)
>
> I guess nobody has all this boards, we should nontheless try to find a
> solution for all of them.
>
> I see three options:
> - Leave legacy code and the other boards as is (pcm052/vf610twr)
> - Drop legacy code, and add platform data support and the corresponding
> platform data for pcm052/vf610twr (in this case, we could also keep the
> colibri_vf non-DT config)
> - Drop legacy code, add device tree for pcm052/vf610twr, extend
> colibri_vf device tree and drop non-DT config for colibri_vf.
>
> I am inclined to say lets go for the pure DT solution, since that is
> where U-Boot is evolving to long term anyway. Not sure how much work is
> required to make that happen. I guess the lpuart only DT for
> pcm052/vf610twr should be fairly easy to create...?
>
> Other opinions?
>
> --

I would go for option 3.

Regards,
Bin

[U-Boot] [PATCH 7/8] serial: lpuart: Add driver model serial support

[Bhuvanchandra DV]

On 01/14/2016 07:54 AM, Bin Meng wrote:
> Hi Stefan,
>
> On Thu, Jan 14, 2016 at 3:20 AM, Stefan Agner <stefan@agner.ch> wrote:
>> On 2016-01-13 00:19, Bin Meng wrote:
>>> +Simon
>>>
>>> Hi Bhuvan,
>>>
>>> On Wed, Jan 13, 2016 at 4:07 PM, Bhuvanchandra DV
>>> <bhuvanchandra.dv@toradex.com> wrote:
>>>> Hi Bin,
>>>>
>>>> On 01/13/2016 11:43 AM, Bin Meng wrote:
>>>>>
>>>>> Hi Bhuvan,
>>>>>
>>>>> On Wed, Jan 13, 2016 at 1:49 PM, Bhuvanchandra DV
>>>>> <bhuvanchandra.dv@gmail.com> wrote:
>>>>>>
>>>>>> Hi Bin,
>>>>>>
>>>>>> With reference to the discussion here[1].
>>>>>>
>>>>>> Unfortunately the lpuart driver is now broken for legacy code and also
>>>>>> the driver doesn't
>>>>>> work with serial driver model enabled on Toradex Colibri VF50/VF61,
>>>>>> Freescale VF610twr
>>>>>> and Phytec pcm052 boards. Did some one tested this patchset on these
>>>>>> boards ?
>>>>>
>>>>>
>>>>> I will fix the legacy code build in v2. About serial driver model not
>>>>> working on these boards, is that the caused by no device tree of these
>>>>> boards?
>>>>
>>>>
>>>> Yes, i tested on Colibri VF50/VF61 with device tree and it works fine.
>>>
>>> Great to know!
>>>
>>>> I think it would be nice to have the support for both platform data and
>>>> device tree so that we can use it with platform data via board files and
>>>> device tree too.
>>>
>>> I believe we should introduce device tree support on these boards. The
>>> configuration data (like in your patches the reg base for LPUART)
>>> should really be put into device tree. I adapted the comments from
>>> platdata.h below:
>>
>> Currently colibri_vf has both, a DT and a non-DT config. There has been
>> only one driver (SPI I think?) which required DT so far, and since most
>> user do not use that driver, we created two configs.
>>
>> However, if something like UART requires DT, then we can as well drop
>> the non-DT config for colibri_vf.
>>
>
> I vote for dropping the non-DT config.
>
>>>
>>> 31/**
>>> 32 * NOTE: Avoid using these except in extreme circumstances, where device tree
>>> 33 * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is
>>> 34 * available). U-Boot's driver model uses device tree for configuration.
>>> 35 */
>>> 36#define U_BOOT_DEVICE(__name)                                           \
>>> 37        ll_entry_declare(struct driver_info, __name, driver_info)
>>>
>>
>> Since Vybrid has so large internal SRAM, there has been no need for SPL
>> at all so far. Not sure about LS1021a/other LPUART SoCs...
>>
>
> LS1021 has 128KB SRAM, and current lp1021atwr_nor_lpuart does not use
> SPL. It boots from NOR flash.
>
>>>>
>>>> Since only few boards are using lpuart driver we can update the driver
>>>> completly to driver model, drop the legacy code and update the boards.
>>>>
>>>
>>> Since in my patches I only updated ls1021atwr board to use driver
>>> model serial, and I don't have those other boards (like Colibri
>>> VF50/VF61) to test this lpuart dm driver. I chose to leave the legacy
>>> codes there. On top of my series, you can prepare a patch to
>>> completely drop those legacy codes after you switch to use driver
>>> model lpuart driver on those boards in your series. Then we get a
>>> legacy-free driver for lpuart boards :)
>>
>> I guess nobody has all this boards, we should nontheless try to find a
>> solution for all of them.
>>
>> I see three options:
>> - Leave legacy code and the other boards as is (pcm052/vf610twr)
>> - Drop legacy code, and add platform data support and the corresponding
>> platform data for pcm052/vf610twr (in this case, we could also keep the
>> colibri_vf non-DT config)
>> - Drop legacy code, add device tree for pcm052/vf610twr, extend
>> colibri_vf device tree and drop non-DT config for colibri_vf.
>>
>> I am inclined to say lets go for the pure DT solution, since that is
>> where U-Boot is evolving to long term anyway. Not sure how much work is
>> required to make that happen. I guess the lpuart only DT for
>> pcm052/vf610twr should be fairly easy to create...?
>>
>> Other opinions?
>>
>> --
>
> I would go for option 3.

I too agree with Stefan and Bin for going with pure DT solution. Will do 
the device tree files for vf610-twr, pcm052 boards and extend 
Colibri-VFxx board with lpuart support. Will submit the new patchset 
after Bin's patchset upstreamed.

>
> Regards,
> Bin
>

-- 
Best regards,
Bhuvan

[U-Boot] [PATCH 8/8] arm: ls1021atwr: Enable driver model lpuart serial driver

[Bin Meng]

Convert ls1021atwr_nor_lpuart to driver model support. As a start,
enable lpuart serial port driver.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>

---

 arch/arm/dts/ls1021a-twr.dts            | 4 ++++
 configs/ls1021atwr_nor_lpuart_defconfig | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm/dts/ls1021a-twr.dts b/arch/arm/dts/ls1021a-twr.dts
index aead13f..deabb12 100644
--- a/arch/arm/dts/ls1021a-twr.dts
+++ b/arch/arm/dts/ls1021a-twr.dts
@@ -21,6 +21,10 @@
 	};
 
 	chosen {
+		/*
+		 * With ls1021atwr_nor_lpuart_defconfig configuration,
+		 * this needs to be changed to &lpuart0.
+		 */
 		stdout-path = &uart0;
 	};
 };
diff --git a/configs/ls1021atwr_nor_lpuart_defconfig b/configs/ls1021atwr_nor_lpuart_defconfig
index 58cd61f..5346359 100644
--- a/configs/ls1021atwr_nor_lpuart_defconfig
+++ b/configs/ls1021atwr_nor_lpuart_defconfig
@@ -1,7 +1,11 @@
 CONFIG_ARM=y
 CONFIG_TARGET_LS1021ATWR=y
+CONFIG_DM_SERIAL=y
+CONFIG_DEFAULT_DEVICE_TREE="ls1021a-twr"
 CONFIG_SYS_EXTRA_OPTIONS="LPUART"
 # CONFIG_CMD_SETEXPR is not set
+CONFIG_OF_CONTROL=y
+CONFIG_DM=y
 CONFIG_NETDEVICES=y
 CONFIG_E1000=y
 CONFIG_FSL_LPUART=y
-- 
1.8.2.1

[U-Boot] [PATCH 8/8] arm: ls1021atwr: Enable driver model lpuart serial driver

[Simon Glass]

On 31 December 2015 at 01:53, Bin Meng <bmeng.cn@gmail.com> wrote:
> Convert ls1021atwr_nor_lpuart to driver model support. As a start,
> enable lpuart serial port driver.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
>  arch/arm/dts/ls1021a-twr.dts            | 4 ++++
>  configs/ls1021atwr_nor_lpuart_defconfig | 4 ++++
>  2 files changed, 8 insertions(+)

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

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Huan Wang]

Hi,

	I tested this set on my LS1021ATWR board. NOR boot using DUART as serial output is ok. But NOR boot using LPUART as serial output failed. How about your test result?


Best Regards,
Alison Wang

> -----Original Message-----
> From: Bin Meng [mailto:bmeng.cn at gmail.com]
> Sent: Thursday, December 31, 2015 4:53 PM
> To: Simon Glass; Yusong Sun; Alison Wang; U-Boot Mailing List
> Subject: [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable
> serial support
> 
> This series converts Freescale LS1021A-TWR board to driver model.
>   - Enable ns16550 serial driver on ls1021atwr_nor configuration
>   - Convert LPUART serial driver to driver model
>   - Enable LPUART serial driver on ls1021atwr_nor_lpuart configuration
> 
> 
> Bin Meng (8):
>   fdt: Fix up stdout correctly in fdt_fixup_stdout()
>   arm: ls1021atwr: Convert to driver model and enable serial support
>   serial: lpuart: Move CONFIG_FSL_LPUART to Kconfig
>   serial: lpuart: Fix several cosmetic issues
>   serial: lpuart: Call local version of setbrg and putc directly
>   serial: lpuart: Prepare the driver for DM conversion
>   serial: lpuart: Add driver model serial support
>   arm: ls1021atwr: Enable driver model lpuart serial driver
> 
>  arch/arm/dts/ls1021a-twr.dts                 |   8 +
>  arch/arm/dts/ls1021a.dtsi                    |   4 -
>  common/fdt_support.c                         |  16 +-
>  configs/colibri_vf_defconfig                 |   1 +
>  configs/colibri_vf_dtb_defconfig             |   1 +
>  configs/ls1021aqds_ddr4_nor_lpuart_defconfig |   1 +
>  configs/ls1021aqds_nor_lpuart_defconfig      |   1 +
>  configs/ls1021atwr_nor_defconfig             |   4 +
>  configs/ls1021atwr_nor_lpuart_defconfig      |   5 +
>  configs/pcm052_defconfig                     |   1 +
>  configs/vf610twr_defconfig                   |   1 +
>  configs/vf610twr_nand_defconfig              |   1 +
>  doc/driver-model/serial-howto.txt            |   1 -
>  drivers/serial/Kconfig                       |   6 +
>  drivers/serial/serial_lpuart.c               | 334
> +++++++++++++++++++++------
>  include/configs/colibri_vf.h                 |   1 -
>  include/configs/ls1021aqds.h                 |   1 -
>  include/configs/ls1021atwr.h                 |   3 +-
>  include/configs/pcm052.h                     |   1 -
>  include/configs/vf610twr.h                   |   1 -
>  20 files changed, 298 insertions(+), 94 deletions(-)
> 
> --
> 1.8.2.1

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Bin Meng]

Hi Alison,

On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
> Hi,
>
>         I tested this set on my LS1021ATWR board. NOR boot using DUART as serial output is ok. But NOR boot using LPUART as serial output failed. How about your test result?
>

Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect you
forgot to update the RCW for LPUART, or change the jumper setting
(J19, J20) to route the LPUART signal.

[snip]

Regards,
Bin

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Huan Wang]

Hi, Bin,

> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
> > Hi,
> >
> >         I tested this set on my LS1021ATWR board. NOR boot using DUART
> as serial output is ok. But NOR boot using LPUART as serial output
> failed. How about your test result?
> >
> 
> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect you
> forgot to update the RCW for LPUART, or change the jumper setting (J19,
> J20) to route the LPUART signal.
> 
[Alison Wang] I updated the RCW for LPUART and changed the jumper setting
(J19, J20). Without this set, NOR boot using LPUART is ok. But after adding
this set, NOR boot using LPUART failed.


Best Regards,
Alison Wang

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Bin Meng]

Hi Alison,

On Thu, Jan 7, 2016 at 2:01 PM, Huan Wang <alison.wang@nxp.com> wrote:
> Hi, Bin,
>
>> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
>> > Hi,
>> >
>> >         I tested this set on my LS1021ATWR board. NOR boot using DUART
>> as serial output is ok. But NOR boot using LPUART as serial output
>> failed. How about your test result?
>> >
>>
>> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect you
>> forgot to update the RCW for LPUART, or change the jumper setting (J19,
>> J20) to route the LPUART signal.
>>
> [Alison Wang] I updated the RCW for LPUART and changed the jumper setting
> (J19, J20). Without this set, NOR boot using LPUART is ok. But after adding
> this set, NOR boot using LPUART failed.
>

That's strange. Can you please send your build instructions? Did you
program the u-boot-dtb.bin (not u-boot.bin anymore) to the NOR?

Regards,
Bin

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Huan Wang]

Hi, Bin,

> On Thu, Jan 7, 2016 at 2:01 PM, Huan Wang <alison.wang@nxp.com> wrote:
> > Hi, Bin,
> >
> >> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
> >> > Hi,
> >> >
> >> >         I tested this set on my LS1021ATWR board. NOR boot using
> >> > DUART
> >> as serial output is ok. But NOR boot using LPUART as serial output
> >> failed. How about your test result?
> >> >
> >>
> >> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect
> >> you forgot to update the RCW for LPUART, or change the jumper setting
> >> (J19,
> >> J20) to route the LPUART signal.
> >>
> > [Alison Wang] I updated the RCW for LPUART and changed the jumper
> > setting (J19, J20). Without this set, NOR boot using LPUART is ok. But
> > after adding this set, NOR boot using LPUART failed.
> >
> 
> That's strange. Can you please send your build instructions? Did you
> program the u-boot-dtb.bin (not u-boot.bin anymore) to the NOR?
> 
[Alison Wang] My build instructions are,

make ARCH=arm ls1021atwr_nor_lpuart_defconfig
make

Yes. I programmed u-boot-dtb.bin to NOR flash.


Best Regards,
Alison Wang

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Bin Meng]

Hi Alison,

On Thu, Jan 7, 2016 at 2:19 PM, Huan Wang <alison.wang@nxp.com> wrote:
> Hi, Bin,
>
>> On Thu, Jan 7, 2016 at 2:01 PM, Huan Wang <alison.wang@nxp.com> wrote:
>> > Hi, Bin,
>> >
>> >> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
>> >> > Hi,
>> >> >
>> >> >         I tested this set on my LS1021ATWR board. NOR boot using
>> >> > DUART
>> >> as serial output is ok. But NOR boot using LPUART as serial output
>> >> failed. How about your test result?
>> >> >
>> >>
>> >> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect
>> >> you forgot to update the RCW for LPUART, or change the jumper setting
>> >> (J19,
>> >> J20) to route the LPUART signal.
>> >>
>> > [Alison Wang] I updated the RCW for LPUART and changed the jumper
>> > setting (J19, J20). Without this set, NOR boot using LPUART is ok. But
>> > after adding this set, NOR boot using LPUART failed.
>> >
>>
>> That's strange. Can you please send your build instructions? Did you
>> program the u-boot-dtb.bin (not u-boot.bin anymore) to the NOR?
>>
> [Alison Wang] My build instructions are,
>
> make ARCH=arm ls1021atwr_nor_lpuart_defconfig
> make
>
> Yes. I programmed u-boot-dtb.bin to NOR flash.
>

Oh, it turns out the last thing to suspect is the /chose node :)

See comments in patch#8 in this series
(http://patchwork.ozlabs.org/patch/561859/)

  chosen {
+ /*
+ * With ls1021atwr_nor_lpuart_defconfig configuration,
+ * this needs to be changed to &lpuart0.
+ */
  stdout-path = &uart0;
  };

I was wondering whether I should introduce another separate dts file
for this LPUART configuration, something like ls1021a-twr_lpuart.dts,
but I felt it's quite some duplication thus I chose to just add some
comments under the /chosen node in the original dts file.

I've done the PCI UART support on the Crown Bay before using the same
way. Simon, do you have some better way to handle this?

Regards,
Bin

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Bin Meng]

Hi Simon,

On Thu, Jan 7, 2016 at 5:22 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Alison,
>
> On Thu, Jan 7, 2016 at 2:19 PM, Huan Wang <alison.wang@nxp.com> wrote:
>> Hi, Bin,
>>
>>> On Thu, Jan 7, 2016 at 2:01 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>> > Hi, Bin,
>>> >
>>> >> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>> >> > Hi,
>>> >> >
>>> >> >         I tested this set on my LS1021ATWR board. NOR boot using
>>> >> > DUART
>>> >> as serial output is ok. But NOR boot using LPUART as serial output
>>> >> failed. How about your test result?
>>> >> >
>>> >>
>>> >> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect
>>> >> you forgot to update the RCW for LPUART, or change the jumper setting
>>> >> (J19,
>>> >> J20) to route the LPUART signal.
>>> >>
>>> > [Alison Wang] I updated the RCW for LPUART and changed the jumper
>>> > setting (J19, J20). Without this set, NOR boot using LPUART is ok. But
>>> > after adding this set, NOR boot using LPUART failed.
>>> >
>>>
>>> That's strange. Can you please send your build instructions? Did you
>>> program the u-boot-dtb.bin (not u-boot.bin anymore) to the NOR?
>>>
>> [Alison Wang] My build instructions are,
>>
>> make ARCH=arm ls1021atwr_nor_lpuart_defconfig
>> make
>>
>> Yes. I programmed u-boot-dtb.bin to NOR flash.
>>
>
> Oh, it turns out the last thing to suspect is the /chose node :)
>
> See comments in patch#8 in this series
> (http://patchwork.ozlabs.org/patch/561859/)
>
>   chosen {
> + /*
> + * With ls1021atwr_nor_lpuart_defconfig configuration,
> + * this needs to be changed to &lpuart0.
> + */
>   stdout-path = &uart0;
>   };
>
> I was wondering whether I should introduce another separate dts file
> for this LPUART configuration, something like ls1021a-twr_lpuart.dts,
> but I felt it's quite some duplication thus I chose to just add some
> comments under the /chosen node in the original dts file.
>
> I've done the PCI UART support on the Crown Bay before using the same
> way. Simon, do you have some better way to handle this?

Alison has tested this series on her board. I believe the only concern
is with the lpuart in the /chosen node. Can you comment on this?

Regards,
Bin

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Simon Glass]

Hi,

On 10 January 2016 at 20:10, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Thu, Jan 7, 2016 at 5:22 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
>> Hi Alison,
>>
>> On Thu, Jan 7, 2016 at 2:19 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>> Hi, Bin,
>>>
>>>> On Thu, Jan 7, 2016 at 2:01 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>>> > Hi, Bin,
>>>> >
>>>> >> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>>> >> > Hi,
>>>> >> >
>>>> >> >         I tested this set on my LS1021ATWR board. NOR boot using
>>>> >> > DUART
>>>> >> as serial output is ok. But NOR boot using LPUART as serial output
>>>> >> failed. How about your test result?
>>>> >> >
>>>> >>
>>>> >> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect
>>>> >> you forgot to update the RCW for LPUART, or change the jumper setting
>>>> >> (J19,
>>>> >> J20) to route the LPUART signal.
>>>> >>
>>>> > [Alison Wang] I updated the RCW for LPUART and changed the jumper
>>>> > setting (J19, J20). Without this set, NOR boot using LPUART is ok. But
>>>> > after adding this set, NOR boot using LPUART failed.
>>>> >
>>>>
>>>> That's strange. Can you please send your build instructions? Did you
>>>> program the u-boot-dtb.bin (not u-boot.bin anymore) to the NOR?
>>>>
>>> [Alison Wang] My build instructions are,
>>>
>>> make ARCH=arm ls1021atwr_nor_lpuart_defconfig
>>> make
>>>
>>> Yes. I programmed u-boot-dtb.bin to NOR flash.
>>>
>>
>> Oh, it turns out the last thing to suspect is the /chose node :)
>>
>> See comments in patch#8 in this series
>> (http://patchwork.ozlabs.org/patch/561859/)
>>
>>   chosen {
>> + /*
>> + * With ls1021atwr_nor_lpuart_defconfig configuration,
>> + * this needs to be changed to &lpuart0.
>> + */
>>   stdout-path = &uart0;
>>   };
>>
>> I was wondering whether I should introduce another separate dts file
>> for this LPUART configuration, something like ls1021a-twr_lpuart.dts,
>> but I felt it's quite some duplication thus I chose to just add some
>> comments under the /chosen node in the original dts file.
>>
>> I've done the PCI UART support on the Crown Bay before using the same
>> way. Simon, do you have some better way to handle this?
>
> Alison has tested this series on her board. I believe the only concern
> is with the lpuart in the /chosen node. Can you comment on this?

It looks OK to me. You could create a separate device tree and put the
common contents in a .dtsi file,. I'm not sure of the trade-offs with
this though.

Regards,
Simon

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Stefan Agner]

On 2016-01-07 01:22, Bin Meng wrote:
> Hi Alison,
> 
> On Thu, Jan 7, 2016 at 2:19 PM, Huan Wang <alison.wang@nxp.com> wrote:
>> Hi, Bin,
>>
>>> On Thu, Jan 7, 2016 at 2:01 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>> > Hi, Bin,
>>> >
>>> >> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>> >> > Hi,
>>> >> >
>>> >> >         I tested this set on my LS1021ATWR board. NOR boot using
>>> >> > DUART
>>> >> as serial output is ok. But NOR boot using LPUART as serial output
>>> >> failed. How about your test result?
>>> >> >
>>> >>
>>> >> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect
>>> >> you forgot to update the RCW for LPUART, or change the jumper setting
>>> >> (J19,
>>> >> J20) to route the LPUART signal.
>>> >>
>>> > [Alison Wang] I updated the RCW for LPUART and changed the jumper
>>> > setting (J19, J20). Without this set, NOR boot using LPUART is ok. But
>>> > after adding this set, NOR boot using LPUART failed.
>>> >
>>>
>>> That's strange. Can you please send your build instructions? Did you
>>> program the u-boot-dtb.bin (not u-boot.bin anymore) to the NOR?
>>>
>> [Alison Wang] My build instructions are,
>>
>> make ARCH=arm ls1021atwr_nor_lpuart_defconfig
>> make
>>
>> Yes. I programmed u-boot-dtb.bin to NOR flash.
>>
> 
> Oh, it turns out the last thing to suspect is the /chose node :)
> 
> See comments in patch#8 in this series
> (http://patchwork.ozlabs.org/patch/561859/)
> 
>   chosen {
> + /*
> + * With ls1021atwr_nor_lpuart_defconfig configuration,
> + * this needs to be changed to &lpuart0.
> + */
>   stdout-path = &uart0;
>   };
> 
> I was wondering whether I should introduce another separate dts file
> for this LPUART configuration, something like ls1021a-twr_lpuart.dts,
> but I felt it's quite some duplication thus I chose to just add some
> comments under the /chosen node in the original dts file.

How about:
ls1021a-twr.dts => ls1021a-twr.dtsi

And include that from
ls1021a-twr-uart.dts
ls1021a-twr-lpuart.dts
and add the /chosen node to those files.

--
Stefan

[U-Boot] [PATCH 0/8] arm: ls1021atwr: Convert to driver model and enable serial support

[Bin Meng]

On Thu, Jan 14, 2016 at 3:03 AM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-01-07 01:22, Bin Meng wrote:
>> Hi Alison,
>>
>> On Thu, Jan 7, 2016 at 2:19 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>> Hi, Bin,
>>>
>>>> On Thu, Jan 7, 2016 at 2:01 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>>> > Hi, Bin,
>>>> >
>>>> >> On Wed, Jan 6, 2016 at 1:31 PM, Huan Wang <alison.wang@nxp.com> wrote:
>>>> >> > Hi,
>>>> >> >
>>>> >> >         I tested this set on my LS1021ATWR board. NOR boot using
>>>> >> > DUART
>>>> >> as serial output is ok. But NOR boot using LPUART as serial output
>>>> >> failed. How about your test result?
>>>> >> >
>>>> >>
>>>> >> Yes, I tested NOR boot using DUART or LPUART. Both boot. I suspect
>>>> >> you forgot to update the RCW for LPUART, or change the jumper setting
>>>> >> (J19,
>>>> >> J20) to route the LPUART signal.
>>>> >>
>>>> > [Alison Wang] I updated the RCW for LPUART and changed the jumper
>>>> > setting (J19, J20). Without this set, NOR boot using LPUART is ok. But
>>>> > after adding this set, NOR boot using LPUART failed.
>>>> >
>>>>
>>>> That's strange. Can you please send your build instructions? Did you
>>>> program the u-boot-dtb.bin (not u-boot.bin anymore) to the NOR?
>>>>
>>> [Alison Wang] My build instructions are,
>>>
>>> make ARCH=arm ls1021atwr_nor_lpuart_defconfig
>>> make
>>>
>>> Yes. I programmed u-boot-dtb.bin to NOR flash.
>>>
>>
>> Oh, it turns out the last thing to suspect is the /chose node :)
>>
>> See comments in patch#8 in this series
>> (http://patchwork.ozlabs.org/patch/561859/)
>>
>>   chosen {
>> + /*
>> + * With ls1021atwr_nor_lpuart_defconfig configuration,
>> + * this needs to be changed to &lpuart0.
>> + */
>>   stdout-path = &uart0;
>>   };
>>
>> I was wondering whether I should introduce another separate dts file
>> for this LPUART configuration, something like ls1021a-twr_lpuart.dts,
>> but I felt it's quite some duplication thus I chose to just add some
>> comments under the /chosen node in the original dts file.
>
> How about:
> ls1021a-twr.dts => ls1021a-twr.dtsi
>
> And include that from
> ls1021a-twr-uart.dts
> ls1021a-twr-lpuart.dts
> and add the /chosen node to those files.
>

Yep, will take this approach in v2. Thanks!

Regards,
Bin