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

Hi, Anand.

-----Original Message-----
From: linux-samsung-soc-owner@vger.kernel.org [mailto:linux-samsung-soc-
owner@vger.kernel.org] On Behalf Of Anand Kumar N
Sent: Wednesday, June 22, 2011 10:25 PM
To: Marek Szyprowski
Cc: Jingoo Han; Kukjin Kim; Paul Mundt; linux-samsung-soc@vger.kernel.org;
linux-fbdev@vger.kernel.org; Jonghun Han; Thomas Abraham; Sylwester
Nawrocki; Kyungmin Park; Inki Dae; ARM Linux; Ben Dooks
Subject: Re: [PATCH V5 5/5] ARM: EXYNOS4: Add platform data for EXYNOS4
FIMD and LTE480WV platform-lcd

Hi Marek/Jingoo,

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

quoted

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,
+};

As Sylwester Nawrocki mentioned, there would be criteria that should be considered
such as continuous clock availability or frequency stability. 
For instance, when EPLL '[7] = &clk_mout_epll.clk,' is selected, there is posibility
that EPLL frequency will be changed by other devices.
However, Exynos4 FIMD can use a range of clocks as source clock, 'mout_mpll' is
most useful  and can cover almost LCD pixel clock, because 'mout_mpll' is 800 MHz
and max frequency of 'sclk_fimd' can be set as 800MHz.

However, other clocks are very low. For example, HDMI PHY, VPLL are 54MHz, 
USB PHY is 48MHz. 

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

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

quoted

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

quoted

(snipped)

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

regards
anand
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-
soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-i

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±ıöİzÿâ�Ø^n‡r¡ö¦zË�ëh™¨è­Ú&£ûàz¿äz¹Ş—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿï�êÿ‘êçz_è®æj:+v‰¨ş)ߣøm