I addressed all of your comments in my next revision except the one below.

quoted

+     time_left = wait_for_completion_timeout(&bus->cmd_complete,
+                                             bus->adap.timeout);
+
+     spin_lock_irqsave(&bus->lock, flags);
+     bus->msgs = NULL;
+     if (time_left == 0)
+             ret = -ETIMEDOUT;
+     else
+             ret = bus->cmd_err;
+     spin_unlock_irqrestore(&bus->lock, flags);

I would make the interrupt handler self-clear msgs and I would copy
cmd_err to a separate field (or ensure it's only set by the interrupt
handler when msgs is non-NULL, by the completion code).

That way you avoid the above lock which is racy, slave activity could
get in there and trigger an error interrupt clobbering cmd_err for
example no ? Or am I missing something...

The slave handler does not touch these fields, so that should be fine.
The only way I can think
that we could get in this state is if we had some sort of error
interrupt fire after we handled a
STOP or previous error; this should not happen, but I think it is
better to be safe than sorry.
Nevertheless, I do not think it is necessary to do more than what I
have already done because
it would mean the bus is in a pretty bad state anyway. Maybe I should
just drop the locks here.

If you disagree, could you elaborate on what you meant by putting
cmd_err in a separate field?
Did you just mean having one for xfer and one for everything else (like resets)?

quoted

+     /* If nothing went wrong, return number of messages transferred. */
+     if (ret >= 0)
+             return bus->msgs_index + 1;
+     else
+             return ret;
+}
+

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