Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling

On Thu, 21 Sep 2017 16:15:28 +0200
Wolfram Sang [off-list ref] wrote:

quoted

quoted

quoted

+/**
+ * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with i2c_msg
+ * @msg: the message to be synced with
+ * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
+ */
+void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
+{
+	if (!buf || buf == msg->buf)
+		return;
+
+	if (msg->flags & I2C_M_RD)
+		memcpy(msg->buf, buf, msg->len);
+
+	kfree(buf);  

Only free when you actually allocated it.  Seems to me like you need
to check if (!(msg->flags & I2C_M_DMA_SAFE)) before kfree.

Otherwise the logic to do this will be needed in every driver
which will get irritating fast.  

Well, I return early if (buf == msg->buf) which is only true for
I2C_M_DMA_SAFE. If not, I allocated the buffer. Am I missing something?
It would be very strange to call this function if the caller allocated
the buffer manually.

Thanks for the review!

Doh missed that check and my comment was bonkers even if it hadn't been there.
I come back to the claim of insufficient caffeine.

You are quite correct.  Please ignore previous comment - the code is
fine as is. 

Jonathan