[RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[Laurent Pinchart]

Hi everybody,

While trying to use an OV7725 camera sensor with an OMAP3 system I ran into a
couple of issues related to the sensor communication protocol. Instead of
using the obiquitous I2C protocol, Omnivision invited the SCCB communication
bus, very similar to I2C but different enough not to be compatible.

SCCB documentation is available at
http://www.ovt.com/download_document.php?type=document&DID=63. SCCB exists in
two flavors, 3-wire and 2-wire. The 3-wire version is too different from I2C
to be of interest here.

SCCB is very close to SMBus. It supports two transactions, an 8-bit register
read and an 8-bit register write. The write transaction transmits a 3-byte
message (addr/w, reg, data). The read transaction transmits 2 2-byte messages
(addr/w, reg, followed by addr/r, data).

The two majors differences between I2C and SCCB are in the acknowledgment bit
and in the stop bit.

SCCB devices generate no ack bit and don't take the ack bit generated by the
master into account. The ack bit is thus an unspecified bit, present in the
transfer but whose value must be ignored. However, in practice, the SCCB
components I've seen implement the ack bit in write transactions.

The stop bit is handled as in I2C, except that a stop bit needs to be
generated between the two messages that make the read transaction (as opposed
to SMBus, where no stop bit must be present between the two messages).

I need SCCB support in the I2C core and in the OMAP3 I2C driver and have two
options. The OMAP3 I2C controller support SCCB natively. The interface exposed
by the hardware is at the transaction level (smbus_xfer), while the i2c-omap
driver exposes a message level interface (master_xfer). To use the native SCCB
support, we would thus have to either allow i2c_smbus_xfer() to choose between
smbus_xfer and i2c_smbus_xfer_emulated based on the client type (I2C or SCCB),
or export i2c_smbus_xfer_emulated() to use it as a fallback in the i2c-omap
driver for I2C clients.

Another option is not to use the hardware SCCB support, but to emulate it
through I2C. In that case the i2c-omap driver must generate a stop bit after
the first message of a read transaction, and ignore the ack bit. Two flags
would then be necessary, one in the message to force the stop bit, and one in
the i2c_client to mark the device as using SCCB.

Implementing support for the second option is required for I2C masters that
have no hardware SCCB support. As the first option won't bring much benefits,
I've decided to skip it for now. The following three patches thus implement
emulated SCCB support in the I2C core, with a small change in the i2c-omap
driver to support the new I2C_M_STOP flag.

Laurent Pinchart (3):
  i2c: Add SCCB support
  i2c: Fall back to emulated SMBus if the operation isn't supported
    natively
  i2c: omap: Add support for I2C_M_STOP message flag

 drivers/i2c/busses/i2c-omap.c |    2 ++
 drivers/i2c/i2c-core.c        |   22 +++++++++++++++++++---
 include/linux/i2c.h           |    2 ++
 3 files changed, 23 insertions(+), 3 deletions(-)

-- 
Regards,

Laurent Pinchart

[RFC/PATCH 1/3] i2c: Add SCCB support

[Laurent Pinchart]

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@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;
+	}
+
 	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 */
-- 
1.7.3.4

[RFC/PATCH 2/3] i2c: Fall back to emulated SMBus if the operation isn't supported natively

[Laurent Pinchart]

Adapter drivers might support only a subset of the SMBus operations
natively. Those drivers currently have to manually emulate unsupported
operations using I2C.

Make the i2c_smbus_xfer() function fall back to
i2c_smbus_xfer_emulated() when the adapter's .smbus_xfer() operation
returns -EOPNOTSUPP, like it already does when the .smbus_xfer()
operation isn't available at all.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/i2c/i2c-core.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 8cfa660..16e750e 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -2113,8 +2113,8 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
 		   union i2c_smbus_data *data)
 {
 	unsigned long orig_jiffies;
+	s32 res = -EOPNOTSUPP;
 	int try;
-	s32 res;
 
 	flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
 
@@ -2134,7 +2134,12 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
 				break;
 		}
 		i2c_unlock_adapter(adapter);
