All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] Add support to fetch baudrate from dtb
@ 2023-05-25  4:02 Venkatesh Yadav Abbarapu
  2023-05-25  4:02 ` [PATCH v5 1/2] configs: Add support in Kconfig and convert for armada boards Venkatesh Yadav Abbarapu
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Venkatesh Yadav Abbarapu @ 2023-05-25  4:02 UTC (permalink / raw)
  To: u-boot; +Cc: michal.simek, sjg, git

In this patch series
- Add support in Kconfig and convert for armada boards
- Fetch baudrate from the dtb and update

Changes in v5:
- Adding DEFAULT_ENV_IS_RW Kconfig in missing files
- Updating DEFAULT_ENV_IS_RW to CONFIG_DEFAULT_ENV_IS_RW

Changes in v4:
- Moved SERIAL_DT_BAUD to another patch
- Added doc file for fetching serial baudrate from DT.
- Changed Kconfig SERIAL_DT_BAUD to OF_SERIAL_DT_BAUD
- Added function docs wherever required.
- Moved changes from fdtdec api to ofnode
- Changed serial_get_valid_baudrate to check_valid_baudrate
- Added function fetch_baud_from_dtb to fetch baud from DT
- Used dectoul() for parsing baudrate

Changes in v3:
- Add SERIAL_DT_BAUD to Kconfig
- Moved DEFAULT_ENV_IS_RW to Kconfig also updated armada files
- Moved filler changes from zynqmp.h to generic file env_default.h
- Removed ENV_RW_FILLER and added padding in the generic file env_default.h.
- Print baudrate parameter properly when SERIAL_DT is enabled.

Changes in v2:
- Changed to #ifdef from #if CONFIG_IS_ENABLED to enable patching in
spl.
- Added SPL_ENV_SUPPORT dependency in SERIAL_DT_BAUD to allow SPL
compilation.
- Moved DEFAULT_ENV_IS_RW to Kconfig also updated armada files
- Moved ENV_RW_FILLER to generic file env_default.h.
- Increased the ENV_RW_FILLER padding to support 8000000 baud.

Algapally Santosh Sagar (2):
  configs: Add support in Kconfig and convert for armada boards
  serial: zynqmp: Fetch baudrate from dtb and update

 configs/eDPU_defconfig                      |  1 +
 configs/mvebu_db-88f3720_defconfig          |  1 +
 configs/mvebu_espressobin-88f3720_defconfig |  1 +
 configs/uDPU_defconfig                      |  1 +
 doc/README.serial_dt_baud                   | 41 ++++++++++++++++++++
 drivers/core/ofnode.c                       | 20 ++++++++++
 drivers/serial/Kconfig                      | 15 ++++++++
 drivers/serial/serial-uclass.c              | 42 +++++++++++++++++++++
 include/configs/mvebu_armada-37xx.h         |  1 -
 include/dm/ofnode.h                         | 14 ++++++-
 include/env_default.h                       |  8 +++-
 include/env_internal.h                      |  2 +-
 include/serial.h                            | 15 ++++++++
 13 files changed, 156 insertions(+), 6 deletions(-)
 create mode 100644 doc/README.serial_dt_baud

-- 
2.17.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 1/2] configs: Add support in Kconfig and convert for armada boards
  2023-05-25  4:02 [PATCH v5 0/2] Add support to fetch baudrate from dtb Venkatesh Yadav Abbarapu
@ 2023-05-25  4:02 ` Venkatesh Yadav Abbarapu
  2023-06-12 21:17   ` Simon Glass
  2023-05-25  4:02 ` [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update Venkatesh Yadav Abbarapu
  2023-06-05 13:41 ` [PATCH v5 0/2] Add support to fetch baudrate from dtb Michal Simek
  2 siblings, 1 reply; 10+ messages in thread
From: Venkatesh Yadav Abbarapu @ 2023-05-25  4:02 UTC (permalink / raw)
  To: u-boot; +Cc: michal.simek, sjg, git, Algapally Santosh Sagar

From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>

The DEFAULT_ENV_IS_RW is moved to the Kconfig for easier configuration.
Hence, the CONFIG_DEFAULT_ENV_IS_RW config is added to the defconfig files
to allow enabling them for armada boards.

Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
---
 configs/eDPU_defconfig                      | 1 +
 configs/mvebu_db-88f3720_defconfig          | 1 +
 configs/mvebu_espressobin-88f3720_defconfig | 1 +
 configs/uDPU_defconfig                      | 1 +
 drivers/serial/Kconfig                      | 6 ++++++
 include/configs/mvebu_armada-37xx.h         | 1 -
 include/env_default.h                       | 2 +-
 include/env_internal.h                      | 2 +-
 8 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/configs/eDPU_defconfig b/configs/eDPU_defconfig
index 77ea2b2eec..61fb9bd2a5 100644
--- a/configs/eDPU_defconfig
+++ b/configs/eDPU_defconfig
@@ -21,6 +21,7 @@ CONFIG_DISTRO_DEFAULTS=y
 CONFIG_USE_PREBOOT=y
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DEFAULT_ENV_IS_RW=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SYS_MAXARGS=32
diff --git a/configs/mvebu_db-88f3720_defconfig b/configs/mvebu_db-88f3720_defconfig
index 829567014f..e6fb80167a 100644
--- a/configs/mvebu_db-88f3720_defconfig
+++ b/configs/mvebu_db-88f3720_defconfig
@@ -22,6 +22,7 @@ CONFIG_USE_PREBOOT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DEFAULT_ENV_IS_RW=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SYS_MAXARGS=32
diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
index fc394a7e9d..64ee99d64b 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -24,6 +24,7 @@ CONFIG_USE_PREBOOT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DEFAULT_ENV_IS_RW=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_BOARD_LATE_INIT=y
diff --git a/configs/uDPU_defconfig b/configs/uDPU_defconfig
index fa1989518b..4d3d53ac7e 100644
--- a/configs/uDPU_defconfig
+++ b/configs/uDPU_defconfig
@@ -21,6 +21,7 @@ CONFIG_DISTRO_DEFAULTS=y
 CONFIG_USE_PREBOOT=y
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DEFAULT_ENV_IS_RW=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SYS_MAXARGS=32
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index f4767c838f..5c9b924e73 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -24,6 +24,12 @@ config BAUDRATE
 	  in the SPL stage (most drivers) or for choosing a default baudrate
 	  in the absence of an environment setting (serial_mxc.c).
 
+config DEFAULT_ENV_IS_RW
+	bool "Make default environment as writable"
+	help
+	  Select this to enable to make default environment writable. This
+	  allows modifying the default environment.
+
 config REQUIRE_SERIAL_CONSOLE
 	bool "Require a serial port for console"
 	# Running without a serial console is not supported by the
diff --git a/include/configs/mvebu_armada-37xx.h b/include/configs/mvebu_armada-37xx.h
index 76e148f55e..18b55be0d8 100644
--- a/include/configs/mvebu_armada-37xx.h
+++ b/include/configs/mvebu_armada-37xx.h
@@ -30,7 +30,6 @@
 /*
  * Environment
  */
-#define DEFAULT_ENV_IS_RW		/* required for configuring default fdtfile= */
 
 #ifdef CONFIG_MMC
 #define BOOT_TARGET_DEVICES_MMC(func, i) func(MMC, mmc, i)
diff --git a/include/env_default.h b/include/env_default.h
index b16c22d5a2..227cad7c34 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -21,7 +21,7 @@ env_t embedded_environment __UBOOT_ENV_SECTION__(environment) = {
 	{
 #elif defined(DEFAULT_ENV_INSTANCE_STATIC)
 static char default_environment[] = {
-#elif defined(DEFAULT_ENV_IS_RW)
+#elif defined(CONFIG_DEFAULT_ENV_IS_RW)
 char default_environment[] = {
 #else
 const char default_environment[] = {
diff --git a/include/env_internal.h b/include/env_internal.h
index 6a69494646..fcb464263f 100644
--- a/include/env_internal.h
+++ b/include/env_internal.h
@@ -89,7 +89,7 @@ typedef struct environment_s {
 extern env_t embedded_environment;
 #endif /* ENV_IS_EMBEDDED */
 
-#ifdef DEFAULT_ENV_IS_RW
+#ifdef CONFIG_DEFAULT_ENV_IS_RW
 extern char default_environment[];
 #else
 extern const char default_environment[];
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update
  2023-05-25  4:02 [PATCH v5 0/2] Add support to fetch baudrate from dtb Venkatesh Yadav Abbarapu
  2023-05-25  4:02 ` [PATCH v5 1/2] configs: Add support in Kconfig and convert for armada boards Venkatesh Yadav Abbarapu
@ 2023-05-25  4:02 ` Venkatesh Yadav Abbarapu
  2023-06-07 19:13   ` Tom Rini
  2023-06-12 21:17   ` Simon Glass
  2023-06-05 13:41 ` [PATCH v5 0/2] Add support to fetch baudrate from dtb Michal Simek
  2 siblings, 2 replies; 10+ messages in thread
From: Venkatesh Yadav Abbarapu @ 2023-05-25  4:02 UTC (permalink / raw)
  To: u-boot; +Cc: michal.simek, sjg, git, Algapally Santosh Sagar

From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>

The baudrate configured in .config is taken by default by serial. If
change of baudrate is required then the .config needs to changed and
u-boot recompilation is required or the u-boot environment needs to be
updated.

To avoid this, support is added to fetch the baudrate directly from the
device tree file and update.
The serial, prints the log with the configured baudrate in the dtb.
The commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
$fdtfile env variable") is taken as reference for changing the default
environment variable.

The default environment stores the default baudrate value, When default
baudrate and dtb baudrate are not same glitches are seen on the serial.
So, the environment also needs to be updated with the dtb baudrate to
avoid the glitches on the serial.

Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
---
 doc/README.serial_dt_baud      | 41 +++++++++++++++++++++++++++++++++
 drivers/core/ofnode.c          | 20 ++++++++++++++++
 drivers/serial/Kconfig         |  9 ++++++++
 drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++
 include/dm/ofnode.h            | 14 ++++++++++--
 include/env_default.h          |  6 ++++-
 include/serial.h               | 15 ++++++++++++
 7 files changed, 144 insertions(+), 3 deletions(-)
 create mode 100644 doc/README.serial_dt_baud

diff --git a/doc/README.serial_dt_baud b/doc/README.serial_dt_baud
new file mode 100644
index 0000000000..02974ab1a7
--- /dev/null
+++ b/doc/README.serial_dt_baud
@@ -0,0 +1,41 @@
+Fetch serial baudrate from DT
+-----------------------------
+
+To support fetching of baudrate from DT, the following is done:-
+
+The baudrate configured in Kconfig symbol CONFIG_BAUDRATE is taken by default by serial.
+If change of baudrate is required then the Kconfig symbol CONFIG_BAUDRATE needs to
+changed and U-Boot recompilation is required or the U-Boot environment needs to be updated.
+
+To avoid this, add support to fetch the baudrate directly from the device tree file and
+update the environment.
+
+The default environment stores the default baudrate value. When default baudrate and dtb
+baudrate are not same glitches are seen on the serial.
+So, the environment also needs to be updated with the dtb baudrate to avoid the glitches on
+the serial which is enabled by SERIAL_DT_BAUD.
+
+The Kconfig SPL_ENV_SUPPORT needs to be enabled to allow patching in SPL.
+
+The Kconfig DEFAULT_ENV_IS_RW which is enabled by SERIAL_DT_BAUD with making the environment
+writable.
+
+The ofnode_read_baud() function parses and fetches the baudrate value from the DT. This value
+is validated and updated to baudrate during serial init. Padding is added at the end of the
+default environment and the dt baudrate is updated with the latest value.
+
+Example:-
+
+The serial port options are of the form "bbbbpnf", where "bbbb" is the baud rate, "p" is parity ("n", "o", or "e"),
+"n" is number of bits, and "f" is flow control ("r" for RTS or omit it). Default is "115200n8".
+
+chosen {
+		bootargs = "earlycon console=ttyPS0,115200 clk_ignore_unused root=/dev/ram0 rw init_fatal_sh=1";
+		stdout-path = "serial0:115200n8";
+	};
+
+From the chosen node, stdout-path property is obtained as string.
+
+	stdout-path = "serial0:115200n8";
+
+The string is parsed to get the baudrate 115200. This string is converted to integer and updated to the environment.
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index ec574c4460..04bdb30b24 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -870,6 +870,26 @@ ofnode ofnode_get_chosen_node(const char *name)
 	return ofnode_path(prop);
 }
 
