[lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Yuantian.Tang]

From: Tang Yuantian <B29983@freescale.com>

1. Not all platforms support i2c_smbus_read_block_data function.
Use i2c_smbus_read_i2c_block_data instead.
2. Not all zlxx chips's id start with zlxx. zl6100's id starts
with 0x10. Take this situation into account.

Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
---
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
branch: master
test platform: P1022DS

 drivers/hwmon/pmbus/zl6100.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/hwmon/pmbus/zl6100.c b/drivers/hwmon/pmbus/zl6100.c
index 2bc9800..8e2fd52 100644
--- a/drivers/hwmon/pmbus/zl6100.c
+++ b/drivers/hwmon/pmbus/zl6100.c
@@ -26,6 +26,7 @@
 #include <linux/i2c.h>
 #include <linux/ktime.h>
 #include <linux/delay.h>
+#include <linux/ctype.h>
 #include "pmbus.h"
 
 enum chips { zl2004, zl2006, zl2008, zl2105, zl2106, zl6100, zl6105 };
@@ -42,6 +43,8 @@ struct zl6100_data {
 
 #define ZL6100_WAIT_TIME		1000	/* uS	*/
 
+#define ZL6100_ID_LEN			16		/* device id length */
+
 static ushort delay = ZL6100_WAIT_TIME;
 module_param(delay, ushort, 0644);
 MODULE_PARM_DESC(delay, "Delay between chip accesses in uS");
@@ -136,19 +139,20 @@ MODULE_DEVICE_TABLE(i2c, zl6100_id);
 static int zl6100_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
-	int ret;
+	int i, ret;
 	struct zl6100_data *data;
 	struct pmbus_driver_info *info;
-	u8 device_id[I2C_SMBUS_BLOCK_MAX + 1];
+	u8 device_id[ZL6100_ID_LEN + 1];
 	const struct i2c_device_id *mid;
 
 	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_BYTE_DATA
-				     | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
+				     I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_WORD_DATA |
+				     I2C_FUNC_SMBUS_READ_I2C_BLOCK))
 		return -ENODEV;
 
