On Tue, 9 Jun 2009 14:15:22 +0300
Imre Deak <imre.d...@nokia.com> wrote:

> 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.d...@nokia.com> 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.
> 

Please post the series after I review your last patch. It should not take 
longer than two days.

Regards,
Krzysztof

----------------------------------------------------------------------
Przekaz dalej wiadomosc: Zawsze warto oszczedzac. Teraz 5,5%!
Sprawdz > http://link.interia.pl/f21b1

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to