diff mbox

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

Message ID 1461165484-2314-17-git-send-email-peda@axentia.se (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Rosin April 20, 2016, 3:17 p.m. UTC
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.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 drivers/i2c/i2c-core.c | 46 ++++++++++++++++++++++++++++------------------
 include/linux/i2c.h    | 28 ++++++++++++++++++++++++++--
 2 files changed, 54 insertions(+), 20 deletions(-)

Comments

Wolfram Sang April 28, 2016, 8:50 p.m. UTC | #1
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.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>

Letting you know that I start reviewing the 2nd part of your series. Did
the first glimpse today. Will hopefully do the in-depth part this
weekend. One thing already:

> +static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)

Shouldn't flags be unsigned?
Peter Rosin April 28, 2016, 9:08 p.m. UTC | #2
On 2016-04-28 22:50, Wolfram Sang wrote:
> 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.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
> Letting you know that I start reviewing the 2nd part of your series. Did
> the first glimpse today. Will hopefully do the in-depth part this
> weekend. One thing already:
>
>> +static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
> Shouldn't flags be unsigned?
>

Yes, obviously... I'll make that change locally and wait for the rest.

Cheers,
Peter

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang April 29, 2016, 7:16 a.m. UTC | #3
> Yes, obviously... I'll make that change locally and wait for the rest.

Another nit: You could use '--strict' with checkpatch and see if you
want to fix the issues reported. I am not keen on those (except for
'space around operators'), it's a matter of taste I guess, but maybe you
like some of the suggestions.

Thanks,

   Wolfram
Peter Rosin April 29, 2016, 9:16 a.m. UTC | #4
On 2016-04-29 09:16, Wolfram Sang wrote:
>> Yes, obviously... I'll make that change locally and wait for the rest.
> Another nit: You could use '--strict' with checkpatch and see if you
> want to fix the issues reported. I am not keen on those (except for
> 'space around operators'), it's a matter of taste I guess, but maybe you
> like some of the suggestions.
>
Yes, they look like reasonable complaints.

So, I fixed all of them locally except the complaint about lack of comment
on the new struct mutex member in struct si2168_dev (patch 21/24),
because that patch is Anttis and he's the maintainer of that driver...

Antti, if you want that fixed as part of this series, send a suitable comment
for the mutex this way and I'll incorporate it.

Cheers,
Peter

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Antti Palosaari May 2, 2016, 7:20 p.m. UTC | #5
On 04/29/2016 12:16 PM, Peter Rosin wrote:
> On 2016-04-29 09:16, Wolfram Sang wrote:
>>> Yes, obviously... I'll make that change locally and wait for the rest.
>> Another nit: You could use '--strict' with checkpatch and see if you
>> want to fix the issues reported. I am not keen on those (except for
>> 'space around operators'), it's a matter of taste I guess, but maybe you
>> like some of the suggestions.
>>
> Yes, they look like reasonable complaints.
>
> So, I fixed all of them locally except the complaint about lack of comment
> on the new struct mutex member in struct si2168_dev (patch 21/24),
> because that patch is Anttis and he's the maintainer of that driver...
>
> Antti, if you want that fixed as part of this series, send a suitable comment
> for the mutex this way and I'll incorporate it.

Ah, I never ran checkpatch with --strict option...

CHECK: struct mutex definition without comment
#202: FILE: drivers/media/dvb-frontends/si2168_priv.h:32:
+	struct mutex i2c_mutex;

If you wish you could add some comment for it, but for me it is still 
pretty much self explaining. It is lock to protect firmware command 
execution. Command is executed always with I2C write and then poll reply 
using I2C read until it timeouts or answers with "ready" status.

regards
Antti
Wolfram Sang May 3, 2016, 9:38 p.m. UTC | #6
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?

> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang May 3, 2016, 9:39 p.m. UTC | #7
> Yes, they look like reasonable complaints.

Thanks for fixing them. I just sent out my latest comments and when you
fix those and send V8, I'll apply that right away. I think we are safe
to fix the rest incrementally if needed. Note that I didn't review the
IIO and media patches, I trust the reviewers on those.

