Re: [PATCH RESEND] clk: meson: pll: copy retry workaround from vendor driver

[Heiner Kallweit <hkallweit1@gmail.com>]

On 06.09.2022 17:21, Jerome Brunet wrote:
> 
> On Tue 30 Aug 2022 at 14:33, Yu Tu <yu.tu@amlogic.com> wrote:
> 
>> On 2022/8/29 17:54, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Sun 14 Aug 2022 at 23:25, Heiner Kallweit <hkallweit1@gmail.com>
>>> wrote:
>>>
>>>> On a S905X4-based system this call fails randomly.
>>>> The vendor driver has a retry mechanism and on my system
>>>> the second attempt is successful always.
> 
> Heiner, ideally commit message should not be about 'your' system
> but describe what the system actually is. This would be more useful down
> the road.
> 
> Based on Yu Tu feedback I'd propose
> 
> '
> The PCIe PLL locking may be unreliable under some circumstance, such as
> high or low temperature. If the PLL fails to lock, reset it a try again.
> 
> This helps on the S905X4
> '
> 
> If this is OK with you, I can amend the message before applying the
> patch, or you may resubmit. As you wish.
> 

Yes, please amend the commit message as suggested.
Thanks for the review and follow-up.

>>>>
>>> This reason looks a bit weak to me.
>>> I'd like AML team to comment on this PLL problem as I suspect it might
>>> relate to other PLL we have been seeing
>>
>> First of all, we've had this problem before. But the probability is very
>> low, most of the PLL will not lock when the chip is tested at high and low
>> temperature.
>>
>> Our actual code has retry mechanism to solve the above problem.
>>
>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>>   drivers/clk/meson/clk-pll.c | 12 ++++++++----
>>>>   1 file changed, 8 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
>>>> index 9e55617bc..daa025b6d 100644
>>>> --- a/drivers/clk/meson/clk-pll.c
>>>> +++ b/drivers/clk/meson/clk-pll.c
>>>> @@ -320,12 +320,16 @@ static int meson_clk_pll_is_enabled(struct clk_hw *hw)
>>>>     static int meson_clk_pcie_pll_enable(struct clk_hw *hw)
>>>>   {
>>>> -	meson_clk_pll_init(hw);
>>>> +	int retries = 10;
>>>>   -	if (meson_clk_pll_wait_lock(hw))
>>>> -		return -EIO;
>>>> +	do {
>>>> +		meson_clk_pll_init(hw);
>>>> +		if (!meson_clk_pll_wait_lock(hw))
>>>> +			return 0;
>>>> +		pr_info("Retry enabling PCIe PLL clock\n");
>>>> +	} while (--retries);
>>>>   -	return 0;
>>>> +	return -EIO;
>>>>   }
>>>>     static int meson_clk_pll_enable(struct clk_hw *hw)
>>> .
>