[U-Boot] [PATCH 03/10] arm: exynos: Add get_lcd_clk and set_lcd_clk callbacks for Exynos5420

[Simon Glass]

Hi,

On 24 June 2014 05:36, Minkyu Kang <mk7.kang at samsung.com> wrote:
> On 24/06/14 20:28, Ajay kumar wrote:
>> Hi Minkyu,
>>
>> On Tue, Jun 24, 2014 at 3:36 AM, Minkyu Kang <mk7.kang at samsung.com> wrote:
>>> On 17/06/14 18:06, Ajay Kumar wrote:
[snip]
>>>
>>> No. Please don't mix cpu_is... and proid_is....
>>> You can refer to other functions.
>> Actually, only "cpu_is_exynos4" and "cpu_is_exynos5" are defined in cpu.h.
>> And, I need different clock setting for 5250 and 5420.
>> The only way to achieve this is by calling appropriate functions based
>> on check to proid_is_exynos5420().
>> Let me know if these is some other way!
>
> unsigned long get_lcd_clk(void)
> {
>         if (cpu_is_exynos4()) {
>                 return exynos4_get_lcd_clk();
>         } else {
>                 if (proid_is_exynos5420())
>                         return exynos5420_get_lcd_clk();
>                 else
>                         return exynos5_get_lcd_clk();
>         }
> }

Please be careful with 'else' in SPL. It can pull in code that we
don't need. I think the second else here should be 'if
prodid_is_exynos5250()' or something like that. Otherwise we can find
ourselves including the tables for both 5250 and 5420 in the SPL for
5420. Granted we might want our SPL to support both, but if the CONFIG
options are not set for both, then the user does not want both, so we
should avoid waiting the code space.

Regards,
Simon