linux-next: manual merge of the net tree with the i2c tree

linux-next: manual merge of the net tree with the i2c tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the net tree got a conflict in
drivers/net/sfc/sfe4001.c between commit
3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
inversion on top of bus lock") from the i2c tree and commit
c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
falcon_boards.c") from the net tree.

I have applied the following merge fixup patch (after removing
drivers/net/sfc/sfe4001.c) and can carry it as necessary.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 26 Oct 2009 12:24:55 +1100
Subject: [PATCH] net: merge fixup for drivers/net/sfc/falcon_boards.c

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/sfc/falcon_boards.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sfc/falcon_boards.c b/drivers/net/sfc/falcon_boards.c
index 99f7372..788b336 100644
--- a/drivers/net/sfc/falcon_boards.c
+++ b/drivers/net/sfc/falcon_boards.c
@@ -327,7 +327,7 @@ static int sfn4111t_reset(struct efx_nic *efx)
 	efx_oword_t reg;
 
 	/* GPIO 3 and the GPIO register are shared with I2C, so block that */
-	mutex_lock(&efx->i2c_adap.bus_lock);
+	rt_mutex_lock(&efx->i2c_adap.bus_lock);
 
 	/* Pull RST_N (GPIO 2) low then let it up again, setting the
 	 * FLASH_CFG_1 strap (GPIO 3) appropriately.  Only change the
@@ -343,7 +343,7 @@ static int sfn4111t_reset(struct efx_nic *efx)
 	efx_writeo(efx, &reg, FR_AB_GPIO_CTL);
 	msleep(1);
 
-	mutex_unlock(&efx->i2c_adap.bus_lock);
+	rt_mutex_unlock(&efx->i2c_adap.bus_lock);
 
 	ssleep(1);
 	return 0;
-- 
1.6.5

linux-next: manual merge of the net tree with the i2c tree

[Stephen Rothwell]

Hi all,

Today's linux-next merge of the net tree got a conflict in
drivers/net/sfc/sfe4001.c between commit
3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
inversion on top of bus lock") from the i2c tree and commit
c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
falcon_boards.c") from the net tree.

I have applied the following merge fixup patch (after removing
drivers/net/sfc/sfe4001.c) and can carry it as necessary.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 26 Oct 2009 12:24:55 +1100
Subject: [PATCH] net: merge fixup for drivers/net/sfc/falcon_boards.c

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/sfc/falcon_boards.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sfc/falcon_boards.c b/drivers/net/sfc/falcon_boards.c
index 99f7372..788b336 100644
--- a/drivers/net/sfc/falcon_boards.c
+++ b/drivers/net/sfc/falcon_boards.c
@@ -327,7 +327,7 @@ static int sfn4111t_reset(struct efx_nic *efx)
 	efx_oword_t reg;
 
 	/* GPIO 3 and the GPIO register are shared with I2C, so block that */
-	mutex_lock(&efx->i2c_adap.bus_lock);
+	rt_mutex_lock(&efx->i2c_adap.bus_lock);
 
 	/* Pull RST_N (GPIO 2) low then let it up again, setting the
 	 * FLASH_CFG_1 strap (GPIO 3) appropriately.  Only change the
@@ -343,7 +343,7 @@ static int sfn4111t_reset(struct efx_nic *efx)
 	efx_writeo(efx, &reg, FR_AB_GPIO_CTL);
 	msleep(1);
 
-	mutex_unlock(&efx->i2c_adap.bus_lock);
+	rt_mutex_unlock(&efx->i2c_adap.bus_lock);
 
 	ssleep(1);
 	return 0;
-- 
1.6.5

Re: linux-next: manual merge of the net tree with the i2c tree

[Ben Hutchings]

On Mon, 2009-10-26 at 13:37 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next merge of the net tree got a conflict in
> drivers/net/sfc/sfe4001.c between commit
> 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> inversion on top of bus lock") from the i2c tree and commit
> c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> falcon_boards.c") from the net tree.
> 
> I have applied the following merge fixup patch (after removing
> drivers/net/sfc/sfe4001.c) and can carry it as necessary.
> 
> -- 
> Cheers,
> Stephen Rothwell                    sfr@canb.auug.org.au
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Mon, 26 Oct 2009 12:24:55 +1100
> Subject: [PATCH] net: merge fixup for drivers/net/sfc/falcon_boards.c
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Shared i2c adapter locking (Was: linux-next: manual merge of the net tree with the i2c tree)

[Jean Delvare]

Hi Stephen,

On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> Today's linux-next merge of the net tree got a conflict in
> drivers/net/sfc/sfe4001.c between commit
> 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> inversion on top of bus lock") from the i2c tree and commit
> c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> falcon_boards.c") from the net tree.
> 
> I have applied the following merge fixup patch (after removing
> drivers/net/sfc/sfe4001.c) and can carry it as necessary.

Thanks for fixing it. The core problem here IMHO is that the sfc
network driver touches i2c internals which it would rather leave alone.
This is the only driver I know of which does this.

I can think of 3 different ways to address the issue.

Method #1: add a public API to grab/release an I2C segment.

void i2c_adapter_lock(struct i2c_adapter *adapter)
{
	rt_mutex_lock(&adapter->bus_lock);
}

void i2c_adapter_unlock(struct i2c_adapter *adapter)
{
	rt_mutex_unlock(&adapter->bus_lock);
}

It has the advantage of simplicity. The problem is that it is not
symmetric. Whatever shares the pins with I2C, I2C is considered the
main user in that its mutex is used to guarantee mutual exclusion. If
the other subsystem also has a mandatory locking mechanism, there will
have to be a decision on who is locked first. If not all drivers agree,
lockdep will get confused.

Method #2: let the driver implement its own "main" locking, and have
all pin users (i2c etc.) take it in addition to any subsystem-specific
mutex. As far as the sfc network driver is concerned, that would mean
adding pre-xfer and post-xfer hooks to i2c-algo-bit, where the "main"
mutex in question would be taken resp. released.

It has the advantage of symmetry. The problem is performance, as
regular operation will require to take and release two mutexes instead
of just one. But it is a fairly generic approach which could solve
other issues too.

Method #3: let individual bus drivers implement segment locking
themselves, with custom locking/unlocking hooks. This way, a device
sharing pins between several functions can have its own mutex
protecting these pins globally, and all user subsystems (i2c etc.) use
this single mutex.

It has the advantage of performance and symmetry, at the price of
fragility. If the i2c bus driver implements locking improperly... I am
also worried by inconsistencies that may arise, if different i2c
adapters are protected by different mutex types (mutex vs. real-time
mutex vs. spinlock etc.) but maybe this will be seen as an advantage
rather than a problem.

I'm not really sure if I have a preference yet, so please speak up if
you do.

-- 
Jean Delvare

Shared i2c adapter locking (Was: linux-next: manual merge of the net tree with the i2c tree)

[Jean Delvare]

Hi Stephen,

On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> Today's linux-next merge of the net tree got a conflict in
> drivers/net/sfc/sfe4001.c between commit
> 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> inversion on top of bus lock") from the i2c tree and commit
> c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> falcon_boards.c") from the net tree.
> 
> I have applied the following merge fixup patch (after removing
> drivers/net/sfc/sfe4001.c) and can carry it as necessary.

Thanks for fixing it. The core problem here IMHO is that the sfc
network driver touches i2c internals which it would rather leave alone.
This is the only driver I know of which does this.

I can think of 3 different ways to address the issue.

Method #1: add a public API to grab/release an I2C segment.

void i2c_adapter_lock(struct i2c_adapter *adapter)
{
	rt_mutex_lock(&adapter->bus_lock);
}

void i2c_adapter_unlock(struct i2c_adapter *adapter)
{
	rt_mutex_unlock(&adapter->bus_lock);
}

It has the advantage of simplicity. The problem is that it is not
symmetric. Whatever shares the pins with I2C, I2C is considered the
main user in that its mutex is used to guarantee mutual exclusion. If
the other subsystem also has a mandatory locking mechanism, there will
have to be a decision on who is locked first. If not all drivers agree,
lockdep will get confused.

Method #2: let the driver implement its own "main" locking, and have
all pin users (i2c etc.) take it in addition to any subsystem-specific
mutex. As far as the sfc network driver is concerned, that would mean
adding pre-xfer and post-xfer hooks to i2c-algo-bit, where the "main"
mutex in question would be taken resp. released.

It has the advantage of symmetry. The problem is performance, as
regular operation will require to take and release two mutexes instead
of just one. But it is a fairly generic approach which could solve
other issues too.

Method #3: let individual bus drivers implement segment locking
themselves, with custom locking/unlocking hooks. This way, a device
sharing pins between several functions can have its own mutex
protecting these pins globally, and all user subsystems (i2c etc.) use
this single mutex.

It has the advantage of performance and symmetry, at the price of
fragility. If the i2c bus driver implements locking improperly... I am
also worried by inconsistencies that may arise, if different i2c
adapters are protected by different mutex types (mutex vs. real-time
mutex vs. spinlock etc.) but maybe this will be seen as an advantage
rather than a problem.

I'm not really sure if I have a preference yet, so please speak up if
you do.

-- 
Jean Delvare

Re: Shared i2c adapter locking (Was: linux-next: manual merge of the net tree with the i2c tree)

[Ben Hutchings]

On Thu, 2009-10-29 at 15:43 +0100, Jean Delvare wrote:
> Hi Stephen,
> 
> On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> > Today's linux-next merge of the net tree got a conflict in
> > drivers/net/sfc/sfe4001.c between commit
> > 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> > inversion on top of bus lock") from the i2c tree and commit
> > c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> > falcon_boards.c") from the net tree.
> > 
> > I have applied the following merge fixup patch (after removing
> > drivers/net/sfc/sfe4001.c) and can carry it as necessary.
> 
> Thanks for fixing it. The core problem here IMHO is that the sfc
> network driver touches i2c internals which it would rather leave alone.

I'm just a little proud of having the idea that we could avoid using an
I/O-expander on this board, but yes, the software side of this
multiplexing is a hack.

> This is the only driver I know of which does this.
> 
> I can think of 3 different ways to address the issue.
> 
> Method #1: add a public API to grab/release an I2C segment.
> 
> void i2c_adapter_lock(struct i2c_adapter *adapter)
> {
> 	rt_mutex_lock(&adapter->bus_lock);
> }
> 
> void i2c_adapter_unlock(struct i2c_adapter *adapter)
> {
> 	rt_mutex_unlock(&adapter->bus_lock);
> }
[...]
> I'm not really sure if I have a preference yet, so please speak up if
> you do.

Indirect lock operations are a recipe for deadlock, and there doesn't
seem to be any other user for this, so method 1 seems best.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: Shared i2c adapter locking

[Jean Delvare]

Hi Ben,

On Thu, 29 Oct 2009 15:09:36 +0000, Ben Hutchings wrote:
> On Thu, 2009-10-29 at 15:43 +0100, Jean Delvare wrote:
> > Hi Stephen,
> > 
> > On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> > > Today's linux-next merge of the net tree got a conflict in
> > > drivers/net/sfc/sfe4001.c between commit
> > > 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> > > inversion on top of bus lock") from the i2c tree and commit
> > > c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> > > falcon_boards.c") from the net tree.
> > > 
> > > I have applied the following merge fixup patch (after removing
> > > drivers/net/sfc/sfe4001.c) and can carry it as necessary.
> > 
> > Thanks for fixing it. The core problem here IMHO is that the sfc
> > network driver touches i2c internals which it would rather leave alone.
> 
> I'm just a little proud of having the idea that we could avoid using an
> I/O-expander on this board, but yes, the software side of this
> multiplexing is a hack.
> 
> > This is the only driver I know of which does this.
> > 
> > I can think of 3 different ways to address the issue.
> > 
> > Method #1: add a public API to grab/release an I2C segment.
> > 
> > void i2c_adapter_lock(struct i2c_adapter *adapter)
> > {
> > 	rt_mutex_lock(&adapter->bus_lock);
> > }
> > 
> > void i2c_adapter_unlock(struct i2c_adapter *adapter)
> > {
> > 	rt_mutex_unlock(&adapter->bus_lock);
> > }
> [...]
> > I'm not really sure if I have a preference yet, so please speak up if
> > you do.
> 
> Indirect lock operations are a recipe for deadlock, and there doesn't
> seem to be any other user for this, so method 1 seems best.

Well, all 3 methods rely on indirect lock operations to some degree.
But I am fine with method #1 for now. We can always move to something
more complex if the need ever arises.

What about the following patch?

From: Jean Delvare <khali@linux-fr.org>
Subject: i2c: Add an interface to lock/unlock I2C bus segment

Some drivers need to be able to prevent access to an I2C bus segment
for a specific period of time. Add an interface for them to do so
without twiddling with i2c-core internals.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/sfe4001.c |    4 ++--
 include/linux/i2c.h       |   18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

--- linux-2.6.32-rc6.orig/drivers/net/sfc/sfe4001.c	2009-11-05 10:51:56.000000000 +0100
+++ linux-2.6.32-rc6/drivers/net/sfc/sfe4001.c	2009-11-05 13:40:17.000000000 +0100
@@ -188,7 +188,7 @@ static int sfn4111t_reset(struct efx_nic
 	efx_oword_t reg;
 
 	/* GPIO 3 and the GPIO register are shared with I2C, so block that */
