[U-Boot] [PATCH 03/11] serial: atmel_usart: Use fixed clock value in SPL version

[Stefan Roese]

On 25.03.19 15:27, Eugen.Hristev at microchip.com wrote:
> 
> 
> On 20.03.2019 09:30, Stefan Roese wrote:
>>
>> On 20.03.19 08:25, Eugen.Hristev at microchip.com wrote:
>>>
>>>
>>> On 19.03.2019 17:56, Stefan Roese wrote:
>>>> External E-Mail
>>>>
>>>>
>>>> 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.
>>>>
>>>> If some boards need a different clock than the one provided with this
>>>> patch, then support for this needs to be added later.
>>>>
>>>> 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>
>>>> ---
>>>>     drivers/serial/atmel_usart.c | 12 ++++++++++++
>>>>     1 file changed, 12 insertions(+)
>>>>
>>>> diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
>>>> index aa8cdff840..049172baef 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,
>>>>     };
>>>> +#ifdef CONFIG_SPL_BUILD
>>>> +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 = 132096000;
>>>
>>> Hi Stefan,
>>>
>>> Does this mean that the SPL will work *if and only if* this clock
>>> matches the board's clock, for all at91 boards? (thus, boards having a
>>> different clock , the serial will not function correctly anymore ?)
>>
>> Yes, thats the case (as explained in the commit message above). For
>> this to work on all boards we need to add some macro like
>> CONFIG_DEBUG_UART_CLOCK which can be set by each board port
>> individually.
>>
>> If agreed on such an idea, I can add this new Kconfig option in a
>> later version of this patch. Perhaps something like
>> CONFIG_SPL_UART_CLOCK?
> 
> If this breaks the serial for the other boards... it's not good.

Currently it breaks no other board port, as no other board port uses
SPL with DM_SERIAL support on AT91, AFAICT. So no breakage is introduced
with this patch (please correct me if I'm wrong).

> I think either we do a patch that fixes a thing, but does not break
> other functionality... or we leave it as is.
> Would hate to see serial failing on all other SPLs

Again, this is only for ports that will move to DM_SERIAL in SPL in
the future.
  
> How big is the SPL reduction with this ?

Frankly I can't tell.

> Maybe just keep old code in case CONFIG_SPL_UART_CLOCK is *not* defined ?
> and if defined, use the hardcoded value from config ?

Which old code are you referring to? Again, this code is new for the
newly introduced SPL & DM_SERIAL support in this driver.

Thanks,
Stefan