On Thu, 15 Dec 2011 17:27:53 +0000 Tabi Timur-B04825 <B04825@xxxxxxxxxxxxx> wrote: > On Fri, Jul 23, 2010 at 9:00 AM, Anatolij Gustschin <agust@xxxxxxx> wrote: > > > @@ -1471,7 +1476,9 @@ static int __devinit fsl_diu_probe(struct of_device *ofdev, > > goto error2; > > } > > > > - out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU anyway*/ > > + diu_mode = in_be32(&dr.diu_reg->diu_mode); > > + if (diu_mode != MFB_MODE1) > > + out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */ > > Anatolij, > > I know this patch is old, but I just noticed something odd about it > that I need your help with. > > In the above snippet, you test for != MFB_MODE1. My understanding is > that U-boot only supports modes 0 and 1, never modes 2 or 3. So > diu_mode can only ever be 0 or 1. That means that that the above code > is equivalent to: > > diu_mode = in_be32(&dr.diu_reg->diu_mode); > if (diu_mode == 0) > out_be32(&dr.diu_reg->diu_mode, 0); /* disable DIU */ > > which is silly, because now we're writing 0 to diu_mode only if it's already 0. > > Am I missing something? the intention of the above code snippet was: not to disable DIU if it is in mode 1 (displaying splash screen) and to disable DIU if it is in modes 2, 3 or 4 for some reason. We cannot guarantee that the DIU is not in these modes. Even if U-Boot didn't set these modes there is still a possibility that such mode is configured. E.g. I've seen U-Boot binary standalone applications for other display controllers initializing the display controller. But you are right. With this snippet, if the DIU is already disabled, there will be not needed mode register access. So the code should better look like: diu_mode = in_be32(&dr.diu_reg->diu_mode); if (diu_mode && diu_mode != MFB_MODE1) out_be32(&dr.diu_reg->diu_mode, 0); Anatolij -- 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