+#ifdef CONFIG_OF_SERIAL_DT_BAUD
+int ofnode_read_baud(void)
+{
+	const char *str, *p;
+	u32 baud;
+
+	str = ofnode_read_chosen_string("stdout-path");
+	if (!str)
+		return -EINVAL;
+
+	/* Parse string serial0:115200n8 */
+	p = strchr(str, ':');
+	if (!p)
+		return -EINVAL;
+
+	baud = dectoul(p + 1, NULL);
+	return baud;
+}
+#endif
+
 const void *ofnode_read_aliases_prop(const char *propname, int *sizep)
 {
 	ofnode node;
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 5c9b924e73..ea2244e5db 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -24,6 +24,15 @@ config BAUDRATE
 	  in the SPL stage (most drivers) or for choosing a default baudrate
 	  in the absence of an environment setting (serial_mxc.c).
 
+config OF_SERIAL_DT_BAUD
+	bool "Fetch serial baudrate from device tree"
+	depends on DM_SERIAL && SPL_ENV_SUPPORT
+	select DEFAULT_ENV_IS_RW
+	help
+	  Select this to enable fetching and setting of the baudrate
+	  configured in the DT. Replace the default baudrate with the DT
+	  baudrate and also set it to the environment.
+
 config DEFAULT_ENV_IS_RW
 	bool "Make default environment as writable"
 	help
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 067fae2614..afa25a1f19 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -154,12 +154,54 @@ static void serial_find_console_or_panic(void)
 }
 #endif /* CONFIG_SERIAL_PRESENT */
 
