todo

I2C总线LINUX驱动程序

// and ensures the clock low time (together with the udelay at the
bottom of the loop)
// It could be changed to udelay(adap->udelay - 1)
		udelay(adap->udelay);
		DEBPROTO(printk(KERN_DEBUG "%d",sb!=0));
// The following call to sclhi() contains a call to delay(adap->udelay)
// which ensures the clock high time
		if (sclhi(adap)<0) { /* timed out */
			sdahi(adap); /* we don't want to block the net */
			DEB2(printk(KERN_DEBUG " i2c_outb: 0x%02x, timeout at bi			
			return -ETIMEDOUT;
		};
		/* do arbitration here: 
		 * if ( sb && ! getsda(adap) ) -> ouch! Get out of here.
		 */
		setscl(adap, 0 );
// The following udelay ensures hold time after the f.e. of the clock,
// and ensures the clock low time (together with the udelay at the top
of the loop)
// It could be changed to udelay(1) and still meet the 300 ns hold time
// (ref. I2C bus spec version 2.1, table 5, note 2)
		udelay(adap->udelay);
	}

=======================================================================
KM:

Unfortunately sclhi() does not ensure clock high time the right way.
The delay should be after we read clock as high.

Usually this means we get no acknowledge from the client, since it has
missed this one clock cycle. If we miss a bit during the address bits,
we may access incorrect chips or more than one chip at a time.


=======================================================================
KM:
(comments about his i2c-algo-biths patch)

Some notes to comment on the changes in the patch..

I have been reading the "Fast Mode" specs to test my setup, at some
points the 1 us I use violates this.

For debug use, we need adapter name from i2c_adapter everywhere.
Current debugging is useless, while the original was simply
incomprehensible. Sorry about the misspelling.

Improved symmetry; outb/inb now both handle the acknowledge timeslot.
Sendbytes/readbytes only loop over the buffer and test for failures.
Pass i2c_msg->flags deeper down to support more i2c mangling.

The i2c_adapter->retry now applies to complete message. Previously
it would fail on any [NA] after the address [A].
I am not 100% sure how auto-incrementing EEPs will tolerate it,
if we fail in the middle of a page and then resend from the beginning.

A message may be sent correctly after ignored [NA], retry or timeout.
The caller could check this in i2c_msg->err, if necessary.


=======================================================================
MDS:

thanks for the notes and the opportunity to make comments.

As I've already said, I like the 50/50 duty cycle, with
1 ns hold and (udelay - 1) setup. That's the way it should be.

One question - is clock low time == udelay at end of a
read, including end of i2c_inb and i2c_stop? it looks to me like
it's only Tmin + Tmin?

One other general comment - sdalo() and sdahi() could be deleted
and inlined into test_bus(), which is the only place they're used?

Other comments below.

Kyosti Malkki wrote:
> I have been reading the "Fast Mode" specs to test my setup, at some
> points the 1 us I use violates this.

Disagree. If you think about it, the max hold spec really only applies
when you are an i2c slave. When you are a master and driving the clock,
you can exceed the max hold time since you are essentially
"extending the clock low time" as described in the footnote.
If 1 us > 0.9 us spec is a problem, then we certainly have a
problem now with a hold time == udelay.

>
> For debug use, we need adapter name from i2c_adapter everywhere.
> Current debugging is useless, while the original was simply
> incomprehensible. Sorry about the misspelling.
>

Agreed that it wasn't very good in the past.

> Improved symmetry; outb/inb now both handle the acknowledge timeslot.
> Sendbytes/readbytes only loop over the buffer and test for failures.
> Pass i2c_msg->flags deeper down to support more i2c mangling.
>

Sounds good.

> The i2c_adapter->retry now applies to complete message. Previously
> it would fail on any [NA] after the address [A].
> I am not 100% sure how auto-incrementing EEPs will tolerate it,
> if we fail in the middle of a page and then resend from the beginning.
>

