From mboxrd@z Thu Jan  1 00:00:00 1970
From: Minkyu Kang <mk7.kang@samsung.com>
Date: Thu, 26 Jun 2014 10:06:02 +0900
Subject: [U-Boot] [PATCH 03/10] arm: exynos: Add get_lcd_clk and
 set_lcd_clk callbacks for Exynos5420
In-Reply-To: <CAEC9eQNFbDw14uHbd+mfVuswhy7sfhjpOp5E=L39TEPSUxUhow@mail.gmail.com>
References: <1402995979-32394-1-git-send-email-ajaykumar.rs@samsung.com>
	<1402995979-32394-4-git-send-email-ajaykumar.rs@samsung.com>
	<53A92A6E.3040806@samsung.com>
	<CAEC9eQOTQ4PzXXEv+Y3TurUSPDKH7bGN0W3w+E9omJ6Tt9tQ9A@mail.gmail.com>
	<53A962C1.909@samsung.com>
	<CAEC9eQNFbDw14uHbd+mfVuswhy7sfhjpOp5E=L39TEPSUxUhow@mail.gmail.com>
Message-ID: <53AB71FA.9060004@samsung.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 24/06/14 20:41, Ajay kumar wrote:
> On Tue, Jun 24, 2014 at 7:36 AM, Minkyu Kang <mk7.kang@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@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@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.