From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: [PATCH v4] davinci: fb: Frame Buffer driver for TI DA8xx/OMAP-L1xx Date: Fri, 24 Jul 2009 14:23:36 -0700 Message-ID: <20090724142336.2b9b4082.akpm@linux-foundation.org> References: <1248238020-8266-1-git-send-email-sudhakar.raj@ti.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1248238020-8266-1-git-send-email-sudhakar.raj@ti.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" Cc: linux-kernel@vger.kernel.org, linux-fbdev-devel@lists.sourceforge.net, davinci-linux-open-source@linux.davincidsp.com, krzysztof.h1@poczta.fm, sudhakar.raj@ti.com, pkiryukhin@ru.mvista.com, schen@mvista.com On Wed, 22 Jul 2009 00:47:00 -0400 Sudhakar Rajashekhara wrote: > Adds LCD controller (LCDC) driver for TI's DA8xx/OMAP-L1xx > architecture. LCDC specifications can be found at > http://www.ti.com/litv/pdf/sprufm0a. > > LCDC on DA8xx consists of two independent controllers, the > Raster Controller and the LCD Interface Display Driver (LIDD) > controller. LIDD further supports character and graphic displays. > > This patch adds support for the graphic display (Sharp LQ035Q3DG01) > found on the DA830 based EVM. The EVM details can be found at: > http://support.spectrumdigital.com/boards/dskda830/revc/. > > Signed-off-by: Sudhakar Rajashekhara > Signed-off-by: Pavel Kiryukhin > Signed-off-by: Steve Chen > Acked-by: Krzysztof Helt > --- > This patch applies to Linus's Kernel tree. > > Since the previous version, return values in ioctl() > function have been modified. That wasn't the only change in version 4: --- a/drivers/video/da8xx-fb.c~davinci-fb-frame-buffer-driver-for-ti-da8xx-omap-l1xx-v4 +++ a/drivers/video/da8xx-fb.c @@ -201,14 +201,13 @@ static int lcd_disable_raster(struct da8 ret = wait_event_interruptible_timeout(par->da8xx_wq, !lcdc_read(LCD_STAT_REG) & LCD_END_OF_FRAME0, WSI_TIMEOUT); + if (ret < 0) + return ret; + if (ret == 0) + ret = -ETIMEDOUT; } - if (ret < 0) - return ret; - if (ret == 0) - return -ETIMEDOUT; - - return 0; + return ret; } static void lcd_blit(int load_mode, struct da8xx_fb_par *par) @@ -634,11 +633,11 @@ static int fb_ioctl(struct fb_info *info case FBIPUT_BRIGHTNESS: case FBIGET_COLOR: case FBIPUT_COLOR: - return -EINVAL; + return -ENOTTY; case FBIPUT_HSYNC: if (copy_from_user(&sync_arg, (char *)arg, sizeof(struct lcd_sync_arg))) - return -EINVAL; + return -EFAULT; lcd_cfg_horizontal_sync(sync_arg.back_porch, sync_arg.pulse_width, sync_arg.front_porch); @@ -646,7 +645,7 @@ static int fb_ioctl(struct fb_info *info case FBIPUT_VSYNC: if (copy_from_user(&sync_arg, (char *)arg, sizeof(struct lcd_sync_arg))) - return -EINVAL; + return -EFAULT; lcd_cfg_vertical_sync(sync_arg.back_porch, sync_arg.pulse_width, sync_arg.front_porch); _ Was that intentional? The change to lcd_disable_raster() is a bit odd: static int lcd_disable_raster(struct da8xx_fb_par *par) { int ret = 0; u32 reg; reg = lcdc_read(LCD_RASTER_CTRL_REG); if (reg & LCD_RASTER_ENABLE) { lcdc_write(reg & ~LCD_RASTER_ENABLE, LCD_RASTER_CTRL_REG); ret = wait_event_interruptible_timeout(par->da8xx_wq, !lcdc_read(LCD_STAT_REG) & LCD_END_OF_FRAME0, WSI_TIMEOUT); if (ret < 0) return ret; if (ret == 0) ret = -ETIMEDOUT; } return ret; } We can do this, yes? --- a/drivers/video/da8xx-fb.c~davinci-fb-frame-buffer-driver-for-ti-da8xx-omap-l1xx-v4-cleanup +++ a/drivers/video/da8xx-fb.c @@ -201,8 +201,6 @@ static int lcd_disable_raster(struct da8 ret = wait_event_interruptible_timeout(par->da8xx_wq, !lcdc_read(LCD_STAT_REG) & LCD_END_OF_FRAME0, WSI_TIMEOUT); - if (ret < 0) - return ret; if (ret == 0) ret = -ETIMEDOUT; } _