From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [Linux-fbdev-devel] [PATCH 02/20] omapfb: Add support for MIPI-DCS compatible LCDs Date: Tue, 9 Jun 2009 14:15:22 +0300 Message-ID: <20090609111522.GA29478@localhost> References: <1244137965-8937-1-git-send-email-imre.deak@nokia.com> <20090608004324.6c5ecf17.krzysztof.h1@poczta.fm> Mime-Version: 1.0 Return-path: Content-Disposition: inline In-Reply-To: <20090608004324.6c5ecf17.krzysztof.h1@poczta.fm> Sender: linux-omap-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ext Krzysztof Helt Cc: Antonino Daplas , Juha Yrjola , Imre Deak , "linux-fbdev-devel@lists.sourceforge.net" , Tony Lindgren , "Valkeinen Tomi (Nokia-D/Helsinki)" , "linux-omap@vger.kernel.org" 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 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