Re: [Linux-stm32] [PATCH RESEND v3 0/3] Update STM DSI PHY driver

[Yannick FERTRE <yannick.fertre@foss.st.com>]

Hi,

we don't give enough attention to older SOCs like stm32f469. This is an 
error on our part.

I think that to fix this point it would be necessary to define the clock 
hse as clock fix.

I hope to be able to release a patch before the end of August

Best regards

Yannick Fertré


Le 01/08/2024 à 11:07, Raphaël Gallais-Pou a écrit :
>
>
> Le 29/07/2024 à 15:28, Yanjun Yang a écrit :
>> On Fri, Jul 26, 2024 at 09:55:35AM +0200, Philippe CORNU wrote:
>>>
>>>
>>> On 7/22/24 10:38, Yanjun Yang wrote:
>>>>
>>>> This patch (commit id:185f99b614427360) seems to break the dsi of
>>>> stm32f469 chip.
>>>> I'm not familiar with the drm and the clock framework, maybe it's
>>>> because there is no
>>>>    "ck_dsi_phy" defined for stm32f469.
>>>> PS:  Sorry for receiving multiple copies of this email, I forgot to
>>>> use plain text mode last time.
>>>>
>>>
>>> Hi,
>>> Thank you for letting us know that there was this error. We should have
>>> detected this before merging, really sorry for the problems caused 
>>> by this
>>> patch. We will investigate the issue and get back to you as soon as
>>> possible. In the meantime, I think you can revert this patch in your 
>>> git
>>> tree.
>>>
>>> Philippe :-)
>>>
>>
>> Hi,
> Hi,
>
> FYI
> DSI clock tree for stm32f469 can be found here:
> https://www.st.com/resource/en/reference_manual/rm0386-stm32f469xx-and-stm32f479xx-advanced-armbased-32bit-mcus-stmicroelectronics.pdf 
>
>
> Refer to Figure 17: DSI clock tree.
>
> After some research I think "ck_dsi_phy" was introduced in stm32h7 
> platforms. There is a mux which interfaces between various clocks 
> (among ck_hse) and the byte lane clock. stm32f469 has a much simpler 
> clock tree in which we did not bother to implement this "go-between" 
> clock, even though they is an equivalent of the mux.
>
>> After some testing, the reason behind my problem is the parent's name of
>> 'clk_dsi_phy' for stm32f4 is 'clk-hse' other than 'ck_hse'.  I don't
>> know which is the better why to fix it:
>> 1. Change "ck_hse" to "clk-hse" in where "clk_dsi_phy" is defined.
> Doing so will definitely break other platforms.
>
>> 2. Use "pll_in_khz = clk_get_rate(dsi->pllref_clk) / 1000" instead of
>>     "pll_in_khz = (unsigned int)(parent_rate / 1000)" when get the clock
>>     rate.
> dsi->pllref_clk refers to the HSE clock if you take a look in the 
> device-tree. This is the reason why this work on your setup. I doubt 
> nevertheless that it wouldn't work on other platforms. But this would 
> be a semantic nonsense, since the DSI byte lane clock is not always 
> derived from HSE clock on other platforms.
>
> Looking again at the clk-stm32f4 driver and the DSI clock tree linked, 
> we can maybe implement the desired clock even if it is not represented 
> on the diagram.
>
> Eventually if this solution does not work we will go to the second 
> solution you suggested and we will test it on all platforms.
>
> @Philippe, @Yannick
> Do you agree with this workflow ?
>
> Regards,
> Raphaël
>
>
>>
>> Both method can fix my problem. The first one might break other
>> platforms. Maybe I should change the clock name of 'clk-hse'. However,
>> I can't find the defination of this clock name for stm32f4.