Re: [RFC PATCH] i2c: add I2C_M_FORCE_STOP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Wed,  9 May 2018 23:24:37 +0900
Akinobu Mita <akinobu.mita@xxxxxxxxx> escreveu:

> This adds a new I2C_M_FORCE_STOP flag that forces a stop condition after
> the message in a combined transaction.
> 
> This flag is intended to be used by the devices that don't support
> repeated starts like SCCB (Serial Camera Control Bus) devices.
> 
> Here is an example usage for ov772x driver that needs to issue two
> separated I2C messages as the ov772x device doesn't support repeated
> starts.
> 
> static int ov772x_read(struct i2c_client *client, u8 addr)
> {
>         u8 val;
>         struct i2c_msg msg[] = {
>                 {
>                         .addr = client->addr,
>                         .flags = I2C_M_FORCE_STOP,
>                         .len = 1,
>                         .buf = &addr,
>                 },
>                 {
>                         .addr = client->addr,
>                         .flags = I2C_M_RD,
>                         .len = 1,
>                         .buf = &val,
>                 },
>         };
>         int ret;
> 
>         ret = i2c_transfer(client->adapter, msg, 2);
>         if (ret != 2)
>                 return (ret < 0) ? ret : -EIO;
> 
>         return val;
> }
> 
> This is another approach based on Mauro's advise for the initial attempt
> (http://patchwork.ozlabs.org/patch/905192/).
> 
> Cc: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx>
> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> Cc: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Cc: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
> Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>

Patch looks good to me.

Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@xxxxxxxxxx>


> ---
>  drivers/i2c/i2c-core-base.c | 46 ++++++++++++++++++++++++++++++++++-----------
>  include/uapi/linux/i2c.h    |  1 +
>  2 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 1ba40bb..6b73484 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1828,6 +1828,25 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
>  	return 0;
>  }
>  
> +static int i2c_transfer_nolock(struct i2c_adapter *adap, struct i2c_msg *msgs,
> +			       int num)
> +{
> +	unsigned long orig_jiffies;
> +	int ret, try;
> +
> +	/* Retry automatically on arbitration loss */
> +	orig_jiffies = jiffies;
> +	for (ret = 0, try = 0; try <= adap->retries; try++) {
> +		ret = adap->algo->master_xfer(adap, msgs, num);
> +		if (ret != -EAGAIN)
> +			break;
> +		if (time_after(jiffies, orig_jiffies + adap->timeout))
> +			break;
> +	}
> +
> +	return ret;
> +}
> +
>  /**
>   * __i2c_transfer - unlocked flavor of i2c_transfer
>   * @adap: Handle to I2C bus
> @@ -1842,8 +1861,8 @@ static int i2c_check_for_quirks(struct i2c_adapter *adap, struct i2c_msg *msgs,
>   */
>  int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  {
> -	unsigned long orig_jiffies;
> -	int ret, try;
> +	int ret;
> +	int i, n;
>  
>  	if (WARN_ON(!msgs || num < 1))
>  		return -EINVAL;
> @@ -1857,7 +1876,6 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  	 * being executed when not needed.
>  	 */
>  	if (static_branch_unlikely(&i2c_trace_msg_key)) {
> -		int i;
>  		for (i = 0; i < num; i++)
>  			if (msgs[i].flags & I2C_M_RD)
>  				trace_i2c_read(adap, &msgs[i], i);
> @@ -1865,18 +1883,24 @@ int __i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>  				trace_i2c_write(adap, &msgs[i], i);
>  	}
>  
> -	/* Retry automatically on arbitration loss */
> -	orig_jiffies = jiffies;
> -	for (ret = 0, try = 0; try <= adap->retries; try++) {
> -		ret = adap->algo->master_xfer(adap, msgs, num);
> -		if (ret != -EAGAIN)
> -			break;
> -		if (time_after(jiffies, orig_jiffies + adap->timeout))
> +	for (i = 0; i < num; i += n) {
> +		for (n = 0; i + n < num; n++) {
> +			if (msgs[i + n].flags & I2C_M_FORCE_STOP) {
> +				n++;
> +				break;
> +			}
> +		}
> +
> +		ret = i2c_transfer_nolock(adap, &msgs[i], n);
> +		if (ret != n) {
> +			if (i > 0)
> +				ret = (ret < 0) ? i : i + ret;
>  			break;
> +		}
> +		ret = i + n;
>  	}
>  
>  	if (static_branch_unlikely(&i2c_trace_msg_key)) {
> -		int i;
>  		for (i = 0; i < ret; i++)
>  			if (msgs[i].flags & I2C_M_RD)
>  				trace_i2c_reply(adap, &msgs[i], i);
> diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
> index f71a175..36e8c7c 100644
> --- a/include/uapi/linux/i2c.h
> +++ b/include/uapi/linux/i2c.h
> @@ -72,6 +72,7 @@ struct i2c_msg {
>  #define I2C_M_RD		0x0001	/* read data, from slave to master */
>  					/* I2C_M_RD is guaranteed to be 0x0001! */
>  #define I2C_M_TEN		0x0010	/* this is a ten bit chip address */
> +#define I2C_M_FORCE_STOP	0x0100	/* force a stop condition after the message */
>  #define I2C_M_DMA_SAFE		0x0200	/* the buffer of this message is DMA safe */
>  					/* makes only sense in kernelspace */
>  					/* userspace buffers are copied anyway */



Thanks,
Mauro



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux