[U-Boot] [PATCH 03/13 v3] serial: atmel_usart: Use fixed clock value in SPL version with DM_SERIAL

[Eugen.Hristev at microchip.com]

On 02.04.2019 11:57, Stefan Roese wrote:

> This patch adds an alterative SPL version of atmel_serial_enable_clk().
> This enables the usage of this driver without full clock support (in
> drivers and DT nodes). This saves some space in the SPL image.
> 
> Please note that this fixed clock support is only added to the SPL code
> in the DM_SERIAL part of this file. All boards not using SPL & DM_SERIAL
> should not be affected.
> 
> This patch also introduces CONFIG_SPL_UART_CLOCK for the fixed UART
> input clock. It defaults to 132096000 for ARCH_AT91 but can be set to
> a different value if needed.
> 
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Heiko Schocher <hs at denx.de>
> Cc: Andreas Bießmann <andreas at biessmann.org>
> Cc: Eugen Hristev <eugen.hristev at microchip.com>
> Reviewed-by: Heiko Schocher <hs at denx.de>
> Tested on the taurus board:
> Tested-by: Heiko Schocher <hs at denx.de>
> ---
> v3:
> - Depend fixed clock atmel_serial_enable_clk() function also on
>    !CONFIG_SPL_CLK so that board with full clocj support in SPL
>    can still use the normal function here
> - Introcude CONFIG_SPL_UART_CLOCK and use this Kconfig option instead
>    of the hardcoded value
> 
> v2:
> - Reword patch subject and commit text to make it more clear, that
>    this change only affects ports with SPL and DM_SERIAL enabled
>    
>   drivers/serial/Kconfig       |  9 +++++++++
>   drivers/serial/atmel_usart.c | 12 ++++++++++++
>   2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 887cd687c0..d6bf8309d3 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -508,6 +508,15 @@ config ATMEL_USART
>   	  configured in the device tree, and input clock frequency can
>   	  be got from the clk node.
>   
> +config SPL_UART_CLOCK
> +	int "SPL fixed UART input clock"
> +	depends on SPL
> +	default 132096000 if ARCH_AT91

Hi Stefan,

This doesn't look good. This has to be unset for the platforms that do 
not specifically set it.

For example, when I build the sama5d2_xplained_mmc_defconfig, I look in 
.config and :

CONFIG_SPL_UART_CLOCK=132096000

This may be unused, but it's confusing

Can you make this default unset and then specifically set it in the 
boards that need it ? ( the specific defconfig )

And if you have a mutual exclusion with CONFIG_SPL_CLK can we make it 
somehow ? (in a menu or choice..., either old behavior or fixed uart clock)

Does this make sense?

Thanks

> +	help
> +	  Provide a fixed clock value as input to the UART controller. This
> +	  might be needed on platforms which can't enable CONFIG_SPL_CLK
> +	  because of SPL image size restirctions.
> +
>   config BCM283X_MU_SERIAL
>   	bool "Support for BCM283x Mini-UART"
>   	depends on DM_SERIAL && ARCH_BCM283X
> diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
> index aa8cdff840..c450a4e08a 100644
> --- a/drivers/serial/atmel_usart.c
> +++ b/drivers/serial/atmel_usart.c
> @@ -218,6 +218,17 @@ static const struct dm_serial_ops atmel_serial_ops = {
>   	.setbrg = atmel_serial_setbrg,
>   };
>   
> +#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_CLK)
> +static int atmel_serial_enable_clk(struct udevice *dev)
> +{
> +	struct atmel_serial_priv *priv = dev_get_priv(dev);
> +
> +	/* Use fixed clock value in SPL */
> +	priv->usart_clk_rate = CONFIG_SPL_UART_CLOCK;
> +
> +	return 0;
> +}
> +#else
>   static int atmel_serial_enable_clk(struct udevice *dev)
>   {
>   	struct atmel_serial_priv *priv = dev_get_priv(dev);
> @@ -245,6 +256,7 @@ static int atmel_serial_enable_clk(struct udevice *dev)
>   
>   	return 0;
>   }
> +#endif
>   
>   static int atmel_serial_probe(struct udevice *dev)
>   {
>