Em Wed, 9 May 2018 23:24:37 +0900
Akinobu Mita <akinobu.m...@gmail.com> 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.reic...@collabora.co.uk>
> Cc: Wolfram Sang <w...@the-dreams.de>
> Cc: Jacopo Mondi <jacopo+rene...@jmondi.org>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: Hans Verkuil <hans.verk...@cisco.com>
> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
Patch looks good to me.
Reviewed-by: Mauro Carvalho Chehab <mchehab+sams...@kernel.org>
> ---
> 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
