Hi, Anand. > -----Original Message----- > From: linux-samsung-soc-owner@xxxxxxxxxxxxxxx [mailto:linux-samsung-soc- > owner@xxxxxxxxxxxxxxx] 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@xxxxxxxxxxxxxxx; > linux-fbdev@xxxxxxxxxxxxxxx; 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 > <m.szyprowski@xxxxxxxxxxx> wrote: > > Hello, > > > > On Wednesday, June 22, 2011 8:42 AM Jingoo Han wrote: > > > >> From: Jonghun Han <jonghun.han@xxxxxxxxxxx> > (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. > > >> +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 > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung- > soc" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-i ÿ淸º{.nÇ+돴윯돪†+%듚ÿ깁負¥Šwÿº{.nÇ+돴¥Š{깸튐꿴筬Êþ)í끾èw*jgП¨¶‰šŽ듶¢jÿ¾?G«앶ÿ◀◁¦j:+v돣ŠwèjØm¶Ÿÿ?®w?듺þf"·hš뤴얎ÿ녪¥