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 <imre.deak@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
