[PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Mark Brown]

Since there are uses for I2C_M_NOSTART which are much more sensible and
standard than most of the protocol mangling functionality (the main one
being gather writes to devices where something like a register address
needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
for this feature and update all the users to use it.

In the case of regmap-i2c we remove the requirement for mangling as
I2C_M_NOSTART is the only mangling feature which is being used.

Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
---
 Documentation/i2c/functionality       |    1 +
 drivers/base/regmap/regmap-i2c.c      |    2 +-
 drivers/i2c/algos/i2c-algo-bit.c      |    2 +-
 drivers/i2c/algos/i2c-algo-pcf.c      |    2 +-
 drivers/i2c/busses/i2c-nuc900.c       |    3 ++-
 drivers/i2c/busses/i2c-s3c2410.c      |    3 ++-
 drivers/input/joystick/as5011.c       |    1 +
 drivers/video/matrox/matroxfb_maven.c |    1 +
 include/linux/i2c.h                   |    5 +++--
 9 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/Documentation/i2c/functionality b/Documentation/i2c/functionality
index 42c17c1..fb2e77e 100644
--- a/Documentation/i2c/functionality
+++ b/Documentation/i2c/functionality
@@ -33,6 +33,7 @@ For the most up-to-date list of functionality constants, please check
   I2C_FUNC_SMBUS_WRITE_BLOCK_DATA Handles the SMBus write_block_data command
   I2C_FUNC_SMBUS_READ_I2C_BLOCK   Handles the SMBus read_i2c_block_data command
   I2C_FUNC_SMBUS_WRITE_I2C_BLOCK  Handles the SMBus write_i2c_block_data command
+  I2C_FUNC_NOSTART                Transfers can be sent without a start
 
 A few combinations of the above flags are also defined for your convenience:
 
diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 5f6b247..fa6bf52 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -42,7 +42,7 @@ static int regmap_i2c_gather_write(void *context,
 	/* If the I2C controller can't do a gather tell the core, it
 	 * will substitute in a linear write for us.
 	 */
-	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
+	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
 		return -ENOTSUPP;
 
 	xfer[0].addr = i2c->addr;
diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
index 7f0b832..fad22b0 100644
--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -608,7 +608,7 @@ bailout:
 
 static u32 bit_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+	return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_SMBUS_READ_BLOCK_DATA |
 	       I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
 	       I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index 5c23795..8b38986 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -401,7 +401,7 @@ out:
 
 static u32 pcf_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+	return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_PROTOCOL_MANGLING;
 }
 
diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
index 03b6157..a26dfb8 100644
--- a/drivers/i2c/busses/i2c-nuc900.c
+++ b/drivers/i2c/busses/i2c-nuc900.c
@@ -502,7 +502,8 @@ static int nuc900_i2c_xfer(struct i2c_adapter *adap,
 /* declare our i2c functionality */
 static u32 nuc900_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+		I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 /* i2c bus registration info */
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index fa0b134..0195915 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -626,7 +626,8 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 /* declare our i2c functionality */
 static u32 s3c24xx_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+		I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 /* i2c bus registration info */
diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
index 3063464..57d19d4 100644
--- a/drivers/input/joystick/as5011.c
+++ b/drivers/input/joystick/as5011.c
@@ -231,6 +231,7 @@ static int __devinit as5011_probe(struct i2c_client *client,
 	}
 
 	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_NOSTART |
 				     I2C_FUNC_PROTOCOL_MANGLING)) {
 		dev_err(&client->dev,
 			"need i2c bus that supports protocol mangling\n");
diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
index 31b8f67..217678e 100644
--- a/drivers/video/matrox/matroxfb_maven.c
+++ b/drivers/video/matrox/matroxfb_maven.c
@@ -1243,6 +1243,7 @@ static int maven_probe(struct i2c_client *client,
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WRITE_WORD_DATA |
 					      I2C_FUNC_SMBUS_BYTE_DATA |
+					      I2C_FUNC_NOSTART |
 					      I2C_FUNC_PROTOCOL_MANGLING))
 		goto ERROR0;
 	if (!(data = kzalloc(sizeof(*data), GFP_KERNEL))) {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index b66cb60..da46925 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -541,7 +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_NOSTART		0x4000	/* if I2C_FUNC_PROTOCOL_MANGLING */
+#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
 #define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
@@ -554,7 +554,7 @@ struct i2c_msg {
 
 #define I2C_FUNC_I2C			0x00000001
 #define I2C_FUNC_10BIT_ADDR		0x00000002
-#define I2C_FUNC_PROTOCOL_MANGLING	0x00000004 /* I2C_M_NOSTART etc. */
+#define I2C_FUNC_PROTOCOL_MANGLING	0x00000004 /* I2C_M_IGNORE_NAK etc. */
 #define I2C_FUNC_SMBUS_PEC		0x00000008
 #define I2C_FUNC_SMBUS_BLOCK_PROC_CALL	0x00008000 /* SMBus 2.0 */
 #define I2C_FUNC_SMBUS_QUICK		0x00010000
@@ -569,6 +569,7 @@ struct i2c_msg {
 #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x02000000
 #define I2C_FUNC_SMBUS_READ_I2C_BLOCK	0x04000000 /* I2C-like block xfer  */
 #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK	0x08000000 /* w/ 1-byte reg. addr. */
+#define I2C_FUNC_NOSTART		0x10000000 /* I2C_M_NOSTART */
 
 #define I2C_FUNC_SMBUS_BYTE		(I2C_FUNC_SMBUS_READ_BYTE | \
 					 I2C_FUNC_SMBUS_WRITE_BYTE)
-- 
1.7.10

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 954 bytes --]

On Thu, May 03, 2012 at 11:53:36AM +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.
> 
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
> 
> Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

Applied to next, thanks! Jean, let me know if you prefer to take it.
Acks from input and fbdev maintainers still appreciated.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Jean Delvare]