-	mutex_lock(&efx->i2c_adap.bus_lock);
+	i2c_lock_adapter(&efx->i2c_adap);
 
 	/* Pull RST_N (GPIO 2) low then let it up again, setting the
 	 * FLASH_CFG_1 strap (GPIO 3) appropriately.  Only change the
@@ -204,7 +204,7 @@ static int sfn4111t_reset(struct efx_nic
 	falcon_write(efx, &reg, GPIO_CTL_REG_KER);
 	msleep(1);
 
-	mutex_unlock(&efx->i2c_adap.bus_lock);
+	i2c_unlock_adapter(&efx->i2c_adap);
 
 	ssleep(1);
 	return 0;
--- linux-2.6.32-rc6.orig/include/linux/i2c.h	2009-11-05 10:51:56.000000000 +0100
+++ linux-2.6.32-rc6/include/linux/i2c.h	2009-11-05 14:03:53.000000000 +0100
@@ -361,6 +361,24 @@ static inline void i2c_set_adapdata(stru
 	dev_set_drvdata(&dev->dev, data);
 }
 
+/**
+ * i2c_lock_adapter - Prevent access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline void i2c_lock_adapter(struct i2c_adapter *adapter)
+{
+	mutex_lock(&adapter->bus_lock);
+}
+
+/**
+ * i2c_unlock_adapter - Reauthorize access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline void i2c_unlock_adapter(struct i2c_adapter *adapter)
+{
+	mutex_unlock(&adapter->bus_lock);
+}
+
 /*flags for the client struct: */
 #define I2C_CLIENT_PEC	0x04		/* Use Packet Error Checking */
 #define I2C_CLIENT_TEN	0x10		/* we have a ten bit chip address */


