Re: [PATCH] i2c : i2c-ibm-iic : use interrupts to perform the data transfer

[Jean-Jacques Hiblot <jjhiblot-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Hi Gregory,

On 20/11/2013 16:53, Gregory CLEMENT wrote:
> Hi Jean-Jacques,
>
> On 20/11/2013 16:08, jean-jacques hiblot wrote:
>> The current implementation uses the interrupt only to wakeup the process doing the data transfer.
>> While this is working, it introduces a sometimes indesirable blanks during the transfer.
>>
>> This patch implements the data transfer in the interrupt handler. It keeps the latency between individual bytes low and the jitter on the total transfer time is reduced.
>> Polling mode is still supported.
>
Polling mode is supported if no valid interrupt is defined in the device 
tree.

> I saw that you added a flag named use_polling, but I didn't see
> how a user can set it (nothing was added in the device tree). Could you
> explain what do you meant by "Polling mode is still supported" ?
>
>>
>> This was tested on a custom board built around a PPC460EX with several different I2C devices (including EEPROMs and smart batteries)
>
> Could you wrap the commit log at ~80 columns?
>
Ok

> I made only a few comments because your patch is hard to review.
> Could you split your patch in smaller and incremental part?
Ok

> However I made few comments inline
>
>>
>> Signed-off-by: jean-jacques hiblot <jjhiblot-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>   drivers/i2c/busses/i2c-ibm_iic.c | 411 +++++++++++++++++++++++----------------
>>   drivers/i2c/busses/i2c-ibm_iic.h |  21 +-
>>   2 files changed, 263 insertions(+), 169 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
>> index ff3caa0..77df857 100644
>> --- a/drivers/i2c/busses/i2c-ibm_iic.c
>> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
>> @@ -163,8 +163,8 @@ static void iic_dev_init(struct ibm_iic_private* dev)
>>   	/* Clear control register */
>>   	out_8(&iic->cntl, 0);
>>
>> -	/* Enable interrupts if possible */
>> -	iic_interrupt_mode(dev, dev->irq >= 0);
>> +	/* disable interrupts*/
>> +	iic_interrupt_mode(dev, 0);
>>
>>   	/* Set mode control */
>>   	out_8(&iic->mdcntl, MDCNTL_FMDB | MDCNTL_EINT | MDCNTL_EUBS
>> @@ -319,197 +319,218 @@ err:
>>   	goto out;
>>   }
>>
>> -/*
>> - * IIC interrupt handler
>> - */
>> -static irqreturn_t iic_handler(int irq, void *dev_id)
>> +static int iic_xfer_bytes(struct ibm_iic_private *dev)
>>   {
>> -	struct ibm_iic_private* dev = (struct ibm_iic_private*)dev_id;
>> -	volatile struct iic_regs __iomem *iic = dev->vaddr;
>> -
>> -	DBG2("%d: irq handler, STS = 0x%02x, EXTSTS = 0x%02x\n",
>> -	     dev->idx, in_8(&iic->sts), in_8(&iic->extsts));
>> -
>> -	/* Acknowledge IRQ and wakeup iic_wait_for_tc */
>> +	struct i2c_msg *pm = &(dev->msgs[dev->current_msg]);
>> +	struct iic_regs *iic = dev->vaddr;
>> +	u8 cntl = in_8(&iic->cntl) & CNTL_AMD;
>> +	int count;
>> +	u32 status;
>> +	u32 ext_status;
>> +
>> +	/* First check the status register */
>> +	status = in_8(&iic->sts);
>> +	ext_status = in_8(&iic->extsts);
>> +
>> +	/* and acknowledge the interrupt */
>> +	out_8(&iic->extsts, EXTSTS_IRQP | EXTSTS_IRQD | EXTSTS_LA |
>> +			    EXTSTS_ICT | EXTSTS_XFRA);
>>   	out_8(&iic->sts, STS_IRQA | STS_SCMP);
>> -	wake_up_interruptible(&dev->wq);
>>
>> -	return IRQ_HANDLED;
>> -}
>> +	if (dev->status == -ECANCELED) {
>> +		dev_dbg(dev->dev, "abort completed\n");
>> +		dev->transfer_complete = 1;
>> +		if (!dev->use_polling)
>> +			complete(&dev->iic_compl);
>> +		return dev->status;
>> +	}
>>
>> -/*
>> - * Get master transfer result and clear errors if any.
>> - * Returns the number of actually transferred bytes or error (<0)
>> - */
>> -static int iic_xfer_result(struct ibm_iic_private* dev)
>> -{
>> -	volatile struct iic_regs __iomem *iic = dev->vaddr;
>> +	if ((status & STS_ERR) ||
>> +	    (ext_status & (EXTSTS_LA | EXTSTS_ICT | EXTSTS_XFRA))) {
>> +		dev_dbg(dev->dev, "status 0x%x\n", status);
>> +		dev_dbg(dev->dev, "extended status 0x%x\n", ext_status);
>> +		if (status & STS_ERR)
>> +			dev_dbg(dev->dev, "Error detected\n");
> Is it really a debug message?  shouldn't it be an error message?
Indeed I'll make it a dev_err

>
>> +		if (ext_status & EXTSTS_LA)
>> +			dev_dbg(dev->dev, "Lost arbitration\n");
>
> Ditto
I don't know if this is really an error. Arbitration loss wan be very
common in a multi master environement. What about dev_warn ou dev_info

>
>> +		if (ext_status & EXTSTS_ICT)
>> +			dev_dbg(dev->dev, "Incomplete transfer\n");
>
> Ditto
>
>> +		if (ext_status & EXTSTS_XFRA)
>> +			dev_dbg(dev->dev, "Transfer aborted\n");
>
> Ditto
>
I'll use dev_err instead for those two.


>> +
>> +		dev->status = -EIO;
>> +		dev->transfer_complete = 1;
>> +		if (!dev->use_polling)
>> +			complete(&dev->iic_compl);
>> +	}
>>
>> -	if (unlikely(in_8(&iic->sts) & STS_ERR)){
>> -		DBG("%d: xfer error, EXTSTS = 0x%02x\n", dev->idx,
>> -			in_8(&iic->extsts));
>> +	if (dev->msgs == NULL) {
>> +		dev_dbg(dev->dev, "spurious !!!!!\n");
>> +		dev->status = -EINVAL;
>> +		return dev->status;
>> +	}
>>
>> -		/* Clear errors and possible pending IRQs */
>> -		out_8(&iic->extsts, EXTSTS_IRQP | EXTSTS_IRQD |
>> -			EXTSTS_LA | EXTSTS_ICT | EXTSTS_XFRA);
>> +	/* check if there's any data to read from the fifo */
>> +	if (pm->flags & I2C_M_RD) {
>> +		while (dev->current_byte_rx < dev->current_byte) {
>> +			u8 *buf = pm->buf + dev->current_byte_rx;
>> +			*buf = in_8(&iic->mdbuf);
>> +			dev->current_byte_rx++;
>> +			dev_dbg(dev->dev, "read 0x%x\n", *buf);
>> +		}
>> +	}
>> +	/* check if we must go with the next tranfer */
>> +	if (pm->len  == dev->current_byte) {
>> +		dev_dbg(dev->dev, "going to next transfer\n");
>> +		dev->current_byte = 0;
>> +		dev->current_byte_rx = 0;
>> +		dev->current_msg++;
>> +		if (dev->current_msg == dev->num_msgs) {
>> +			dev_dbg(dev->dev, "end of transfer\n");
>> +			dev->transfer_complete = 1;
>> +			if (!dev->use_polling)
>> +				complete(&dev->iic_compl);
>> +			return dev->status;
>> +		}
>> +		pm++;
>> +	}
>>
>> -		/* Flush master data buffer */
>> -		out_8(&iic->mdcntl, in_8(&iic->mdcntl) | MDCNTL_FMDB);
>> +	/* take care of the direction */
>> +	if (pm->flags & I2C_M_RD)
>> +		cntl |= CNTL_RW;
>>
>> -		/* Is bus free?
>> -		 * If error happened during combined xfer
>> -		 * IIC interface is usually stuck in some strange
>> -		 * state, the only way out - soft reset.
>> -		 */
>> -		if ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
>> -			DBG("%d: bus is stuck, resetting\n", dev->idx);
>> -			iic_dev_reset(dev);
>> +	/*  how much data are we going to transfer this time ?
>> +	 *  (up to 4 bytes at once)
>> +	 */
>> +	count = pm->len  - dev->current_byte;
>> +	count = (count > 4) ? 4 : count;
>> +	cntl |= (count - 1) << CNTL_TCT_SHIFT;
>> +
>> +	if ((pm->flags & I2C_M_RD) == 0) {
>> +		/* This is a write. we must fill the fifo with the data */
>> +		int i;
>> +		u8 *buf = pm->buf + dev->current_byte;
>> +
>> +		for (i = 0; i < count; i++) {
>> +			out_8(&iic->mdbuf, buf[i]);
>> +			dev_dbg(dev->dev, "write : 0x%x\n", buf[i]);
>>   		}
>> -		return -EREMOTEIO;
>>   	}
>> -	else
>> -		return in_8(&iic->xfrcnt) & XFRCNT_MTC_MASK;
>> -}
>>
>> -/*
>> - * Try to abort active transfer.
>> - */
>> -static void iic_abort_xfer(struct ibm_iic_private* dev)
>> -{
>> -	volatile struct iic_regs __iomem *iic = dev->vaddr;
>> -	unsigned long x;
>> +	/* will the current transfer complete with this chunk of data ? */
>> +	if (pm->len  > dev->current_byte + 4) {
>> +		/* we're not done with the current transfer.
>> +		 * Don't send a repeated start
>> +		 */
>> +		cntl |= CNTL_CHT;
>> +	}
>> +	/* This transfer will be complete with this chunk of data
>> +	 * BUT is this the last transfer of the list ?
>> +	 */
>> +	else if (dev->current_msg != (dev->num_msgs-1)) {
>> +		/* not the last tranfer */
>> +		cntl |= CNTL_BCL; /* Do not send a STOP condition */
>> +		/* check if the NEXT transfer needs a repeated start */
>> +		if (pm[1].flags & I2C_M_NOSTART)
>> +			cntl |= CNTL_CHT;
>> +	}
>>
>> -	DBG("%d: iic_abort_xfer\n", dev->idx);
>> +	if ((cntl & CNTL_BCL) == 0)
>> +		dev_dbg(dev->dev, "STOP required\n");
>>
>> -	out_8(&iic->cntl, CNTL_HMT);
>> +	if ((cntl & CNTL_CHT) == 0)
>> +		dev_dbg(dev->dev, "next transfer will begin with START\n");
>>
>> -	/*
>> -	 * Wait for the abort command to complete.
>> -	 * It's not worth to be optimized, just poll (timeout >= 1 tick)
>> -	 */
>> -	x = jiffies + 2;
>> -	while ((in_8(&iic->extsts) & EXTSTS_BCS_MASK) != EXTSTS_BCS_FREE){
>> -		if (time_after(jiffies, x)){
>> -			DBG("%d: abort timeout, resetting...\n", dev->idx);
>> -			iic_dev_reset(dev);
>> -			return;
>> -		}
>> -		schedule();
>> +	/* keep track of the amount of data transfered (RX and TX) */
>> +	dev->current_byte += count;
>> +
>> +	/* actually start the transfer of the current data chunk */
>> +	out_8(&iic->cntl, cntl | CNTL_PT);
>> +
>> +	/* The transfer must be aborted. */
>> +	if (dev->abort) {
>> +		dev_dbg(dev->dev, "aborting\n");
>> +		out_8(&iic->cntl, CNTL_HMT);
>> +		dev->status = -ECANCELED;
>>   	}
>>
>> -	/* Just to clear errors */
>> -	iic_xfer_result(dev);
>> +	return dev->status;
>>   }
>>
>>   /*
>> - * Wait for master transfer to complete.
>> - * It puts current process to sleep until we get interrupt or timeout expires.
>> - * Returns the number of transferred bytes or error (<0)
>> + * IIC interrupt handler
>>    */
>> -static int iic_wait_for_tc(struct ibm_iic_private* dev){
>> +static irqreturn_t iic_handler(int irq, void *dev_id)
>> +{
>> +	struct ibm_iic_private *dev = (struct ibm_iic_private *) dev_id;
>> +	iic_xfer_bytes(dev);
>
> Shouldn't you do something with the return value of this function?
Not really. We don't really care of the return value. except when
transmiting the first byte of the global transfer.

>
>> +	return IRQ_HANDLED;
>> +}
>>
>> +/*
>> + * Polling used when interrupt can't be used
>> + */
>> +static int poll_for_end_of_transfer(struct ibm_iic_private *dev, u32 timeout)
>> +{
>>   	volatile struct iic_regs __iomem *iic = dev->vaddr;
>> -	int ret = 0;
>> -
>> -	if (dev->irq >= 0){
>> -		/* Interrupt mode */
>> -		ret = wait_event_interruptible_timeout(dev->wq,
>> -			!(in_8(&iic->sts) & STS_PT), dev->adap.timeout);
>> -
>> -		if (unlikely(ret < 0))
>> -			DBG("%d: wait interrupted\n", dev->idx);
>> -		else if (unlikely(in_8(&iic->sts) & STS_PT)){
>> -			DBG("%d: wait timeout\n", dev->idx);
>> -			ret = -ETIMEDOUT;
>> -		}
>> -	}
>> -	else {
>> -		/* Polling mode */
>> -		unsigned long x = jiffies + dev->adap.timeout;
>> -
>> -		while (in_8(&iic->sts) & STS_PT){
>> -			if (unlikely(time_after(jiffies, x))){
>> -				DBG("%d: poll timeout\n", dev->idx);
>> -				ret = -ETIMEDOUT;
>> -				break;
>> -			}
>> -
>> -			if (unlikely(signal_pending(current))){
>> -				DBG("%d: poll interrupted\n", dev->idx);
>> -				ret = -ERESTARTSYS;
>> -				break;
>> -			}
>> +	u32 status;
>> +	unsigned long end = jiffies + timeout;
>> +
>> +	while ((dev->transfer_complete == 0) &&
>> +	   time_after(end, jiffies) &&
>> +	   !signal_pending(current)) {
>> +		status = in_8(&iic->sts);
>> +		/* check if the transfer is done or an error occured */
>> +		if ((status & (STS_ERR | STS_SCMP)) || !(status & STS_PT))
>> +			iic_xfer_bytes(dev);
>> +		/* The transfer is not complete,
>> +		 * let's wait a bit to give time to the controller to update
>> +		 * its registers.
>> +		 * calling schedule relaxes the CPU load and allows to know
>> +		 * if the process is being signaled (for abortion)
>> +		 */
>> +		if (dev->transfer_complete == 0)
>>   			schedule();
>> -		}
>>   	}
>>
>> -	if (unlikely(ret < 0))
>> -		iic_abort_xfer(dev);
>> -	else
>> -		ret = iic_xfer_result(dev);
>> +	if (signal_pending(current))
>> +		return -ERESTARTSYS;
>>
>> -	DBG2("%d: iic_wait_for_tc -> %d\n", dev->idx, ret);
>> +	if (dev->transfer_complete == 0)
>> +		return 0;
>>
>> -	return ret;
>> +	return 1;
>>   }
>>
>>   /*
>> - * Low level master transfer routine
>> + * Try to abort active transfer.
>>    */
>> -static int iic_xfer_bytes(struct ibm_iic_private* dev, struct i2c_msg* pm,
>> -			  int combined_xfer)
>> +static void iic_abort_xfer(struct ibm_iic_private *dev)
>>   {
>> -	volatile struct iic_regs __iomem *iic = dev->vaddr;
>> -	char* buf = pm->buf;
>> -	int i, j, loops, ret = 0;
>> -	int len = pm->len;
>> -
>> -	u8 cntl = (in_8(&iic->cntl) & CNTL_AMD) | CNTL_PT;
>> -	if (pm->flags & I2C_M_RD)
>> -		cntl |= CNTL_RW;
>> -
>> -	loops = (len + 3) / 4;
>> -	for (i = 0; i < loops; ++i, len -= 4){
>> -		int count = len > 4 ? 4 : len;
>> -		u8 cmd = cntl | ((count - 1) << CNTL_TCT_SHIFT);
>> -
>> -		if (!(cntl & CNTL_RW))
>> -			for (j = 0; j < count; ++j)
>> -				out_8((void __iomem *)&iic->mdbuf, *buf++);
>> -
>> -		if (i < loops - 1)
>> -			cmd |= CNTL_CHT;
>> -		else if (combined_xfer)
>> -			cmd |= CNTL_RPST;
>> -
>> -		DBG2("%d: xfer_bytes, %d, CNTL = 0x%02x\n", dev->idx, count, cmd);
>> -
>> -		/* Start transfer */
>> -		out_8(&iic->cntl, cmd);
>> -
>> -		/* Wait for completion */
>> -		ret = iic_wait_for_tc(dev);
>> -
>> -		if (unlikely(ret < 0))
>> -			break;
>> -		else if (unlikely(ret != count)){
>> -			DBG("%d: xfer_bytes, requested %d, transferred %d\n",
>> -				dev->idx, count, ret);
>> -
>> -			/* If it's not a last part of xfer, abort it */
>> -			if (combined_xfer || (i < loops - 1))
>> -    				iic_abort_xfer(dev);
>> -
>> -			ret = -EREMOTEIO;
>> -			break;
>> +	struct iic_regs *iic = dev->vaddr;
>> +	unsigned long end;
>> +
>> +	dev_dbg(dev->dev, "aborting transfer\n");
>> +	/* transfer should be aborted within 10ms */
>> +	end = jiffies + 10;
>> +	dev->abort = 1;
>> +
>> +	while (time_after(end, jiffies) && !dev->transfer_complete) {
>> +		u32 sts;
>> +		if (dev->use_polling) {
>> +			sts = in_8(&iic->sts);
>> +			/* check if the transfer is done or an error occured */
>> +			if ((sts & (STS_ERR | STS_SCMP)) || !(sts & STS_PT))
>> +				iic_xfer_bytes(dev);
>>   		}
>> -
>> -		if (cntl & CNTL_RW)
>> -			for (j = 0; j < count; ++j)
>> -				*buf++ = in_8((void __iomem *)&iic->mdbuf);
>> +		if (dev->transfer_complete == 0)
>> +			schedule();
>>   	}
>>
>> -	return ret > 0 ? 0 : ret;
>> +	if (!dev->transfer_complete) {
>> +		dev_err(dev->dev, "abort operation failed\n");
>> +		iic_dev_reset(dev);
>> +	}
>>   }
>>
>>   /*
>> @@ -607,19 +628,74 @@ static int iic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
>>   			return -EREMOTEIO;
>>   		}
>>   	}
>> -	else {
>> -		/* Flush master data buffer (just in case) */
>> -		out_8(&iic->mdcntl, in_8(&iic->mdcntl) | MDCNTL_FMDB);
>> +
>> +	dev->current_byte = dev->current_msg = dev->current_byte_rx = 0;
>> +	dev->msgs = msgs;
>> +	dev->num_msgs = num;
>> +	dev->transfer_complete = 0;
>> +	dev->status = 0;
>> +	dev->abort = 0;
>> +
>> +	/* clear the buffers */
>> +	out_8(&iic->mdcntl, MDCNTL_FMDB);
>> +	i = 100;
>> +	while (in_8(&iic->mdcntl) & MDCNTL_FMDB) {
>> +		if (i-- < 0) {
>> +			DBG("%d: iic_xfer, unable to flush\n", dev->idx);
>> +			return -EREMOTEIO;
>> +		}
>>   	}
>>
>> +	/* clear all interrupts masks */
>> +	out_8(&iic->intmsk, 0x0);
>> +	/* clear any status */
>> +	out_8(&iic->sts, STS_IRQA | STS_SCMP);
>> +	out_8(&iic->extsts, EXTSTS_IRQP | EXTSTS_IRQD | EXTSTS_LA |
>> +			    EXTSTS_ICT | EXTSTS_XFRA);
>> +
>>   	/* Load slave address */
>>   	iic_address(dev, &msgs[0]);
>>
>> -	/* Do real transfer */
>> -    	for (i = 0; i < num && !ret; ++i)
>> -		ret = iic_xfer_bytes(dev, &msgs[i], i < num - 1);
>> +	if (!dev->use_polling)
>> +		init_completion(&dev->iic_compl);
>> +
>> +	/* start the transfer */
>> +	ret = iic_xfer_bytes(dev);
>> +
>> +	if (ret == 0) {
>> +		if (dev->use_polling) {
>> +			ret = poll_for_end_of_transfer(dev, num * HZ);
>> +		} else {
>> +			/* enable the interrupts */
>> +			out_8(&iic->mdcntl, MDCNTL_EINT);
>> +			/*  unmask the interrupts */
>> +			out_8(&iic->intmsk,	INTRMSK_EIMTC | INTRMSK_EITA  |
>> +						INTRMSK_EIIC | INTRMSK_EIHE);
>> +			/*  wait for the transfer to complete */
>> +			ret = wait_for_completion_interruptible_timeout(
>> +				&dev->iic_compl, num * HZ);
>> +			/* we don't mask the interrupts here because we may
>> +			 * need them to abort the transfer gracefully
>> +			 */
>> +		}
>> +	}
>>
>> -	return ret < 0 ? ret : num;
>> +	if (ret == 0) {
>> +		dev_err(dev->dev, "transfer timed out\n");
>> +		ret = -ETIMEDOUT;
>> +	} else if (ret < 0) {
>> +		if (ret == -ERESTARTSYS)
>> +			dev_err(dev->dev, "transfer interrupted\n");
>> +		iic_abort_xfer(dev);
>> +	} else {
>> +		/* Transfer is complete */
>> +		ret = (dev->status) ? dev->status : num;
>> +	}
>> +
>> +	/* mask the interrupts */
>> +	out_8(&iic->intmsk, 0);
>> +
>> +	return ret;
>>   }
>>
>>   static u32 iic_func(struct i2c_adapter *adap)
>> @@ -668,6 +744,7 @@ static int iic_request_irq(struct platform_device *ofdev,
>>   	if (iic_force_poll)
>>   		return 0;
>>
>> +	dev->use_polling = 1;
>>   	irq = irq_of_parse_and_map(np, 0);
>>   	if (!irq) {
>>   		dev_err(&ofdev->dev, "irq_of_parse_and_map failed\n");
>> @@ -677,13 +754,14 @@ static int iic_request_irq(struct platform_device *ofdev,
>>   	/* Disable interrupts until we finish initialization, assumes
>>   	 *  level-sensitive IRQ setup...
>>   	 */
>> +	init_completion(&dev->iic_compl);
>>   	iic_interrupt_mode(dev, 0);
>>   	if (request_irq(irq, iic_handler, 0, "IBM IIC", dev)) {
>>   		dev_err(&ofdev->dev, "request_irq %d failed\n", irq);
>>   		/* Fallback to the polling mode */
>>   		return 0;
>>   	}
>> -
>> +	dev->use_polling = 0;
>>   	return irq;
>>   }
>>
>> @@ -705,6 +783,7 @@ static int iic_probe(struct platform_device *ofdev)
>>   	}
>>
>>   	platform_set_drvdata(ofdev, dev);
>> +	dev->dev = &ofdev->dev;
>
> Is it related to the topic of this patch?
I needed the pointer to the struct device for the dev_err, dev_dbg etc., 
so I stored it in the private structure. If it's better I can modify the 
driver to get it from adap->dev.parent when needed.

 >
>>
>>   	dev->vaddr = of_iomap(np, 0);
>>   	if (dev->vaddr == NULL) {
>> diff --git a/drivers/i2c/busses/i2c-ibm_iic.h b/drivers/i2c/busses/i2c-ibm_iic.h
>> index fdaa482..8fa2b6b 100644
>> --- a/drivers/i2c/busses/i2c-ibm_iic.h
>> +++ b/drivers/i2c/busses/i2c-ibm_iic.h
>> @@ -22,11 +22,14 @@
>>   #ifndef __I2C_IBM_IIC_H_
>>   #define __I2C_IBM_IIC_H_
>>
>> +#include <linux/device.h>
>>   #include <linux/i2c.h>
>>
>>   struct iic_regs {
>> -	u16 mdbuf;
>> -	u16 sbbuf;
>> +	u8 mdbuf;
>> +	u8 reserved1;
>> +	u8 sbbuf;
>> +	u8 reserved2;
>>   	u8 lmadr;
>>   	u8 hmadr;
>>   	u8 cntl;
>> @@ -44,12 +47,23 @@ struct iic_regs {
>>
>>   struct ibm_iic_private {
>>   	struct i2c_adapter adap;
>> -	volatile struct iic_regs __iomem *vaddr;
>> +	struct iic_regs *vaddr;
>>   	wait_queue_head_t wq;
>>   	int idx;
>>   	int irq;
>>   	int fast_mode;
>>   	u8  clckdiv;
>> +	struct i2c_msg *msgs;
>> +	int num_msgs;
>> +	int current_msg;
>> +	int current_byte;
>> +	int current_byte_rx;
>> +	int transfer_complete;
>> +	int use_polling;
>> +	int status;
>> +	int abort;
>> +	struct completion iic_compl;
>> +	struct device *dev;
>>   };
>>
>>   /* IICx_CNTL register */
>> @@ -58,6 +72,7 @@ struct ibm_iic_private {
>>   #define CNTL_TCT_MASK	0x30
>>   #define CNTL_TCT_SHIFT	4
>>   #define CNTL_RPST	0x08
>> +#define CNTL_BCL	CNTL_RPST
>>   #define CNTL_CHT	0x04
>>   #define CNTL_RW		0x02
>>   #define CNTL_PT		0x01
>>
>
>