Hi Wolfram,

On Thu, 3 May 2012 13:35:46 +0200, Wolfram Sang wrote:
> On Thu, May 03, 2012 at 11:53:36AM +0100, Mark Brown wrote:
> > Since there are uses for I2C_M_NOSTART which are much more sensible and
> > standard than most of the protocol mangling functionality (the main one
> > being gather writes to devices where something like a register address
> > needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> > for this feature and update all the users to use it.
> > 
> > In the case of regmap-i2c we remove the requirement for mangling as
> > I2C_M_NOSTART is the only mangling feature which is being used.
> > 
> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> Applied to next, thanks! Jean, let me know if you prefer to take it.
> Acks from input and fbdev maintainers still appreciated.

I'd prefer to take it, yes, as it touches the core infrastructure.

Thanks,
-- 
Jean Delvare

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

> > Applied to next, thanks! Jean, let me know if you prefer to take it.
> > Acks from input and fbdev maintainers still appreciated.
> 
> I'd prefer to take it, yes, as it touches the core infrastructure.

OK, didn't know if you were busy. Dropped it from my tree now.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Jean Delvare]

Hi Mark,

On Thu,  3 May 2012 11:53:36 +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.

This is all correct, but it should be documented in
Documentation/i2c/i2c-protocol. At the moment documentation still says
that I2C_M_NOSTART is a weird protocol quirk nobody should be using.

When you update the documentation, I think it is important to stress
that there are now two use cases of I2C_M_NOSTART. If direction
changes, it is a rarely needed protocol quirk. If direction doesn't
change, it is used for buffer gathering.

> 
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
> 
> Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
> ---
>  Documentation/i2c/functionality       |    1 +
>  drivers/base/regmap/regmap-i2c.c      |    2 +-
>  drivers/i2c/algos/i2c-algo-bit.c      |    2 +-
>  drivers/i2c/algos/i2c-algo-pcf.c      |    2 +-
>  drivers/i2c/busses/i2c-nuc900.c       |    3 ++-
>  drivers/i2c/busses/i2c-s3c2410.c      |    3 ++-
>  drivers/input/joystick/as5011.c       |    1 +
>  drivers/video/matrox/matroxfb_maven.c |    1 +
>  include/linux/i2c.h                   |    5 +++--
>  9 files changed, 13 insertions(+), 7 deletions(-)

Review:

> 
> diff --git a/Documentation/i2c/functionality b/Documentation/i2c/functionality
> index 42c17c1..fb2e77e 100644
> --- a/Documentation/i2c/functionality
> +++ b/Documentation/i2c/functionality
> @@ -33,6 +33,7 @@ For the most up-to-date list of functionality constants, please check

You must also update the description of I2C_FUNC_PROTOCOL_MANGLING to
no longer mention I2C_M_NOSTART.

>    I2C_FUNC_SMBUS_WRITE_BLOCK_DATA Handles the SMBus write_block_data command
>    I2C_FUNC_SMBUS_READ_I2C_BLOCK   Handles the SMBus read_i2c_block_data command
>    I2C_FUNC_SMBUS_WRITE_I2C_BLOCK  Handles the SMBus write_i2c_block_data command
> +  I2C_FUNC_NOSTART                Transfers can be sent without a start

Not true. Transfers always begin with a Start, even when I2C_M_NOSTART
is used, because it is never set for the first message.
I2C_FUNC_NOSTART means that messages other than the first may not get
their usual Repeated Start and address prefix. So a better working IMHO
would be:

+  I2C_FUNC_NOSTART                Can skip repeated start sequence

>  
>  A few combinations of the above flags are also defined for your convenience:
>  
> diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
> index 5f6b247..fa6bf52 100644
> --- a/drivers/base/regmap/regmap-i2c.c
> +++ b/drivers/base/regmap/regmap-i2c.c
> @@ -42,7 +42,7 @@ static int regmap_i2c_gather_write(void *context,
>  	/* If the I2C controller can't do a gather tell the core, it
>  	 * will substitute in a linear write for us.
>  	 */
> -	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
> +	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
>  		return -ENOTSUPP;
>  
>  	xfer[0].addr = i2c->addr;
> diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
> index 7f0b832..fad22b0 100644
> --- a/drivers/i2c/algos/i2c-algo-bit.c
> +++ b/drivers/i2c/algos/i2c-algo-bit.c
> @@ -608,7 +608,7 @@ bailout:
>  
>  static u32 bit_func(struct i2c_adapter *adap)
>  {
> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> +	return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
>  	       I2C_FUNC_SMBUS_READ_BLOCK_DATA |
>  	       I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
>  	       I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
> diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
> index 5c23795..8b38986 100644
> --- a/drivers/i2c/algos/i2c-algo-pcf.c
> +++ b/drivers/i2c/algos/i2c-algo-pcf.c
> @@ -401,7 +401,7 @@ out:
>  
>  static u32 pcf_func(struct i2c_adapter *adap)
>  {
> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
> +	return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
>  	       I2C_FUNC_PROTOCOL_MANGLING;
>  }
>  

A quick grep suggests that i2c-algo-pcf doesn't support I2C_M_NOSTART,
only I2C_M_REV_DIR_ADDR. So you don't want to add I2C_FUNC_NOSTART to
the functionality mask.

> diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
> index 03b6157..a26dfb8 100644
> --- a/drivers/i2c/busses/i2c-nuc900.c
> +++ b/drivers/i2c/busses/i2c-nuc900.c
> @@ -502,7 +502,8 @@ static int nuc900_i2c_xfer(struct i2c_adapter *adap,
>  /* declare our i2c functionality */
>  static u32 nuc900_i2c_func(struct i2c_adapter *adap)
>  {
> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
> +		I2C_FUNC_PROTOCOL_MANGLING;
>  }
>  
>  /* i2c bus registration info */
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index fa0b134..0195915 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -626,7 +626,8 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
>  /* declare our i2c functionality */
>  static u32 s3c24xx_i2c_func(struct i2c_adapter *adap)
>  {
> -	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
> +		I2C_FUNC_PROTOCOL_MANGLING;
>  }
>  
>  /* i2c bus registration info */
> diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
> index 3063464..57d19d4 100644
> --- a/drivers/input/joystick/as5011.c
> +++ b/drivers/input/joystick/as5011.c
> @@ -231,6 +231,7 @@ static int __devinit as5011_probe(struct i2c_client *client,
>  	}
>  
>  	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_NOSTART |
>  				     I2C_FUNC_PROTOCOL_MANGLING)) {
>  		dev_err(&client->dev,
>  			"need i2c bus that supports protocol mangling\n");
> diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
> index 31b8f67..217678e 100644
> --- a/drivers/video/matrox/matroxfb_maven.c
> +++ b/drivers/video/matrox/matroxfb_maven.c
> @@ -1243,6 +1243,7 @@ static int maven_probe(struct i2c_client *client,
>  
>  	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WRITE_WORD_DATA |
>  					      I2C_FUNC_SMBUS_BYTE_DATA |
> +					      I2C_FUNC_NOSTART |
>  					      I2C_FUNC_PROTOCOL_MANGLING))
>  		goto ERROR0;
>  	if (!(data = kzalloc(sizeof(*data), GFP_KERNEL))) {
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index b66cb60..da46925 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -541,7 +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_NOSTART		0x4000	/* if I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
>  #define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
>  #define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
>  #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
> @@ -554,7 +554,7 @@ struct i2c_msg {
>  
>  #define I2C_FUNC_I2C			0x00000001
>  #define I2C_FUNC_10BIT_ADDR		0x00000002
> -#define I2C_FUNC_PROTOCOL_MANGLING	0x00000004 /* I2C_M_NOSTART etc. */
> +#define I2C_FUNC_PROTOCOL_MANGLING	0x00000004 /* I2C_M_IGNORE_NAK etc. */
>  #define I2C_FUNC_SMBUS_PEC		0x00000008
>  #define I2C_FUNC_SMBUS_BLOCK_PROC_CALL	0x00008000 /* SMBus 2.0 */
>  #define I2C_FUNC_SMBUS_QUICK		0x00010000
> @@ -569,6 +569,7 @@ struct i2c_msg {
>  #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x02000000
>  #define I2C_FUNC_SMBUS_READ_I2C_BLOCK	0x04000000 /* I2C-like block xfer  */
>  #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK	0x08000000 /* w/ 1-byte reg. addr. */
> +#define I2C_FUNC_NOSTART		0x10000000 /* I2C_M_NOSTART */

Sorry for nitpicking but wouldn't I2C_FUNC_GATHER be a better name?
NOSTART is an implementation detail now, the high level feature is the
ability to gather multiple messages into one.

Also I think I would prefer if you used 0x00000010 for the new
functionality, so that it isn't too far away from
I2C_FUNC_PROTOCOL_MANGLING.

>  
>  #define I2C_FUNC_SMBUS_BYTE		(I2C_FUNC_SMBUS_READ_BYTE | \
>  					 I2C_FUNC_SMBUS_WRITE_BYTE)

Thanks,
-- 
Jean Delvare

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1201 bytes --]

On Thu, May 03, 2012 at 08:36:17PM +0200, Jean Delvare wrote:

> You must also update the description of I2C_FUNC_PROTOCOL_MANGLING to
> no longer mention I2C_M_NOSTART.

For backwards ABI compatibility _PROTCOL_MANGING still has to imply
_NOSTART, though it's unclear if that should be documented here.  For
drivers it should for the most part flow naturally as I'd expect they'll
end up implementing one or more of the mangling flags anyway.  For
applications I guess it means that they should fall back to checking for
_PROTOCOL_MANGLING if _NOSTART isn't there.

> > +#define I2C_FUNC_NOSTART		0x10000000 /* I2C_M_NOSTART */

> Sorry for nitpicking but wouldn't I2C_FUNC_GATHER be a better name?
> NOSTART is an implementation detail now, the high level feature is the
> ability to gather multiple messages into one.

I kept it this way mostly because it means that the capability flag has
the same name as the feature flag which seemed helpful from a usability
point of view.

There is also the fact you could in theory also implement gather support
in other ways (eg, using DMA) though at present there's no API level
support for this and the users do have to code this specific
implementation.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 736 bytes --]

On Thu, May 03, 2012 at 08:36:17PM +0200, Jean Delvare wrote:

> This is all correct, but it should be documented in
> Documentation/i2c/i2c-protocol. At the moment documentation still says
> that I2C_M_NOSTART is a weird protocol quirk nobody should be using.

> When you update the documentation, I think it is important to stress
> that there are now two use cases of I2C_M_NOSTART. If direction
> changes, it is a rarely needed protocol quirk. If direction doesn't
> change, it is used for buffer gathering.

Hrm, actually rereading the documentation it wasn't all that clear that
these were particularly disrecommended - it just mentioned that the
flags existed and might be needed.  I strengthened the documentation
here as well.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Mark Brown]

