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

[JinGoo Han <jg1.han@samsung.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 6516 bytes --]


Hi, Marek.
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Marek Szyprowski
> Sent: Wednesday, June 22, 2011 6:47 PM
> To: 'Jingoo Han'; 'Kukjin Kim'; 'Paul Mundt'; linux-samsung-
> soc@vger.kernel.org; linux-fbdev@vger.kernel.org; 'Jonghun Han'
> Cc: 'Anand Kumar N'; 'Thomas Abraham'; Sylwester Nawrocki; 'Kyungmin Park';
> 'Inki Dae'; 'ARM Linux'; 'Ben Dooks'; 'Jonghun Han'
> Subject: RE: [PATCH V5 5/5] ARM: EXYNOS4: Add platform data for EXYNOS4
> FIMD and LTE480WV platform-lcd
> 
> Hello,
> 
> On Wednesday, June 22, 2011 8:42 AM Jingoo Han wrote:
> 
> > From: Jonghun Han <jonghun.han@samsung.com>
> >
> > This patch adds support EXYNOS4 FIMD0 and LTE480WV LCD pannel.
> >
> > Signed-off-by: Jonghun Han <jonghun.han@samsung.com>
> > Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> > ---
> >  arch/arm/mach-exynos4/mach-smdkc210.c |  114
> > +++++++++++++++++++++++++++++++++
> >  arch/arm/mach-exynos4/mach-smdkv310.c |  114
> > +++++++++++++++++++++++++++++++++
> >  2 files changed, 228 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos4/mach-smdkc210.c b/arch/arm/mach-
> > exynos4/mach-smdkc210.c
> > index e645f7a..360a50a 100644
> > --- a/arch/arm/mach-exynos4/mach-smdkc210.c
> > +++ b/arch/arm/mach-exynos4/mach-smdkc210.c
> > @@ -9,26 +9,33 @@
> >  */
> >
> >  #include <linux/serial_core.h>
> > +#include <linux/delay.h>
> >  #include <linux/gpio.h>
> > +#include <linux/lcd.h>
> >  #include <linux/mmc/host.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/smsc911x.h>
> >  #include <linux/io.h>
> >  #include <linux/i2c.h>
> > +#include <linux/clk.h>
> >
> >  #include <asm/mach/arch.h>
> >  #include <asm/mach-types.h>
> >
> > +#include <video/platform_lcd.h>
> > +
> >  #include <plat/regs-serial.h>
> >  #include <plat/regs-srom.h>
> >  #include <plat/exynos4.h>
> >  #include <plat/cpu.h>
> >  #include <plat/devs.h>
> > +#include <plat/fb.h>
> >  #include <plat/sdhci.h>
> >  #include <plat/iic.h>
> >  #include <plat/pd.h>
> >
> >  #include <mach/map.h>
> > +#include <mach/regs-fb.h>
> >
> >  /* Following are default values for UCON, ULCON and UFCON UART
> registers
> > */
> >  #define SMDKC210_UCON_DEFAULT	(S3C2410_UCON_TXILEVEL |	\
> > @@ -111,6 +118,69 @@ static struct s3c_sdhci_platdata
> smdkc210_hsmmc3_pdata
> > __initdata = {
> >  	.clk_type		= S3C_SDHCI_CLK_DIV_EXTERNAL,
> >  };
> >
> > +static void lcd_lte480wv_set_power(struct plat_lcd_data *pd,
> > +				   unsigned int power)
> > +{
> > +	if (power) {
> > +#if !defined(CONFIG_BACKLIGHT_PWM)
> > +		gpio_request(EXYNOS4_GPD0(1), "GPD0");
> > +		gpio_direction_output(EXYNOS4_GPD0(1), 1);
> > +		gpio_free(EXYNOS4_GPD0(1));
> > +#endif
> > +		/* fire nRESET on power up */
> > +		gpio_request(EXYNOS4_GPX0(6), "GPX0");
> > +
> > +		gpio_direction_output(EXYNOS4_GPX0(6), 1);
> > +		mdelay(100);
> > +
> > +		gpio_set_value(EXYNOS4_GPX0(6), 0);
> > +		mdelay(10);
> > +
> > +		gpio_set_value(EXYNOS4_GPX0(6), 1);
> > +		mdelay(10);
> > +
> > +		gpio_free(EXYNOS4_GPX0(6));
> > +	} else {
> > +#if !defined(CONFIG_BACKLIGHT_PWM)
> > +		gpio_request(EXYNOS4_GPD0(1), "GPD0");
> > +		gpio_direction_output(EXYNOS4_GPD0(1), 0);
> > +		gpio_free(EXYNOS4_GPD0(1));
> > +#endif
> > +	}
> > +}
> > +
> > +static struct plat_lcd_data smdkc210_lcd_lte480wv_data = {
> > +	.set_power      = lcd_lte480wv_set_power,
> > +};
> > +
> > +static struct platform_device smdkc210_lcd_lte480wv = {
> > +	.name                   = "platform-lcd",
> > +	.dev.parent             = &s5p_device_fimd0.dev,
> > +	.dev.platform_data      = &smdkc210_lcd_lte480wv_data,
> > +};
> > +
> > +static struct s3c_fb_pd_win smdkc210_fb_win0 = {
> > +	.win_mode = {
> > +		.left_margin    = 13,
> > +		.right_margin   = 8,
> > +		.upper_margin   = 7,
> > +		.lower_margin   = 5,
> > +		.hsync_len      = 3,
> > +		.vsync_len      = 1,
> > +		.xres   = 800,
> > +		.yres   = 480,
> > +	},
> > +	.max_bpp        = 32,
> > +	.default_bpp    = 24,
> > +};
> > +
> > +static struct s3c_fb_platdata smdkc210_lcd0_pdata __initdata = {
> > +	.win[0]		= &smdkc210_fb_win0,
> > +	.vidcon0	= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB,
> > +	.vidcon1	= VIDCON1_INV_HSYNC | VIDCON1_INV_VSYNC,
> > +	.setup_gpio	= exynos4_fimd0_gpio_setup_24bpp,
> > +};
> > +
> >  static struct resource smdkc210_smsc911x_resources[] = {
> >  	[0] = {
> >  		.start	= EXYNOS4_PA_SROM_BANK(1),
> > @@ -165,6 +235,8 @@ static struct platform_device *smdkc210_devices[]
> > __initdata = {
> >  	&exynos4_device_pd[PD_GPS],
> >  	&exynos4_device_sysmmu,
> >  	&samsung_asoc_dma,
> > +	&s5p_device_fimd0,
> > +	&smdkc210_lcd_lte480wv,
> >  	&smdkc210_smsc911x,
> >  };
> >
> > @@ -191,6 +263,44 @@ static void __init smdkc210_smsc911x_init(void)
> >  		     (0x1 << S5P_SROM_BCX__TACS__SHIFT), S5P_SROM_BC1);
> >  }
> >
> > +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 agree with your suggestion.
I will remove the parent clock setting from the machine as fimc does.
I will send V7 patches, soon.

I really appreciate your review. Thank you.

> 
> (snipped)
> 
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±ýöÝzÿâžØ^n‡r¡ö¦zË\x1aëh™¨è­Ú&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~†­†Ûiÿÿïêÿ‘êçz_è®\x0fæj:+v‰¨þ)ߣøm