Thanks for your work on this! I need a break now, this is
mind-boggling...

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Rosin May 4, 2016, 10:01 a.m. UTC | #8
Hi!

On 2016-05-03 23:38, Wolfram Sang wrote:
> 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?

Correct. Locking the ADAPTER and the SEGMENT is the same thing for
the root adapter and for traditional muxes (i.e. not mux-locked).
Traditional muxes simply feed the locking request upwards to the
root adapter. The new mux-locked muxes behave the same for ADAPTER
locking; all locks all the way up to the root adapter are grabbed
instantly. If you instead lock SEGMENT on a mux-locked mux, only
the new mux lock in the parent adapter is grabbed right away, but
when the mux then fires off accesses on its parent adapter, that
triggers SEGMENT locks one level up in the tree and the process
recurses.

So, SEGMENT locking is the normal thing that happens when e.g. normal
i2c_transfer calls are made. ADAPTER locking is used for transactions.
The patch enables muxes to use more relaxed locking compared to
locking the ADAPTER for its transations.

The naming can probably be improved. SEGMENT made more sense when
it did not lock all mux accesses one level up (that changed in v6,
but I didn't change the I2C_LOCK_SEGMENT name at that time), so it
is kind of outdated. I2C_LOCK_ROOT_ADAPTER and I2C_LOCK_MUXES instead
of I2C_LOCK_ADAPTER and L2C_LOCK_SEGMENT perhaps?

But I don't really like I2C_LOCK_MUXES as I find it a bit too specific,
and people not thinking about i2c muxes at all (most people I gather,
ignorance is bliss) will not think that it is something for them...

It is also the case that the two flags are mutually exclusive, and
at this point there is no valid uses of using neither flag, nor for
using both. It is a binary decision and one flag would technically be
enough. So, not setting e.g. I2C_LOCK_ADAPTER could in theory imply
I2C_LOCK_SEGMENT. I did it as two separate flags since there might
be a third (or fourth even) option in the future (I don't see what
it would be though, I have no crystal ball...)

So maybe there should be only one flag, e.g. I2C_LOCK_ROOT_ADAPTER?
I.e. perhaps leave the future for later?

Hmmm, I just now realized that you were not really suggesting any
changes other than to the commit message. Oh well, I can perhaps
rephrase some of the above in the commit message if you think that
we should not unnecessarily touch the code at this point...

>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>>  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.

Yes, I was not really satisfied with having documentation on static
functions. But if I move it, there is no natural home for the current
i2c_trylock_adapter docs, and I'd hate killing documentation that
still applies. Do you have a suggestion? Maybe keep that one doc at
the static i2c_trylock_adapter for now and move it to ->trylock_bus
when someone decides to write kerneldoc for struct i2c_adapter?

Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang May 4, 2016, 12:07 p.m. UTC | #9
Hi Peter,

thanks for the detailed explanation!

> So maybe there should be only one flag, e.g. I2C_LOCK_ROOT_ADAPTER?
> I.e. perhaps leave the future for later?

I think this makes the current code easier understandable at this point,
but I'll leave the decision to you. I am fine with both. Maybe a few
words of explanation would be good if you want to keep both flags.

> Hmmm, I just now realized that you were not really suggesting any
> changes other than to the commit message. Oh well, I can perhaps
> rephrase some of the above in the commit message if you think that
> we should not unnecessarily touch the code at this point...

Yes, updated commit description is enough for me now. If you want to
change to one flag, we should do it incrementally. I think I can apply
this as a fixup until around rc3 I'd say.

> > 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.
> 
> Yes, I was not really satisfied with having documentation on static
> functions. But if I move it, there is no natural home for the current
> i2c_trylock_adapter docs, and I'd hate killing documentation that
> still applies. Do you have a suggestion? Maybe keep that one doc at
> the static i2c_trylock_adapter for now and move it to ->trylock_bus
> when someone decides to write kerneldoc for struct i2c_adapter?

Well, because I think redundancy is acceptable when it comes to
documentation, how about keeping the chunks you already have and copy an
adapted one over to the functions in i2c.h?

Regards,

   Wolfram
Peter Rosin May 4, 2016, 2:10 p.m. UTC | #10
On 2016-05-03 23:39, Wolfram Sang wrote:
>> Yes, they look like reasonable complaints.
> 
> Thanks for fixing them. I just sent out my latest comments and when you
> fix those and send V8, I'll apply that right away. I think we are safe
> to fix the rest incrementally if needed. Note that I didn't review the

Sounds like a plan.

> IIO and media patches, I trust the reviewers on those.
> 
> Thanks for your work on this! I need a break now, this is
> mind-boggling...

And thanks for reviewing it!

A question on best practices here... I already did a v8 (but only as
a branch) so I think this will be v9, bit that's a minor detail. The
real question is what I should do about patches 1-15 that are already
in next? Send them too? If not, should I send 16-24 with the same old
patch numbers or should they be numbered 1-9 now? And should such a
shortened series be rebased on 1-15 in next?

Or does it not really matter?

Cheers,
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang May 4, 2016, 4:38 p.m. UTC | #11
> A question on best practices here... I already did a v8 (but only as
> a branch) so I think this will be v9, bit that's a minor detail. The
> real question is what I should do about patches 1-15 that are already
> in next? Send them too? If not, should I send 16-24 with the same old
> patch numbers or should they be numbered 1-9 now? And should such a
> shortened series be rebased on 1-15 in next?
> 
> Or does it not really matter?

Easiest for me is:

Send as v9, only the patches not yet applied, numbered from 1-9, based
on my for-next.
diff mbox

Patch

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
  */
-void i2c_lock_adapter(struct i2c_adapter *adapter)
+static void i2c_adapter_lock_bus(struct i2c_adapter *adapter, int flags)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
 
@@ -972,27 +974,30 @@  void i2c_lock_adapter(struct i2c_adapter *adapter)
 	else
 		rt_mutex_lock(&adapter->bus_lock);
 }
-EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 
 /**
- * i2c_trylock_adapter - Try to get exclusive access to an I2C bus segment
+ * i2c_adapter_trylock_bus - Try to get exclusive access to an I2C bus segment
  * @adapter: Target I2C bus segment
+ * @flags: I2C_LOCK_ADAPTER trylocks the root i2c adapter, I2C_LOCK_SEGMENT
+ *	trylocks only this branch in the adapter tree
  */
-static int i2c_trylock_adapter(struct i2c_adapter *adapter)
+static int i2c_adapter_trylock_bus(struct i2c_adapter *adapter, int flags)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
 
 	if (parent)
-		return i2c_trylock_adapter(parent);
+		return parent->trylock_bus(parent, flags);
 	else
 		return rt_mutex_trylock(&adapter->bus_lock);
 }
 
 /**
- * i2c_unlock_adapter - Release exclusive access to an I2C bus segment
+ * i2c_adapter_unlock_bus - Release exclusive access to an I2C bus segment
  * @adapter: Target I2C bus segment
+ * @flags: I2C_LOCK_ADAPTER unlocks the root i2c adapter, I2C_LOCK_SEGMENT
+ *	unlocks only this branch in the adapter tree
  */
-void i2c_unlock_adapter(struct i2c_adapter *adapter)
+static void i2c_adapter_unlock_bus(struct i2c_adapter *adapter, int flags)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
 
@@ -1001,7 +1006,6 @@  void i2c_unlock_adapter(struct i2c_adapter *adapter)
 	else
 		rt_mutex_unlock(&adapter->bus_lock);
 }
-EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
 
 static void i2c_dev_set_name(struct i2c_adapter *adap,
 			     struct i2c_client *client)
@@ -1547,6 +1551,12 @@  static int i2c_register_adapter(struct i2c_adapter *adap)
 		return -EINVAL;
 	}
 
+	if (!adap->lock_bus) {
+		adap->lock_bus = i2c_adapter_lock_bus;
+		adap->trylock_bus = i2c_adapter_trylock_bus;
+		adap->unlock_bus = i2c_adapter_unlock_bus;
+	}
+
 	rt_mutex_init(&adap->bus_lock);
 	mutex_init(&adap->userspace_clients_lock);
 	INIT_LIST_HEAD(&adap->userspace_clients);
