public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jean Delvare <khali@linux-fr.org>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Greg KH <greg@kroah.com>,
	Linux Kernel list <linux-kernel@vger.kernel.org>,
	Adrian Bunk <bunk@stusta.de>
Subject: Re: i2c/ smbus question
Date: Sun, 8 Jan 2006 11:30:13 +0100	[thread overview]
Message-ID: <20060108113013.34fe5447.khali@linux-fr.org> (raw)
In-Reply-To: <1136673364.30123.20.camel@localhost.localdomain>

Hi Benjamin,

On January 7th, 2006, Benjamin Herrenschmidt asked:
> Can you confirm the difference between writing a block of data with
> I2C_SMBUS_BLOCK_DATA vs I2C_SMBUS_I2C_BLOCK_DATA on the wire ? It's my
> understanding that the former will actually send the length byte on the
> wire before the data while the later won't, though they both send a
> subaddress.

I confirm you are correct.

> I'm completely rewriting the powermac i2c support (consolidating all
> busses behind a low level layer that I need to use in circumstances
> where the linux i2c layer isn't useable, and with a single driver in
> drivers/i2c/busses/* replacing i2c-keywest.c and i2c-pmac-smu.c) and I
> think I'm hitting a problem where i2c-keywest didn't implement
> I2C_SMBUS_BLOCK_DATA properly (didn't send the length byte) and some
> drivers (our sound drivers) rely on that behaviour (that's fine, I can
> fix them too, I just want to make sure I understand what the semantic
> should be).

I just took a quick look at i2c-keywest.c and you seem to right: the
driver supposedly implements SMBus block transfers
(I2C_SMBUS_BLOCK_DATA) but the actual implementation pretty much looks
like I2C block transfers (I2C_SMBUS_I2C_BLOCK_DATA). Good catch.

> I'm a bit surprised that there seem to be no wrapper for writing with
> I2C_SMBUS_I2C_BLOCK_DATA, only for reading, in i2c-core.c since it
> appears to me that it's the most common one, at least for all devices
> I've dealt with so far (mostly sound & clock chips in addition to
> sensors)...

I suspect that these drivers which do I2C block writes do so by calling
i2c_master_send (or even i2c_transfer) directly, rather than relying on
the SMBus compatibility layer.

The i2c_smbus_write_i2c_block_data wrapper function used to be defined,
but was removed in 2.6.10-rc2 together with a couple other similar
wrappers [1] on request by Adrian Bunk, the reason being that they had
no user back then. I was a bit reluctant at first, but we finally agreed
with Adrian to remove the functions, and to reintroduce them later if
they were ever needed.

So, if you need i2c_smbus_write_i2c_block_data(), we can easily
resurrect it. See the patch below. I made the new version a bit faster
(I hope) than the original by using memcpy, please confirm it works for
you.

[1] http://jdelvare.net2.nerim.net/quian/2.6-wrc/diff.php?patch=patch-2.6.10-rc1-rc2.bz2&file=include/linux/i2c.h
    http://jdelvare.net2.nerim.net/quian/2.6-wrc/diff.php?patch=patch-2.6.10-rc1-rc2.bz2&file=drivers/i2c/i2c-core.c

* * * * *

Resurrect i2c_smbus_write_i2c_block_data.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
---
 drivers/i2c/i2c-core.c |   15 +++++++++++++++
 include/linux/i2c.h    |    3 +++
 2 files changed, 18 insertions(+)

--- linux-2.6.15-git.orig/drivers/i2c/i2c-core.c	2006-01-08 10:55:58.000000000 +0100
+++ linux-2.6.15-git/drivers/i2c/i2c-core.c	2006-01-08 11:17:57.000000000 +0100
@@ -948,6 +948,20 @@
 	}
 }
 
+s32 i2c_smbus_write_i2c_block_data(struct i2c_client *client, u8 command,
+				   u8 length, u8 *values)
+{
+	union i2c_smbus_data data;
+
+	if (length > I2C_SMBUS_BLOCK_MAX)
+		length = I2C_SMBUS_BLOCK_MAX;
+	data.block[0] = length;
+	memcpy(data.block + 1, values, length);
+	return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+			      I2C_SMBUS_WRITE, command,
+			      I2C_SMBUS_I2C_BLOCK_DATA, &data);
+}
+
 /* Simulate a SMBus command using the i2c protocol 
    No checking of parameters is done!  */
 static s32 i2c_smbus_xfer_emulated(struct i2c_adapter * adapter, u16 addr, 
@@ -1152,6 +1166,7 @@
 EXPORT_SYMBOL(i2c_smbus_write_word_data);
 EXPORT_SYMBOL(i2c_smbus_write_block_data);
 EXPORT_SYMBOL(i2c_smbus_read_i2c_block_data);
+EXPORT_SYMBOL(i2c_smbus_write_i2c_block_data);
 
 MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
 MODULE_DESCRIPTION("I2C-Bus main module");
--- linux-2.6.15-git.orig/include/linux/i2c.h	2006-01-08 10:56:08.000000000 +0100
+++ linux-2.6.15-git/include/linux/i2c.h	2006-01-08 11:02:00.000000000 +0100
@@ -100,6 +100,9 @@
 /* Returns the number of read bytes */
 extern s32 i2c_smbus_read_i2c_block_data(struct i2c_client * client,
 					 u8 command, u8 *values);
+extern s32 i2c_smbus_write_i2c_block_data(struct i2c_client * client,
+					  u8 command, u8 length,
+					  u8 *values);
 
 /*
  * A driver is capable of handling one or more physical devices present on

-- 
Jean Delvare

  reply	other threads:[~2006-01-08 10:30 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-01-07 22:36 i2c/ smbus question Benjamin Herrenschmidt
2006-01-08 10:30 ` Jean Delvare [this message]
2006-01-08 21:10   ` Benjamin Herrenschmidt
2006-01-08 22:58   ` Benjamin Herrenschmidt
2006-01-09  3:53     ` Greg KH
2006-01-09  4:19       ` Benjamin Herrenschmidt

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=20060108113013.34fe5447.khali@linux-fr.org \
    --to=khali@linux-fr.org \
    --cc=benh@kernel.crashing.org \
    --cc=bunk@stusta.de \
    --cc=greg@kroah.com \
    --cc=linux-kernel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox