Thread (44 messages) flat view 44 messages, 4 authors, 2016-05-04

Re: [PATCH v7 16/24] i2c: allow adapter drivers to override the adapter locking

From: Wolfram Sang <hidden>
Date: 2016-05-03 21:38:42
Also in: linux-i2c, linux-iio, linux-media, lkml

On Wed, Apr 20, 2016 at 05:17:56PM +0200, Peter Rosin wrote:
Add i2c_lock_bus() and i2c_unlock_bus(), which call the new lock_bus and
unlock_bus ops in the adapter. These funcs/ops take an additional flags
argument that indicates for what purpose the adapter is locked.

There are two flags, I2C_LOCK_ADAPTER and I2C_LOCK_SEGMENT, but they are
both implemented the same. For now. Locking the adapter means that the
whole bus is locked, locking the segment means that only the current bus
segment is locked (i.e. i2c traffic on the parent side of mux is still
allowed even if the child side of the mux is locked.

Also support a trylock_bus op (but no function to call it, as it is not
expected to be needed outside of the i2c core).

Implement i2c_lock_adapter/i2c_unlock_adapter in terms of the new locking
scheme (i.e. lock with the I2C_LOCK_ADAPTER flag).

Annotate some of the locking with explicit I2C_LOCK_SEGMENT flags.
Can you explain a little why it is SEGMENT and not ADAPTER here? That
probably makes it easier to get into this patch.

And to double check my understanding: I was surprised to not see any
i2c_lock_adapter() or I2C_LOCK_ADAPTER in action. This is because muxes
call I2C_LOCK_SEGMENT on their parent which in case of the parent being
the root adapter is essentially the same as I2C_LOCK_ADAPTER. Correct?
quoted hunk ↗ jump to hunk
Signed-off-by: Peter Rosin <redacted>
---
 drivers/i2c/i2c-core.c | 46 ++++++++++++++++++++++++++++------------------
 include/linux/i2c.h    | 28 ++++++++++++++++++++++++++--
 2 files changed, 54 insertions(+), 20 deletions(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 0f2f8484e8ec..21f46d011c33 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -960,10 +960,12 @@ static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
 }
 
 /**
- * i2c_lock_adapter - Get exclusive access to an I2C bus segment
+ * i2c_adapter_lock_bus - Get exclusive access to an I2C bus segment
  * @adapter: Target I2C bus segment
+ * @flags: I2C_LOCK_ADAPTER locks the root i2c adapter, I2C_LOCK_SEGMENT
+ *	locks only this branch in the adapter tree
  */
I think this kerneldoc should be moved to i2c_lock_adapter and/or
i2c_lock_bus() which are now in i2c.h. This is what users will use, not
this static, adapter-specific implementation. I think it is enough to
have a comment here explaining what is special in handling adapters.

Thanks,

   Wolfram
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help