Re: [PATCH V5 5/5] ARM: EXYNOS4: Add platform data for EXYNOS4 FIMD

[Anand Kumar N <anand.kn@samsung.com>]

Hi Marek/Jingoo,

On Wed, Jun 22, 2011 at 3:17 PM, Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
> Hello,
>
> On Wednesday, June 22, 2011 8:42 AM Jingoo Han wrote:
>
>> From: Jonghun Han <jonghun.han@samsung.com>
(snipped)
Instead of hardcoding the parent clock in platform/bootloader code ,is
it not possible to select/set the
parent clock based on the pixel clk(plat data) having the least delta
with the 9 src clks.with this change I have checked it for WA101S.
These are the changes I am proposing..

arch/arm/mach-exynos4/clock.c
+struct clk *clkset_sclk_fimd0_list[] = {
+        [0] = &clk_sclk_xxti,
+        [1] = &clk_sclk_xusbxti,
+        [2] = &clk_sclk_hdmi27m,
+        [3] = &clk_sclk_usbphy0,
+        [4] = &clk_sclk_usbphy1,
+        [5] = &clk_sclk_hdmiphy,
+        [6] = &clk_mout_mpll.clk,
+        [7] = &clk_mout_epll.clk,
+        [8] = &clk_sclk_vpll.clk,
+};
arch/arm/mach-exynos4/mach-smdkv310.c
+static int fimd_s3c_consider_clock(struct device *sfb,int src
,unsigned int wanted)
+{
+        unsigned long rate = 0;
+        int div =1;
+        struct clk *mout_mpll = NULL;
+
+        if(clkset_sclk_fimd0_list[src]){
+        mout_mpll = clk_get(sfb,clkset_sclk_fimd0_list[src]->name);
+        if (IS_ERR(mout_mpll)) {
+               dev_err(sfb, "failed to clk_get
%s\n",clkset_sclk_fimd0_list[src]->name);
+        }
+
+        rate = clk_get_rate(mout_mpll);
+
+        for (div = 1; div < 256; div *= 2) {
+                if ((rate / div) <= wanted)
+                        break;
+        }
+
+
+       }
+        return (wanted - (rate / div));
+}
+
+int exynos4_fimd0_find_clock(struct platform_device *pdev,struct clk
**lcd_clk,unsigned int clock(pixel clock needed for the lcd))
+{
+        int best = 0;
+        int delta = 0;
+        int best_src = 0;
+        int src;
+        struct clk *best_clk_src = NULL;
+       struct clk *clk  = NULL;
+
+       if (clock = 0)
+                return 0;
+
+        for (src = 0; src < MAX_NUM_CLKS ;src++) {
+                delta = fimd_s3c_consider_clock(&pdev->dev,src,clock);
+                if (delta < best) {
+                        best = delta;
+                        best_src = src;
+                }
+        }
+       clk = clk_get(&pdev->dev, "sclk_fimd");
+        if (IS_ERR(clk)) {
+               dev_err(&pdev->dev, "failed to get sclk for fimd\n");
+               goto err_clk2;
+       }
+
+       best_clk_src clk_get(&pdev->dev,clkset_sclk_fimd0_list[best_src]->name);
+       if (IS_ERR(best_clk_src)) {
+               dev_err(&pdev->dev, "failed to get best_src\n");
+               goto err_clk1;
+       }
+       clk_set_parent(clk,best_clk_src);
+       *lcd_clk = clk;
+        clk_put(best_clk_src);
+        clk_enable(clk);
+        dev_dbg(&pdev->dev, "set fimd sclk rate to %d\n", clock);
+
+err_clk1:
+       clk_put(best_clk_src);
+err_clk2:
+       clk_put(clk);
+
+       return -EINVAL;
+}
I have based these patches on the v1 version of this patch.I can base
these changes
on your latest changes(v6) and send as a patch.

>> +static int __init smdkc210_fimd0_setup_clock(void)
>> +{
>> +     struct clk *sclk = NULL;
>> +     struct clk *mout_mpll = NULL;
>> +
>> +     u32 rate = 0;
>> +
>> +     sclk = clk_get(&s5p_device_fimd0.dev, "sclk_fimd");
>> +     if (IS_ERR(sclk)) {
>> +             printk(KERN_ERR "failed to get sclk for fimd\n");
>> +             goto err_clk2;
>> +     }
>> +
>> +     mout_mpll = clk_get(NULL, "mout_mpll");
>> +     if (IS_ERR(mout_mpll)) {
>> +             printk(KERN_ERR "failed to get mout_mpll\n");
>> +             goto err_clk1;
>> +     }
>> +
>> +     clk_set_parent(sclk, mout_mpll);
>> +     if (!rate)
>> +             rate = 134000000;
>> +
>> +     clk_set_rate(sclk, rate);
>> +
>> +     clk_put(sclk);
>> +     clk_put(mout_mpll);
>> +
>> +     return 0;
>> +
>> +err_clk1:
>> +     clk_put(mout_mpll);
>> +err_clk2:
>> +     clk_put(sclk);
>> +
>> +     return -EINVAL;
>> +}
>> +
>
> I'm not sure if mach-smdk*.c is the right place for the above code.
> IMHO all the code that configures very low level, board specific parameters
> (like clocks and their relations) should be performed in boot loarder.
>

I feel pushing the clock enabling into the bootloader,will create an
unneccesary dependcy for the lcd on the u-boot.

> (snipped)
>
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
>
>
>
regards
anand