[PATCH v8 2/6] clk: samsung: add cpu clock configuration data and instantiate cpu clock

[Tomasz Figa]

Hi Thomas,

Just few minor comments for things I probably missed before.

On 29.07.2014 07:28, Thomas Abraham wrote:

[snip]

> @@ -1356,6 +1357,16 @@ static struct samsung_pll_clock exynos4x12_plls[nr_plls] __initdata = {
>  			VPLL_LOCK, VPLL_CON0, NULL),
>  };
>  
> +static const struct exynos_cpuclk_cfg_data e4210_armclk_d[] __initconst = {
> +	{ 1200000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 5), },
> +	{ 1000000, E4210_CPU_DIV0(7, 1, 4, 3, 7, 3), E4210_CPU_DIV1(0, 4), },
> +	{  800000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
> +	{  500000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },
> +	{  400000, E4210_CPU_DIV0(7, 1, 3, 3, 7, 3), E4210_CPU_DIV1(0, 3), },

I have noticed that the old driver does not have this operating point.
While it is probably OK to add this one and even few more for all
possible APLL settings, I am interested in how you obtained the values
for DIV0 and DIV1 registers for this configuration.

> +	{  200000, E4210_CPU_DIV0(0, 1, 1, 1, 3, 1), E4210_CPU_DIV1(0, 3), },
> +	{  0 },
> +};

[snip]

> diff --git a/include/dt-bindings/clock/exynos5250.h b/include/dt-bindings/clock/exynos5250.h
> index 4273891..855d809 100644
> --- a/include/dt-bindings/clock/exynos5250.h
> +++ b/include/dt-bindings/clock/exynos5250.h
> @@ -21,6 +21,7 @@
>  #define CLK_FOUT_CPLL		6
>  #define CLK_FOUT_EPLL		7
>  #define CLK_FOUT_VPLL		8
> +#define CLK_ARM_CLK		12

Why 12 not 9?

Best regards,
Tomasz