Ben Dooks wrote:
On Sat, Mar 28, 2009 at 10:36:50PM +0100, Guennadi Liakhovetski wrote:
On Wed, 25 Mar 2009, Wolfram Sang wrote:

Because I hurry to submit this, because merge window is open.
Ok, I waited for this driver, I really did, and I really prefer having one driver for as many hardware (SoC) variants as possible instead of having different drivers, and I really hoped its new version will work out-of-the-box in my setup (pcm037 with a mt9t031 camera). Unfortunately, it didn't. I've spent more than a full working day trying to get it to work, but I didn't succeed so far. It works with the on-board rtc, but it doesn't work with the camera, connected over a flex cable.

Attached to this email is a version of the i2c driver for i.mx31 that I've been using with my setup for about half a year now. I adjusted it slightly to accept the same platform data, so, it is really a drop-in replacement for i2c-imx.c. Of course, you have to adjust or extend the Makefile. I actually added a new Kconfig entry for this driver, so I could easily compare them during the tests.

The source of "my" version does look more logical and more clean to me on quite a few occasions. So, maybe someone could give it a quick spin on other *mx* SoCs and see if we can use this one instead? You might have to replace {read,write}w with readb and writeb, so, it might be a good idea to abstract them into inlines or macros. Otherwise, I think, it might work for other CPUs straight away.

I just would like to avoid committing a driver and having to spend hours or days fixing it afterwards when a possibly more mature version exists.
I reply to Ben mail, because Guennadi sent code as attachment.
My driver has better debug possibilities and most of below comments fixed.
I suggest to find why do you get problems whit it on MX3 and fix it in my driver.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
/*
 * Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
 *
 * The code contained herein is licensed under the GNU General Public
 * License. You may obtain a copy of the GNU General Public License
 * Version 2 or later at the following locations:
 *
 * http://www.opensource.org/licenses/gpl-license.html
 * http://www.gnu.org/copyleft/gpl.html
 */

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/i2c.h>
#include <linux/clk.h>
#include <linux/err.h>

#include <asm/irq.h>
#include <asm/io.h>

#include <mach/i2c.h>
#include <mach/iomux-mx3.h>

/* Address offsets of the I2C registers */
#define MXC_IADR                0x00    /* Address Register */
#define MXC_IFDR                0x04    /* Freq div register */
#define MXC_I2CR                0x08    /* Control regsiter */
#define MXC_I2SR                0x0C    /* Status register */
#define MXC_I2DR                0x10    /* Data I/O register */

/* Bit definitions of I2CR */
#define MXC_I2CR_IEN            0x0080
#define MXC_I2CR_IIEN           0x0040
#define MXC_I2CR_MSTA           0x0020
#define MXC_I2CR_MTX            0x0010
#define MXC_I2CR_TXAK           0x0008
#define MXC_I2CR_RSTA           0x0004

/* Bit definitions of I2SR */
#define MXC_I2SR_ICF            0x0080
#define MXC_I2SR_IAAS           0x0040
#define MXC_I2SR_IBB            0x0020
#define MXC_I2SR_IAL            0x0010
#define MXC_I2SR_SRW            0x0004
#define MXC_I2SR_IIF            0x0002
#define MXC_I2SR_RXAK           0x0001

#define MXC_ADAPTER_NAME        "MXC I2C Adapter"

/*
 * In case the MXC device has multiple I2C modules, this structure is used to
 * store information specific to each I2C module.
 */
struct mxc_i2c_device {
        struct i2c_adapter adap;
        wait_queue_head_t wq;           /* Transfer completion wait queue */
        void __iomem *membase;
        int irq;
        unsigned int clkdiv;            /* Default clock divider */
        struct clk *clk;
        bool low_power;                 /* Current power state */
        bool transfer_done;
        bool tx_success;                /* ACK received */
        struct resource *res;
};

struct clk_div_table {
        int reg_value;
        int div;
};

static const struct clk_div_table i2c_clk_table[] = {
        {0x20, 22}, {0x21, 24}, {0x22, 26}, {0x23, 28},
        {0, 30}, {1, 32}, {0x24, 32}, {2, 36},
        {0x25, 36}, {0x26, 40}, {3, 42}, {0x27, 44},
        {4, 48}, {0x28, 48}, {5, 52}, {0x29, 56},
        {6, 60}, {0x2A, 64}, {7, 72}, {0x2B, 72},
        {8, 80}, {0x2C, 80}, {9, 88}, {0x2D, 96},
        {0xA, 104}, {0x2E, 112}, {0xB, 128}, {0x2F, 128},
        {0xC, 144}, {0xD, 160}, {0x30, 160}, {0xE, 192},
        {0x31, 192}, {0x32, 224}, {0xF, 240}, {0x33, 256},
        {0x10, 288}, {0x11, 320}, {0x34, 320}, {0x12, 384},
        {0x35, 384}, {0x36, 448}, {0x13, 480}, {0x37, 512},
        {0x14, 576}, {0x15, 640}, {0x38, 640}, {0x16, 768},
        {0x39, 768}, {0x3A, 896}, {0x17, 960}, {0x3B, 1024},
        {0x18, 1152}, {0x19, 1280}, {0x3C, 1280}, {0x1A, 1536},
        {0x3D, 1536}, {0x3E, 1792}, {0x1B, 1920}, {0x3F, 2048},
        {0x1C, 2304}, {0x1D, 2560}, {0x1E, 3072}, {0x1F, 3840},
        {0, 0}
};
there you have lot of duplicated values. My driver has more clearly array.
/**
 * Transmit a \b STOP signal to the slave device.
 *
 * @param   dev   the mxc i2c structure used to get to the right i2c device
 */
static void mxc_i2c_stop(struct mxc_i2c_device * dev)
{
        unsigned int cr;
        int retry = 200;
what means this magic number?


        cr = readw(dev->membase + MXC_I2CR);
        cr &= ~(MXC_I2CR_MSTA | MXC_I2CR_MTX);
        writew(cr, dev->membase + MXC_I2CR);

no reason to use writew/readw, because all bits in I2C registers are lower 8 bits.

        /*
         * Make sure STOP meets setup requirement.
         */
        for (;;) {
                unsigned int sr = readw(dev->membase + MXC_I2SR);
                if ((sr & MXC_I2SR_IBB) == 0)

you test if I2C is busy in many loacations. It is worth to have separate function like "i2c_is_busy".


                        break;
                if (retry-- <= 0) {
                        printk(KERN_DEBUG "Bus busy\n");
                        break;
                }
                udelay(3);

I got comments form ML few timer not to use udelay() and let other processes to run.
My driver does this more efficient.

        }
}

/**
 * Wait for the transmission of the data byte to complete. This function waits
 * till we get a signal from the interrupt service routine indicating completion
 * of the address cycle or we time out.
 *
 * @param   dev         the mxc i2c structure used to get to the right i2c 
device
 * @param   trans_flag  transfer flag
 *
 *
 * @return  The function returns 0 on success or -1 if an ack was not received
 */

static int mxc_i2c_wait_for_tc(struct mxc_i2c_device *dev, int trans_flag)
{
        int retry = 16;

again strange constant.

        while (retry-- && !dev->transfer_done)
                wait_event_interruptible_timeout(dev->wq,
                                                 dev->transfer_done,
                                                 dev->adap.timeout);

        dev->transfer_done = false;

        if (retry <= 0) {
                /* Unable to send data */
                dev_warn(&dev->adap.dev, "Data not transmitted\n");
                return -ETIMEDOUT;
        }

        if (!dev->tx_success) {
                /* An ACK was not received for transmitted byte */
                dev_dbg(&dev->adap.dev, "ACK not received \n");
                return -EREMOTEIO;
        }

        return 0;
}

/**
 * Transmit a \b START signal to the slave device.
 *
 * @param   dev   the mxc i2c structure used to get to the right i2c device
 * @param   *msg  pointer to a message structure that contains the slave
 *                address
 */
