On Wed, Dec 03, 2008 at 09:58:10PM +0100, Guennadi Liakhovetski wrote:
> Hi Darius,
>
> it would be better, if you didn't drop my email from cc, thanks.
>
> On Wed, 3 Dec 2008, Darius wrote:
>
> > Guennadi Liakhovetski wrote:
> > > One more thing: you also want to do something like this
> > >
> > > i2c_imx->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
> >
> > some time ago, it was I2C_CLASS_ALL, but Jean told, that it will be removed
> > at
> > all :)
> >
> > >
> > > in i2c_imx_probe().
> > >
> > > If you want, I can send you my current version of your driver, so you can
> > > compare.
> >
> > yes, please send it.
>
> Below.
>
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
>
> >From 1d592e3a94b3e13537a680e3e0198c345248692d Mon Sep 17 00:00:00 2001
> From: Guennadi Liakhovetski <[EMAIL PROTECTED]>
> Date: Wed, 3 Dec 2008 17:02:37 +0100
> Subject: [PATCH] i2x-imx: fix client addresses, remove needless delays,
> support multiple busses
>
> With above bugs fixed now also tested to work on i.MX31. bus frequency is now
> configured over platform data, which allows different frequencies on different
> busses. Add an adapter.class assignment.
>
> Signed-off-by: Guennadi Liakhovetski <[EMAIL PROTECTED]>
> ---
> arch/arm/plat-mxc/include/mach/i2c.h | 8 ++
> drivers/i2c/busses/i2c-imx.c | 143
> ++++++++++++++++------------------
> 2 files changed, 76 insertions(+), 75 deletions(-)
>
> diff --git a/arch/arm/plat-mxc/include/mach/i2c.h
> b/arch/arm/plat-mxc/include/mach/i2c.h
> index 9e6a9d8..37d068b 100644
> --- a/arch/arm/plat-mxc/include/mach/i2c.h
> +++ b/arch/arm/plat-mxc/include/mach/i2c.h
> @@ -10,8 +10,16 @@
> #define __ASM_ARCH_I2C_H_
>
> struct imxi2c_platform_data {
> + u32 i2c_clk;
> int (*init)(struct device *dev);
> int (*exit)(struct device *dev);
> };
>
> +struct imxi2c_board_data {
> + struct imxi2c_platform_data data;
> + unsigned int id;
> +};
> +
> +extern int mxc_i2c_register_adapters(struct imxi2c_board_data *data, int n);
if there are only 1 or two busses, why not do
mx2_i2c_register_board(int i2c_channel_nr, struct
imx_i2c_platform_data *pd);
> #endif /* __ASM_ARCH_I2C_H_ */
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index fd2d16d..eeea7d7 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -23,12 +23,6 @@
> * Implementation of I2C Adapter/Algorithm Driver
> * for I2C Bus integrated in Freescale i.MX/MXC processors
> *
> - * module parameters:
> - * - clkfreq:
> - * Sets the desired clock rate
> - * The default value is 100000
> - * Max value is 400000
> - *
> * Derived from Motorola GSG China I2C example driver
> *
> * Copyright (C) 2005 Torsten Koschorrek <koschorrek at synertronixx.de
> @@ -64,14 +58,6 @@
> /* This will be the driver name the kernel reports */
> #define DRIVER_NAME "imx-i2c"
>
> -/* Default values of module parameters */
> -#define IMX_I2C_BIT_RATE 100000 /* 100kHz */
> -
> -/* Timeouts */
> -#define I2C_IMX_TIME_BUSY 2000 /* loop count */
> -#define I2C_IMX_TIME_ACK 2000 /* loop count */
> -#define I2C_IMX_TIME_TRX 5 /* seconds */
> -
> /* IMX I2C registers */
> #define IMX_I2C_IADR 0x00 /* i2c slave address */
> #define IMX_I2C_IFDR 0x04 /* i2c frequency divider */
> @@ -97,7 +83,6 @@
> /** Variables
> ******************************************************************
>
> *******************************************************************************/
>
> -static unsigned int clkfreq = IMX_I2C_BIT_RATE;
> static unsigned int disable_delay; /* Dummy delay */
>
> /*
> @@ -135,6 +120,21 @@ struct imx_i2c_struct {
> unsigned long i2csr;
> };
>
> +/*
> + * i.MX has 32-bit registers, i.MX31 16-bit, but only 8 bits are used. So far
> + * 8-bit access seems to work on both.
> + */
> +static inline unsigned int reg_read(const void __iomem *p)
> +{
> + return __raw_readb(p);
> +}
> +
> +static inline void reg_write(unsigned int value, void __iomem *p)
> +{
> + __raw_writeb(value, p);
> +}
readb and writeb() are the right verions to use here.
> +
> +
> /** Functions for IMX I2C adapter driver
> ***************************************
>
> *******************************************************************************/
>
> @@ -145,7 +145,7 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct
> *i2c_imx)
> dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
> /* wait for bus not busy */
> - while (readb(i2c_imx->base + IMX_I2C_I2SR) & I2SR_IBB) {
> + while (reg_read(i2c_imx->base + IMX_I2C_I2SR) & I2SR_IBB) {
> if (signal_pending(current)) {
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> I2C Interrupted\n", __func__);
> @@ -167,7 +167,7 @@ static int i2c_imx_trx_complete(struct imx_i2c_struct
> *i2c_imx)
> int result;
>
> result = wait_event_interruptible_timeout(i2c_imx->queue,
> - i2c_imx->i2csr & I2SR_IIF, I2C_IMX_TIME_TRX * HZ);
> + i2c_imx->i2csr & I2SR_IIF, i2c_imx->adapter.timeout);
>
> if (unlikely(result < 0)) {
> dev_dbg(&i2c_imx->adapter.dev, "<%s> result < 0\n", __func__);
> @@ -183,62 +183,50 @@ static int i2c_imx_trx_complete(struct imx_i2c_struct
> *i2c_imx)
>
> static int i2c_imx_acked(struct imx_i2c_struct *i2c_imx)
> {
> - unsigned long orig_jiffies = jiffies;
> -
> - /* Wait for ack */
> - while (readb(i2c_imx->base + IMX_I2C_I2SR) & I2SR_RXAK) {
> - if (signal_pending(current)) {
> - dev_dbg(&i2c_imx->adapter.dev,
> - "<%s> I2C Interrupted\n", __func__);
> - return -EINTR;
> - }
> - if (time_after(jiffies, orig_jiffies + HZ)) {
> - dev_dbg(&i2c_imx->adapter.dev,
> - "<%s> No ACK\n", __func__);
> - return -EIO;
> - }
> - schedule();
> + if (reg_read(i2c_imx->base + IMX_I2C_I2SR) & I2SR_RXAK) {
> + dev_dbg(&i2c_imx->adapter.dev, "<%s> No ACK\n", __func__);
> + return -EIO; /* No ACK */
> }
>
> dev_dbg(&i2c_imx->adapter.dev, "<%s> ACK received\n", __func__);
> - return 0; /* No ACK */
> + return 0;
> }
>
> static void i2c_imx_start(struct imx_i2c_struct *i2c_imx)
> {
> - unsigned int temp = 0;
> + unsigned int temp;
>
> dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
>
> /* Enable I2C controller */
> - writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> /* Start I2C transaction */
> - temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2CR);
> temp |= I2CR_MSTA;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2CR);
> temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2CR);
> }
>
> static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
> {
> - unsigned int temp = 0;
> + unsigned int temp;
>
> /* Stop I2C transaction */
> dev_dbg(&i2c_imx->adapter.dev, "<%s>\n", __func__);
> - temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2CR);
> temp &= ~I2CR_MSTA;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2CR);
> /* setup chip registers to defaults */
> - writeb(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> - writeb(0, i2c_imx->base + IMX_I2C_I2SR);
> + reg_write(I2CR_IEN, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(0, i2c_imx->base + IMX_I2C_I2SR);
> /*
> * This delay caused by an i.MXL hardware bug.
> * If no (or too short) delay, no "STOP" bit will be generated.
> */
> udelay(disable_delay);
> /* Disable I2C controller */
> - writeb(0, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(0, i2c_imx->base + IMX_I2C_I2CR);
> }
>
> static void __init i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx,
> @@ -259,7 +247,7 @@ static void __init i2c_imx_set_clk(struct imx_i2c_struct
> *i2c_imx,
> for (i = 0; i2c_clk_div[i][0] < div; i++);
>
> /* Write divider value to register */
> - writeb(i2c_clk_div[i][1], i2c_imx->base + IMX_I2C_IFDR);
> + reg_write(i2c_clk_div[i][1], i2c_imx->base + IMX_I2C_IFDR);
>
> /*
> * There dummy delay is calculated.
> @@ -284,12 +272,12 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
> struct imx_i2c_struct *i2c_imx = dev_id;
> unsigned int temp;
>
> - temp = readb(i2c_imx->base + IMX_I2C_I2SR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2SR);
> if (temp & I2SR_IIF) {
> /* save status register */
> i2c_imx->i2csr = temp;
> temp &= ~I2SR_IIF;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2SR);
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2SR);
> wake_up_interruptible(&i2c_imx->queue);
> }
>
> @@ -299,12 +287,15 @@ static irqreturn_t i2c_imx_isr(int irq, void *dev_id)
> static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg
> *msgs)
> {
> int i, result;
> + unsigned int addr_trans;
>
> dev_dbg(&i2c_imx->adapter.dev, "<%s> write slave address: addr=0x%x\n",
> __func__, msgs->addr);
>
> + addr_trans = msgs->addr << 1;
> +
> /* write slave address */
> - writeb(msgs->addr, i2c_imx->base + IMX_I2C_I2DR);
> + reg_write(addr_trans, i2c_imx->base + IMX_I2C_I2DR);
> result = i2c_imx_trx_complete(i2c_imx);
> if (result)
> return result;
> @@ -318,7 +309,7 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx,
> struct i2c_msg *msgs)
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> write byte: B%d=0x%X\n",
> __func__, i, msgs->buf[i]);
> - writeb(msgs->buf[i], i2c_imx->base + IMX_I2C_I2DR);
> + reg_write(msgs->buf[i], i2c_imx->base + IMX_I2C_I2DR);
> result = i2c_imx_trx_complete(i2c_imx);
> if (result)
> return result;
> @@ -333,13 +324,16 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx,
> struct i2c_msg *msgs)
> {
> int i, result;
> unsigned int temp;
> + unsigned int addr_trans;
>
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> write slave address: addr=0x%x\n",
> __func__, msgs->addr | 0x01);
>
> + addr_trans = (msgs->addr << 1) | 0x01;
> +
> /* write slave address */
> - writeb(msgs->addr | 0x01, i2c_imx->base + IMX_I2C_I2DR);
> + reg_write(addr_trans, i2c_imx->base + IMX_I2C_I2DR);
> result = i2c_imx_trx_complete(i2c_imx);
> if (result)
> return result;
> @@ -350,12 +344,12 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx,
> struct i2c_msg *msgs)
> dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
>
> /* setup bus to read data */
> - temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2CR);
> temp &= ~I2CR_MTX;
> if (msgs->len - 1)
> temp &= ~I2CR_TXAK;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> - readb(i2c_imx->base + IMX_I2C_I2DR); /* dummy read */
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2CR);
> + reg_read(i2c_imx->base + IMX_I2C_I2DR); /* dummy read */
>
> dev_dbg(&i2c_imx->adapter.dev, "<%s> read data\n", __func__);
>
> @@ -367,17 +361,17 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx,
> struct i2c_msg *msgs)
> if (i == (msgs->len - 1)) {
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> clear MSTA\n", __func__);
> - temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2CR);
> temp &= ~I2CR_MSTA;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2CR);
> } else if (i == (msgs->len - 2)) {
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> set TXAK\n", __func__);
> - temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2CR);
> temp |= I2CR_TXAK;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2CR);
> }
> - msgs->buf[i] = readb(i2c_imx->base + IMX_I2C_I2DR);
> + msgs->buf[i] = reg_read(i2c_imx->base + IMX_I2C_I2DR);
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> read byte: B%d=0x%X\n",
> __func__, i, msgs->buf[i]);
> @@ -407,21 +401,21 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
> if (i) {
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> repeated start\n", __func__);
> - temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2CR);
> temp |= I2CR_RSTA;
> - writeb(temp, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(temp, i2c_imx->base + IMX_I2C_I2CR);
> }
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> transfer message: %d\n", __func__, i);
> /* write/read data */
> #ifdef CONFIG_I2C_DEBUG_BUS
> - temp = readb(i2c_imx->base + IMX_I2C_I2CR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2CR);
> dev_dbg(&i2c_imx->adapter.dev, "<%s> CONTROL: IEN=%d, IIEN=%d, "
> "MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n", __func__,
> (temp & I2CR_IEN ? 1 : 0), (temp & I2CR_IIEN ? 1 : 0),
> (temp & I2CR_MSTA ? 1 : 0), (temp & I2CR_MTX ? 1 : 0),
> (temp & I2CR_TXAK ? 1 : 0), (temp & I2CR_RSTA ? 1 : 0));
> - temp = readb(i2c_imx->base + IMX_I2C_I2SR);
> + temp = reg_read(i2c_imx->base + IMX_I2C_I2SR);
> dev_dbg(&i2c_imx->adapter.dev,
> "<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, "
> "IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n", __func__,
> @@ -442,8 +436,8 @@ fail0:
>
> dev_dbg(&i2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
> (result < 0) ? "error" : "success msg",
> - (result < 0) ? result : num);
> - return (result < 0) ? result : num;
> + result < 0 ? result : num);
> + return result < 0 ? result : num;
> }
>
> static u32 i2c_imx_func(struct i2c_adapter *adapter)
> @@ -518,12 +512,14 @@ static int __init i2c_imx_probe(struct platform_device
> *pdev)
> i2c_imx->adapter.algo = &i2c_imx_algo;
> i2c_imx->adapter.dev.parent = &pdev->dev;
> i2c_imx->adapter.nr = pdev->id;
> + i2c_imx->adapter.class = I2C_CLASS_HWMON | I2C_CLASS_SPD;
> + i2c_imx->adapter.timeout = 1;
> i2c_imx->irq = irq;
> i2c_imx->base = base;
> i2c_imx->res = res;
>
> /* Get I2C clock */
> - i2c_imx->clk = clk_get(NULL, "i2c_clk");
> + i2c_imx->clk = clk_get(&pdev->dev, "i2c_clk");
> if (IS_ERR(i2c_imx->clk)) {
> ret = PTR_ERR(i2c_imx->clk);
> dev_err(&pdev->dev, "can't get I2C clock\n");
> @@ -546,11 +542,11 @@ static int __init i2c_imx_probe(struct platform_device
> *pdev)
> i2c_set_adapdata(&i2c_imx->adapter, i2c_imx);
>
> /* Set up clock divider */
> - i2c_imx_set_clk(i2c_imx, clkfreq);
> + i2c_imx_set_clk(i2c_imx, pdata->i2c_clk);
>
> /* Set up chip registers to defaults */
> - writeb(0, i2c_imx->base + IMX_I2C_I2CR);
> - writeb(0, i2c_imx->base + IMX_I2C_I2SR);
> + reg_write(0, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(0, i2c_imx->base + IMX_I2C_I2SR);
>
> /* Add I2C adapter */
> ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
> @@ -604,10 +600,10 @@ static int __exit i2c_imx_remove(struct platform_device
> *pdev)
> free_irq(i2c_imx->irq, i2c_imx);
>
> /* setup chip registers to defaults */
> - writeb(0, i2c_imx->base + IMX_I2C_IADR);
> - writeb(0, i2c_imx->base + IMX_I2C_IFDR);
> - writeb(0, i2c_imx->base + IMX_I2C_I2CR);
> - writeb(0, i2c_imx->base + IMX_I2C_I2SR);
> + reg_write(0, i2c_imx->base + IMX_I2C_IADR);
> + reg_write(0, i2c_imx->base + IMX_I2C_IFDR);
> + reg_write(0, i2c_imx->base + IMX_I2C_I2CR);
> + reg_write(0, i2c_imx->base + IMX_I2C_I2SR);
>
> /* Shut down hardware */
> if (pdata->exit)
> @@ -647,9 +643,6 @@ static void __exit i2c_adap_imx_exit(void)
> module_init(i2c_adap_imx_init);
> module_exit(i2c_adap_imx_exit);
>
> -module_param(clkfreq, uint, S_IRUGO);
> -MODULE_PARM_DESC(clkfreq, "desired IMX I2C Clock Rate in Hz");
> -
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Darius Augulis");
> MODULE_DESCRIPTION("I2C adapter driver for IMX I2C bus");
> --
> 1.5.4
>
> --
> 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
--
Ben ([EMAIL PROTECTED], http://www.fluff.org/)
'a smiley only costs 4 bytes'
--
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
