[i2c] linux-i2c and smbalert

[Jean Delvare]

Hi Hendrik,

> > > The patch for i2c_i801 will follow based on this one. It is a bit tricky
> > > as the polled read/write from the register from a timer should be secured
> > > by a mutex to not interfere with the main transaction function.
> >
> > True. Each i2c_adapter has a mutex already (bus_lock), which we use to
> > serialize the transactions on the bus. Maybe you can use it for that
> > purpose too?
> 
> Would be a big lock though, so better have a second one for the read/write 
> action of that register.

With what benefit? This would slow down the regular transactions
significantly. It may decrease latency a bit, but I don't think we care
here. Anyway, this is a per-driver decision, so no need to worry to
much either.

> > Returning 0 on error, interesting convention ;)
> 
> Not really:
>  addr = i2c_smbus_read_ara(me);
>  if (addr) i2c_smbus_alert_notify(me,addr);
> But...
> 
> > Better make the return type an int and let the error go through.
> 
> ...this may be more like other places in kernel.

Yes, absolutely:

	addr = i2c_smbus_read_ara(adap);
	if (addr < 0)
		return -EIO;
	i2c_smbus_alert_notify(adap, addr);

would look much more like regular (kernrl) code.

> > One thing which may need to be discussed is that the ARA could have
> > been requested by a driver already. We should either make sure this
> > address can never be requested, or test that it is still free before
> > calling i2c_smbus_read_byte.c
> 
> Well, this would restrict a SMBus over an I²C bus.

What do you mean?

> There may be I²C chips out 
> there that use this address. OTOH, it is an illegal address for a chip 
> according to SMBus. However, in I²C 2.1 it is allowed.

True.

> So either restrict I²C or omit small parts SMBus specs. As devices from both 
> can be on the same bus, I'd for restricting I²C.

On the other hand, putting both an I2C device with address 0x0c, and an
SMBus Alert-capable chip, on the same bus, would be such a gross design
error that it is unlikely to happen, so I start wondering if we need to
care at all...

The easiest way would be to test at runtime. However this would cause a
performance penalty. If we could do all the needed tests at
initialization time, it would perform better, but as I see it, it
implies making aribtrary decisions.

> And while we are at it, the following addresses should not be allowed to 
> register:
>  <= 0x07 (0x04-0x07 are for HS-mode(I²C) or reserved(SMBus))
>  >= 0x70 (10bit or reserverd (I²C and SMBus))

Not true, 0x70 to 0x77 are valid 7-bit I2C/SMBus addresses.

>     0x08 (Host(SMBus))

I think I remember this is simply the default host slave address. Any
SMBus host may decide to use a different address if they want. If
nothing else, that would be needed on a multi-master bus (not sure if
SMBus allows that, or only I2C?)

>     0x0C (alert response address(SMBus))
>     0x61 (device default address(SMBus)), are there known ARP devices?

Yes there are ARP devices out there, even though we don't support ARP
for now.

> Probably optional:
>     0x28 (ACCESS bus host(SMBus))
>     0x37 (ACCESS bus default address(SMBus))

I'm pretty sure we've already seen devices at these addresses.

> So it's more than just the ARA to think about.

I don't think so. The addresses listed above are not just "special
addresses". Each is special for a different reason, so the way to
handle this specificity will differ for each of them.

For SMBus Alert, the only address we care about is 0x0c.

-- 
Jean Delvare