-	ret = i2c_smbus_read_block_data(client, ZL6100_DEVICE_ID,
-					device_id);
+	ret = i2c_smbus_read_i2c_block_data(client, ZL6100_DEVICE_ID,
+					ZL6100_ID_LEN, device_id);
 	if (ret < 0) {
 		dev_err(&client->dev, "Failed to read device ID\n");
 		return ret;
@@ -156,9 +160,12 @@ static int zl6100_probe(struct i2c_client *client,
 	device_id[ret] = '\0';
 	dev_info(&client->dev, "Device ID %s\n", device_id);
 
+	for (i = 0; i < ret; i++)
+		device_id[i] = tolower(device_id[i]);
+
 	mid = NULL;
 	for (mid = zl6100_id; mid->name[0]; mid++) {
-		if (!strncasecmp(mid->name, device_id, strlen(mid->name)))
+		if (strstr(device_id, mid->name))
 			break;
 	}
 	if (!mid->name[0]) {
-- 
1.6.4



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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Jean Delvare]

Hi Tang,

On Mon, 14 Nov 2011 13:41:06 +0800, Yuantian.Tang@freescale.com wrote:
> From: Tang Yuantian <B29983@freescale.com>
> 
> 1. Not all platforms support i2c_smbus_read_block_data function.
> Use i2c_smbus_read_i2c_block_data instead.

You can't do that. These are two different transfer formats, they are
not interchangeable. If the device wants the SMBus block format, then
you have to use this in the driver. If the bus driver doesn't support
that, it has to be fixed. In general it is only a matter of adding
support for I2C_M_RECV_LEN to the bus driver. You can check in
i2c-core:i2c_smbus_xfer_emulated() for a reference implementation.

> 2. Not all zlxx chips's id start with zlxx. zl6100's id starts
> with 0x10. Take this situation into account.

No, this is incorrect. 0x10 is the first byte returned because SMBus
block reads receive the block length as the first byte. 0x10 = 16,
which is the length of the ID for these chips, this is no coincidence.
This is a bug introduced by your above change of SMBus block read for
I2C block read.

So, nack.

> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
> ---
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> branch: master
> test platform: P1022DS
> 
>  drivers/hwmon/pmbus/zl6100.c |   21 ++++++++++++++-------
>  1 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/zl6100.c b/drivers/hwmon/pmbus/zl6100.c
> index 2bc9800..8e2fd52 100644
> --- a/drivers/hwmon/pmbus/zl6100.c
> +++ b/drivers/hwmon/pmbus/zl6100.c
> @@ -26,6 +26,7 @@
>  #include <linux/i2c.h>
>  #include <linux/ktime.h>
>  #include <linux/delay.h>
> +#include <linux/ctype.h>
>  #include "pmbus.h"
>  
>  enum chips { zl2004, zl2006, zl2008, zl2105, zl2106, zl6100, zl6105 };
> @@ -42,6 +43,8 @@ struct zl6100_data {
>  
>  #define ZL6100_WAIT_TIME		1000	/* uS	*/
>  
> +#define ZL6100_ID_LEN			16		/* device id length */
> +
>  static ushort delay = ZL6100_WAIT_TIME;
>  module_param(delay, ushort, 0644);
>  MODULE_PARM_DESC(delay, "Delay between chip accesses in uS");
> @@ -136,19 +139,20 @@ MODULE_DEVICE_TABLE(i2c, zl6100_id);
>  static int zl6100_probe(struct i2c_client *client,
>  			const struct i2c_device_id *id)
>  {
> -	int ret;
> +	int i, ret;
>  	struct zl6100_data *data;
>  	struct pmbus_driver_info *info;
> -	u8 device_id[I2C_SMBUS_BLOCK_MAX + 1];
> +	u8 device_id[ZL6100_ID_LEN + 1];
>  	const struct i2c_device_id *mid;
>  
>  	if (!i2c_check_functionality(client->adapter,
> -				     I2C_FUNC_SMBUS_READ_BYTE_DATA
> -				     | I2C_FUNC_SMBUS_READ_BLOCK_DATA))
> +				     I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_READ_I2C_BLOCK))
>  		return -ENODEV;
>  
> -	ret = i2c_smbus_read_block_data(client, ZL6100_DEVICE_ID,
> -					device_id);
> +	ret = i2c_smbus_read_i2c_block_data(client, ZL6100_DEVICE_ID,
> +					ZL6100_ID_LEN, device_id);
>  	if (ret < 0) {
>  		dev_err(&client->dev, "Failed to read device ID\n");
>  		return ret;
> @@ -156,9 +160,12 @@ static int zl6100_probe(struct i2c_client *client,
>  	device_id[ret] = '\0';
>  	dev_info(&client->dev, "Device ID %s\n", device_id);
>  
> +	for (i = 0; i < ret; i++)
> +		device_id[i] = tolower(device_id[i]);
> +
>  	mid = NULL;
>  	for (mid = zl6100_id; mid->name[0]; mid++) {
> -		if (!strncasecmp(mid->name, device_id, strlen(mid->name)))
> +		if (strstr(device_id, mid->name))
>  			break;
>  	}
>  	if (!mid->name[0]) {


-- 
Jean Delvare

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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Tang Yuantian-B29983]

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogSmVhbiBEZWx2YXJlIFtt
YWlsdG86a2hhbGlAbGludXgtZnIub3JnXQ0KPiBTZW50OiAyMDExxOoxMdTCMTTI1SAxNjowMg0K
PiBUbzogVGFuZyBZdWFudGlhbi1CMjk5ODMNCj4gQ2M6IGxtLXNlbnNvcnNAbG0tc2Vuc29ycy5v
cmc7IFRhbmcgWXVhbnRpYW4tQjI5OTgzOyBIdWFuZyBDaGFuZ21pbmctDQo+IFI2NjA5MzsgVGFi
aSBUaW11ci1CMDQ4MjUNCj4gU3ViamVjdDogUmU6IFtsbS1zZW5zb3JzXSBbUEFUQ0hdIGh3bW9u
OiAocG1idXMpIEZpeCB0d28gaXNzdWVzDQo+IA0KPiBIaSBUYW5nLA0KPiANCj4gT24gTW9uLCAx
NCBOb3YgMjAxMSAxMzo0MTowNiArMDgwMCwgWXVhbnRpYW4uVGFuZ0BmcmVlc2NhbGUuY29tIHdy
b3RlOg0KPiA+IEZyb206IFRhbmcgWXVhbnRpYW4gPEIyOTk4M0BmcmVlc2NhbGUuY29tPg0KPiA+
DQo+ID4gMS4gTm90IGFsbCBwbGF0Zm9ybXMgc3VwcG9ydCBpMmNfc21idXNfcmVhZF9ibG9ja19k
YXRhIGZ1bmN0aW9uLg0KPiA+IFVzZSBpMmNfc21idXNfcmVhZF9pMmNfYmxvY2tfZGF0YSBpbnN0
ZWFkLg0KPiANCj4gWW91IGNhbid0IGRvIHRoYXQuIFRoZXNlIGFyZSB0d28gZGlmZmVyZW50IHRy
YW5zZmVyIGZvcm1hdHMsIHRoZXkgYXJlIG5vdA0KPiBpbnRlcmNoYW5nZWFibGUuIElmIHRoZSBk
ZXZpY2Ugd2FudHMgdGhlIFNNQnVzIGJsb2NrIGZvcm1hdCwgdGhlbiB5b3UNCj4gaGF2ZSB0byB1
c2UgdGhpcyBpbiB0aGUgZHJpdmVyLiBJZiB0aGUgYnVzIGRyaXZlciBkb2Vzbid0IHN1cHBvcnQg
dGhhdCwNCj4gaXQgaGFzIHRvIGJlIGZpeGVkLiBJbiBnZW5lcmFsIGl0IGlzIG9ubHkgYSBtYXR0
ZXIgb2YgYWRkaW5nIHN1cHBvcnQgZm9yDQo+IEkyQ19NX1JFQ1ZfTEVOIHRvIHRoZSBidXMgZHJp
dmVyLiBZb3UgY2FuIGNoZWNrIGluDQo+IGkyYy1jb3JlOmkyY19zbWJ1c194ZmVyX2VtdWxhdGVk
KCkgZm9yIGEgcmVmZXJlbmNlIGltcGxlbWVudGF0aW9uLg0KPiANCltZdWFudGlhbjpdIGFsbW9z
dCBhbGwgcGxhdGZvcm0ncyBpMmMgZHJpdmVyIGRvbid0IHN1cHBvcnQgaTJjX3NtYnVzX3JlYWRf
YmxvY2tfZGF0YS4NCldoeSBjYW4ndCB3ZSBqdXN0IHVzZSBpMmNfc21idXNfcmVhZF9pMmNfYmxv
Y2tfZGF0YSBpbnN0ZWFkLg0KQmVzaWRlLCBhY2NvcmRpbmcgbXkgdGVzdCwgaTJjX3NtYnVzX3Jl
YWRfYmxvY2tfZGF0YSBjYW4ndCByZXR1cm4gY29ycmVjdCB2YWx1ZSwNCkJ1dCBpMmNfc21idXNf
cmVhZF9pMmNfYmxvY2tfZGF0YS4NCkkgdGhpbmsgaTJjX3NtYnVzX3JlYWRfaTJjX2Jsb2NrX2Rh
dGEgaXMgb25seSByaWdodCBjaG9pY2UuDQoNCj4gPiAyLiBOb3QgYWxsIHpseHggY2hpcHMncyBp
ZCBzdGFydCB3aXRoIHpseHguIHpsNjEwMCdzIGlkIHN0YXJ0cyB3aXRoDQo+ID4gMHgxMC4gVGFr
ZSB0aGlzIHNpdHVhdGlvbiBpbnRvIGFjY291bnQuDQo+IA0KPiBObywgdGhpcyBpcyBpbmNvcnJl
Y3QuIDB4MTAgaXMgdGhlIGZpcnN0IGJ5dGUgcmV0dXJuZWQgYmVjYXVzZSBTTUJ1cw0KPiBibG9j
ayByZWFkcyByZWNlaXZlIHRoZSBibG9jayBsZW5ndGggYXMgdGhlIGZpcnN0IGJ5dGUuIDB4MTAg
PT0gMTYsIHdoaWNoDQo+IGlzIHRoZSBsZW5ndGggb2YgdGhlIElEIGZvciB0aGVzZSBjaGlwcywg
dGhpcyBpcyBubyBjb2luY2lkZW5jZS4NCj4gVGhpcyBpcyBhIGJ1ZyBpbnRyb2R1Y2VkIGJ5IHlv
dXIgYWJvdmUgY2hhbmdlIG9mIFNNQnVzIGJsb2NrIHJlYWQgZm9yIEkyQw0KPiBibG9jayByZWFk
Lg0KPiANCj4gU28sIG5hY2suDQo+IA0KW1l1YW50aWFuOl0gSSBtaXN1bmRlcnN0YW5kIHRoZSAw
eDEwLiBCdXQgdGhlIGV4aXN0aW5nIHpsNjEwMCBkcml2ZXIgZGlkbid0IGNvbnNpZGVyIGl0IHRv
by4NCkFuZCBzdGlsbCBuZWVkIHRvIGJlIGZpeGVkLg0KDQo+ID4gU2lnbmVkLW9mZi1ieTogVGFu
ZyBZdWFudGlhbiA8WXVhbnRpYW4uVGFuZ0BmcmVlc2NhbGUuY29tPg0KPiA+IC0tLQ0KPiA+IGdp
dDovL2dpdC5rZXJuZWwub3JnL3B1Yi9zY20vbGludXgva2VybmVsL2dpdC90b3J2YWxkcy9saW51
eC5naXQNCj4gPiBicmFuY2g6IG1hc3Rlcg0KPiA+IHRlc3QgcGxhdGZvcm06IFAxMDIyRFMNCj4g
Pg0KPiA+ICBkcml2ZXJzL2h3bW9uL3BtYnVzL3psNjEwMC5jIHwgICAyMSArKysrKysrKysrKysr
Ky0tLS0tLS0NCj4gPiAgMSBmaWxlcyBjaGFuZ2VkLCAxNCBpbnNlcnRpb25zKCspLCA3IGRlbGV0
aW9ucygtKQ0KPiA+DQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvaHdtb24vcG1idXMvemw2MTAw
LmMNCj4gPiBiL2RyaXZlcnMvaHdtb24vcG1idXMvemw2MTAwLmMgaW5kZXggMmJjOTgwMC4uOGUy
ZmQ1MiAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2h3bW9uL3BtYnVzL3psNjEwMC5jDQo+ID4g
KysrIGIvZHJpdmVycy9od21vbi9wbWJ1cy96bDYxMDAuYw0KPiA+IEBAIC0yNiw2ICsyNiw3IEBA
DQo+ID4gICNpbmNsdWRlIDxsaW51eC9pMmMuaD4NCj4gPiAgI2luY2x1ZGUgPGxpbnV4L2t0aW1l
Lmg+DQo+ID4gICNpbmNsdWRlIDxsaW51eC9kZWxheS5oPg0KPiA+ICsjaW5jbHVkZSA8bGludXgv
Y3R5cGUuaD4NCj4gDQo+IA0KPiAtLQ0KPiBKZWFuIERlbHZhcmUNCg0KX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QK
bG0tc2Vuc29yc0BsbS1zZW5zb3JzLm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFp
bG1hbi9saXN0aW5mby9sbS1zZW5zb3Jz

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Jean Delvare]

T24gTW9uLCAxNCBOb3YgMjAxMSAwODo0ODozOCArMDAwMCwgVGFuZyBZdWFudGlhbi1CMjk5ODMg
d3JvdGU6Cj4gCj4gCj4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQo+ID4gRnJvbTogSmVh
biBEZWx2YXJlIFttYWlsdG86a2hhbGlAbGludXgtZnIub3JnXQo+ID4gU2VudDogMjAxMcTqMTHU
wjE0yNUgMTY6MDIKPiA+IFRvOiBUYW5nIFl1YW50aWFuLUIyOTk4Mwo+ID4gQ2M6IGxtLXNlbnNv
cnNAbG0tc2Vuc29ycy5vcmc7IFRhbmcgWXVhbnRpYW4tQjI5OTgzOyBIdWFuZyBDaGFuZ21pbmct
Cj4gPiBSNjYwOTM7IFRhYmkgVGltdXItQjA0ODI1Cj4gPiBTdWJqZWN0OiBSZTogW2xtLXNlbnNv
cnNdIFtQQVRDSF0gaHdtb246IChwbWJ1cykgRml4IHR3byBpc3N1ZXMKPiA+IAo+ID4gSGkgVGFu
ZywKPiA+IAo+ID4gT24gTW9uLCAxNCBOb3YgMjAxMSAxMzo0MTowNiArMDgwMCwgWXVhbnRpYW4u
VGFuZ0BmcmVlc2NhbGUuY29tIHdyb3RlOgo+ID4gPiBGcm9tOiBUYW5nIFl1YW50aWFuIDxCMjk5
ODNAZnJlZXNjYWxlLmNvbT4KPiA+ID4KPiA+ID4gMS4gTm90IGFsbCBwbGF0Zm9ybXMgc3VwcG9y
dCBpMmNfc21idXNfcmVhZF9ibG9ja19kYXRhIGZ1bmN0aW9uLgo+ID4gPiBVc2UgaTJjX3NtYnVz
X3JlYWRfaTJjX2Jsb2NrX2RhdGEgaW5zdGVhZC4KPiA+IAo+ID4gWW91IGNhbid0IGRvIHRoYXQu
IFRoZXNlIGFyZSB0d28gZGlmZmVyZW50IHRyYW5zZmVyIGZvcm1hdHMsIHRoZXkgYXJlIG5vdAo+
ID4gaW50ZXJjaGFuZ2VhYmxlLiBJZiB0aGUgZGV2aWNlIHdhbnRzIHRoZSBTTUJ1cyBibG9jayBm
b3JtYXQsIHRoZW4geW91Cj4gPiBoYXZlIHRvIHVzZSB0aGlzIGluIHRoZSBkcml2ZXIuIElmIHRo
ZSBidXMgZHJpdmVyIGRvZXNuJ3Qgc3VwcG9ydCB0aGF0LAo+ID4gaXQgaGFzIHRvIGJlIGZpeGVk
LiBJbiBnZW5lcmFsIGl0IGlzIG9ubHkgYSBtYXR0ZXIgb2YgYWRkaW5nIHN1cHBvcnQgZm9yCj4g
PiBJMkNfTV9SRUNWX0xFTiB0byB0aGUgYnVzIGRyaXZlci4gWW91IGNhbiBjaGVjayBpbgo+ID4g
aTJjLWNvcmU6aTJjX3NtYnVzX3hmZXJfZW11bGF0ZWQoKSBmb3IgYSByZWZlcmVuY2UgaW1wbGVt
ZW50YXRpb24uCj4gPiAKPiBbWXVhbnRpYW46XSBhbG1vc3QgYWxsIHBsYXRmb3JtJ3MgaTJjIGRy
aXZlciBkb24ndCBzdXBwb3J0IGkyY19zbWJ1c19yZWFkX2Jsb2NrX2RhdGEuCj4gV2h5IGNhbid0
IHdlIGp1c3QgdXNlIGkyY19zbWJ1c19yZWFkX2kyY19ibG9ja19kYXRhIGluc3RlYWQuCj4gQmVz
aWRlLCBhY2NvcmRpbmcgbXkgdGVzdCwgaTJjX3NtYnVzX3JlYWRfYmxvY2tfZGF0YSBjYW4ndCBy
ZXR1cm4gY29ycmVjdCB2YWx1ZSwKPiBCdXQgaTJjX3NtYnVzX3JlYWRfaTJjX2Jsb2NrX2RhdGEu
Cj4gSSB0aGluayBpMmNfc21idXNfcmVhZF9pMmNfYmxvY2tfZGF0YSBpcyBvbmx5IHJpZ2h0IGNo
b2ljZS4KCkFnYWluLCBuby4gUE1CdXMgZ29lcyBvbiB0b3Agb2YgU01CdXMgd2hpY2ggaW5jbHVk
ZXMgU01CdXMgYmxvY2sgcmVhZHMKKGkyY19zbWJ1c19yZWFkX2Jsb2NrX2RhdGEpIGJ1dCBub3Qg
STJDIGJsb2NrIHJlYWRzCihpMmNfc21idXNfcmVhZF9pMmNfYmxvY2tfZGF0YSkuIEFzIGEgcmVz
dWx0LCBtYW55IGhhcmR3YXJlIFNNQnVzCmNvbnRyb2xsZXJzIGRvIG5vdCBzdXBwb3J0IEkyQyBi
bG9jayByZWFkcywgYW5kIGl0IGNhbid0IGJlIGFkZGVkIChvbmx5CnRoZSBzZXQgb2YgdHJhbnNm
ZXJzIHN1cHBvcnRlZCBieSB0aGUgaGFyZHdhcmUgY29udHJvbGxlciBjYW4gYmUKaW1wbGVtZW50
ZWQgYnkgdGhlIGRyaXZlcikgd2hpY2ggbWVhbnMgdGhhdCB5b3VyIHByb3Bvc2VkIGNoYW5nZSB3
b3VsZApicmVhayBzdXBwb3J0IG9uIGV4aXN0aW5nIHN5c3RlbXMuIFRoaXMgaXMgbm90IGFjY2Vw
dGFibGUuCgpPbiB0aGUgb3RoZXIgaGFuZCwgYW4gSTJDIGNvbnRyb2xsZXIgd2hpY2ggc3VwcG9y
dHMgSTJDIGJsb2NrIHJlYWRzIGNhbgphbG1vc3QgYWx3YXlzIGdldCBTTUJ1cyBibG9jayByZWFk
cyBhZGRlZCwgaXQncyBvbmx5IGEgbWF0dGVyIG9mIGFkZGluZwphIGZldyBsaW5lcyBvZiBjb2Rl
IHRvIHRoZSBidXMgZHJpdmVyLgoKV2hpY2ggaTJjIGJ1cyBkcml2ZXIgYXJlIHlvdSB1c2luZz8g
RnJvbSB3aGljaCBrZXJuZWwgdmVyc2lvbj8KCj4gPiA+IDIuIE5vdCBhbGwgemx4eCBjaGlwcydz
IGlkIHN0YXJ0IHdpdGggemx4eC4gemw2MTAwJ3MgaWQgc3RhcnRzIHdpdGgKPiA+ID4gMHgxMC4g
VGFrZSB0aGlzIHNpdHVhdGlvbiBpbnRvIGFjY291bnQuCj4gPiAKPiA+IE5vLCB0aGlzIGlzIGlu
Y29ycmVjdC4gMHgxMCBpcyB0aGUgZmlyc3QgYnl0ZSByZXR1cm5lZCBiZWNhdXNlIFNNQnVzCj4g
PiBibG9jayByZWFkcyByZWNlaXZlIHRoZSBibG9jayBsZW5ndGggYXMgdGhlIGZpcnN0IGJ5dGUu
IDB4MTAgPT0gMTYsIHdoaWNoCj4gPiBpcyB0aGUgbGVuZ3RoIG9mIHRoZSBJRCBmb3IgdGhlc2Ug
Y2hpcHMsIHRoaXMgaXMgbm8gY29pbmNpZGVuY2UuCj4gPiBUaGlzIGlzIGEgYnVnIGludHJvZHVj
ZWQgYnkgeW91ciBhYm92ZSBjaGFuZ2Ugb2YgU01CdXMgYmxvY2sgcmVhZCBmb3IgSTJDCj4gPiBi
bG9jayByZWFkLgo+ID4gCj4gPiBTbywgbmFjay4KPiA+IAo+IFtZdWFudGlhbjpdIEkgbWlzdW5k
ZXJzdGFuZCB0aGUgMHgxMC4gQnV0IHRoZSBleGlzdGluZyB6bDYxMDAgZHJpdmVyIGRpZG4ndCBj
b25zaWRlciBpdCB0b28uCj4gQW5kIHN0aWxsIG5lZWQgdG8gYmUgZml4ZWQuCgpJIGRpZG4ndCB3
cml0ZSB0aGUgZHJpdmVyLCBhbmQgZG9uJ3QgaGF2ZSB0aGUgZGV2aWNlLCBidXQgSSB0aGluayB0
aGUKZHJpdmVyIGlzIE9LIGFzIGlzLiBUaGUgYmxvY2sgbGVuZ3RoICgweDEwKSBpcyBoYW5kbGVk
IGJ5IHRoZQp1bmRlcmx5aW5nIGJ1cyBkcml2ZXIsIHRoZSBkZXZpY2UgZHJpdmVyIG5ldmVyIGdl
dHMgdG8gc2VlIGl0LCBpZiB5b3UKcHJvcGVybHkgY2FsbCBpMmNfc21idXNfcmVhZF9ibG9ja19k
YXRhKCkgYXMgeW91IGFyZSBzdXBwb3NlZCB0by4KCi0tIApKZWFuIERlbHZhcmUKCl9fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxtLXNlbnNvcnMgbWFpbGlu
ZyBsaXN0CmxtLXNlbnNvcnNAbG0tc2Vuc29ycy5vcmcKaHR0cDovL2xpc3RzLmxtLXNlbnNvcnMu
b3JnL21haWxtYW4vbGlzdGluZm8vbG0tc2Vuc29ycw=

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Guenter Roeck]

T24gTW9uLCAyMDExLTExLTE0IGF0IDA3OjU5IC0wNTAwLCBKZWFuIERlbHZhcmUgd3JvdGU6Cj4g
T24gTW9uLCAxNCBOb3YgMjAxMSAwODo0ODozOCArMDAwMCwgVGFuZyBZdWFudGlhbi1CMjk5ODMg
d3JvdGU6Cj4gPiAKPiA+IAo+ID4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQo+ID4gPiBG
cm9tOiBKZWFuIERlbHZhcmUgW21haWx0bzpraGFsaUBsaW51eC1mci5vcmddCj4gPiA+IFNlbnQ6
IDIwMTHlubQxMeaciDE05pelIDE2OjAyCj4gPiA+IFRvOiBUYW5nIFl1YW50aWFuLUIyOTk4Mwo+
ID4gPiBDYzogbG0tc2Vuc29yc0BsbS1zZW5zb3JzLm9yZzsgVGFuZyBZdWFudGlhbi1CMjk5ODM7
IEh1YW5nIENoYW5nbWluZy0KPiA+ID4gUjY2MDkzOyBUYWJpIFRpbXVyLUIwNDgyNQo+ID4gPiBT
dWJqZWN0OiBSZTogW2xtLXNlbnNvcnNdIFtQQVRDSF0gaHdtb246IChwbWJ1cykgRml4IHR3byBp
c3N1ZXMKPiA+ID4gCj4gPiA+IEhpIFRhbmcsCj4gPiA+IAo+ID4gPiBPbiBNb24sIDE0IE5vdiAy
MDExIDEzOjQxOjA2ICswODAwLCBZdWFudGlhbi5UYW5nQGZyZWVzY2FsZS5jb20gd3JvdGU6Cj4g
PiA+ID4gRnJvbTogVGFuZyBZdWFudGlhbiA8QjI5OTgzQGZyZWVzY2FsZS5jb20+Cj4gPiA+ID4K
PiA+ID4gPiAxLiBOb3QgYWxsIHBsYXRmb3JtcyBzdXBwb3J0IGkyY19zbWJ1c19yZWFkX2Jsb2Nr
X2RhdGEgZnVuY3Rpb24uCj4gPiA+ID4gVXNlIGkyY19zbWJ1c19yZWFkX2kyY19ibG9ja19kYXRh
IGluc3RlYWQuCj4gPiA+IAo+ID4gPiBZb3UgY2FuJ3QgZG8gdGhhdC4gVGhlc2UgYXJlIHR3byBk
aWZmZXJlbnQgdHJhbnNmZXIgZm9ybWF0cywgdGhleSBhcmUgbm90Cj4gPiA+IGludGVyY2hhbmdl
YWJsZS4gSWYgdGhlIGRldmljZSB3YW50cyB0aGUgU01CdXMgYmxvY2sgZm9ybWF0LCB0aGVuIHlv
dQo+ID4gPiBoYXZlIHRvIHVzZSB0aGlzIGluIHRoZSBkcml2ZXIuIElmIHRoZSBidXMgZHJpdmVy
IGRvZXNuJ3Qgc3VwcG9ydCB0aGF0LAo+ID4gPiBpdCBoYXMgdG8gYmUgZml4ZWQuIEluIGdlbmVy
YWwgaXQgaXMgb25seSBhIG1hdHRlciBvZiBhZGRpbmcgc3VwcG9ydCBmb3IKPiA+ID4gSTJDX01f
UkVDVl9MRU4gdG8gdGhlIGJ1cyBkcml2ZXIuIFlvdSBjYW4gY2hlY2sgaW4KPiA+ID4gaTJjLWNv
cmU6aTJjX3NtYnVzX3hmZXJfZW11bGF0ZWQoKSBmb3IgYSByZWZlcmVuY2UgaW1wbGVtZW50YXRp
b24uCj4gPiA+IAo+ID4gW1l1YW50aWFuOl0gYWxtb3N0IGFsbCBwbGF0Zm9ybSdzIGkyYyBkcml2
ZXIgZG9uJ3Qgc3VwcG9ydCBpMmNfc21idXNfcmVhZF9ibG9ja19kYXRhLgo+ID4gV2h5IGNhbid0
IHdlIGp1c3QgdXNlIGkyY19zbWJ1c19yZWFkX2kyY19ibG9ja19kYXRhIGluc3RlYWQuCj4gPiBC
ZXNpZGUsIGFjY29yZGluZyBteSB0ZXN0LCBpMmNfc21idXNfcmVhZF9ibG9ja19kYXRhIGNhbid0
IHJldHVybiBjb3JyZWN0IHZhbHVlLAo+ID4gQnV0IGkyY19zbWJ1c19yZWFkX2kyY19ibG9ja19k
YXRhLgo+ID4gSSB0aGluayBpMmNfc21idXNfcmVhZF9pMmNfYmxvY2tfZGF0YSBpcyBvbmx5IHJp
Z2h0IGNob2ljZS4KPiAKPiBBZ2Fpbiwgbm8uIFBNQnVzIGdvZXMgb24gdG9wIG9mIFNNQnVzIHdo
aWNoIGluY2x1ZGVzIFNNQnVzIGJsb2NrIHJlYWRzCj4gKGkyY19zbWJ1c19yZWFkX2Jsb2NrX2Rh
dGEpIGJ1dCBub3QgSTJDIGJsb2NrIHJlYWRzCj4gKGkyY19zbWJ1c19yZWFkX2kyY19ibG9ja19k
YXRhKS4gQXMgYSByZXN1bHQsIG1hbnkgaGFyZHdhcmUgU01CdXMKPiBjb250cm9sbGVycyBkbyBu
b3Qgc3VwcG9ydCBJMkMgYmxvY2sgcmVhZHMsIGFuZCBpdCBjYW4ndCBiZSBhZGRlZCAob25seQo+
IHRoZSBzZXQgb2YgdHJhbnNmZXJzIHN1cHBvcnRlZCBieSB0aGUgaGFyZHdhcmUgY29udHJvbGxl
ciBjYW4gYmUKPiBpbXBsZW1lbnRlZCBieSB0aGUgZHJpdmVyKSB3aGljaCBtZWFucyB0aGF0IHlv
dXIgcHJvcG9zZWQgY2hhbmdlIHdvdWxkCj4gYnJlYWsgc3VwcG9ydCBvbiBleGlzdGluZyBzeXN0
ZW1zLiBUaGlzIGlzIG5vdCBhY2NlcHRhYmxlLgo+IAo+IE9uIHRoZSBvdGhlciBoYW5kLCBhbiBJ
MkMgY29udHJvbGxlciB3aGljaCBzdXBwb3J0cyBJMkMgYmxvY2sgcmVhZHMgY2FuCj4gYWxtb3N0
IGFsd2F5cyBnZXQgU01CdXMgYmxvY2sgcmVhZHMgYWRkZWQsIGl0J3Mgb25seSBhIG1hdHRlciBv
ZiBhZGRpbmcKPiBhIGZldyBsaW5lcyBvZiBjb2RlIHRvIHRoZSBidXMgZHJpdmVyLgo+IAo+IFdo
aWNoIGkyYyBidXMgZHJpdmVyIGFyZSB5b3UgdXNpbmc/IEZyb20gd2hpY2gga2VybmVsIHZlcnNp
b24/Cj4gCj4gPiA+ID4gMi4gTm90IGFsbCB6bHh4IGNoaXBzJ3MgaWQgc3RhcnQgd2l0aCB6bHh4
LiB6bDYxMDAncyBpZCBzdGFydHMgd2l0aAo+ID4gPiA+IDB4MTAuIFRha2UgdGhpcyBzaXR1YXRp
b24gaW50byBhY2NvdW50Lgo+ID4gPiAKPiA+ID4gTm8sIHRoaXMgaXMgaW5jb3JyZWN0LiAweDEw
IGlzIHRoZSBmaXJzdCBieXRlIHJldHVybmVkIGJlY2F1c2UgU01CdXMKPiA+ID4gYmxvY2sgcmVh
ZHMgcmVjZWl2ZSB0aGUgYmxvY2sgbGVuZ3RoIGFzIHRoZSBmaXJzdCBieXRlLiAweDEwID09IDE2
LCB3aGljaAo+ID4gPiBpcyB0aGUgbGVuZ3RoIG9mIHRoZSBJRCBmb3IgdGhlc2UgY2hpcHMsIHRo
aXMgaXMgbm8gY29pbmNpZGVuY2UuCj4gPiA+IFRoaXMgaXMgYSBidWcgaW50cm9kdWNlZCBieSB5
b3VyIGFib3ZlIGNoYW5nZSBvZiBTTUJ1cyBibG9jayByZWFkIGZvciBJMkMKPiA+ID4gYmxvY2sg
cmVhZC4KPiA+ID4gCj4gPiA+IFNvLCBuYWNrLgo+ID4gPiAKPiA+IFtZdWFudGlhbjpdIEkgbWlz
dW5kZXJzdGFuZCB0aGUgMHgxMC4gQnV0IHRoZSBleGlzdGluZyB6bDYxMDAgZHJpdmVyIGRpZG4n
dCBjb25zaWRlciBpdCB0b28uCj4gPiBBbmQgc3RpbGwgbmVlZCB0byBiZSBmaXhlZC4KPiAKPiBJ
IGRpZG4ndCB3cml0ZSB0aGUgZHJpdmVyLCBhbmQgZG9uJ3QgaGF2ZSB0aGUgZGV2aWNlLCBidXQg
SSB0aGluayB0aGUKPiBkcml2ZXIgaXMgT0sgYXMgaXMuIFRoZSBibG9jayBsZW5ndGggKDB4MTAp
IGlzIGhhbmRsZWQgYnkgdGhlCj4gdW5kZXJseWluZyBidXMgZHJpdmVyLCB0aGUgZGV2aWNlIGRy
aXZlciBuZXZlciBnZXRzIHRvIHNlZSBpdCwgaWYgeW91Cj4gcHJvcGVybHkgY2FsbCBpMmNfc21i
dXNfcmVhZF9ibG9ja19kYXRhKCkgYXMgeW91IGFyZSBzdXBwb3NlZCB0by4KPiAKSSBoYXZlIGV2
YWwgYm9hcmRzIGZvciBhbGwgY2hpcHMgc3VwcG9ydGVkIGJ5IHRoZSBkcml2ZXIgKGxvdHMgb2Yg
a3Vkb3MKdG8gSW50ZXJzaWwpLCBzbyB0aGUgZHJpdmVyIHdhcyB0ZXN0ZWQgYW5kIGlzIGtub3du
IHRvIHdvcmsgZm9yIGFsbApjaGlwcyBpdCBjdXJyZW50bHkgc3VwcG9ydHMuIEludGVyc2lsIHRv
bGQgbWUgdGhhdCB0aGV5IGRvbid0IHVwZGF0ZSB0aGUKZmlybXdhcmUgYWZ0ZXIgdGhleSByZWxl
YXNlZCBhIGNoaXAsIHNvIHdlIGNhbiBiZSBzdXJlIHRoYXQgdGhlIGZpcm13YXJlCnZlcnNpb24g
b24gYWxsIGNoaXBzIG9mIHRoZSBzYW1lIHR5cGUgaXMgdGhlIHNhbWUsIGFuZCB3ZSBkb24ndCBo
YXZlIHRvCmxvb2sgZm9yIHZlcnNpb24gc3BlY2lmaWMgYmVoYXZpb3IuCgpJIGFncmVlIHRoYXQg
d2Ugc2hvdWxkIG5vdCByZXBsYWNlIHRoZSBTTUJ1cyBibG9jayByZWFkIHdpdGggU01CdXMgSTJD
CmJsb2NrIHJlYWQuIFRoZSBJMkMgYnVzIGRyaXZlciBzaG91bGQgcmVhbGx5IGltcGxlbWVudCBT
TUJ1cyBibG9jayByZWFkCmNvbW1hbmQgc3VwcG9ydC4KClRoZXJlIGFyZSBzb21lIEkyQyBidXMg
ZHJpdmVycyB3aGljaCBjYW4gbm90IHN1cHBvcnQgU01CdXMgYmxvY2sgcmVhZHMKZHVlIHRvIGNo
aXAgbGltaXRhdGlvbnMsIHN1Y2ggYXMgdGhlIFNpYnl0ZSBkcml2ZXIsIGJ1dCBpZiB0aGUgZHJp
dmVyCmNhbiBzdXBwb3J0IFNNQnVzIEkyQyBibG9jayByZWFkcyBpdCBzaG91bGQgcG9zc2libGUg
dG8gc3VwcG9ydCBTTUJ1cwpibG9jayByZWFkcyBhcyB3ZWxsLiBBcyBmYXIgYXMgSSBjYW4gc2Vl
LCB0aGUgb25seSBkcml2ZXIgc3VwcG9ydGluZwpTTUJVUyBJMkMgYmxvY2sgcmVhZHMgYnV0IG5v
dCBTTUJVUyBibG9jayByZWFkcyBpcyBzY3gyMDBfYWNiLiBUaGF0CnNlZW1zIHRvIGJlIGFuIG92
ZXJzaWdodCAob3IgbWF5YmUgbGF6aW5lc3MgOyksIGFuZCBzaG91bGQgYmUgZWFzeSB0bwpmaXgu
CgpUaGFua3MsCkd1ZW50ZXIKCgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3Jz
Lm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5z
b3Jz

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Tang Yuantian-B29983]

Hi Guenter,

Please find my comments in line.

> -----Original Message-----
> > > > SMBus block reads receive the block length as the first byte. 0x10
> > > > = 16, which is the length of the ID for these chips, this is no
> > I didn't write the driver, and don't have the device, but I think the
> > driver is OK as is. The block length (0x10) is handled by the
> > underlying bus driver, the device driver never gets to see it, if you
> > properly call i2c_smbus_read_block_data() as you are supposed to.
> >
> I have eval boards for all chips supported by the driver (lots of kudos
> to Intersil), so the driver was tested and is known to work for all chips
> it currently supports. Intersil told me that they don't update the
> firmware after they released a chip, so we can be sure that the firmware
> version on all chips of the same type is the same, and we don't have to
> look for version specific behavior.
> 
> I agree that we should not replace the SMBus block read with SMBus I2C
> block read. The I2C bus driver should really implement SMBus block read
> command support.
> 
> There are some I2C bus drivers which can not support SMBus block reads
> due to chip limitations, such as the Sibyte driver, but if the driver can
> support SMBus I2C block reads it should possible to support SMBus block
> reads as well. As far as I can see, the only driver supporting SMBUS I2C
> block reads but not SMBUS block reads is scx200_acb. That seems to be an
> oversight (or maybe laziness ;), and should be easy to fix.
> 
[Yuantian:] what platform are you testing on?
What I see is most i2c bus drivers don't support "SMBus block".
I use newest kernel 3.2.

> Thanks,
> Guenter
> 
> 

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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Tang Yuantian-B29983]

