On Fri, Feb 18, 2011 at 09:30:55AM +0100, Sascha Hauer wrote: > On Fri, Feb 18, 2011 at 01:24:20PM +0800, Shawn Guo wrote: > > Sorry, I did not catch up with v1 of the patch set. > > > > I have a overall comment on the driver. There is many occurrences > > of mx23, mx28 etc. throughout the file. IMHO, this is not a good > > idea. It may be better to use the IP version to handle the > > differences. In this way, if we have another SoC coming later > > using the same LCDIF revision as i.MX28. The driver could > > immediately fit in without code change, ideally. > > > > The only problem with version register is that the offset of LCDIF > > version register is different on i.MX28 from i.MX23. > > Can opener inside can. I love it ;) > Well, that's the situation we have to deal with. > > You still need > > cpu_is_mx23 to read the correct version. > > > > BTW, I would try to use cpu_is_mx23 than cpu_is_mx28, as the 'else' > > of cpu_is_mx23 could _possibly_ cover later SoC as well as i.MX28. > > There is only one cpu_is_mx28 in the driver without else. I can switch > MXSFB_MX23 and MXSFB_MX28 to MXSFB_V3 and MXSFB_V4 if you like it > better. > Yes, I like it better. With it, if we have a new SoC using LCDIF v4, the driver needs no change. -- Regards, Shawn -- 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