-	} else
+	}
+
+	/* Fall back to i2c_smbus_xfer_emulated of the adapter doesn't implement
+	 * native support for the SMBus operation.
+	 */
+	if (res == -EOPNOTSUPP && adapter->algo->master_xfer)
 		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
 					      command, protocol, data);
 
-- 
1.7.3.4

[RFC/PATCH 3/3] i2c: omap: Add support for I2C_M_STOP message flag

[Laurent Pinchart]

Generate a stop condition after each message marked with I2C_M_STOP.

Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 drivers/i2c/busses/i2c-omap.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 801df60..cf1bda0 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -545,6 +545,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	if (dev->speed > 400)
 		w |= OMAP_I2C_CON_OPMODE_HS;
 
+	if (msg->flags & I2C_M_STOP)
+		stop = 1;
 	if (msg->flags & I2C_M_TEN)
 		w |= OMAP_I2C_CON_XA;
 	if (!(msg->flags & I2C_M_RD))
-- 
1.7.3.4

Re: [RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[jean-philippe francois]

2012/6/26 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> Hi everybody,
>
> While trying to use an OV7725 camera sensor with an OMAP3 system I ran into a
> couple of issues related to the sensor communication protocol. Instead of
> using the obiquitous I2C protocol, Omnivision invited the SCCB communication
> bus, very similar to I2C but different enough not to be compatible.
>

I have been using omnivision sensor without being aware of this issue,
and without any i2c reliability issue either.

Here is typical code I use :

/* This function is used to read value from register for i2c client */
static int ov2710_read(struct i2c_client *client, unsigned short reg,
unsigned char *val)
{
    int err = 0;
    unsigned char outbuf[] = {(reg>>8)&0xff, reg&0xff};
    unsigned char inbuf[1];
    struct i2c_msg msg[] = {
        { .addr = client->addr, .flags = 0, .buf = outbuf, .len = 2 },
        { .addr = client->addr, .flags = I2C_M_RD, .buf = inbuf, .len = 1 },
    };
    err = i2c_transfer(client->adapter, msg, 2);
    /* error handling and pass by ref handling */
    ....
}

Is the point of this patch to avoid writing such functions again and
again in every ov driver ?
What is solved in practice by this patch that is not solved by a
regular i2c transfer ?

Regards,
Jean-Philippe François

> SCCB documentation is available at
> http://www.ovt.com/download_document.php?type=document&DID=63. SCCB exists in
> two flavors, 3-wire and 2-wire. The 3-wire version is too different from I2C
> to be of interest here.
>
> SCCB is very close to SMBus. It supports two transactions, an 8-bit register
> read and an 8-bit register write. The write transaction transmits a 3-byte
> message (addr/w, reg, data). The read transaction transmits 2 2-byte messages
> (addr/w, reg, followed by addr/r, data).
>
> The two majors differences between I2C and SCCB are in the acknowledgment bit
> and in the stop bit.
>
> SCCB devices generate no ack bit and don't take the ack bit generated by the
> master into account. The ack bit is thus an unspecified bit, present in the
> transfer but whose value must be ignored. However, in practice, the SCCB
> components I've seen implement the ack bit in write transactions.
>
> The stop bit is handled as in I2C, except that a stop bit needs to be
> generated between the two messages that make the read transaction (as opposed
> to SMBus, where no stop bit must be present between the two messages).
>
> I need SCCB support in the I2C core and in the OMAP3 I2C driver and have two
> options. The OMAP3 I2C controller support SCCB natively. The interface exposed
> by the hardware is at the transaction level (smbus_xfer), while the i2c-omap
> driver exposes a message level interface (master_xfer). To use the native SCCB
> support, we would thus have to either allow i2c_smbus_xfer() to choose between
> smbus_xfer and i2c_smbus_xfer_emulated based on the client type (I2C or SCCB),
> or export i2c_smbus_xfer_emulated() to use it as a fallback in the i2c-omap
> driver for I2C clients.
>
> Another option is not to use the hardware SCCB support, but to emulate it
> through I2C. In that case the i2c-omap driver must generate a stop bit after
> the first message of a read transaction, and ignore the ack bit. Two flags
> would then be necessary, one in the message to force the stop bit, and one in
> the i2c_client to mark the device as using SCCB.
>
> Implementing support for the second option is required for I2C masters that
> have no hardware SCCB support. As the first option won't bring much benefits,
> I've decided to skip it for now. The following three patches thus implement
> emulated SCCB support in the I2C core, with a small change in the i2c-omap
> driver to support the new I2C_M_STOP flag.
>
> Laurent Pinchart (3):
>  i2c: Add SCCB support
>  i2c: Fall back to emulated SMBus if the operation isn't supported
>    natively
>  i2c: omap: Add support for I2C_M_STOP message flag
>
>  drivers/i2c/busses/i2c-omap.c |    2 ++
>  drivers/i2c/i2c-core.c        |   22 +++++++++++++++++++---
>  include/linux/i2c.h           |    2 ++
>  3 files changed, 23 insertions(+), 3 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[Laurent Pinchart]

Hi Jean-Philippe,

On Tuesday 26 June 2012 16:49:35 jean-philippe francois wrote:
> 2012/6/26 Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> > Hi everybody,
> > 
> > While trying to use an OV7725 camera sensor with an OMAP3 system I ran
> > into a couple of issues related to the sensor communication protocol.
> > Instead of using the obiquitous I2C protocol, Omnivision invited the SCCB
> > communication bus, very similar to I2C but different enough not to be
> > compatible.
> I have been using omnivision sensor without being aware of this issue,
> and without any i2c reliability issue either.
> 
> Here is typical code I use :
> 
> /* This function is used to read value from register for i2c client */
> static int ov2710_read(struct i2c_client *client, unsigned short reg,
> unsigned char *val)
> {
>     int err = 0;
>     unsigned char outbuf[] = {(reg>>8)&0xff, reg&0xff};

According to the SCCB specification (or at least the version I've found) 
register addresses are 8-bit. The OV2710 might just be I2C-compatible.

>     unsigned char inbuf[1];
>     struct i2c_msg msg[] = {
>         { .addr = client->addr, .flags = 0, .buf = outbuf, .len = 2 },
>         { .addr = client->addr, .flags = I2C_M_RD, .buf = inbuf, .len = 1 },
> };
>     err = i2c_transfer(client->adapter, msg, 2);
>     /* error handling and pass by ref handling */
>     ....
> }

The ov772x driver uses i2c_smbus_write_byte_data() and 
i2c_smbus_read_byte_data(). The later calls

i2c_smbus_xfer(client->adapter, client->addr, client->flags,
               I2C_SMBUS_READ, I2C_SMBUS_BYTE_DATA, &data);

which calls i2c_smbus_xfer_emulated() for hosts that don't support SMBus 
transfers natively, and that's pretty much equivalent to your above code 
(except for the 8/16 bit register address).

It might be a good idea to implement i2c_smbus_*-like functions for 16-bit 
register addresses in the I2C core, they could be reused across drivers.

> Is the point of this patch to avoid writing such functions again and again
> in every ov driver ?

No, but that's a good idea as well :-)

> What is solved in practice by this patch that is not solved by a regular i2c
> transfer ?

The above code will not ignore acks/nacks and will not generate a stop 
condition between the two messages. The SCCB specification states that 
acks/nacks must be ignored and that a stop condition must be generated. The 
OV7725 handles acks/nacks correctly, but chokes if no stop condition is 
generated. The point of this patch set is to fix both problems (although the 
acks/nacks issue might be theoretical only).

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[Gary Thomas]

On 2012-06-26 10:20, Laurent Pinchart wrote:
> Hi Jean-Philippe,
>
> On Tuesday 26 June 2012 16:49:35 jean-philippe francois wrote:
>> 2012/6/26 Laurent Pinchart<laurent.pinchart@ideasonboard.com>:
>>> Hi everybody,
>>>
>>> While trying to use an OV7725 camera sensor with an OMAP3 system I ran
>>> into a couple of issues related to the sensor communication protocol.
>>> Instead of using the obiquitous I2C protocol, Omnivision invited the SCCB
>>> communication bus, very similar to I2C but different enough not to be
>>> compatible.
>> I have been using omnivision sensor without being aware of this issue,
>> and without any i2c reliability issue either.
>>
>> Here is typical code I use :
>>
>> /* This function is used to read value from register for i2c client */
>> static int ov2710_read(struct i2c_client *client, unsigned short reg,
>> unsigned char *val)
>> {
>>      int err = 0;
>>      unsigned char outbuf[] = {(reg>>8)&0xff, reg&0xff};
>
> According to the SCCB specification (or at least the version I've found)
> register addresses are 8-bit. The OV2710 might just be I2C-compatible.
>
>>      unsigned char inbuf[1];
>>      struct i2c_msg msg[] = {
>>          { .addr = client->addr, .flags = 0, .buf = outbuf, .len = 2 },
>>          { .addr = client->addr, .flags = I2C_M_RD, .buf = inbuf, .len = 1 },
>> };
>>      err = i2c_transfer(client->adapter, msg, 2);
>>      /* error handling and pass by ref handling */
>>      ....
>> }
>
> The ov772x driver uses i2c_smbus_write_byte_data() and
> i2c_smbus_read_byte_data(). The later calls
>
> i2c_smbus_xfer(client->adapter, client->addr, client->flags,
>                 I2C_SMBUS_READ, I2C_SMBUS_BYTE_DATA,&data);
>
> which calls i2c_smbus_xfer_emulated() for hosts that don't support SMBus
> transfers natively, and that's pretty much equivalent to your above code
> (except for the 8/16 bit register address).
>
> It might be a good idea to implement i2c_smbus_*-like functions for 16-bit
> register addresses in the I2C core, they could be reused across drivers.
>
>> Is the point of this patch to avoid writing such functions again and again
>> in every ov driver ?
>
> No, but that's a good idea as well :-)
>
>> What is solved in practice by this patch that is not solved by a regular i2c
>> transfer ?
>
> The above code will not ignore acks/nacks and will not generate a stop
> condition between the two messages. The SCCB specification states that
> acks/nacks must be ignored and that a stop condition must be generated. The
> OV7725 handles acks/nacks correctly, but chokes if no stop condition is
> generated. The point of this patch set is to fix both problems (although the
> acks/nacks issue might be theoretical only).

How does it "choke"?  I've had success talking to the OV8820 using the
normal I2C driver as well.

-- 
------------------------------------------------------------
Gary Thomas                 |  Consulting for the
MLB Associates              |    Embedded world
------------------------------------------------------------

Re: [RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[Laurent Pinchart]

Hi Gary,

On Tuesday 26 June 2012 10:25:58 Gary Thomas wrote:
> On 2012-06-26 10:20, Laurent Pinchart wrote:
> > On Tuesday 26 June 2012 16:49:35 jean-philippe francois wrote:
> >> 2012/6/26 Laurent Pinchart<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>:
> >>> Hi everybody,
> >>> 
> >>> While trying to use an OV7725 camera sensor with an OMAP3 system I ran
> >>> into a couple of issues related to the sensor communication protocol.
> >>> Instead of using the obiquitous I2C protocol, Omnivision invited the
> >>> SCCB communication bus, very similar to I2C but different enough not to
> >>> be compatible.
> >> 
> >> I have been using omnivision sensor without being aware of this issue,
> >> and without any i2c reliability issue either.
> >> 
> >> Here is typical code I use :
> >> 
> >> /* This function is used to read value from register for i2c client */
> >> static int ov2710_read(struct i2c_client *client, unsigned short reg,
> >> unsigned char *val)
> >> {
> >> 
> >>      int err = 0;
> >>      unsigned char outbuf[] = {(reg>>8)&0xff, reg&0xff};
> > 
> > According to the SCCB specification (or at least the version I've found)
> > register addresses are 8-bit. The OV2710 might just be I2C-compatible.
> > 
> >>      unsigned char inbuf[1];
> >>      struct i2c_msg msg[] = {
> >>      
> >>          { .addr = client->addr, .flags = 0, .buf = outbuf, .len = 2 },
> >>          { .addr = client->addr, .flags = I2C_M_RD, .buf = inbuf, .len =
> >>          1 },
> >> 
> >> };
> >> 
> >>      err = i2c_transfer(client->adapter, msg, 2);
> >>      /* error handling and pass by ref handling */
> >>      ....
> >> 
> >> }
> > 
> > The ov772x driver uses i2c_smbus_write_byte_data() and
> > i2c_smbus_read_byte_data(). The later calls
> > 
> > i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> > 
> >                 I2C_SMBUS_READ, I2C_SMBUS_BYTE_DATA,&data);
> > 
> > which calls i2c_smbus_xfer_emulated() for hosts that don't support SMBus
> > transfers natively, and that's pretty much equivalent to your above code
> > (except for the 8/16 bit register address).
> > 
> > It might be a good idea to implement i2c_smbus_*-like functions for 16-bit
> > register addresses in the I2C core, they could be reused across drivers.
> > 
> >> Is the point of this patch to avoid writing such functions again and
> >> again in every ov driver ?
> > 
> > No, but that's a good idea as well :-)
> > 
> >> What is solved in practice by this patch that is not solved by a regular
> >> i2c transfer ?
> > 
> > The above code will not ignore acks/nacks and will not generate a stop
> > condition between the two messages. The SCCB specification states that
> > acks/nacks must be ignored and that a stop condition must be generated.
> > The OV7725 handles acks/nacks correctly, but chokes if no stop condition
> > is generated. The point of this patch set is to fix both problems
> > (although the acks/nacks issue might be theoretical only).
> 
> How does it "choke"?  I've had success talking to the OV8820 using the
> normal I2C driver as well.

With the patches applied:

[   23.277893] omap3isp omap3isp: Revision 15.0 found
[   23.283721] omap-iommu omap-iommu.0: isp: version 1.1
[   25.244079] ov772x 2-0021: ov7725 Product ID 77:21 Manufacturer ID 7f:a2

Without the patches applied:

[   23.505493] omap3isp omap3isp: Revision 15.0 found
[   23.511138] omap-iommu omap-iommu.0: isp: version 1.1
[   25.552246] omap_i2c omap_i2c.2: Arbitration lost
[   26.583160] omap_i2c omap_i2c.2: timeout waiting for bus ready
[   27.598785] omap_i2c omap_i2c.2: timeout waiting for bus ready
[   28.614196] omap_i2c omap_i2c.2: timeout waiting for bus ready
[   28.623260] isp_register_subdev_group: Unable to register subdev ov772x

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[Jean Delvare]

Salut Laurent,

On Tue, 26 Jun 2012 18:20:33 +0200, Laurent Pinchart wrote:
> It might be a good idea to implement i2c_smbus_*-like functions for 16-bit 
> register addresses in the I2C core, they could be reused across drivers.

Except that they would preferably not be called i2c_smbus_*, not being
part of the SMBus subset nor being commonly implemented by SMBus-only
controllers. Finding a proper name for these helpers function might be
tough, as may be figuring out which ones you really want to implement
and which ones aren't worth it.

Also note that this is more or less what regmap is trying to do, so you
may want to check in that direction (see drivers/base/regmap) first.

-- 
Jean Delvare

Re: [RFC/PATCH 0/3] OMAP3 I2C/SCCB support

[Laurent Pinchart]

Hi Jean,

On Tuesday 26 June 2012 18:56:16 Jean Delvare wrote:
> On Tue, 26 Jun 2012 18:20:33 +0200, Laurent Pinchart wrote:
> > It might be a good idea to implement i2c_smbus_*-like functions for 16-bit
> > register addresses in the I2C core, they could be reused across drivers.
> 
> Except that they would preferably not be called i2c_smbus_*, not being
> part of the SMBus subset nor being commonly implemented by SMBus-only
> controllers.

Sure. We need another name.

> Finding a proper name for these helpers function might be tough, as may be
> figuring out which ones you really want to implement and which ones aren't
> worth it.
> 
> Also note that this is more or less what regmap is trying to do, so you
> may want to check in that direction (see drivers/base/regmap) first.

Thanks, I didn't know about that.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/3] i2c: Add SCCB support

[Jean Delvare]

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

Re: [RFC/PATCH 1/3] i2c: Add SCCB support

[Laurent Pinchart]

Hi Jean,

On Tuesday 17 July 2012 13:53:07 Jean Delvare wrote:
> 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-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> > ---
> > 
> >  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.

I'm OK with dropping the check. Should I resubmit the patch or can you modify 
it when applying ?

> >  	msgbuf0[0] = command;
> >  	switch (size) {
> > 
> >  	case I2C_SMBUS_QUICK:

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 1/3] i2c: Add SCCB support

[Jean Delvare]

On Tue, 17 Jul 2012 13:57:25 +0200, Laurent Pinchart wrote:
> On Tuesday 17 July 2012 13:53:07 Jean Delvare wrote:
> > On Tue, 26 Jun 2012 16:17:07 +0200, Laurent Pinchart wrote:
> > > +	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.
> 
> I'm OK with dropping the check. Should I resubmit the patch or can you modify 
> it when applying ?

I'll do it myself, no problem.

Thanks,
-- 
Jean Delvare

Re: [RFC/PATCH 2/3] i2c: Fall back to emulated SMBus if the operation isn't supported natively

[Jean Delvare]

Hi Laurent,

On Tue, 26 Jun 2012 16:17:08 +0200, Laurent Pinchart wrote:
> Adapter drivers might support only a subset of the SMBus operations
> natively. Those drivers currently have to manually emulate unsupported
> operations using I2C.
> 
> Make the i2c_smbus_xfer() function fall back to
> i2c_smbus_xfer_emulated() when the adapter's .smbus_xfer() operation
> returns -EOPNOTSUPP, like it already does when the .smbus_xfer()
> operation isn't available at all.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> ---
>  drivers/i2c/i2c-core.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index 8cfa660..16e750e 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -2113,8 +2113,8 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>  		   union i2c_smbus_data *data)
>  {
>  	unsigned long orig_jiffies;
> +	s32 res = -EOPNOTSUPP;
>  	int try;
> -	s32 res;
>  
>  	flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
>  
> @@ -2134,7 +2134,12 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
>  				break;
>  		}
>  		i2c_unlock_adapter(adapter);
> -	} else
> +	}
> +
> +	/* Fall back to i2c_smbus_xfer_emulated of the adapter doesn't implement
> +	 * native support for the SMBus operation.
> +	 */
> +	if (res == -EOPNOTSUPP && adapter->algo->master_xfer)
>  		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
>  					      command, protocol, data);
>  

