Hi, On Mon, Jun 08, 2009 at 12:43:24AM +0200, ext Krzysztof Helt wrote: > On Thu, 4 Jun 2009 20:52:27 +0300 > Imre Deak <[email protected]> wrote: >[...] > > + > > +#define to_mipid_device(p) container_of(p, struct mipid_device, \ > > + panel) > > +struct mipid_device { > > + int enabled; > > + int model; > > This one is only set and never read. A name is probably enough.
Ok, I'll remove model. > > > + int revision; > > + u8 display_id[3]; > > This one should be a local variable. Ok, I'll move it to the func where it's used. > > > + unsigned int saved_bklight_level; > > + unsigned long hw_guard_end; /* next value of jiffies > > + when we can issue the > > + next sleep in/out command */ > > + unsigned long hw_guard_wait; /* max guard time in jiffies */ > > + > > + struct omapfb_device *fbdev; > > + struct spi_device *spi; > > + struct mutex mutex; > > + struct lcd_panel panel; > > How does it differ from fbdev->panel? Is it duplicated field? fbdev->panel is a pointer to this device instance specific data. It's embedded here so that we can get to struct mipid_device with the container_of macro when fbdev->panel is passed to us. > > I am sorry but I had not enough time to review the rest. Thanks for the review, if there is nothing else I can post a new version with the above changes. --Imre -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
