Hi Venkatesh,
On Thu, 25 May 2023 at 05:03, Venkatesh Yadav Abbarapu
<venkatesh.abbar...@amd.com> wrote:
>
> From: Algapally Santosh Sagar <santoshsagar.algapa...@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.algapa...@amd.com>
> Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbar...@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
