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

Abbarapu, Venkatesh venkatesh.abbarapu at amd.com
Wed Aug 16 11:30:15 CEST 2023


Hi Simon,

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Tuesday, June 13, 2023 2:48 AM
> To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>
> Cc: u-boot at lists.denx.de; Simek, Michal <michal.simek at amd.com>;
> git at xilinx.com; Algapally, Santosh Sagar <SantoshSagar.Algapally at 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 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() ?

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


More information about the U-Boot mailing list