diff mbox

[V2,5/9] I2C: Add smbus quick read/write helper function

Message ID 1398695268-28645-6-git-send-email-tianyu.lan@intel.com
State Superseded
Headers show

Commit Message

Lan Tianyu April 28, 2014, 2:27 p.m. UTC
Add i2c_smbus_quick_write/read() helper function. These will be used
in the implementation of i2c ACPI address space handler.

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 drivers/i2c/i2c-core.c | 30 ++++++++++++++++++++++++++++++
 include/linux/i2c.h    |  2 ++
 2 files changed, 32 insertions(+)

Comments

Wolfram Sang May 17, 2014, 9:41 a.m. UTC | #1
On Mon, Apr 28, 2014 at 10:27:44PM +0800, Lan Tianyu wrote:
> Add i2c_smbus_quick_write/read() helper function. These will be used
> in the implementation of i2c ACPI address space handler.
> 
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>

We had such a function once but removed because of no users. Please
check 67c2e66571c383404a5acd08189194da660da942 what it takes to bring
them back. Especially missing are documentation updates...

> +s32 i2c_smbus_quick_write(const struct i2c_client *client)

... and I like the original function much better.

1) It is named *_write_quick which follows other function name patterns
2) It uses a parameter for the r/w bit. Make sense to me, since this bit
is the information we send to the device. quick_read doesn't make sense
to me. We don't receive a bit from the device.
Lan Tianyu May 17, 2014, 1:13 p.m. UTC | #2
On 05/17/2014 05:41 PM, Wolfram Sang wrote:
> On Mon, Apr 28, 2014 at 10:27:44PM +0800, Lan Tianyu wrote:
>> Add i2c_smbus_quick_write/read() helper function. These will be used
>> in the implementation of i2c ACPI address space handler.
>>
>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>
> We had such a function once but removed because of no users. Please
> check 67c2e66571c383404a5acd08189194da660da942 what it takes to bring
> them back. Especially missing are documentation updates...
>
>> +s32 i2c_smbus_quick_write(const struct i2c_client *client)
>
> ... and I like the original function much better.
>
> 1) It is named *_write_quick which follows other function name patterns
> 2) It uses a parameter for the r/w bit. Make sense to me, since this bit
> is the information we send to the device. quick_read doesn't make sense
> to me. We don't receive a bit from the device.
>

Hi Wolfram:
	Great thanks for your review. Ok. I will follow commit 67c2e665 to bring 
i2c_smbus_write_quick(struct i2c_client *client, u8 value) back.

BTW, how about i2c_probe_func_quick_read()? Should we replace it with the 
original i2c_smbus_write_quick()?

	

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang May 17, 2014, 5:15 p.m. UTC | #3
> >2) It uses a parameter for the r/w bit. Make sense to me, since this bit
> >is the information we send to the device. quick_read doesn't make sense
> >to me. We don't receive a bit from the device.
> >
> 
> Hi Wolfram:
> 	Great thanks for your review. Ok. I will follow commit 67c2e665 to
> bring i2c_smbus_write_quick(struct i2c_client *client, u8 value)
> back.
> 
> BTW, how about i2c_probe_func_quick_read()? Should we replace it
> with the original i2c_smbus_write_quick()?

As said above "quick_read doesn't make sense to me. We don't receive a
bit from the device."

Start implementing and you will see why we don't need it :)
diff mbox

Patch

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 7c7f4b8..3bf0048 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -2162,6 +2162,36 @@  static int i2c_smbus_check_pec(u8 cpec, struct i2c_msg *msg)
 }
 
 /**
+ * i2c_smbus_quick_write - SMBus "quick write" protocol
+ * @client: Handle to slave device
+ *
+ * This executes the SMBus "quick write" protocol, returning negative errno
+ * else zero on success.
+ */
+s32 i2c_smbus_quick_write(const struct i2c_client *client)
+{
+	return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+				I2C_SMBUS_WRITE, 0,
+				I2C_SMBUS_QUICK, NULL);
+}
+EXPORT_SYMBOL_GPL(i2c_smbus_quick_write);
+
+/**
+ * i2c_smbus_quick_read - SMBus "quick read" protocol
+ * @client: Handle to slave device
+ *
+ * This executes the SMBus "quick read" protocol, returning negative errno
+ * else zero on success.
+ */
+s32 i2c_smbus_quick_read(const struct i2c_client *client)
+{
+	return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
+				I2C_SMBUS_READ, 0,
+				I2C_SMBUS_QUICK, NULL);
+}
+EXPORT_SYMBOL_GPL(i2c_smbus_quick_read);
+
+/**
  * i2c_smbus_read_byte - SMBus "receive byte" protocol
  * @client: Handle to slave device
  *
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b556e0a..3e6ea90 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -82,6 +82,8 @@  extern s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
 /* Now follow the 'nice' access routines. These also document the calling
    conventions of i2c_smbus_xfer. */
 
+extern s32 i2c_smbus_quick_write(const struct i2c_client *client);
+extern s32 i2c_smbus_quick_read(const struct i2c_client *client);
 extern s32 i2c_smbus_read_byte(const struct i2c_client *client);
 extern s32 i2c_smbus_write_byte(const struct i2c_client *client, u8 value);
 extern s32 i2c_smbus_read_byte_data(const struct i2c_client *client,