Hi Jean Delvare,

Please find my comments in line.

> -----Original Message-----
> 
> Again, no. PMBus goes on top of SMBus which includes SMBus block reads
> (i2c_smbus_read_block_data) but not I2C block reads
> (i2c_smbus_read_i2c_block_data). As a result, many hardware SMBus
> controllers do not support I2C block reads, and it can't be added (only
> the set of transfers supported by the hardware controller can be
> implemented by the driver) which means that your proposed change would
> break support on existing systems. This is not acceptable.
> 
> On the other hand, an I2C controller which supports I2C block reads can
> almost always get SMBus block reads added, it's only a matter of adding a
> few lines of code to the bus driver.
> 
> Which i2c bus driver are you using? From which kernel version?
> 
[Yuantian:] I am using newest kernel 3.2. PowerPC arch. 
I2c bus driver file is: drivers/i2c/busses/i2c-mpc.c.

I greped the i2c bus driver and found the following platform's i2c don't 
Support i2c_smbus_read_block_data. So Zl6100 driver can't work on these platforms.
1. drivers/i2c/busses/i2c-mpc.c(powerpc arch, which I am working on).
2. drivers/i2c/busses/i2c-mv64xxx.c
3. drivers/i2c/busses/i2c-nomadik.c
4. drivers/i2c/busses/i2c-octeon.c
5. drivers/i2c/busses/i2c-pxa.c
6. drivers/i2c/busses/i2c-s6000.c
Almost all the platforms the kernel support.
But they all support I2C_FUNC_SMBUS_EMUL, which means support i2c_smbus_read_i2c_block_data.
How can you explain this?

