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 <marex@xxxxxxx>
> Cc: Fabio Estevam <fabio.estevam@xxxxxxxxxxxxx>
> Cc: Shawn Guo <shawn.guo@xxxxxxxxxx>
> Cc: Wolfram Sang <w.sang@xxxxxxxxxxxxxx>
> ---
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
