On Wednesday, August 08, 2012 9:15 PM Phil Edworthy wrote Hi Phil Edworthy, I reviewed your patch. Please refer to my comments. Good luck. Best regards, Jingoo Han > > The vdc4 display hardware is found on the sh7269 device. > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx> Please insert one line between the commit message and Signed-off-by. > --- > drivers/video/Kconfig | 10 + > drivers/video/Makefile | 1 + > drivers/video/ren_vdc4fb.c | 653 ++++++++++++++++++++++++++++++++++++++++++++ > include/video/ren_vdc4fb.h | 19 ++ > 4 files changed, 683 insertions(+), 0 deletions(-) > create mode 100644 drivers/video/ren_vdc4fb.c > create mode 100644 include/video/ren_vdc4fb.h > > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > index 0217f74..89c9250 100644 > --- a/drivers/video/Kconfig > +++ b/drivers/video/Kconfig > @@ -1990,6 +1990,16 @@ config FB_W100 > > If unsure, say N. > > +config FB_REN_VDC4FB > + tristate "Renesas VDC4 framebuffer support" > + depends on FB && CPU_SUBTYPE_SH7269 > + select FB_SYS_FILLRECT > + select FB_SYS_COPYAREA > + select FB_SYS_IMAGEBLIT > + select FB_SYS_FOPS > + ---help--- > + Frame buffer driver for the Renesas VDC4. > + > config FB_SH_MOBILE_LCDC > tristate "SuperH Mobile LCDC framebuffer support" > depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK > diff --git a/drivers/video/Makefile b/drivers/video/Makefile > index ee8dafb..ba69fcb 100644 > --- a/drivers/video/Makefile > +++ b/drivers/video/Makefile > @@ -140,6 +140,7 @@ obj-$(CONFIG_SH_MIPI_DSI) += sh_mipi_dsi.o > obj-$(CONFIG_FB_SH_MOBILE_HDMI) += sh_mobile_hdmi.o > obj-$(CONFIG_FB_SH_MOBILE_MERAM) += sh_mobile_meram.o > obj-$(CONFIG_FB_SH_MOBILE_LCDC) += sh_mobile_lcdcfb.o > +obj-$(CONFIG_FB_REN_VDC4FB) += ren_vdc4fb.o > obj-$(CONFIG_FB_OMAP) += omap/ > obj-y += omap2/ > obj-$(CONFIG_XEN_FBDEV_FRONTEND) += xen-fbfront.o > diff --git a/drivers/video/ren_vdc4fb.c b/drivers/video/ren_vdc4fb.c > new file mode 100644 > index 0000000..1a31e85 > --- /dev/null > +++ b/drivers/video/ren_vdc4fb.c > @@ -0,0 +1,653 @@ > +/* > + * Renesas VDC4 Framebuffer > + * > + * Based on sh_mobile_lcdcfb.c > + * Copyright (c) 2012 Renesas Electronics Europe Ltd > + * > + * This file is subject to the terms and conditions of the GNU General Public > + * License. See the file "COPYING" in the main directory of this archive > + * for more details. > + */ > + > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/mm.h> > +#include <linux/clk.h> > +#include <linux/sh_clk.h> > +#include <linux/platform_device.h> > +#include <linux/dma-mapping.h> > +#include <linux/interrupt.h> > +#include <linux/vmalloc.h> > +#include <linux/module.h> > +#include <video/ren_vdc4fb.h> > + > +#define PALETTE_NR 16 > + > +struct ren_vdc4_priv { > + void __iomem *base; > + int irq; > + struct clk *dot_clk; > + struct clk *clk; > + struct fb_info *info; > + dma_addr_t dma_handle; > + struct ren_vdc4_info *cfg; > + u32 pseudo_palette[PALETTE_NR]; > +}; > + > +/* Register offsets/reading and writing functions */ > +enum { > + SCL0_UPDATE, SCL0_FRC1, SCL0_FRC2, SCL0_FRC3, > + SCL0_FRC4, SCL0_FRC5, SCL0_FRC6, SCL0_FRC7, > + SCL0_DS1, SCL0_US1, > + > + GR1_UPDATE, GR1_AB1, > + > + GR2_UPDATE, GR2_AB1, > + > + GR3_UPDATE, GR3_FLM_RD, GR3_FLM1, GR3_FLM2, > + GR3_FLM3, GR3_FLM4, GR3_FLM5, GR3_FLM6, GR3_AB1, > + GR3_AB2, GR3_AB3, GR3_AB4, GR3_AB5, GR3_AB6, > + GR3_AB7, GR3_AB8, GR3_AB9, GR3_AB10, GR3_AB11, > + GR3_BASE, GR3_CLUT_INT, GR3_MON, > + > + TCON_UPDATE, TCON_TIM, TCON_TIM_STVA1, TCON_TIM_STVA2, > + TCON_TIM_STVB1, TCON_TIM_STVB2, TCON_TIM_STH1, > + TCON_TIM_STH2, TCON_TIM_STB1, TCON_TIM_STB2, > + TCON_TIM_CPV1, TCON_TIM_CPV2, TCON_TIM_POLA1, > + TCON_TIM_POLA2, TCON_TIM_POLB1, TCON_TIM_POLB2, > + TCON_TIM_DE, > + > + OUT_UPDATE, OUT_SET, OUT_BRIGHT1, > + OUT_BRIGHT2, OUT_CONTRAST, OUT_PDTHA, OUT_CLK_PHASE, > + > + SYSCNT_INT1, SYSCNT_INT2, SYSCNT_INT3, SYSCNT_INT4, > + SYSCNT_PANEL_CLK, SYSCNT_CLUT > +}; > + > +static unsigned long vdc4_offsets[] = { > + [SCL0_UPDATE] = 0x0100, > + [SCL0_FRC1] = 0x0104, > + [SCL0_FRC2] = 0x0108, > + [SCL0_FRC3] = 0x010C, > + [SCL0_FRC4] = 0x0110, > + [SCL0_FRC5] = 0x0114, > + [SCL0_FRC6] = 0x0118, > + [SCL0_FRC7] = 0x011C, > + [SCL0_DS1] = 0x012C, > + [SCL0_US1] = 0x0148, > + [GR1_UPDATE] = 0x0200, > + [GR1_AB1] = 0x0220, > + [GR2_UPDATE] = 0x0300, > + [GR2_AB1] = 0x0320, > + [GR3_UPDATE] = 0x0380, > + [GR3_FLM_RD] = 0x0384, > + [GR3_FLM1] = 0x0388, > + [GR3_FLM2] = 0x038C, > + [GR3_FLM3] = 0x0390, > + [GR3_FLM4] = 0x0394, > + [GR3_FLM5] = 0x0398, > + [GR3_FLM6] = 0x039C, > + [GR3_AB1] = 0x03A0, > + [GR3_AB2] = 0x03A4, > + [GR3_AB3] = 0x03A8, > + [GR3_AB4] = 0x03AC, > + [GR3_AB5] = 0x03B0, > + [GR3_AB6] = 0x03B4, > + [GR3_AB7] = 0x03B8, > + [GR3_AB8] = 0x03BC, > + [GR3_AB9] = 0x03C0, > + [GR3_AB10] = 0x03C4, > + [GR3_AB11] = 0x03C8, > + [GR3_BASE] = 0x03CC, > + [GR3_CLUT_INT] = 0x03D0, > + [GR3_MON] = 0x03D4, > + [TCON_UPDATE] = 0x0580, > + [TCON_TIM] = 0x0584, > + [TCON_TIM_STVA1] = 0x0588, > + [TCON_TIM_STVA2] = 0x058C, > + [TCON_TIM_STVB1] = 0x0590, > + [TCON_TIM_STVB2] = 0x0594, > + [TCON_TIM_STH1] = 0x0598, > + [TCON_TIM_STH2] = 0x059C, > + [TCON_TIM_STB1] = 0x05A0, > + [TCON_TIM_STB2] = 0x05A4, > + [TCON_TIM_CPV1] = 0x05A8, > + [TCON_TIM_CPV2] = 0x05AC, > + [TCON_TIM_POLA1] = 0x05B0, > + [TCON_TIM_POLA2] = 0x05B4, > + [TCON_TIM_POLB1] = 0x05B8, > + [TCON_TIM_POLB2] = 0x05BC, > + [TCON_TIM_DE] = 0x05C0, > + [OUT_UPDATE] = 0x0600, > + [OUT_SET] = 0x0604, > + [OUT_BRIGHT1] = 0x0608, > + [OUT_BRIGHT2] = 0x060C, > + [OUT_CONTRAST] = 0x0610, > + [OUT_PDTHA] = 0x0614, > + [OUT_CLK_PHASE] = 0x0624, > + [SYSCNT_INT1] = 0x0680, > + [SYSCNT_INT2] = 0x0684, > + [SYSCNT_INT3] = 0x0688, > + [SYSCNT_INT4] = 0x068C, > + [SYSCNT_PANEL_CLK] = 0x0690, /* 16-bit */ > + [SYSCNT_CLUT] = 0x0692, /* 16-bit */ > +}; > + > +/* SYSCNT */ > +#define ICKEN (1 << 8) > + > +/* SCL Syncs */ > +#define FREE_RUN_VSYNC 0x0001 > + > +/* OUTPUT */ > +#define OUT_FMT_RGB666 (1 << 12) > + > +/* TCON Timings */ > +#define STVB_SEL_BITS 0x0007 > +#define STVB_HS_SEL 2 > + > +#define STH2_SEL_BITS 0x0007 > +#define STH2_DE_SEL 7 > + > +/* OUTCLK */ > +#define LCD_DATA_EDGE 0x0100 > +#define STVB_EDGE 0x0020 > +#define STH_EDGE 0x0010 > + > +/* SCL_UPDATE */ > +#define SCL0_UPDATE_BIT 0x0100 > +#define SCL0_VEN_BIT 0x0010 > + > +/* TCON_UPDATE */ > +#define TCON_VEN_BIT 0x0001 > + > +/* OUT_UPDATE */ > +#define OUTCNT_VEN_BIT 0x0001 > + > +/* GR_UPDATE */ > +#define P_VEN_UPDATE 0x0010 > +#define IBUS_VEN_UPDATE 0x0001 > + > +/* GR_AB1 */ > +#define DISPSEL_BCKGND 0x0000 > +#define DISPSEL_LOWER 0x0001 > +#define DISPSEL_CUR 0x0002 > + > +/* GR_FLM_RD */ > +#define FB_R_ENB 0x01 > + > + Please remove unnecessary line. > +static void vdc4_write(struct ren_vdc4_priv *priv, > + unsigned long reg_offs, unsigned long data) > +{ > + if ((SYSCNT_PANEL_CLK == reg_offs) || (SYSCNT_CLUT == reg_offs)) > + iowrite16(data, priv->base + vdc4_offsets[reg_offs]); > + else > + iowrite32(data, priv->base + vdc4_offsets[reg_offs]); > +} > + > +static unsigned long vdc4_read(struct ren_vdc4_priv *priv, > + unsigned long reg_offs) > +{ > + if ((SYSCNT_PANEL_CLK == reg_offs) || (SYSCNT_CLUT == reg_offs)) > + return ioread16(priv->base + vdc4_offsets[reg_offs]); > + else > + return ioread32(priv->base + vdc4_offsets[reg_offs]); > +} > + > +static irqreturn_t ren_vdc4_irq(int irq, void *data) > +{ > + /* Not currently implemented/used */ > + return IRQ_HANDLED; > +} > + > +static void lcd_clear_display(struct ren_vdc4_priv *priv) > +{ > + unsigned char *pdest; > + unsigned long size; > + > + pdest = (unsigned char *)priv->dma_handle; > + size = priv->cfg->lcd_cfg.xres * priv->cfg->lcd_cfg.yres * 2; > + > + memset(pdest, 0, size); > +} > + > +static void restart_tft_display(struct ren_vdc4_priv *priv, > + int clock_source) > +{ > + struct fb_videomode *lcd; > + unsigned long h; > + unsigned long v; > + unsigned long tmp; > + > + /* FB setup */ > + lcd = &priv->cfg->lcd_cfg; > + lcd_clear_display(priv); > + > + /* VDC clock Setup */ > + tmp = priv->cfg->clock_divider; > + tmp |= clock_source << 12; > + tmp |= ICKEN; > + vdc4_write(priv, SYSCNT_PANEL_CLK, tmp); > + > + /* Clear and Disable all interrupts */ > + vdc4_write(priv, SYSCNT_INT1, 0); > + vdc4_write(priv, SYSCNT_INT2, 0); > + vdc4_write(priv, SYSCNT_INT3, 0); > + vdc4_write(priv, SYSCNT_INT4, 0); > + > + /* Setup free-running syncs */ > + vdc4_write(priv, SCL0_FRC3, FREE_RUN_VSYNC); > + > + /* Disable scale up/down */ > + vdc4_write(priv, SCL0_DS1, 0); > + vdc4_write(priv, SCL0_US1, 0); > + > + /* Timing registers */ > + h = lcd->hsync_len + lcd->left_margin + lcd->xres + lcd->right_margin; > + v = lcd->vsync_len + lcd->upper_margin + lcd->yres + lcd->lower_margin; > + tmp = (v - 1) << 16; > + tmp |= h - 1; > + vdc4_write(priv, SCL0_FRC4, tmp); > + > + vdc4_write(priv, TCON_TIM, (((h - 1) / 2) << 16)); > + > + tmp = (lcd->vsync_len + lcd->upper_margin) << 16; > + tmp |= lcd->yres; > + vdc4_write(priv, SCL0_FRC6, tmp); > + vdc4_write(priv, TCON_TIM_STVB1, tmp); > + vdc4_write(priv, GR3_AB2, tmp); > + > + tmp = lcd->left_margin << 16; > + tmp |= lcd->xres; > + vdc4_write(priv, SCL0_FRC7, tmp); > + vdc4_write(priv, TCON_TIM_STB1, tmp); > + vdc4_write(priv, GR3_AB3, tmp); > + > + vdc4_write(priv, SCL0_FRC1, 0); > + vdc4_write(priv, SCL0_FRC2, 0); > + vdc4_write(priv, SCL0_FRC5, 0); > + > + /* Set output format */ > + vdc4_write(priv, OUT_SET, OUT_FMT_RGB666); > + > + /* STH TCON Timing */ > + tmp = priv->cfg->hs_pulse_width; > + tmp |= priv->cfg->hs_start_pos << 16; > + vdc4_write(priv, TCON_TIM_STH1, tmp); > + > + /* Setup STVB as HSYNC */ > + tmp = vdc4_read(priv, TCON_TIM_STVB2); > + tmp &= ~STVB_SEL_BITS; > + tmp |= STVB_HS_SEL; > + vdc4_write(priv, TCON_TIM_STVB2, tmp); > + > + tmp = vdc4_read(priv, OUT_CLK_PHASE); > + tmp &= ~STVB_EDGE; > + vdc4_write(priv, OUT_CLK_PHASE, tmp); > + > + /* Setup STH as DE */ > + tmp = vdc4_read(priv, TCON_TIM_STH2); > + tmp &= ~STH2_SEL_BITS; > + tmp |= STH2_DE_SEL; > + vdc4_write(priv, TCON_TIM_STH2, tmp); > + > + tmp = vdc4_read(priv, OUT_CLK_PHASE); > + tmp &= ~STH_EDGE; > + vdc4_write(priv, OUT_CLK_PHASE, tmp); > + > + /* Output clock rising edge */ > + tmp = vdc4_read(priv, OUT_CLK_PHASE); > + tmp &= ~LCD_DATA_EDGE; > + vdc4_write(priv, OUT_CLK_PHASE, tmp); > + > + /* Setup graphics buffers and update all registers */ > + vdc4_write(priv, GR1_AB1, DISPSEL_BCKGND); > + vdc4_write(priv, GR2_AB1, DISPSEL_LOWER); > + vdc4_write(priv, GR3_AB1, DISPSEL_CUR); > + > + /* Setup framebuffer base/output */ > + vdc4_write(priv, GR3_FLM_RD, FB_R_ENB); > + > + vdc4_write(priv, GR3_FLM2, (unsigned long)priv->info->screen_base); > + > + vdc4_write(priv, GR3_FLM3, (lcd->xres * 2) << 16); > + > + tmp = vdc4_read(priv, GR3_FLM5); > + tmp |= lcd->yres << 16; > + vdc4_write(priv, GR3_FLM5, tmp); > + > + tmp = lcd->xres << 16; > + vdc4_write(priv, GR3_FLM6, tmp); > + > + /* Apply all register settings */ > + vdc4_write(priv, SCL0_UPDATE, SCL0_VEN_BIT | SCL0_UPDATE_BIT); > + vdc4_write(priv, GR1_UPDATE, P_VEN_UPDATE); > + vdc4_write(priv, GR2_UPDATE, P_VEN_UPDATE); > + vdc4_write(priv, GR3_UPDATE, P_VEN_UPDATE | IBUS_VEN_UPDATE); > + vdc4_write(priv, OUT_UPDATE, OUTCNT_VEN_BIT); > + vdc4_write(priv, TCON_UPDATE, TCON_VEN_BIT); > +} > + > +static int ren_vdc4_setup_clocks(struct platform_device *pdev, > + int clock_source, > + struct ren_vdc4_priv *priv) > +{ > + priv->clk = clk_get(&pdev->dev, "vdc4"); How about using devm_clk_get() instead of kzalloc()? It makes the code simpler. > + if (IS_ERR(priv->clk)) { > + dev_err(&pdev->dev, "cannot get clock \"vdc4\"\n"); > + return PTR_ERR(priv->clk); > + } > + > + if (clock_source == VDC4_PERI_CLK) { > + priv->dot_clk = clk_get(&pdev->dev, "peripheral_clk"); How about using devm_clk_get() instead of kzalloc()? It makes the code simpler. > + if (IS_ERR(priv->dot_clk)) { > + dev_err(&pdev->dev, "cannot get peripheral clock\n"); > + clk_put(priv->clk); > + return PTR_ERR(priv->dot_clk); > + } > + } > + > + return 0; > +} > + > +static int ren_vdc4_setcolreg(u_int regno, > + u_int red, u_int green, u_int blue, > + u_int transp, struct fb_info *info) > +{ > + u32 *palette = info->pseudo_palette; > + > + if (regno >= PALETTE_NR) > + return -EINVAL; > + > + /* only FB_VISUAL_TRUECOLOR supported */ > + > + red >>= 16 - info->var.red.length; > + green >>= 16 - info->var.green.length; > + blue >>= 16 - info->var.blue.length; > + transp >>= 16 - info->var.transp.length; > + > + palette[regno] = (red << info->var.red.offset) | > + (green << info->var.green.offset) | > + (blue << info->var.blue.offset) | > + (transp << info->var.transp.offset); > + > + return 0; > +} > + > +static struct fb_fix_screeninfo ren_vdc4_fix = { > + .id = "Renesas VDC4FB", > + .type = FB_TYPE_PACKED_PIXELS, > + .visual = FB_VISUAL_TRUECOLOR, > + .accel = FB_ACCEL_NONE, > +}; > + > +static struct fb_ops ren_vdc4_ops = { > + .owner = THIS_MODULE, ^^^^^^^^ Please use tab instead of spaces. > + .fb_setcolreg = ren_vdc4_setcolreg, > + .fb_read = fb_sys_read, > + .fb_write = fb_sys_write, Same as above. > + .fb_fillrect = sys_fillrect, > + .fb_copyarea = sys_copyarea, > + .fb_imageblit = sys_imageblit, > +}; > + > +static int ren_vdc4_set_bpp(struct fb_var_screeninfo *var, int bpp) > +{ > + switch (bpp) { > + case 16: /* RGB 565 */ > + var->red.offset = 11; > + var->red.length = 5; > + var->green.offset = 5; > + var->green.length = 6; > + var->blue.offset = 0; > + var->blue.length = 5; > + var->transp.offset = 0; > + var->transp.length = 0; > + break; > + default: > + return -EINVAL; > + } > + > + var->bits_per_pixel = bpp; > + var->red.msb_right = 0; > + var->green.msb_right = 0; > + var->blue.msb_right = 0; > + var->transp.msb_right = 0; > + return 0; > +} > + > +/* PM Functions */ > +static int ren_vdc4_start(struct ren_vdc4_priv *priv, > + int clock_source) > +{ > + int ret; > + > + ret = clk_enable(priv->clk); > + if (ret < 0) > + return ret; > + > + if (priv->dot_clk) { > + ret = clk_enable(priv->dot_clk); > + if (ret < 0) > + return ret; > + } > + > + restart_tft_display(priv, clock_source); > + > + return ret; > +} > + > +static void ren_vdc4_stop(struct ren_vdc4_priv *priv) > +{ > + if (priv->dot_clk) > + clk_disable(priv->dot_clk); > + clk_disable(priv->clk); > +} > + > +static int ren_vdc4_suspend(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + > + ren_vdc4_stop(platform_get_drvdata(pdev)); > + return 0; > +} > + > +static int ren_vdc4_resume(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + struct ren_vdc4_info *pdata = pdev->dev.platform_data; > + > + return ren_vdc4_start(platform_get_drvdata(pdev), pdata->clock_source); > +} > + > +static const struct dev_pm_ops ren_vdc4_dev_pm_ops = { > + .suspend = ren_vdc4_suspend, > + .resume = ren_vdc4_resume, > +}; > + > +static int ren_vdc4_remove(struct platform_device *pdev); > + > +static int __devinit ren_vdc4_probe(struct platform_device *pdev) > +{ > + struct fb_info *info; > + struct ren_vdc4_priv *priv; > + struct ren_vdc4_info *pdata = pdev->dev.platform_data; > + struct resource *res; > + void *buf; > + int irq, error; > + > + if (!pdata) { > + dev_err(&pdev->dev, "no platform data defined\n"); > + return -EINVAL; > + } > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + irq = platform_get_irq(pdev, 0); > + if (!res || irq < 0) { > + dev_err(&pdev->dev, "cannot get platform resources\n"); > + return -ENOENT; > + } > + > + priv = kzalloc(sizeof(*priv), GFP_KERNEL); How about using devm_kzalloc() instead of kzalloc()? It makes the code simpler. > + if (!priv) { > + dev_err(&pdev->dev, "cannot allocate device data\n"); > + return -ENOMEM; > + } > + > + platform_set_drvdata(pdev, priv); > + > + error = request_irq(irq, ren_vdc4_irq, 0, dev_name(&pdev->dev), priv); How about using devm_request_irq()instead of request_irq()? It makes the code simpler. > + if (error) { > + dev_err(&pdev->dev, "unable to request irq\n"); > + goto err1; > + } > + > + priv->irq = irq; > + pdata = pdev->dev.platform_data; > + > + priv->cfg = pdata; > + > + error = ren_vdc4_setup_clocks(pdev, pdata->clock_source, priv); > + if (error) { > + dev_err(&pdev->dev, "unable to setup clocks\n"); > + goto err1; > + } > + > + priv->base = ioremap_nocache(res->start, resource_size(res)); How about using devm_ioremap_nocache() instead of ioremap_nocache()? It makes the code simpler. > + if (!priv->base) { > + dev_err(&pdev->dev, "unable to ioremap\n"); > + goto err1; > + } > + > + priv->info = framebuffer_alloc(0, &pdev->dev); > + if (!priv->info) { > + dev_err(&pdev->dev, "unable to allocate fb_info\n"); > + goto err1; > + } > + > + info = priv->info; > + info->fbops = &ren_vdc4_ops; > + info->var.xres = info->var.xres_virtual = pdata->lcd_cfg.xres; > + info->var.yres = info->var.yres_virtual = pdata->lcd_cfg.yres; > + info->var.width = pdata->panel_width; > + info->var.height = pdata->panel_height; > + info->var.activate = FB_ACTIVATE_NOW; > + info->pseudo_palette = priv->pseudo_palette; > + error = ren_vdc4_set_bpp(&info->var, pdata->bpp); > + if (error) > + goto err1; > + > + info->fix = ren_vdc4_fix; > + info->fix.line_length = pdata->lcd_cfg.xres * (pdata->bpp / 8); > + info->fix.smem_len = info->fix.line_length * pdata->lcd_cfg.yres; > + > + buf = dma_alloc_coherent(&pdev->dev, info->fix.smem_len, > + &priv->dma_handle, GFP_KERNEL); > + if (!buf) { > + dev_err(&pdev->dev, "unable to allocate buffer\n"); > + goto err1; > + } > + > + info->flags = FBINFO_FLAG_DEFAULT; > + > + error = fb_alloc_cmap(&info->cmap, PALETTE_NR, 0); > + if (error < 0) { > + dev_err(&pdev->dev, "unable to allocate cmap\n"); > + goto err1; > + } > + > + memset(buf, 0, info->fix.smem_len); > + info->fix.smem_start = priv->dma_handle; > + info->screen_base = buf; > + info->device = &pdev->dev; > + info->par = priv; > + > + if (error) > + goto err1; > + > + ren_vdc4_start(priv, pdata->clock_source); Return value should be checked as follow: + error = ren_vdc4_start(priv, pdata->clock_source); > + if (error) { > + dev_err(&pdev->dev, "unable to start hardware\n"); > + goto err1; > + } > + > + info = priv->info; > + > + error = register_framebuffer(info); > + if (error < 0) > + goto err1; > + > + dev_info(info->dev, > + "registered %s as %udx%ud %dbpp.\n", > + pdev->name, > + (int) pdata->lcd_cfg.xres, > + (int) pdata->lcd_cfg.yres, > + pdata->bpp); > + > + return 0; > + > +err1: > + ren_vdc4_remove(pdev); > + return error; > +} > + > +static int ren_vdc4_remove(struct platform_device *pdev) > +{ > + struct ren_vdc4_priv *priv = platform_get_drvdata(pdev); > + struct fb_info *info; > + > + if (priv->info->dev) > + unregister_framebuffer(priv->info); > + > + ren_vdc4_stop(priv); > + > + info = priv->info; > + > + if (!info || !info->device) { > + dev_err(&pdev->dev, "Failed to dealloc/release fb_info\n"); > + } else { > + fb_dealloc_cmap(&info->cmap); > + framebuffer_release(info); > + } > + > + if (priv->dot_clk) > + clk_put(priv->dot_clk); > + clk_put(priv->clk); If devm_clk_get() is used in probe(), this clk_put() is not needed. > + > + if (priv->base) > + iounmap(priv->base); If devm_ioremap_nocache()is used in probe(), this iounmap() is not needed. > + > + if (priv->irq) > + free_irq(priv->irq, priv); If devm_request_irq()is used in probe(), this free_irq() is not needed. > + > + kfree(priv); If devm_kzalloc() is used in probe(), this kfree() is not needed. > + return 0; > +} > + > +static struct platform_driver ren_vdc4_driver = { > + .driver = { > + .name = "ren_vdc4fb", > + .owner = THIS_MODULE, > + .pm = &ren_vdc4_dev_pm_ops, > + }, > + .probe = ren_vdc4_probe, > + .remove = ren_vdc4_remove, > +}; > + > +static int __init ren_vdc4_init(void) > +{ > + return platform_driver_register(&ren_vdc4_driver); > +} > + > +static void __exit ren_vdc4_exit(void) > +{ > + platform_driver_unregister(&ren_vdc4_driver); > +} > + > +module_init(ren_vdc4_init); > +module_exit(ren_vdc4_exit); > + > +MODULE_DESCRIPTION("Renesas VDC4 Framebuffer driver"); > +MODULE_AUTHOR("Phil Edworthy <phil.edworthy@xxxxxxxxxxx>"); > +MODULE_LICENSE("GPL v2"); > diff --git a/include/video/ren_vdc4fb.h b/include/video/ren_vdc4fb.h > new file mode 100644 > index 0000000..e91a515 > --- /dev/null > +++ b/include/video/ren_vdc4fb.h > @@ -0,0 +1,19 @@ > +#ifndef __REN_VDC4_H__ > +#define __REN_VDC4_H__ > + > +#include <linux/fb.h> > + > +enum { VDC4_EXTCLK = 1, VDC4_PERI_CLK }; > + > +struct ren_vdc4_info { > + int bpp; > + int clock_source; > + int clock_divider; > + int hs_pulse_width; > + int hs_start_pos; > + struct fb_videomode lcd_cfg; > + unsigned long panel_width; > + unsigned long panel_height; > +}; > + > +#endif > -- > 1.7.5.4 > > -- -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html