[PATCH v3 0/2] Add support to fetch baudrate from dtb
From: Algapally Santosh Sagar santoshsagar.algapally@amd.com
In this patch series - Fetch baudrate from the dtb and update - Add support in Kconfig and convert for armada boards
Changes in v3: - 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. - Add SERIAL_DT_BAUD to Kconfig - Moved DEFAULT_ENV_IS_RW to Kconfig also updated armada files
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): serial: zynqmp: Fetch baudrate from dtb and update configs: Add support in Kconfig and convert for armada boards
configs/mvebu_db-88f3720_defconfig | 1 + configs/mvebu_espressobin-88f3720_defconfig | 1 + configs/mvebu_mcbin-88f8040_defconfig | 1 + drivers/serial/Kconfig | 16 +++++++++ drivers/serial/serial-uclass.c | 32 +++++++++++++++++ include/configs/mvebu_armada-37xx.h | 1 - include/env_default.h | 7 ++-- include/env_internal.h | 2 +- include/fdtdec.h | 8 +++++ include/serial.h | 1 + lib/fdtdec.c | 40 +++++++++++++++++++++ 11 files changed, 106 insertions(+), 4 deletions(-)
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 --- drivers/serial/serial-uclass.c | 32 +++++++++++++++++++++++++++ include/env_default.h | 7 ++++-- include/env_internal.h | 2 +- include/fdtdec.h | 8 +++++++ include/serial.h | 1 + lib/fdtdec.c | 40 ++++++++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 067fae2614..d77d3bda36 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -154,12 +154,44 @@ static void serial_find_console_or_panic(void) } #endif /* CONFIG_SERIAL_PRESENT */
+#ifdef CONFIG_SERIAL_DT_BAUD +int serial_get_valid_baudrate(int baud) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) { + if (baud == baudrate_table[i]) + return 0; + } + + return -EINVAL; +} +#endif + /* 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_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 = fdtdec_get_baud_from_dtb(gd->fdt_blob); + 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/env_default.h b/include/env_default.h index c0df39d62f..4f286ffc9e 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -23,7 +23,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[] = { @@ -44,7 +44,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_SERIAL_DT_BAUD) && defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0) "baudrate=" __stringify(CONFIG_BAUDRATE) "\0" #endif #ifdef CONFIG_LOADS_ECHO @@ -120,6 +120,9 @@ const char default_environment[] = { #endif #ifdef CFG_EXTRA_ENV_SETTINGS CFG_EXTRA_ENV_SETTINGS +#endif +#ifdef CONFIG_SERIAL_DT_BAUD + "\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/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[]; diff --git a/include/fdtdec.h b/include/fdtdec.h index aa61a0fca1..48937a7a46 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -657,6 +657,14 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int node, */ int fdtdec_get_alias_highest_id(const void *blob, const char *base);
+/** + * Get baudrate from the dtb + * + * @param blob Device tree blob (if NULL, then error is returned) + * @return Baud rate value, or -ve error . + */ +int fdtdec_get_baud_from_dtb(const void *blob); + /** * Get a property from the /chosen node * diff --git a/include/serial.h b/include/serial.h index 42bdf3759c..48834b517c 100644 --- a/include/serial.h +++ b/include/serial.h @@ -337,6 +337,7 @@ int serial_setconfig(struct udevice *dev, uint config); */ int serial_getinfo(struct udevice *dev, struct serial_device_info *info);
+int serial_get_valid_baudrate(int baud); void atmel_serial_initialize(void); void mcf_serial_initialize(void); void mpc85xx_serial_initialize(void); diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 0827e16859..2fcc12148e 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -570,6 +570,46 @@ int fdtdec_get_alias_highest_id(const void *blob, const char *base) return max; }
+#ifdef CONFIG_SERIAL_DT_BAUD +int fdtdec_get_baud_from_dtb(const void *blob) +{ + const char *str, *p, *baud_start; + u32 baud; + + if (!blob) + return -EFAULT; + + str = fdtdec_get_chosen_prop(blob, "stdout-path"); + if (!str) + return -EINVAL; + + p = strchr(str, ':'); + if (!p) + return -EINVAL; + + baud = 0; + baud_start = p + 1; + while (*baud_start != '\0') { + /* + * Uart binding is <baud>{<parity>{<bits>{...}}} + * So the baudrate either is the whole string, or + * ends in the parity characters. + */ + if (*baud_start == 'n' || *baud_start == 'o' || + *baud_start == 'e') + break; + + baud = baud * 10 + (*baud_start - '0'); + baud_start++; + } + + if (serial_get_valid_baudrate(baud) == -EINVAL) + return -EINVAL; + + return baud; +} +#endif + const char *fdtdec_get_chosen_prop(const void *blob, const char *name) { int chosen_node;
Hi Venkatesh,
On Tue, 25 Apr 2023 at 06:05, 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
drivers/serial/serial-uclass.c | 32 +++++++++++++++++++++++++++ include/env_default.h | 7 ++++-- include/env_internal.h | 2 +- include/fdtdec.h | 8 +++++++ include/serial.h | 1 + lib/fdtdec.c | 40 ++++++++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 3 deletions(-)
Can you please add something to doc/ for this feature?
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 067fae2614..d77d3bda36 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -154,12 +154,44 @@ static void serial_find_console_or_panic(void) } #endif /* CONFIG_SERIAL_PRESENT */
+#ifdef CONFIG_SERIAL_DT_BAUD
Please drop this #ifdef
+int serial_get_valid_baudrate(int baud) +{
int i;for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {if (baud == baudrate_table[i])return 0;}return -EINVAL;+} +#endif
/* 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_SERIAL_DT_BAUD
if (IS_ENABLED(CONFIG_SERIAL_DT_BAUD)
You also should add a Kconfig option. I suggest naming it OF_SERIAL_BOARD since we typically put an OF_ prefix on such options.
int ret = 0;char *ptr = &default_environment[0];/** Fetch the baudrate from the dtb and update the value in the* default environment.*/ret = fdtdec_get_baud_from_dtb(gd->fdt_blob);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/env_default.h b/include/env_default.h index c0df39d62f..4f286ffc9e 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -23,7 +23,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[] = { @@ -44,7 +44,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_SERIAL_DT_BAUD) && defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0) "baudrate=" __stringify(CONFIG_BAUDRATE) "\0" #endif #ifdef CONFIG_LOADS_ECHO @@ -120,6 +120,9 @@ const char default_environment[] = { #endif #ifdef CFG_EXTRA_ENV_SETTINGS CFG_EXTRA_ENV_SETTINGS +#endif +#ifdef CONFIG_SERIAL_DT_BAUD
"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
What is this for?
#endif "\0" #else /* CONFIG_USE_DEFAULT_ENV_FILE */ 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
What is this change for?
extern char default_environment[]; #else extern const char default_environment[]; diff --git a/include/fdtdec.h b/include/fdtdec.h index aa61a0fca1..48937a7a46 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -657,6 +657,14 @@ int fdtdec_get_alias_seq(const void *blob, const char *base, int node, */ int fdtdec_get_alias_highest_id(const void *blob, const char *base);
Could you make this an ofnode_...() function instead? Then you can call ofnode_read_chosen_string() from your function.
We are trying to drop the fdtdec API.
+/**
- Get baudrate from the dtb
- @param blob Device tree blob (if NULL, then error is returned)
- @return Baud rate value, or -ve error .
Return: Baud ...
(we changed to that syntax to work with Sphinx)
But this should become ofnode_read_baud(void)
Exported functions should be documented in the header file, not the C file
- */
+int fdtdec_get_baud_from_dtb(const void *blob);
/**
- Get a property from the /chosen node
diff --git a/include/serial.h b/include/serial.h index 42bdf3759c..48834b517c 100644 --- a/include/serial.h +++ b/include/serial.h @@ -337,6 +337,7 @@ int serial_setconfig(struct udevice *dev, uint config); */ int serial_getinfo(struct udevice *dev, struct serial_device_info *info);
+int serial_get_valid_baudrate(int baud);
check_valid_baudrate() would be better since it is checking the provided rate, not returning a valid rate (as I understand it).
This needs function docs
void atmel_serial_initialize(void); void mcf_serial_initialize(void); void mpc85xx_serial_initialize(void); diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 0827e16859..2fcc12148e 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -570,6 +570,46 @@ int fdtdec_get_alias_highest_id(const void *blob, const char *base) return max; }
+#ifdef CONFIG_SERIAL_DT_BAUD +int fdtdec_get_baud_from_dtb(const void *blob) +{
const char *str, *p, *baud_start;u32 baud;if (!blob)return -EFAULT;str = fdtdec_get_chosen_prop(blob, "stdout-path");if (!str)return -EINVAL;p = strchr(str, ':');if (!p)return -EINVAL;baud = 0;baud_start = p + 1;
It would help to have a comment about the format you are parsing, e.g. an example string.
I thought it was something like: "serial0:115200n8" in which case I think you could do:
p = strchr(str, ':') if (p) { uint baud = dectoul(p + 1, NULL);
...
}
while (*baud_start != '\0') {/** Uart binding is <baud>{<parity>{<bits>{...}}}* So the baudrate either is the whole string, or* ends in the parity characters.*/if (*baud_start == 'n' || *baud_start == 'o' ||*baud_start == 'e')break;baud = baud * 10 + (*baud_start - '0');baud_start++;}if (serial_get_valid_baudrate(baud) == -EINVAL)return -EINVAL;
ret = func ... if (ret) return ret
(i.e. don't change the error number, and catch all errors that it might return)
return baud;+} +#endif
const char *fdtdec_get_chosen_prop(const void *blob, const char *name) { int chosen_node; -- 2.17.1
Regards, Simon
Hi Simon,
Thanks, A Santosh Sagar.
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Tuesday, April 25, 2023 11:31 PM 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 v3 1/2] serial: zynqmp: Fetch baudrate from dtb and update
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
Hi Venkatesh,
On Tue, 25 Apr 2023 at 06:05, 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
drivers/serial/serial-uclass.c | 32 +++++++++++++++++++++++++++ include/env_default.h | 7 ++++-- include/env_internal.h | 2 +- include/fdtdec.h | 8 +++++++ include/serial.h | 1 + lib/fdtdec.c | 40 ++++++++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 3 deletions(-)
Can you please add something to doc/ for this feature?
Ok sure
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 067fae2614..d77d3bda36 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -154,12 +154,44 @@ static void serial_find_console_or_panic(void) } #endif /* CONFIG_SERIAL_PRESENT */
+#ifdef CONFIG_SERIAL_DT_BAUD
Please drop this #ifdef
Will drop
+int serial_get_valid_baudrate(int baud) {
int i;for (i = 0; i < ARRAY_SIZE(baudrate_table); ++i) {if (baud == baudrate_table[i])return 0;}return -EINVAL;+} +#endif
/* 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_SERIAL_DT_BAUD
if (IS_ENABLED(CONFIG_SERIAL_DT_BAUD)
On using this if (IS_ENABLED(CONFIG_SERIAL_DT_BAUD)) The variables declared below are getting compiled and the below warning is thrown. So, not changing this.
drivers/serial/serial-uclass.c: In function 'serial_init': drivers/serial/serial-uclass.c:189:21: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] 189 | char *ptr = &default_environment[0];
You also should add a Kconfig option. I suggest naming it OF_SERIAL_BOARD since we typically put an OF_ prefix on such options.
Will change SERIAL_DT_BAUD to 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 = fdtdec_get_baud_from_dtb(gd->fdt_blob);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/env_default.h b/include/env_default.h index c0df39d62f..4f286ffc9e 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -23,7 +23,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[] = { @@ -44,7 +44,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_SERIAL_DT_BAUD) && defined(CONFIG_BAUDRATE) && +(CONFIG_BAUDRATE >= 0) "baudrate=" __stringify(CONFIG_BAUDRATE) "\0" #endif #ifdef CONFIG_LOADS_ECHO @@ -120,6 +120,9 @@ const char default_environment[] = { #endif #ifdef CFG_EXTRA_ENV_SETTINGS CFG_EXTRA_ENV_SETTINGS +#endif +#ifdef CONFIG_SERIAL_DT_BAUD
"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"What is this for?
The padding is needed at the end of default environment for baudrate when environment is writable.
#endif "\0" #else /* CONFIG_USE_DEFAULT_ENV_FILE */ 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
What is this change for?
This is done as DEFAULT_ENV_IS_RW is moved to Kconfig.
extern char default_environment[]; #else extern const char default_environment[]; diff --git a/include/fdtdec.h b/include/fdtdec.h index aa61a0fca1..48937a7a46 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -657,6 +657,14 @@ int fdtdec_get_alias_seq(const void *blob, const char
*base, int node,
*/ int fdtdec_get_alias_highest_id(const void *blob, const char *base);
Could you make this an ofnode_...() function instead? Then you can call ofnode_read_chosen_string() from your function.
We are trying to drop the fdtdec API.
Will change
+/**
- Get baudrate from the dtb
- @param blob Device tree blob (if NULL, then error is returned)
- @return Baud rate value, or -ve error .
Return: Baud ...
(we changed to that syntax to work with Sphinx)
But this should become ofnode_read_baud(void)
Exported functions should be documented in the header file, not the C file
Will do
- */
+int fdtdec_get_baud_from_dtb(const void *blob);
/**
- Get a property from the /chosen node
diff --git a/include/serial.h b/include/serial.h index 42bdf3759c..48834b517c 100644 --- a/include/serial.h +++ b/include/serial.h @@ -337,6 +337,7 @@ int serial_setconfig(struct udevice *dev, uint config); */ int serial_getinfo(struct udevice *dev, struct serial_device_info *info);
+int serial_get_valid_baudrate(int baud);
check_valid_baudrate() would be better since it is checking the provided rate, not returning a valid rate (as I understand it).
This needs function docs
Ok sure
void atmel_serial_initialize(void); void mcf_serial_initialize(void); void mpc85xx_serial_initialize(void); diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 0827e16859..2fcc12148e 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -570,6 +570,46 @@ int fdtdec_get_alias_highest_id(const void *blob,
const char *base)
return max;}
+#ifdef CONFIG_SERIAL_DT_BAUD +int fdtdec_get_baud_from_dtb(const void *blob) {
const char *str, *p, *baud_start;u32 baud;if (!blob)return -EFAULT;str = fdtdec_get_chosen_prop(blob, "stdout-path");if (!str)return -EINVAL;p = strchr(str, ':');if (!p)return -EINVAL;baud = 0;baud_start = p + 1;It would help to have a comment about the format you are parsing, e.g. an example string.
I thought it was something like: "serial0:115200n8" in which case I think you could do:
p = strchr(str, ':') if (p) { uint baud = dectoul(p + 1, NULL);
...
}
Will do
while (*baud_start != '\0') {/** Uart binding is <baud>{<parity>{<bits>{...}}}* So the baudrate either is the whole string, or* ends in the parity characters.*/if (*baud_start == 'n' || *baud_start == 'o' ||*baud_start == 'e')break;baud = baud * 10 + (*baud_start - '0');baud_start++;}if (serial_get_valid_baudrate(baud) == -EINVAL)return -EINVAL;ret = func ... if (ret) return ret
(i.e. don't change the error number, and catch all errors that it might return)
Ok sure
return baud;+} +#endif
const char *fdtdec_get_chosen_prop(const void *blob, const char *name) { int chosen_node; -- 2.17.1
Regards, Simon
Will do the changes and send v4
From: Algapally Santosh Sagar santoshsagar.algapally@amd.com
The SERIAL_DT_BAUD is added to Kconfig and 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/mvebu_db-88f3720_defconfig | 1 + configs/mvebu_espressobin-88f3720_defconfig | 1 + configs/mvebu_mcbin-88f8040_defconfig | 1 + drivers/serial/Kconfig | 16 ++++++++++++++++ include/configs/mvebu_armada-37xx.h | 1 - 5 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/configs/mvebu_db-88f3720_defconfig b/configs/mvebu_db-88f3720_defconfig index ed0d28fd7d..e3bbaa2173 100644 --- a/configs/mvebu_db-88f3720_defconfig +++ b/configs/mvebu_db-88f3720_defconfig @@ -21,6 +21,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_ARCH_EARLY_INIT_R=y CONFIG_BOARD_EARLY_INIT_F=y diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig index ce696787e8..a06eb2dd42 100644 --- a/configs/mvebu_espressobin-88f3720_defconfig +++ b/configs/mvebu_espressobin-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_ARCH_EARLY_INIT_R=y CONFIG_BOARD_EARLY_INIT_F=y diff --git a/configs/mvebu_mcbin-88f8040_defconfig b/configs/mvebu_mcbin-88f8040_defconfig index 058c04333a..4ee5f242f7 100644 --- a/configs/mvebu_mcbin-88f8040_defconfig +++ b/configs/mvebu_mcbin-88f8040_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_ARCH_EARLY_INIT_R=y CONFIG_BOARD_EARLY_INIT_F=y diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 10d07daf27..96cea87f45 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -24,6 +24,22 @@ 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 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" + depends on DM_SERIAL + 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)
On 4/25/23 14:04, Venkatesh Yadav Abbarapu wrote:
From: Algapally Santosh Sagar santoshsagar.algapally@amd.com
The SERIAL_DT_BAUD is added to Kconfig and 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/mvebu_db-88f3720_defconfig | 1 + configs/mvebu_espressobin-88f3720_defconfig | 1 + configs/mvebu_mcbin-88f8040_defconfig | 1 + drivers/serial/Kconfig | 16 ++++++++++++++++ include/configs/mvebu_armada-37xx.h | 1 - 5 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/configs/mvebu_db-88f3720_defconfig b/configs/mvebu_db-88f3720_defconfig index ed0d28fd7d..e3bbaa2173 100644 --- a/configs/mvebu_db-88f3720_defconfig +++ b/configs/mvebu_db-88f3720_defconfig @@ -21,6 +21,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_ARCH_EARLY_INIT_R=y CONFIG_BOARD_EARLY_INIT_F=y diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig index ce696787e8..a06eb2dd42 100644 --- a/configs/mvebu_espressobin-88f3720_defconfig +++ b/configs/mvebu_espressobin-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_ARCH_EARLY_INIT_R=y CONFIG_BOARD_EARLY_INIT_F=y diff --git a/configs/mvebu_mcbin-88f8040_defconfig b/configs/mvebu_mcbin-88f8040_defconfig index 058c04333a..4ee5f242f7 100644 --- a/configs/mvebu_mcbin-88f8040_defconfig +++ b/configs/mvebu_mcbin-88f8040_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_ARCH_EARLY_INIT_R=y CONFIG_BOARD_EARLY_INIT_F=y diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 10d07daf27..96cea87f45 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -24,6 +24,22 @@ 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 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 baudrateconfigured in the DT. Replace the default baudrate with the DTbaudrate and also set it to the environment.
This should be in 1/2.
And the whole patch itself should be the first one in this series.
M
participants (4)
-
Algapally, Santosh Sagar -
Michal Simek -
Simon Glass -
Venkatesh Yadav Abbarapu