[PATCH] i2c: davinci: Add block read functionality for IPMI

[PATCH] i2c: davinci: Add block read functionality for IPMI

[Murali Karicheri]

Intelligent Plaform Management Interface (IPMI) requires I2C driver
to support block read, where the first byte received from slave is
the length of following data:-
 Added length check if the read type is block read (I2C_M_RECV_LEN)
 Send NACK/STOP bits before last byte is received

Signed-off-by: Garrett Ding <g-ding-l0cyMroinI0@public.gmane.org>
Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
Tested-by: Garrett Ding <g-ding-l0cyMroinI0@public.gmane.org>
CC: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
CC: Kevin Hilman <khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
CC: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
CC: Santosh Shilimkar <santosh.shilimkar-l0cyMroinI0@public.gmane.org>
---
 Tested on a customer board based on K2HK SoC
 drivers/i2c/busses/i2c-davinci.c |   42 +++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 389bc68..cd97920 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -97,6 +97,10 @@
 #define DAVINCI_I2C_IMR_NACK	BIT(1)
 #define DAVINCI_I2C_IMR_AL	BIT(0)
 
+/* capabilities */
+#define I2C_CAPABILITIES	(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | \
+				 I2C_FUNC_SMBUS_READ_BLOCK_DATA)
+
 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;
+	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;
+
 				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);
+				}
+
 				if (dev->buf_len)
 					continue;
 
-- 
1.7.9.5

[PATCH] i2c: davinci: Add block read functionality for IPMI

[Murali Karicheri]

Intelligent Plaform Management Interface (IPMI) requires I2C driver
to support block read, where the first byte received from slave is
the length of following data:-
 Added length check if the read type is block read (I2C_M_RECV_LEN)
 Send NACK/STOP bits before last byte is received

Signed-off-by: Garrett Ding <g-ding@ti.com>
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Tested-by: Garrett Ding <g-ding@ti.com>
CC: Sekhar Nori <nsekhar@ti.com>
CC: Kevin Hilman <khilman@deeprootsystems.com>
CC: Wolfram Sang <wsa@the-dreams.de>
CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 Tested on a customer board based on K2HK SoC
 drivers/i2c/busses/i2c-davinci.c |   42 +++++++++++++++++++++++++++++++++-----
 1 file changed, 37 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 389bc68..cd97920 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -97,6 +97,10 @@
 #define DAVINCI_I2C_IMR_NACK	BIT(1)
 #define DAVINCI_I2C_IMR_AL	BIT(0)
 
+/* capabilities */
+#define I2C_CAPABILITIES	(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | \
+				 I2C_FUNC_SMBUS_READ_BLOCK_DATA)
+
 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;
+	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;
+
 				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);
+				}
+
 				if (dev->buf_len)
 					continue;
 
-- 
1.7.9.5

RE: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Karicheri, Muralidharan]

>-----Original Message-----
>From: Karicheri, Muralidharan
>Sent: Thursday, May 01, 2014 2:50 PM
>To: davinci-linux-open-source@linux.davincidsp.com; linux-i2c@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Cc: Karicheri, Muralidharan; Ding, Garrett; Nori, Sekhar; Kevin Hilman; Wolfram Sang;
>Shilimkar, Santosh
>Subject: [PATCH] i2c: davinci: Add block read functionality for IPMI
>
>Intelligent Plaform Management Interface (IPMI) requires I2C driver to support block read,
>where the first byte received from slave is the length of following data:-  Added length
>check if the read type is block read (I2C_M_RECV_LEN)  Send NACK/STOP bits before last
>byte is received
>
>Signed-off-by: Garrett Ding <g-ding@ti.com>
>Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>Tested-by: Garrett Ding <g-ding@ti.com>
>CC: Sekhar Nori <nsekhar@ti.com>
>CC: Kevin Hilman <khilman@deeprootsystems.com>
>CC: Wolfram Sang <wsa@the-dreams.de>
>CC: Santosh Shilimkar <santosh.shilimkar@ti.com>
>---
> Tested on a customer board based on K2HK SoC
> drivers/i2c/busses/i2c-davinci.c |   42 +++++++++++++++++++++++++++++++++-
>----
> 1 file changed, 37 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
>index 389bc68..cd97920 100644
>--- a/drivers/i2c/busses/i2c-davinci.c
>+++ b/drivers/i2c/busses/i2c-davinci.c
>@@ -97,6 +97,10 @@
> #define DAVINCI_I2C_IMR_NACK	BIT(1)
> #define DAVINCI_I2C_IMR_AL	BIT(0)
>
>+/* capabilities */
>+#define I2C_CAPABILITIES	(I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | \
>+				 I2C_FUNC_SMBUS_READ_BLOCK_DATA)
>+
> 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;
>+	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;
>+
> 				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);
>+				}
>+
> 				if (dev->buf_len)
> 					continue;
>
>--
>1.7.9.5
All,

