Hi, thanks for the patch. > >+/* capabilities */ > >+#define I2C_CAPABILITIES (I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | \ > >+ I2C_FUNC_SMBUS_READ_BLOCK_DATA) I don't see the need for a seperate define. > >+ > > struct davinci_i2c_dev { > > struct device *dev; > > void __iomem *base; > >@@ -318,7 +322,13 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg > >*msg, int stop) > > davinci_i2c_write_reg(dev, DAVINCI_I2C_SAR_REG, msg->addr); > > > > dev->buf = msg->buf; > >- dev->buf_len = msg->len; > >+ > >+ /* if first received byte is length, set buf_len = 0xffff as flag */ > >+ if (msg->flags & I2C_M_RECV_LEN) > >+ dev->buf_len = 0xffff; a) this magic value should be a define instead of a comment b) i2c messages easily have a 16 bit range, so 0xffff is a troublesome choice. > >+ else > >+ dev->buf_len = msg->len; > >+ > > dev->stop = stop; > > > > davinci_i2c_write_reg(dev, DAVINCI_I2C_CNT_REG, dev->buf_len); @@ -456,7 > >+466,7 @@ i2c_davinci_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > > > > static u32 i2c_davinci_func(struct i2c_adapter *adap) { > >- return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; > >+ return I2C_CAPABILITIES; > > } > > > > static void terminate_read(struct davinci_i2c_dev *dev) @@ -528,10 +538,32 @@ static > >irqreturn_t i2c_davinci_isr(int this_irq, void *dev_id) > > > > case DAVINCI_I2C_IVR_RDR: > > if (dev->buf_len) { > >- *dev->buf++ = > >- davinci_i2c_read_reg(dev, > >- DAVINCI_I2C_DRR_REG); > >+ *dev->buf++ = davinci_i2c_read_reg(dev, > >+ DAVINCI_I2C_DRR_REG); > >+ /* > >+ * check if the first received byte is message > >+ * length, i.e, I2C_M_RECV_LEN > >+ */ > >+ if (dev->buf_len == 0xffff) > >+ dev->buf_len = *(dev->buf - 1) + 1; Please rework the code to get rid of the '- 1' and '+ 1'. They look hackish and make the code less readable. > >+ > > dev->buf_len--; > >+ /* > >+ * send NACK/STOP bits BEFORE last byte is > >+ * received > >+ */ > >+ if (dev->buf_len == 1) { > >+ w = davinci_i2c_read_reg(dev, > >+ DAVINCI_I2C_MDR_REG); > >+ w |= DAVINCI_I2C_MDR_NACK; > >+ davinci_i2c_write_reg(dev, > >+ DAVINCI_I2C_MDR_REG, w); > >+ > >+ w |= DAVINCI_I2C_MDR_STP; > >+ davinci_i2c_write_reg(dev, > >+ DAVINCI_I2C_MDR_REG, w); > >+ } > >+ Looks like an unreleated change to me? Why is this I2C_M_RECV_LEN specific? Kind regards, Wolfram