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

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

[Heiner Kallweit]

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.

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)
-- 
2.37.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jerome Brunet]

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.
>

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

> 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)


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Yu Tu]

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.
>>
> 
> 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)
> 
> .

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jerome Brunet]

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.

>>>
>> 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)
>> .


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Heiner Kallweit]

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)
>>> .
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Jerome Brunet]

On Tue 06 Sep 2022 at 20:29, Heiner Kallweit <hkallweit1@gmail.com> wrote:

> 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.
>

Applied, Thx

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel