| Message ID | 1359968595-21291-1-git-send-email-yuvaraj.cd@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Below are the links on multi master bus arbitration where it has been discussing. http://comments.gmane.org/gmane.linux.kernel/1410276 http://www.kernelhub.org/?msg=179505&p=2 On Mon, Feb 4, 2013 at 2:33 PM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: > This RFC patch is w.r.t multimaster bus arbitration which is already > being discussing in the mainline. > This patch provides hooks for the i2c multimaster bus arbitration > and to have the arbitration parameters. > > Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com> > --- > drivers/i2c/i2c-core.c | 13 ++++++++++++- > include/linux/i2c.h | 12 ++++++++++++ > 2 files changed, 24 insertions(+), 1 deletion(-) > > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c > index e388590..ed89fc8 100644 > --- a/drivers/i2c/i2c-core.c > +++ b/drivers/i2c/i2c-core.c > @@ -1408,18 +1408,29 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) > (msgs[ret].flags & I2C_M_RECV_LEN) ? "+" : ""); > } > #endif > + if (adap->mm_arbit_algo) { > + ret = adap->mm_arbit_algo->i2c_mm_bus_get(adap); > + if (ret) > + /* I2C bus under control of another master. */ > + return -EAGAIN; > + } > > if (in_atomic() || irqs_disabled()) { > ret = i2c_trylock_adapter(adap); > - if (!ret) > + if (!ret && adap->mm_arbit_algo) { > + adap->mm_arbit_algo->i2c_mm_bus_release(adap); > + > /* I2C activity is ongoing. */ > return -EAGAIN; > + } > } else { > i2c_lock_adapter(adap); > } > > ret = __i2c_transfer(adap, msgs, num); > i2c_unlock_adapter(adap); > + if (adap->mm_arbit_algo) > + adap->mm_arbit_algo->i2c_mm_bus_release(adap); > > return ret; > } else { > diff --git a/include/linux/i2c.h b/include/linux/i2c.h > index d0c4db7..61fbfe3 100644 > --- a/include/linux/i2c.h > +++ b/include/linux/i2c.h > @@ -371,6 +371,17 @@ struct i2c_algorithm { > }; > > /* > + *This struct provides hooks for i2c multi-master arbitration needs. > + */ > +struct i2c_mm_arbitration { > + void *arbitration_data; > + > + /* Should return 0 if mastership could be successfully established */ > + int (*i2c_mm_bus_get)(struct i2c_adapter *adap); > + void (*i2c_mm_bus_release)(struct i2c_adapter *adap); > +}; > + > +/* > * i2c_adapter is the structure used to identify a physical i2c bus along > * with the access algorithms necessary to access it. > */ > @@ -393,6 +404,7 @@ struct i2c_adapter { > > struct mutex userspace_clients_lock; > struct list_head userspace_clients; > + struct i2c_mm_arbitration *mm_arbit_algo; > }; > #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev) > > -- > 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Yavaraj, On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> wrote: > > This RFC patch is w.r.t multimaster bus arbitration which is already > being discussing in the mainline. > This patch provides hooks for the i2c multimaster bus arbitration > and to have the arbitration parameters. I may have missed something in all the threads, but my recollection was that the request was that this should be implemented without touching the i2c core code. Your patch modifies the i2c core code. My impression was that the best solution was to use the infrastructure in place for i2c multiplexing. ...but in your case you would only multiplex one thing. This was suggested by Grant Likely and seems the cleanest... [apologies for double-email; darn gmail and HTML email] -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 6, 2013 at 5:48 AM, Doug Anderson <dianders@chromium.org> wrote: > Yavaraj, > > On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> > wrote: >> >> This RFC patch is w.r.t multimaster bus arbitration which is already >> being discussing in the mainline. >> This patch provides hooks for the i2c multimaster bus arbitration >> and to have the arbitration parameters. > > > I may have missed something in all the threads, but my recollection was that > the request was that this should be implemented without touching the i2c > core code. Your patch modifies the i2c core code. Different i2c devices will directly talk to i2c core using i2c_transfer(). If we put this out of the i2c core,the i2c devices like tpsxxxx and etc should be registered through the bridge/mux . > > My impression was that the best solution was to use the infrastructure in > place for i2c multiplexing. ...but in your case you would only multiplex > one thing. This was suggested by Grant Likely and seems the cleanest... I agree that Grants idea was the cleanest but we may end up in change in > > -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 6, 2013 at 10:58 AM, Yuvaraj Kumar <yuvaraj.cd@gmail.com> wrote: > On Wed, Feb 6, 2013 at 5:48 AM, Doug Anderson <dianders@chromium.org> wrote: >> Yavaraj, >> >> On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> >> wrote: >>> >>> This RFC patch is w.r.t multimaster bus arbitration which is already >>> being discussing in the mainline. >>> This patch provides hooks for the i2c multimaster bus arbitration >>> and to have the arbitration parameters. >> >> >> I may have missed something in all the threads, but my recollection was that >> the request was that this should be implemented without touching the i2c >> core code. Your patch modifies the i2c core code. Different i2c devices will directly talk to i2c core using i2c_transfer(). If we put this out of the i2c core,the i2c devices like tpsxxxx and etc should be registered through the bridge/mux . >> >> My impression was that the best solution was to use the infrastructure in >> place for i2c multiplexing. ...but in your case you would only multiplex >> one thing. This was suggested by Grant Likely and seems the cleanest... > I agree that Grants idea was the cleanest but we may end up in change in i2c device probe to register through mux/bridge.(for the device's connected on that particular bus which requires arbitration). > >> >> -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Feb 5, 2013 at 9:33 PM, Yuvaraj Kumar <yuvaraj.cd@gmail.com> wrote: > On Wed, Feb 6, 2013 at 10:58 AM, Yuvaraj Kumar <yuvaraj.cd@gmail.com> wrote: >> On Wed, Feb 6, 2013 at 5:48 AM, Doug Anderson <dianders@chromium.org> wrote: >>> Yavaraj, >>> >>> On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd@gmail.com> >>> wrote: >>>> >>>> This RFC patch is w.r.t multimaster bus arbitration which is already >>>> being discussing in the mainline. >>>> This patch provides hooks for the i2c multimaster bus arbitration >>>> and to have the arbitration parameters. >>> >>> >>> I may have missed something in all the threads, but my recollection was that >>> the request was that this should be implemented without touching the i2c >>> core code. Your patch modifies the i2c core code. > > Different i2c devices will directly talk to i2c core using i2c_transfer(). > If we put this out of the i2c core,the i2c devices like tpsxxxx and etc > should be registered through the bridge/mux . > >>> >>> My impression was that the best solution was to use the infrastructure in >>> place for i2c multiplexing. ...but in your case you would only multiplex >>> one thing. This was suggested by Grant Likely and seems the cleanest... >> > I agree that Grants idea was the cleanest but we may end up in change in > i2c device probe to register through mux/bridge.(for the > device's connected on that > particular bus which requires arbitration). Does the device tree not handle that case automatically? How are muxes/bridges done in that case? It's not problem to adjust the device tree file accordingly if Grant's approach is the cleanest method. Regards, Simon >> >>> >>> -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 04, 2013 at 02:33:15PM +0530, Yuvaraj Kumar C D wrote: > This RFC patch is w.r.t multimaster bus arbitration which is already > being discussing in the mainline. > This patch provides hooks for the i2c multimaster bus arbitration > and to have the arbitration parameters. I still haven't understood why the arbitration specified in the I2C standard is not enough for you. Or what you would need to make use of it. Thanks, Wolfram -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Wolfram, On Sun, Feb 10, 2013 at 10:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > On Mon, Feb 04, 2013 at 02:33:15PM +0530, Yuvaraj Kumar C D wrote: >> This RFC patch is w.r.t multimaster bus arbitration which is already >> being discussing in the mainline. >> This patch provides hooks for the i2c multimaster bus arbitration >> and to have the arbitration parameters. > > I still haven't understood why the arbitration specified in the I2C > standard is not enough for you. Or what you would need to make use of > it. At the moment perhaps the strongest argument for why this particular arbitration scheme is needed is that's what the EC (embedded controller) on the ARM Chromebook uses. There have been several arguments in-house about whether this was the most ideal way to structure things, but that's what we shipped with. Thus, if we want to be able to talk to i2c devices on this bus (contains the keyboard, battery, and a bunch of power switches) we need something in the system that implements this arbitration scheme. As a side note: I'm trying to code up a quick arbitrator now using the muxing framework as Grant suggested. I will play with this and post it if I can get it to work. It's quite short so far. -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 11, 2013 at 3:53 PM, Doug Anderson <dianders@chromium.org> wrote: > Wolfram, > > On Sun, Feb 10, 2013 at 10:17 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: >> On Mon, Feb 04, 2013 at 02:33:15PM +0530, Yuvaraj Kumar C D wrote: >>> This RFC patch is w.r.t multimaster bus arbitration which is already >>> being discussing in the mainline. >>> This patch provides hooks for the i2c multimaster bus arbitration >>> and to have the arbitration parameters. >> >> I still haven't understood why the arbitration specified in the I2C >> standard is not enough for you. Or what you would need to make use of >> it. > > At the moment perhaps the strongest argument for why this particular > arbitration scheme is needed is that's what the EC (embedded > controller) on the ARM Chromebook uses. There have been several > arguments in-house about whether this was the most ideal way to > structure things, but that's what we shipped with. Thus, if we want > to be able to talk to i2c devices on this bus (contains the keyboard, > battery, and a bunch of power switches) we need something in the > system that implements this arbitration scheme. > There was some discussion about using the built-in I2C multi-master support but no one had had a good experience getting it to work reliably in a real system so we ended up deciding not to rely on this. > As a side note: I'm trying to code up a quick arbitrator now using the > muxing framework as Grant suggested. I will play with this and post > it if I can get it to work. It's quite short so far. > > -Doug -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > > At the moment perhaps the strongest argument for why this particular > > arbitration scheme is needed is that's what the EC (embedded > > controller) on the ARM Chromebook uses. There have been several > > arguments in-house about whether this was the most ideal way to > > structure things, but that's what we shipped with. Thus, if we want > > to be able to talk to i2c devices on this bus (contains the keyboard, > > battery, and a bunch of power switches) we need something in the > > system that implements this arbitration scheme. > > > > There was some discussion about using the built-in I2C multi-master > support but no one had had a good experience getting it to work > reliably in a real system so we ended up deciding not to rely on this. So, were there no experiences at all or bad experiences when trying? I am interested in hearing what I2C multi master restrictions might exist in practice. Thanks, Wolfram -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index e388590..ed89fc8 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -1408,18 +1408,29 @@ int i2c_transfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num) (msgs[ret].flags & I2C_M_RECV_LEN) ? "+" : ""); } #endif + if (adap->mm_arbit_algo) { + ret = adap->mm_arbit_algo->i2c_mm_bus_get(adap); + if (ret) + /* I2C bus under control of another master. */ + return -EAGAIN; + } if (in_atomic() || irqs_disabled()) { ret = i2c_trylock_adapter(adap); - if (!ret) + if (!ret && adap->mm_arbit_algo) { + adap->mm_arbit_algo->i2c_mm_bus_release(adap); + /* I2C activity is ongoing. */ return -EAGAIN; + } } else { i2c_lock_adapter(adap); } ret = __i2c_transfer(adap, msgs, num); i2c_unlock_adapter(adap); + if (adap->mm_arbit_algo) + adap->mm_arbit_algo->i2c_mm_bus_release(adap); return ret; } else { diff --git a/include/linux/i2c.h b/include/linux/i2c.h index d0c4db7..61fbfe3 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -371,6 +371,17 @@ struct i2c_algorithm { }; /* + *This struct provides hooks for i2c multi-master arbitration needs. + */ +struct i2c_mm_arbitration { + void *arbitration_data; + + /* Should return 0 if mastership could be successfully established */ + int (*i2c_mm_bus_get)(struct i2c_adapter *adap); + void (*i2c_mm_bus_release)(struct i2c_adapter *adap); +}; + +/* * i2c_adapter is the structure used to identify a physical i2c bus along * with the access algorithms necessary to access it. */ @@ -393,6 +404,7 @@ struct i2c_adapter { struct mutex userspace_clients_lock; struct list_head userspace_clients; + struct i2c_mm_arbitration *mm_arbit_algo; }; #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
This RFC patch is w.r.t multimaster bus arbitration which is already being discussing in the mainline. This patch provides hooks for the i2c multimaster bus arbitration and to have the arbitration parameters. Signed-off-by: Yuvaraj Kumar C D <yuvaraj.cd@samsung.com> --- drivers/i2c/i2c-core.c | 13 ++++++++++++- include/linux/i2c.h | 12 ++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-)