On Tue, 9 Jun 2009 14:15:22 +0300
Imre Deak [off-list ref] wrote:

Hi,

On Mon, Jun 08, 2009 at 12:43:24AM +0200, ext Krzysztof Helt wrote:

quoted

On Thu,  4 Jun 2009 20:52:27 +0300
Imre Deak [off-list ref] wrote:
[...]

quoted

+
+#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.

quoted

quoted

+	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.

quoted

quoted

+	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.

quoted

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