Re: [PATCH 1/3] phy: stm32-usbphyc: add counter of PLL consumer

[Sean Anderson <seanga2@gmail.com>]

On 5/17/22 3:42 AM, Patrick DELAUNAY wrote:
> Hi Sean,
> 
> On 5/11/22 18:48, Sean Anderson wrote:
>> On 5/10/22 5:51 AM, Amelie Delaunay wrote:
>>> Hi Patrick,
>>> Hi Sean,
>>>
>>> On 5/9/22 16:37, Patrick DELAUNAY wrote:
>>>> Hi Sean,
>>>>
>>>> On 5/8/22 20:21, Sean Anderson wrote:
>>>>> On 4/26/22 8:37 AM, Patrick Delaunay wrote:
>>>>>> Add the counter of the PLL user n_pll_cons managed by the 2 functions
>>>>>> stm32_usbphyc_pll_enable / stm32_usbphyc_pll_disable.
>>>>>>
>>>>>> This counter allow to remove the function stm32_usbphyc_is_init
>>>>>> and it is a preliminary step for ck_usbo_48m introduction.
>>>>>
>>>>> Is it necessary to disable this clock before booting to Linux? If it isn't,
>>>>> then perhaps it is simpler to just not disable the clock.
>>>>>
>>>>> --Sean
>>>>
>>>>
>>>> No, it is not necessary, we only need to enable the clock for the first user.
>>>>
>>>> I copy the clock behavior from kernel,
>>>>
>>>> but I agree that can be simpler.
>>>>
>>>>
>>>> Amelie any notice about this point ?
>>>>
>>>> Do you prefer that I kept the behavior - same as kernel driver - or I simplify the U-Boot driver ?
>>>
>>> In case the PLL has not been disabled before Kernel boot, usbphyc Kernel driver will wait for the PLL pwerdown.
>>> USB could also not being used in Kernel, so PLL would remain enabled, and would waste power.
>>> I am rather in favor of disabling the PLL.
>>
>> It should be disabled if clk_ignore_unused is not in the kernel parameters,
>> as long as Linux is also aware of the clock.
>>
>> Generally, I would like to avoid refcounting if possible. Many U-Boot
>> drivers do not disable their clocks (because they don't do any cleanup),
>> so you can end up with the clock staying on anyway.
>>
>> --Sean
>>
>>> Regards,
>>> Amelie
>>>
>>>>
>>>>
>>>> Patrick
>>>>
>>>>
>>>>>
> 
> In general, I'm also in favor of not disabling the clock in U-Boot.
> 
> But this PLL with vdda1v1 and vdda1v8 dependency is requested for
> 
> - USBPHYC himself or
> 
> - source clock of RCC,
> 
> 
> And we have have see many issue for this PLL initialization sequence / dependencies.
> 
> 
> So for clock support, I only reused the existing/working function called by the PHY ops init & exit =
> 
> stm32_usbphyc_phy_init & stm32_usbphyc_phy_exit
> 
> => the PLL and the associated VDD  are already deactivated on USBPHYC exit.
> 
> 
> And to be coherent I for the same for the clock to avoid conflict between these 2 users
> 
> (USBPHYC or RCC) as it is done also in Linux kernel.
> 
> 
> Avoid to deactivate PLL on clock disable is a major objection or just a advice?

Just advice. If all of the clock's users call disable, then it is fine.

--Sean