Since there are uses for I2C_M_NOSTART which are much more sensible and
standard than most of the protocol mangling functionality (the main one
being gather writes to devices where something like a register address
needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
for this feature and update all the users to use it.

Also strengthen the disrecommendation of the protocol mangling while we're
at it.

In the case of regmap-i2c we remove the requirement for mangling as
I2C_M_NOSTART is the only mangling feature which is being used.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 Documentation/i2c/functionality       |    8 +++++---
 Documentation/i2c/i2c-protocol        |    9 ++++++++-
 drivers/base/regmap/regmap-i2c.c      |    2 +-
 drivers/i2c/algos/i2c-algo-bit.c      |    2 +-
 drivers/i2c/busses/i2c-nuc900.c       |    3 ++-
 drivers/i2c/busses/i2c-s3c2410.c      |    3 ++-
 drivers/input/joystick/as5011.c       |    1 +
 drivers/video/matrox/matroxfb_maven.c |    1 +
 include/linux/i2c.h                   |    5 +++--
 9 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/Documentation/i2c/functionality b/Documentation/i2c/functionality
index 42c17c1..7cbd5da 100644
--- a/Documentation/i2c/functionality
+++ b/Documentation/i2c/functionality
@@ -18,9 +18,8 @@ For the most up-to-date list of functionality constants, please check
                                   adapters typically can not do these)
   I2C_FUNC_10BIT_ADDR             Handles the 10-bit address extensions
   I2C_FUNC_PROTOCOL_MANGLING      Knows about the I2C_M_IGNORE_NAK,
-                                  I2C_M_REV_DIR_ADDR, I2C_M_NOSTART and
-                                  I2C_M_NO_RD_ACK flags (which modify the
-                                  I2C protocol!)
+                                  I2C_M_REV_DIR_ADDR and I2C_M_NO_RD_ACK
+				  flags (which modify the I2C protocol!).
   I2C_FUNC_SMBUS_QUICK            Handles the SMBus write_quick command
   I2C_FUNC_SMBUS_READ_BYTE        Handles the SMBus read_byte command
   I2C_FUNC_SMBUS_WRITE_BYTE       Handles the SMBus write_byte command
@@ -33,6 +32,7 @@ For the most up-to-date list of functionality constants, please check
   I2C_FUNC_SMBUS_WRITE_BLOCK_DATA Handles the SMBus write_block_data command
   I2C_FUNC_SMBUS_READ_I2C_BLOCK   Handles the SMBus read_i2c_block_data command
   I2C_FUNC_SMBUS_WRITE_I2C_BLOCK  Handles the SMBus write_i2c_block_data command
+  I2C_FUNC_NOSTART                Can skip repeated start sequence
 
 A few combinations of the above flags are also defined for your convenience:
 
@@ -50,6 +50,8 @@ A few combinations of the above flags are also defined for your convenience:
                                   emulated by a real I2C adapter (using
                                   the transparent emulation layer)
 
+In kernel versions prior to 3.5 I2C_FUNC_NOSTART was implemented as
+part of I2C_FUNC_PROTOCOL_MANGLING.
 
 ADAPTER IMPLEMENTATION
 ----------------------
diff --git a/Documentation/i2c/i2c-protocol b/Documentation/i2c/i2c-protocol
index 10518dd..0b3e62d 100644
--- a/Documentation/i2c/i2c-protocol
+++ b/Documentation/i2c/i2c-protocol
@@ -49,7 +49,9 @@ a byte read, followed by a byte write:
 Modified transactions
 =====================
 
-We have found some I2C devices that needs the following modifications:
+The following modifications to the I2C protocol can also be generated,
+with the exception of I2C_M_NOSTART these are usually only needed to
+work around device issues:
 
   Flag I2C_M_NOSTART: 
     In a combined transaction, no 'S Addr Wr/Rd [A]' is generated at some
@@ -60,6 +62,11 @@ We have found some I2C devices that needs the following modifications:
     we do not generate Addr, but we do generate the startbit S. This will
     probably confuse all other clients on your bus, so don't try this.
 
+    This is often used to gather transmits from multiple data buffers in
+    system memory into something that appears as a single transfer to the
+    I2C device but may also be used between direction changes by some
+    rare devices.
+
   Flags I2C_M_REV_DIR_ADDR
     This toggles the Rd/Wr flag. That is, if you want to do a write, but
     need to emit an Rd instead of a Wr, or vice versa, you set this
diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 9a3a8c5..157bbd5 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -40,7 +40,7 @@ static int regmap_i2c_gather_write(struct device *dev,
 	/* If the I2C controller can't do a gather tell the core, it
 	 * will substitute in a linear write for us.
 	 */
-	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
+	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
 		return -ENOTSUPP;
 
 	xfer[0].addr = i2c->addr;
diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
index 7f0b832..fad22b0 100644
--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -608,7 +608,7 @@ bailout:
 
 static u32 bit_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+	return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_SMBUS_READ_BLOCK_DATA |
 	       I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
 	       I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
index 03b6157..a26dfb8 100644
--- a/drivers/i2c/busses/i2c-nuc900.c
+++ b/drivers/i2c/busses/i2c-nuc900.c
@@ -502,7 +502,8 @@ static int nuc900_i2c_xfer(struct i2c_adapter *adap,
 /* declare our i2c functionality */
 static u32 nuc900_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+		I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 /* i2c bus registration info */
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 737f721..37b1f90 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -591,7 +591,8 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 /* declare our i2c functionality */
 static u32 s3c24xx_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+		I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 /* i2c bus registration info */
diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
index 3063464..57d19d4 100644
--- a/drivers/input/joystick/as5011.c
+++ b/drivers/input/joystick/as5011.c
@@ -231,6 +231,7 @@ static int __devinit as5011_probe(struct i2c_client *client,
 	}
 
 	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_NOSTART |
 				     I2C_FUNC_PROTOCOL_MANGLING)) {
 		dev_err(&client->dev,
 			"need i2c bus that supports protocol mangling\n");
diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
index 31b8f67..217678e 100644
--- a/drivers/video/matrox/matroxfb_maven.c
+++ b/drivers/video/matrox/matroxfb_maven.c
@@ -1243,6 +1243,7 @@ static int maven_probe(struct i2c_client *client,
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WRITE_WORD_DATA |
 					      I2C_FUNC_SMBUS_BYTE_DATA |
+					      I2C_FUNC_NOSTART |
 					      I2C_FUNC_PROTOCOL_MANGLING))
 		goto ERROR0;
 	if (!(data = kzalloc(sizeof(*data), GFP_KERNEL))) {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 195d8b3..85d4ef1 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -540,7 +540,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_NOSTART		0x4000	/* if I2C_FUNC_PROTOCOL_MANGLING */
+#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
 #define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
@@ -553,8 +553,9 @@ struct i2c_msg {
 
 #define I2C_FUNC_I2C			0x00000001
 #define I2C_FUNC_10BIT_ADDR		0x00000002
-#define I2C_FUNC_PROTOCOL_MANGLING	0x00000004 /* I2C_M_NOSTART etc. */
+#define I2C_FUNC_PROTOCOL_MANGLING	0x00000004 /* I2C_M_IGNORE_NAK etc. */
 #define I2C_FUNC_SMBUS_PEC		0x00000008
+#define I2C_FUNC_NOSTART		0x00000010 /* I2C_M_NOSTART */
 #define I2C_FUNC_SMBUS_BLOCK_PROC_CALL	0x00008000 /* SMBus 2.0 */
 #define I2C_FUNC_SMBUS_QUICK		0x00010000
 #define I2C_FUNC_SMBUS_READ_BYTE	0x00020000
-- 
1.7.10

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Jean Delvare]

On Fri,  4 May 2012 11:26:48 +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.
> 
> Also strengthen the disrecommendation of the protocol mangling while we're
> at it.
> 
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  Documentation/i2c/functionality       |    8 +++++---
>  Documentation/i2c/i2c-protocol        |    9 ++++++++-
>  drivers/base/regmap/regmap-i2c.c      |    2 +-
>  drivers/i2c/algos/i2c-algo-bit.c      |    2 +-
>  drivers/i2c/busses/i2c-nuc900.c       |    3 ++-
>  drivers/i2c/busses/i2c-s3c2410.c      |    3 ++-
>  drivers/input/joystick/as5011.c       |    1 +
>  drivers/video/matrox/matroxfb_maven.c |    1 +
>  include/linux/i2c.h                   |    5 +++--
>  9 files changed, 24 insertions(+), 10 deletions(-)
> (...)

Applied with minor cleanups, thanks.

-- 
Jean Delvare

[PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Mark Brown]

Since there are uses for I2C_M_NOSTART which are much more sensible and
standard than most of the protocol mangling functionality (the main one
being gather writes to devices where something like a register address
needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
for this feature and update all the users to use it.

In the case of regmap-i2c we remove the requirement for mangling as
I2C_M_NOSTART is the only mangling feature which is being used.

Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>
---
 Documentation/i2c/functionality       |    1 +
 drivers/base/regmap/regmap-i2c.c      |    2 +-
 drivers/i2c/algos/i2c-algo-bit.c      |    2 +-
 drivers/i2c/algos/i2c-algo-pcf.c      |    2 +-
 drivers/i2c/busses/i2c-nuc900.c       |    3 ++-
 drivers/i2c/busses/i2c-s3c2410.c      |    3 ++-
 drivers/input/joystick/as5011.c       |    1 +
 drivers/video/matrox/matroxfb_maven.c |    1 +
 include/linux/i2c.h                   |    3 ++-
 9 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/i2c/functionality b/Documentation/i2c/functionality
index 42c17c1..fb2e77e 100644
--- a/Documentation/i2c/functionality
+++ b/Documentation/i2c/functionality
@@ -33,6 +33,7 @@ For the most up-to-date list of functionality constants, please check
   I2C_FUNC_SMBUS_WRITE_BLOCK_DATA Handles the SMBus write_block_data command
   I2C_FUNC_SMBUS_READ_I2C_BLOCK   Handles the SMBus read_i2c_block_data command
   I2C_FUNC_SMBUS_WRITE_I2C_BLOCK  Handles the SMBus write_i2c_block_data command
+  I2C_FUNC_NOSTART                Transfers can be sent without a start
 
 A few combinations of the above flags are also defined for your convenience:
 
diff --git a/drivers/base/regmap/regmap-i2c.c b/drivers/base/regmap/regmap-i2c.c
index 5f6b247..fa6bf52 100644
--- a/drivers/base/regmap/regmap-i2c.c
+++ b/drivers/base/regmap/regmap-i2c.c
@@ -42,7 +42,7 @@ static int regmap_i2c_gather_write(void *context,
 	/* If the I2C controller can't do a gather tell the core, it
 	 * will substitute in a linear write for us.
 	 */
-	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_PROTOCOL_MANGLING))
+	if (!i2c_check_functionality(i2c->adapter, I2C_FUNC_NOSTART))
 		return -ENOTSUPP;
 
 	xfer[0].addr = i2c->addr;
diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c
index 7f0b832..fad22b0 100644
--- a/drivers/i2c/algos/i2c-algo-bit.c
+++ b/drivers/i2c/algos/i2c-algo-bit.c
@@ -608,7 +608,7 @@ bailout:
 
 static u32 bit_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+	return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_SMBUS_READ_BLOCK_DATA |
 	       I2C_FUNC_SMBUS_BLOCK_PROC_CALL |
 	       I2C_FUNC_10BIT_ADDR | I2C_FUNC_PROTOCOL_MANGLING;
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index 5c23795..8b38986 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -401,7 +401,7 @@ out:
 
 static u32 pcf_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+	return I2C_FUNC_I2C | I2C_FUNC_NOSTART | I2C_FUNC_SMBUS_EMUL |
 	       I2C_FUNC_PROTOCOL_MANGLING;
 }
 
