On 2018-07-08 13:58, Peter Rosin wrote: > On 2018-07-07 11:29, Jun Gao wrote: >> From: Jun Gao <jun.gao@xxxxxxxxxxxx> >> >> This function is needed by i2c_get_dma_safe_msg_buf() potentially. >> It is used to free DMA safe buffer when DMA operation fails. >> >> Signed-off-by: Jun Gao <jun.gao@xxxxxxxxxxxx> >> --- >> drivers/i2c/i2c-core-base.c | 14 ++++++++++++++ >> include/linux/i2c.h | 1 + >> 2 files changed, 15 insertions(+) >> >> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c >> index 31d16ad..2b518ea 100644 >> --- a/drivers/i2c/i2c-core-base.c >> +++ b/drivers/i2c/i2c-core-base.c >> @@ -2288,6 +2288,20 @@ void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf) >> } >> EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_buf); >> >> +/** >> + * i2c_free_dma_safe_msg_buf - free DMA safe buffer >> + * @msg: the message related to DMA safe buffer >> + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL. >> + */ >> +void i2c_free_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf) >> +{ >> + if (!buf || buf == msg->buf) >> + return; >> + >> + kfree(buf); > > Considering that the i2c-core-smbus.c file does the following for its > DMA safe buffers... > > static void i2c_smbus_try_get_dmabuf(struct i2c_msg *msg, u8 init_val) > { > bool is_read = msg->flags & I2C_M_RD; > unsigned char *dma_buf; > > dma_buf = kzalloc(I2C_SMBUS_BLOCK_MAX + (is_read ? 2 : 3), GFP_KERNEL); > if (!dma_buf) > return; > > msg->buf = dma_buf; > msg->flags |= I2C_M_DMA_SAFE; > > if (init_val) > msg->buf[0] = init_val; > } > > ...I do not think your variant of i2c_release_dma_safe_msg_buf is > appropriate for the i2c-core-base.c file. It's simply not possible to have > central knowledge of the rules for when to free the buffer, and encoding one > set of rules is only confusing (when there are more than one set of rules). > I suggest that you make your variant driver specific. Ignore me. Your i2c_free_dma_safe_msg_buf is of course compatible with the existing i2c_smbus_try_get_dmabuf. Sorry for the noise. Cheers, Peter ( However, the naming of these two functions are not really consistent... ) > Cheers, > Peter > >> +} >> +EXPORT_SYMBOL_GPL(i2c_free_dma_safe_msg_buf); >> + >> MODULE_AUTHOR("Simon G. Vogl <simon@xxxxxxxxxxxxxxxxx>"); >> MODULE_DESCRIPTION("I2C-Bus main module"); >> MODULE_LICENSE("GPL"); >> diff --git a/include/linux/i2c.h b/include/linux/i2c.h >> index 254cd34..6d62f93 100644 >> --- a/include/linux/i2c.h >> +++ b/include/linux/i2c.h >> @@ -860,6 +860,7 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg) >> >> u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold); >> void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf); >> +void i2c_free_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf); >> >> int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr); >> /** >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html