> > > > 2. Not all zlxx chips's id start with zlxx. zl6100's id starts
> > > > with 0x10. Take this situation into account.
> bus driver, the device driver never gets to see it, if you properly call
> i2c_smbus_read_block_data() as you are supposed to.
> 
> --
> Jean Delvare

Regards,
Yuantian


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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Guenter Roeck]

On Mon, Nov 14, 2011 at 09:28:40PM -0500, Tang Yuantian-B29983 wrote:
> Hi Guenter,
> 
> Please find my comments in line.
> 
> > -----Original Message-----
> > > > > SMBus block reads receive the block length as the first byte. 0x10
> > > > > = 16, which is the length of the ID for these chips, this is no
> > > I didn't write the driver, and don't have the device, but I think the
> > > driver is OK as is. The block length (0x10) is handled by the
> > > underlying bus driver, the device driver never gets to see it, if you
> > > properly call i2c_smbus_read_block_data() as you are supposed to.
> > >
> > I have eval boards for all chips supported by the driver (lots of kudos
> > to Intersil), so the driver was tested and is known to work for all chips
> > it currently supports. Intersil told me that they don't update the
> > firmware after they released a chip, so we can be sure that the firmware
> > version on all chips of the same type is the same, and we don't have to
> > look for version specific behavior.
> > 
> > I agree that we should not replace the SMBus block read with SMBus I2C
> > block read. The I2C bus driver should really implement SMBus block read
> > command support.
> > 
> > There are some I2C bus drivers which can not support SMBus block reads
> > due to chip limitations, such as the Sibyte driver, but if the driver can
> > support SMBus I2C block reads it should possible to support SMBus block
> > reads as well. As far as I can see, the only driver supporting SMBUS I2C
> > block reads but not SMBUS block reads is scx200_acb. That seems to be an
> > oversight (or maybe laziness ;), and should be easy to fix.
> > 
> [Yuantian:] what platform are you testing on?
> What I see is most i2c bus drivers don't support "SMBus block".

