* RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-10-18 12:55 ` [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD Sangbeom Kim
@ 2010-10-19 7:22 ` Marek Szyprowski
2010-10-21 3:45 ` Jonghun Han
2010-10-21 4:58 ` Inki Dae
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Marek Szyprowski @ 2010-10-19 7:22 UTC (permalink / raw)
To: linux-arm-kernel
1AFAAVgAzADEAMAAgAEY
ASQBNAEQA
x-cr-puzzleid: {C4CCB40E-D5C6-4119-AA7C-BBBA7A1503E4}
Hello,
On Monday, October 18, 2010 2:55 PM Sangbeom Kim wrote:
> From: Jonghun Han <jonghun.han@samsung.com>
>
> This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is added
> to distinguish clock type in structure s3c_fb_variant and lcd_clk is added
> in structure s3c_fb to calculate divider for lcd panel.
> FIMD can handles two clocks. The one is for FIMD IP and the other is for
> LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD IP
> clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
>
> Signed-off-by: Jonghun Han <jonghun.han@samsung.com>
> Reviewed-by: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
> Cc: Ben Dooks <ben-linux@fluff.org>
> ---
> NOTE: This patch is only for FIMD0.
> FIMD1 will be implemented later.
> drivers/video/Kconfig | 2 +-
> drivers/video/s3c-fb.c | 128 ++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 114 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 8b31fdf..3e2e02a 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -1946,7 +1946,7 @@ config FB_TMIO_ACCELL
>
> config FB_S3C
> tristate "Samsung S3C framebuffer support"
> - depends on FB && S3C_DEV_FB
> + depends on FB && (S3C_DEV_FB || S5P_DEV_FIMD0)
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index f9aca9d..bc182ea 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -65,6 +65,9 @@ struct s3c_fb;
> #define VIDOSD_C(win, variant) (OSD_BASE(win, variant) + 0x08)
> #define VIDOSD_D(win, variant) (OSD_BASE(win, variant) + 0x0C)
>
> +#define FIMD_CLK_TYPE0 0
> +#define FIMD_CLK_TYPE1 1
> +
> /**
> * struct s3c_fb_variant - fb variant information
> * @is_2443: Set if S3C2443/S3C2416 style hardware.
> @@ -97,6 +100,7 @@ struct s3c_fb_variant {
>
> unsigned int has_prtcon:1;
> unsigned int has_shadowcon:1;
> + unsigned int clk_type:1;
> };
>
> /**
> @@ -183,7 +187,8 @@ struct s3c_fb_vsync {
> * struct s3c_fb - overall hardware state of the hardware
> * @dev: The device that we bound to, for printing, etc.
> * @regs_res: The resource we claimed for the IO registers.
> - * @bus_clk: The clk (hclk) feeding our interface and possibly pixclk.
> + * @bus_clk: The clk (hclk) feeding FIMD IP core.
> + * @lcd_clk: The clk (sclk) feeding our interface and possibly pixclk.
> * @regs: The mapped hardware registers.
> * @variant: Variant information for this hardware.
> * @enabled: A bitmask of enabled hardware windows.
> @@ -197,6 +202,7 @@ struct s3c_fb {
> struct device *dev;
> struct resource *regs_res;
> struct clk *bus_clk;
> + struct clk *lcd_clk;
> void __iomem *regs;
> struct s3c_fb_variant variant;
>
> @@ -334,7 +340,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo *var,
> */
> static int s3c_fb_calc_pixclk(struct s3c_fb *sfb, unsigned int pixclk)
> {
> - unsigned long clk = clk_get_rate(sfb->bus_clk);
> + unsigned long clk = clk_get_rate(sfb->lcd_clk);
> unsigned long long tmp;
> unsigned int result;
>
> @@ -517,7 +523,7 @@ static int s3c_fb_set_par(struct fb_info *info)
>
> data = VIDTCON2_LINEVAL(var->yres - 1) |
> VIDTCON2_HOZVAL(var->xres - 1);
> - writel(data, regs +sfb->variant.vidtcon + 8 );
> + writel(data, regs + sfb->variant.vidtcon + 8);
> }
>
> /* write the buffer address */
> @@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> struct s3c_fb_platdata *pd;
> struct s3c_fb *sfb;
> struct resource *res;
> + struct clk *mout_mpll = NULL;
> int win;
> int ret = 0;
> + u32 rate = 134000000;
>
> fbdrv = (struct s3c_fb_driverdata *)platform_get_device_id(pdev)->driver_data;
>
> @@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> sfb->pdata = pd;
> sfb->variant = fbdrv->variant;
>
> - sfb->bus_clk = clk_get(dev, "lcd");
> - if (IS_ERR(sfb->bus_clk)) {
> - dev_err(dev, "failed to get bus clock\n");
> + switch (sfb->variant.clk_type) {
> + case FIMD_CLK_TYPE0:
> + sfb->bus_clk = clk_get(dev, "lcd");
> + if (IS_ERR(sfb->bus_clk)) {
> + dev_err(dev, "failed to get bus clock\n");
> + goto err_sfb;
> + }
> +
> + clk_enable(sfb->bus_clk);
> +
> + sfb->lcd_clk = sfb->bus_clk;
> + break;
> +
> + case FIMD_CLK_TYPE1:
> + sfb->bus_clk = clk_get(&pdev->dev, "fimd");
> + if (IS_ERR(sfb->bus_clk)) {
> + dev_err(&pdev->dev, "failed to get clock for fimd\n");
> + goto err_sfb;
> + }
> + clk_enable(sfb->bus_clk);
> +
> + sfb->lcd_clk = clk_get(&pdev->dev, "sclk_fimd");
> + if (IS_ERR(sfb->lcd_clk)) {
> + dev_err(&pdev->dev, "failed to get sclk for fimd\n");
> + goto err_bus_clk;
> + }
> +
> + mout_mpll = clk_get(&pdev->dev, "mout_mpll");
> + if (IS_ERR(mout_mpll)) {
> + dev_err(&pdev->dev, "failed to get mout_mpll\n");
> + goto err_lcd_clk;
> + }
> + clk_set_parent(sfb->lcd_clk, mout_mpll);
> + clk_put(mout_mpll);
I don't think that the driver is the right place to change the parent of the sclk_fimd
clock. It should be done in the boot loader or the board startup code.
We should also be a bit more consistent on clock naming. s3c6410..s5pv210 used the 'lcd'
clock name. Maybe you should also provide a patch to rename all these clocks to common
name.
> +
> + clk_set_rate(sfb->lcd_clk, rate);
> + clk_enable(sfb->lcd_clk);
> + dev_dbg(&pdev->dev, "set fimd sclk rate to %d\n", rate);
> + break;
> +
> + default:
> + dev_err(dev, "failed to enable clock for FIMD\n");
> goto err_sfb;
> }
>
> - clk_enable(sfb->bus_clk);
> -
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> dev_err(dev, "failed to find registers\n");
> ret = -ENOENT;
> - goto err_clk;
> + goto err_lcd_clk;
> }
>
> sfb->regs_res = request_mem_region(res->start, resource_size(res),
> @@ -1334,7 +1379,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)
> if (!sfb->regs_res) {
> dev_err(dev, "failed to claim register region\n");
> ret = -ENOENT;
> - goto err_clk;
> + goto err_lcd_clk;
> }
>
> sfb->regs = ioremap(res->start, resource_size(res));
> @@ -1413,9 +1458,15 @@ err_req_region:
> release_resource(sfb->regs_res);
> kfree(sfb->regs_res);
>
> -err_clk:
> - clk_disable(sfb->bus_clk);
> - clk_put(sfb->bus_clk);
> +err_lcd_clk:
> + clk_disable(sfb->lcd_clk);
> + clk_put(sfb->lcd_clk);
> +
> +err_bus_clk:
> + if (sfb->variant.clk_type != FIMD_CLK_TYPE0) {
> + clk_disable(sfb->bus_clk);
> + clk_put(sfb->bus_clk);
> + }
>
> err_sfb:
> kfree(sfb);
> @@ -1442,8 +1493,20 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)
>
> iounmap(sfb->regs);
>
> - clk_disable(sfb->bus_clk);
> - clk_put(sfb->bus_clk);
> + switch (sfb->variant.clk_type) {
> + case FIMD_CLK_TYPE1:
> + clk_disable(sfb->lcd_clk);
> + clk_put(sfb->lcd_clk);
> + /* fall through to default case */
> + case FIMD_CLK_TYPE0:
> + clk_disable(sfb->bus_clk);
> + clk_put(sfb->bus_clk);
> + break;
> + default:
> + dev_err(sfb->dev, "invalid clock type(%d)\n",
> + sfb->variant.clk_type);
> + break;
> + }
>
> release_resource(sfb->regs_res);
> kfree(sfb->regs_res);
> @@ -1470,6 +1533,7 @@ static int s3c_fb_suspend(struct platform_device *pdev, pm_message_t state)
> }
>
> clk_disable(sfb->bus_clk);
> +
> return 0;
> }
>
> @@ -1656,6 +1720,37 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 = {
> .win[4] = &s3c_fb_data_64xx_wins[4],
> };
>
> +static struct s3c_fb_driverdata s3c_fb_data_s5pv310 = {
> + .variant = {
> + .nr_windows = 5,
> + .vidtcon = VIDTCON0,
> + .wincon = WINCON(0),
> + .winmap = WINxMAP(0),
> + .keycon = WKEYCON,
> + .osd = VIDOSD_BASE,
> + .osd_stride = 16,
> + .buf_start = VIDW_BUF_START(0),
> + .buf_size = VIDW_BUF_SIZE(0),
> + .buf_end = VIDW_BUF_END(0),
> +
> + .palette = {
> + [0] = 0x2400,
> + [1] = 0x2800,
> + [2] = 0x2c00,
> + [3] = 0x3000,
> + [4] = 0x3400,
> + },
> +
> + .has_shadowcon = 1,
> + .clk_type = FIMD_CLK_TYPE1,
> + },
> + .win[0] = &s3c_fb_data_64xx_wins[0],
> + .win[1] = &s3c_fb_data_64xx_wins[1],
> + .win[2] = &s3c_fb_data_64xx_wins[2],
> + .win[3] = &s3c_fb_data_64xx_wins[3],
> + .win[4] = &s3c_fb_data_64xx_wins[4],
> +};
> +
> /* S3C2443/S3C2416 style hardware */
> static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
> .variant = {
> @@ -1703,6 +1798,9 @@ static struct platform_device_id s3c_fb_driver_ids[] = {
> .name = "s5pv210-fb",
> .driver_data = (unsigned long)&s3c_fb_data_s5pv210,
> }, {
> + .name = "s5pv310-fb",
> + .driver_data = (unsigned long)&s3c_fb_data_s5pv310,
> + }, {
> .name = "s3c2443-fb",
> .driver_data = (unsigned long)&s3c_fb_data_s3c2443,
> },
> --
Best regards
--
Marek Szyprowski
Samsung Poland R&D Center
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-10-19 7:22 ` Marek Szyprowski
@ 2010-10-21 3:45 ` Jonghun Han
0 siblings, 0 replies; 15+ messages in thread
From: Jonghun Han @ 2010-10-21 3:45 UTC (permalink / raw)
To: linux-arm-kernel
1AFAAVgAzADEAMAAgAEY
ASQBNAEQA
x-cr-puzzleid: {B7526269-A73C-412A-8DAE-B8EB603CF9D0}
Hi,
Marek Szyprowski wrote:
> -----Original Message-----
> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
> Sent: Tuesday, October 19, 2010 4:22 PM
> To: 'Sangbeom Kim'; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; linux-fbdev@vger.kernel.org
> Cc: 'Jonghun Han'; akpm@linux-foundation.org; kgene.kim@samsung.com; ben-
> linux@fluff.org; kyungmin.park@samsung.com
> Subject: RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
>
> 1AFAAVgAzADEAMAAgAEY
> ASQBNAEQA
> x-cr-puzzleid: {C4CCB40E-D5C6-4119-AA7C-BBBA7A1503E4}
>
> Hello,
>
> On Monday, October 18, 2010 2:55 PM Sangbeom Kim wrote:
>
> > From: Jonghun Han <jonghun.han@samsung.com>
> >
> > This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is
> added
> > to distinguish clock type in structure s3c_fb_variant and lcd_clk is
> added
> > in structure s3c_fb to calculate divider for lcd panel.
> > FIMD can handles two clocks. The one is for FIMD IP and the other is for
> > LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD IP
> > clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
> >
> > Signed-off-by: Jonghun Han <jonghun.han@samsung.com>
> > Reviewed-by: Kukjin Kim <kgene.kim@samsung.com>
> > Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
> > Cc: Ben Dooks <ben-linux@fluff.org>
> > ---
> > NOTE: This patch is only for FIMD0.
> > FIMD1 will be implemented later.
> > drivers/video/Kconfig | 2 +-
> > drivers/video/s3c-fb.c | 128
> ++++++++++++++++++++++++++++++++++++++++++------
> > 2 files changed, 114 insertions(+), 16 deletions(-)
> >
[snip]
> > /* write the buffer address */
> > @@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct
> platform_device *pdev)
> > struct s3c_fb_platdata *pd;
> > struct s3c_fb *sfb;
> > struct resource *res;
> > + struct clk *mout_mpll = NULL;
> > int win;
> > int ret = 0;
> > + u32 rate = 134000000;
> >
> > fbdrv = (struct s3c_fb_driverdata *)platform_get_device_id(pdev)-
> >driver_data;
> >
> > @@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct
> platform_device *pdev)
> > sfb->pdata = pd;
> > sfb->variant = fbdrv->variant;
> >
> > - sfb->bus_clk = clk_get(dev, "lcd");
> > - if (IS_ERR(sfb->bus_clk)) {
> > - dev_err(dev, "failed to get bus clock\n");
> > + switch (sfb->variant.clk_type) {
> > + case FIMD_CLK_TYPE0:
> > + sfb->bus_clk = clk_get(dev, "lcd");
> > + if (IS_ERR(sfb->bus_clk)) {
> > + dev_err(dev, "failed to get bus clock\n");
> > + goto err_sfb;
> > + }
> > +
> > + clk_enable(sfb->bus_clk);
> > +
> > + sfb->lcd_clk = sfb->bus_clk;
> > + break;
> > +
> > + case FIMD_CLK_TYPE1:
> > + sfb->bus_clk = clk_get(&pdev->dev, "fimd");
> > + if (IS_ERR(sfb->bus_clk)) {
> > + dev_err(&pdev->dev, "failed to get clock for
fimd\n");
> > + goto err_sfb;
> > + }
> > + clk_enable(sfb->bus_clk);
> > +
> > + sfb->lcd_clk = clk_get(&pdev->dev, "sclk_fimd");
> > + if (IS_ERR(sfb->lcd_clk)) {
> > + dev_err(&pdev->dev, "failed to get sclk for
fimd\n");
> > + goto err_bus_clk;
> > + }
> > +
> > + mout_mpll = clk_get(&pdev->dev, "mout_mpll");
> > + if (IS_ERR(mout_mpll)) {
> > + dev_err(&pdev->dev, "failed to get mout_mpll\n");
> > + goto err_lcd_clk;
> > + }
> > + clk_set_parent(sfb->lcd_clk, mout_mpll);
> > + clk_put(mout_mpll);
>
> I don't think that the driver is the right place to change the parent of
> the sclk_fimd
> clock. It should be done in the boot loader or the board startup code.
>
As you know, there are two clock sources for LCD pixel clock.
s3c-fb only used 'lcd' clock as a parent of LCD pixel clock.
But from S5PV310 SCLK_FIMD is only used as a source of LCD pixel clock.
And SCLK_FIMD is only for FIMD. So the parent of the SCLK_FIMD can be
selected in the driver.
In my opinion it doesn't matter.
> We should also be a bit more consistent on clock naming. s3c6410..s5pv210
> used the 'lcd'
> clock name. Maybe you should also provide a patch to rename all these
> clocks to common
> name.
>
Please refer to the following diagram.
Before S5PV310 clk 'lcd' was used for FIMD IP core clock and source of the
LCD pixel clock.
But the mux used to select source of LCD pixel clock is removed.
So 'lcd' clock is only used for core clock of FIMD IP. It isn't used for LCD
pixel clock.
As a result clock name was changed from lcd to fimd in the S5PV310
datasheet.
I agree to rearrange clock name later.
Before S5PV310
------------------------------------
dsys bus
----------------+-------------------
|
|1.clk 'lcd'
|
| FIMD block
+---+-----------+
4.mout_mpll |\ | | |
--------|m| | +-+-+ +----+ |
|u|-+ | | +-+core| |
|x| | | | +----+ |
|/ | | | |\ |
| | +-|m| +---+ |
| | |u|--+div| |
+------+---|x| +---+ |
2.SCLK_FIMD | |/ | |
| | |
+----------+----+
|
inside of SoC |
-----------------------+--------------------------
outside of SoC |
| 3.LCD pixel clock
|
+--------------+
| LCD module |
+--------------+
S5PV310
------------------------------------
dsys bus
----------------+-------------------
|
|1.clk 'fimd'
|
| FIMD block
+---+-----------+
4.mout_mpll |\ | | |
--------|m| | | +----+ |
|u|-+ | +---+core| |
|x| | | +----+ |
|/ | | |
| | +---+ |
| | +--+div| |
+------+-----+ +---+ |
2.SCLK_FIMD | | |
| | |
+----------+----+
|
inside of SoC |
-----------------------+--------------------------
outside of SoC |
| 3.LCD pixel clock
|
+--------------+
| LCD module |
+--------------+
> > +
> > + clk_set_rate(sfb->lcd_clk, rate);
> > + clk_enable(sfb->lcd_clk);
> > + dev_dbg(&pdev->dev, "set fimd sclk rate to %d\n", rate);
> > + break;
> > +
> > + default:
> > + dev_err(dev, "failed to enable clock for FIMD\n");
> > goto err_sfb;
> > }
> >
> > - clk_enable(sfb->bus_clk);
> > -
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > if (!res) {
> > dev_err(dev, "failed to find registers\n");
> > ret = -ENOENT;
> > - goto err_clk;
> > + goto err_lcd_clk;
> > }
> >
> > sfb->regs_res = request_mem_region(res->start, resource_size(res),
> > @@ -1334,7 +1379,7 @@ static int __devinit s3c_fb_probe(struct
> platform_device *pdev)
> > if (!sfb->regs_res) {
> > dev_err(dev, "failed to claim register region\n");
> > ret = -ENOENT;
> > - goto err_clk;
> > + goto err_lcd_clk;
> > }
> >
> > sfb->regs = ioremap(res->start, resource_size(res));
> > @@ -1413,9 +1458,15 @@ err_req_region:
> > release_resource(sfb->regs_res);
> > kfree(sfb->regs_res);
> >
> > -err_clk:
> > - clk_disable(sfb->bus_clk);
> > - clk_put(sfb->bus_clk);
> > +err_lcd_clk:
> > + clk_disable(sfb->lcd_clk);
> > + clk_put(sfb->lcd_clk);
> > +
> > +err_bus_clk:
> > + if (sfb->variant.clk_type != FIMD_CLK_TYPE0) {
> > + clk_disable(sfb->bus_clk);
> > + clk_put(sfb->bus_clk);
> > + }
> >
> > err_sfb:
> > kfree(sfb);
[snip]
BRs,
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-10-18 12:55 ` [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD Sangbeom Kim
2010-10-19 7:22 ` Marek Szyprowski
@ 2010-10-21 4:58 ` Inki Dae
2010-10-25 7:10 ` han jonghun
2010-11-12 5:26 ` Kukjin Kim
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Inki Dae @ 2010-10-21 4:58 UTC (permalink / raw)
To: linux-fbdev
Hello, Jonghun.
Below is my opinion.
Have a good time :)
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Jonghun Han
> Sent: Thursday, October 21, 2010 12:45 PM
> To: 'Marek Szyprowski'; 'Sangbeom Kim'; linux-arm-
> kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; linux-
> fbdev@vger.kernel.org
> Cc: akpm@linux-foundation.org; kgene.kim@samsung.com; ben-linux@fluff.org;
> kyungmin.park@samsung.com
> Subject: RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
>
> 1AFAAVgAzADEAMAAgAEY
> ASQBNAEQA
> x-cr-puzzleid: {B7526269-A73C-412A-8DAE-B8EB603CF9D0}
>
>
> Hi,
>
> Marek Szyprowski wrote:
>
> > -----Original Message-----
> > From: Marek Szyprowski [mailto:m.szyprowski@samsung.com]
> > Sent: Tuesday, October 19, 2010 4:22 PM
> > To: 'Sangbeom Kim'; linux-arm-kernel@lists.infradead.org; linux-samsung-
> > soc@vger.kernel.org; linux-fbdev@vger.kernel.org
> > Cc: 'Jonghun Han'; akpm@linux-foundation.org; kgene.kim@samsung.com;
> ben-
> > linux@fluff.org; kyungmin.park@samsung.com
> > Subject: RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
> >
> > 1AFAAVgAzADEAMAAgAEY
> > ASQBNAEQA
> > x-cr-puzzleid: {C4CCB40E-D5C6-4119-AA7C-BBBA7A1503E4}
> >
> > Hello,
> >
> > On Monday, October 18, 2010 2:55 PM Sangbeom Kim wrote:
> >
> > > From: Jonghun Han <jonghun.han@samsung.com>
> > >
> > > This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is
> > added
> > > to distinguish clock type in structure s3c_fb_variant and lcd_clk is
> > added
> > > in structure s3c_fb to calculate divider for lcd panel.
> > > FIMD can handles two clocks. The one is for FIMD IP and the other is
> for
> > > LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD
> IP
> > > clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
> > >
> > > Signed-off-by: Jonghun Han <jonghun.han@samsung.com>
> > > Reviewed-by: Kukjin Kim <kgene.kim@samsung.com>
> > > Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
> > > Cc: Ben Dooks <ben-linux@fluff.org>
> > > ---
> > > NOTE: This patch is only for FIMD0.
> > > FIMD1 will be implemented later.
> > > drivers/video/Kconfig | 2 +-
> > > drivers/video/s3c-fb.c | 128
> > ++++++++++++++++++++++++++++++++++++++++++------
> > > 2 files changed, 114 insertions(+), 16 deletions(-)
> > >
>
> [snip]
>
> > > /* write the buffer address */
> > > @@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct
> > platform_device *pdev)
> > > struct s3c_fb_platdata *pd;
> > > struct s3c_fb *sfb;
> > > struct resource *res;
> > > + struct clk *mout_mpll = NULL;
> > > int win;
> > > int ret = 0;
> > > + u32 rate = 134000000;
> > >
> > > fbdrv = (struct s3c_fb_driverdata *)platform_get_device_id(pdev)-
> > >driver_data;
> > >
> > > @@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct
> > platform_device *pdev)
> > > sfb->pdata = pd;
> > > sfb->variant = fbdrv->variant;
> > >
> > > - sfb->bus_clk = clk_get(dev, "lcd");
> > > - if (IS_ERR(sfb->bus_clk)) {
> > > - dev_err(dev, "failed to get bus clock\n");
> > > + switch (sfb->variant.clk_type) {
> > > + case FIMD_CLK_TYPE0:
> > > + sfb->bus_clk = clk_get(dev, "lcd");
> > > + if (IS_ERR(sfb->bus_clk)) {
> > > + dev_err(dev, "failed to get bus clock\n");
> > > + goto err_sfb;
> > > + }
> > > +
> > > + clk_enable(sfb->bus_clk);
> > > +
> > > + sfb->lcd_clk = sfb->bus_clk;
> > > + break;
> > > +
> > > + case FIMD_CLK_TYPE1:
> > > + sfb->bus_clk = clk_get(&pdev->dev, "fimd");
> > > + if (IS_ERR(sfb->bus_clk)) {
> > > + dev_err(&pdev->dev, "failed to get clock for
> fimd\n");
> > > + goto err_sfb;
> > > + }
> > > + clk_enable(sfb->bus_clk);
> > > +
> > > + sfb->lcd_clk = clk_get(&pdev->dev, "sclk_fimd");
> > > + if (IS_ERR(sfb->lcd_clk)) {
> > > + dev_err(&pdev->dev, "failed to get sclk for
> fimd\n");
> > > + goto err_bus_clk;
> > > + }
> > > +
> > > + mout_mpll = clk_get(&pdev->dev, "mout_mpll");
> > > + if (IS_ERR(mout_mpll)) {
> > > + dev_err(&pdev->dev, "failed to get mout_mpll\n");
> > > + goto err_lcd_clk;
> > > + }
> > > + clk_set_parent(sfb->lcd_clk, mout_mpll);
> > > + clk_put(mout_mpll);
> >
> > I don't think that the driver is the right place to change the parent of
> > the sclk_fimd
> > clock. It should be done in the boot loader or the board startup code.
> >
>
> As you know, there are two clock sources for LCD pixel clock.
> s3c-fb only used 'lcd' clock as a parent of LCD pixel clock.
> But from S5PV310 SCLK_FIMD is only used as a source of LCD pixel clock.
> And SCLK_FIMD is only for FIMD. So the parent of the SCLK_FIMD can be
> selected in the driver.
> In my opinion it doesn't matter.
>
Parent clock is specific to machine
because machines could have different type of lcd panel each other and
any parent clocks for FIMD could be used according to the lcd panel.
So for stable pixel clock setting, parent clock should be set
at machine specific code and FIMD driver should preserve just clock gating.
>
> > We should also be a bit more consistent on clock naming.
> s3c6410..s5pv210
> > used the 'lcd'
> > clock name. Maybe you should also provide a patch to rename all these
> > clocks to common
> > name.
> >
> Please refer to the following diagram.
> Before S5PV310 clk 'lcd' was used for FIMD IP core clock and source of the
> LCD pixel clock.
> But the mux used to select source of LCD pixel clock is removed.
> So 'lcd' clock is only used for core clock of FIMD IP. It isn't used for
> LCD
> pixel clock.
> As a result clock name was changed from lcd to fimd in the S5PV310
> datasheet.
> I agree to rearrange clock name later.
>
> Before S5PV310
> ------------------------------------
> dsys bus
> ----------------+-------------------
> |
> |1.clk 'lcd'
> |
> | FIMD block
> +---+-----------+
> 4.mout_mpll |\ | | |
> --------|m| | +-+-+ +----+ |
> |u|-+ | | +-+core| |
> |x| | | | +----+ |
> |/ | | | |\ |
> | | +-|m| +---+ |
> | | |u|--+div| |
> +------+---|x| +---+ |
> 2.SCLK_FIMD | |/ | |
> | | |
> +----------+----+
> |
> inside of SoC |
> -----------------------+--------------------------
> outside of SoC |
> | 3.LCD pixel clock
> |
> +--------------+
> | LCD module |
> +--------------+
>
>
> S5PV310
> ------------------------------------
> dsys bus
> ----------------+-------------------
> |
> |1.clk 'fimd'
> |
> | FIMD block
> +---+-----------+
> 4.mout_mpll |\ | | |
> --------|m| | | +----+ |
> |u|-+ | +---+core| |
> |x| | | +----+ |
> |/ | | |
> | | +---+ |
> | | +--+div| |
> +------+-----+ +---+ |
> 2.SCLK_FIMD | | |
> | | |
> +----------+----+
> |
> inside of SoC |
> -----------------------+--------------------------
> outside of SoC |
> | 3.LCD pixel clock
> |
> +--------------+
> | LCD module |
> +--------------+
>
> > > +
> > > + clk_set_rate(sfb->lcd_clk, rate);
> > > + clk_enable(sfb->lcd_clk);
> > > + dev_dbg(&pdev->dev, "set fimd sclk rate to %d\n", rate);
> > > + break;
> > > +
> > > + default:
> > > + dev_err(dev, "failed to enable clock for FIMD\n");
> > > goto err_sfb;
> > > }
> > >
> > > - clk_enable(sfb->bus_clk);
> > > -
> > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > if (!res) {
> > > dev_err(dev, "failed to find registers\n");
> > > ret = -ENOENT;
> > > - goto err_clk;
> > > + goto err_lcd_clk;
> > > }
> > >
> > > sfb->regs_res = request_mem_region(res->start, resource_size(res),
> > > @@ -1334,7 +1379,7 @@ static int __devinit s3c_fb_probe(struct
> > platform_device *pdev)
> > > if (!sfb->regs_res) {
> > > dev_err(dev, "failed to claim register region\n");
> > > ret = -ENOENT;
> > > - goto err_clk;
> > > + goto err_lcd_clk;
> > > }
> > >
> > > sfb->regs = ioremap(res->start, resource_size(res));
> > > @@ -1413,9 +1458,15 @@ err_req_region:
> > > release_resource(sfb->regs_res);
> > > kfree(sfb->regs_res);
> > >
> > > -err_clk:
> > > - clk_disable(sfb->bus_clk);
> > > - clk_put(sfb->bus_clk);
> > > +err_lcd_clk:
> > > + clk_disable(sfb->lcd_clk);
> > > + clk_put(sfb->lcd_clk);
> > > +
> > > +err_bus_clk:
> > > + if (sfb->variant.clk_type != FIMD_CLK_TYPE0) {
> > > + clk_disable(sfb->bus_clk);
> > > + clk_put(sfb->bus_clk);
> > > + }
> > >
> > > err_sfb:
> > > kfree(sfb);
> [snip]
>
> BRs,
>
>
> --
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-10-21 4:58 ` Inki Dae
@ 2010-10-25 7:10 ` han jonghun
0 siblings, 0 replies; 15+ messages in thread
From: han jonghun @ 2010-10-25 7:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
Thanks for comment.
[snip]
>> >
>> > I don't think that the driver is the right place to change the parent of
>> > the sclk_fimd
>> > clock. It should be done in the boot loader or the board startup code.
>> >
>>
>> As you know, there are two clock sources for LCD pixel clock.
>> s3c-fb only used 'lcd' clock as a parent of LCD pixel clock.
>> But from S5PV310 SCLK_FIMD is only used as a source of LCD pixel clock.
>> And SCLK_FIMD is only for FIMD. So the parent of the SCLK_FIMD can be
>> selected in the driver.
>> In my opinion it doesn't matter.
>>
>
> Parent clock is specific to machine
> because machines could have different type of lcd panel each other and
> any parent clocks for FIMD could be used according to the lcd panel.
> So for stable pixel clock setting, parent clock should be set
> at machine specific code and FIMD driver should preserve just clock gating.
>
Right. Parent clock is specific to machine.
But as you know, 'mout_mpll' can cover almost LCD pixel clock.
If other parent clock is needed, I'll get the parent clock or
separator via platform data from board file.
It is better than setting parent clock in board file at the point of
samsung mainline scheme.
BRs,
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-10-18 12:55 ` [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD Sangbeom Kim
2010-10-19 7:22 ` Marek Szyprowski
2010-10-21 4:58 ` Inki Dae
@ 2010-11-12 5:26 ` Kukjin Kim
2010-11-12 9:54 ` Inki Dae
2010-11-12 10:40 ` Inki Dae
4 siblings, 0 replies; 15+ messages in thread
From: Kukjin Kim @ 2010-11-12 5:26 UTC (permalink / raw)
To: linux-arm-kernel
Sangbeom Kim wrote:
>
> From: Jonghun Han <jonghun.han@samsung.com>
>
> This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is added
> to distinguish clock type in structure s3c_fb_variant and lcd_clk is added
> in structure s3c_fb to calculate divider for lcd panel.
> FIMD can handles two clocks. The one is for FIMD IP and the other is for
> LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD IP
> clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
>
> Signed-off-by: Jonghun Han <jonghun.han@samsung.com>
> Reviewed-by: Kukjin Kim <kgene.kim@samsung.com>
> Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
> Cc: Ben Dooks <ben-linux@fluff.org>
Hi Ben,
How do you think about this?
If you're ok, I'd like to send this to upstream via mmotm.
> ---
> NOTE: This patch is only for FIMD0.
> FIMD1 will be implemented later.
> drivers/video/Kconfig | 2 +-
> drivers/video/s3c-fb.c | 128
++++++++++++++++++++++++++++++++++++++++++----
> --
> 2 files changed, 114 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 8b31fdf..3e2e02a 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -1946,7 +1946,7 @@ config FB_TMIO_ACCELL
>
> config FB_S3C
> tristate "Samsung S3C framebuffer support"
> - depends on FB && S3C_DEV_FB
> + depends on FB && (S3C_DEV_FB || S5P_DEV_FIMD0)
> select FB_CFB_FILLRECT
> select FB_CFB_COPYAREA
> select FB_CFB_IMAGEBLIT
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index f9aca9d..bc182ea 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -65,6 +65,9 @@ struct s3c_fb;
> #define VIDOSD_C(win, variant) (OSD_BASE(win, variant) + 0x08)
> #define VIDOSD_D(win, variant) (OSD_BASE(win, variant) + 0x0C)
>
> +#define FIMD_CLK_TYPE0 0
> +#define FIMD_CLK_TYPE1 1
> +
> /**
> * struct s3c_fb_variant - fb variant information
> * @is_2443: Set if S3C2443/S3C2416 style hardware.
> @@ -97,6 +100,7 @@ struct s3c_fb_variant {
>
> unsigned int has_prtcon:1;
> unsigned int has_shadowcon:1;
> + unsigned int clk_type:1;
> };
>
> /**
> @@ -183,7 +187,8 @@ struct s3c_fb_vsync {
> * struct s3c_fb - overall hardware state of the hardware
> * @dev: The device that we bound to, for printing, etc.
> * @regs_res: The resource we claimed for the IO registers.
> - * @bus_clk: The clk (hclk) feeding our interface and possibly pixclk.
> + * @bus_clk: The clk (hclk) feeding FIMD IP core.
> + * @lcd_clk: The clk (sclk) feeding our interface and possibly pixclk.
> * @regs: The mapped hardware registers.
> * @variant: Variant information for this hardware.
> * @enabled: A bitmask of enabled hardware windows.
> @@ -197,6 +202,7 @@ struct s3c_fb {
> struct device *dev;
> struct resource *regs_res;
> struct clk *bus_clk;
> + struct clk *lcd_clk;
> void __iomem *regs;
> struct s3c_fb_variant variant;
>
> @@ -334,7 +340,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo
*var,
> */
> static int s3c_fb_calc_pixclk(struct s3c_fb *sfb, unsigned int pixclk)
> {
> - unsigned long clk = clk_get_rate(sfb->bus_clk);
> + unsigned long clk = clk_get_rate(sfb->lcd_clk);
> unsigned long long tmp;
> unsigned int result;
>
> @@ -517,7 +523,7 @@ static int s3c_fb_set_par(struct fb_info *info)
>
> data = VIDTCON2_LINEVAL(var->yres - 1) |
> VIDTCON2_HOZVAL(var->xres - 1);
> - writel(data, regs +sfb->variant.vidtcon + 8 );
> + writel(data, regs + sfb->variant.vidtcon + 8);
> }
>
> /* write the buffer address */
> @@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct
> platform_device *pdev)
> struct s3c_fb_platdata *pd;
> struct s3c_fb *sfb;
> struct resource *res;
> + struct clk *mout_mpll = NULL;
> int win;
> int ret = 0;
> + u32 rate = 134000000;
>
> fbdrv = (struct s3c_fb_driverdata *)platform_get_device_id(pdev)-
> >driver_data;
>
> @@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct
> platform_device *pdev)
> sfb->pdata = pd;
> sfb->variant = fbdrv->variant;
>
> - sfb->bus_clk = clk_get(dev, "lcd");
> - if (IS_ERR(sfb->bus_clk)) {
> - dev_err(dev, "failed to get bus clock\n");
> + switch (sfb->variant.clk_type) {
> + case FIMD_CLK_TYPE0:
> + sfb->bus_clk = clk_get(dev, "lcd");
> + if (IS_ERR(sfb->bus_clk)) {
> + dev_err(dev, "failed to get bus clock\n");
> + goto err_sfb;
> + }
> +
> + clk_enable(sfb->bus_clk);
> +
> + sfb->lcd_clk = sfb->bus_clk;
> + break;
> +
> + case FIMD_CLK_TYPE1:
> + sfb->bus_clk = clk_get(&pdev->dev, "fimd");
> + if (IS_ERR(sfb->bus_clk)) {
> + dev_err(&pdev->dev, "failed to get clock for
fimd\n");
> + goto err_sfb;
> + }
> + clk_enable(sfb->bus_clk);
> +
> + sfb->lcd_clk = clk_get(&pdev->dev, "sclk_fimd");
> + if (IS_ERR(sfb->lcd_clk)) {
> + dev_err(&pdev->dev, "failed to get sclk for
fimd\n");
> + goto err_bus_clk;
> + }
> +
> + mout_mpll = clk_get(&pdev->dev, "mout_mpll");
> + if (IS_ERR(mout_mpll)) {
> + dev_err(&pdev->dev, "failed to get mout_mpll\n");
> + goto err_lcd_clk;
> + }
> + clk_set_parent(sfb->lcd_clk, mout_mpll);
> + clk_put(mout_mpll);
> +
> + clk_set_rate(sfb->lcd_clk, rate);
> + clk_enable(sfb->lcd_clk);
> + dev_dbg(&pdev->dev, "set fimd sclk rate to %d\n", rate);
> + break;
> +
> + default:
> + dev_err(dev, "failed to enable clock for FIMD\n");
> goto err_sfb;
> }
>
> - clk_enable(sfb->bus_clk);
> -
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!res) {
> dev_err(dev, "failed to find registers\n");
> ret = -ENOENT;
> - goto err_clk;
> + goto err_lcd_clk;
> }
>
> sfb->regs_res = request_mem_region(res->start, resource_size(res),
> @@ -1334,7 +1379,7 @@ static int __devinit s3c_fb_probe(struct
> platform_device *pdev)
> if (!sfb->regs_res) {
> dev_err(dev, "failed to claim register region\n");
> ret = -ENOENT;
> - goto err_clk;
> + goto err_lcd_clk;
> }
>
> sfb->regs = ioremap(res->start, resource_size(res));
> @@ -1413,9 +1458,15 @@ err_req_region:
> release_resource(sfb->regs_res);
> kfree(sfb->regs_res);
>
> -err_clk:
> - clk_disable(sfb->bus_clk);
> - clk_put(sfb->bus_clk);
> +err_lcd_clk:
> + clk_disable(sfb->lcd_clk);
> + clk_put(sfb->lcd_clk);
> +
> +err_bus_clk:
> + if (sfb->variant.clk_type != FIMD_CLK_TYPE0) {
> + clk_disable(sfb->bus_clk);
> + clk_put(sfb->bus_clk);
> + }
>
> err_sfb:
> kfree(sfb);
> @@ -1442,8 +1493,20 @@ static int __devexit s3c_fb_remove(struct
> platform_device *pdev)
>
> iounmap(sfb->regs);
>
> - clk_disable(sfb->bus_clk);
> - clk_put(sfb->bus_clk);
> + switch (sfb->variant.clk_type) {
> + case FIMD_CLK_TYPE1:
> + clk_disable(sfb->lcd_clk);
> + clk_put(sfb->lcd_clk);
> + /* fall through to default case */
> + case FIMD_CLK_TYPE0:
> + clk_disable(sfb->bus_clk);
> + clk_put(sfb->bus_clk);
> + break;
> + default:
> + dev_err(sfb->dev, "invalid clock type(%d)\n",
> + sfb->variant.clk_type);
> + break;
> + }
>
> release_resource(sfb->regs_res);
> kfree(sfb->regs_res);
> @@ -1470,6 +1533,7 @@ static int s3c_fb_suspend(struct platform_device
*pdev,
> pm_message_t state)
> }
>
> clk_disable(sfb->bus_clk);
> +
> return 0;
> }
>
> @@ -1656,6 +1720,37 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210
> {
> .win[4] = &s3c_fb_data_64xx_wins[4],
> };
>
> +static struct s3c_fb_driverdata s3c_fb_data_s5pv310 = {
> + .variant = {
> + .nr_windows = 5,
> + .vidtcon = VIDTCON0,
> + .wincon = WINCON(0),
> + .winmap = WINxMAP(0),
> + .keycon = WKEYCON,
> + .osd = VIDOSD_BASE,
> + .osd_stride = 16,
> + .buf_start = VIDW_BUF_START(0),
> + .buf_size = VIDW_BUF_SIZE(0),
> + .buf_end = VIDW_BUF_END(0),
> +
> + .palette = {
> + [0] = 0x2400,
> + [1] = 0x2800,
> + [2] = 0x2c00,
> + [3] = 0x3000,
> + [4] = 0x3400,
> + },
> +
> + .has_shadowcon = 1,
> + .clk_type = FIMD_CLK_TYPE1,
> + },
> + .win[0] = &s3c_fb_data_64xx_wins[0],
> + .win[1] = &s3c_fb_data_64xx_wins[1],
> + .win[2] = &s3c_fb_data_64xx_wins[2],
> + .win[3] = &s3c_fb_data_64xx_wins[3],
> + .win[4] = &s3c_fb_data_64xx_wins[4],
> +};
> +
> /* S3C2443/S3C2416 style hardware */
> static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
> .variant = {
> @@ -1703,6 +1798,9 @@ static struct platform_device_id s3c_fb_driver_ids[]
> {
> .name = "s5pv210-fb",
> .driver_data = (unsigned long)&s3c_fb_data_s5pv210,
> }, {
> + .name = "s5pv310-fb",
> + .driver_data = (unsigned long)&s3c_fb_data_s5pv310,
> + }, {
> .name = "s3c2443-fb",
> .driver_data = (unsigned long)&s3c_fb_data_s3c2443,
> },
> --
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-10-18 12:55 ` [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD Sangbeom Kim
` (2 preceding siblings ...)
2010-11-12 5:26 ` Kukjin Kim
@ 2010-11-12 9:54 ` Inki Dae
2010-11-12 10:08 ` Paul Mundt
2010-11-12 10:40 ` Inki Dae
4 siblings, 1 reply; 15+ messages in thread
From: Inki Dae @ 2010-11-12 9:54 UTC (permalink / raw)
To: linux-fbdev
Hello, all.
This is my opinion and patch.
Jonghun's patch is adding clock type(FIMD_CLK_TYPE1 or FIMD_CLK_TYPE2) and
also
clk_type variable to s3c_fb_variant to select which clock would be used.
I think we could meet that simply if source clock and parent clock
names are set to platform data in machine file.
I added my comments to Jonghun's patch and attached modified patch below.
Thank you.
diff --git a/arch/arm/plat-samsung/include/plat/fb.h
b/arch/arm/plat-samsung/include/plat/fb.h
index ed70fc5..411380e 100644
--- a/arch/arm/plat-samsung/include/plat/fb.h
+++ b/arch/arm/plat-samsung/include/plat/fb.h
@@ -47,6 +47,10 @@ struct s3c_fb_pd_win {
* @vidcon1: The base vidcon1 values to control the panel data output.
* @win: The setup data for each hardware window, or NULL for unused.
* @display_mode: The LCD output display mode.
+ * @sclk_name: source clock name and sclk_name valiable should be set
+ * at machine specific file.
+ * @pclk_name: parent clock name and pclk_name valiable should be set
+ * at machine specific file.
*
* The platform data supplies the video driver with all the information
* it requires to work with the display(s) attached to the machine. It
@@ -63,6 +67,9 @@ struct s3c_fb_platdata {
u32 vidcon0;
u32 vidcon1;
+
+ const char *sclk_name;
+ const char *pclk_name;
};
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index f9aca9d..96a746c 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -27,6 +27,7 @@
#include <mach/map.h>
#include <plat/regs-fb-v4.h>
#include <plat/fb.h>
+#include <plat/clock.h>
/* This driver will export a number of framebuffer interfaces depending
* on the configuration passed in via the platform data. Each fb instance
@@ -183,7 +184,7 @@ struct s3c_fb_vsync {
* struct s3c_fb - overall hardware state of the hardware
* @dev: The device that we bound to, for printing, etc.
* @regs_res: The resource we claimed for the IO registers.
- * @bus_clk: The clk (hclk) feeding our interface and possibly pixclk.
+ * @lcd_clk: The clk (hclk or sclk) feeding our interface and possibly
pixclk.
* @regs: The mapped hardware registers.
* @variant: Variant information for this hardware.
* @enabled: A bitmask of enabled hardware windows.
@@ -196,7 +197,7 @@ struct s3c_fb_vsync {
struct s3c_fb {
struct device *dev;
struct resource *regs_res;
- struct clk *bus_clk;
+ struct clk *lcd_clk;
void __iomem *regs;
struct s3c_fb_variant variant;
@@ -334,7 +335,11 @@ static int s3c_fb_check_var(struct fb_var_screeninfo
*var,
*/
static int s3c_fb_calc_pixclk(struct s3c_fb *sfb, unsigned int pixclk)
{
- unsigned long clk = clk_get_rate(sfb->bus_clk);
+ /**
+ * if it fails to get clock rate from lcd_clk
+ * then would get it from parent clock of lcd_clk.
+ */
+ unsigned long clk = clk_get_rate(sfb->lcd_clk);
unsigned long long tmp;
unsigned int result;
@@ -1314,13 +1319,22 @@ static int __devinit s3c_fb_probe(struct
platform_device *pdev)
sfb->pdata = pd;
sfb->variant = fbdrv->variant;
- sfb->bus_clk = clk_get(dev, "lcd");
- if (IS_ERR(sfb->bus_clk)) {
- dev_err(dev, "failed to get bus clock\n");
+ /* if sclk_name is NULL then it would use bus clock as default. */
+ if (!pd->sclk_name)
+ sfb->lcd_clk = clk_get(dev, "lcd");
+ else
+ sfb->lcd_clk = clk_get(dev, pd->sclk_name);
+
+ if (IS_ERR(sfb->lcd_clk)) {
+ dev_err(dev, "failed to get lcd clock\n");
+ clk_put(sfb->lcd_clk);
goto err_sfb;
}
- clk_enable(sfb->bus_clk);
+ if (pd->pclk_name)
+ sfb->lcd_clk->parent = clk_get(dev, pd->pclk_name);
+
+ clk_enable(sfb->lcd_clk);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
@@ -1414,8 +1428,8 @@ err_req_region:
kfree(sfb->regs_res);
err_clk:
- clk_disable(sfb->bus_clk);
- clk_put(sfb->bus_clk);
+ clk_disable(sfb->lcd_clk);
+ clk_put(sfb->lcd_clk);
err_sfb:
kfree(sfb);
@@ -1442,8 +1456,8 @@ static int __devexit s3c_fb_remove(struct
platform_device *pdev)
iounmap(sfb->regs);
- clk_disable(sfb->bus_clk);
- clk_put(sfb->bus_clk);
+ clk_disable(sfb->lcd_clk);
+ clk_put(sfb->lcd_clk);
release_resource(sfb->regs_res);
kfree(sfb->regs_res);
@@ -1469,7 +1483,8 @@ static int s3c_fb_suspend(struct platform_device
*pdev, pm_message_t state)
s3c_fb_blank(FB_BLANK_POWERDOWN, win->fbinfo);
}
- clk_disable(sfb->bus_clk);
+ clk_disable(sfb->lcd_clk);
+
return 0;
}
@@ -1480,7 +1495,7 @@ static int s3c_fb_resume(struct platform_device *pdev)
struct s3c_fb_win *win;
int win_no;
- clk_enable(sfb->bus_clk);
+ clk_enable(sfb->lcd_clk);
/* setup registers */
writel(pd->vidcon1, sfb->regs + VIDCON1);
@@ -1656,6 +1671,36 @@ static struct s3c_fb_driverdata s3c_fb_data_s5pv210 {
.win[4] = &s3c_fb_data_64xx_wins[4],
};
+static struct s3c_fb_driverdata s3c_fb_data_s5pv310 = {
+ .variant = {
+ .nr_windows = 5,
+ .vidtcon = VIDTCON0,
+ .wincon = WINCON(0),
+ .winmap = WINxMAP(0),
+ .keycon = WKEYCON,
+ .osd = VIDOSD_BASE,
+ .osd_stride = 16,
+ .buf_start = VIDW_BUF_START(0),
+ .buf_size = VIDW_BUF_SIZE(0),
+ .buf_end = VIDW_BUF_END(0),
+
+ .palette = {
+ [0] = 0x2400,
+ [1] = 0x2800,
+ [2] = 0x2c00,
+ [3] = 0x3000,
+ [4] = 0x3400,
+ },
+
+ .has_shadowcon = 1,
+ },
+ .win[0] = &s3c_fb_data_64xx_wins[0],
+ .win[1] = &s3c_fb_data_64xx_wins[1],
+ .win[2] = &s3c_fb_data_64xx_wins[2],
+ .win[3] = &s3c_fb_data_64xx_wins[3],
+ .win[4] = &s3c_fb_data_64xx_wins[4],
+};
+
/* S3C2443/S3C2416 style hardware */
static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
.variant = {
@@ -1703,6 +1748,9 @@ static struct platform_device_id s3c_fb_driver_ids[] {
.name = "s5pv210-fb",
.driver_data = (unsigned long)&s3c_fb_data_s5pv210,
}, {
+ .name = "s5pv310-fb",
+ .driver_data = (unsigned long)&s3c_fb_data_s5pv310,
+ }, {
.name = "s3c2443-fb",
.driver_data = (unsigned long)&s3c_fb_data_s3c2443,
},
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Kukjin Kim
> Sent: Friday, November 12, 2010 2:26 PM
> To: 'Sangbeom Kim'; linux-arm-kernel@lists.infradead.org; linux-samsung-
> soc@vger.kernel.org; linux-fbdev@vger.kernel.org
> Cc: ben-linux@fluff.org; akpm@linux-foundation.org; 'Jonghun Han'
> Subject: RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
>
> Sangbeom Kim wrote:
> >
> > From: Jonghun Han <jonghun.han@samsung.com>
> >
> > This patch adds s3c_fb_driverdata for S5PV310 FIMD0. The clk_type is
> added
> > to distinguish clock type in structure s3c_fb_variant and lcd_clk is
> added
> > in structure s3c_fb to calculate divider for lcd panel.
> > FIMD can handles two clocks. The one is for FIMD IP and the other is for
> > LCD pixel clock. Before S5PV310 LCD pixel clock can be same with FIMD IP
> > clock. From S5PV310 LCD pixel clock is separated from FIMD IP clock.
> >
> > Signed-off-by: Jonghun Han <jonghun.han@samsung.com>
> > Reviewed-by: Kukjin Kim <kgene.kim@samsung.com>
> > Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
> > Cc: Ben Dooks <ben-linux@fluff.org>
>
> Hi Ben,
>
> How do you think about this?
> If you're ok, I'd like to send this to upstream via mmotm.
>
> > ---
> > NOTE: This patch is only for FIMD0.
> > FIMD1 will be implemented later.
> > drivers/video/Kconfig | 2 +-
> > drivers/video/s3c-fb.c | 128
> ++++++++++++++++++++++++++++++++++++++++++----
> > --
> > 2 files changed, 114 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 8b31fdf..3e2e02a 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -1946,7 +1946,7 @@ config FB_TMIO_ACCELL
> >
> > config FB_S3C
> > tristate "Samsung S3C framebuffer support"
> > - depends on FB && S3C_DEV_FB
> > + depends on FB && (S3C_DEV_FB || S5P_DEV_FIMD0)
> > select FB_CFB_FILLRECT
> > select FB_CFB_COPYAREA
> > select FB_CFB_IMAGEBLIT
> > diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> > index f9aca9d..bc182ea 100644
> > --- a/drivers/video/s3c-fb.c
> > +++ b/drivers/video/s3c-fb.c
> > @@ -65,6 +65,9 @@ struct s3c_fb;
> > #define VIDOSD_C(win, variant) (OSD_BASE(win, variant) + 0x08)
> > #define VIDOSD_D(win, variant) (OSD_BASE(win, variant) + 0x0C)
> >
> > +#define FIMD_CLK_TYPE0 0
> > +#define FIMD_CLK_TYPE1 1
> > +
this macro is unnecessary.
> > /**
> > * struct s3c_fb_variant - fb variant information
> > * @is_2443: Set if S3C2443/S3C2416 style hardware.
> > @@ -97,6 +100,7 @@ struct s3c_fb_variant {
> >
> > unsigned int has_prtcon:1;
> > unsigned int has_shadowcon:1;
> > + unsigned int clk_type:1;
> > };
> >
> > /**
> > @@ -183,7 +187,8 @@ struct s3c_fb_vsync {
> > * struct s3c_fb - overall hardware state of the hardware
> > * @dev: The device that we bound to, for printing, etc.
> > * @regs_res: The resource we claimed for the IO registers.
> > - * @bus_clk: The clk (hclk) feeding our interface and possibly pixclk.
> > + * @bus_clk: The clk (hclk) feeding FIMD IP core.
> > + * @lcd_clk: The clk (sclk) feeding our interface and possibly pixclk.
> > * @regs: The mapped hardware registers.
> > * @variant: Variant information for this hardware.
> > * @enabled: A bitmask of enabled hardware windows.
> > @@ -197,6 +202,7 @@ struct s3c_fb {
> > struct device *dev;
> > struct resource *regs_res;
> > struct clk *bus_clk;
> > + struct clk *lcd_clk;
> > void __iomem *regs;
> > struct s3c_fb_variant variant;
> >
bus_clk and lcd_clk could be integrated as lcd_clk.
and Ben, lcd_clk is better then bus_clk because hclk or sclk could be used
according to lcd panel on machine for stable use.
> > @@ -334,7 +340,7 @@ static int s3c_fb_check_var(struct fb_var_screeninfo
> *var,
> > */
> > static int s3c_fb_calc_pixclk(struct s3c_fb *sfb, unsigned int pixclk)
> > {
> > - unsigned long clk = clk_get_rate(sfb->bus_clk);
> > + unsigned long clk = clk_get_rate(sfb->lcd_clk);
> > unsigned long long tmp;
> > unsigned int result;
> >
> > @@ -517,7 +523,7 @@ static int s3c_fb_set_par(struct fb_info *info)
> >
> > data = VIDTCON2_LINEVAL(var->yres - 1) |
> > VIDTCON2_HOZVAL(var->xres - 1);
> > - writel(data, regs +sfb->variant.vidtcon + 8 );
> > + writel(data, regs + sfb->variant.vidtcon + 8);
> > }
> >
> > /* write the buffer address */
> > @@ -1286,8 +1292,10 @@ static int __devinit s3c_fb_probe(struct
> > platform_device *pdev)
> > struct s3c_fb_platdata *pd;
> > struct s3c_fb *sfb;
> > struct resource *res;
> > + struct clk *mout_mpll = NULL;
> > int win;
> > int ret = 0;
> > + u32 rate = 134000000;
> >
> > fbdrv = (struct s3c_fb_driverdata *)platform_get_device_id(pdev)-
> > >driver_data;
> >
> > @@ -1314,19 +1322,56 @@ static int __devinit s3c_fb_probe(struct
> > platform_device *pdev)
> > sfb->pdata = pd;
> > sfb->variant = fbdrv->variant;
> >
> > - sfb->bus_clk = clk_get(dev, "lcd");
> > - if (IS_ERR(sfb->bus_clk)) {
> > - dev_err(dev, "failed to get bus clock\n");
> > + switch (sfb->variant.clk_type) {
> > + case FIMD_CLK_TYPE0:
> > + sfb->bus_clk = clk_get(dev, "lcd");
> > + if (IS_ERR(sfb->bus_clk)) {
> > + dev_err(dev, "failed to get bus clock\n");
> > + goto err_sfb;
> > + }
> > +
> > + clk_enable(sfb->bus_clk);
> > +
> > + sfb->lcd_clk = sfb->bus_clk;
> > + break;
> > +
> > + case FIMD_CLK_TYPE1:
> > + sfb->bus_clk = clk_get(&pdev->dev, "fimd");
> > + if (IS_ERR(sfb->bus_clk)) {
> > + dev_err(&pdev->dev, "failed to get clock for
> fimd\n");
> > + goto err_sfb;
> > + }
> > + clk_enable(sfb->bus_clk);
> > +
> > + sfb->lcd_clk = clk_get(&pdev->dev, "sclk_fimd");
> > + if (IS_ERR(sfb->lcd_clk)) {
> > + dev_err(&pdev->dev, "failed to get sclk for
> fimd\n");
> > + goto err_bus_clk;
> > + }
> > +
> > + mout_mpll = clk_get(&pdev->dev, "mout_mpll");
> > + if (IS_ERR(mout_mpll)) {
> > + dev_err(&pdev->dev, "failed to get mout_mpll\n");
> > + goto err_lcd_clk;
> > + }
> > + clk_set_parent(sfb->lcd_clk, mout_mpll);
> > + clk_put(mout_mpll);
> > +
> > + clk_set_rate(sfb->lcd_clk, rate);
> > + clk_enable(sfb->lcd_clk);
> > + dev_dbg(&pdev->dev, "set fimd sclk rate to %d\n", rate);
> > + break;
> > +
> > + default:
> > + dev_err(dev, "failed to enable clock for FIMD\n");
> > goto err_sfb;
> > }
> >
This code above is unnecessary. if sclk_name of platform data is null then
it could get clock "lcd" otherwise sclk_name.
in case of "lcd", fimd would use bus clock as parent clock but
for sclk_name, sclk_fimd clock.
and
if (pd->pclk_name)
sfb->lcd_clk->parent = clk_get(dev, pd->pclk_name);
by adding the code above,
we could get appropriate clock rate through clk_get_rate(sfb->lcd_clk) call
at s3c_fb_calc_pixclk function.
(clk_get_rate function gets clock rate of parent if lcd_clk->rate is 0)
> > - clk_enable(sfb->bus_clk);
> > -
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > if (!res) {
> > dev_err(dev, "failed to find registers\n");
> > ret = -ENOENT;
> > - goto err_clk;
> > + goto err_lcd_clk;
> > }
> >
> > sfb->regs_res = request_mem_region(res->start, resource_size(res),
> > @@ -1334,7 +1379,7 @@ static int __devinit s3c_fb_probe(struct
> > platform_device *pdev)
> > if (!sfb->regs_res) {
> > dev_err(dev, "failed to claim register region\n");
> > ret = -ENOENT;
> > - goto err_clk;
> > + goto err_lcd_clk;
> > }
> >
> > sfb->regs = ioremap(res->start, resource_size(res));
> > @@ -1413,9 +1458,15 @@ err_req_region:
> > release_resource(sfb->regs_res);
> > kfree(sfb->regs_res);
> >
> > -err_clk:
> > - clk_disable(sfb->bus_clk);
> > - clk_put(sfb->bus_clk);
> > +err_lcd_clk:
> > + clk_disable(sfb->lcd_clk);
> > + clk_put(sfb->lcd_clk);
> > +
> > +err_bus_clk:
> > + if (sfb->variant.clk_type != FIMD_CLK_TYPE0) {
> > + clk_disable(sfb->bus_clk);
> > + clk_put(sfb->bus_clk);
> > + }
> >
> > err_sfb:
> > kfree(sfb);
> > @@ -1442,8 +1493,20 @@ static int __devexit s3c_fb_remove(struct
> > platform_device *pdev)
> >
> > iounmap(sfb->regs);
> >
> > - clk_disable(sfb->bus_clk);
> > - clk_put(sfb->bus_clk);
> > + switch (sfb->variant.clk_type) {
> > + case FIMD_CLK_TYPE1:
> > + clk_disable(sfb->lcd_clk);
> > + clk_put(sfb->lcd_clk);
> > + /* fall through to default case */
> > + case FIMD_CLK_TYPE0:
> > + clk_disable(sfb->bus_clk);
> > + clk_put(sfb->bus_clk);
> > + break;
> > + default:
> > + dev_err(sfb->dev, "invalid clock type(%d)\n",
> > + sfb->variant.clk_type);
> > + break;
> > + }
this code above is unnecessary anymore.
> >
> > release_resource(sfb->regs_res);
> > kfree(sfb->regs_res);
> > @@ -1470,6 +1533,7 @@ static int s3c_fb_suspend(struct platform_device
> *pdev,
> > pm_message_t state)
> > }
> >
> > clk_disable(sfb->bus_clk);
> > +
> > return 0;
> > }
> >
> > @@ -1656,6 +1720,37 @@ static struct s3c_fb_driverdata
> s3c_fb_data_s5pv210
> > > {
> > .win[4] = &s3c_fb_data_64xx_wins[4],
> > };
> >
> > +static struct s3c_fb_driverdata s3c_fb_data_s5pv310 = {
> > + .variant = {
> > + .nr_windows = 5,
> > + .vidtcon = VIDTCON0,
> > + .wincon = WINCON(0),
> > + .winmap = WINxMAP(0),
> > + .keycon = WKEYCON,
> > + .osd = VIDOSD_BASE,
> > + .osd_stride = 16,
> > + .buf_start = VIDW_BUF_START(0),
> > + .buf_size = VIDW_BUF_SIZE(0),
> > + .buf_end = VIDW_BUF_END(0),
> > +
> > + .palette = {
> > + [0] = 0x2400,
> > + [1] = 0x2800,
> > + [2] = 0x2c00,
> > + [3] = 0x3000,
> > + [4] = 0x3400,
> > + },
> > +
> > + .has_shadowcon = 1,
> > + .clk_type = FIMD_CLK_TYPE1,
clk_type is unnecessary anymore.
> > + },
> > + .win[0] = &s3c_fb_data_64xx_wins[0],
> > + .win[1] = &s3c_fb_data_64xx_wins[1],
> > + .win[2] = &s3c_fb_data_64xx_wins[2],
> > + .win[3] = &s3c_fb_data_64xx_wins[3],
> > + .win[4] = &s3c_fb_data_64xx_wins[4],
> > +};
> > +
> > /* S3C2443/S3C2416 style hardware */
> > static struct s3c_fb_driverdata s3c_fb_data_s3c2443 = {
> > .variant = {
> > @@ -1703,6 +1798,9 @@ static struct platform_device_id
> s3c_fb_driver_ids[]
> > > {
> > .name = "s5pv210-fb",
> > .driver_data = (unsigned long)&s3c_fb_data_s5pv210,
> > }, {
> > + .name = "s5pv310-fb",
> > + .driver_data = (unsigned long)&s3c_fb_data_s5pv310,
> > + }, {
> > .name = "s3c2443-fb",
> > .driver_data = (unsigned long)&s3c_fb_data_s3c2443,
> > },
> > --
>
>
> Thanks.
>
> Best regards,
> Kgene.
> --
> Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
> SW Solution Development Team, Samsung Electronics Co., Ltd.
>
> --
> 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-11-12 9:54 ` Inki Dae
@ 2010-11-12 10:08 ` Paul Mundt
0 siblings, 0 replies; 15+ messages in thread
From: Paul Mundt @ 2010-11-12 10:08 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 12, 2010 at 06:54:29PM +0900, Inki Dae wrote:
> @@ -1314,13 +1319,22 @@ static int __devinit s3c_fb_probe(struct
> platform_device *pdev)
> sfb->pdata = pd;
> sfb->variant = fbdrv->variant;
>
> - sfb->bus_clk = clk_get(dev, "lcd");
> - if (IS_ERR(sfb->bus_clk)) {
> - dev_err(dev, "failed to get bus clock\n");
> + /* if sclk_name is NULL then it would use bus clock as default. */
> + if (!pd->sclk_name)
> + sfb->lcd_clk = clk_get(dev, "lcd");
> + else
> + sfb->lcd_clk = clk_get(dev, pd->sclk_name);
> +
No, this is totally pointless. Simply establish a clk lookup for the
configuration you are running on in your board code and always have
clk_get() operate on a fixed id. You can use this same scheme with any
optional clocks you have too, and reconfigure according to whether the
lookups for optional clocks succeeded or not.
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-10-18 12:55 ` [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD Sangbeom Kim
` (3 preceding siblings ...)
2010-11-12 9:54 ` Inki Dae
@ 2010-11-12 10:40 ` Inki Dae
2010-11-12 10:43 ` Paul Mundt
4 siblings, 1 reply; 15+ messages in thread
From: Inki Dae @ 2010-11-12 10:40 UTC (permalink / raw)
To: linux-fbdev
> -----Original Message-----
> From: Paul Mundt [mailto:lethal@linux-sh.org]
> Sent: Friday, November 12, 2010 7:09 PM
> To: Inki Dae
> Cc: 'Kukjin Kim'; 'Sangbeom Kim'; linux-arm-kernel@lists.infradead.org;
> linux-samsung-soc@vger.kernel.org; linux-fbdev@vger.kernel.org; ben-
> linux@fluff.org; akpm@linux-foundation.org; 'Jonghun Han'
> Subject: Re: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
>
> On Fri, Nov 12, 2010 at 06:54:29PM +0900, Inki Dae wrote:
> > @@ -1314,13 +1319,22 @@ static int __devinit s3c_fb_probe(struct
> > platform_device *pdev)
> > sfb->pdata = pd;
> > sfb->variant = fbdrv->variant;
> >
> > - sfb->bus_clk = clk_get(dev, "lcd");
> > - if (IS_ERR(sfb->bus_clk)) {
> > - dev_err(dev, "failed to get bus clock\n");
> > + /* if sclk_name is NULL then it would use bus clock as default. */
> > + if (!pd->sclk_name)
> > + sfb->lcd_clk = clk_get(dev, "lcd");
> > + else
> > + sfb->lcd_clk = clk_get(dev, pd->sclk_name);
> > +
> No, this is totally pointless. Simply establish a clk lookup for the
> configuration you are running on in your board code
You mean, call the function at s3c_fb_probe() establishing clk lookup in
board code?
Sorry but I could understand. Please give me more detail.
> and always have
> clk_get() operate on a fixed id. You can use this same scheme with any
> optional clocks you have too, and reconfigure according to whether the
> lookups for optional clocks succeeded or not.
optional clocks could be sclk_name?
I will glad to show me some example for it.
thank you.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
2010-11-12 10:40 ` Inki Dae
@ 2010-11-12 10:43 ` Paul Mundt
0 siblings, 0 replies; 15+ messages in thread
From: Paul Mundt @ 2010-11-12 10:43 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Nov 12, 2010 at 07:40:16PM +0900, Inki Dae wrote:
>
> > -----Original Message-----
> > From: Paul Mundt [mailto:lethal@linux-sh.org]
> > Sent: Friday, November 12, 2010 7:09 PM
> > To: Inki Dae
> > Cc: 'Kukjin Kim'; 'Sangbeom Kim'; linux-arm-kernel@lists.infradead.org;
> > linux-samsung-soc@vger.kernel.org; linux-fbdev@vger.kernel.org; ben-
> > linux@fluff.org; akpm@linux-foundation.org; 'Jonghun Han'
> > Subject: Re: [PATCH 3/4] s3c-fb: Add support S5PV310 FIMD
> >
> > On Fri, Nov 12, 2010 at 06:54:29PM +0900, Inki Dae wrote:
> > > @@ -1314,13 +1319,22 @@ static int __devinit s3c_fb_probe(struct
> > > platform_device *pdev)
> > > sfb->pdata = pd;
> > > sfb->variant = fbdrv->variant;
> > >
> > > - sfb->bus_clk = clk_get(dev, "lcd");
> > > - if (IS_ERR(sfb->bus_clk)) {
> > > - dev_err(dev, "failed to get bus clock\n");
> > > + /* if sclk_name is NULL then it would use bus clock as default. */
> > > + if (!pd->sclk_name)
> > > + sfb->lcd_clk = clk_get(dev, "lcd");
> > > + else
> > > + sfb->lcd_clk = clk_get(dev, pd->sclk_name);
> > > +
> > No, this is totally pointless. Simply establish a clk lookup for the
> > configuration you are running on in your board code
> You mean, call the function at s3c_fb_probe() establishing clk lookup in
> board code?
> Sorry but I could understand. Please give me more detail.
>
I mean use a clkdev lookup for resolving the clock instead of passing
around a string via platform data. Look at the asm/clkdev.h API and all
of the boards that are using it.
^ permalink raw reply [flat|nested] 15+ messages in thread