static void mxc_i2c_start(struct mxc_i2c_device *dev, struct i2c_msg *msg)
{
        unsigned int cr, sr;
        unsigned int addr_trans;
        int retry = 16;

        /*
         * Set the slave address and the requested transfer mode
         * in the data register
         */
        addr_trans = msg->addr << 1;
        if (msg->flags & I2C_M_RD)
                addr_trans |= 0x01;

        dev_dbg(&dev->adap.dev, "%s: start %x\n", __func__, msg->addr);

        /* Set the Master bit */
        cr = readw(dev->membase + MXC_I2CR);
        cr |= MXC_I2CR_MSTA;
        writew(cr, dev->membase + MXC_I2CR);

        /* Wait till the Bus Busy bit is set */
        sr = readw(dev->membase + MXC_I2SR);
        while (retry-- && (!(sr & MXC_I2SR_IBB))) {
                udelay(3);
                sr = readw(dev->membase + MXC_I2SR);
        }
        if (retry <= 0)
                dev_warn(&dev->adap.dev, "Could not grab Bus ownership\n");

        /* Set the Transmit bit */
        cr = readw(dev->membase + MXC_I2CR);
        cr |= MXC_I2CR_MTX;
        writew(cr, dev->membase + MXC_I2CR);

        writew(addr_trans, dev->membase + MXC_I2DR);
}

/**
 * Transmit a \b REPEAT START to the slave device
 *
 * @param   dev   the mxc i2c structure used to get to the right i2c device
 * @param   *msg  pointer to a message structure that contains the slave
 *                address
 */
static void mxc_i2c_repstart(struct mxc_i2c_device *dev, struct i2c_msg *msg)
{
        unsigned int cr;
        unsigned int addr_trans;

        /*
         * Set the slave address and the requested transfer mode
         * in the data register
         */
        addr_trans = msg->addr << 1;
        if (msg->flags & I2C_M_RD)
                addr_trans |= 0x01;

        dev_dbg(&dev->adap.dev, "%s: repeat start %x\n", __func__, msg->addr);

        cr = readw(dev->membase + MXC_I2CR);
        cr |= MXC_I2CR_RSTA;
        writew(cr, dev->membase + MXC_I2CR);
        udelay(3);
        writew(addr_trans, dev->membase + MXC_I2DR);
}

/**
 * Read the received data. The function waits till data is available or times
 * out. Generates a stop signal if this is the last message to be received.
 * Sends an ack for all the bytes received except the last byte.
 *
 * @param  dev       the mxc i2c structure used to get to the right i2c device
 * @param  *msg      pointer to a message structure that contains the slave
 *                   address and a pointer to the receive buffer
 * @param  last      indicates that this is the last message to be received
 * @param  addr_comp flag indicates that we just finished the address cycle
 *
 * @return  The function returns the number of bytes read or -1 on time out.
 */
static int mxc_i2c_readbytes(struct mxc_i2c_device *dev, struct i2c_msg *msg,
                             int last, int addr_comp)
{
        int i;
        char *buf = msg->buf;
        int len = msg->len;
        unsigned int cr;

        dev_dbg(&dev->adap.dev, "%s: last %d, addr_comp %d\n",
                __func__, last, addr_comp);

        cr = readw(dev->membase + MXC_I2CR);
        /*
         * Clear MTX to switch to receive mode.
         */
        cr &= ~MXC_I2CR_MTX;
        /*
         * Clear the TXAK bit to gen an ack when receiving only one byte.
         */
        if (len == 1)
                cr |= MXC_I2CR_TXAK;
        else
                cr &= ~MXC_I2CR_TXAK;

        writew(cr, dev->membase + MXC_I2CR);
        /*
         * Dummy read only at the end of an address cycle
         */
        if (addr_comp > 0)
                readw(dev->membase + MXC_I2DR);

        for (i = 0; i < len; i++) {
                int ret;
                /*
                 * Wait for data transmission to complete
                 */
                ret = mxc_i2c_wait_for_tc(dev, msg->flags);
                if (ret < 0) {
                        dev_err(&dev->adap.dev, "%s: rx #%d of %d failed!\n",
                                __func__, i, len);
                        mxc_i2c_stop(dev);
                        return ret;
                }
                /*
                 * Do not generate an ACK for the last byte
                 */
                if (i == len - 2) {
                        cr = readw(dev->membase + MXC_I2CR);
                        cr |= MXC_I2CR_TXAK;
                        writew(cr, dev->membase + MXC_I2CR);
                } else if (i == len - 1) {
                        if (last)
                                mxc_i2c_stop(dev);
                }
                /* Read the data */
                *buf++ = readw(dev->membase + MXC_I2DR);
        }

        return i;
}