@@ -2315,16 +2325,16 @@  int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 #endif
 
 		if (in_atomic() || irqs_disabled()) {
-			ret = i2c_trylock_adapter(adap);
+			ret = adap->trylock_bus(adap, I2C_LOCK_SEGMENT);
 			if (!ret)
 				/* I2C activity is ongoing. */
 				return -EAGAIN;
 		} else {
-			i2c_lock_adapter(adap);
+			i2c_lock_bus(adap, I2C_LOCK_SEGMENT);
 		}
 
 		ret = __i2c_transfer(adap, msgs, num);
-		i2c_unlock_adapter(adap);
+		i2c_unlock_bus(adap, I2C_LOCK_SEGMENT);
 
 		return ret;
 	} else {
@@ -3099,7 +3109,7 @@  s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
 	flags &= I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB;
 
 	if (adapter->algo->smbus_xfer) {
-		i2c_lock_adapter(adapter);
+		i2c_lock_bus(adapter, I2C_LOCK_SEGMENT);
 
 		/* Retry automatically on arbitration loss */
 		orig_jiffies = jiffies;
@@ -3113,7 +3123,7 @@  s32 i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr, unsigned short flags,
 				       orig_jiffies + adapter->timeout))
 				break;
 		}
-		i2c_unlock_adapter(adapter);
+		i2c_unlock_bus(adapter, I2C_LOCK_SEGMENT);
 
 		if (res != -EOPNOTSUPP || !adapter->algo->master_xfer)
 			goto trace;
@@ -3224,9 +3234,9 @@  int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
 
 	client->slave_cb = slave_cb;
 
-	i2c_lock_adapter(client->adapter);
+	i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
 	ret = client->adapter->algo->reg_slave(client);
-	i2c_unlock_adapter(client->adapter);
+	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
 
 	if (ret) {
 		client->slave_cb = NULL;
@@ -3246,9 +3256,9 @@  int i2c_slave_unregister(struct i2c_client *client)
 		return -EOPNOTSUPP;
 	}
 
-	i2c_lock_adapter(client->adapter);
+	i2c_lock_bus(client->adapter, I2C_LOCK_SEGMENT);
 	ret = client->adapter->algo->unreg_slave(client);
-	i2c_unlock_adapter(client->adapter);
+	i2c_unlock_bus(client->adapter, I2C_LOCK_SEGMENT);
 
 	if (ret == 0)
 		client->slave_cb = NULL;
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 200cf13b00f6..c5f79fec1bfb 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -538,6 +538,10 @@  struct i2c_adapter {
 
 	struct i2c_bus_recovery_info *bus_recovery_info;
 	const struct i2c_adapter_quirks *quirks;
+
+	void (*lock_bus)(struct i2c_adapter *, int flags);
+	int (*trylock_bus)(struct i2c_adapter *, int flags);
+	void (*unlock_bus)(struct i2c_adapter *, int flags);
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
@@ -567,8 +571,28 @@  i2c_parent_is_i2c_adapter(const struct i2c_adapter *adapter)
 int i2c_for_each_dev(void *data, int (*fn)(struct device *, void *));
 
 /* Adapter locking functions, exported for shared pin cases */
-void i2c_lock_adapter(struct i2c_adapter *);
-void i2c_unlock_adapter(struct i2c_adapter *);
+#define I2C_LOCK_ADAPTER 0x01
+#define I2C_LOCK_SEGMENT 0x02
+static inline void
+i2c_lock_bus(struct i2c_adapter *adapter, int flags)
+{
+	adapter->lock_bus(adapter, flags);
+}
+static inline void
+i2c_unlock_bus(struct i2c_adapter *adapter, int flags)
+{
+	adapter->unlock_bus(adapter, flags);
+}
+static inline void
+i2c_lock_adapter(struct i2c_adapter *adapter)
+{
+	i2c_lock_bus(adapter, I2C_LOCK_ADAPTER);
+}
+static inline void
+i2c_unlock_adapter(struct i2c_adapter *adapter)
+{
+	i2c_unlock_bus(adapter, I2C_LOCK_ADAPTER);
+}
 
 /*flags for the client struct: */
 #define I2C_CLIENT_PEC		0x04	/* Use Packet Error Checking */