-- 
Jean Delvare

Re: Shared i2c adapter locking

[Jean Delvare]

Hi Ben,

On Thu, 29 Oct 2009 15:09:36 +0000, Ben Hutchings wrote:
> On Thu, 2009-10-29 at 15:43 +0100, Jean Delvare wrote:
> > Hi Stephen,
> > 
> > On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> > > Today's linux-next merge of the net tree got a conflict in
> > > drivers/net/sfc/sfe4001.c between commit
> > > 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> > > inversion on top of bus lock") from the i2c tree and commit
> > > c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> > > falcon_boards.c") from the net tree.
> > > 
> > > I have applied the following merge fixup patch (after removing
> > > drivers/net/sfc/sfe4001.c) and can carry it as necessary.
> > 
> > Thanks for fixing it. The core problem here IMHO is that the sfc
> > network driver touches i2c internals which it would rather leave alone.
> 
> I'm just a little proud of having the idea that we could avoid using an
> I/O-expander on this board, but yes, the software side of this
> multiplexing is a hack.
> 
> > This is the only driver I know of which does this.
> > 
> > I can think of 3 different ways to address the issue.
> > 
> > Method #1: add a public API to grab/release an I2C segment.
> > 
> > void i2c_adapter_lock(struct i2c_adapter *adapter)
> > {
> > 	rt_mutex_lock(&adapter->bus_lock);
> > }
> > 
> > void i2c_adapter_unlock(struct i2c_adapter *adapter)
> > {
> > 	rt_mutex_unlock(&adapter->bus_lock);
> > }
> [...]
> > I'm not really sure if I have a preference yet, so please speak up if
> > you do.
> 
> Indirect lock operations are a recipe for deadlock, and there doesn't
> seem to be any other user for this, so method 1 seems best.

Well, all 3 methods rely on indirect lock operations to some degree.
But I am fine with method #1 for now. We can always move to something
more complex if the need ever arises.

What about the following patch?

From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Subject: i2c: Add an interface to lock/unlock I2C bus segment

Some drivers need to be able to prevent access to an I2C bus segment
for a specific period of time. Add an interface for them to do so
without twiddling with i2c-core internals.

Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
Cc: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
---
 drivers/net/sfc/sfe4001.c |    4 ++--
 include/linux/i2c.h       |   18 ++++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

--- linux-2.6.32-rc6.orig/drivers/net/sfc/sfe4001.c	2009-11-05 10:51:56.000000000 +0100
+++ linux-2.6.32-rc6/drivers/net/sfc/sfe4001.c	2009-11-05 13:40:17.000000000 +0100
@@ -188,7 +188,7 @@ static int sfn4111t_reset(struct efx_nic
 	efx_oword_t reg;
 
 	/* GPIO 3 and the GPIO register are shared with I2C, so block that */
-	mutex_lock(&efx->i2c_adap.bus_lock);
+	i2c_lock_adapter(&efx->i2c_adap);
 
 	/* Pull RST_N (GPIO 2) low then let it up again, setting the
 	 * FLASH_CFG_1 strap (GPIO 3) appropriately.  Only change the
@@ -204,7 +204,7 @@ static int sfn4111t_reset(struct efx_nic
 	falcon_write(efx, &reg, GPIO_CTL_REG_KER);
 	msleep(1);
 
-	mutex_unlock(&efx->i2c_adap.bus_lock);
+	i2c_unlock_adapter(&efx->i2c_adap);
 
 	ssleep(1);
 	return 0;
--- linux-2.6.32-rc6.orig/include/linux/i2c.h	2009-11-05 10:51:56.000000000 +0100
+++ linux-2.6.32-rc6/include/linux/i2c.h	2009-11-05 14:03:53.000000000 +0100
@@ -361,6 +361,24 @@ static inline void i2c_set_adapdata(stru
 	dev_set_drvdata(&dev->dev, data);
 }
 
+/**
+ * i2c_lock_adapter - Prevent access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline void i2c_lock_adapter(struct i2c_adapter *adapter)
+{
+	mutex_lock(&adapter->bus_lock);
+}
+
+/**
+ * i2c_unlock_adapter - Reauthorize access to an I2C bus segment
+ * @adapter: Target I2C bus segment
+ */
+static inline void i2c_unlock_adapter(struct i2c_adapter *adapter)
+{
+	mutex_unlock(&adapter->bus_lock);
+}
+
 /*flags for the client struct: */
 #define I2C_CLIENT_PEC	0x04		/* Use Packet Error Checking */
 #define I2C_CLIENT_TEN	0x10		/* we have a ten bit chip address */


-- 
Jean Delvare

Re: Shared i2c adapter locking

[Ben Hutchings]

On Thu, 2009-11-05 at 14:11 +0100, Jean Delvare wrote:
[...]
> What about the following patch?
> 
> From: Jean Delvare <khali@linux-fr.org>
> Subject: i2c: Add an interface to lock/unlock I2C bus segment
> 
> Some drivers need to be able to prevent access to an I2C bus segment
> for a specific period of time. Add an interface for them to do so
> without twiddling with i2c-core internals.
> 
> Signed-off-by: Jean Delvare <khali@linux-fr.org>
> Cc: Ben Hutchings <bhutchings@solarflare.com>
Acked-by: Ben Hutchings <bhutchings@solarflare.com>

Presumably this is meant for net-next-2.6, and you'll implement
i2c_{lock,unlock}_adapter() using rt_mutex in your i2c tree?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: Shared i2c adapter locking

[Jean Delvare]

On Thu, 05 Nov 2009 13:57:24 +0000, Ben Hutchings wrote:
> On Thu, 2009-11-05 at 14:11 +0100, Jean Delvare wrote:
> [...]
> > What about the following patch?
> > 
> > From: Jean Delvare <khali@linux-fr.org>
> > Subject: i2c: Add an interface to lock/unlock I2C bus segment
> > 
> > Some drivers need to be able to prevent access to an I2C bus segment
> > for a specific period of time. Add an interface for them to do so
> > without twiddling with i2c-core internals.
> > 
> > Signed-off-by: Jean Delvare <khali@linux-fr.org>
> > Cc: Ben Hutchings <bhutchings@solarflare.com>
> Acked-by: Ben Hutchings <bhutchings@solarflare.com>
> 
> Presumably this is meant for net-next-2.6, and you'll implement

Actually I meant to push this to Linus immediately, through my i2c
tree. This is essentially a no-op: the binary code will be the same as
before the patch, so guaranteed to be safe, and this will solve
conflicts in linux-next.

> i2c_{lock,unlock}_adapter() using rt_mutex in your i2c tree?

Correct.

-- 
Jean Delvare

Re: Shared i2c adapter locking

[Jean Delvare]

On Thu, 05 Nov 2009 13:57:24 +0000, Ben Hutchings wrote:
> On Thu, 2009-11-05 at 14:11 +0100, Jean Delvare wrote:
> [...]
> > What about the following patch?
> > 
> > From: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Subject: i2c: Add an interface to lock/unlock I2C bus segment
> > 
> > Some drivers need to be able to prevent access to an I2C bus segment
> > for a specific period of time. Add an interface for them to do so
> > without twiddling with i2c-core internals.
> > 
> > Signed-off-by: Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org>
> > Cc: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
> Acked-by: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>
> 
> Presumably this is meant for net-next-2.6, and you'll implement

Actually I meant to push this to Linus immediately, through my i2c
tree. This is essentially a no-op: the binary code will be the same as
before the patch, so guaranteed to be safe, and this will solve
conflicts in linux-next.

> i2c_{lock,unlock}_adapter() using rt_mutex in your i2c tree?

Correct.

-- 
Jean Delvare

Re: linux-next: manual merge of the net tree with the i2c tree

[Jean Delvare]

Hi Stephen, Ben,

On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> Today's linux-next merge of the net tree got a conflict in
> drivers/net/sfc/sfe4001.c between commit
> 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> inversion on top of bus lock") from the i2c tree and commit
> c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> falcon_boards.c") from the net tree.
> 
> I have applied the following merge fixup patch (after removing
> drivers/net/sfc/sfe4001.c) and can carry it as necessary.

I've merged the new API to get and release the i2c_adapter mutex:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=afa08974fe80c198b8650f73ed8ab59135ca10d0

Ben, you can adjust your own patches to make use of this API instead of
accessing the i2c_adapter mutex directly. That way, you are no longer
dependent of implementation changes, and this should solve the conflict.

Stephen, you can then drop your fixup patch.

Thanks,
-- 
Jean Delvare

Re: linux-next: manual merge of the net tree with the i2c tree

[Jean Delvare]

Hi Stephen, Ben,

On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> Today's linux-next merge of the net tree got a conflict in
> drivers/net/sfc/sfe4001.c between commit
> 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> inversion on top of bus lock") from the i2c tree and commit
> c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> falcon_boards.c") from the net tree.
> 
> I have applied the following merge fixup patch (after removing
> drivers/net/sfc/sfe4001.c) and can carry it as necessary.

I've merged the new API to get and release the i2c_adapter mutex:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=afa08974fe80c198b8650f73ed8ab59135ca10d0

Ben, you can adjust your own patches to make use of this API instead of
accessing the i2c_adapter mutex directly. That way, you are no longer
dependent of implementation changes, and this should solve the conflict.

Stephen, you can then drop your fixup patch.

Thanks,
-- 
Jean Delvare

Re: linux-next: manual merge of the net tree with the i2c tree

[Ben Hutchings]

On Tue, 2009-11-10 at 12:42 +0100, Jean Delvare wrote:
> Hi Stephen, Ben,
> 
> On Mon, 26 Oct 2009 13:37:57 +1100, Stephen Rothwell wrote:
> > Today's linux-next merge of the net tree got a conflict in
> > drivers/net/sfc/sfe4001.c between commit
> > 3f7c0648f727a6d5baf6117653e4001dc877b90b ("i2c: Prevent priority
> > inversion on top of bus lock") from the i2c tree and commit
> > c9597d4f89565b6562bd3026adbe6eac6c317f47 ("sfc: Merge sfe4001.c into
> > falcon_boards.c") from the net tree.
> > 
> > I have applied the following merge fixup patch (after removing
> > drivers/net/sfc/sfe4001.c) and can carry it as necessary.
> 
> I've merged the new API to get and release the i2c_adapter mutex:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=afa08974fe80c198b8650f73ed8ab59135ca10d0

Thanks, Jean.

> Ben, you can adjust your own patches to make use of this API instead of
> accessing the i2c_adapter mutex directly. That way, you are no longer
> dependent of implementation changes, and this should solve the conflict.
> 
> Stephen, you can then drop your fixup patch.

I don't think so, since the conflict resulted from joining two files
including sfe4001.c in net-next-2.6.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: linux-next: manual merge of the net tree with the i2c tree

[Jean Delvare]

On Tue, 10 Nov 2009 13:22:58 +0000, Ben Hutchings wrote:
> On Tue, 2009-11-10 at 12:42 +0100, Jean Delvare wrote:
> > Ben, you can adjust your own patches to make use of this API instead of
> > accessing the i2c_adapter mutex directly. That way, you are no longer
> > dependent of implementation changes, and this should solve the conflict.
> > 
> > Stephen, you can then drop your fixup patch.
> 
> I don't think so, since the conflict resulted from joining two files
> including sfe4001.c in net-next-2.6.

My patch series no longer touches sfe4001.c, so how would the conflict
remain?

-- 
Jean Delvare

Re: linux-next: manual merge of the net tree with the i2c tree

[Ben Hutchings]

On Tue, 2009-11-10 at 16:02 +0100, Jean Delvare wrote:
> On Tue, 10 Nov 2009 13:22:58 +0000, Ben Hutchings wrote:
> > On Tue, 2009-11-10 at 12:42 +0100, Jean Delvare wrote:
> > > Ben, you can adjust your own patches to make use of this API instead of
> > > accessing the i2c_adapter mutex directly. That way, you are no longer
> > > dependent of implementation changes, and this should solve the conflict.
> > > 
> > > Stephen, you can then drop your fixup patch.
> > 
> > I don't think so, since the conflict resulted from joining two files
> > including sfe4001.c in net-next-2.6.
> 
> My patch series no longer touches sfe4001.c, so how would the conflict
> remain?

Because your patch to introduce i2c_{lock,unlock}_adapter() are not in
net-next-2.6 yet.

David, you might want to pull from Linus and resolve this, giving
Stephen a break.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: Shared i2c adapter locking

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

Hi Jean,

On Thu, 5 Nov 2009 15:07:15 +0100 Jean Delvare <khali@linux-fr.org> wrote:
>
> > Presumably this is meant for net-next-2.6, and you'll implement
> 
> Actually I meant to push this to Linus immediately, through my i2c
> tree. This is essentially a no-op: the binary code will be the same as
> before the patch, so guaranteed to be safe, and this will solve
> conflicts in linux-next.

Any word on this?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: Shared i2c adapter locking

[Jean Delvare]

Hi Stephen,

On Tue, 17 Nov 2009 19:33:13 +1100, Stephen Rothwell wrote:
> Hi Jean,
> 
> On Thu, 5 Nov 2009 15:07:15 +0100 Jean Delvare <khali@linux-fr.org> wrote:
> >
> > > Presumably this is meant for net-next-2.6, and you'll implement
> > 
> > Actually I meant to push this to Linus immediately, through my i2c
> > tree. This is essentially a no-op: the binary code will be the same as
> > before the patch, so guaranteed to be safe, and this will solve
> > conflicts in linux-next.
> 
> Any word on this?

Haven't you read this:
http://www.spinics.net/lists/linux-next/msg07839.html

and the 3 following posts?

If you did and something is still not clear, please let me know. My
understanding is that the action token is in David's hands now.

-- 
Jean Delvare

Re: Shared i2c adapter locking

[Jean Delvare]

Hi Stephen,

On Tue, 17 Nov 2009 19:33:13 +1100, Stephen Rothwell wrote:
> Hi Jean,
> 
> On Thu, 5 Nov 2009 15:07:15 +0100 Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
> >
> > > Presumably this is meant for net-next-2.6, and you'll implement
> > 
> > Actually I meant to push this to Linus immediately, through my i2c
> > tree. This is essentially a no-op: the binary code will be the same as
> > before the patch, so guaranteed to be safe, and this will solve
> > conflicts in linux-next.
> 
> Any word on this?

Haven't you read this:
http://www.spinics.net/lists/linux-next/msg07839.html

and the 3 following posts?

If you did and something is still not clear, please let me know. My
understanding is that the action token is in David's hands now.

-- 
Jean Delvare

Re: Shared i2c adapter locking

[David Miller]

From: Jean Delvare <khali@linux-fr.org>
Date: Tue, 17 Nov 2009 10:35:54 +0100

> If you did and something is still not clear, please let me know. My
> understanding is that the action token is in David's hands now.

So what exactly do I need to do?

All I see that needs to happen is to pull from Linus's tree into
net-2.6

But frankly I don't see how that helps resolve anything in linux-next
since Stephen starts with Linus's tree then pulls in everyone's work.

Or is that pull necessary so that some other patch can be applied
to net-2.6 or similar?

Re: Shared i2c adapter locking

[Ben Hutchings]

On Tue, 2009-11-17 at 03:51 -0800, David Miller wrote:
> From: Jean Delvare <khali@linux-fr.org>
> Date: Tue, 17 Nov 2009 10:35:54 +0100
> 
> > If you did and something is still not clear, please let me know. My
> > understanding is that the action token is in David's hands now.
> 
> So what exactly do I need to do?
> 
> All I see that needs to happen is to pull from Linus's tree into
> net-2.6
> 
> But frankly I don't see how that helps resolve anything in linux-next
> since Stephen starts with Linus's tree then pulls in everyone's work.
> 
> Or is that pull necessary so that some other patch can be applied
> to net-2.6 or similar?

There is now a conflict between this in Linus's tree:

commit afa08974fe80c198b8650f73ed8ab59135ca10d0
Author: Jean Delvare <khali@linux-fr.org>
Date:   Sat Nov 7 13:10:46 2009 +0100

    i2c: Add an interface to lock/unlock an I2C bus segment

and this in net-next-2.6:

commit c9597d4f89565b6562bd3026adbe6eac6c317f47
Author: Ben Hutchings <bhutchings@solarflare.com>
Date:   Fri Oct 23 08:29:33 2009 +0000

    sfc: Merge sfe4001.c into falcon_boards.c

You will need to merge Linus's tree into net-next-2.6 and resolve the
conflict by applying Jean's changes to drivers/net/sfc/falcon_boards.c.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: Shared i2c adapter locking

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Tue, 17 Nov 2009 13:32:54 +0000

> You will need to merge Linus's tree into net-next-2.6 and resolve the
> conflict by applying Jean's changes to drivers/net/sfc/falcon_boards.c.

Ok, I'll sort this out after I next push to Linus, thanks!

Re: Shared i2c adapter locking

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

Hi Jean,

On Tue, 17 Nov 2009 10:35:54 +0100 Jean Delvare <khali@linux-fr.org> wrote:
>
> Haven't you read this:
> http://www.spinics.net/lists/linux-next/msg07839.html
> 
> and the 3 following posts?

Somehow I missed that, sorry :-(

> If you did and something is still not clear, please let me know. My
> understanding is that the action token is in David's hands now.

OK, I will await developments :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: Shared i2c adapter locking

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]

Hi Jean,

On Tue, 17 Nov 2009 10:35:54 +0100 Jean Delvare <khali-PUYAD+kWke1g9hUCZPvPmw@public.gmane.org> wrote:
>
> Haven't you read this:
> http://www.spinics.net/lists/linux-next/msg07839.html
> 
> and the 3 following posts?

Somehow I missed that, sorry :-(

> If you did and something is still not clear, please let me know. My
> understanding is that the action token is in David's hands now.

OK, I will await developments :-)

-- 
Cheers,
Stephen Rothwell                    sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: Shared i2c adapter locking

[David Miller]

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 18 Nov 2009 07:59:17 +1100

>> If you did and something is still not clear, please let me know. My
>> understanding is that the action token is in David's hands now.
> 
> OK, I will await developments :-)

I just took care of all of this.

Thanks.

Re: Shared i2c adapter locking

[David Miller]

From: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
Date: Wed, 18 Nov 2009 07:59:17 +1100

>> If you did and something is still not clear, please let me know. My
>> understanding is that the action token is in David's hands now.
> 
> OK, I will await developments :-)

I just took care of all of this.

Thanks.

Re: Shared i2c adapter locking

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 553 bytes --]

Hi Dave,

On Wed, 18 Nov 2009 22:53:32 -0800 (PST) David Miller <davem@davemloft.net> wrote:
>
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 18 Nov 2009 07:59:17 +1100
> 
> >> If you did and something is still not clear, please let me know. My
> >> understanding is that the action token is in David's hands now.
> > 
> > OK, I will await developments :-)
> 
> I just took care of all of this.

Excellent, thanks.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]