[i2c] [PATCH] I2C bus driver for Philips ARM boards
Jean Delvare
khali at linux-fr.org
Thu Sep 28 11:23:11 CEST 2006
Hi Vitaly,
> please find the next iteration of the I2C PNX driver (differs
> from the previous one mostly by whitespace cleanups...)
Final review:
> arch/arm/mach-pnx4008/Makefile | 2
> arch/arm/mach-pnx4008/i2c.c | 167 ++++++++
> drivers/i2c/busses/Kconfig | 19
> drivers/i2c/busses/Makefile | 1
> drivers/i2c/busses/i2c-pnx.c | 734 +++++++++++++++++++++++++++++++++++++
> include/asm-arm/arch-pnx4008/i2c.h | 67 +++
> include/linux/i2c-pnx.h | 44 ++
> 7 files changed, 1033 insertions(+), 1 deletion(-)
>
> Signed-off-by: Vitaly Wool <vitalywool at gmail.com>
>
> Index: linux-2.6.git/drivers/i2c/busses/Kconfig
> ===================================================================
> --- linux-2.6.git.orig/drivers/i2c/busses/Kconfig
> +++ linux-2.6.git/drivers/i2c/busses/Kconfig
> @@ -537,4 +537,23 @@ config I2C_MV64XXX
> This driver can also be built as a module. If so, the module
> will be called i2c-mv64xxx.
>
> +config I2C_PNX
> + tristate "I2C bus support for Philips PNX targets"
> + depends on ARCH_PNX4008 && I2C
> + help
> + This driver supports the Philips IP3204 I2C IP block master and/or
> + slave controller
> +
> + This driver can also be built as a module. If so, the module
> + will be called i2c-pnx.
> +
> +config I2C_PNX_EARLY
> + bool "Early initialization for I2C on PNXxxxx"
> + depends on I2C_PNX=y
> + help
> + Under certain circumstances one may need to make sure I2C on PNXxxxx
> + is initialized earlier than some other driver that depends on it
> + (for instance, that might be USB in case of PNX4008). With this
> + option turned on you can guarantee that.
> +
> endmenu
> Index: linux-2.6.git/drivers/i2c/busses/Makefile
> ===================================================================
> --- linux-2.6.git.orig/drivers/i2c/busses/Makefile
> +++ linux-2.6.git/drivers/i2c/busses/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_I2C_PARPORT) += i2c-parport
> obj-$(CONFIG_I2C_PARPORT_LIGHT) += i2c-parport-light.o
> obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
> obj-$(CONFIG_I2C_PIIX4) += i2c-piix4.o
> +obj-$(CONFIG_I2C_PNX) += i2c-pnx.o
> obj-$(CONFIG_I2C_PROSAVAGE) += i2c-prosavage.o
> obj-$(CONFIG_I2C_PXA) += i2c-pxa.o
> obj-$(CONFIG_I2C_RPXLITE) += i2c-rpx.o
> Index: linux-2.6.git/drivers/i2c/busses/i2c-pnx.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/drivers/i2c/busses/i2c-pnx.c
> @@ -0,0 +1,734 @@
> +/*
> + * Provides I2C support for Philips PNX010x/PNX4008 boards.
> + *
> + * Authors: Dennis Kovalev <dkovalev at ru.mvista.com>
> + * Vitaly Wool <vwool at ru.mvista.com>
> + *
> + * 2004-2006 (c) MontaVista Software, Inc. This file is licensed under
> + * the terms of the GNU General Public License version 2. This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioport.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/timer.h>
> +#include <linux/completion.h>
> +#include <linux/platform_device.h>
> +#include <linux/i2c-pnx.h>
> +#include <asm/hardware.h>
> +#include <asm/irq.h>
> +#include <asm/uaccess.h>
> +
> +#define I2C_PNX_TIMEOUT 200 /* msec */
> +#define I2C_PNX_SPEED_KHZ 100
> +#define I2C_PNX_REGION_SIZE 0x100
> +#define PNX_DEFAULT_FREQ 13 /* MHz */
> +
> +static inline void i2c_pnx_arm_timer(struct i2c_adapter *adap)
> +{
> + struct i2c_pnx_algo_data *data = adap->algo_data;
> + struct timer_list *timer = &data->mif.timer;
> + int expires = I2C_PNX_TIMEOUT / (1000 / HZ);
> +
> + del_timer_sync(timer);
> +
> + dev_dbg(&adap->dev, "Timer armed at %lu plus %u jiffies.\n",
> + jiffies, expires);
> +
> + timer->expires = jiffies + expires;
> + timer->data = (unsigned long)data;
> +
> + add_timer(timer);
> +}
> +
> +static inline int i2c_pnx_bus_busy(struct i2c_pnx_algo_data *a)
> +{
> + long timeout = I2C_PNX_TIMEOUT;
> +
> + while (timeout > 0 && (ioread32(I2C_REG_STS(a)) & mstatus_active)) {
> + mdelay(1);
> + timeout--;
> + }
Huh. I see you've been replacing all sleeps by delays. I guess you were
trying to address my concern that you could be sleeping longer than
expected. I agree mdelay() will respect the requested duration better,
however this is busy waiting! That's hardly acceptable when waiting for
up to 200 ms. You really want to sleep here, or make the timeout value
much shorter. So your options are:
* Use msleep() and ignore my comments about timouts being longer than
expected. Maybe it's not really a problem in practice after all.
* Use msleep() and make I2C_PNX_TIMEOUT lower because you know it'll
always sleep longer that you asked for.
* Use mdelay() and make the timeout much shorter. I'd say 10 ms is
about the max you can busy-wait.
* Use msleep() with I2C_PNX_TIMEOUT unchanged, but check for the
cumulated time which was actually spend sleeping, rather than assuming
it was exactly what you asked for (as we know it isn't.) Something
like the following would do if I'm not mistaken:
long timeout = jiffies + HZ * I2C_PNX_TIMEOUT / 1000;
while (!time_after(jiffies, timeout) &&
(ioread32(I2C_REG_STS(a)) & mstatus_active)) {
msleep(1);
}
I believe option 4 is the cleanest one. But it's up to you as long as
you don't busy-wait for more than 10 ms.
> +
> + return (timeout <= 0) ? 1 : 0;
> +}
> +
> +/**
> + * i2c_pnx_start - start a device
> + * @slave_addr: slave address
> + * @adap: pointer to adapter structure
> + *
> + * Generate a START signal in the desired mode.
> + */
> +static int i2c_pnx_start(unsigned char slave_addr, struct i2c_adapter *adap)
> +{
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> +
> + dev_dbg(&adap->dev, "%s(): addr 0x%x mode %d\n", __FUNCTION__,
> + slave_addr, alg_data->mif.mode);
> +
> + /* Check for 7 bit slave addresses only */
> + if (slave_addr & ~0x7f) {
> + dev_err(&adap->dev, "%s: Invalid slave address %x. "
> + "Only 7-bit addresses are supported\n",
> + adap->name, slave_addr);
> + return -EINVAL;
> + }
> +
> + /* First, make sure bus is idle */
> + if (i2c_pnx_bus_busy(alg_data)) {
> + /* Somebody else is monopolizing the bus */
> + dev_err(&adap->dev, "%s: Bus busy. Slave addr = %02x, "
> + "cntrl = %x, stat = %x\n",
> + adap->name, slave_addr,
> + ioread32(I2C_REG_CTL(alg_data)),
> + ioread32(I2C_REG_STS(alg_data)));
> + return -EBUSY;
> + } else if (ioread32(I2C_REG_STS(alg_data)) & mstatus_afi) {
> + /* Sorry, we lost the bus */
> + dev_err(&adap->dev, "%s: Arbitration failure. "
> + "Slave addr = %02x\n", adap->name, slave_addr);
> + return -EIO;
> + }
> +
> + /* OK, I2C is enabled and we have the bus. */
> + /* Clear the current TDI and AFI status flags. */
> + iowrite32(ioread32(I2C_REG_STS(alg_data)) | mstatus_tdi | mstatus_afi,
> + I2C_REG_STS(alg_data));
> +
> + dev_dbg(&adap->dev, "%s(): sending %#x\n", __FUNCTION__,
> + (slave_addr << 1) | start_bit | alg_data->mif.mode);
> +
> + /* Write the slave address, START bit and R/W bit */
> + iowrite32((slave_addr << 1) | start_bit | alg_data->mif.mode,
> + I2C_REG_TX(alg_data));
> +
> + dev_dbg(&adap->dev, "%s(): exit\n", __FUNCTION__);
> +
> + return 0;
> +}
> +
> +/**
> + * i2c_pnx_stop - stop a device
> + * @adap: pointer to I2C adapter structure
> + *
> + * Generate a STOP signal to terminate the master transaction.
> + */
> +static void i2c_pnx_stop(struct i2c_adapter *adap)
> +{
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> + /* Only 1 msec max timeout due to interrupt context */
> + long timeout = 5 * I2C_PNX_TIMEOUT;
You can't really say "only 1 msec" when the value depends on
I2C_PNX_TIMEOUT which is defined elsewhere. Furthermore, you are
abusing the value of I2C_PNX_TIMEOUT here, it is supposed to be a
number of msec and you use it as a number of usec. You'd be better
hardcoding the value, or defining a separate constant.
> +
> + dev_dbg(&adap->dev, "%s(): entering: stat = %04x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +
> + /* Write a STOP bit to TX FIFO */
> + iowrite32(0xff | stop_bit, I2C_REG_TX(alg_data));
> +
> + /* Wait until the STOP is seen. */
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_STS(alg_data)) & mstatus_active)) {
> + /* may be called from interrupt context */
> + udelay(1);
> + timeout--;
> + }
> +
> + dev_dbg(&adap->dev, "%s(): exiting: stat = %04x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +}
> +
> +/**
> + * i2c_pnx_master_xmit - transmit data to slave
> + * @adap: pointer to I2C adapter structure
> + *
> + * Sends one byte of data to the slave
> + */
> +static int i2c_pnx_master_xmit(struct i2c_adapter *adap)
> +{
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> + u32 val;
> + long timeout = I2C_PNX_TIMEOUT;
> +
> + dev_dbg(&adap->dev, "%s(): entering: stat = %04x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +
> + if (alg_data->mif.len > 0) {
> + /* We still have something to talk about... */
> + val = *alg_data->mif.buf++;
> +
> + if (alg_data->mif.len == 1) {
> + val |= stop_bit;
> + if (!alg_data->last)
> + val |= start_bit;
> + }
Ah, I see you finally found a way to implement the repeated start
condition, good :)
> +
> + alg_data->mif.len--;
> + iowrite32(val, I2C_REG_TX(alg_data));
> +
> + dev_dbg(&adap->dev, "%s(): xmit %#x [%d]\n", __FUNCTION__,
> + val, alg_data->mif.len + 1);
> +
> + if (alg_data->mif.len == 0) {
> + if (alg_data->last) {
> + /* Wait until the STOP is seen. */
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_STS(alg_data)) &
> + mstatus_active)) {
> + mdelay(1);
> + timeout--;
> + }
> + if (timeout <= 0)
> + dev_err(&adap->dev, "The bus is still "
> + "active after timeout\n");
> + }
> + /* Disable master interrupts */
> + iowrite32(ioread32(I2C_REG_CTL(alg_data)) &
> + ~(mcntrl_afie | mcntrl_naie | mcntrl_drmie),
> + I2C_REG_CTL(alg_data));
> +
> + del_timer_sync(&alg_data->mif.timer);
> +
> + dev_dbg(&adap->dev, "%s(): Waking up xfer routine.\n",
> + __FUNCTION__);
> +
> + complete(&alg_data->mif.complete);
> + }
> + } else if (alg_data->mif.len == 0) {
> + /* zero-sized transfer */
> + i2c_pnx_stop(adap);
> +
> + /* Disable master interrupts. */
> + iowrite32(ioread32(I2C_REG_CTL(alg_data)) &
> + ~(mcntrl_afie | mcntrl_naie | mcntrl_drmie),
> + I2C_REG_CTL(alg_data));
> +
> + /* Stop timer. */
> + del_timer_sync(&alg_data->mif.timer);
> + dev_dbg(&adap->dev, "%s(): Waking up xfer routine after "
> + "zero-xfer.\n", __FUNCTION__);
> +
> + complete(&alg_data->mif.complete);
> + }
> +
> + dev_dbg(&adap->dev, "%s(): exiting: stat = %04x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +
> + return 0;
> +}
> +
> +/**
> + * i2c_pnx_master_rcv - receive data from slave
> + * @adap: pointer to I2C adapter structure
> + *
> + * Reads one byte data from the slave
> + */
> +static int i2c_pnx_master_rcv(struct i2c_adapter *adap)
> +{
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> + unsigned int val = 0;
> + long timeout = I2C_PNX_TIMEOUT;
> + u32 ctl = 0;
> +
> + dev_dbg(&adap->dev, "%s(): entering: stat = %04x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +
> + /* Check, whether there is already data,
> + * or we didn't 'ask' for it yet.
> + */
> + if (ioread32(I2C_REG_STS(alg_data)) & mstatus_rfe) {
> + dev_dbg(&adap->dev, "%s(): Write dummy data to fill "
> + "Rx-fifo...\n", __FUNCTION__);
> +
> + if (alg_data->mif.len == 1) {
> + /* Last byte, do not acknowledge next rcv. */
> + val |= stop_bit;
You don't seem to properly generate the repeated start when in receive
mode, only when in the transmit mode? You need both.
> +
> + /*
> + * Enable interrupt RFDAIE (data in Rx fifo),
> + * and disable DRMIE (need data for Tx)
> + */
> + ctl = ioread32(I2C_REG_CTL(alg_data));
> + ctl |= mcntrl_rffie | mcntrl_daie;
> + ctl &= ~mcntrl_drmie;
> + iowrite32(ctl, I2C_REG_CTL(alg_data));
> + }
> +
> + /*
> + * Now we'll 'ask' for data:
> + * For each byte we want to receive, we must
> + * write a (dummy) byte to the Tx-FIFO.
> + */
> + iowrite32(val, I2C_REG_TX(alg_data));
> +
> + return 0;
> + }
> +
> + /* Handle data. */
> + if (alg_data->mif.len > 0) {
> + val = ioread32(I2C_REG_RX(alg_data));
> + *alg_data->mif.buf++ = (u8) (val & 0xff);
> + dev_dbg(&adap->dev, "%s(): rcv 0x%x [%d]\n", __FUNCTION__, val,
> + alg_data->mif.len);
> +
> + alg_data->mif.len--;
> + if (alg_data->mif.len == 0) {
> + /* Wait until the STOP is seen. */
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_STS(alg_data)) &
> + mstatus_active)) {
> + mdelay(1);
> + timeout--;
> + }
> +
> + /* Disable master interrupts */
> + ctl = ioread32(I2C_REG_CTL(alg_data));
> + ctl &= ~(mcntrl_afie | mcntrl_naie | mcntrl_rffie |
> + mcntrl_drmie | mcntrl_daie);
> + iowrite32(ctl, I2C_REG_CTL(alg_data));
> +
> + /* Kill timer. */
> + del_timer_sync(&alg_data->mif.timer);
> + complete(&alg_data->mif.complete);
> + }
> + }
> +
> + dev_dbg(&adap->dev, "%s(): exiting: stat = %04x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +
> + return 0;
> +}
> +
> +static irqreturn_t
> +i2c_pnx_interrupt(int irq, void *dev_id, struct pt_regs *regs)
> +{
> + u32 stat, ctl;
> + struct i2c_adapter *adap = dev_id;
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> +
> + dev_dbg(&adap->dev, "%s(): mstat = %x mctrl = %x, mode = %d\n",
> + __FUNCTION__,
> + ioread32(I2C_REG_STS(alg_data)),
> + ioread32(I2C_REG_CTL(alg_data)),
> + alg_data->mif.mode);
> + stat = ioread32(I2C_REG_STS(alg_data));
> +
> + /* let's see what kind of event this is */
> + if (stat & mstatus_afi) {
> + /* We lost arbitration in the midst of a transfer */
> + alg_data->mif.ret = -EIO;
> +
> + /* Disable master interrupts. */
> + ctl = ioread32(I2C_REG_CTL(alg_data));
> + ctl &= ~(mcntrl_afie | mcntrl_naie | mcntrl_rffie |
> + mcntrl_drmie);
> + iowrite32(ctl, I2C_REG_CTL(alg_data));
> +
> + /* Stop timer, to prevent timeout. */
> + del_timer_sync(&alg_data->mif.timer);
> + complete(&alg_data->mif.complete);
> + } else if (stat & mstatus_nai) {
> + /* Slave did not acknowledge, generate a STOP */
> + dev_dbg(&adap->dev, "%s(): "
> + "Slave did not acknowledge, generating a STOP.\n",
> + __FUNCTION__);
> + i2c_pnx_stop(adap);
> +
> + /* Disable master interrupts. */
> + ctl = ioread32(I2C_REG_CTL(alg_data));
> + ctl &= ~(mcntrl_afie | mcntrl_naie | mcntrl_rffie |
> + mcntrl_drmie);
> + iowrite32(ctl, I2C_REG_CTL(alg_data));
> +
> + /* Our return value. */
> + alg_data->mif.ret = -EIO;
> +
> + /* Stop timer, to prevent timeout. */
> + del_timer_sync(&alg_data->mif.timer);
> + complete(&alg_data->mif.complete);
> + } else {
> + /*
> + * Two options:
> + * - Master Tx needs data.
> + * - There is data in the Rx-fifo
> + * The latter is only the case if we have requested for data,
> + * via a dummy write. (See 'i2c_pnx_master_rcv'.)
> + * We therefore check, as a sanity check, whether that interrupt
> + * has been enabled.
> + */
> + if ((stat & mstatus_drmi) || !(stat & mstatus_rfe)) {
> + if (alg_data->mif.mode == I2C_SMBUS_WRITE) {
> + i2c_pnx_master_xmit(adap);
> + } else if (alg_data->mif.mode == I2C_SMBUS_READ) {
> + i2c_pnx_master_rcv(adap);
> + }
> + }
> + }
> +
> + /* Clear TDI and AFI bits */
> + stat = ioread32(I2C_REG_STS(alg_data));
> + iowrite32(stat | mstatus_tdi | mstatus_afi, I2C_REG_STS(alg_data));
> +
> + dev_dbg(&adap->dev, "%s(): exiting, stat = %x ctrl = %x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)),
> + ioread32(I2C_REG_CTL(alg_data)));
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void i2c_pnx_timeout(unsigned long data)
> +{
> + struct i2c_adapter *adap = (struct i2c_adapter *)data;
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> + u32 ctl;
> + long timeout = I2C_PNX_TIMEOUT;
> +
> + dev_err(&adap->dev, "Master timed out. stat = %04x, cntrl = %04x. "
> + "Resetting master...\n",
> + ioread32(I2C_REG_STS(alg_data)),
> + ioread32(I2C_REG_CTL(alg_data)));
> +
> + /* Reset master and disable interrupts */
> + ctl = ioread32(I2C_REG_CTL(alg_data));
> + ctl &= ~(mcntrl_afie | mcntrl_naie | mcntrl_rffie | mcntrl_drmie);
> + iowrite32(ctl, I2C_REG_CTL(alg_data));
> +
> + ctl |= mcntrl_reset;
> + iowrite32(ctl, I2C_REG_CTL(alg_data));
> +
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_CTL(alg_data)) & mcntrl_reset)) {
> + mdelay(1);
> + timeout -= 1000;
Should now be timeout--.
> + }
> + alg_data->mif.ret = -EIO;
> +
> + complete(&alg_data->mif.complete);
> +}
> +
> +static inline void bus_reset_if_active(struct i2c_adapter *adap)
> +{
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> + u32 stat;
> + long timeout = I2C_PNX_TIMEOUT;
> +
> + if ((stat = ioread32(I2C_REG_STS(alg_data))) & mstatus_active) {
> + dev_err(&adap->dev,
> + "%s: Bus is still active after xfer. Reset it...\n",
> + adap->name);
> + iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_reset,
> + I2C_REG_CTL(alg_data));
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_CTL(alg_data)) & mcntrl_reset)) {
> + mdelay(1);
> + timeout -= 1000;
Same here.
> + }
> + } else if (!(stat & mstatus_rfe) || !(stat & mstatus_tfe)) {
> + /* If there is data in the fifo's after transfer,
> + * flush fifo's by reset.
> + */
> + iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_reset,
> + I2C_REG_CTL(alg_data));
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_CTL(alg_data)) & mcntrl_reset)) {
> + mdelay(1);
> + timeout -= 1000;
And here.
> + }
> + } else if (stat & mstatus_nai) {
> + iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_reset,
> + I2C_REG_CTL(alg_data));
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_CTL(alg_data)) & mcntrl_reset)) {
> + mdelay(1);
> + timeout -= 1000;
And here.
> + }
> + }
> +}
> +
> +/**
> + * i2c_pnx_xfer - generic transfer entry point
> + * @adap: pointer to I2C adapter structure
> + * @msgs: array of messages
> + * @num: number of messages
> + *
> + * Initiates the transfer
> + */
> +static int
> +i2c_pnx_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> + struct i2c_msg *pmsg;
> + int rc = 0, completed = 0, i;
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> + u32 stat = ioread32(I2C_REG_STS(alg_data));
> +
> + dev_dbg(&adap->dev, "%s(): entering: stat = %04x.\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +
> + bus_reset_if_active(adap);
> +
> + /* Process transactions in a loop. */
> + for (i = 0; rc >= 0 && i < num; i++) {
> + u8 addr;
> +
> + pmsg = &msgs[i];
> + addr = pmsg->addr;
> +
> + if (pmsg->flags & I2C_M_TEN) {
> + dev_err(&adap->dev,
> + "%s: 10 bits addr not supported!\n",
> + adap->name);
> + rc = -EINVAL;
> + break;
> + }
> +
> + alg_data->mif.buf = pmsg->buf;
> + alg_data->mif.len = pmsg->len;
> + alg_data->mif.mode = (pmsg->flags & I2C_M_RD) ?
> + I2C_SMBUS_READ : I2C_SMBUS_WRITE;
> + alg_data->mif.ret = 0;
> + alg_data->last = (i == num - 1);
> +
> + dev_dbg(&adap->dev, "%s(): mode %d, %d bytes\n", __FUNCTION__,
> + alg_data->mif.mode,
> + alg_data->mif.len);
> +
> + i2c_pnx_arm_timer(adap);
> +
> + /* initialize the completion var */
> + init_completion(&alg_data->mif.complete);
> +
> + /* Enable master interrupt */
> + iowrite32(ioread32(I2C_REG_CTL(alg_data)) | mcntrl_afie |
> + mcntrl_naie | mcntrl_drmie,
> + I2C_REG_CTL(alg_data));
> +
> + /* Put start-code and slave-address on the bus. */
> + rc = i2c_pnx_start(addr, adap);
> + if (rc < 0)
> + break;
> +
> + /* Wait for completion */
> + wait_for_completion(&alg_data->mif.complete);
> +
> + if (!(rc = alg_data->mif.ret))
> + completed++;
> + dev_dbg(&adap->dev, "%s(): Complete, return code = %d.\n",
> + __FUNCTION__, rc);
> +
> + /* Clear TDI and AFI bits in case they are set. */
> + if ((stat = ioread32(I2C_REG_STS(alg_data))) & mstatus_tdi) {
> + dev_dbg(&adap->dev,
> + "%s: TDI still set... clearing now.\n",
> + adap->name);
> + iowrite32(stat, I2C_REG_STS(alg_data));
> + }
> + if ((stat = ioread32(I2C_REG_STS(alg_data))) & mstatus_afi) {
> + dev_dbg(&adap->dev,
> + "%s: AFI still set... clearing now.\n",
> + adap->name);
> + iowrite32(stat, I2C_REG_STS(alg_data));
> + }
> + }
> +
> + bus_reset_if_active(adap);
> +
> + /* Cleanup to be sure... */
> + alg_data->mif.buf = NULL;
> + alg_data->mif.len = 0;
> +
> + dev_dbg(&adap->dev, "%s(): exiting, stat = %x\n",
> + __FUNCTION__, ioread32(I2C_REG_STS(alg_data)));
> +
> + if (completed != num)
> + return ((rc < 0) ? rc : -EREMOTEIO);
> +
> + return num;
> +}
> +
> +static u32 i2c_pnx_func(struct i2c_adapter *adapter)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static struct i2c_algorithm pnx_algorithm = {
> + .master_xfer = i2c_pnx_xfer,
> + .functionality = i2c_pnx_func,
> +};
> +
> +static int i2c_pnx_controller_suspend(struct platform_device *pdev,
> + pm_message_t state)
> +{
> + struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
> + return i2c_pnx->suspend(pdev, state);
> +}
> +
> +static int i2c_pnx_controller_resume(struct platform_device *pdev)
> +{
> + struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
> + return i2c_pnx->resume(pdev);
> +}
> +
> +static int __devinit i2c_pnx_probe(struct platform_device *pdev)
> +{
> + unsigned long tmp;
> + int ret = 0;
> + struct i2c_pnx_algo_data *alg_data;
> + int freq_mhz;
> + struct i2c_pnx_data *i2c_pnx = pdev->dev.platform_data;
> + long timeout = I2C_PNX_TIMEOUT;
> +
> + if (!i2c_pnx) {
> + dev_err(&pdev->dev, "%s: no platform data supplied\n",
> + __FUNCTION__);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + platform_set_drvdata(pdev, i2c_pnx);
> +
> + if (i2c_pnx->calculate_input_freq)
> + freq_mhz = i2c_pnx->calculate_input_freq(pdev);
> + else {
> + freq_mhz = PNX_DEFAULT_FREQ;
> + dev_info(&pdev->dev, "Setting bus frequency to default value: "
> + "%d MHz", freq_mhz);
> + }
> +
> + i2c_pnx->adapter->algo = &pnx_algorithm;
> +
> + alg_data = i2c_pnx->adapter->algo_data;
> + init_timer(&alg_data->mif.timer);
> + alg_data->mif.timer.function = i2c_pnx_timeout;
> + alg_data->mif.timer.data = (unsigned long)i2c_pnx->adapter;
> +
> + /* Register I/O resource */
> + if (!request_region(alg_data->base, I2C_PNX_REGION_SIZE, pdev->name)) {
> + dev_err(&pdev->dev,
> + "I/O region 0x%08x for I2C already in use.\n",
> + alg_data->base);
> + ret = -ENODEV;
> + goto out_free;
> + }
> +
> + if (!(alg_data->ioaddr =
> + (u32)ioremap(alg_data->base, I2C_PNX_REGION_SIZE))) {
> + dev_err(&pdev->dev, "Couldn't ioremap I2C I/O region\n");
> + ret = -ENOMEM;
> + goto out_release;
> + }
> +
> + i2c_pnx->set_clock_run(pdev);
> +
> + /*
> + * Clock Divisor High This value is the number of system clocks
> + * the serial clock (SCL) will be high.
> + * For example, if the system clock period is 50 ns and the maximum
> + * desired serial period is 10000 ns (100 kHz), then CLKHI would be
> + * set to 0.5*(f_sys/f_i2c)-2=0.5*(20e6/100e3)-2=98. The actual value
> + * programmed into CLKHI will vary from this slightly due to
> + * variations in the output pad's rise and fall times as well as
> + * the deglitching filter length.
> + */
> +
> + tmp = ((freq_mhz * 1000) / I2C_PNX_SPEED_KHZ) / 2 - 2;
> + iowrite32(tmp, I2C_REG_CKH(alg_data));
> + iowrite32(tmp, I2C_REG_CKL(alg_data));
> +
> + iowrite32(mcntrl_reset, I2C_REG_CTL(alg_data));
> + while (timeout > 0 &&
> + (ioread32(I2C_REG_CTL(alg_data)) & mcntrl_reset)) {
> + mdelay(1);
> + timeout -= 1000;
timeout--
> + }
> + if (timeout <= 0) {
> + ret = -ENODEV;
> + goto out_unmap;
> + }
> + init_completion(&alg_data->mif.complete);
> +
> + ret = request_irq(alg_data->irq, i2c_pnx_interrupt,
> + 0, pdev->name, i2c_pnx->adapter);
> + if (ret)
> + goto out_clock;
> +
> + /* Register this adapter with the I2C subsystem */
> + i2c_pnx->adapter->dev.parent = &pdev->dev;
> + ret = i2c_add_adapter(i2c_pnx->adapter);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "I2C: Failed to add bus\n");
> + goto out_irq;
> + }
> +
> + dev_dbg(&pdev->dev, "%s: Master at %#8x, irq %d.\n",
> + i2c_pnx->adapter->name, alg_data->base, alg_data->irq);
> +
> + return 0;
> +
> +out_irq:
> + free_irq(alg_data->irq, alg_data);
> +out_clock:
> + i2c_pnx->set_clock_stop(pdev);
> +out_unmap:
> + iounmap((void *)alg_data->ioaddr);
> +out_release:
> + release_region(alg_data->base, I2C_PNX_REGION_SIZE);
> +out_free:
> + platform_set_drvdata(pdev, NULL);
> + kfree(i2c_pnx);
> +out:
> + return ret;
> +}
> +
> +static int __devexit i2c_pnx_remove(struct platform_device *pdev)
> +{
> + struct i2c_pnx_data *i2c_pnx = platform_get_drvdata(pdev);
> + struct i2c_adapter *adap = i2c_pnx->adapter;
> + struct i2c_pnx_algo_data *alg_data = adap->algo_data;
> +
> + free_irq(alg_data->irq, alg_data);
> + i2c_del_adapter(adap);
> + i2c_pnx->set_clock_stop(pdev);
> + iounmap((void *)alg_data->ioaddr);
> + release_region(alg_data->base, I2C_PNX_REGION_SIZE);
> + platform_set_drvdata(pdev, NULL);
> + kfree(i2c_pnx);
> +
> + return 0;
> +}
> +
> +static struct platform_driver i2c_pnx_driver = {
> + .driver = {
> + .name = "pnx-i2c",
> + .owner = THIS_MODULE,
> + },
> + .probe = i2c_pnx_probe,
> + .remove = __devexit_p(i2c_pnx_remove),
> + .suspend = i2c_pnx_controller_suspend,
> + .resume = i2c_pnx_controller_resume,
> +};
> +
> +static int __init i2c_adap_pnx_init(void)
> +{
> + return platform_driver_register(&i2c_pnx_driver);
> +}
> +
> +static void __exit i2c_adap_pnx_exit(void)
> +{
> + platform_driver_unregister(&i2c_pnx_driver);
> +}
> +
> +MODULE_AUTHOR("Vitaly Wool, Dennis Kovalev <source at mvista.com>");
> +MODULE_DESCRIPTION("I2C driver for Philips IP3204-based I2C busses");
> +MODULE_LICENSE("GPL");
> +
> +#ifdef CONFIG_I2C_PNX_EARLY
> +/* We need to make sure I2C is initialized before USB */
> +subsys_initcall(i2c_adap_pnx_init);
> +#else
> +mudule_init(i2c_adap_pnx_init);
> +#endif
> +module_exit(i2c_adap_pnx_exit);
> Index: linux-2.6.git/include/asm-arm/arch-pnx4008/i2c.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/include/asm-arm/arch-pnx4008/i2c.h
> @@ -0,0 +1,67 @@
> +/*
> + * PNX4008-specific tweaks for I2C IP3204 block
> + *
> + * Author: Vitaly Wool <vwool at ru.mvista.com>
> + *
> + * 2005 (c) MontaVista Software, Inc. This file is licensed under
> + * the terms of the GNU General Public License version 2. This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + */
> +
> +#ifndef __ASM_ARCH_I2C_H__
> +#define __ASM_ARCH_I2C_H__
> +
> +#include <linux/pm.h>
> +#include <linux/platform_device.h>
> +
> +enum {
> + mstatus_tdi = 0x00000001,
> + mstatus_afi = 0x00000002,
> + mstatus_nai = 0x00000004,
> + mstatus_drmi = 0x00000008,
> + mstatus_active = 0x00000020,
> + mstatus_scl = 0x00000040,
> + mstatus_sda = 0x00000080,
> + mstatus_rff = 0x00000100,
> + mstatus_rfe = 0x00000200,
> + mstatus_tff = 0x00000400,
> + mstatus_tfe = 0x00000800,
> +};
> +
> +enum {
> + mcntrl_tdie = 0x00000001,
> + mcntrl_afie = 0x00000002,
> + mcntrl_naie = 0x00000004,
> + mcntrl_drmie = 0x00000008,
> + mcntrl_daie = 0x00000020,
> + mcntrl_rffie = 0x00000040,
> + mcntrl_tffie = 0x00000080,
> + mcntrl_reset = 0x00000100,
> + mcntrl_cdbmode = 0x00000400,
> +};
> +
> +enum {
> + rw_bit = 1 << 0,
> + start_bit = 1 << 8,
> + stop_bit = 1 << 9,
> +};
> +
> +#define I2C_REG_RX(a) ((a)->ioaddr) /* Rx FIFO reg (RO) */
> +#define I2C_REG_TX(a) ((a)->ioaddr) /* Tx FIFO reg (WO) */
> +#define I2C_REG_STS(a) ((a)->ioaddr + 0x04) /* Status reg (RO) */
> +#define I2C_REG_CTL(a) ((a)->ioaddr + 0x08) /* Ctl reg */
> +#define I2C_REG_CKL(a) ((a)->ioaddr + 0x0c) /* Clock divider low */
> +#define I2C_REG_CKH(a) ((a)->ioaddr + 0x10) /* Clock divider high */
> +#define I2C_REG_ADR(a) ((a)->ioaddr + 0x14) /* I2C address */
> +#define I2C_REG_RFL(a) ((a)->ioaddr + 0x18) /* Rx FIFO level (RO) */
> +#define I2C_REG_TFL(a) ((a)->ioaddr + 0x1c) /* Tx FIFO level (RO) */
> +#define I2C_REG_RXB(a) ((a)->ioaddr + 0x20) /* Num of bytes Rx-ed (RO) */
> +#define I2C_REG_TXB(a) ((a)->ioaddr + 0x24) /* Num of bytes Tx-ed (RO) */
> +#define I2C_REG_TXS(a) ((a)->ioaddr + 0x28) /* Tx slave FIFO (RO) */
> +#define I2C_REG_STFL(a) ((a)->ioaddr + 0x2c) /* Tx slave FIFO level (RO) */
> +
> +#define HCLK_MHZ 13
> +#define I2C_CHIP_NAME "PNX4008-I2C"
> +
> +#endif /* __ASM_ARCH_I2C_H___ */
> Index: linux-2.6.git/include/linux/i2c-pnx.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/include/linux/i2c-pnx.h
> @@ -0,0 +1,44 @@
> +/*
> + * Header file for I2C support on PNX010x/4008.
> + *
> + * Author: Dennis Kovalev <dkovalev at ru.mvista.com>
> + *
> + * 2004-2006 (c) MontaVista Software, Inc. This file is licensed under
> + * the terms of the GNU General Public License version 2. This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + */
> +
> +#ifndef __I2C_PNX_H__
> +#define __I2C_PNX_H__
> +
> +#include <asm/arch/i2c.h>
> +
> +struct i2c_pnx_mif {
> + int ret; /* Return value */
> + int mode; /* Interface mode */
> + struct completion complete; /* I/O completion */
> + struct timer_list timer; /* Timeout */
> + char * buf; /* Data buffer */
> + int len; /* Length of data buffer */
> +};
> +
> +struct i2c_pnx_algo_data {
> + u32 base;
> + u32 ioaddr;
> + int irq;
> + /* buffer for data received from I2C bus */
> + struct i2c_pnx_mif mif;
> + int last;
> +};
> +
> +struct i2c_pnx_data {
> + int (*suspend) (struct platform_device *pdev, pm_message_t state);
> + int (*resume) (struct platform_device *pdev);
> + u32 (*calculate_input_freq) (struct platform_device *pdev);
> + int (*set_clock_run) (struct platform_device *pdev);
> + int (*set_clock_stop) (struct platform_device *pdev);
> + struct i2c_adapter *adapter;
> +};
> +
> +#endif /* __I2C_PNX_H__ */
> Index: linux-2.6.git/arch/arm/mach-pnx4008/Makefile
> ===================================================================
> --- linux-2.6.git.orig/arch/arm/mach-pnx4008/Makefile
> +++ linux-2.6.git/arch/arm/mach-pnx4008/Makefile
> @@ -2,7 +2,7 @@
> # Makefile for the linux kernel.
> #
>
> -obj-y := core.o irq.o time.o clock.o gpio.o serial.o dma.o
> +obj-y := core.o irq.o time.o clock.o gpio.o serial.o dma.o i2c.o
> obj-m :=
> obj-n :=
> obj- :=
> Index: linux-2.6.git/arch/arm/mach-pnx4008/i2c.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6.git/arch/arm/mach-pnx4008/i2c.c
> @@ -0,0 +1,167 @@
> +/*
> + * I2C initialization for PNX4008.
> + *
> + * Author: Vitaly Wool <vitalywool at gmail.com>
> + *
> + * 2005-2006 (c) MontaVista Software, Inc. This file is licensed under
> + * the terms of the GNU General Public License version 2. This program
> + * is licensed "as is" without any warranty of any kind, whether express
> + * or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-pnx.h>
> +#include <linux/platform_device.h>
> +#include <linux/err.h>
> +#include <asm/arch/platform.h>
> +#include <asm/arch/i2c.h>
> +
> +static int set_clock_run(struct platform_device *pdev)
> +{
> + struct clk *clk;
> + char name[10];
> + int retval = 0;
> +
> + snprintf(name, 10, "i2c%d_ck", pdev->id);
> + clk = clk_get(&pdev->dev, name);
> + if (!IS_ERR(clk)) {
> + clk_set_rate(clk, 1);
> + clk_put(clk);
> + } else
> + retval = -ENOENT;
> +
> + return retval;
> +}
> +
> +static int set_clock_stop(struct platform_device *pdev)
> +{
> + struct clk *clk;
> + char name[10];
> + int retval = 0;
> +
> + snprintf(name, 10, "i2c%d_ck", pdev->id);
> + clk = clk_get(&pdev->dev, name);
> + if (!IS_ERR(clk)) {
> + clk_set_rate(clk, 0);
> + clk_put(clk);
> + } else
> + retval = -ENOENT;
> +
> + return retval;
> +}
> +
> +static int i2c_pnx_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> + int retval = 0;
> +#ifdef CONFIG_PM
> + retval = set_clock_run(pdev);
> +#endif
> + return retval;
> +}
> +
> +static int i2c_pnx_resume(struct platform_device *pdev)
> +{
> + int retval = 0;
> +#ifdef CONFIG_PM
> + retval = set_clock_run(pdev);
> +#endif
> + return retval;
> +}
> +
> +static u32 calculate_input_freq(struct platform_device *pdev)
> +{
> + return HCLK_MHZ;
> +}
> +
> +
> +static struct i2c_pnx_algo_data pnx_algo_data0 = {
> + .base = PNX4008_I2C1_BASE,
> + .irq = I2C_1_INT,
> +};
> +
> +static struct i2c_pnx_algo_data pnx_algo_data1 = {
> + .base = PNX4008_I2C2_BASE,
> + .irq = I2C_2_INT,
> +};
> +
> +static struct i2c_pnx_algo_data pnx_algo_data2 = {
> + .base = (PNX4008_USB_CONFIG_BASE + 0x300),
> + .irq = USB_I2C_INT,
> +};
> +
> +static struct i2c_adapter pnx_adapter0 = {
> + .name = I2C_CHIP_NAME "0",
> + .algo_data = &pnx_algo_data0,
> +};
> +static struct i2c_adapter pnx_adapter1 = {
> + .name = I2C_CHIP_NAME "1",
> + .algo_data = &pnx_algo_data1,
> +};
> +
> +static struct i2c_adapter pnx_adapter2 = {
> + .name = "USB-I2C",
> + .algo_data = &pnx_algo_data2,
> +};
> +
> +static struct i2c_pnx_data i2c0_data = {
> + .suspend = i2c_pnx_suspend,
> + .resume = i2c_pnx_resume,
> + .calculate_input_freq = calculate_input_freq,
> + .set_clock_run = set_clock_run,
> + .set_clock_stop = set_clock_stop,
> + .adapter = &pnx_adapter0,
> +};
> +
> +static struct i2c_pnx_data i2c1_data = {
> + .suspend = i2c_pnx_suspend,
> + .resume = i2c_pnx_resume,
> + .calculate_input_freq = calculate_input_freq,
> + .set_clock_run = set_clock_run,
> + .set_clock_stop = set_clock_stop,
> + .adapter = &pnx_adapter1,
> +};
> +
> +static struct i2c_pnx_data i2c2_data = {
> + .suspend = i2c_pnx_suspend,
> + .resume = i2c_pnx_resume,
> + .calculate_input_freq = calculate_input_freq,
> + .set_clock_run = set_clock_run,
> + .set_clock_stop = set_clock_stop,
> + .adapter = &pnx_adapter2,
> +};
> +
> +static struct platform_device i2c0_device = {
> + .name = "pnx-i2c",
> + .id = 0,
> + .dev = {
> + .platform_data = &i2c0_data,
> + },
> +};
> +
> +static struct platform_device i2c1_device = {
> + .name = "pnx-i2c",
> + .id = 1,
> + .dev = {
> + .platform_data = &i2c1_data,
> + },
> +};
> +
> +static struct platform_device i2c2_device = {
> + .name = "pnx-i2c",
> + .id = 2,
> + .dev = {
> + .platform_data = &i2c2_data,
> + },
> +};
> +
> +static struct platform_device *devices[] __initdata = {
> + &i2c0_device,
> + &i2c1_device,
> + &i2c2_device,
> +};
> +
> +void __init pnx4008_register_i2c_devices(void)
> +{
> + platform_add_devices(devices, ARRAY_SIZE(devices));
> +}
All the rest look OK to me now, thanks for fixing all the issues I had
pointed you to. We're almost there, the most important now is to handle
the timeouts properly, and then I can push your driver upstream.
You can submit your final version either as a complete patch, or as an
incremental patch against this one, at your option.
Thanks,
--
Jean Delvare
More information about the i2c
mailing list