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

Hi Marek/Jingoo,

On Wed, Jun 22, 2011 at 3:17 PM, Marek Szyprowski
[off-list ref] wrote:

Hello,

On Wednesday, June 22, 2011 8:42 AM Jingoo Han wrote:

quoted

From: Jonghun Han <redacted>

(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.

quoted

+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