Re: [PATCH 4/7] clk: samsung: exynos5433: fix PLL rates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 14.02.2018 06:45, Chanwoo Choi wrote:
> Hi Sylwester and Andrzej,
>
> Exynos5433 TRM shows different PLL frequency from the calculated value. Actually, I'm not sure which value is the correct value between TRM's value and real calculated value.
>
> If we only consider the equation, I agree that we have to replace them with real calculated value with equation. But, TRM shows the diagram of clock domain which contains the multiple IPs. And each diagram specifies the required source clock rate of each child IP. Maybe, this source clock rate might be calculated from the PLL rate provided by TRM. (even if it's different from real calculated value).
>
> Although the real rate is a little bit incorrect, we might better to keep PLL rate provided by TRM in order to get the required source clock of child IPs as the TRM. If we modify the PLL rate provided by TRM, the source clock rate of child IP might be different from TRM.
>
> Basically, I have no objection of this patch. Just I think that need to check it.

After verifying documentation it seems that in case of fractional PLLs
TRM shows rates with lower precision, so I wouldn't say in such case it
is different.
Only 667MHz frequency looks odd, but it seems to be a bug in TRM, I
guess copy/paste bug from Exyons5430, where 667MHz rate was generated
from different PMS coefficients.
Android code has also tables proving it should be 666MHz [1].


[1]:
https://github.com/googyanas/Googy-Max-N4-Kernel/blob/master/drivers/clk/samsung/clk-exynos5433.c#L3260

Regards
Andrzej

>
> Regards,
> Chanwoo Choi
>
> On 2018년 02월 13일 22:40, Andrzej Hajda wrote:
>> Declared rates did not match rates generated by PLL.
>> As a result driver behaved inconsitently.
>>
>> Signed-off-by: Andrzej Hajda <a.hajda@xxxxxxxxxxx>
>> ---
>>  drivers/clk/samsung/clk-exynos5433.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
>> index db270908037a..335bebfa21c0 100644
>> --- a/drivers/clk/samsung/clk-exynos5433.c
>> +++ b/drivers/clk/samsung/clk-exynos5433.c
>> @@ -729,7 +729,7 @@ static const struct samsung_pll_rate_table exynos5433_pll_rates[] __initconst =
>>  	PLL_35XX_RATE(800000000U,  400, 6,  1),
>>  	PLL_35XX_RATE(733000000U,  733, 12, 1),
>>  	PLL_35XX_RATE(700000000U,  175, 3,  1),
>> -	PLL_35XX_RATE(667000000U,  222, 4,  1),
>> +	PLL_35XX_RATE(666000000U,  222, 4,  1),
>>  	PLL_35XX_RATE(633000000U,  211, 4,  1),
>>  	PLL_35XX_RATE(600000000U,  500, 5,  2),
>>  	PLL_35XX_RATE(552000000U,  460, 5,  2),
>> @@ -757,12 +757,12 @@ static const struct samsung_pll_rate_table exynos5433_pll_rates[] __initconst =
>>  /* AUD_PLL */
>>  static const struct samsung_pll_rate_table exynos5433_aud_pll_rates[] __initconst = {
>>  	PLL_36XX_RATE(400000000U, 200, 3, 2,      0),
>> -	PLL_36XX_RATE(393216000U, 197, 3, 2, -25690),
>> +	PLL_36XX_RATE(393216003U, 197, 3, 2, -25690),
>>  	PLL_36XX_RATE(384000000U, 128, 2, 2,      0),
>> -	PLL_36XX_RATE(368640000U, 246, 4, 2, -15729),
>> -	PLL_36XX_RATE(361507200U, 181, 3, 2, -16148),
>> -	PLL_36XX_RATE(338688000U, 113, 2, 2,  -6816),
>> -	PLL_36XX_RATE(294912000U,  98, 1, 3,  19923),
>> +	PLL_36XX_RATE(368639991U, 246, 4, 2, -15729),
>> +	PLL_36XX_RATE(361507202U, 181, 3, 2, -16148),
>> +	PLL_36XX_RATE(338687988U, 113, 2, 2,  -6816),
>> +	PLL_36XX_RATE(294912002U,  98, 1, 3,  19923),
>>  	PLL_36XX_RATE(288000000U,  96, 1, 3,      0),
>>  	PLL_36XX_RATE(252000000U,  84, 1, 3,      0),
>>  	{ /* sentinel */ }
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux