Add support for tracking nested calls to iicbus_request/release_bus().

Usually it is sufficient to use iicbus_transfer_excl(), or one of the
higher-level convenience functions that use it, to reserve the bus for the
duration of each register access.  Occasionally it is important that a
series of accesses or read-modify-write operations must be done without any
other intervening access to the device, to prevent corrupting state.

Without support for nested request/release, slave device drivers would have
to stop using high-level convenience functions and resort to working with
arrays of iic_msg structs just for a few operations (often involving
one-time device setup or infrequent configuration changes).

The changes here appear large from a glance at the diff, but in fact they're
nearly trivial, and the large diff is because of changes in indentation and
the re-wrapping of comments caused by that.  One notable change is that
iicbus_release_bus() now ignores the IICBUS_CALLBACK(IIC_RELEASE_BUS) return
value.  The old error handling left the bus in a kind of limbo state where
it was still owned at the iicbus layer, but drivers rarely check the return
of the release call, and it's unclear what they would do to recover from an
error return anyway.  No existing low-level drivers return any kind of error
from IIC_RELEASE_BUS except one EINVAL for "you don't own the bus", to which
the right response is probably to carry on with the process of releasing the
reference to the bus anyway.
This commit is contained in:
Ian Lepore 2017-07-26 21:06:26 +00:00
parent 7ec74c580d
commit 34199ee049
2 changed files with 28 additions and 34 deletions

View File

@ -39,6 +39,7 @@ struct iicbus_softc
{
device_t dev; /* Myself */
device_t owner; /* iicbus owner device structure */
u_int owncount; /* iicbus ownership nesting count */
u_char started; /* address of the 'started' slave
* 0 if no start condition succeeded */
u_char strict; /* deny operations that violate the

View File

@ -113,26 +113,30 @@ iicbus_request_bus(device_t bus, device_t dev, int how)
IICBUS_LOCK(sc);
while ((error == 0) && (sc->owner != NULL))
while (error == 0 && sc->owner != NULL && sc->owner != dev)
error = iicbus_poll(sc, how);
if (error == 0) {
sc->owner = dev;
/*
* Drop the lock around the call to the bus driver.
* This call should be allowed to sleep in the IIC_WAIT case.
* Drivers might also need to grab locks that would cause LOR
* if our lock is held.
*/
IICBUS_UNLOCK(sc);
/* Ask the underlying layers if the request is ok */
error = IICBUS_CALLBACK(device_get_parent(bus),
IIC_REQUEST_BUS, (caddr_t)&how);
IICBUS_LOCK(sc);
if (error != 0) {
sc->owner = NULL;
wakeup_one(sc);
++sc->owncount;
if (sc->owner == NULL) {
sc->owner = dev;
/*
* Drop the lock around the call to the bus driver, it
* should be allowed to sleep in the IIC_WAIT case.
* Drivers might also need to grab locks that would
* cause a LOR if our lock is held.
*/
IICBUS_UNLOCK(sc);
/* Ask the underlying layers if the request is ok */
error = IICBUS_CALLBACK(device_get_parent(bus),
IIC_REQUEST_BUS, (caddr_t)&how);
IICBUS_LOCK(sc);
if (error != 0) {
sc->owner = NULL;
sc->owncount = 0;
wakeup_one(sc);
}
}
}
@ -150,7 +154,6 @@ int
iicbus_release_bus(device_t bus, device_t dev)
{
struct iicbus_softc *sc = (struct iicbus_softc *)device_get_softc(bus);
int error;
IICBUS_LOCK(sc);
@ -159,26 +162,16 @@ iicbus_release_bus(device_t bus, device_t dev)
return (IIC_EBUSBSY);
}
/*
* Drop the lock around the call to the bus driver.
* This call should be allowed to sleep in the IIC_WAIT case.
* Drivers might also need to grab locks that would cause LOR
* if our lock is held.
*/
IICBUS_UNLOCK(sc);
/* Ask the underlying layers if the release is ok */
error = IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS, NULL);
if (error == 0) {
if (--sc->owncount == 0) {
/* Drop the lock while informing the low-level driver. */
IICBUS_UNLOCK(sc);
IICBUS_CALLBACK(device_get_parent(bus), IIC_RELEASE_BUS, NULL);
IICBUS_LOCK(sc);
sc->owner = NULL;
/* wakeup a waiting thread */
wakeup_one(sc);
IICBUS_UNLOCK(sc);
}
return (error);
IICBUS_UNLOCK(sc);
return (0);
}
/*