diff --git a/drivers/i2c/busses/i2c-nuc900.c b/drivers/i2c/busses/i2c-nuc900.c
index 03b6157..a26dfb8 100644
--- a/drivers/i2c/busses/i2c-nuc900.c
+++ b/drivers/i2c/busses/i2c-nuc900.c
@@ -502,7 +502,8 @@ static int nuc900_i2c_xfer(struct i2c_adapter *adap,
 /* declare our i2c functionality */
 static u32 nuc900_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+		I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 /* i2c bus registration info */
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
index 8764d6a..faa1f23 100644
--- a/drivers/i2c/busses/i2c-s3c2410.c
+++ b/drivers/i2c/busses/i2c-s3c2410.c
@@ -591,7 +591,8 @@ static int s3c24xx_i2c_xfer(struct i2c_adapter *adap,
 /* declare our i2c functionality */
 static u32 s3c24xx_i2c_func(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_NOSTART |
+		I2C_FUNC_PROTOCOL_MANGLING;
 }
 
 /* i2c bus registration info */
diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
index 3063464..57d19d4 100644
--- a/drivers/input/joystick/as5011.c
+++ b/drivers/input/joystick/as5011.c
@@ -231,6 +231,7 @@ static int __devinit as5011_probe(struct i2c_client *client,
 	}
 
 	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_NOSTART |
 				     I2C_FUNC_PROTOCOL_MANGLING)) {
 		dev_err(&client->dev,
 			"need i2c bus that supports protocol mangling\n");
diff --git a/drivers/video/matrox/matroxfb_maven.c b/drivers/video/matrox/matroxfb_maven.c
index 31b8f67..217678e 100644
--- a/drivers/video/matrox/matroxfb_maven.c
+++ b/drivers/video/matrox/matroxfb_maven.c
@@ -1243,6 +1243,7 @@ static int maven_probe(struct i2c_client *client,
 
 	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WRITE_WORD_DATA |
 					      I2C_FUNC_SMBUS_BYTE_DATA |
+					      I2C_FUNC_NOSTART |
 					      I2C_FUNC_PROTOCOL_MANGLING))
 		goto ERROR0;
 	if (!(data = kzalloc(sizeof(*data), GFP_KERNEL))) {
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 195d8b3..c18b5b1 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -540,7 +540,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_NOSTART		0x4000	/* if I2C_FUNC_PROTOCOL_MANGLING */
+#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
 #define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
@@ -568,6 +568,7 @@ struct i2c_msg {
 #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x02000000
 #define I2C_FUNC_SMBUS_READ_I2C_BLOCK	0x04000000 /* I2C-like block xfer  */
 #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK	0x08000000 /* w/ 1-byte reg. addr. */
+#define I2C_FUNC_NOSTART		0x10000000 /* I2C_M_NOSTART */
 
 #define I2C_FUNC_SMBUS_BYTE		(I2C_FUNC_SMBUS_READ_BYTE | \
 					 I2C_FUNC_SMBUS_WRITE_BYTE)
-- 
1.7.10

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

On Thu, Apr 26, 2012 at 01:37:19PM +0100, Mark Brown wrote:
> Since there are uses for I2C_M_NOSTART which are much more sensible and
> standard than most of the protocol mangling functionality (the main one
> being gather writes to devices where something like a register address
> needs to be inserted before a block of data) create a new I2C_FUNC_NOSTART
> for this feature and update all the users to use it.
> 
> In the case of regmap-i2c we remove the requirement for mangling as
> I2C_M_NOSTART is the only mangling feature which is being used.
> 
> Signed-off-by: Mark Brown <broonie-yzvPICuk2AATkU/dhu1WVueM+bqZidxxQQ4Iyu8u01E@public.gmane.org>

In general, I like it. One thing below:

> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -540,7 +540,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_NOSTART		0x4000	/* if I2C_FUNC_PROTOCOL_MANGLING */
> +#define I2C_M_NOSTART		0x4000	/* if I2C_FUNC_NOSTART */
>  #define I2C_M_REV_DIR_ADDR	0x2000	/* if I2C_FUNC_PROTOCOL_MANGLING */
>  #define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
>  #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
> @@ -568,6 +568,7 @@ struct i2c_msg {
>  #define I2C_FUNC_SMBUS_WRITE_BLOCK_DATA 0x02000000
>  #define I2C_FUNC_SMBUS_READ_I2C_BLOCK	0x04000000 /* I2C-like block xfer  */
>  #define I2C_FUNC_SMBUS_WRITE_I2C_BLOCK	0x08000000 /* w/ 1-byte reg. addr. */
> +#define I2C_FUNC_NOSTART		0x10000000 /* I2C_M_NOSTART */

In this file, the comment for MANGLING should be adapted, too. It says
currently:

#define I2C_FUNC_PROTOCOL_MANGLING      0x00000004 /* I2C_M_NOSTART etc. */

NOSTART is now out of MANGLING.

Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. This
will be more intuitive, probably?

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

On Thu, May 03, 2012 at 11:52:11AM +0200, Wolfram Sang wrote:

> Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. This
> will be more intuitive, probably?

What is the value in renumbering everything?  It just seems like it
makes the diff less clear and has no practical value.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Jean Delvare]

On Thu, 3 May 2012 10:58:15 +0100, Mark Brown wrote:
> On Thu, May 03, 2012 at 11:52:11AM +0200, Wolfram Sang wrote:
> 
> > Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. This
> > will be more intuitive, probably?
> 
> What is the value in renumbering everything?  It just seems like it
> makes the diff less clear and has no practical value.

You actually just can't renumber them, as they are part of the ABI to
user-space (through i2c-dev's ioctl I2C_FUNCS.)

-- 
Jean Delvare

Re: [PATCH] i2c: Split I2C_M_NOSTART support out of I2C_FUNC_PROTOCOL_MANGLING

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Thu, May 03, 2012 at 10:58:15AM +0100, Mark Brown wrote:
> On Thu, May 03, 2012 at 11:52:11AM +0200, Wolfram Sang wrote:
> 
> > Also, I'd think the FUNC_NOSTART bit should be 0x08 and SMBUS_PEC 0x4000. This
> > will be more intuitive, probably?
> 
> What is the value in renumbering everything?  It just seems like it
> makes the diff less clear and has no practical value.

Not everything, only those 2. The result would be having one block dealing
with I2C and one block with SMBUS. But Jean's comment is an ultimate
"no" anyway.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]