[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 13:40, Stefan Roese wrote:
> External E-Mail
> 
> 
> Hi Eugen,
> 
> On 02.04.19 12:12, Eugen.Hristev at microchip.com wrote:
>>
>>
>> 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
> 
> Okay, I agree.
> 
>> 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)
> 
> How about this version?
> 
> config SPL_UART_CLOCK
>      int "SPL fixed UART input clock"
>      depends on SPL && !SPL_CLK
>      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.

Looks much better. Typo on restrictirions

Can also add some default then, to not break build in case someone 
disables SPL_CLK but does not set SPL_UART_CLOCK ?

Thanks !

> 
> Thanks,
> Stefan