Please review this when you get a chance.
Murali

Re: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Murali Karicheri]

On 5/22/2014 5:00 AM, Wolfram Sang wrote:
> 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
Wolfram,

Thanks for reviewing the patch. I will review your comments and get back 
to you. This patch
is tested on a customer board and thus it might be a while before I can 
incorporate these changes
and provide an updated patch to the list. Meanwhile I will be reviewing 
the comment in the
next few weeks and get back to you in case I have questions.

Thanks and regards,

Murali

Re: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Sekhar Nori]

On Friday 02 May 2014 12:19 AM, Murali Karicheri wrote:
> Intelligent Plaform Management Interface (IPMI) requires I2C driver
> to support block read, where the first byte received from slave is
> the length of following data:-
>  Added length check if the read type is block read (I2C_M_RECV_LEN)
>  Send NACK/STOP bits before last byte is received
> 
> Signed-off-by: Garrett Ding <g-ding-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>

I tested this on a DA850 using i2cdetect and it did not seem to break 
anything so:

Tested-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>

There are some checks that were triggered in checkpatch. You may want 
to fix them up.

Thanks,
Sekhar

CHECK: Alignment should match open parenthesis
#112: FILE: drivers/i2c/busses/i2c-davinci.c:557:
+					w = davinci_i2c_read_reg(dev,
+							DAVINCI_I2C_MDR_REG);

CHECK: Alignment should match open parenthesis
#115: FILE: drivers/i2c/busses/i2c-davinci.c:560:
+					davinci_i2c_write_reg(dev,
+							DAVINCI_I2C_MDR_REG, w);

CHECK: Alignment should match open parenthesis
#119: FILE: drivers/i2c/busses/i2c-davinci.c:564:
+					davinci_i2c_write_reg(dev,
+							DAVINCI_I2C_MDR_REG, w);

total: 0 errors, 0 warnings, 3 checks, 67 lines checked

Re: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Sekhar Nori]

On Friday 02 May 2014 12:19 AM, Murali Karicheri wrote:
> Intelligent Plaform Management Interface (IPMI) requires I2C driver
> to support block read, where the first byte received from slave is
> the length of following data:-
>  Added length check if the read type is block read (I2C_M_RECV_LEN)
>  Send NACK/STOP bits before last byte is received
> 
> Signed-off-by: Garrett Ding <g-ding@ti.com>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>

I tested this on a DA850 using i2cdetect and it did not seem to break 
anything so:

Tested-by: Sekhar Nori <nsekhar@ti.com>

There are some checks that were triggered in checkpatch. You may want 
to fix them up.

Thanks,
Sekhar

CHECK: Alignment should match open parenthesis
#112: FILE: drivers/i2c/busses/i2c-davinci.c:557:
+					w = davinci_i2c_read_reg(dev,
+							DAVINCI_I2C_MDR_REG);

CHECK: Alignment should match open parenthesis
#115: FILE: drivers/i2c/busses/i2c-davinci.c:560:
+					davinci_i2c_write_reg(dev,
+							DAVINCI_I2C_MDR_REG, w);