Looks good overall, but maybe the following variant would be preferable
from a performance perspective:

--- linux-3.5-rc7.orig/drivers/i2c/i2c-core.c	2012-07-17 16:35:42.566799611 +0200
+++ linux-3.5-rc7/drivers/i2c/i2c-core.c	2012-07-17 16:59:55.334530352 +0200
@@ -2140,11 +2140,17 @@ s32 i2c_smbus_xfer(struct i2c_adapter *a
 				break;
 		}
 		i2c_unlock_adapter(adapter);
-	} else
-		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
-					      command, protocol, data);
 
-	return res;
+		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
+			return res;
+		/*
+		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't implement
+		 * native support for the SMBus operation.
+		 */
+	}
+
+	return i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
+				       command, protocol, data);
 }
 EXPORT_SYMBOL(i2c_smbus_xfer);
 

What do you think? The advantage is that we can skip the tests for
adapters which only implement adapter->algo->master_xfer().

-- 
Jean Delvare

Re: [RFC/PATCH 2/3] i2c: Fall back to emulated SMBus if the operation isn't supported natively

[Laurent Pinchart]

Hi Jean,

On Tuesday 17 July 2012 17:02:43 Jean Delvare wrote:
> On Tue, 26 Jun 2012 16:17:08 +0200, Laurent Pinchart wrote:
> > Adapter drivers might support only a subset of the SMBus operations
> > natively. Those drivers currently have to manually emulate unsupported
> > operations using I2C.
> > 
> > Make the i2c_smbus_xfer() function fall back to
> > i2c_smbus_xfer_emulated() when the adapter's .smbus_xfer() operation
> > returns -EOPNOTSUPP, like it already does when the .smbus_xfer()
> > operation isn't available at all.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> > ---
> > 
> >  drivers/i2c/i2c-core.c |    9 +++++++--
> >  1 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> > index 8cfa660..16e750e 100644
> > --- a/drivers/i2c/i2c-core.c
> > +++ b/drivers/i2c/i2c-core.c
> > @@ -2113,8 +2113,8 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16
> > addr, unsigned short flags,
> >  		   union i2c_smbus_data *data)
> >  {
> >  	unsigned long orig_jiffies;
> > +	s32 res = -EOPNOTSUPP;
> >  	int try;
> > -	s32 res;
> > 
> >  	flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
> > 
> > @@ -2134,7 +2134,12 @@ s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16
> > addr, unsigned short flags,
> >  				break;
> >  		}
> >  		i2c_unlock_adapter(adapter);
> > -	} else
> > +	}
> > +
> > +	/* Fall back to i2c_smbus_xfer_emulated of the adapter doesn't 
implement
> > +	 * native support for the SMBus operation.
> > +	 */
> > +	if (res == -EOPNOTSUPP && adapter->algo->master_xfer)
> >  		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
> >  					      command, protocol, data);
> 
> Looks good overall, but maybe the following variant would be preferable
> from a performance perspective:
> 
> --- linux-3.5-rc7.orig/drivers/i2c/i2c-core.c	2012-07-17 
16:35:42.566799611
> +0200 +++ linux-3.5-rc7/drivers/i2c/i2c-core.c	2012-07-17
> 16:59:55.334530352 +0200 @@ -2140,11 +2140,17 @@ s32 i2c_smbus_xfer(struct
> i2c_adapter *a
>  				break;
>  		}
>  		i2c_unlock_adapter(adapter);
> -	} else
> -		res = i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
> -					      command, protocol, data);
> 
> -	return res;
> +		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
> +			return res;
> +		/*
> +		 * Fall back to i2c_smbus_xfer_emulated if the adapter doesn't 
implement
> +		 * native support for the SMBus operation.
> +		 */
> +	}
> +
> +	return i2c_smbus_xfer_emulated(adapter, addr, flags, read_write,
> +				       command, protocol, data);
>  }
>  EXPORT_SYMBOL(i2c_smbus_xfer);
> 
> 
> What do you think? The advantage is that we can skip the tests for
> adapters which only implement adapter->algo->master_xfer().

