On Thu, Feb 25, 2010 at 12:07:38PM +0100, Jiri Kosina wrote:

On Wed, 24 Feb 2010, Bruno Prémont wrote:

quoted

+/* Update fb_vbitmap from the screen_base and send changed tiles to device */
+static void picolcd_fb_update(struct picolcd_data *data)
+{
+	int chip, tile;
+
+	spin_lock(&data->lock);
+	if (!(data->status & PICOLCD_READY_FB)) {
+		spin_unlock(&data->lock);
+		picolcd_fb_reset(data->hdev, 0);
+	} else
+		spin_unlock(&data->lock);

Please put the brackets to the else branch as well.

Well, it looks like it can be rewritten as:

	spin_lock(&data->lock);
	need_reset = !(data->status & PICOLCD_READY_FB);
	spin_unlock(&data->lock);

	if (need_reset)
		picolcd_fb_reset(data->hdev, 0);

Which plainly shows that the locking here is bogus.

-- 
Dmitry