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.pinch...@ideasonboard.com>
> ---
>  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-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to