I'm fine with that.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 3/3] i2c: omap: Add support for I2C_M_STOP message flag

[Jean Delvare]

On Tue, 26 Jun 2012 16:17:09 +0200, Laurent Pinchart wrote:
> Generate a stop condition after each message marked with I2C_M_STOP.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-omap.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 801df60..cf1bda0 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -545,6 +545,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>  	if (dev->speed > 400)
>  		w |= OMAP_I2C_CON_OPMODE_HS;
>  
> +	if (msg->flags & I2C_M_STOP)
> +		stop = 1;
>  	if (msg->flags & I2C_M_TEN)
>  		w |= OMAP_I2C_CON_XA;
>  	if (!(msg->flags & I2C_M_RD))

Looks OK to me, but I can't test it, and I also don't know a thing
about OMAP. Maybe it would be a good idea to let omap_i2c_func() return
I2C_FUNC_PROTOCOL_MANGLING now?

-- 
Jean Delvare

Re: [RFC/PATCH 3/3] i2c: omap: Add support for I2C_M_STOP message flag

[Laurent Pinchart]

Hi Jean,

On Tuesday 17 July 2012 17:29:35 Jean Delvare wrote:
> On Tue, 26 Jun 2012 16:17:09 +0200, Laurent Pinchart wrote:
> > Generate a stop condition after each message marked with I2C_M_STOP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> > ---
> > 
> >  drivers/i2c/busses/i2c-omap.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index 801df60..cf1bda0 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -545,6 +545,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> > 
> >  	if (dev->speed > 400)
> >  	
> >  		w |= OMAP_I2C_CON_OPMODE_HS;
> > 
> > +	if (msg->flags & I2C_M_STOP)
> > +		stop = 1;
> > 
> >  	if (msg->flags & I2C_M_TEN)
> >  	
> >  		w |= OMAP_I2C_CON_XA;
> >  	
> >  	if (!(msg->flags & I2C_M_RD))
> 
> Looks OK to me, but I can't test it, and I also don't know a thing
> about OMAP.

