All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <guenter.roeck@ericsson.com>
To: lm-sensors@vger.kernel.org
Subject: Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues
Date: Tue, 15 Nov 2011 04:14:34 +0000	[thread overview]
Message-ID: <20111115041434.GA29403@ericsson.com> (raw)
In-Reply-To: <1321249266-17071-1-git-send-email-Yuantian.Tang@freescale.com>

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

On Mon, Nov 14, 2011 at 09:58:03PM -0500, Tang Yuantian-B29983 wrote:
> 
> 
> > -----Original Message-----
> > From: Guenter Roeck [mailto:guenter.roeck@ericsson.com]
> > Hmm ... looking into the code, I2C_FUNC_SMBUS_READ_BLOCK_DATA is missing
> > from I2C_FUNC_SMBUS_EMUL.
> > Now that is interesting.
> > 
> > If you change mpc_functionality() to
> > 	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> > I2C_FUNC_SMBUS_READ_BLOCK_DATA; I bet your problem will disappear.
> > 
> > Jean, any idea why I2C_FUNC_SMBUS_EMUL doesn't include
> > I2C_FUNC_SMBUS_READ_BLOCK_DATA ?
> > 
> [Yuantian:] unfortunately no, Even I add I2C_FUNC_SMBUS_READ_BLOCK_DATA support,
> The return value is not correct.
> 
Actually, you are right. The i2c-mpc driver would also have to handle the I2C_M_RECV_LEN
flag, where the 1st received byte indicates the receive length. That would probably require
a change in mpc_read(), telling it that the first byte to be received is the length.
Something like the attached. Completely untested, of course ...

Guenter

[-- Attachment #2: mpc.patch --]
[-- Type: text/x-diff, Size: 2564 bytes --]

diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
index 107397a..77aade7 100644
--- a/drivers/i2c/busses/i2c-mpc.c
+++ b/drivers/i2c/busses/i2c-mpc.c
@@ -454,7 +454,7 @@ static int mpc_write(struct mpc_i2c *i2c, int target,
 }
 
 static int mpc_read(struct mpc_i2c *i2c, int target,
-		    u8 *data, int length, int restart)
+		    u8 *data, int length, int restart, bool block)
 {
 	unsigned timeout = i2c->adap.timeout;
 	int i, result;
@@ -470,7 +470,7 @@ static int mpc_read(struct mpc_i2c *i2c, int target,
 		return result;
 
 	if (length) {
-		if (length == 1)
+		if (length == 1 && !block)
 			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK);
 		else
 			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA);
@@ -479,17 +479,28 @@ static int mpc_read(struct mpc_i2c *i2c, int target,
 	}
 
 	for (i = 0; i < length; i++) {
+		u8 byte;
+
 		result = i2c_wait(i2c, timeout, 0);
 		if (result < 0)
 			return result;
 
+		byte = readb(i2c->base + MPC_I2C_DR);
+		/*
+		 * Adjust length if first received byte is length
+		 */
+		if (i == 0 && block) {
+			if (byte == 0 || byte > I2C_SMBUS_BLOCK_MAX)
+				return -EPROTO;
+			length += byte;
+		}
+		data[i] = byte;
 		/* Generate txack on next to last byte */
 		if (i == length - 2)
 			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_TXAK);
 		/* Do not generate stop on last byte */
 		if (i == length - 1)
 			writeccr(i2c, CCR_MIEN | CCR_MEN | CCR_MSTA | CCR_MTX);
-		data[i] = readb(i2c->base + MPC_I2C_DR);
 	}
 
 	return length;
@@ -532,12 +543,17 @@ static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 			"Doing %s %d bytes to 0x%02x - %d of %d messages\n",
 			pmsg->flags & I2C_M_RD ? "read" : "write",
 			pmsg->len, pmsg->addr, i + 1, num);
-		if (pmsg->flags & I2C_M_RD)
-			ret =
-			    mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
-		else
+		if (pmsg->flags & I2C_M_RD) {
+			bool block = pmsg->flags & I2C_M_RECV_LEN;
+
+			ret = mpc_read(i2c, pmsg->addr, pmsg->buf, pmsg->len, i,
+				       block);
+			if (block && ret > 0)
+				pmsg->len = ret;
+		} else {
 			ret =
 			    mpc_write(i2c, pmsg->addr, pmsg->buf, pmsg->len, i);
+		}
 	}
 	mpc_i2c_stop(i2c);
 	return (ret < 0) ? ret : num;
@@ -545,7 +561,8 @@ static int mpc_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 
 static u32 mpc_functionality(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL
+	  | I2C_FUNC_SMBUS_READ_BLOCK_DATA;
 }
 
 static const struct i2c_algorithm mpc_algo = {

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

  parent reply	other threads:[~2011-11-15  4:14 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-14  5:41 [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues Yuantian.Tang
2011-11-14  8:01 ` Jean Delvare
2011-11-14  8:48 ` Tang Yuantian-B29983
2011-11-14 12:59 ` Jean Delvare
2011-11-14 16:28 ` Guenter Roeck
2011-11-15  2:28 ` Tang Yuantian-B29983
2011-11-15  2:35 ` Tang Yuantian-B29983
2011-11-15  2:42 ` Guenter Roeck
2011-11-15  2:54 ` Guenter Roeck
2011-11-15  2:58 ` Tang Yuantian-B29983
2011-11-15  3:00 ` Tabi Timur-B04825
2011-11-15  3:39 ` Guenter Roeck
2011-11-15  4:14 ` Guenter Roeck [this message]
2011-11-15  5:37 ` Tang Yuantian-B29983
2011-11-15  6:14 ` Guenter Roeck
2011-11-15  7:24 ` Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111115041434.GA29403@ericsson.com \
    --to=guenter.roeck@ericsson.com \
    --cc=lm-sensors@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.