You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by an...@apache.org on 2018/11/23 16:16:54 UTC
[mynewt-core] 22/26: hw/bus: Update locking API
This is an automated email from the ASF dual-hosted git repository.
andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git
commit 079e583eddac1f2fe731105571f17fd58bb03389
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Mon Nov 19 14:35:25 2018 +0100
hw/bus: Update locking API
Bus locking APIs names were changed to more friendly bus_node_lock()
and bus_node_unlock().
Also locking bus will automatically configure bus for given node since
we lock the bus in order to talk to given node so can have extra call
here.
bus_node_unlock() will not return an error since it's a bit useless -
probably no one cares about it and it can only fail if things are really
broken already, so we'll just assert internally to aid debugging.
---
hw/bus/include/bus/bus.h | 27 ++++++++-------
hw/bus/src/bus.c | 89 +++++++++++++++++-------------------------------
2 files changed, 47 insertions(+), 69 deletions(-)
diff --git a/hw/bus/include/bus/bus.h b/hw/bus/include/bus/bus.h
index 49770b2..11a8d07 100644
--- a/hw/bus/include/bus/bus.h
+++ b/hw/bus/include/bus/bus.h
@@ -164,7 +164,7 @@ bus_node_simple_write_read_transact(struct os_dev *node, const void *wbuf,
*
* \deprecated
* This API is only provided for compatibility with legacy drivers where locking
- * is provided by Sensors interface. To lock bus for compount transactions use
+ * is provided by Sensors interface. To lock bus for compound transactions use
* bus_dev_lock() and bus_dev_unlock() instead.
*
* @param bus Bus device object
@@ -178,34 +178,37 @@ bus_dev_get_lock(struct os_dev *bus, struct os_mutex **mutex);
/**
* Lock bus for exclusive access
*
- * Locks bus which given node is attached to for exclusive access. This should
- * be only used for compound transactions where bus shall be locked for the
- * duration of entire transaction. Simple operations like read, write or
- * write-read (i.e. those with dedicated APIs) lock bus automatically.
+ * Locks bus for exclusive access. The parent bus of given node (i.e. bus where
+ * this node is attached) will be locked. This should be only used for compound
+ * transactions where bus shall be locked for the duration of entire transaction.
+ * Simple operations like read, write or write-read (i.e. those with dedicated
+ * APIs) lock bus automatically.
*
- * @param node Node attached to bus to lock
+ * After successful locking, bus is configured to be used with given node.
+ *
+ * @param node Node to lock its parent bus
* @param timeout Timeout on locking attempt
*
* return 0 on success
* SYS_ETIMEOUT on lock timeout
*/
int
-bus_dev_lock_by_node(struct os_dev *node, os_time_t timeout);
+bus_node_lock(struct os_dev *node, os_time_t timeout);
/**
- * Unlock bus
+ * Unlock bus node
*
- * Unlocks bus access. This shall be only used when bus was previously locked
- * with bus_dev_lock_by_node(). API operations which lock bus will also unlock
+ * Unlocks exclusive bus access. This shall be only used when bus was previously
+ * locked with bus_node_lock(). API operations which lock bus will also unlock
* bus automatically.
*
- * @param node Node attached to bus to unlock
+ * @param node Node to unlock its parent bus
*
* return 0 on success
* SYS_EACCESS when bus was not locked by current task
*/
int
-bus_dev_unlock_by_node(struct os_dev *node);
+bus_node_unlock(struct os_dev *node);
#ifdef __cplusplus
}
diff --git a/hw/bus/src/bus.c b/hw/bus/src/bus.c
index e8d0a9f..4635969 100644
--- a/hw/bus/src/bus.c
+++ b/hw/bus/src/bus.c
@@ -72,27 +72,6 @@ bus_node_close_func(struct os_dev *odev)
return 0;
}
-static inline int
-bus_dev_configure_for(struct bus_dev *bdev, struct bus_node *bnode)
-{
- int rc;
-
- assert((void *)bdev == (void *)bnode->parent_bus);
-
- if (bdev->configured_for == bnode) {
- return 0;
- }
-
- rc = bdev->dops->configure(bdev, bnode);
- if (rc) {
- bdev->configured_for = NULL;
- } else {
- bdev->configured_for = bnode;
- }
-
- return rc;
-}
-
void
bus_node_set_callbacks(struct os_dev *node, struct bus_node_callbacks *cbs)
{
@@ -166,21 +145,14 @@ bus_node_read(struct os_dev *node, void *buf, uint16_t length,
return SYS_ENOTSUP;
}
- rc = bus_dev_lock_by_node(node, timeout);
+ rc = bus_node_lock(node, timeout);
if (rc) {
return rc;
}
- rc = bus_dev_configure_for(bdev, bnode);
- if (rc) {
- goto done;
- }
-
rc = bdev->dops->read(bdev, bnode, buf, length, timeout, flags);
-done:
- rc = bus_dev_unlock_by_node(node);
- assert(rc == 0);
+ (void)bus_node_unlock(node);
return rc;
}
@@ -200,21 +172,14 @@ bus_node_write(struct os_dev *node, const void *buf, uint16_t length,
return SYS_ENOTSUP;
}
- rc = bus_dev_lock_by_node(node, timeout);
+ rc = bus_node_lock(node, timeout);
if (rc) {
return rc;
}
- rc = bus_dev_configure_for(bdev, bnode);
- if (rc) {
- goto done;
- }
-
rc = bdev->dops->write(bdev, bnode, buf, length, timeout, flags);
-done:
- rc = bus_dev_unlock_by_node(node);
- assert(rc == 0);
+ (void)bus_node_unlock(node);
return rc;
}
@@ -226,7 +191,7 @@ bus_node_write_read_transact(struct os_dev *node, const void *wbuf,
{
struct bus_node *bnode = (struct bus_node *)node;
struct bus_dev *bdev = bnode->parent_bus;
- int rc, rc_unlock;
+ int rc;
BUS_DEBUG_VERIFY_DEV(bdev);
BUS_DEBUG_VERIFY_NODE(bnode);
@@ -235,16 +200,11 @@ bus_node_write_read_transact(struct os_dev *node, const void *wbuf,
return SYS_ENOTSUP;
}
- rc = bus_dev_lock_by_node(node, timeout);
+ rc = bus_node_lock(node, timeout);
if (rc) {
return rc;
}
- rc = bus_dev_configure_for(bdev, bnode);
- if (rc) {
- goto done;
- }
-
/*
* XXX we probably should pass flags here but with some of them stripped,
* e.g. BUS_F_NOSTOP should not be present here, but since we do not have
@@ -262,20 +222,19 @@ bus_node_write_read_transact(struct os_dev *node, const void *wbuf,
}
done:
- /* This shall succeed because we locked it */
- rc_unlock = bus_dev_unlock_by_node(node);
- assert(rc_unlock == 0);
+ (void)bus_node_unlock(node);
return rc;
}
int
-bus_dev_lock_by_node(struct os_dev *node, os_time_t timeout)
+bus_node_lock(struct os_dev *node, os_time_t timeout)
{
struct bus_node *bnode = (struct bus_node *)node;
struct bus_dev *bdev = bnode->parent_bus;
os_error_t err;
+ int rc;
BUS_DEBUG_VERIFY_DEV(bdev);
BUS_DEBUG_VERIFY_NODE(bnode);
@@ -285,13 +244,25 @@ bus_dev_lock_by_node(struct os_dev *node, os_time_t timeout)
return SYS_ETIMEOUT;
}
- assert(err == OS_OK);
+ assert(err == OS_OK || err == OS_NOT_STARTED);
- return 0;
+ /* No need to configure if already configured for the same node */
+ if (bdev->configured_for == bnode) {
+ return 0;
+ }
+
+ rc = bdev->dops->configure(bdev, bnode);
+ if (rc) {
+ bdev->configured_for = NULL;
+ } else {
+ bdev->configured_for = bnode;
+ }
+
+ return rc;
}
int
-bus_dev_unlock_by_node(struct os_dev *node)
+bus_node_unlock(struct os_dev *node)
{
struct bus_node *bnode = (struct bus_node *)node;
struct bus_dev *bdev = bnode->parent_bus;
@@ -301,11 +272,15 @@ bus_dev_unlock_by_node(struct os_dev *node)
BUS_DEBUG_VERIFY_NODE(bnode);
err = os_mutex_release(&bdev->lock);
- if (err == OS_BAD_MUTEX) {
- return SYS_EACCES;
- }
- assert(err == OS_OK);
+ /*
+ * Probably no one cares about return value from unlock, so for debugging
+ * purposes let's assert on anything that is not a success. This includes
+ * OS_INVALID_PARM (we basically can't pass invalid mutex here unless our
+ * structs are broken) and OS_BAD_MUTEX (unlock shall be only done by the
+ * same task which locked it).
+ */
+ assert(err == OS_OK || err == OS_NOT_STARTED);
return 0;
}