+int check_valid_baudrate(int baud)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {
+		if (baud == baudrate_table[i])
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+int fetch_baud_from_dtb(void)
+{
+	int baud_value, ret;
+
+	baud_value = ofnode_read_baud();
+	ret = check_valid_baudrate(baud_value);
+	if (ret)
+		return ret;
+
+	return baud_value;
+}
+
 /* Called prior to relocation */
 int serial_init(void)
 {
 #if CONFIG_IS_ENABLED(SERIAL_PRESENT)
 	serial_find_console_or_panic();
 	gd->flags |= GD_FLG_SERIAL_READY;
+#ifdef CONFIG_OF_SERIAL_DT_BAUD
+	int ret = 0;
+	char *ptr = &default_environment[0];
+
+	/*
+	 * Fetch the baudrate from the dtb and update the value in the
+	 * default environment.
+	 */
+	ret = fetch_baud_from_dtb();
+	if (ret != -EINVAL && ret != -EFAULT) {
+		gd->baudrate = ret;
+
+		while (*ptr != '\0' && *(ptr + 1) != '\0')
+			ptr++;
+		ptr += 2;
+		sprintf(ptr, "baudrate=%d", gd->baudrate);
+	}
+#endif
 	serial_setbrg();
 #endif
 
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 443db6252d..5f90b6c65e 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -932,12 +932,22 @@ const char *ofnode_read_chosen_string(const char *propname);
 ofnode ofnode_get_chosen_node(const char *propname);
 
 /**
- * ofnode_read_aliases_prop() - get the value of a aliases property
+ * ofnode_read_baud() - get the baudrate from string value of chosen property
  *
- * This looks for a property within the /aliases node and returns its value
+ * This looks for stdout-path property within the /chosen node and parses its
+ * value to return baudrate.
  *
  * This only works with the control FDT.
  *
+ * Return: baudrate value if found, else error
+ */
+int ofnode_read_baud(void);
+
+/**
+ * ofnode_read_aliases_prop() - get the value of a aliases property
+ *
+ * This looks for a property within the /aliases node and returns its value
+ *
  * @propname: Property name to look for
  * @sizep: Returns size of property, or `FDT_ERR_...` error code if function
  *	returns NULL
diff --git a/include/env_default.h b/include/env_default.h
index 227cad7c34..1c859a8f65 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -42,7 +42,7 @@ const char default_environment[] = {
 #if defined(CONFIG_BOOTDELAY)
 	"bootdelay="	__stringify(CONFIG_BOOTDELAY)	"\0"
 #endif
-#if defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
+#if !defined(CONFIG_OF_SERIAL_DT_BAUD) && defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
 	"baudrate="	__stringify(CONFIG_BAUDRATE)	"\0"
 #endif
 #ifdef	CONFIG_LOADS_ECHO
@@ -118,6 +118,10 @@ const char default_environment[] = {
 #endif
 #ifdef	CFG_EXTRA_ENV_SETTINGS
 	CFG_EXTRA_ENV_SETTINGS
+#endif
+#ifdef CONFIG_OF_SERIAL_DT_BAUD
+	/* Padding for baudrate at the end when environment is writable */
+	"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
 #endif
 	"\0"
 #else /* CONFIG_USE_DEFAULT_ENV_FILE */
diff --git a/include/serial.h b/include/serial.h
index 42bdf3759c..b01047d07a 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -337,6 +337,21 @@ int serial_setconfig(struct udevice *dev, uint config);
  */
 int serial_getinfo(struct udevice *dev, struct serial_device_info *info);
 
+/**
+ * check_valid_baudrate() - Check whether baudrate is valid or not
+ *
+ * @baud: baudrate
+ * Return: 0 if OK, -ve on error
+ */
+int check_valid_baudrate(int baud);
+
+/**
+ * fetch_baud_from_dtb() - Fetch the baudrate value from DT
+ *
+ * Return: baudrate if OK, -ve on error
+ */
+int fetch_baud_from_dtb(void);
+
 void atmel_serial_initialize(void);
 void mcf_serial_initialize(void);
 void mpc85xx_serial_initialize(void);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/2] Add support to fetch baudrate from dtb
  2023-05-25  4:02 [PATCH v5 0/2] Add support to fetch baudrate from dtb Venkatesh Yadav Abbarapu
  2023-05-25  4:02 ` [PATCH v5 1/2] configs: Add support in Kconfig and convert for armada boards Venkatesh Yadav Abbarapu
  2023-05-25  4:02 ` [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update Venkatesh Yadav Abbarapu
@ 2023-06-05 13:41 ` Michal Simek
  2 siblings, 0 replies; 10+ messages in thread
From: Michal Simek @ 2023-06-05 13:41 UTC (permalink / raw)
  To: Venkatesh Yadav Abbarapu, u-boot, Simon Glass; +Cc: git

Hi Simon,

On 5/25/23 06:02, Venkatesh Yadav Abbarapu wrote:
> In this patch series
> - Add support in Kconfig and convert for armada boards
> - Fetch baudrate from the dtb and update
> 
> Changes in v5:
> - Adding DEFAULT_ENV_IS_RW Kconfig in missing files
> - Updating DEFAULT_ENV_IS_RW to CONFIG_DEFAULT_ENV_IS_RW
> 
> Changes in v4:
> - Moved SERIAL_DT_BAUD to another patch
> - Added doc file for fetching serial baudrate from DT.
> - Changed Kconfig SERIAL_DT_BAUD to OF_SERIAL_DT_BAUD
> - Added function docs wherever required.
> - Moved changes from fdtdec api to ofnode
> - Changed serial_get_valid_baudrate to check_valid_baudrate
> - Added function fetch_baud_from_dtb to fetch baud from DT
> - Used dectoul() for parsing baudrate
> 
> Changes in v3:
> - Add SERIAL_DT_BAUD to Kconfig
> - Moved DEFAULT_ENV_IS_RW to Kconfig also updated armada files
> - Moved filler changes from zynqmp.h to generic file env_default.h
> - Removed ENV_RW_FILLER and added padding in the generic file env_default.h.
> - Print baudrate parameter properly when SERIAL_DT is enabled.
> 
> Changes in v2:
> - Changed to #ifdef from #if CONFIG_IS_ENABLED to enable patching in
> spl.
> - Added SPL_ENV_SUPPORT dependency in SERIAL_DT_BAUD to allow SPL
> compilation.
> - Moved DEFAULT_ENV_IS_RW to Kconfig also updated armada files
> - Moved ENV_RW_FILLER to generic file env_default.h.
> - Increased the ENV_RW_FILLER padding to support 8000000 baud.
> 
> Algapally Santosh Sagar (2):
>    configs: Add support in Kconfig and convert for armada boards
>    serial: zynqmp: Fetch baudrate from dtb and update
> 
>   configs/eDPU_defconfig                      |  1 +
>   configs/mvebu_db-88f3720_defconfig          |  1 +
>   configs/mvebu_espressobin-88f3720_defconfig |  1 +
>   configs/uDPU_defconfig                      |  1 +
>   doc/README.serial_dt_baud                   | 41 ++++++++++++++++++++
>   drivers/core/ofnode.c                       | 20 ++++++++++
>   drivers/serial/Kconfig                      | 15 ++++++++
>   drivers/serial/serial-uclass.c              | 42 +++++++++++++++++++++
>   include/configs/mvebu_armada-37xx.h         |  1 -
>   include/dm/ofnode.h                         | 14 ++++++-
>   include/env_default.h                       |  8 +++-
>   include/env_internal.h                      |  2 +-
>   include/serial.h                            | 15 ++++++++
>   13 files changed, 156 insertions(+), 6 deletions(-)
>   create mode 100644 doc/README.serial_dt_baud
> 

as you commented previous series. Can you please comment these patches?

Thanks,
Michal

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update
  2023-05-25  4:02 ` [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update Venkatesh Yadav Abbarapu
@ 2023-06-07 19:13   ` Tom Rini
  2023-06-08  8:50     ` Michal Simek
  2023-06-12 21:17   ` Simon Glass
  1 sibling, 1 reply; 10+ messages in thread
From: Tom Rini @ 2023-06-07 19:13 UTC (permalink / raw)
  To: Venkatesh Yadav Abbarapu
  Cc: u-boot, michal.simek, sjg, git, Algapally Santosh Sagar

[-- Attachment #1: Type: text/plain, Size: 1353 bytes --]

On Thu, May 25, 2023 at 09:32:59AM +0530, Venkatesh Yadav Abbarapu wrote:

> From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
> 
> The baudrate configured in .config is taken by default by serial. If
> change of baudrate is required then the .config needs to changed and
> u-boot recompilation is required or the u-boot environment needs to be
> updated.
> 
> To avoid this, support is added to fetch the baudrate directly from the
> device tree file and update.
> The serial, prints the log with the configured baudrate in the dtb.
> The commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
> $fdtfile env variable") is taken as reference for changing the default
> environment variable.
> 
> The default environment stores the default baudrate value, When default
> baudrate and dtb baudrate are not same glitches are seen on the serial.
> So, the environment also needs to be updated with the dtb baudrate to
> avoid the glitches on the serial.
> 
> Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> ---
>  doc/README.serial_dt_baud      | 41 +++++++++++++++++++++++++++++++++

Sorry I missed this until v5.  This needs to be in rST and under doc/
somewhere (and make htmldocs happy).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update
  2023-06-07 19:13   ` Tom Rini
@ 2023-06-08  8:50     ` Michal Simek
  2023-06-08 16:35       ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Michal Simek @ 2023-06-08  8:50 UTC (permalink / raw)
  To: Tom Rini, Venkatesh Yadav Abbarapu
  Cc: u-boot, sjg, git, Algapally Santosh Sagar



On 6/7/23 21:13, Tom Rini wrote:
> On Thu, May 25, 2023 at 09:32:59AM +0530, Venkatesh Yadav Abbarapu wrote:
> 
>> From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
>>
>> The baudrate configured in .config is taken by default by serial. If
>> change of baudrate is required then the .config needs to changed and
>> u-boot recompilation is required or the u-boot environment needs to be
>> updated.
>>
>> To avoid this, support is added to fetch the baudrate directly from the
>> device tree file and update.
>> The serial, prints the log with the configured baudrate in the dtb.
>> The commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
>> $fdtfile env variable") is taken as reference for changing the default
>> environment variable.
>>
>> The default environment stores the default baudrate value, When default
>> baudrate and dtb baudrate are not same glitches are seen on the serial.
>> So, the environment also needs to be updated with the dtb baudrate to
>> avoid the glitches on the serial.
>>
>> Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
>> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
>> ---
>>   doc/README.serial_dt_baud      | 41 +++++++++++++++++++++++++++++++++
> 
> Sorry I missed this until v5.  This needs to be in rST and under doc/
> somewhere (and make htmldocs happy).

Any other issue with implementation?

M

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update
  2023-06-08  8:50     ` Michal Simek
@ 2023-06-08 16:35       ` Tom Rini
  0 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2023-06-08 16:35 UTC (permalink / raw)
  To: Michal Simek
  Cc: Venkatesh Yadav Abbarapu, u-boot, sjg, git, Algapally Santosh Sagar

[-- Attachment #1: Type: text/plain, Size: 1679 bytes --]

On Thu, Jun 08, 2023 at 10:50:36AM +0200, Michal Simek wrote:
> 
> 
> On 6/7/23 21:13, Tom Rini wrote:
> > On Thu, May 25, 2023 at 09:32:59AM +0530, Venkatesh Yadav Abbarapu wrote:
> > 
> > > From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
> > > 
> > > The baudrate configured in .config is taken by default by serial. If
> > > change of baudrate is required then the .config needs to changed and
> > > u-boot recompilation is required or the u-boot environment needs to be
> > > updated.
> > > 
> > > To avoid this, support is added to fetch the baudrate directly from the
> > > device tree file and update.
> > > The serial, prints the log with the configured baudrate in the dtb.
> > > The commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
> > > $fdtfile env variable") is taken as reference for changing the default
> > > environment variable.
> > > 
> > > The default environment stores the default baudrate value, When default
> > > baudrate and dtb baudrate are not same glitches are seen on the serial.
> > > So, the environment also needs to be updated with the dtb baudrate to
> > > avoid the glitches on the serial.
> > > 
> > > Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
> > > Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> > > ---
> > >   doc/README.serial_dt_baud      | 41 +++++++++++++++++++++++++++++++++
> > 
> > Sorry I missed this until v5.  This needs to be in rST and under doc/
> > somewhere (and make htmldocs happy).
> 
> Any other issue with implementation?

Nope, I noticed this while preparing to apply it to -next.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 1/2] configs: Add support in Kconfig and convert for armada boards
  2023-05-25  4:02 ` [PATCH v5 1/2] configs: Add support in Kconfig and convert for armada boards Venkatesh Yadav Abbarapu
@ 2023-06-12 21:17   ` Simon Glass
  0 siblings, 0 replies; 10+ messages in thread
From: Simon Glass @ 2023-06-12 21:17 UTC (permalink / raw)
  To: Venkatesh Yadav Abbarapu
  Cc: u-boot, michal.simek, git, Algapally Santosh Sagar

On Thu, 25 May 2023 at 05:03, Venkatesh Yadav Abbarapu
<venkatesh.abbarapu@amd.com> wrote:
>
> From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
>
> The DEFAULT_ENV_IS_RW is moved to the Kconfig for easier configuration.
> Hence, the CONFIG_DEFAULT_ENV_IS_RW config is added to the defconfig files
> to allow enabling them for armada boards.
>
> Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> ---
>  configs/eDPU_defconfig                      | 1 +
>  configs/mvebu_db-88f3720_defconfig          | 1 +
>  configs/mvebu_espressobin-88f3720_defconfig | 1 +
>  configs/uDPU_defconfig                      | 1 +
>  drivers/serial/Kconfig                      | 6 ++++++
>  include/configs/mvebu_armada-37xx.h         | 1 -
>  include/env_default.h                       | 2 +-
>  include/env_internal.h                      | 2 +-
>  8 files changed, 12 insertions(+), 3 deletions(-)
>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update
  2023-05-25  4:02 ` [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update Venkatesh Yadav Abbarapu
  2023-06-07 19:13   ` Tom Rini
@ 2023-06-12 21:17   ` Simon Glass
  2023-08-16  9:30     ` Abbarapu, Venkatesh
  1 sibling, 1 reply; 10+ messages in thread
From: Simon Glass @ 2023-06-12 21:17 UTC (permalink / raw)
  To: Venkatesh Yadav Abbarapu
  Cc: u-boot, michal.simek, git, Algapally Santosh Sagar

Hi Venkatesh,

On Thu, 25 May 2023 at 05:03, Venkatesh Yadav Abbarapu
<venkatesh.abbarapu@amd.com> wrote:
>
> From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
>
> The baudrate configured in .config is taken by default by serial. If
> change of baudrate is required then the .config needs to changed and
> u-boot recompilation is required or the u-boot environment needs to be
> updated.
>
> To avoid this, support is added to fetch the baudrate directly from the
> device tree file and update.
> The serial, prints the log with the configured baudrate in the dtb.
> The commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
> $fdtfile env variable") is taken as reference for changing the default
> environment variable.
>
> The default environment stores the default baudrate value, When default
> baudrate and dtb baudrate are not same glitches are seen on the serial.
> So, the environment also needs to be updated with the dtb baudrate to
> avoid the glitches on the serial.
>
> Signed-off-by: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> ---
>  doc/README.serial_dt_baud      | 41 +++++++++++++++++++++++++++++++++
>  drivers/core/ofnode.c          | 20 ++++++++++++++++
>  drivers/serial/Kconfig         |  9 ++++++++
>  drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++
>  include/dm/ofnode.h            | 14 ++++++++++--
>  include/env_default.h          |  6 ++++-
>  include/serial.h               | 15 ++++++++++++
>  7 files changed, 144 insertions(+), 3 deletions(-)
>  create mode 100644 doc/README.serial_dt_baud
>
> diff --git a/doc/README.serial_dt_baud b/doc/README.serial_dt_baud
> new file mode 100644
> index 0000000000..02974ab1a7
> --- /dev/null
> +++ b/doc/README.serial_dt_baud
> @@ -0,0 +1,41 @@
> +Fetch serial baudrate from DT
> +-----------------------------
> +
> +To support fetching of baudrate from DT, the following is done:-
> +
> +The baudrate configured in Kconfig symbol CONFIG_BAUDRATE is taken by default by serial.
> +If change of baudrate is required then the Kconfig symbol CONFIG_BAUDRATE needs to
> +changed and U-Boot recompilation is required or the U-Boot environment needs to be updated.
> +
> +To avoid this, add support to fetch the baudrate directly from the device tree file and
> +update the environment.
> +
> +The default environment stores the default baudrate value. When default baudrate and dtb
> +baudrate are not same glitches are seen on the serial.
> +So, the environment also needs to be updated with the dtb baudrate to avoid the glitches on
> +the serial which is enabled by SERIAL_DT_BAUD.

The baud rate is read from the env into gd->baudrate by init_baud_rate().

Updating the default environment seems pretty nasty to me...why is
that necessary?

Could we read it from DT in init_baud_rate() ?

> +
> +The Kconfig SPL_ENV_SUPPORT needs to be enabled to allow patching in SPL.
> +
> +The Kconfig DEFAULT_ENV_IS_RW which is enabled by SERIAL_DT_BAUD with making the environment
> +writable.
> +
> +The ofnode_read_baud() function parses and fetches the baudrate value from the DT. This value
> +is validated and updated to baudrate during serial init. Padding is added at the end of the
> +default environment and the dt baudrate is updated with the latest value.
> +
> +Example:-
> +
> +The serial port options are of the form "bbbbpnf", where "bbbb" is the baud rate, "p" is parity ("n", "o", or "e"),
> +"n" is number of bits, and "f" is flow control ("r" for RTS or omit it). Default is "115200n8".
> +
> +chosen {
> +               bootargs = "earlycon console=ttyPS0,115200 clk_ignore_unused root=/dev/ram0 rw init_fatal_sh=1";
> +               stdout-path = "serial0:115200n8";
> +       };
> +
> +From the chosen node, stdout-path property is obtained as string.
> +
> +       stdout-path = "serial0:115200n8";
> +
> +The string is parsed to get the baudrate 115200. This string is converted to integer and updated to the environment.
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index ec574c4460..04bdb30b24 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -870,6 +870,26 @@ ofnode ofnode_get_chosen_node(const char *name)
>         return ofnode_path(prop);
>  }
>
> +#ifdef CONFIG_OF_SERIAL_DT_BAUD

You should not need this #ifdef

> +int ofnode_read_baud(void)
> +{
> +       const char *str, *p;
> +       u32 baud;
> +
> +       str = ofnode_read_chosen_string("stdout-path");
> +       if (!str)
> +               return -EINVAL;
> +
> +       /* Parse string serial0:115200n8 */
> +       p = strchr(str, ':');
> +       if (!p)
> +               return -EINVAL;
> +
> +       baud = dectoul(p + 1, NULL);
> +       return baud;
> +}
> +#endif
> +
>  const void *ofnode_read_aliases_prop(const char *propname, int *sizep)
>  {
>         ofnode node;
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 5c9b924e73..ea2244e5db 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -24,6 +24,15 @@ config BAUDRATE
>           in the SPL stage (most drivers) or for choosing a default baudrate
>           in the absence of an environment setting (serial_mxc.c).
>
> +config OF_SERIAL_DT_BAUD

Please can we use OF_SERIAL_BAUD ?

OF sort-of means DT so we don't need both

> +       bool "Fetch serial baudrate from device tree"
> +       depends on DM_SERIAL && SPL_ENV_SUPPORT
> +       select DEFAULT_ENV_IS_RW
> +       help
> +         Select this to enable fetching and setting of the baudrate
> +         configured in the DT. Replace the default baudrate with the DT
> +         baudrate and also set it to the environment.
> +
>  config DEFAULT_ENV_IS_RW
>         bool "Make default environment as writable"
>         help
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 067fae2614..afa25a1f19 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -154,12 +154,54 @@ static void serial_find_console_or_panic(void)
>  }
>  #endif /* CONFIG_SERIAL_PRESENT */
>
> +int check_valid_baudrate(int baud)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {
> +               if (baud == baudrate_table[i])
> +                       return 0;
> +       }
> +
> +       return -EINVAL;
> +}
> +
> +int fetch_baud_from_dtb(void)
> +{
> +       int baud_value, ret;
> +
> +       baud_value = ofnode_read_baud();
> +       ret = check_valid_baudrate(baud_value);
> +       if (ret)
> +               return ret;
> +
> +       return baud_value;
> +}
> +
>  /* Called prior to relocation */
>  int serial_init(void)
>  {
>  #if CONFIG_IS_ENABLED(SERIAL_PRESENT)
>         serial_find_console_or_panic();
>         gd->flags |= GD_FLG_SERIAL_READY;
> +#ifdef CONFIG_OF_SERIAL_DT_BAUD

Can this use

if (IS_ENABLED(...)) ...

?

> +       int ret = 0;
> +       char *ptr = &default_environment[0];
> +
> +       /*
> +        * Fetch the baudrate from the dtb and update the value in the
> +        * default environment.
> +        */
> +       ret = fetch_baud_from_dtb();
> +       if (ret != -EINVAL && ret != -EFAULT) {
> +               gd->baudrate = ret;
> +
> +               while (*ptr != '\0' && *(ptr + 1) != '\0')
> +                       ptr++;
> +               ptr += 2;
> +               sprintf(ptr, "baudrate=%d", gd->baudrate);
> +       }
> +#endif
>         serial_setbrg();
>  #endif
>
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 443db6252d..5f90b6c65e 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -932,12 +932,22 @@ const char *ofnode_read_chosen_string(const char *propname);
>  ofnode ofnode_get_chosen_node(const char *propname);
>
>  /**
> - * ofnode_read_aliases_prop() - get the value of a aliases property
> + * ofnode_read_baud() - get the baudrate from string value of chosen property
>   *
> - * This looks for a property within the /aliases node and returns its value
> + * This looks for stdout-path property within the /chosen node and parses its
> + * value to return baudrate.
>   *
>   * This only works with the control FDT.
>   *
> + * Return: baudrate value if found, else error

-ve error code?

> + */
> +int ofnode_read_baud(void);
> +
> +/**
> + * ofnode_read_aliases_prop() - get the value of a aliases property
> + *
> + * This looks for a property within the /aliases node and returns its value
> + *
>   * @propname: Property name to look for
>   * @sizep: Returns size of property, or `FDT_ERR_...` error code if function
>   *     returns NULL
> diff --git a/include/env_default.h b/include/env_default.h
> index 227cad7c34..1c859a8f65 100644
> --- a/include/env_default.h
> +++ b/include/env_default.h
> @@ -42,7 +42,7 @@ const char default_environment[] = {
>  #if defined(CONFIG_BOOTDELAY)
>         "bootdelay="    __stringify(CONFIG_BOOTDELAY)   "\0"
>  #endif
> -#if defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
> +#if !defined(CONFIG_OF_SERIAL_DT_BAUD) && defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
>         "baudrate="     __stringify(CONFIG_BAUDRATE)    "\0"
>  #endif
>  #ifdef CONFIG_LOADS_ECHO
> @@ -118,6 +118,10 @@ const char default_environment[] = {
>  #endif
>  #ifdef CFG_EXTRA_ENV_SETTINGS
>         CFG_EXTRA_ENV_SETTINGS
> +#endif
> +#ifdef CONFIG_OF_SERIAL_DT_BAUD
> +       /* Padding for baudrate at the end when environment is writable */
> +       "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
>  #endif
>         "\0"
>  #else /* CONFIG_USE_DEFAULT_ENV_FILE */
> diff --git a/include/serial.h b/include/serial.h
> index 42bdf3759c..b01047d07a 100644
> --- a/include/serial.h
> +++ b/include/serial.h
> @@ -337,6 +337,21 @@ int serial_setconfig(struct udevice *dev, uint config);
>   */
>  int serial_getinfo(struct udevice *dev, struct serial_device_info *info);
>
> +/**
> + * check_valid_baudrate() - Check whether baudrate is valid or not
> + *
> + * @baud: baudrate

baud rate to check

> + * Return: 0 if OK, -ve on error
> + */
> +int check_valid_baudrate(int baud);
> +
> +/**
> + * fetch_baud_from_dtb() - Fetch the baudrate value from DT
> + *
> + * Return: baudrate if OK, -ve on error
> + */
> +int fetch_baud_from_dtb(void);
> +
>  void atmel_serial_initialize(void);
>  void mcf_serial_initialize(void);
>  void mpc85xx_serial_initialize(void);
> --
> 2.17.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update
  2023-06-12 21:17   ` Simon Glass
@ 2023-08-16  9:30     ` Abbarapu, Venkatesh
  0 siblings, 0 replies; 10+ messages in thread
From: Abbarapu, Venkatesh @ 2023-08-16  9:30 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, Simek, Michal, git, Algapally, Santosh Sagar

Hi Simon,

> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: Tuesday, June 13, 2023 2:48 AM
> To: Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>
> Cc: u-boot@lists.denx.de; Simek, Michal <michal.simek@amd.com>;
> git@xilinx.com; Algapally, Santosh Sagar <SantoshSagar.Algapally@amd.com>
> Subject: Re: [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and
> update
> 
> Hi Venkatesh,
> 
> On Thu, 25 May 2023 at 05:03, Venkatesh Yadav Abbarapu
> <venkatesh.abbarapu@amd.com> wrote:
> >
> > From: Algapally Santosh Sagar <santoshsagar.algapally@amd.com>
> >
> > The baudrate configured in .config is taken by default by serial. If
> > change of baudrate is required then the .config needs to changed and
> > u-boot recompilation is required or the u-boot environment needs to be
> > updated.
> >
> > To avoid this, support is added to fetch the baudrate directly from
> > the device tree file and update.
> > The serial, prints the log with the configured baudrate in the dtb.
> > The commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value
> > for $fdtfile env variable") is taken as reference for changing the
> > default environment variable.
> >
> > The default environment stores the default baudrate value, When
> > default baudrate and dtb baudrate are not same glitches are seen on the
> serial.
> > So, the environment also needs to be updated with the dtb baudrate to
> > avoid the glitches on the serial.
> >
> > Signed-off-by: Algapally Santosh Sagar
> > <santoshsagar.algapally@amd.com>
> > Signed-off-by: Venkatesh Yadav Abbarapu
> <venkatesh.abbarapu@amd.com>
> > ---
> >  doc/README.serial_dt_baud      | 41
> +++++++++++++++++++++++++++++++++
> >  drivers/core/ofnode.c          | 20 ++++++++++++++++
> >  drivers/serial/Kconfig         |  9 ++++++++
> >  drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++
> >  include/dm/ofnode.h            | 14 ++++++++++--
> >  include/env_default.h          |  6 ++++-
> >  include/serial.h               | 15 ++++++++++++
> >  7 files changed, 144 insertions(+), 3 deletions(-)  create mode
> > 100644 doc/README.serial_dt_baud
> >
> > diff --git a/doc/README.serial_dt_baud b/doc/README.serial_dt_baud
> new
> > file mode 100644 index 0000000000..02974ab1a7
> > --- /dev/null
> > +++ b/doc/README.serial_dt_baud
> > @@ -0,0 +1,41 @@
> > +Fetch serial baudrate from DT
> > +-----------------------------
> > +
> > +To support fetching of baudrate from DT, the following is done:-
> > +
> > +The baudrate configured in Kconfig symbol CONFIG_BAUDRATE is taken by
> default by serial.
> > +If change of baudrate is required then the Kconfig symbol
> > +CONFIG_BAUDRATE needs to changed and U-Boot recompilation is
> required or the U-Boot environment needs to be updated.
> > +
> > +To avoid this, add support to fetch the baudrate directly from the
> > +device tree file and update the environment.
> > +
> > +The default environment stores the default baudrate value. When
> > +default baudrate and dtb baudrate are not same glitches are seen on the
> serial.
> > +So, the environment also needs to be updated with the dtb baudrate to
> > +avoid the glitches on the serial which is enabled by SERIAL_DT_BAUD.
> 
> The baud rate is read from the env into gd->baudrate by init_baud_rate().
> 
> Updating the default environment seems pretty nasty to me...why is that
> necessary?
> 
> Could we read it from DT in init_baud_rate() ?

We are seeing glitches if the default env baudrate and dtb baudrate are not same. So, we are updating the default baudrate with dtb baudrate, same thing has been explained above.

> 
> > +
> > +The Kconfig SPL_ENV_SUPPORT needs to be enabled to allow patching in
> SPL.
> > +
> > +The Kconfig DEFAULT_ENV_IS_RW which is enabled by SERIAL_DT_BAUD
> with
> > +making the environment writable.
> > +
> > +The ofnode_read_baud() function parses and fetches the baudrate value
> > +from the DT. This value is validated and updated to baudrate during
> > +serial init. Padding is added at the end of the default environment and the
> dt baudrate is updated with the latest value.
> > +
> > +Example:-
> > +
> > +The serial port options are of the form "bbbbpnf", where "bbbb" is
> > +the baud rate, "p" is parity ("n", "o", or "e"), "n" is number of bits, and "f"
> is flow control ("r" for RTS or omit it). Default is "115200n8".
> > +
> > +chosen {
> > +               bootargs = "earlycon console=ttyPS0,115200 clk_ignore_unused
> root=/dev/ram0 rw init_fatal_sh=1";
> > +               stdout-path = "serial0:115200n8";
> > +       };
> > +
> > +From the chosen node, stdout-path property is obtained as string.
> > +
> > +       stdout-path = "serial0:115200n8";
> > +
> > +The string is parsed to get the baudrate 115200. This string is converted to
> integer and updated to the environment.
> > diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index
> > ec574c4460..04bdb30b24 100644
> > --- a/drivers/core/ofnode.c
> > +++ b/drivers/core/ofnode.c
> > @@ -870,6 +870,26 @@ ofnode ofnode_get_chosen_node(const char
> *name)
> >         return ofnode_path(prop);
> >  }
> >
> > +#ifdef CONFIG_OF_SERIAL_DT_BAUD
> 
> You should not need this #ifdef

Ok...I will remove this and update.


> 
> > +int ofnode_read_baud(void)
> > +{
> > +       const char *str, *p;
> > +       u32 baud;
> > +
> > +       str = ofnode_read_chosen_string("stdout-path");
> > +       if (!str)
> > +               return -EINVAL;
> > +
> > +       /* Parse string serial0:115200n8 */
> > +       p = strchr(str, ':');
> > +       if (!p)
> > +               return -EINVAL;
> > +
> > +       baud = dectoul(p + 1, NULL);
> > +       return baud;
> > +}
> > +#endif
> > +
> >  const void *ofnode_read_aliases_prop(const char *propname, int
> > *sizep)  {
> >         ofnode node;
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
> > 5c9b924e73..ea2244e5db 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -24,6 +24,15 @@ config BAUDRATE
> >           in the SPL stage (most drivers) or for choosing a default baudrate
> >           in the absence of an environment setting (serial_mxc.c).
> >
> > +config OF_SERIAL_DT_BAUD
> 
> Please can we use OF_SERIAL_BAUD ?
> 
> OF sort-of means DT so we don't need both

Ok...sure will change to OF_SERIAL_BAUD.

> 
> > +       bool "Fetch serial baudrate from device tree"
> > +       depends on DM_SERIAL && SPL_ENV_SUPPORT
> > +       select DEFAULT_ENV_IS_RW
> > +       help
> > +         Select this to enable fetching and setting of the baudrate
> > +         configured in the DT. Replace the default baudrate with the DT
> > +         baudrate and also set it to the environment.
> > +
> >  config DEFAULT_ENV_IS_RW
> >         bool "Make default environment as writable"
> >         help
> > diff --git a/drivers/serial/serial-uclass.c
> > b/drivers/serial/serial-uclass.c index 067fae2614..afa25a1f19 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -154,12 +154,54 @@ static void serial_find_console_or_panic(void)
> >  }
> >  #endif /* CONFIG_SERIAL_PRESENT */
> >
> > +int check_valid_baudrate(int baud)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {
> > +               if (baud == baudrate_table[i])
> > +                       return 0;
> > +       }
> > +
> > +       return -EINVAL;
> > +}
> > +
> > +int fetch_baud_from_dtb(void)
> > +{
> > +       int baud_value, ret;
> > +
> > +       baud_value = ofnode_read_baud();
> > +       ret = check_valid_baudrate(baud_value);
> > +       if (ret)
> > +               return ret;
> > +
> > +       return baud_value;
> > +}
> > +
> >  /* Called prior to relocation */
> >  int serial_init(void)
> >  {
> >  #if CONFIG_IS_ENABLED(SERIAL_PRESENT)
> >         serial_find_console_or_panic();
> >         gd->flags |= GD_FLG_SERIAL_READY;
> > +#ifdef CONFIG_OF_SERIAL_DT_BAUD
> 
> Can this use
> 
> if (IS_ENABLED(...)) ...
> 
> ?

Ok...will update to IS_ENABLED().

> 
> > +       int ret = 0;
> > +       char *ptr = &default_environment[0];
> > +
> > +       /*
> > +        * Fetch the baudrate from the dtb and update the value in the
> > +        * default environment.
> > +        */
> > +       ret = fetch_baud_from_dtb();
> > +       if (ret != -EINVAL && ret != -EFAULT) {
> > +               gd->baudrate = ret;
> > +
> > +               while (*ptr != '\0' && *(ptr + 1) != '\0')
> > +                       ptr++;
> > +               ptr += 2;
> > +               sprintf(ptr, "baudrate=%d", gd->baudrate);
> > +       }
> > +#endif
> >         serial_setbrg();
> >  #endif
> >
> > diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index
> > 443db6252d..5f90b6c65e 100644
> > --- a/include/dm/ofnode.h
> > +++ b/include/dm/ofnode.h
> > @@ -932,12 +932,22 @@ const char *ofnode_read_chosen_string(const
> char
> > *propname);  ofnode ofnode_get_chosen_node(const char *propname);
> >
> >  /**
> > - * ofnode_read_aliases_prop() - get the value of a aliases property
> > + * ofnode_read_baud() - get the baudrate from string value of chosen
> > + property
> >   *
> > - * This looks for a property within the /aliases node and returns its
> > value
> > + * This looks for stdout-path property within the /chosen node and
> > + parses its
> > + * value to return baudrate.
> >   *
> >   * This only works with the control FDT.
> >   *
> > + * Return: baudrate value if found, else error
> 
> -ve error code?

Okk...Will update this.


> 
> > + */
> > +int ofnode_read_baud(void);
> > +
> > +/**
> > + * ofnode_read_aliases_prop() - get the value of a aliases property
> > + *
> > + * This looks for a property within the /aliases node and returns its
> > +value
> > + *
> >   * @propname: Property name to look for
> >   * @sizep: Returns size of property, or `FDT_ERR_...` error code if function
> >   *     returns NULL
> > diff --git a/include/env_default.h b/include/env_default.h index
> > 227cad7c34..1c859a8f65 100644
> > --- a/include/env_default.h
> > +++ b/include/env_default.h
> > @@ -42,7 +42,7 @@ const char default_environment[] = {  #if
> > defined(CONFIG_BOOTDELAY)
> >         "bootdelay="    __stringify(CONFIG_BOOTDELAY)   "\0"
> >  #endif
> > -#if defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
> > +#if !defined(CONFIG_OF_SERIAL_DT_BAUD) &&
> defined(CONFIG_BAUDRATE) &&
> > +(CONFIG_BAUDRATE >= 0)
> >         "baudrate="     __stringify(CONFIG_BAUDRATE)    "\0"
> >  #endif
> >  #ifdef CONFIG_LOADS_ECHO
> > @@ -118,6 +118,10 @@ const char default_environment[] = {  #endif
> > #ifdef CFG_EXTRA_ENV_SETTINGS
> >         CFG_EXTRA_ENV_SETTINGS
> > +#endif
> > +#ifdef CONFIG_OF_SERIAL_DT_BAUD
> > +       /* Padding for baudrate at the end when environment is writable */
> > +       "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
> >  #endif
> >         "\0"
> >  #else /* CONFIG_USE_DEFAULT_ENV_FILE */ diff --git a/include/serial.h
> > b/include/serial.h index 42bdf3759c..b01047d07a 100644
> > --- a/include/serial.h
> > +++ b/include/serial.h
> > @@ -337,6 +337,21 @@ int serial_setconfig(struct udevice *dev, uint
> config);
> >   */
> >  int serial_getinfo(struct udevice *dev, struct serial_device_info
> > *info);
> >
> > +/**
> > + * check_valid_baudrate() - Check whether baudrate is valid or not
> > + *
> > + * @baud: baudrate
> 
> baud rate to check

Ok...will update this.


> 
> > + * Return: 0 if OK, -ve on error
> > + */
> > +int check_valid_baudrate(int baud);
> > +
> > +/**
> > + * fetch_baud_from_dtb() - Fetch the baudrate value from DT
> > + *
> > + * Return: baudrate if OK, -ve on error  */ int
> > +fetch_baud_from_dtb(void);
> > +
> >  void atmel_serial_initialize(void);
> >  void mcf_serial_initialize(void);
> >  void mpc85xx_serial_initialize(void);
> > --
> > 2.17.1
> >
> 
> Regards,
> Simon

Thanks
Venkatesh

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-08-16 14:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-25  4:02 [PATCH v5 0/2] Add support to fetch baudrate from dtb Venkatesh Yadav Abbarapu
2023-05-25  4:02 ` [PATCH v5 1/2] configs: Add support in Kconfig and convert for armada boards Venkatesh Yadav Abbarapu
2023-06-12 21:17   ` Simon Glass
2023-05-25  4:02 ` [PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update Venkatesh Yadav Abbarapu
2023-06-07 19:13   ` Tom Rini
2023-06-08  8:50     ` Michal Simek
2023-06-08 16:35       ` Tom Rini
2023-06-12 21:17   ` Simon Glass
2023-08-16  9:30     ` Abbarapu, Venkatesh
2023-06-05 13:41 ` [PATCH v5 0/2] Add support to fetch baudrate from dtb Michal Simek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.