[PATCH v5 2/2] serial: zynqmp: Fetch baudrate from dtb and update

Simon Glass sjg at chromium.org
Mon Jun 12 23:17:33 CEST 2023


Hi Venkatesh,

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


More information about the U-Boot mailing list