Actually, almost all drivers support it, via SMBus emulation code. And if a driver
supports SMBus I2C block commands, there is no technical reason why it should not
support SMBus block commands.

To answer your question, PC based systems, PowerPC based systems, Octeon based
systems, and a Sibyte based system (where block commands don't work).

Guenter

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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Guenter Roeck]

On Mon, Nov 14, 2011 at 09:35:44PM -0500, Tang Yuantian-B29983 wrote:
> Hi Jean Delvare,
> 
> Please find my comments in line.
> 
> > -----Original Message-----
> > 
> > Again, no. PMBus goes on top of SMBus which includes SMBus block reads
> > (i2c_smbus_read_block_data) but not I2C block reads
> > (i2c_smbus_read_i2c_block_data). As a result, many hardware SMBus
> > controllers do not support I2C block reads, and it can't be added (only
> > the set of transfers supported by the hardware controller can be
> > implemented by the driver) which means that your proposed change would
> > break support on existing systems. This is not acceptable.
> > 
> > On the other hand, an I2C controller which supports I2C block reads can
> > almost always get SMBus block reads added, it's only a matter of adding a
> > few lines of code to the bus driver.
> > 
> > Which i2c bus driver are you using? From which kernel version?
> > 
> [Yuantian:] I am using newest kernel 3.2. PowerPC arch. 
> I2c bus driver file is: drivers/i2c/busses/i2c-mpc.c.
> 
> I greped the i2c bus driver and found the following platform's i2c don't 
> Support i2c_smbus_read_block_data. So Zl6100 driver can't work on these platforms.
> 1. drivers/i2c/busses/i2c-mpc.c(powerpc arch, which I am working on).
> 2. drivers/i2c/busses/i2c-mv64xxx.c
> 3. drivers/i2c/busses/i2c-nomadik.c
> 4. drivers/i2c/busses/i2c-octeon.c
> 5. drivers/i2c/busses/i2c-pxa.c
> 6. drivers/i2c/busses/i2c-s6000.c
> Almost all the platforms the kernel support.
> But they all support I2C_FUNC_SMBUS_EMUL, which means support i2c_smbus_read_i2c_block_data.

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 ?