I've tested it and it works :-)

> Maybe it would be a good idea to let omap_i2c_func() return
> I2C_FUNC_PROTOCOL_MANGLING now?

Good point. It should already have been returned, as the driver already 
supports I2C_M_IGNORE_ACK.

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 3/3] i2c: omap: Add support for I2C_M_STOP message flag

[Shubhrajyoti]

Laurent,
A minor comment below.
On Tuesday 26 June 2012 07:47 PM, Laurent Pinchart wrote:
> Generate a stop condition after each message marked with I2C_M_STOP.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-omap.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 801df60..cf1bda0 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -545,6 +545,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>  	if (dev->speed > 400)
>  		w |= OMAP_I2C_CON_OPMODE_HS;
>  
> +	if (msg->flags & I2C_M_STOP)
> +		stop = 1;
How about patching omap_i2c_xfer (caller) instead.
There are some debug prints of stop  above that will not reflect the
real value.

>  	if (msg->flags & I2C_M_TEN)
>  		w |= OMAP_I2C_CON_XA;
>  	if (!(msg->flags & I2C_M_RD))

Re: [RFC/PATCH 3/3] i2c: omap: Add support for I2C_M_STOP message flag

[Laurent Pinchart]

Hi Shubhrajyoti,

On Wednesday 18 July 2012 11:49:24 Shubhrajyoti wrote:
> On Tuesday 26 June 2012 07:47 PM, Laurent Pinchart wrote:
> > Generate a stop condition after each message marked with I2C_M_STOP.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> > ---
> > 
> >  drivers/i2c/busses/i2c-omap.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> > index 801df60..cf1bda0 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -545,6 +545,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
> > 
> >  	if (dev->speed > 400)
> >  	
> >  		w |= OMAP_I2C_CON_OPMODE_HS;
> > 
> > +	if (msg->flags & I2C_M_STOP)
> > +		stop = 1;
> 
> How about patching omap_i2c_xfer (caller) instead.
> There are some debug prints of stop  above that will not reflect the
> real value.

