From patchwork Fri Apr 12 09:30:03 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= X-Patchwork-Id: 235999 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id A70A12C00B4 for ; Fri, 12 Apr 2013 19:30:23 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753421Ab3DLJaN (ORCPT ); Fri, 12 Apr 2013 05:30:13 -0400 Received: from metis.ext.pengutronix.de ([92.198.50.35]:57578 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753405Ab3DLJaF (ORCPT ); Fri, 12 Apr 2013 05:30:05 -0400 Received: from dude.hi.pengutronix.de ([2001:6f8:1178:2:21e:67ff:fe11:9c5c]) by metis.ext.pengutronix.de with esmtp (Exim 4.72) (envelope-from ) id 1UQaJ2-0007Pi-4U; Fri, 12 Apr 2013 11:30:04 +0200 Received: from ukl by dude.hi.pengutronix.de with local (Exim 4.80) (envelope-from ) id 1UQaJ1-0008Lm-K9; Fri, 12 Apr 2013 11:30:03 +0200 Date: Fri, 12 Apr 2013 11:30:03 +0200 From: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= To: Marek Vasut , Wolfram Sang Cc: linux-i2c@vger.kernel.org Subject: I2C_M_RECV_LEN for i2c-mxs Message-ID: <20130412093003.GE30416@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-SA-Exim-Connect-IP: 2001:6f8:1178:2:21e:67ff:fe11:9c5c X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-i2c@vger.kernel.org Sender: linux-i2c-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-i2c@vger.kernel.org Hello, I'm currently trying to teach the i2c-mxs driver to handle the I2C_M_RECV_LEN flag but failed up to now. Maybe someone has an good idea how to make it running? My current patch looks as follows: The problem is that mxs_i2c_pio_wait_dmareq that is called in the if (msg->flags & I2C_M_RECV_LEN) block times out, i.e. the hardware doesn't set MXS_I2C_DEBUG0_DMAREQ after the read request for the length byte. The same problem happens when I use the dma path to implement I2C_M_RECV_LEN support. In that case mxs_i2c_isr is called, but the dma callback is not. On the wire the following happens for a i2c_smbus_read_block_data call (from userspace): S $clientaddr W A $command A Sr $clientaddr R A 0x01 as expected and then the i.MX28 starts sending an ack: SDA ..____________/¯¯¯\____ SCL .._/¯\_/¯\_/¯\_/¯\_/¯¯¯ .. 5 6 7 8 9 slave sending | mxs starts length byte (1) | to send A but MXS_I2C_DEBUG0 stays 0 while mxs_i2c_pio_wait_dmareq waits. After that a P is put on the wire. That fits to what I see when using dma: HW_APBX_CH6_DEBUG1.STATEMACHINE == 0x15 meaning: WAIT_END — When the Wait for Command End bit is set, the state machine enters this state until the DMA device indicates that the command is complete. So the problem seems to be that the i2c core fails to tell the dma core that it's done. Other things I wonder about in the i2c-mxs driver: - I don't understand why RETAIN_CLOCK is added to MXS_CMD_I2C_SELECT. From my understanding of i2c and the hardware manual I'd say this is only relevant for slave mode? Setting RETAIN_CLOCK for the cycle that reads the length byte is broken in a different way. (After reading the 1 the mxs pulls low SDA for acking but doesn't pulse SCL. Then another 9 clock pulses are sent with SDA high. After the 9th falling edge SDA is pulled low by the i.MX28 and then both lines go back to 1 at the same time. I will debug a bit more here.) So it seems RETAIN_CLOCK has a meaning for master mode, too. If you have a more concrete perception of its meaning it would be great if you could share it. - Is it correct to have I2C_FUNC_SMBUS_EMUL in .functionality without supporting I2C_M_RECV_LEN? Best regards Uwe diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -1,3 +1,4 @@ +#define DEBUG /* * Freescale MXS I2C bus driver * @@ -289,14 +290,23 @@ write_init_pio_fail: static int mxs_i2c_pio_wait_dmareq(struct mxs_i2c_dev *i2c) { unsigned long timeout = jiffies + msecs_to_jiffies(1000); + u32 reg, reg_old; + int first = 1; - while (!(readl(i2c->regs + MXS_I2C_DEBUG0) & + while (!(reg = readl(i2c->regs + MXS_I2C_DEBUG0) & MXS_I2C_DEBUG0_DMAREQ)) { + if (first || reg != reg_old) { + pr_debug("%s: I2C_DEBUG = 0x%08x\n", __func__, reg); + first = 0; + reg_old = reg; + } if (time_after(jiffies, timeout)) return -ETIMEDOUT; cond_resched(); } + pr_debug("%s: I2C_DEBUG = 0x%08x\n", __func__, reg); + writel(MXS_I2C_DEBUG0_DMAREQ, i2c->regs + MXS_I2C_DEBUG0_CLR); return 0; @@ -338,6 +348,9 @@ static int mxs_i2c_pio_setup_xfer(struct i2c_adapter *adap, writel(MXS_I2C_IRQ_MASK << 8, i2c->regs + MXS_I2C_CTRL1_CLR); if (msg->flags & I2C_M_RD) { + int readlen = msg->len; + size_t buf_offset = 0; + addr_data |= I2C_SMBUS_READ; /* SELECT command. */ @@ -354,19 +367,39 @@ static int mxs_i2c_pio_setup_xfer(struct i2c_adapter *adap, if (ret) return ret; + if (msg->flags & I2C_M_RECV_LEN) { + pr_debug("read first byte\n"); + writel(MXS_I2C_CTRL0_RUN | MXS_I2C_CTRL0_MASTER_MODE | + MXS_I2C_CTRL0_XFER_COUNT(1), + i2c->regs + MXS_I2C_CTRL0); + + ret = mxs_i2c_pio_wait_dmareq(i2c); + if (ret) + return ret; + + data = readl(i2c->regs + MXS_I2C_DATA) & 0xff; + if (data > I2C_SMBUS_BLOCK_MAX) + return -EPROTO; + + msg->len += data; + readlen += data - 1; + } + + pr_debug("%s: data = %u, msg->len = %u, readlen = %u\n", __func__, data, msg->len, readlen); + /* READ command. */ writel(MXS_I2C_CTRL0_RUN | MXS_CMD_I2C_READ | flags | - MXS_I2C_CTRL0_XFER_COUNT(msg->len), + MXS_I2C_CTRL0_XFER_COUNT(readlen), i2c->regs + MXS_I2C_CTRL0); - for (i = 0; i < msg->len; i++) { + for (i = 0; i < readlen; 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; + msg->buf[buf_offset + i] = data & 0xff; data >>= 8; } } else { @@ -438,6 +471,9 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, * is set to 8 bytes, transfers shorter than 8 bytes are transfered * using PIO mode while longer transfers use DMA. The 8 byte border is * based on this empirical measurement and a lot of previous frobbing. + * + * Note that only mxs_i2c_pio_setup_xfer has support for I2C_M_RECV_LEN. + * That is OK as msg->len is <= 2 when that flag is set. */ if (msg->len < 8) { ret = mxs_i2c_pio_setup_xfer(adap, msg, flags); @@ -497,6 +533,8 @@ static irqreturn_t mxs_i2c_isr(int this_irq, void *dev_id) struct mxs_i2c_dev *i2c = dev_id; u32 stat = readl(i2c->regs + MXS_I2C_CTRL1) & MXS_I2C_IRQ_MASK; + pr_debug("%s: stat = 0x%08x\n", __func__, stat); + if (!stat) return IRQ_NONE;