We may want to think about this more. Or maybe do something different
on reads than on writes. We don't want to corrupt eeproms.

> A message may be sent correctly after ignored [NA], retry or timeout.
> The caller could check this in i2c_msg->err, if necessary.
>

OK. But the whole mechanism of saving and checking the error, etc. is somewhat elaborate,
not sure if useful or not. If you look at the smbus-layer calls (which in turn use
the "emulation layer" if the adapter is i2c-only), errors aren't returned at all.
Which isn't great but that's the way it is now. In fact we have a two-year-old ticket
(#517 http://www2.lm-sensors.nu/~lm78/readticket.cgi?ticket=517 )
asking for improvement. I don't have any answers, just wondering if anybody
has a "vision" (!) on error handling and whether the changes in i2c-algo-bit
fit that vision.

=======================================================================
KM:

> One question - is clock low time == udelay at end of a
> read, including end of i2c_inb and i2c_stop? it looks to me like
> it's only Tmin + Tmin?

Did you mean "end of i2c_inb and i2c_outb"? At end of i2c_stop clock
remains high, perhaps need to insert missing "Stop to Start" delay
there.

For repetive inb/outb clock low after ACK is T_hold+T_setup == T_scllo.
Between inb/outb ACK and stop, it is T_hold+T_min == T_min+T_min,
should really be T_hold+T_setup there.

> One other general comment - sdalo() and sdahi() could be deleted
> and inlined into test_bus(), which is the only place they're used?

I thought of adding 'paranoid' mode, that reads back bus state after
any set. This is likely to recombine bus set, get and delay in one
inlined call.

> > I have been reading the "Fast Mode" specs to test my setup, at some
> > points the 1 us I use violates this.
>
> Disagree. If you think about it, the max hold spec really only applies
> when you are an i2c slave.

Actually, I meant 1 us violates "bus free time between Stop and
Start", "Low period of SCL" and "SCL clock frequency".
With 1us resolution best we can do within fast mode specs is
udelay=2 and get 250kHz. I think stock kernel tree doesn't export
anything to improve the resolution, so I will settle for udelay.

> > The i2c_adapter->retry now applies to complete message. Previously
> > it would fail on any [NA] after the address [A].
> > I am not 100% sure how auto-incrementing EEPs will tolerate it,
> > if we fail in the middle of a page and then resend from the beginning.
>
> We may want to think about this more. Or maybe do something different
> on reads than on writes. We don't want to corrupt eeproms.

Yep. Thinking more into it, correct retry sequence is device specific if
failure happens after address byte. Both i2c<->async and i2c<->parallel
converters should continue with with the first failed byte.

Should we keep adapter->retry for address retry? Getting [NA] for
address may reflect eeprom busy writing, disconnected device or
bus problems. Do SMBus hosts ignore this?

> > A message may be sent correctly after ignored [NA], retry or timeout.
> > The caller could check this in i2c_msg->err, if necessary.
>
> OK. But the whole mechanism of saving and checking the error, etc. is
> somewhat elaborate, not sure if useful or not. If you look at the
> smbus-layer calls (which in turn use the "emulation layer" if the
> adapter is i2c-only), errors aren't returned at all.

With algo-bit this is messy, for SMBus style adapter, you can get return
code from status register directly? For simplicity, the errorcode
set in adapter driver could pass unchanged to the caller. If adapter
drivers already return <0 for failure, nothing is changed from the
caller's point of view. Unless they test for -1 explicitly, which they
should not.

Some means (besides log) to report a stuck or disconnected bus may be
necessary to support removable SMBus devices like batteries, AC adapters
etc.


=======================================================================
KM:

> One question - is clock low time == udelay at end of a
> read, including end of i2c_inb and i2c_stop? it looks to me like
> it's only Tmin + Tmin?

Did you mean "end of i2c_inb and i2c_outb"? At end of i2c_stop clock
remains high, perhaps need to insert missing "Stop to Start" delay
there.

For repetive inb/outb clock low after ACK is T_hold+T_setup == T_scllo.
Between inb/outb ACK and stop, it is T_hold+T_min == T_min+T_min,
should really be T_hold+T_setup there.

> One other general comment - sdalo() and sdahi() could be deleted
> and inlined into test_bus(), which is the only place they're used?

I thought of adding 'paranoid' mode, that reads back bus state after
any set. This is likely to recombine bus set, get and delay in one
inlined call.

> > I have been reading the "Fast Mode" specs to test my setup, at some

> BTW, I want to make sure you've read the suggestions Dori Eldar made
> this summer, which I summarized in i2c/TODO. They're the ones marked
> "D.E.".

Yes. I got some idea what is doable and what to drop on the floor.

> > Actually, I meant 1 us violates "bus free time between Stop and
> > Start", "Low period of SCL" and "SCL clock frequency".
> > With 1us resolution best we can do within fast mode specs is
> > udelay=2 and get 250kHz. I think stock kernel tree doesn't export
> > anything to improve the resolution, so I will settle for udelay.
> >
> Agreed. Fast mode max is a udelay of 1.25. If people want to
> violate that and set a udelay of 1, they can at their peril.
> We should add documentation that a udelay of 2 (250 kHz) is a practical
> lower limit and we do not recommend a udelay of 1.
>
> When nanosleep or whatever gets exported, we can do better.

I decided to go for a new algorithm driver that pushes bus timing to the
adapter code. Currently it falls back to udelay(), but could use any
approriate counter/timer available.
I hope to get this tested a bit and commited today.

> Reading ticket 517 more carefully, the i2c-core layer DOES return
> error codes pretty faithfully. I thing the bus drivers do too.
> So I misspoke. It's the CHIP drivers that generally ignore the
> error returns and the -1 return gets cast to an FF value.

Yes, i2c-core gives -1 for any error. Being more specific about what
went wrong can be done without breaking compatibility. In i2c_transfer,
we could pass return from algo->xfer as-is.

> Should we keep adapter->retry? Seems like more trouble than it is worth.
> Maybe you could hold it back as part 2 of the patch?

Well my patch got so radical that I broke compatibility, and some
changes are not easy to port back.
I think I have caught and fixed following problems in original code:

All the duty cycle stuff.

If SDA is stuck low, writes seem to work, reads returns 0x00.
If SCL is stuck low, we keep hitting timeout.
The driver should die more gracefully on bus failure, and advice the
caller to give up by returning -ENODEV or something besides -1.

After SCL timeout, SCL is not pulled low by host.
Client may release SCL at any time between timeout and stop.
At some situations, timeout is ignored and/or SDA is toggled.
Stop assumes SCL low on entry.


> The i2c_smbus_xxx_xxx calls return an s32, -1 on failure.
> In a chip driver, for example, it should probably check for -1
> returns and throw out that reading, and show the user the last good rea=
ding.
> Otherwise, cast s32 to u8 as usual.

Why test for -1 explicitly, just test negative for error?


=======================================================================
KM:

Here we go. In order of the TODO.

1. Timing considerations

Maximum of 50us SCL high, I quess this is an alternative for detecting
bus busy condition instead of Start-to-Stop. Sampling for 50us before
start is easier to achieve than sampling lines all the time for
Start/Stop. Neither was done by i2c-algo-bit, nor I have plans to do so.

With new driver these can be supported, if approriate capture/compare
unit is wired on the SCL line. In this case adapter code does not use
the provided inlines from i2c-algo-biths.h.


2. Arbitration

Without busy detection, no point for arbitration. The best we can do is
stop the message that already was corrupted if we notice difference in
set and get bus state. It is easy to change the recovery action from
Stop to release, if bus busy is detected.


Both issues can be ignored on a single-master system. One designing
multi-master bus system, really should use HW glue logic for SMBus
access anyway.