Hi Ajay,
On 2 March 2015 at 09:07, Ajay kumar <ajayn...@gmail.com> wrote:
> Hi Simon,
>
> On Mon, Mar 2, 2015 at 7:53 AM, Simon Glass <s...@google.com> wrote:
>> Hi Ajay,
>>
>> On 8 December 2014 at 15:40, Simon Glass <s...@google.com> wrote:
>>> Hi Ajay,
>>>
>>>
>>> On 7 December 2014 at 22:43, Ajay kumar <ajayn...@gmail.com> wrote:
>>>>
>>>> Hi Minkyu,
>>>>
>>>>
>>>> On Mon, Dec 8, 2014 at 11:07 AM, Ajay kumar <ajayn...@gmail.com> wrote:
>>>> > On Fri, Dec 5, 2014 at 9:02 PM, Minkyu Kang <proms...@gmail.com> wrote:
>>>> >> Dear Ajay Kumar,
>>>> >>
>>>> >> On 5 December 2014 at 23:13, Ajay Kumar <ajaykumar...@samsung.com>
>>>> >> wrote:
>>>> >>
>>>> >>> Add get_lcd_clk and set_lcd_clk callbacks for Exynos5800 needed by
>>>> >>> exynos video driver.
>>>> >>>
>>>> >>> Signed-off-by: Ajay Kumar <ajaykumar...@samsung.com>
>>>> >>> ---
>>>> >>> arch/arm/cpu/armv7/exynos/clock.c | 63
>>>> >>> +++++++++++++++++++++++++++++++-
>>>> >>> arch/arm/include/asm/arch-exynos/clk.h | 3 ++
>>>> >>> 2 files changed, 64 insertions(+), 2 deletions(-)
>>>> >>>
>>>> >>> diff --git a/arch/arm/cpu/armv7/exynos/clock.c
>>>> >>> b/arch/arm/cpu/armv7/exynos/clock.c
>>>> >>> index 8fab135..1a34ad6 100644
>>>> >>> --- a/arch/arm/cpu/armv7/exynos/clock.c
>>>> >>> +++ b/arch/arm/cpu/armv7/exynos/clock.c
>>>> >>> @@ -1066,6 +1066,36 @@ static unsigned long
>>>> >>> exynos5420_get_lcd_clk(void)
>>>> >>> return pclk;
>>>> >>> }
>>>> >>>
>>>> >>> +static unsigned long exynos5800_get_lcd_clk(void)
>>>> >>> +{
>>>> >>> + struct exynos5420_clock *clk =
>>>> >>> + (struct exynos5420_clock *)samsung_get_base_clock();
>>>> >>> + unsigned long sclk;
>>>> >>> + unsigned sel;
>>>> >>>
>>>> >>
>>>> >> just unsigned? why don't you specify in detail?
>>>> I will fix this.
>>>>
>>>> >>
>>>> >>> + unsigned ratio;
>>>> >>> +
>>>> >>> + sel = (readl(&clk->src_disp10) >> 4) & 7;
>>>> >>>
>>>> >>
>>>> >> please add comment how you get "sel" from disp10.
>>>> >> and if 7 means mask then please use 0x7. it looks more clearly.
>>>> Ok.
>>>>
>>>> >> +
>>>> >>> + /*
>>>> >>> + * Mapping of CLK_SRC_DISP10 CLKMUX_FIMD1 [6:4] values into
>>>> >>> + * PLLs. The first element is a placeholder to bypass the
>>>> >>> + * default settig.
>>>> >>> + */
>>>> >>> + const int reg_map[] = {0, CPLL, DPLL, MPLL, SPLL,
>>>> >>> + IPLL, EPLL, RPLL};
>>>> >>>
>>>> >>
>>>> >> please don't define local variable at middle of function.
>>>> >> you can move it to top of the function or
>>>> >> it seems to use sel is true then you can move it into the if statement.
>>>> I will move this to the top of the function.
>>>>
>>>> >>
>>>> >>> + if (sel)
>>>> >>> + sclk = get_pll_clk(reg_map[sel]);
>>>> >>> + else
>>>> >>> + sclk = 24000000;
>>>> >>>
>>>> >>
>>>> >> please define this value.
>>>> Ok.
>>>>
>>>> >>
>>>> >>> + /*
>>>> >>> + * CLK_DIV_DISP10
>>>> >>> + * FIMD1_RATIO [3:0]
>>>> >>> + */
>>>> >>> + ratio = readl(&clk->div_disp10) & 0xf;
>>>> >>> +
>>>> >>> + return sclk / (ratio + 1);
>>>> >>> +}
>>>> >>> +
>>>> >>> void exynos4_set_lcd_clk(void)
>>>> >>> {
>>>> >>> struct exynos4_clock *clk =
>>>> >>> @@ -1197,6 +1227,31 @@ void exynos5420_set_lcd_clk(void)
>>>> >>> writel(cfg, &clk->div_disp10);
>>>> >>> }
>>>> >>>
>>>> >>> +void exynos5800_set_lcd_clk(void)
>>>> >>> +{
>>>> >>> + struct exynos5420_clock *clk =
>>>> >>> + (struct exynos5420_clock *)samsung_get_base_clock();
>>>> >>> + unsigned int cfg;
>>>> >>> +
>>>> >>> + /*
>>>> >>> + * Use RPLL for pixel clock
>>>> >>> + * CLK_SRC_DISP10 CLKMUX_FIMD1 [6:4]
>>>> >>> + * ==================
>>>> >>> + * 111: SCLK_RPLL
>>>> >>> + */
>>>> >>> + cfg = readl(&clk->src_disp10) | (7 << 4);
>>>> >>> + writel(cfg, &clk->src_disp10);
>>>> >>> +
>>>> >>> + /*
>>>> >>> + * CLK_DIV_DISP10
>>>> >>> + * FIMD1_RATIO [3:0]
>>>> >>> + */
>>>> >>> + cfg = readl(&clk->div_disp10);
>>>> >>> + cfg &= ~(0xf << 0);
>>>> >>> + cfg |= (0 << 0);
>>>> >>>
>>>> >>
>>>> >> it looks meaningless.
>>>> Why not?
>>>> I agree that the calculation can be skipped, and directly FIMD
>>>> clock divider can be set to 0. But, I prefer to keep the readability.
>>>> In fact, similar "meaningless" code is already part of the tree:
>>>>
>>>> http://git.denx.de/?p=u-boot/u-boot-samsung.git;a=blob;f=arch/arm/cpu/armv7/exynos/clock.c;h=8fab135bebf4ef6900677847b60a8e1a1520254c;hb=HEAD#l1194
>>>
>>>
>>> Well perhaps a #define for the shift value? Then it would mean something.
>>>
>>> Also this can probably use clrsetbits_le32().
>>
>> This is one of four patches that I think were never applied and need a
>> respin. Are you able to do that? Then Pi LCD will work OK I think.
> Yes, I am respinning this patch. Also, trying to move out GPIOs from
> smdk5420.c. Will be posting them tomorrow.
OK great. You might find gpio_request_by_name() useful, and
gpio_request_by_name_nodev() where the thing requesting the GPIOs does
not use driver model yet.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
