[U-Boot] [PATCH 1/1] rpi: set serial port clock to 48 MHz

[Stephen Warren]

On 11/29/2016 11:47 PM, Mirza Krak wrote:
> 2016-11-30 5:05 GMT+01:00 Stephen Warren <swarren at wwwdotorg.org>:
>> On 11/27/2016 11:05 PM, Mirza Krak wrote:
>>>
>>> From: Mirza Krak <mirza.krak at gmail.com>
>>>
>>> Recently the default UART clock rate has been changed to 48 MHz on all
>>> pi`s in the firmware files, which broke UART support in u-boot.
>>>
>>> Align configuration to boot firmware.
>>
>>
>>> diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
>>
>>
>>> @@ -37,7 +37,7 @@ static const struct pl01x_serial_platdata
>>> serial_platdata = {
>>>         .base = 0x20201000,
>>>  #endif
>>>         .type = TYPE_PL011,
>>> -       .clock = 3000000,
>>> +       .clock = 48000000,

Now that I look into this some more, I will point out that this code 
doesn't even exist in the latest U-Boot, so your patch doesn't apply.

Instead, the UART is now instantiated from DT:-( Presumably the UART 
drivers attempt to dynamically query the clock rate from the clock 
provider described in DT, yet since there is no such provider 
implemented in U-Boot, this must currently fail, and I presume that if 
U-Boot still works at all on the Pi, then the UART drivers must simply 
skip programming the divider at all, and thus rely on the FW having 
initialized it.

Does updating to the latest U-Boot code solve your problem?

I've left my earlier reply below, but I suspect it's no longer relevant.

>> I'm not sure this is the best fix, since it will prevent the latest U-Boot
>> from working with the FW before the change.
>
> Agree that there are better ways of doing this. My purpose with this
> was mainly bringing this problem to your attention as well.
>
>>
>> Better would be:
>>
>> a) Revert the change in the FW, since it violates the contract that
>> previously existed between FW and OS, and will likely break UART usage on
>> (almost?) all SW.
>
> I am not really in position to tell anyone reverting anything. I am
> merely a user reporting a problem.
>
> This does not break UART on Linux, if you skip U-boot. Because the FW
> patches the DT with the correct value. But it is a problem if you use
> U-boot and then load the DT from U-boot, then you will lose the
> "patched" DT and break UART on Linux as well.
>
> Should be said that the "init_uart_clock" has always been a
> configurable option for raspberry pi, U-boot has never actually
> supported configuration of that property and hence there is a problem
> now when the default value of this property has changed. So the best
> solutions is probably supporting parsing the options from firmware to
> setup the UART clock rate correctly (DT or Mailbox).
>
> Jumping to b1), for more text.
>
>> b) Detect the correct clock rate at run-time rather than hard-coding it.
>>
>> b1) Perhaps the rate is available in the DT passed by the FW, which
>> following http://patchwork.ozlabs.org/patch/690769/ "rpi: save firmware
>> provided boot param for later use" should now be available within U-Boot to
>> parse.
>
> This is good to know that it is possible too do, since the FW provided
> DT does contain the correct value for UART clock and I think that this
> would be the best approach for solving this and also supporting any
> value that the user sees fit (by changing the init_uart_clock
> property).
>
>>
>> b2) Perhaps query the rate from the FW via the mailbox API; see
>> https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
>
> Is this API implemented in u-boot? Could be even better to use this,
> then we are not depended on a DT file from firmware (which is my use
> case, where I do not provide a DT to the firmware at all).

U-Boot already queries the MMC clock rate using the mbox API. I imagine 
all you'd need to do is duplicate the logic in board_mmc_init(), and 
update it to query the UART clock rate rather than the MMC clock rate.

Be aware that the "UART clock" only feeds one of the two UARTs in the 
system; this new query code should probably only run if the appropriate 
UART is in use; see code that's #ifdef CONFIG_PL01X_SERIAL.

If you could test this with the FW both before and after the default 
rate change that'd be great, just to be sure.

I do think querying the FW rather than parsing the DT is the better 
option, since that isolates U-Boot from any DT binding changes in this 
area. There shouldn't be any, but you never know. DT is often more fluid 
than it should be.

(Update having noticed the RPi DT conversion: To make U-Boot query the 
clock from the FW, you should implement a clock provider that talks to 
the FW, and presumably the UART drivers will automatically use this)