/**
 * Write the data to the data register. Generates a stop signal if this is
 * the last message to be sent or if no ack was received for the data sent.
 *
 * @param   dev   the mxc i2c structure used to get to the right i2c device
 * @param   *msg  pointer to a message structure that contains the slave
 *                address and data to be sent
 * @param   last  indicates that this is the last message to be received
 *
 * @return  The function returns the number of bytes written or -1 on time out
 *          or if no ack was received for the data that was sent.
 */
static int mxc_i2c_writebytes(struct mxc_i2c_device *dev, struct i2c_msg *msg,
                              int last)
{
        int i;
        char *buf = msg->buf;
        int len = msg->len;
        unsigned int cr;

        dev_dbg(&dev->adap.dev, "%s: last %d\n", __func__, last);

        cr = readw(dev->membase + MXC_I2CR);
        /* Set MTX to switch to transmit mode */
        writew(cr | MXC_I2CR_MTX, dev->membase + MXC_I2CR);

        for (i = 0; i < len; i++) {
                int ret;
                /*
                 * Write the data
                 */
                writew(*buf++, dev->membase + MXC_I2DR);
                ret = mxc_i2c_wait_for_tc(dev, msg->flags);
                if (ret < 0) {
                        dev_err(&dev->adap.dev, "%s: tx #%d of %d timed out!\n",
                                __func__, i, len);
                        mxc_i2c_stop(dev);
                        return ret;
                }
        }
        if (last > 0)
                mxc_i2c_stop(dev);

        return i;
}

/**
 * Function enables the I2C module and initializes the registers.
 *
 * @param   dev   the mxc i2c structure used to get to the right i2c device
 * @param   trans_flag  transfer flag
 */
static void mxc_i2c_module_en(struct mxc_i2c_device *dev, int trans_flag)
{
        clk_enable(dev->clk);
        /* Set the frequency divider */
        writew(dev->clkdiv, dev->membase + MXC_IFDR);
        /* Clear the status register */
        writew(0x0, dev->membase + MXC_I2SR);
        /* Enable I2C and its interrupts */
        writew(MXC_I2CR_IEN, dev->membase + MXC_I2CR);
        writew(MXC_I2CR_IEN | MXC_I2CR_IIEN, dev->membase + MXC_I2CR);
}

/**
 * Disables the I2C module.
 *
 * @param   dev   the mxc i2c structure used to get to the right i2c device
 */
static void mxc_i2c_module_dis(struct mxc_i2c_device * dev)
{
        writew(0x0, dev->membase + MXC_I2CR);
        clk_disable(dev->clk);
}

/**
 * The function is registered in the adapter structure. It is called when an MXC
 * driver wishes to transfer data to a device connected to the I2C device.
 *
 * @param   adap   adapter structure for the MXC i2c device
 * @param   msgs[] array of messages to be transferred to the device
 * @param   num    number of messages to be transferred to the device
 *
 * @return  The function returns the number of messages transferred,
 *          \b -EREMOTEIO on I2C failure and a 0 if the num argument is
 *          less than 0.
 */
