On Tue, Mar 14, 2023 at 02:16:27PM +0530, Venkatesh Yadav Abbarapu 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>
> ---
> 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
signature.asc
Description: PGP signature
