On 14/08/15 16:50, Sifan Naeem wrote:
> In version 3.3 of the IP when transaction halt is set, an interrupt
> will be generated after each byte of a transfer instead of after
> every transfer but before the stop bit.
> Due to this behaviour we have to be careful that every time we
> release the transaction halt we have to re-enable it straight away
> so that we only process a single byte, not doing so will result in
> all remaining bytes been processed and a stop bit being issued,
> which will prevent us having a repeated start.
>
> This change will have no effect on earlier versions of the IP.
>
> Signed-off-by: Sifan Naeem <sifan.na...@imgtec.com>
> Reviewed-by: James Hartley <james.hart...@imgtec.com>
> ---
> drivers/i2c/busses/i2c-img-scb.c | 45
> ++++++++++++++++++++++++++++++--------
> 1 file changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-img-scb.c
> b/drivers/i2c/busses/i2c-img-scb.c
> index 837a73a43a6d..7a51f39528d7 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -513,7 +513,17 @@ static void img_i2c_soft_reset(struct img_i2c *i2c)
> SCB_CONTROL_CLK_ENABLE | SCB_CONTROL_SOFT_RESET);
> }
>
> -/* enable or release transaction halt for control of repeated starts */
> +/*
> + * Enable or release transaction halt for control of repeated starts.
> + * In version 3.3 of the IP when transaction halt is set, an interrupt
> + * will be generated after each byte of a transfer instead of after
> + * every transfer but before the stop bit.
> + * Due to this behaviour we have to be careful that every time we
> + * release the transaction halt we have to re-enable it straight away
> + * so that we only process a single byte, not doing so will result in
> + * all remaining bytes been processed and a stop bit being issued,
> + * which will prevent us having a repeated start.
> + */
> static void img_i2c_transaction_halt(struct img_i2c *i2c, bool t_halt)
> {
> u32 val;
> @@ -582,7 +592,6 @@ static void img_i2c_read(struct img_i2c *i2c)
> img_i2c_writel(i2c, SCB_READ_ADDR_REG, i2c->msg.addr);
> img_i2c_writel(i2c, SCB_READ_COUNT_REG, i2c->msg.len);
>
> - img_i2c_transaction_halt(i2c, false);
> mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
> }
>
> @@ -596,7 +605,6 @@ static void img_i2c_write(struct img_i2c *i2c)
> img_i2c_writel(i2c, SCB_WRITE_ADDR_REG, i2c->msg.addr);
> img_i2c_writel(i2c, SCB_WRITE_COUNT_REG, i2c->msg.len);
>
> - img_i2c_transaction_halt(i2c, false);
> mod_timer(&i2c->check_timer, jiffies + msecs_to_jiffies(1));
> img_i2c_write_fifo(i2c);
>
> @@ -863,7 +871,7 @@ static unsigned int img_i2c_auto(struct img_i2c *i2c,
> /* Enable transaction halt on start bit */
> if (line_status & LINESTAT_START_BIT_DET) {
> if (!i2c->last_msg) {
> - img_i2c_transaction_halt(i2c, true);
> + img_i2c_transaction_halt(i2c, !i2c->last_msg);
This hunk seems pointless given the "if" conditional it resides in.
Otherwise
Acked-by: James Hogan <james.ho...@imgtec.com>
Cheers
James
> /* we're no longer interested in the slave event */
> i2c->int_enable &= ~INT_SLAVE_EVENT;
> }
> @@ -1093,12 +1101,31 @@ static int img_i2c_xfer(struct i2c_adapter *adap,
> struct i2c_msg *msgs,
> img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
> img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
>
> - if (atomic)
> + if (atomic) {
> img_i2c_atomic_start(i2c);
> - else if (msg->flags & I2C_M_RD)
> - img_i2c_read(i2c);
> - else
> - img_i2c_write(i2c);
> + } else {
> + /*
> + * Enable transaction halt if not the last message in
> + * the queue so that we can control repeated starts.
> + */
> + img_i2c_transaction_halt(i2c, !i2c->last_msg);
> +
> + if (msg->flags & I2C_M_RD)
> + img_i2c_read(i2c);
> + else
> + img_i2c_write(i2c);
> +
> + /*
> + * Release and then enable transaction halt, to
> + * allow only a single byte to proceed.
> + * This doesn't have an effect on the initial transfer
> + * but will allow the following transfers to start
> + * processing if the previous transfer was marked as
> + * complete while the i2c block was halted.
> + */
> + img_i2c_transaction_halt(i2c, false);
> + img_i2c_transaction_halt(i2c, !i2c->last_msg);
> + }
> spin_unlock_irqrestore(&i2c->lock, flags);
>
> time_left = wait_for_completion_timeout(&i2c->msg_complete,
>
signature.asc
Description: OpenPGP digital signature