Thanks,
Guenter


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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Tang Yuantian-B29983]

> -----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.

> Thanks,
> Guenter
> 

Regards,
Yuantian


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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Tabi Timur-B04825]

Tang Yuantian-B29983 wrote:
> [Yuantian:] unfortunately no, Even I add I2C_FUNC_SMBUS_READ_BLOCK_DATA support,
> The return value is not correct.

Yuantian,

Are you saying that without your patch, the zl6100 won't work on the 
P1022DS?

-- 
Timur Tabi
Linux kernel developer at Freescale

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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Guenter Roeck]

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.
> 
I am not sure I understand what you mean with "not correct".

Can you provide the output of the i2cdump command ?
Command should be something like "i2cdump <bus> <addr> s 0xe4"
with a recent version of i2cdump.

Thanks,
Guenter

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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Guenter Roeck]

[-- 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

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Tang Yuantian-B29983]

UHJvYmxlbSBzb2x2ZWQuDQpUaGlzIGRyaXZlciBpcyBwZXJmZWN0Lg0KVGhlIGkyYyBidXMgZHJp
dmVyIG5lZWQgdG8gYmUgbW9kaWZpZWQgdG8gc3VwcG9ydCBJMkNfRlVOQ19TTUJVU19SRUFEX0JM
T0NLX0RBVEEsDQpKdXN0IGxpa2UgR3VlbnRlcidzIHBhdGNoLg0KTWF5YmUgb3RoZXIgaTJjIGJ1
cyBkcml2ZXJzIG5lZWQgdG8gYmUgbW9kaWZpZWQgdG9vLg0KDQpUaGFua3MgeW91IGd1eXMhDQoN
ClJlZ2FyZHMsDQpZdWFudGlhbg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZy
b206IEd1ZW50ZXIgUm9lY2sgW21haWx0bzpndWVudGVyLnJvZWNrQGVyaWNzc29uLmNvbV0NCj4g
U2VudDogMjAxMcTqMTHUwjE1yNUgMTI6MTUNCj4gVG86IFRhbmcgWXVhbnRpYW4tQjI5OTgzDQo+
IENjOiBKZWFuIERlbHZhcmU7IEh1YW5nIENoYW5nbWluZy1SNjYwOTM7IFRhYmkgVGltdXItQjA0
ODI1OyBsbS0NCj4gc2Vuc29yc0BsbS1zZW5zb3JzLm9yZw0KPiBTdWJqZWN0OiBSZTogW2xtLXNl
bnNvcnNdIFtQQVRDSF0gaHdtb246IChwbWJ1cykgRml4IHR3byBpc3N1ZXMNCj4gPiBJMkNfRlVO
Q19TTUJVU19SRUFEX0JMT0NLX0RBVEEgc3VwcG9ydCwgVGhlIHJldHVybiB2YWx1ZSBpcyBub3Qg
Y29ycmVjdC4NCj4gPg0KPiBBY3R1YWxseSwgeW91IGFyZSByaWdodC4gVGhlIGkyYy1tcGMgZHJp
dmVyIHdvdWxkIGFsc28gaGF2ZSB0byBoYW5kbGUgdGhlDQo+IEkyQ19NX1JFQ1ZfTEVOIGZsYWcs
IHdoZXJlIHRoZSAxc3QgcmVjZWl2ZWQgYnl0ZSBpbmRpY2F0ZXMgdGhlIHJlY2VpdmUNCj4gbGVu
Z3RoLiBUaGF0IHdvdWxkIHByb2JhYmx5IHJlcXVpcmUgYSBjaGFuZ2UgaW4gbXBjX3JlYWQoKSwg
dGVsbGluZyBpdA0KPiB0aGF0IHRoZSBmaXJzdCBieXRlIHRvIGJlIHJlY2VpdmVkIGlzIHRoZSBs
ZW5ndGguDQo+IFNvbWV0aGluZyBsaWtlIHRoZSBhdHRhY2hlZC4gQ29tcGxldGVseSB1bnRlc3Rl
ZCwgb2YgY291cnNlIC4uLg0KPiANCj4gR3VlbnRlcg0KX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fX18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29y
c0BsbS1zZW5zb3JzLm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0
aW5mby9sbS1zZW5zb3Jz

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Guenter Roeck]

T24gVHVlLCBOb3YgMTUsIDIwMTEgYXQgMTI6Mzc6MzVBTSAtMDUwMCwgVGFuZyBZdWFudGlhbi1C
Mjk5ODMgd3JvdGU6Cj4gUHJvYmxlbSBzb2x2ZWQuCj4gVGhpcyBkcml2ZXIgaXMgcGVyZmVjdC4K
PiBUaGUgaTJjIGJ1cyBkcml2ZXIgbmVlZCB0byBiZSBtb2RpZmllZCB0byBzdXBwb3J0IEkyQ19G
VU5DX1NNQlVTX1JFQURfQkxPQ0tfREFUQSwKPiBKdXN0IGxpa2UgR3VlbnRlcidzIHBhdGNoLgo+
IE1heWJlIG90aGVyIGkyYyBidXMgZHJpdmVycyBuZWVkIHRvIGJlIG1vZGlmaWVkIHRvby4KPiAK
PiBUaGFua3MgeW91IGd1eXMhCj4gCkhpIFl1YW50aWFuLAoKdGhhdCB3YXMgYSBuaWNlIGJsaW5k
IGZpeCAuLi4KCkknbGwgY3JlYXRlIGEgY2xlYW4gcGF0Y2ggYW5kIHN1Ym1pdCBmb3IgdGhlIGky
YyBzdWJzeXN0ZW0uCgpHdWVudGVyCgo+IFJlZ2FyZHMsCj4gWXVhbnRpYW4KPiAKPiA+IC0tLS0t
T3JpZ2luYWwgTWVzc2FnZS0tLS0tCj4gPiBGcm9tOiBHdWVudGVyIFJvZWNrIFttYWlsdG86Z3Vl
bnRlci5yb2Vja0Blcmljc3Nvbi5jb21dCj4gPiBTZW50OiAyMDEx5bm0MTHmnIgxNeaXpSAxMjox
NQo+ID4gVG86IFRhbmcgWXVhbnRpYW4tQjI5OTgzCj4gPiBDYzogSmVhbiBEZWx2YXJlOyBIdWFu
ZyBDaGFuZ21pbmctUjY2MDkzOyBUYWJpIFRpbXVyLUIwNDgyNTsgbG0tCj4gPiBzZW5zb3JzQGxt
LXNlbnNvcnMub3JnCj4gPiBTdWJqZWN0OiBSZTogW2xtLXNlbnNvcnNdIFtQQVRDSF0gaHdtb246
IChwbWJ1cykgRml4IHR3byBpc3N1ZXMKPiA+ID4gSTJDX0ZVTkNfU01CVVNfUkVBRF9CTE9DS19E
QVRBIHN1cHBvcnQsIFRoZSByZXR1cm4gdmFsdWUgaXMgbm90IGNvcnJlY3QuCj4gPiA+Cj4gPiBB
Y3R1YWxseSwgeW91IGFyZSByaWdodC4gVGhlIGkyYy1tcGMgZHJpdmVyIHdvdWxkIGFsc28gaGF2
ZSB0byBoYW5kbGUgdGhlCj4gPiBJMkNfTV9SRUNWX0xFTiBmbGFnLCB3aGVyZSB0aGUgMXN0IHJl
Y2VpdmVkIGJ5dGUgaW5kaWNhdGVzIHRoZSByZWNlaXZlCj4gPiBsZW5ndGguIFRoYXQgd291bGQg
cHJvYmFibHkgcmVxdWlyZSBhIGNoYW5nZSBpbiBtcGNfcmVhZCgpLCB0ZWxsaW5nIGl0Cj4gPiB0
aGF0IHRoZSBmaXJzdCBieXRlIHRvIGJlIHJlY2VpdmVkIGlzIHRoZSBsZW5ndGguCj4gPiBTb21l
dGhpbmcgbGlrZSB0aGUgYXR0YWNoZWQuIENvbXBsZXRlbHkgdW50ZXN0ZWQsIG9mIGNvdXJzZSAu
Li4KPiA+IAo+ID4gR3VlbnRlcgoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3Jz
Lm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5z
b3Jz

