Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote on Thu [2013-Aug-01 08:44:00 +0300]: > On 31/07/13 21:56, Etheridge, Darren wrote: > >>> > >>> +static int da8xxfb_set_par(struct fb_info *info) { > >>> + struct da8xx_fb_par *par = info->par; > >>> + int ret; > >>> + bool raster = da8xx_fb_is_raster_enabled(); > >>> + > >>> + if (raster) > >>> + lcd_disable_raster(true); > >>> + else > >>> + lcd_disable_raster(false); > >> > >> This looks odd. If raster is enabled, you disable it. And if raster is disabled, > >> you disable it. > > > > I corrected this one in patch 0011 - I agree this code is very confusing. > > In patch 11 you add the enum. I wasn't referring to that. My point was > that even if raster is already disabled, > lcd_disable_raster(dont-wait-for-framedone) is called. Agreed and removed, the lcd_disable_raster function does the same check as da8xx_fb_is_raster_enabled() and the immediately exits if not enabled so this path of the conditional appears completely redundant. Darren -- 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