omap_i2c_xfer() doesn't look at the content of individual messages, I thought 
it was cleaner not to modify that. Note that the message flags are printed in 
the debug message you mention, which is why I've decided to modify the stop 
variable after printing the message. If you think it's better to modify it 
before, we can move this chunk above the dev_dbg().


> >  	if (msg->flags & I2C_M_TEN)
> >  	
> >  		w |= OMAP_I2C_CON_XA;
> >  	
> >  	if (!(msg->flags & I2C_M_RD))

-- 
Regards,

Laurent Pinchart

Re: [RFC/PATCH 3/3] i2c: omap: Add support for I2C_M_STOP message flag

[Shubhrajyoti Datta]

On Wed, Jul 18, 2012 at 4:43 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Shubhrajyoti,
>
> On Wednesday 18 July 2012 11:49:24 Shubhrajyoti wrote:
>> On Tuesday 26 June 2012 07:47 PM, Laurent Pinchart wrote:
>> > Generate a stop condition after each message marked with I2C_M_STOP.
>> >
>> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> > ---
>> >
>> >  drivers/i2c/busses/i2c-omap.c |    2 ++
>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
>> > index 801df60..cf1bda0 100644
>> > --- a/drivers/i2c/busses/i2c-omap.c
>> > +++ b/drivers/i2c/busses/i2c-omap.c
>> > @@ -545,6 +545,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>> >
>> >     if (dev->speed > 400)
>> >
>> >             w |= OMAP_I2C_CON_OPMODE_HS;
>> >
>> > +   if (msg->flags & I2C_M_STOP)
>> > +           stop = 1;
>>
>> How about patching omap_i2c_xfer (caller) instead.
>> There are some debug prints of stop  above that will not reflect the
>> real value.
>
> omap_i2c_xfer() doesn't look at the content of individual messages, I thought
> it was cleaner not to modify that.

OK fair enough.