Re: [lm-sensors] [PATCH] hwmon: (pmbus) Fix two issues

[Jean Delvare]

On Mon, 14 Nov 2011 18:54:22 -0800, Guenter Roeck wrote:
> On Mon, Nov 14, 2011 at 09:35:44PM -0500, Tang Yuantian-B29983 wrote:
> > [Yuantian:] I am using newest kernel 3.2. PowerPC arch. 
> > I2c bus driver file is: drivers/i2c/busses/i2c-mpc.c.
> > 
> > I greped the i2c bus driver and found the following platform's i2c don't 
> > Support i2c_smbus_read_block_data. So Zl6100 driver can't work on these platforms.
> > 1. drivers/i2c/busses/i2c-mpc.c(powerpc arch, which I am working on).
> > 2. drivers/i2c/busses/i2c-mv64xxx.c
> > 3. drivers/i2c/busses/i2c-nomadik.c
> > 4. drivers/i2c/busses/i2c-octeon.c
> > 5. drivers/i2c/busses/i2c-pxa.c
> > 6. drivers/i2c/busses/i2c-s6000.c
> > Almost all the platforms the kernel support.
> > But they all support I2C_FUNC_SMBUS_EMUL, which means support i2c_smbus_read_i2c_block_data.
> 
> 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 ?

Because I2C_FUNC_SMBUS_READ_BLOCK_DATA needs proper handling of
I2C_M_RECV_LEN, which some drivers have implemented and others have
not. Bus driver authors are expected to add
I2C_FUNC_SMBUS_READ_BLOCK_DATA (and I2C_FUNC_SMBUS_BLOCK_PROC_CALL) to
functionality after implementing I2C_M_RECV_LEN.

-- 
Jean Delvare

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