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

[Tom Rini]

On Tue, Mar 14, 2023 at 02:16:27PM +0530, Venkatesh Yadav Abbarapu 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>
> ---
>  drivers/serial/Kconfig          |  8 +++++++
>  drivers/serial/serial-uclass.c  | 32 ++++++++++++++++++++++++++
>  include/configs/xilinx_zynqmp.h | 10 ++++++++-
>  include/fdtdec.h                |  8 +++++++
>  include/serial.h                |  1 +
>  lib/fdtdec.c                    | 40 +++++++++++++++++++++++++++++++++
>  6 files changed, 98 insertions(+), 1 deletion(-)

Conceptually? OK.

Implementation? So, this relies on DEFAULT_ENV_IS_RW but that's not in
Kconfig.  I think it's an easy enough option to move to Kconfig and
select when needed (first by armada-37xx platforms, then when
SERIAL_DT_BAUD is set).

Then:

> diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h
> index 011f0034c5..79d0a2a214 100644
> --- a/include/configs/xilinx_zynqmp.h
> +++ b/include/configs/xilinx_zynqmp.h
> @@ -15,6 +15,13 @@
>  #define GICC_BASE	0xF9020000
>  
>  /* Serial setup */
> +#if CONFIG_IS_ENABLED(SERIAL_DT_BAUD)

This would end up not patching in SPL, so just #ifdef.

> +#define DEFAULT_ENV_IS_RW
> +#define ENV_RW_FILLER "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"

Isn't this two too short?
baudrate=115200 | wc -c
16
And that doesn't include that you need a null at the end. And, since
this is a generic option, we need to go about modifying
include/env_default.h. Given that BAUDRATE is an option, I think we
should go about patching the baudrate entry, and make it be something
like:
#if defined(CONFIG_BAUDRATE) && (CONFIG_BAUDRATE >= 0)
	"baudrate="     __stringify(CONFIG_BAUDRATE)    "\0"
#ifdef CONFIG_SERIAL_DT_BAUD
... more padding, so that we can extend it up to 8000000 ...
#endif
#endif

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230321/588b2bb5/attachment.sig>