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

[Chanwoo Choi <cw00.choi@samsung.com>]

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.

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@samsung.com>
> ---
>  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 */ }
>