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

[Minkyu Kang]

On 24/06/14 20:41, Ajay kumar wrote:
> On Tue, Jun 24, 2014 at 7:36 AM, 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:
>>>>> Add get_lcd_clk and set_lcd_clk callbacks for Exynos5420 needed by
>>>>> exynos video driver.
>>>>> Also, configure ACLK_400_DISP1 as the parent for MUX_ACLK_400_DISP1_SUB_SEL.
>>>>>
>>>>> Signed-off-by: Ajay Kumar <ajaykumar.rs at samsung.com>
>>>>> ---
>>>>>  arch/arm/cpu/armv7/exynos/clock.c         | 74 +++++++++++++++++++++++++++++--
>>>>>  arch/arm/cpu/armv7/exynos/exynos5_setup.h |  2 +-
>>>>>  arch/arm/include/asm/arch-exynos/clk.h    |  1 +
>>>>>  3 files changed, 73 insertions(+), 4 deletions(-)
>>>>>
>>>>>  void exynos4_set_mipi_clk(void)
>>>>>  {
>>>>>       struct exynos4_clock *clk =
>>>>> @@ -1602,14 +1667,17 @@ unsigned long get_lcd_clk(void)
>>>>>  {
>>>>>       if (cpu_is_exynos4())
>>>>>               return exynos4_get_lcd_clk();
>>>>> -     else
>>>>> -             return exynos5_get_lcd_clk();
>>>>> +     else if (proid_is_exynos5420())
>>>>> +             return exynos5420_get_lcd_clk();
>>>>> +     return exynos5_get_lcd_clk();
>>>>
>>>> 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();
>>         }
>> }
> Actually, both the ways, functionality is the same.
> Its just that, in your case readability is fine, and in my case
> the code takes fewer number of lines. ^^
> 

At the same level if statement, it should be same level context.
cpu_is... and proid_is are not same level context.
Please consider it.

Thanks,
Minkyu Kang.