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