Hi Marek,

On Wed, Nov 21, 2012 at 06:51:28AM +0100, Marek Vasut wrote:
> Add support for the PIO mode and mixed PIO/DMA mode support. The mixed
> PIO/DMA is the default mode of operation. This shall leverage overhead
> that the driver creates due to setting up DMA descriptors even for very
> short transfers.
> 
> The current boundary between PIO/DMA 8 bytes, transfers shorter than 8
> bytes are transfered by PIO, longer transfers use DMA. The performance
> of write transfers remains unchanged, while there is a minor improvement
> of read performance. Reading 16KB EEPROM with DMA-only operations gives
> a read speed of 39.5KB/s, while with then new mixed-mode the speed is
> blazing 40.6KB/s.
> 
> Signed-off-by: Marek Vasut <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Shawn Guo <[email protected]>
> Cc: Wolfram Sang <[email protected]>
> ---
>  drivers/i2c/busses/i2c-mxs.c |  163 
> ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 150 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index 47d5d53..9048f34 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -39,6 +39,7 @@
>  #define MXS_I2C_CTRL0_SET    (0x04)
>  
>  #define MXS_I2C_CTRL0_SFTRST                 0x80000000
> +#define MXS_I2C_CTRL0_RUN                    0x20000000
>  #define MXS_I2C_CTRL0_SEND_NAK_ON_LAST               0x02000000
>  #define MXS_I2C_CTRL0_RETAIN_CLOCK           0x00200000
>  #define MXS_I2C_CTRL0_POST_SEND_STOP         0x00100000
> @@ -64,6 +65,13 @@
>  #define MXS_I2C_CTRL1_SLAVE_STOP_IRQ         0x02
>  #define MXS_I2C_CTRL1_SLAVE_IRQ                      0x01
>  
> +#define MXS_I2C_DATA         (0xa0)
> +
> +#define MXS_I2C_DEBUG0               (0xb0)
> +#define MXS_I2C_DEBUG0_CLR   (0xb8)
> +
> +#define MXS_I2C_DEBUG0_DMAREQ        0x80000000
> +
>  #define MXS_I2C_IRQ_MASK     (MXS_I2C_CTRL1_DATA_ENGINE_CMPLT_IRQ | \
>                                MXS_I2C_CTRL1_NO_SLAVE_ACK_IRQ | \
>                                MXS_I2C_CTRL1_EARLY_TERM_IRQ | \
> @@ -298,6 +306,128 @@ write_init_pio_fail:
>       return -EINVAL;
>  }
>  
> +static int mxs_i2c_pio_wait_dmareq(struct mxs_i2c_dev *i2c)
> +{
> +     unsigned long timeout = jiffies + msecs_to_jiffies(1000);
> +
> +     while (!(readl(i2c->regs + MXS_I2C_DEBUG0) &
> +             MXS_I2C_DEBUG0_DMAREQ)) {
> +             if (time_after(jiffies, timeout))
> +                     return -ETIMEDOUT;
> +             cond_resched();
> +     }
> +
> +     writel(MXS_I2C_DEBUG0_DMAREQ, i2c->regs + MXS_I2C_DEBUG0_CLR);
> +
> +     return 0;
> +}
> +
> +static int mxs_i2c_pio_wait_cplt(struct mxs_i2c_dev *i2c)
> +{
> +     unsigned long timeout = jiffies + msecs_to_jiffies(1000);
> +
> +     while (!(readl(i2c->regs + MXS_I2C_CTRL1) &
> +             MXS_I2C_CTRL1_DATA_ENGINE_CMPLT_IRQ)) {
> +             if (time_after(jiffies, timeout))
> +                     return -ETIMEDOUT;
> +             cond_resched();
> +     }
> +
> +     writel(MXS_I2C_CTRL1_DATA_ENGINE_CMPLT_IRQ,
> +             i2c->regs + MXS_I2C_CTRL1_CLR);
> +
> +     return 0;
> +}

That's a lot of polling. Didn't interrupts work?