CHECK: Alignment should match open parenthesis
#119: FILE: drivers/i2c/busses/i2c-davinci.c:564:
+					davinci_i2c_write_reg(dev,
+							DAVINCI_I2C_MDR_REG, w);

total: 0 errors, 0 warnings, 3 checks, 67 lines checked

Re: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Murali Karicheri]

On 5/22/2014 6:51 AM, Sekhar Nori wrote:
> On Friday 02 May 2014 12:19 AM, Murali Karicheri wrote:
>> Intelligent Plaform Management Interface (IPMI) requires I2C driver
>> to support block read, where the first byte received from slave is
>> the length of following data:-
>>   Added length check if the read type is block read (I2C_M_RECV_LEN)
>>   Send NACK/STOP bits before last byte is received
>>
>> Signed-off-by: Garrett Ding <g-ding-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
> I tested this on a DA850 using i2cdetect and it did not seem to break
> anything so:
>
> Tested-by: Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>
>
> There are some checks that were triggered in checkpatch. You may want
> to fix them up.
>
> Thanks,
> Sekhar
>
> CHECK: Alignment should match open parenthesis
> #112: FILE: drivers/i2c/busses/i2c-davinci.c:557:
> +					w = davinci_i2c_read_reg(dev,
> +							DAVINCI_I2C_MDR_REG);
>
> CHECK: Alignment should match open parenthesis
> #115: FILE: drivers/i2c/busses/i2c-davinci.c:560:
> +					davinci_i2c_write_reg(dev,
> +							DAVINCI_I2C_MDR_REG, w);
>
> CHECK: Alignment should match open parenthesis
> #119: FILE: drivers/i2c/busses/i2c-davinci.c:564:
> +					davinci_i2c_write_reg(dev,
> +							DAVINCI_I2C_MDR_REG, w);
>
> total: 0 errors, 0 warnings, 3 checks, 67 lines checked
>
Sekhar,

Thanks for testing this. I have other comments against this patch and 
will be sending an
updated patch in few weeks.

Thanks and regards,

Murali

Re: [PATCH] i2c: davinci: Add block read functionality for IPMI

[Murali Karicheri]

On 5/22/2014 6:51 AM, Sekhar Nori wrote:
> On Friday 02 May 2014 12:19 AM, Murali Karicheri wrote:
>> Intelligent Plaform Management Interface (IPMI) requires I2C driver
>> to support block read, where the first byte received from slave is
>> the length of following data:-
>>   Added length check if the read type is block read (I2C_M_RECV_LEN)
>>   Send NACK/STOP bits before last byte is received
>>
>> Signed-off-by: Garrett Ding <g-ding@ti.com>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> I tested this on a DA850 using i2cdetect and it did not seem to break
> anything so:
>
> Tested-by: Sekhar Nori <nsekhar@ti.com>
>
> There are some checks that were triggered in checkpatch. You may want
> to fix them up.
>
> Thanks,
> Sekhar
>
> CHECK: Alignment should match open parenthesis
> #112: FILE: drivers/i2c/busses/i2c-davinci.c:557:
> +					w = davinci_i2c_read_reg(dev,
> +							DAVINCI_I2C_MDR_REG);
>
> CHECK: Alignment should match open parenthesis
> #115: FILE: drivers/i2c/busses/i2c-davinci.c:560:
> +					davinci_i2c_write_reg(dev,
> +							DAVINCI_I2C_MDR_REG, w);
>
> CHECK: Alignment should match open parenthesis
> #119: FILE: drivers/i2c/busses/i2c-davinci.c:564:
> +					davinci_i2c_write_reg(dev,
> +							DAVINCI_I2C_MDR_REG, w);
>
> total: 0 errors, 0 warnings, 3 checks, 67 lines checked
>
Sekhar,

Thanks for testing this. I have other comments against this patch and 
will be sending an
updated patch in few weeks.

Thanks and regards,

Murali