coherent: tighten up the API

src/include/sof/coherent.h

@@ -20,11 +20,37 @@
 #define __coherent __attribute__((packed, aligned(DCACHE_LINE_SIZE)))
 
 /*
+ * The coherent API allows optimized access to memory by multiple cores, using
+ * cache, taking care about coherence. The intended use is to let cores acquire
+ * ownership of such shared objects, use them, and then release them, possibly
+ * to be re-acquired by other cores. Such shared objects must only be accessed
+ * via this API. It's designed to be primarily used with dynamically allocated
+ * objects because of their well-defined life span. It can also be used with
+ * objects from .data or .bss sections but greater care must be takenwith them
+ * to strictly follow the API flow.
+ *
+ * The API assumes, that in the beginning no core has cache lines associated
+ * with the memory area, used with it. That is true for dynamically allocated
+ * memory, because when such memory is freed, its cache is invalidated - as long
+ * as that memory was never accessed by other cores, except by using this API.
+ * The first call must be coherent_init(), which initializes the header. If the
+ * object will be used by multiple cores, next coherent_shared() must be called.
+ * After that to use that memory, coherent_acquire_irq() or coherent_acquire()
+ * must be called, which acquires ownership of the object and returns a cached
+ * address of the memory. After that the user can perform cached access to the
+ * memory. To release the memory, one of coherent_release_irq() or
+ * coherent_release() must be called. The only time when the memory is accessed
+ * using cache is between those two calls, so only when releasing the memory we
+ * have to write back and invalidate caches to make sure, that next time we
+ * acquire this memory, our uncached header access will not be overwritten! When
+ * memory is not needed any more, typically before freeing the memory,
+ * coherent_free() should be called.
+ *
  * This structure needs to be embedded at the start of any container to ensure
  * container object cache alignment and to minimise non cache access when
  * acquiring ownership.
  *
- * This structure should not be accessed outside of these APIs.
+ * This structure must not be accessed outside of these APIs.
  * The shared flag is only set at coherent init and thereafter it's RO.
  */
 struct coherent {
@@ -58,12 +84,8 @@ struct coherent {
 #define CHECK_COHERENT_IRQ(_c)
 #endif
 
-/*
- * Incoherent devices require manual cache invalidation and writeback as
- * well as locking to manage shared access.
- */
-
 #if CONFIG_INCOHERENT
+/* When coherent_acquire() is called, we are sure not to have cache for this memory */
 __must_check static inline struct coherent *coherent_acquire(struct coherent *c,
 							     const size_t size)
 {
@@ -149,46 +171,46 @@ static inline struct coherent *coherent_release_irq(struct coherent *c, const si
 	return cache_to_uncache(c);
 }
 
-#define coherent_init(object, member)			\
-	do {	\
+#define coherent_init(object, member)						\
+	do {									\
 		/* assert if someone passes a cache/local address in here. */	\
 		ADDR_IS_COHERENT(object);					\
 		/* TODO static assert if we are not cache aligned */		\
 		spinlock_init(&object->member.lock);				\
-		object->member.shared = false;				\
+		object->member.shared = false;					\
 		object->member.core = cpu_get_id();				\
 		list_init(&object->member.list);				\
-		/* wtb and inv local data to coherent object */			\
-		dcache_writeback_invalidate_region(uncache_to_cache(object), sizeof(*object)); \
+		/* inv local data to coherent object */				\
+		dcache_invalidate_region(uncache_to_cache(object), sizeof(*object)); \
 	} while (0)
 
-#define coherent_free(object, member)			\
-	do { \
-		/* assert if someone passes a cache address in here. */	\
+#define coherent_free(object, member)						\
+	do {									\
+		/* assert if someone passes a cache address in here. */		\
 		ADDR_IS_COHERENT(object);					\
 		/* wtb and inv local data to coherent object */			\
 		dcache_writeback_invalidate_region(uncache_to_cache(object), sizeof(*object)); \
 	} while (0)
 
 /* set the object to shared mode with coherency managed by SW */
-#define coherent_shared(object, member)					\
-	do { \
+#define coherent_shared(object, member)						\
+	do {									\
 		/* assert if someone passes a cache/local address in here. */	\
 		ADDR_IS_COHERENT(object);					\
 		spin_lock(&(object)->member.lock);				\
 		(object)->member.shared = true;					\
-		dcache_writeback_invalidate_region(object, sizeof(*object));	\
-		spin_unlock(&(object)->member.lock); \
+		dcache_invalidate_region(object, sizeof(*object));		\
+		spin_unlock(&(object)->member.lock);				\
 	} while (0)
 
 /* set the object to shared mode with coherency managed by SW */
-#define coherent_shared_irq(object, member)				\
-	do { \
+#define coherent_shared_irq(object, member)					\
+	do {									\
 		/* assert if someone passes a cache/local address in here. */	\
 		ADDR_IS_COHERENT(object);					\
 		spin_lock_irq(&(object)->member.lock, &(object)->member.flags);	\
 		(object)->member.shared = true;					\
-		dcache_writeback_invalidate_region(object, sizeof(*object));	\
+		dcache_invalidate_region(object, sizeof(*object));		\
 		spin_unlock_irq(&(object)->member.lock, &(object)->member.flags); \
 	} while (0)
 #else
@@ -245,8 +267,8 @@ static inline struct coherent *coherent_release_irq(struct coherent *c, const si
 	return c;
 }
 
-#define coherent_init(object, member)					\
-	do { \
+#define coherent_init(object, member)						\
+	do {									\
 		/* TODO static assert if we are not cache aligned */		\
 		spinlock_init(&object->member.lock);				\
 		object->member.shared = 0;					\