Salut Laurent,
On Tue, 26 Jun 2012 16:17:07 +0200, Laurent Pinchart wrote:
> SCCB is a serial communication bus developed by Omnivision. Its 2-wire
> mode is very similar to SMBus byte data transactions, but requires the
> controller to ignore the ACK bit and to insert a stop condition after
> each message.
>
> Add a device SCCB flag and a message stop flag to be passed to
> controller drivers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/i2c/i2c-core.c | 13 ++++++++++++-
> include/linux/i2c.h | 2 ++
> 2 files changed, 14 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index feb7dc3..8cfa660 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -1939,6 +1939,12 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
> u8 partial_pec = 0;
> int status;
>
> + if (unlikely(flags & I2C_CLIENT_SCCB) && size != I2C_SMBUS_BYTE_DATA) {
> + dev_err(&adapter->dev,
> + "SCCB devices only support I2C_SMBUS_BYTE_DATA\n");
> + return -EINVAL;
> + }
> +
I'm not sure if we really want this. If the SCCB protocol evolves,
we'll have to loosen the check. If a devices follows SCCB for byte data
transactions and I2C/SMBus for others, it won't work. Plus it slows
down the function a bit, to catch a developer error which would not
result in anything catastrophic anyway.
I propose that we either drop the check completely (my preference) or
make it depend on DEBUG.
> msgbuf0[0] = command;
> switch (size) {
> case I2C_SMBUS_QUICK:
> @@ -1956,6 +1962,11 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr,
> }
> break;
> case I2C_SMBUS_BYTE_DATA:
> + if (unlikely(flags & I2C_CLIENT_SCCB)) {
> + msg[0].flags |= I2C_M_IGNORE_NAK | I2C_M_STOP;
> + msg[1].flags |= I2C_M_IGNORE_NAK | I2C_M_STOP;
> + }
> +
> if (read_write == I2C_SMBUS_READ)
> msg[1].len = 1;
> else {
> @@ -2105,7 +2116,7 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
> int try;
> s32 res;
>
> - flags &= I2C_M_TEN | I2C_CLIENT_PEC;
> + flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
>
> if (adapter->algo->smbus_xfer) {
> i2c_lock_adapter(adapter);
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 195d8b3..bd42914 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -420,6 +420,7 @@ void i2c_lock_adapter(struct i2c_adapter *);
> void i2c_unlock_adapter(struct i2c_adapter *);
>
> /*flags for the client struct: */
> +#define I2C_CLIENT_SCCB 0x02 /* Use Omnivision SCCB protocol */
> #define I2C_CLIENT_PEC 0x04 /* Use Packet Error Checking */
> #define I2C_CLIENT_TEN 0x10 /* we have a ten bit chip address */
> /* Must equal I2C_M_TEN below */
> @@ -540,6 +541,7 @@ struct i2c_msg {
> __u16 flags;
> #define I2C_M_TEN 0x0010 /* this is a ten bit chip address */
> #define I2C_M_RD 0x0001 /* read data, from slave to master */
> +#define I2C_M_STOP 0x8000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> #define I2C_M_NOSTART 0x4000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> #define I2C_M_REV_DIR_ADDR 0x2000 /* if I2C_FUNC_PROTOCOL_MANGLING */
> #define I2C_M_IGNORE_NAK 0x1000 /* if I2C_FUNC_PROTOCOL_MANGLING */
Other than this your patch looks fine, I'll apply it, thanks.
--
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
