[4/9] ARM: dts: stm32mp1: move FDCAN to PLL4_R

From: Antonio Borneo <antonio.borneo at st.com>

LTDC modifies the clock frequency to adapt it to the display. Such
frequency change is not detected by the FDCAN driver that instead
cache the value at probe and pretend to use it later.

Keep the LTDC alone on PLL4_Q by moving the FDCAN to PLL4_R.

Signed-off-by: Antonio Borneo <antonio.borneo at st.com>
Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
---

 arch/arm/dts/stm32mp157a-avenger96-u-boot.dtsi | 2 +-
 arch/arm/dts/stm32mp157a-dk1-u-boot.dtsi       | 2 +-
 arch/arm/dts/stm32mp157c-ed1-u-boot.dtsi       | 2 +-
 arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

On 1/28/20 10:11 AM, Patrick Delaunay wrote:
> From: Antonio Borneo <antonio.borneo at st.com>
> 
> LTDC modifies the clock frequency to adapt it to the display. Such
> frequency change is not detected by the FDCAN driver that instead
> cache the value at probe and pretend to use it later.
> 
> Keep the LTDC alone on PLL4_Q by moving the FDCAN to PLL4_R.

Now this looks like a grisly workaround. Can you fix the LTDC driver to
do something sane on boards which didn't update bootloader yet ?

Hi Marek,

> From: Marek Vasut <marex at denx.de>
> Sent: mardi 28 janvier 2020 13:16
> 
> On 1/28/20 10:11 AM, Patrick Delaunay wrote:
> > From: Antonio Borneo <antonio.borneo at st.com>
> >
> > LTDC modifies the clock frequency to adapt it to the display. Such
> > frequency change is not detected by the FDCAN driver that instead
> > cache the value at probe and pretend to use it later.
> >
> > Keep the LTDC alone on PLL4_Q by moving the FDCAN to PLL4_R.
> 
> Now this looks like a grisly workaround. Can you fix the LTDC driver to do
> something sane on boards which didn't update bootloader yet ?

In fact it more a issue in the initial clock-tree used when I upstream the ST board the first time... based on our delivery v1.0.0

It is already corrected in downstream on v1.1.0:
+ For U-Boot = https://github.com/STMicroelectronics/u-boot/commit/d62f14dece32e41c2854b9ff44aca7b8384aa8a0
+ For TF-A = https://github.com/STMicroelectronics/arm-trusted-firmware/commit/9a24ceda6c3ba060d9acf2b26d069fedde9f0807

The LTDC/DSI need to set the pixel clock according the panel configuration and settings: it is normal and needed.

But If the pixel clock is shared with FDCAN, which expects that its input clock is fixed, an issue occur when the pixel clock change.

We could add protection in FDCAN driver (don't assume fixed clock in probe for example) 
but anyway that don't protect for any issue (pending FDCAN transfer when pixel clock change).

The main issue is that we try to share a clock source between 2 IP that are not compatible:
1/ FDCAN => clock source configurated by CLK_FDCAN_PLL4Q
2/ pixel clocl for LTDC and DSI = LTDC_PX or DSI_PX  => _PLL4_Q  (hardcoded in RCC)

The clock source for pixel clock PLL4_Q need only managed only by LDTC as it can modify the source clock.

It is why we decide to change the reference clock tree used on ST Microelectronic boards.
And unfortunately that impacts the first stage bootloader.

For information in our solution the clock tree is fixed and configurated at boot by first stage bootloader 
(TF-A normally for trusted boot chain / SPL for basic boot chain) as this configuration is  done in secured
registers with information provided by device-tree.

See https://wiki.st.com/stm32mpu/wiki/STM32MP15_clock_tree for details

Regards

Patrick

On 1/29/20 5:51 PM, Patrick DELAUNAY wrote:
> Hi Marek,

Hi,

>> From: Marek Vasut <marex at denx.de>
>> Sent: mardi 28 janvier 2020 13:16
>>
>> On 1/28/20 10:11 AM, Patrick Delaunay wrote:
>>> From: Antonio Borneo <antonio.borneo at st.com>
>>>
>>> LTDC modifies the clock frequency to adapt it to the display. Such
>>> frequency change is not detected by the FDCAN driver that instead
>>> cache the value at probe and pretend to use it later.
>>>
>>> Keep the LTDC alone on PLL4_Q by moving the FDCAN to PLL4_R.
>>
>> Now this looks like a grisly workaround. Can you fix the LTDC driver to do
>> something sane on boards which didn't update bootloader yet ?
> 
> In fact it more a issue in the initial clock-tree used when I upstream the ST board the first time... based on our delivery v1.0.0
> 
> It is already corrected in downstream on v1.1.0:
> + For U-Boot = https://github.com/STMicroelectronics/u-boot/commit/d62f14dece32e41c2854b9ff44aca7b8384aa8a0
> + For TF-A = https://github.com/STMicroelectronics/arm-trusted-firmware/commit/9a24ceda6c3ba060d9acf2b26d069fedde9f0807
> 
> The LTDC/DSI need to set the pixel clock according the panel configuration and settings: it is normal and needed.
> 
> But If the pixel clock is shared with FDCAN, which expects that its input clock is fixed, an issue occur when the pixel clock change.

I understand the problem you are trying to solve.

What I think you are missing is that not everyone will update
ATF/U-Boot/Linux in lockstep. That is the problem you need to deal with
here.

Hi Marek,

> From: Marek Vasut <marex at denx.de>
> Sent: jeudi 30 janvier 2020 03:23
> 
> On 1/29/20 5:51 PM, Patrick DELAUNAY wrote:
> > Hi Marek,
> 
> Hi,
> 
> >> From: Marek Vasut <marex at denx.de>
> >> Sent: mardi 28 janvier 2020 13:16
> >>
> >> On 1/28/20 10:11 AM, Patrick Delaunay wrote:
> >>> From: Antonio Borneo <antonio.borneo at st.com>
> >>>
> >>> LTDC modifies the clock frequency to adapt it to the display. Such
> >>> frequency change is not detected by the FDCAN driver that instead
> >>> cache the value at probe and pretend to use it later.
> >>>
> >>> Keep the LTDC alone on PLL4_Q by moving the FDCAN to PLL4_R.
> >>
> >> Now this looks like a grisly workaround. Can you fix the LTDC driver
> >> to do something sane on boards which didn't update bootloader yet ?
> >
> > In fact it more a issue in the initial clock-tree used when I upstream
> > the ST board the first time... based on our delivery v1.0.0
> >
> > It is already corrected in downstream on v1.1.0:
> > + For U-Boot =
> > + https://github.com/STMicroelectronics/u-boot/commit/d62f14dece32e41c
> > + 2854b9ff44aca7b8384aa8a0 For TF-A =
> > + https://github.com/STMicroelectronics/arm-trusted-firmware/commit/9a
> > + 24ceda6c3ba060d9acf2b26d069fedde9f0807
> >
> > The LTDC/DSI need to set the pixel clock according the panel configuration and
> settings: it is normal and needed.
> >
> > But If the pixel clock is shared with FDCAN, which expects that its input clock is
> fixed, an issue occur when the pixel clock change.
> 
> I understand the problem you are trying to solve.
> 
> What I think you are missing is that not everyone will update ATF/U-Boot/Linux in
> lockstep. That is the problem you need to deal with here.

I understood the possible issue and I hope that I will be not the case
(at least for the ST Microelectronics boards).

We are aware of the possible issue to fixe these clocks in first stage bootloader but after a long
debate, we don't found a better solution, with the constraints:
- M4 firmware require clock initialization before start and it can be loaded by U-Boot
- clock tree is generated by CubeMX tools which generate device tree (for bootloaders and kernel)
- "assigned-clock" management in Linux kernel could lead us to a race condition for shared clock

We spent a long time to found a clock tree which respect all the constraints of the SOC
before to our first release v1.0 and before I upstream the stm32mp1 support in U-Boot.

Then I wait a year before to upstream the patches on clock tree initialization (and the second
release v1.2) to avoid too many iteration.
 And this patch on FDCAN is the only issue found on the initial clock tree.

Today I think (hope?) it will be the last patch on this part.

Regards

Patrick