[U-Boot] [PATCHv1] mx53ppd: Change UART clock divider for high baudrate

[Stefano Babic]

Hi Rick,

On 18/02/2018 00:24, Rick Bronson wrote:
> Hi Fabio,
> 
>   First a disclaimer.  I no longer work for the client that had me
> make this change.  I also no longer have any iMX53 hardware to do any
> kind of tests.  But I'm happy to help out with this email.
> 

Thanks for your support !

>> Hi Rick and Sebastian,
>>
>> On Thu, Feb 15, 2018 at 2:15 PM, Sebastian Reichel
>> <sebastian.reichel at collabora.co.uk> wrote:
>>> From: Rick Bronson <rick at efn.org>
>>>
>>> Allow higher UART baud rates for communications to ACHC at 13.5 MBaud
>>
>> What does ACHC mean?
> 
>   ACHC is an product acronym, probably should change that sentence to:
> 
> Allow higher UART baud rates for communications up to 13.5 MBaud

ok, understood. The whole commit message is misleading, and it is not
clear what the patch is supposed to address.

> 
>>> Done in u-boot since it changes a divider common to all UARTs, including
>>> the console. If it were done in Linux we would get garbled characters
>>> during the handoff from u-boot to the kernel.
>>
>> It would be nice to explain a bit how you achieve the higher baud rates.
> 
>   Background: PLL2 runs at 400 MHz, PLL3 runs at 216 MHz.  One of
> these PLL's are fed to a UART divider, the output of which sets the
> maximum UART baud rates that are based on these rates divided by 16.

ok

> 
>   Before patch: PLL3 (400 MHz) for UART's, divide by 6, get 66.66 MHz
> from it.  Max baud rate is 4.166 MBaud.
> 

Generally enough for a console....

>   After patch: PLL2 (216 MHz), divide by 1, get 216 MHz from it.  Max
> baud rate is 13.5 MBaud.
>
>   The last I saw this code was 2014 so it's a little fuzzy.  Not sure
> why I didn't keep PLL3 and just change the divider from 6 to 3, that
> would have allowed a max of 8.33 MBaud and at the time, I was just
> looking for at least 8 MBaud.

ok, understood. In any case, it looks to me that this requirement is
very specific to this target (even if I have not got why we cannot set
the parent clock in the DT in linux, it would be my first choice). It
should be implemented into board code, and not in lowlevelinit with some
#ifdef. We already spent a lot of effort to reduce nasty compiler switch
in common code.

> 
>>
>> Looks like you don't use PLL2 as the UART clock anyomore.
> 
>   That's correct, see above.
> 
>>
>> Please expland a bit further on the new UART clock sheme.
>>
>>>
>>> Signed-off-by: Sebastian Reichel <sebastian.reichel at collabora.co.uk>
>>> ---
>>>  arch/arm/mach-imx/mx5/lowlevel_init.S | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-imx/mx5/lowlevel_init.S
> b/arch/arm/mach-imx/mx5/lowlevel_init.S
>>> index f5bc6728b7c2..ee4c3afd2fed 100644
>>> --- a/arch/arm/mach-imx/mx5/lowlevel_init.S
>>> +++ b/arch/arm/mach-imx/mx5/lowlevel_init.S
>>> @@ -319,11 +319,13 @@ setup_pll_func:
>>>         ldr r1, =0x00016154
>>>         str r1, [r0, #CLKCTL_CBCMR]
>>>
>>> +#if !defined(CONFIG_TARGET_MX53PPD)
>>
>> It would be better if this option could be a generic one, something
>> like CONFIG_MX53_SERIAL_HIGHBAUD, so that other boads could use it if
>> needed.
> 
>   Yes it probably would.

Anyway, I have doubt this is useful. U-Boot is just a bootloader, and
the high baud protocol is running in Linux. It is not used in U-Boot,
because U-Boot needs just a console. The well known approach in kernel
is to not rely to the bootloader (I know there are some exceptions..).
Kernel sets all peripherals and does not bother what the bootloader did
or didn't. This is really a good approach and makes no sense to add some
kind of dependency here.

Best regards,
Stefano Babic


-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================