Fix for i2c-ibm_iic.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 12 Apr 2004 10:43:06 -0700
Eugene Surovegin <ebs at ebshome.net> wrote:

> On Mon, Apr 12, 2004 at 03:59:32PM +0400, Evgeniy Polyakov wrote:
> > Attached diff against 2.4 kernel fixes "new" (for i2c-ibm_iic) i2c
> > initalization path.
> > This i2c adapter can't understand commands with zero length so it
> > doesn't recognize SMBUS_QUICK command and none chip can be attached
> > to this. Patch fixes this cruft.
> > Tested on ppc32 with max663x chip attached.
> > 
> > Please review and commit.
> 
> Well, this doesn't look correct.
> Could you explain what are you trying to do? Also, why did you change
> debugging macros?

Because they don't even compile with debug level > 0.

> Chip doesn't support 0-length transactions, and your code trying to
> start 1-byte transaction without writing any data to the output FIFO.

It does support although not fair.

I have tested with this (as you say) "hack" max663x chip.

> IBM IIC is broken enough and I don't see any point in such hacks.

Agree. But it is not point not to fixing at least initialization at
least in such manner.

> There is another patch I made for MV guys a month ago 
> (http://kernel.ebshome.net/iic_0_len-2.4.diff, 
> http://kernel.ebshome.net/iic_0_len-2.6.diff). 
> 
> It restores _incorrect_ old driver behavior and most probably I'll
> send that version upstream (I'm still waiting for OK from them).

Driver with your patch works exactly as driver without it.
It always returns 0 with or without any ACK read from bus.
It is wrong.
I "found" 7 max6634 sensors on the bus.

> 
> > ===== drivers/i2c/i2c-ibm_iic.c 1.4 vs 1.5 =====
> > --- 1.4/drivers/i2c/i2c-ibm_iic.c	Fri Jun 27 15:19:03 2003
> > +++ 1.5/drivers/i2c/i2c-ibm_iic.c	Fri Apr  9 20:48:20 2004
> > @@ -61,14 +61,14 @@
> >  #define DBG_LEVEL 0
> >  
> >  #if DBG_LEVEL > 0
> > -#  define DBG(x...)	printk(KERN_DEBUG "ibm-iic" ##x)
> > +#  define DBG(f, x...)	printk("ibm-iic" f, ##x)
> >  #else
> > -#  define DBG(x...)	((void)0)
> > +#  define DBG(f, x...)	((void)0)
> >  #endif
> >  #if DBG_LEVEL > 1
> > -#  define DBG2(x...) 	DBG( ##x )
> > +#  define DBG2(f, x...) 	DBG( f, x )
> >  #else
> > -#  define DBG2(x...) 	((void)0)
> > +#  define DBG2(f, x...) 	((void)0)
> >  #endif
> >  #if DBG_LEVEL > 2
> >  static void dump_iic_regs(const char* header, struct
> >  ibm_iic_private* dev)
> > @@ -328,6 +328,24 @@
> >  	return ret;
> >  }
> >  
> > +static int iic_write_empty(struct ibm_iic_private* dev)
> > +{
> > +	volatile struct iic_regs *iic = dev->vaddr;
> > +	int ret = 0;
> > +
> > +	u8 cntl = (in_8(&iic->cntl) & CNTL_AMD) | CNTL_PT;
> > +	
> > +	DBG2("%d: xfer_bytes, %d, CNTL = 0x%02x\n", dev->idx, 0, cntl);
> > +	
> > +	/* Start transfer */
> > +	out_8(&iic->cntl, cntl);
> > +	
> > +	/* Wait for completion */
> > +	ret = iic_wait_for_tc(dev);
> > +
> > +	return ret > 0 ? 0 : ret;
> > +}
> > +
> >  /*
> >   * Low level master transfer routine
> >   */
> > @@ -338,6 +356,11 @@
> >  	char* buf = pm->buf;
> >  	int i, j, loops, ret = 0;
> >  	int len = pm->len;
> > +	
> > +	if (len == 0)
> > +	{
> > +		return iic_write_empty(dev);
> > +	}
> >  
> >  	u8 cntl = (in_8(&iic->cntl) & CNTL_AMD) | CNTL_PT;
> >  	if (pm->flags & I2C_M_RD)
> > @@ -445,11 +468,6 @@
> >  		return -EINVAL;
> >  	}		
> >  	for (i = 0; i < num; ++i){
> > -		if (unlikely(msgs[i].len <= 0)){
> > -			DBG("%d: invalid len %d in msg[%d]\n", dev->idx, 
> > -				msgs[i].len, i);
> > -			return -EINVAL;
> > -		}
> >  		if (unlikely(iic_address_neq(&msgs[0], &msgs[i]))){
> >  			DBG("%d: invalid addr in msg[%d]\n", dev->idx, i);
> >  			return -EINVAL;
> 
> 


	Evgeniy Polyakov ( s0mbre )

Only failure makes us experts. -- Theo de Raadt



[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux