Locking inconsistencies in I2C land

The locking in I2C land is inconsistent.

For the sake of this explanation, I will concentrate on
DEVMETHOD(iicbus_transfer), of which we have about a dozen
implementations, and which seems to be the most used part
of the KAPI.


The first level of locking must ensure that there can only be one
DEVMETHOD(iicbus_transfer) active on the I2C bus at any time.

Some drivers do this with a mutex or sxlock.

Some drivers do no locking, leaving it to the users to fight it out.
In particular drivers which implement wire-primitives and leave
*_transfer() to iicbus_transfer_gen() get no locking.


At the second level of locking, some users need to be able to execute
a sequence of transactions with no other traffic on the bus, and
preferably also without the bus/slave going away.

This is implemented via iicbus_{request|release}_bus() which call
the underlying driver using the surprisingly named
DEVMETHOD(iicbus_callback) to hear if it has an opinion.

But iicbus_{request|release}_bus() is only advisory:  Nothing
prevents other callers to intrude with traffic subject only to
whatever locking the primary level might provide.


The ichiic driver tries to mitigate with both levels sharing the
same sxlock.

That only makes matters worse, because the transfer function contains
this logic:

	allocated = sx_xlocked(&sc->call_lock) != 0;
	if (!allocated)
		sx_xlock(&sc->call_lock);
	[…do the job…]
	if (!allocated)
		sx_unlock(&sc->call_lock);

Which means that as long as /somebody/ holds iic_request_bus(), there
will be no first level locking for /anybody/.


We also have other equally hard to fix I2C architectual trouble.

For instance our shifting slave addresses one bit to the left, which
neither FDT, ACPI nor anybody else does, so we cannot use the
iicbus-methods directly from those contexts, because we have to
munge the slave addresses one bit to the right first.


Most of the trouble comes from I2C migrating from being "I've tied
some fun chips to my parallel port" to "there is silicon for 24
I2C controllers in this laptop, and we need to get at least six of
them working", which is where I'm sitting now.


I have a hard time imagining a remedy for these problems which do
not involve going over every single I2C driver and I2C user in the
tree and breaking a lot of out-of-tree stuff in the process, and
at the end we would still have a KAPI designed last century and
still ill suited for modern hardware and use cases.


It looks, to me, like the sensible cure to adopt or design a new
I2C KAPI, aimed squarly at ACPI/FDT land, and leave the old stuff
to rot away, for some number of release cycles.

Input ?

Poul-Henning

-- 
Poul-Henning Kamp       | UNIX since Zilog Zeus 3.20
phk@FreeBSD.ORG         | TCP/IP since RFC 956
FreeBSD committer       | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.