> +static int mxs_i2c_pio_setup_xfer(struct i2c_adapter *adap,
> +                     struct i2c_msg *msg, uint32_t flags)
> +{
> +     struct mxs_i2c_dev *i2c = i2c_get_adapdata(adap);
> +     uint32_t addr_data = msg->addr << 1;
> +     uint32_t data = 0;
> +     int i, shifts_left, ret;
> +
> +     /* Mute IRQs coming from this block. */
> +     writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_CLR);
> +
> +     if (msg->flags & I2C_M_RD) {
> +             addr_data |= I2C_SMBUS_READ;
> +
> +             /* SELECT command. */
> +             writel(MXS_I2C_CTRL0_RUN | MXS_CMD_I2C_SELECT,
> +                     i2c->regs + MXS_I2C_CTRL0);
> +
> +             ret = mxs_i2c_pio_wait_dmareq(i2c);
> +             if (ret)
> +                     return ret;
> +
> +             writel(addr_data, i2c->regs + MXS_I2C_DATA);
> +
> +             ret = mxs_i2c_pio_wait_cplt(i2c);
> +             if (ret)
> +                     return ret;
> +
> +             /* READ command. */
> +             writel(MXS_I2C_CTRL0_RUN | MXS_CMD_I2C_READ | flags |
> +                     MXS_I2C_CTRL0_XFER_COUNT(msg->len),
> +                     i2c->regs + MXS_I2C_CTRL0);
> +
> +             for (i = 0; i < msg->len; i++) {
> +                     if ((i & 3) == 0) {
> +                             ret = mxs_i2c_pio_wait_dmareq(i2c);
> +                             if (ret)
> +                                     return ret;
> +                             data = readl(i2c->regs + MXS_I2C_DATA);
> +                     }
> +                     msg->buf[i] = data & 0xff;
> +                     data >>= 8;
> +             }
> +     } else {
> +             addr_data |= I2C_SMBUS_WRITE;
> +
> +             /* WRITE command. */
> +             writel(MXS_I2C_CTRL0_RUN | MXS_CMD_I2C_WRITE | flags |
> +                     MXS_I2C_CTRL0_XFER_COUNT(msg->len + 1),
> +                     i2c->regs + MXS_I2C_CTRL0);
> +
> +             /*
> +              * The LSB of data buffer is the first byte blasted across
> +              * the bus. Higher order bytes follow. Thus the following
> +              * filling schematic.
> +              */

Ah, my old code again :) What was wrong with my comment?

> +             data = addr_data << 24;
> +             for (i = 0; i < msg->len; i++) {
> +                     data >>= 8;
> +                     data |= (msg->buf[i] << 24);
> +                     if ((i & 3) == 2) {
> +                             ret = mxs_i2c_pio_wait_dmareq(i2c);
> +                             if (ret)
> +                                     return ret;
> +                             writel(data, i2c->regs + MXS_I2C_DATA);
> +                     }
> +             }
> +
> +             shifts_left = 24 - (i & 3) * 8;
> +             if (shifts_left) {
> +                     data >>= shifts_left;
> +                     ret = mxs_i2c_pio_wait_dmareq(i2c);
> +                     if (ret)
> +                             return ret;
> +                     writel(data, i2c->regs + MXS_I2C_DATA);
> +             }
> +     }
> +
> +     ret = mxs_i2c_pio_wait_cplt(i2c);
> +     if (ret)
> +             return ret;
> +
> +     /* Clear any dangling IRQs and re-enable interrupts. */
> +     writel(MXS_I2C_IRQ_MASK, i2c->regs + MXS_I2C_CTRL1_CLR);
> +     writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_SET);
> +
> +     return 0;
> +}
> +
>  /*
>   * Low level master read/write transaction.
>   */
> @@ -316,24 +446,31 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, 
> struct i2c_msg *msg,
>       if (msg->len == 0)
>               return -EINVAL;
>  
> -     INIT_COMPLETION(i2c->cmd_complete);
> -     i2c->cmd_err = 0;
> -
> -     ret = mxs_i2c_dma_setup_xfer(adap, msg, flags);
> -     if (ret)
> -             return ret;
> +     if (msg->len < 8) {

Some THRESHOLD define would be nice here.

> +             ret = mxs_i2c_pio_setup_xfer(adap, msg, flags);
> +             if (ret)
> +                     mxs_i2c_reset(i2c);
> +     } else {
> +             i2c->cmd_err = 0;
> +             INIT_COMPLETION(i2c->cmd_complete);
> +             ret = mxs_i2c_dma_setup_xfer(adap, msg, flags);
> +             if (ret)
> +                     return ret;
>  
> -     ret = wait_for_completion_timeout(&i2c->cmd_complete,
> +             ret = wait_for_completion_timeout(&i2c->cmd_complete,
>                                               msecs_to_jiffies(1000));
> -     if (ret == 0)
> -             goto timeout;
> +             if (ret == 0)
> +                     goto timeout;
> +
> +             if (i2c->cmd_err == -ENXIO)
> +                     mxs_i2c_reset(i2c);
>  
> -     if (i2c->cmd_err == -ENXIO)
> -             mxs_i2c_reset(i2c);
> +             ret = i2c->cmd_err;
> +     }
>  
> -     dev_dbg(i2c->dev, "Done with err=%d\n", i2c->cmd_err);
> +     dev_dbg(i2c->dev, "Done with err=%d\n", ret);
>  
> -     return i2c->cmd_err;
> +     return ret;
>  
>  timeout:
>       dev_dbg(i2c->dev, "Timeout!\n");

Regards,

   Wolfram
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to