On Mon, Apr 12, 2004 at 10:41:43PM +0400, Evgeniy Polyakov wrote: > > 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. Huh? I don't have any problems here (gcc 2.95.3). > > > 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. No, it doesn't. Please read any 4xx IBM manual. Your code starts 1-byte _write_ transaction with whatever crap was in output FIFO. You clear TCT field in IICx_CNTL, and it means _1_byte_ will be transfered _after_ i2c address. This is not correct and this is not safe. > I have tested with this (as you say) "hack" max663x chip. So what? You are lucky and it "works" with you particular chip, it doesn't mean it'll work with another. I have a board here where even 1-byte _read_ access locks the bus completely (device is write-only clock driver). > > 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. What _initialization_ are you talking about? I'm using this driver on several different custom boards in production systems. > > 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. No, with my patch driver will not initiate any _unsafe_ operation on the bus like in your case. > It always returns 0 with or without any ACK read from bus. > It is wrong. Yes, but the _only_ correct way I see of implementing 0-length transactions is bit-banging, for the time being I choose to just implement the _old_ _driver_ _behavior_ until somebody shows me that there is a problem with this approach. IMHO all this _auto_ detection stuff is one big hack, I don't see _any_ provisions for this in i2c specs. Eugene.