static int mxc_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
{
        struct mxc_i2c_device *dev = i2c_get_adapdata(adap);
        int i, ret = 0, addr_comp = 0;
        unsigned int sr;

        if (dev->low_power) {
                dev_warn(&dev->adap.dev, "I2C Device in low power mode\n");
                return -EREMOTEIO;
        }

        if (num < 1)
                return 0;

        dev_dbg(&dev->adap.dev, "%s: xfer %d msgs\n", __func__, num);

        mxc_i2c_module_en(dev, msgs[0].flags);
        sr = readw(dev->membase + MXC_I2SR);
        /*
         * Check bus state
         */
        if (sr & MXC_I2SR_IBB) {
                mxc_i2c_module_dis(dev);
                printk(KERN_DEBUG "Bus busy\n");
                return -EREMOTEIO;
        }

        dev->transfer_done = false;
        dev->tx_success = false;
        for (i = 0; i < num && ret >= 0; i++) {
                addr_comp = 0;
                /*
                 * Send the slave address and transfer direction in the
                 * address cycle
                 */
                if (i == 0) {

this long if () else () statement could be replaced with very simple, like this one from my driver:

if (i) {
dev_dbg(&i2c_imx->adapter.dev, "<%s> repeated start\n", __func__);
           temp = readb(i2c_imx->base + IMX_I2C_I2CR);
           temp |= I2CR_RSTA;
           writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
}

                        /*
                         * Send a start or repeat start signal
                         */
                        mxc_i2c_start(dev, &msgs[0]);

you don't send there repeat start signal (wrong comment).

                        /* Wait for the address cycle to complete */
                        if (mxc_i2c_wait_for_tc(dev, msgs[0].flags)) {
                                mxc_i2c_stop(dev);
                                mxc_i2c_module_dis(dev);
                                dev_err(&dev->adap.dev,
                                        "%s: Address failed!\n", __func__);
                                return -EREMOTEIO;
                        }
                        addr_comp = 1;
                } else {
                        /*
                         * Generate repeat start only if required i.e. the
                         * address changed or the transfer direction changed
                         */
                        if ((msgs[i].addr != msgs[i - 1].addr) ||
                            ((msgs[i].flags & I2C_M_RD) !=
                             (msgs[i - 1].flags & I2C_M_RD))) {
                                mxc_i2c_repstart(dev, &msgs[i]);
                                /* Wait for the address cycle to complete */
                                if (mxc_i2c_wait_for_tc(dev, msgs[i].flags)) {
                                        mxc_i2c_stop(dev);
                                        mxc_i2c_module_dis(dev);
                                        dev_err(&dev->adap.dev,
                                                "%s: Address repeat failed!\n",
                                                __func__);
                                        return -EREMOTEIO;
                                }
                                addr_comp = 1;
                        }
                }

                /* Transfer the data */
                if (msgs[i].flags & I2C_M_RD) {
                        /* Read the data */
                        ret = mxc_i2c_readbytes(dev, &msgs[i], i + 1 == num,
                                                addr_comp);
                        if (ret < 0) {
                                dev_err(&dev->adap.dev, "mxc_i2c_readbytes: 
fail.\n");
                                break;
                        }
                } else {
                        /* Write the data */
                        ret = mxc_i2c_writebytes(dev, &msgs[i], i + 1 == num);
                        if (ret < 0) {
                                dev_err(&dev->adap.dev, "mxc_i2c_writebytes: 
fail.\n");
                                break;
                        }
                }
        }
My driver has a delay before disabling I2C module. I added this, because no STOP signal is generated otherwise. Your driver works on i.MXL without this delay. So there could be one of possible bugs in my driver.

        mxc_i2c_module_dis(dev);

        dev_dbg(&dev->adap.dev, "%s: %d msgs success\n", __func__, i);

        /*
         * Decrease by 1 as we do not want Start message to be included in
         * the count
         */
        return i;
}

/**
 * Returns the i2c functionality supported by this driver.
 *
 * @param   adap adapter structure for this i2c device
 *
 * @return Returns the functionality that is supported.
 */
static u32 mxc_i2c_func(struct i2c_adapter *adap)
{
        return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
}

static struct i2c_algorithm mxc_i2c_algorithm = {
        .master_xfer = mxc_i2c_xfer,
        .functionality = mxc_i2c_func
};

/*
 * Interrupt Service Routine. It signals to the process about the data transfer
 * completion. Also sets a flag if bus arbitration is lost.
 */
static irqreturn_t mxc_i2c_handler(int irq, void *dev_id)
{
        struct mxc_i2c_device *dev = dev_id;
        unsigned int sr, cr;

        sr = readw(dev->membase + MXC_I2SR);
        cr = readw(dev->membase + MXC_I2CR);

        /*
         * Clear the interrupt bit
         */
        writew(0x0, dev->membase + MXC_I2SR);

there you clear all status register, not IFF bit.

        if (sr & MXC_I2SR_IAL) {
                printk(KERN_DEBUG "Bus Arbitration lost\n");
        } else {
                /* Interrupt due byte transfer completion */

probably you should check if IFF bit is set to return IRQ_HANDLED.

                dev->tx_success = true;
                /* Check if RXAK is received in Transmit mode */
                if ((cr & MXC_I2CR_MTX) && (sr & MXC_I2SR_RXAK))
                        dev->tx_success = false;

                dev->transfer_done = true;
                wake_up_interruptible(&dev->wq);
        }

        return IRQ_HANDLED;
}

static int mxci2c_suspend(struct platform_device *pdev, pm_message_t state)
{
        struct mxc_i2c_device *mxcdev = platform_get_drvdata(pdev);
        struct imxi2c_platform_data *i2c_plat_data = pdev->dev.platform_data;
        unsigned int sr;

        if (mxcdev == NULL)
                return -ENODEV;

        /* Prevent further calls to be processed */
        mxcdev->low_power = true;
        /* Wait till we finish the current transfer */
        sr = readw(mxcdev->membase + MXC_I2SR);
        while (sr & MXC_I2SR_IBB) {
                msleep(10);
                sr = readw(mxcdev->membase + MXC_I2SR);
        }

        if (i2c_plat_data->exit)
                i2c_plat_data->exit(&pdev->dev);

        return 0;
}

static int mxci2c_resume(struct platform_device *pdev)
{
        struct mxc_i2c_device *mxcdev = platform_get_drvdata(pdev);
        struct imxi2c_platform_data *i2c_plat_data = pdev->dev.platform_data;

        if (mxcdev == NULL)
                return -ENODEV;

        mxcdev->low_power = false;
        if (i2c_plat_data->init)
                i2c_plat_data->init(&pdev->dev);

        return 0;
}

static int __init mxci2c_probe(struct platform_device *pdev)
{
        struct mxc_i2c_device *mxc_i2c;
        struct imxi2c_platform_data *i2c_plat_data = pdev->dev.platform_data;
        struct resource *res;
        int id = pdev->id;
        u32 clk_freq;
        int ret;
        int i;

        mxc_i2c = kzalloc(sizeof(struct mxc_i2c_device), GFP_KERNEL);
        if (!mxc_i2c)
                return -ENOMEM;

        res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
        if (res == NULL) {
                ret = -ENODEV;
                goto egetres;
        }

        if (!request_mem_region(res->start, resource_size(res), res->name)) {
                ret = -ENOMEM;
                goto ereqmemr;
        }

        mxc_i2c->membase = ioremap(res->start, resource_size(res));
        if (!mxc_i2c->membase) {
                ret = -ENOMEM;
                goto eiomap;
        }

        mxc_i2c->res = res;

        /*
         * Request the I2C interrupt
         */
        mxc_i2c->irq = platform_get_irq(pdev, 0);
        if (mxc_i2c->irq < 0) {
                ret = mxc_i2c->irq;
                goto epgirq;
        }

        ret = request_irq(mxc_i2c->irq, mxc_i2c_handler,
                          0, pdev->name, mxc_i2c);
        if (ret < 0)
                goto ereqirq;

        init_waitqueue_head(&mxc_i2c->wq);

        mxc_i2c->low_power   = false;
        mxc_i2c->transfer_done       = false;
        mxc_i2c->tx_success  = false;

        if (i2c_plat_data->init)
                i2c_plat_data->init(&pdev->dev);

        mxc_i2c->clk = clk_get(&pdev->dev, "i2c_clk");
        if (IS_ERR(mxc_i2c->clk)) {
                ret = PTR_ERR(mxc_i2c->clk);
                dev_err(&pdev->dev, "can't get I2C clock\n");
                goto eclkget;
        }
        clk_freq = clk_get_rate(mxc_i2c->clk);

        mxc_i2c->clkdiv = -1;
        if (i2c_plat_data->bitrate) {
                /* Calculate divider and round up any fractional part */
                int div = (clk_freq + i2c_plat_data->bitrate - 1) /
                        i2c_plat_data->bitrate;
                for (i = 0; i2c_clk_table[i].div != 0; i++) {
                        if (i2c_clk_table[i].div >= div) {
                                mxc_i2c->clkdiv = i2c_clk_table[i].reg_value;
                                break;
                        }
                }
        }
        if (mxc_i2c->clkdiv == -1) {
                i = ARRAY_SIZE(i2c_clk_table) - 2;
                /* Use max divider */
                mxc_i2c->clkdiv = i2c_clk_table[i].reg_value;
        }

my drivers does this in separate function in more efficient and readable way.

        dev_dbg(&pdev->dev, "i2c speed is %d/%d = %d bps, reg val = 0x%02X\n",
                clk_freq, i2c_clk_table[i].div, clk_freq / i2c_clk_table[i].div,
                mxc_i2c->clkdiv);

        /*
         * Set the adapter information
         */
        strcpy(mxc_i2c->adap.name, MXC_ADAPTER_NAME);
        mxc_i2c->adap.nr     = id;
        mxc_i2c->adap.algo   = &mxc_i2c_algorithm;
        mxc_i2c->adap.timeout        = 1;
        mxc_i2c->adap.dev.parent= &pdev->dev;
        mxc_i2c->adap.class  = I2C_CLASS_HWMON | I2C_CLASS_SPD;

I think adap.class should be removed at all.

        platform_set_drvdata(pdev, mxc_i2c);
        i2c_set_adapdata(&mxc_i2c->adap, mxc_i2c);
        if ((ret = i2c_add_numbered_adapter(&mxc_i2c->adap)) < 0)
                goto eaddadap;

        return 0;

eaddadap:
        platform_set_drvdata(pdev, NULL);
        clk_put(mxc_i2c->clk);
eclkget:
        if (i2c_plat_data->exit)
                i2c_plat_data->exit(&pdev->dev);
        free_irq(mxc_i2c->irq, mxc_i2c);
ereqirq:
epgirq:
        iounmap(mxc_i2c->membase);
eiomap:
        release_mem_region(res->start, resource_size(res));
ereqmemr:
egetres:
        kfree(mxc_i2c);
        dev_err(&pdev->dev, "failed to probe i2c adapter\n");
        return ret;
}

static int __exit mxci2c_remove(struct platform_device *pdev)
{
        struct mxc_i2c_device *mxc_i2c = platform_get_drvdata(pdev);
        struct imxi2c_platform_data *i2c_plat_data = pdev->dev.platform_data;

        free_irq(mxc_i2c->irq, mxc_i2c);
        i2c_del_adapter(&mxc_i2c->adap);
        if (i2c_plat_data->exit)
                i2c_plat_data->exit(&pdev->dev);
        clk_put(mxc_i2c->clk);
        platform_set_drvdata(pdev, NULL);
        iounmap(mxc_i2c->membase);
        release_mem_region(mxc_i2c->res->start, resource_size(mxc_i2c->res));
        kfree(mxc_i2c);
        return 0;
}

static struct platform_driver mxci2c_driver = {
        .driver = {
                   .name = "imx-i2c",
                   .owner = THIS_MODULE,
        },
        .remove = __exit_p(mxci2c_remove),
        .suspend = mxci2c_suspend,
        .resume = mxci2c_resume,
};

static int __init mxc_i2c_init(void)
{
        return platform_driver_probe(&mxci2c_driver, mxci2c_probe);
}

static void __exit mxc_i2c_exit(void)
{
        platform_driver_unregister(&mxci2c_driver);
}

subsys_initcall(mxc_i2c_init);
module_exit(mxc_i2c_exit);

MODULE_AUTHOR("Freescale Semiconductor, Inc.");
MODULE_DESCRIPTION("MXC I2C driver");
MODULE_LICENSE("GPL");



--
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