[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar C D]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar C D]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

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
>

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

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
>

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Doug Anderson]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Doug Anderson]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

On Wed, Feb 6, 2013 at 5:48 AM, Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> Yavaraj,
>
> On Mon, Feb 4, 2013 at 1:03 AM, Yuvaraj Kumar C D <yuvaraj.cd-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Yuvaraj Kumar]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Simon Glass]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Simon Glass]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Wolfram Sang]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Wolfram Sang]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Doug Anderson]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Doug Anderson]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Simon Glass]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Simon Glass]

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

Re: [RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Wolfram Sang]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Wolfram Sang]

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

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Ben Dooks]

On 10/02/13 18:17, Wolfram Sang 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.

I am a little concerned as there is no way to actually lock a bus to
stop another master from transmitting. You can get collisions in a
multi-master environment. Drivers need fixing to deal with collisions
and let the sender know the operation has failed.

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

[RFC] i2c: Providing hooks for i2c multimaster bus arbitration.

[Uwe Kleine-König]

Hello Ben,

On Tue, Feb 12, 2013 at 10:49:29AM +0000, Ben Dooks wrote:
> On 10/02/13 18:17, Wolfram Sang 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.
> 
> I am a little concerned as there is no way to actually lock a bus to
> stop another master from transmitting. You can get collisions in a
> multi-master environment. Drivers need fixing to deal with collisions
> and let the sender know the operation has failed.
In theory everything should just work. If a master send a Start, the bus
is actually locked and other masters must wait until Stop. If two
masters start concurrently one master just looses arbitration and the
winning master's transfer isn't corrupted. So client drivers don't need
adaption and master drivers "only" need to handle arbitration and
synchronization (which they should already now :-)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

i2c multimaster and the device driver detect function

[Jean-François Dagenais]

Hi all,

I've read the discussion on multimaster and I have to agree with Uwe about masters doing the arbitration (and retry). However, there's another issue which one quickly discovers when adding a second master on a physical i2c bus.

Here's the scenario: Using driver jc42 which adds a "detect" function, and using two masters which end-up creating "i2c-0" and "i2c-1", the temp sensors get instanciated twice, i.e. once on each bus master. This is obviously very problematic as there are two driver instances controlling the exact same chip.

Before I dive a bit too much into this, is there something I am missing?

Should there be a "struct i2c_bus" along side "struct i2c_adapter" where the adapter would point to an i2c_bus or something? Sounds like this would have many downstream implications, and until today, I've only written device drivers not core. I could try something, but due diligence dictates i2c maintainers should provide input before I get lost into un-mergeable work.

Thanks for your input!

[lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

Hi all,

I've read the discussion on multimaster and I have to agree with Uwe about masters doing the arbitration (and retry). However, there's another issue which one quickly discovers when adding a second master on a physical i2c bus.

Here's the scenario: Using driver jc42 which adds a "detect" function, and using two masters which end-up creating "i2c-0" and "i2c-1", the temp sensors get instanciated twice, i.e. once on each bus master. This is obviously very problematic as there are two driver instances controlling the exact same chip.

Before I dive a bit too much into this, is there something I am missing?

Should there be a "struct i2c_bus" along side "struct i2c_adapter" where the adapter would point to an i2c_bus or something? Sounds like this would have many downstream implications, and until today, I've only written device drivers not core. I could try something, but due diligence dictates i2c maintainers should provide input before I get lost into un-mergeable work.

Thanks for your input!


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Wed, May 08, 2013 at 11:57:59AM -0400, Jean-François Dagenais wrote:
> Hi all,
> 
> I've read the discussion on multimaster and I have to agree with Uwe about masters doing the arbitration (and retry). However, there's another issue which one quickly discovers when adding a second master on a physical i2c bus.
> 
> Here's the scenario: Using driver jc42 which adds a "detect" function, and using two masters which end-up creating "i2c-0" and "i2c-1", the temp sensors get instanciated twice, i.e. once on each bus master. This is obviously very problematic as there are two driver instances controlling the exact same chip.
> 
> Before I dive a bit too much into this, is there something I am missing?
> 
Isn't it the point of having multiple masters on the same bus, that each of
them can manage the same devices ?

Question for me is why you would want two masters in the same system context
point to the same I2C bus. Usually the second master would be something like an
IPMI controller or a second CPU/controller board in a redundant system.

Even then the multi-master scenario is problematic, as you still end up with
multiple masters controlling the same device. That is a problem inherent to I2C,
and especially problematic with multi-page devices (typical problem: master 1
sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
is a clean solution to solve that, other than to block i2c access for one of the
masters entirely.

Guenter

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Wed, May 08, 2013 at 11:57:59AM -0400, Jean-François Dagenais wrote:
> Hi all,
> 
> I've read the discussion on multimaster and I have to agree with Uwe about masters doing the arbitration (and retry). However, there's another issue which one quickly discovers when adding a second master on a physical i2c bus.
> 
> Here's the scenario: Using driver jc42 which adds a "detect" function, and using two masters which end-up creating "i2c-0" and "i2c-1", the temp sensors get instanciated twice, i.e. once on each bus master. This is obviously very problematic as there are two driver instances controlling the exact same chip.
> 
> Before I dive a bit too much into this, is there something I am missing?
> 
Isn't it the point of having multiple masters on the same bus, that each of
them can manage the same devices ?

Question for me is why you would want two masters in the same system context
point to the same I2C bus. Usually the second master would be something like an
IPMI controller or a second CPU/controller board in a redundant system.

Even then the multi-master scenario is problematic, as you still end up with
multiple masters controlling the same device. That is a problem inherent to I2C,
and especially problematic with multi-page devices (typical problem: master 1
sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
is a clean solution to solve that, other than to block i2c access for one of the
masters entirely.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

On 2013-05-08, at 13:54, Guenter Roeck wrote:

> [...]
> Isn't it the point of having multiple masters on the same bus, that each of
> them can manage the same devices ?

I wouldn't think so. You mention why in your last paragraph and I totally see
and agree with that. So that is not my intention at all.

> 
> Question for me is why you would want two masters in the same system context
> point to the same I2C bus. Usually the second master would be something like an
> IPMI controller or a second CPU/controller board in a redundant system.

I guess that's one scenario. In our scenario, we have an assambled board stack.
The main board has a sub-par i2c master onto which heavy slaves such as the
ad7147 capacitive input device consumes too much cpu power (cpu has to poll the
Kontron PLD which houses the i2c master core). Our hoped solution was to
allocate an i2c IP core in our FPGA. But board inter-connect budget didn't allow
for separate i2c segments, especially in the light that both master are said to
support multimaster operation. I got this working experimentally right now, I
just stumbled on jc42's detect, and am trying to project this kind of design to
the future. Right now, my only solution is to patch my version of jc42 not to
auto-detect, and explicitely enumerate my temperature sensors like my other
slaves in arch setup... yikes.

> 
> Even then the multi-master scenario is problematic, as you still end up with
> multiple masters controlling the same device. That is a problem inherent to I2C,
> and especially problematic with multi-page devices (typical problem: master 1
> sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
> is a clean solution to solve that, other than to block i2c access for one of the
> masters entirely.

Other than the redundant/resiliency scenario, why would you setup the same slave
on two different masters? Right now I have a working setup where some slaves are
declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
yet to stress test the setup within the next day or so, but so far, it seems to
work ok.

Thoughts?--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

On 2013-05-08, at 13:54, Guenter Roeck wrote:

> [...]
> Isn't it the point of having multiple masters on the same bus, that each of
> them can manage the same devices ?

I wouldn't think so. You mention why in your last paragraph and I totally see
and agree with that. So that is not my intention at all.

> 
> Question for me is why you would want two masters in the same system context
> point to the same I2C bus. Usually the second master would be something like an
> IPMI controller or a second CPU/controller board in a redundant system.

I guess that's one scenario. In our scenario, we have an assambled board stack.
The main board has a sub-par i2c master onto which heavy slaves such as the
ad7147 capacitive input device consumes too much cpu power (cpu has to poll the
Kontron PLD which houses the i2c master core). Our hoped solution was to
allocate an i2c IP core in our FPGA. But board inter-connect budget didn't allow
for separate i2c segments, especially in the light that both master are said to
support multimaster operation. I got this working experimentally right now, I
just stumbled on jc42's detect, and am trying to project this kind of design to
the future. Right now, my only solution is to patch my version of jc42 not to
auto-detect, and explicitely enumerate my temperature sensors like my other
slaves in arch setup... yikes.

> 
> Even then the multi-master scenario is problematic, as you still end up with
> multiple masters controlling the same device. That is a problem inherent to I2C,
> and especially problematic with multi-page devices (typical problem: master 1
> sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
> is a clean solution to solve that, other than to block i2c access for one of the
> masters entirely.

Other than the redundant/resiliency scenario, why would you setup the same slave
on two different masters? Right now I have a working setup where some slaves are
declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
yet to stress test the setup within the next day or so, but so far, it seems to
work ok.

Thoughts?
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: i2c multimaster and the device driver detect function

[Guenter Roeck]

On Wed, May 08, 2013 at 09:50:44PM -0400, Jean-François Dagenais wrote:
> 
> On 2013-05-08, at 13:54, Guenter Roeck wrote:
> 
> > [...]
> > Isn't it the point of having multiple masters on the same bus, that each of
> > them can manage the same devices ?
> 
> I wouldn't think so. You mention why in your last paragraph and I totally see
> and agree with that. So that is not my intention at all.
> 
Guess the real conclusion is that one should avoid two active masters
in the first place if possible.

> > 
> > Question for me is why you would want two masters in the same system context
> > point to the same I2C bus. Usually the second master would be something like an
> > IPMI controller or a second CPU/controller board in a redundant system.
> 
> I guess that's one scenario. In our scenario, we have an assambled board stack.
> The main board has a sub-par i2c master onto which heavy slaves such as the
> ad7147 capacitive input device consumes too much cpu power (cpu has to poll the
> Kontron PLD which houses the i2c master core). Our hoped solution was to
> allocate an i2c IP core in our FPGA. But board inter-connect budget didn't allow
> for separate i2c segments, especially in the light that both master are said to
> support multimaster operation. I got this working experimentally right now, I
> just stumbled on jc42's detect, and am trying to project this kind of design to
> the future. Right now, my only solution is to patch my version of jc42 not to
> auto-detect, and explicitely enumerate my temperature sensors like my other
> slaves in arch setup... yikes.
> 
Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
in the adapter code by setting the adapter class to 0 (specifically, don't set it
to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
source (it is GPL so you should be able to find it). While not perfect, that should
be better than disabling auto-detection in the affected chip drivers.
Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
auto-detected on address 0x50.

> > 
> > Even then the multi-master scenario is problematic, as you still end up with
> > multiple masters controlling the same device. That is a problem inherent to I2C,
> > and especially problematic with multi-page devices (typical problem: master 1
> > sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
> > is a clean solution to solve that, other than to block i2c access for one of the
> > masters entirely.
> 
> Other than the redundant/resiliency scenario, why would you setup the same slave
> on two different masters? Right now I have a working setup where some slaves are
> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
> yet to stress test the setup within the next day or so, but so far, it seems to
> work ok.
> 
Sure, it does work, I am just not sure what the benefits are of having two
masters in this scenario. Guess you are saying that the I2C master in the
Kontron PLD can not drive the AD7147 - is that correct ? If so, and if that
means you have to use your own I2C core anyway, why bother using the Kontron
PLD's I2C bus to start with ? You could just ignore it (ie not instantiate
or build it at all) and use your own.

Did you tell Kontron about the problems with their PLD ? Maybe they have a
solution.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Wed, May 08, 2013 at 09:50:44PM -0400, Jean-François Dagenais wrote:
> 
> On 2013-05-08, at 13:54, Guenter Roeck wrote:
> 
> > [...]
> > Isn't it the point of having multiple masters on the same bus, that each of
> > them can manage the same devices ?
> 
> I wouldn't think so. You mention why in your last paragraph and I totally see
> and agree with that. So that is not my intention at all.
> 
Guess the real conclusion is that one should avoid two active masters
in the first place if possible.

> > 
> > Question for me is why you would want two masters in the same system context
> > point to the same I2C bus. Usually the second master would be something like an
> > IPMI controller or a second CPU/controller board in a redundant system.
> 
> I guess that's one scenario. In our scenario, we have an assambled board stack.
> The main board has a sub-par i2c master onto which heavy slaves such as the
> ad7147 capacitive input device consumes too much cpu power (cpu has to poll the
> Kontron PLD which houses the i2c master core). Our hoped solution was to
> allocate an i2c IP core in our FPGA. But board inter-connect budget didn't allow
> for separate i2c segments, especially in the light that both master are said to
> support multimaster operation. I got this working experimentally right now, I
> just stumbled on jc42's detect, and am trying to project this kind of design to
> the future. Right now, my only solution is to patch my version of jc42 not to
> auto-detect, and explicitely enumerate my temperature sensors like my other
> slaves in arch setup... yikes.
> 
Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
in the adapter code by setting the adapter class to 0 (specifically, don't set it
to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
source (it is GPL so you should be able to find it). While not perfect, that should
be better than disabling auto-detection in the affected chip drivers.
Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
auto-detected on address 0x50.

> > 
> > Even then the multi-master scenario is problematic, as you still end up with
> > multiple masters controlling the same device. That is a problem inherent to I2C,
> > and especially problematic with multi-page devices (typical problem: master 1
> > sets page, master 2 sets page, master 1 accesses wrong data). I don't think there
> > is a clean solution to solve that, other than to block i2c access for one of the
> > masters entirely.
> 
> Other than the redundant/resiliency scenario, why would you setup the same slave
> on two different masters? Right now I have a working setup where some slaves are
> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
> yet to stress test the setup within the next day or so, but so far, it seems to
> work ok.
> 
Sure, it does work, I am just not sure what the benefits are of having two
masters in this scenario. Guess you are saying that the I2C master in the
Kontron PLD can not drive the AD7147 - is that correct ? If so, and if that
means you have to use your own I2C core anyway, why bother using the Kontron
PLD's I2C bus to start with ? You could just ignore it (ie not instantiate
or build it at all) and use your own.

Did you tell Kontron about the problems with their PLD ? Maybe they have a
solution.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:

> Guess the real conclusion is that one should avoid two active masters
> in the first place if possible.

I agree, I can't think of any benefits worth the trouble.

> Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> in the adapter code by setting the adapter class to 0 (specifically, don't set it
> to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> source (it is GPL so you should be able to find it). While not perfect, that should
> be better than disabling auto-detection in the affected chip drivers.

That's great advice!! I will look into this, thanks!

> Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> auto-detected on address 0x50.

Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
Kontron's JIDA chip.

>> [...] Right now I have a working setup where some slaves are
>> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
>> yet to stress test the setup within the next day or so, but so far, it seems to
>> work ok.
>> 
> Sure, it does work, I am just not sure what the benefits are of having two
> masters in this scenario.

My thoughts exactly. I would have avoided it. Right now I am trying to improve
and existing design.

>  Guess you are saying that the I2C master in the
> Kontron PLD can not drive the AD7147 - is that correct ?

Well no, it does drive it, we have it in stable production. It's just that when
you have your finger on the wheel, CPU usage goes up to 5-15%. More intense
polling of the sensors also takes a toll on the CPU. For different reasons other
than the ad714x, the FPGA i2c core option suddenly became attractive.

> If so, and if that
> means you have to use your own I2C core anyway, why bother using the Kontron
> PLD's I2C bus to start with ? You could just ignore it (ie not instantiate

> or build it at all) and use your own.

Yeah, we already considered that... our FPGA's reset and flash select are
controlled by an i2c I/O expander! Hehe, so this I/O expander needs to be
mastered from the kempld. Reality is a bummer ;)

> 
> Did you tell Kontron about the problems with their PLD ? Maybe they have a
> solution.

Yes we did. If I remember correctly, the problem is the absence of an interrupt
line from the PLD to the CPU, which explains the high CPU since the driver
sleeps-polls for async PLD completions and statuses. ... I know...

Thanks for the reply!
Cheers,
/jfd--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:

> Guess the real conclusion is that one should avoid two active masters
> in the first place if possible.

I agree, I can't think of any benefits worth the trouble.

> Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> in the adapter code by setting the adapter class to 0 (specifically, don't set it
> to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> source (it is GPL so you should be able to find it). While not perfect, that should
> be better than disabling auto-detection in the affected chip drivers.

That's great advice!! I will look into this, thanks!

> Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> auto-detected on address 0x50.

Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
Kontron's JIDA chip.

>> [...] Right now I have a working setup where some slaves are
>> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
>> yet to stress test the setup within the next day or so, but so far, it seems to
>> work ok.
>> 
> Sure, it does work, I am just not sure what the benefits are of having two
> masters in this scenario.

My thoughts exactly. I would have avoided it. Right now I am trying to improve
and existing design.

>  Guess you are saying that the I2C master in the
> Kontron PLD can not drive the AD7147 - is that correct ?

Well no, it does drive it, we have it in stable production. It's just that when
you have your finger on the wheel, CPU usage goes up to 5-15%. More intense
polling of the sensors also takes a toll on the CPU. For different reasons other
than the ad714x, the FPGA i2c core option suddenly became attractive.

> If so, and if that
> means you have to use your own I2C core anyway, why bother using the Kontron
> PLD's I2C bus to start with ? You could just ignore it (ie not instantiate

> or build it at all) and use your own.

Yeah, we already considered that... our FPGA's reset and flash select are
controlled by an i2c I/O expander! Hehe, so this I/O expander needs to be
mastered from the kempld. Reality is a bummer ;)

> 
> Did you tell Kontron about the problems with their PLD ? Maybe they have a
> solution.

Yes we did. If I remember correctly, the problem is the absence of an interrupt
line from the PLD to the CPU, which explains the high CPU since the driver
sleeps-polls for async PLD completions and statuses. ... I know...

Thanks for the reply!
Cheers,
/jfd
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Thu, May 09, 2013 at 08:38:28AM -0400, Jean-François Dagenais wrote:
> 
> On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:
> 
> > Guess the real conclusion is that one should avoid two active masters
> > in the first place if possible.
> 
> I agree, I can't think of any benefits worth the trouble.
> 
> > Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> > in the adapter code by setting the adapter class to 0 (specifically, don't set it
> > to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> > source (it is GPL so you should be able to find it). While not perfect, that should
> > be better than disabling auto-detection in the affected chip drivers.
> 
> That's great advice!! I will look into this, thanks!
> 
> > Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> > auto-detected on address 0x50.
> 
> Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
> Kontron's JIDA chip.
> 
Interesting.

> >> [...] Right now I have a working setup where some slaves are
> >> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
> >> yet to stress test the setup within the next day or so, but so far, it seems to
> >> work ok.
> >> 
> > Sure, it does work, I am just not sure what the benefits are of having two
> > masters in this scenario.
> 
> My thoughts exactly. I would have avoided it. Right now I am trying to improve
> and existing design.
> 
> >  Guess you are saying that the I2C master in the
> > Kontron PLD can not drive the AD7147 - is that correct ?
> 
> Well no, it does drive it, we have it in stable production. It's just that when
> you have your finger on the wheel, CPU usage goes up to 5-15%. More intense
> polling of the sensors also takes a toll on the CPU. For different reasons other
> than the ad714x, the FPGA i2c core option suddenly became attractive.
> 
> > If so, and if that
> > means you have to use your own I2C core anyway, why bother using the Kontron
> > PLD's I2C bus to start with ? You could just ignore it (ie not instantiate
> 
> > or build it at all) and use your own.
> 
> Yeah, we already considered that... our FPGA's reset and flash select are
> controlled by an i2c I/O expander! Hehe, so this I/O expander needs to be
> mastered from the kempld. Reality is a bummer ;)
> 
> > 
> > Did you tell Kontron about the problems with their PLD ? Maybe they have a
> > solution.
> 
> Yes we did. If I remember correctly, the problem is the absence of an interrupt
> line from the PLD to the CPU, which explains the high CPU since the driver
> sleeps-polls for async PLD completions and statuses. ... I know...
> 
Ah, that one. Yes, the higher CPU utilization is an indication that this is due
to polling, not due to power draw on the I2C lines. You might try to increase
the usleep_range interval in the Kontron driver to reduce the load a bit.

Anyway (kind of) good news - newer Kontron boards do support interrupts and no
longer have to revert to polling.

Thanks,
Guenter

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Thu, May 09, 2013 at 08:38:28AM -0400, Jean-François Dagenais wrote:
> 
> On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:
> 
> > Guess the real conclusion is that one should avoid two active masters
> > in the first place if possible.
> 
> I agree, I can't think of any benefits worth the trouble.
> 
> > Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> > in the adapter code by setting the adapter class to 0 (specifically, don't set it
> > to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> > source (it is GPL so you should be able to find it). While not perfect, that should
> > be better than disabling auto-detection in the affected chip drivers.
> 
> That's great advice!! I will look into this, thanks!
> 
> > Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> > auto-detected on address 0x50.
> 
> Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
> Kontron's JIDA chip.
> 
Interesting.

> >> [...] Right now I have a working setup where some slaves are
> >> declared on bus 0 (PLD i2c master) and others on bus 1 (FPGA i2c master). I have
> >> yet to stress test the setup within the next day or so, but so far, it seems to
> >> work ok.
> >> 
> > Sure, it does work, I am just not sure what the benefits are of having two
> > masters in this scenario.
> 
> My thoughts exactly. I would have avoided it. Right now I am trying to improve
> and existing design.
> 
> >  Guess you are saying that the I2C master in the
> > Kontron PLD can not drive the AD7147 - is that correct ?
> 
> Well no, it does drive it, we have it in stable production. It's just that when
> you have your finger on the wheel, CPU usage goes up to 5-15%. More intense
> polling of the sensors also takes a toll on the CPU. For different reasons other
> than the ad714x, the FPGA i2c core option suddenly became attractive.
> 
> > If so, and if that
> > means you have to use your own I2C core anyway, why bother using the Kontron
> > PLD's I2C bus to start with ? You could just ignore it (ie not instantiate
> 
> > or build it at all) and use your own.
> 
> Yeah, we already considered that... our FPGA's reset and flash select are
> controlled by an i2c I/O expander! Hehe, so this I/O expander needs to be
> mastered from the kempld. Reality is a bummer ;)
> 
> > 
> > Did you tell Kontron about the problems with their PLD ? Maybe they have a
> > solution.
> 
> Yes we did. If I remember correctly, the problem is the absence of an interrupt
> line from the PLD to the CPU, which explains the high CPU since the driver
> sleeps-polls for async PLD completions and statuses. ... I know...
> 
Ah, that one. Yes, the higher CPU utilization is an indication that this is due
to polling, not due to power draw on the I2C lines. You might try to increase
the usleep_range interval in the Kontron driver to reduce the load a bit.

Anyway (kind of) good news - newer Kontron boards do support interrupts and no
longer have to revert to polling.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean Delvare]

Salut Jean-François, hi Guenter,

Sorry for jumping in a little late, I am just back from vacation.

On Thu, 9 May 2013 08:38:28 -0400, Jean-François Dagenais wrote:
> On 2013-05-08, at 11:53 PM, Guenter Roeck wrote:
> > Is one of the I2C adapter drivers your own ? If so, you can disable auto-detection
> > in the adapter code by setting the adapter class to 0 (specifically, don't set it
> > to I2C_CLASS_HWMON). You can do the same in the Kontron driver if you have the
> > source (it is GPL so you should be able to find it). While not perfect, that should
> > be better than disabling auto-detection in the affected chip drivers.
> 
> That's great advice!! I will look into this, thanks!

Guenter is right. You never have to disable auto-detection in the slave
drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
auto-detection", not "I _will_ do auto-detection." It's always up to the
I2C adapter driver, whether auto-detection will happen or not. And it
is disabled by default. So if you don't want it, just do not enable it
in the bus driver. You can even set it per adapter, when the driver
controls more than one adapter, and per bus segment, when multiplexing
is taking place.

Please also note: the jc42 driver now uses I2C_CLASS_SPD not
I2C_CLASS_HWMON, because memory modules typically use a single chip for
SPD EEPROM and JEDEC JC42.2 temperature sensor. Think of I2C_CLASS_SPD
as "I2C class for memory modules."

> > Note that the Kontron driver also sets I2C_CLASS_SPD, which means EEPROMs are
> > auto-detected on address 0x50.
> 
> Funny, I had to explicitly inject "I2C_BOARD_INFO("24c32", 0x50)" to see
> Kontron's JIDA chip.

The only EEPROMs which are auto-detected are SPD and EDID EEPROMs. The
legacy eeprom driver is used for these. The 24C32 is a larger EEPROM,
you must use the at24 driver for it and it doesn't support
auto-detection (this is simply not possible.)

In the long run, the legacy eeprom driver could be killed, but not
before someone verifies that the at24 driver can take over completely,
including the auto-detection feature, performance optimizations and
corner case coverage.

> > (...)
> > Sure, it does work, I am just not sure what the benefits are of having two
> > masters in this scenario.
> 
> My thoughts exactly. I would have avoided it. Right now I am trying to improve
> and existing design.

It might be a little late now, but you may want to look into the
PCA9541, for which Guenter has written a driver.

-- 
Jean Delvare

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean Delvare]

U2FsdXQgSmVhbi1GcmFuw6dvaXMsIGhpIEd1ZW50ZXIsCgpTb3JyeSBmb3IganVtcGluZyBpbiBh
IGxpdHRsZSBsYXRlLCBJIGFtIGp1c3QgYmFjayBmcm9tIHZhY2F0aW9uLgoKT24gVGh1LCA5IE1h
eSAyMDEzIDA4OjM4OjI4IC0wNDAwLCBKZWFuLUZyYW7Dp29pcyBEYWdlbmFpcyB3cm90ZToKPiBP
biAyMDEzLTA1LTA4LCBhdCAxMTo1MyBQTSwgR3VlbnRlciBSb2VjayB3cm90ZToKPiA+IElzIG9u
ZSBvZiB0aGUgSTJDIGFkYXB0ZXIgZHJpdmVycyB5b3VyIG93biA/IElmIHNvLCB5b3UgY2FuIGRp
c2FibGUgYXV0by1kZXRlY3Rpb24KPiA+IGluIHRoZSBhZGFwdGVyIGNvZGUgYnkgc2V0dGluZyB0
aGUgYWRhcHRlciBjbGFzcyB0byAwIChzcGVjaWZpY2FsbHksIGRvbid0IHNldCBpdAo+ID4gdG8g
STJDX0NMQVNTX0hXTU9OKS4gWW91IGNhbiBkbyB0aGUgc2FtZSBpbiB0aGUgS29udHJvbiBkcml2
ZXIgaWYgeW91IGhhdmUgdGhlCj4gPiBzb3VyY2UgKGl0IGlzIEdQTCBzbyB5b3Ugc2hvdWxkIGJl
IGFibGUgdG8gZmluZCBpdCkuIFdoaWxlIG5vdCBwZXJmZWN0LCB0aGF0IHNob3VsZAo+ID4gYmUg
YmV0dGVyIHRoYW4gZGlzYWJsaW5nIGF1dG8tZGV0ZWN0aW9uIGluIHRoZSBhZmZlY3RlZCBjaGlw
IGRyaXZlcnMuCj4gCj4gVGhhdCdzIGdyZWF0IGFkdmljZSEhIEkgd2lsbCBsb29rIGludG8gdGhp
cywgdGhhbmtzIQoKR3VlbnRlciBpcyByaWdodC4gWW91IG5ldmVyIGhhdmUgdG8gZGlzYWJsZSBh
dXRvLWRldGVjdGlvbiBpbiB0aGUgc2xhdmUKZHJpdmVycyAoamM0MiBldGMuKSBBbGwgdGhlc2Ug
c2xhdmUgZHJpdmVycyBkbyBpcyBjbGFpbSAiSSBfY2FuXyBkbwphdXRvLWRldGVjdGlvbiIsIG5v
dCAiSSBfd2lsbF8gZG8gYXV0by1kZXRlY3Rpb24uIiBJdCdzIGFsd2F5cyB1cCB0byB0aGUKSTJD
IGFkYXB0ZXIgZHJpdmVyLCB3aGV0aGVyIGF1dG8tZGV0ZWN0aW9uIHdpbGwgaGFwcGVuIG9yIG5v
dC4gQW5kIGl0CmlzIGRpc2FibGVkIGJ5IGRlZmF1bHQuIFNvIGlmIHlvdSBkb24ndCB3YW50IGl0
LCBqdXN0IGRvIG5vdCBlbmFibGUgaXQKaW4gdGhlIGJ1cyBkcml2ZXIuIFlvdSBjYW4gZXZlbiBz
ZXQgaXQgcGVyIGFkYXB0ZXIsIHdoZW4gdGhlIGRyaXZlcgpjb250cm9scyBtb3JlIHRoYW4gb25l
IGFkYXB0ZXIsIGFuZCBwZXIgYnVzIHNlZ21lbnQsIHdoZW4gbXVsdGlwbGV4aW5nCmlzIHRha2lu
ZyBwbGFjZS4KClBsZWFzZSBhbHNvIG5vdGU6IHRoZSBqYzQyIGRyaXZlciBub3cgdXNlcyBJMkNf
Q0xBU1NfU1BEIG5vdApJMkNfQ0xBU1NfSFdNT04sIGJlY2F1c2UgbWVtb3J5IG1vZHVsZXMgdHlw
aWNhbGx5IHVzZSBhIHNpbmdsZSBjaGlwIGZvcgpTUEQgRUVQUk9NIGFuZCBKRURFQyBKQzQyLjIg
dGVtcGVyYXR1cmUgc2Vuc29yLiBUaGluayBvZiBJMkNfQ0xBU1NfU1BECmFzICJJMkMgY2xhc3Mg
Zm9yIG1lbW9yeSBtb2R1bGVzLiIKCj4gPiBOb3RlIHRoYXQgdGhlIEtvbnRyb24gZHJpdmVyIGFs
c28gc2V0cyBJMkNfQ0xBU1NfU1BELCB3aGljaCBtZWFucyBFRVBST01zIGFyZQo+ID4gYXV0by1k
ZXRlY3RlZCBvbiBhZGRyZXNzIDB4NTAuCj4gCj4gRnVubnksIEkgaGFkIHRvIGV4cGxpY2l0bHkg
aW5qZWN0ICJJMkNfQk9BUkRfSU5GTygiMjRjMzIiLCAweDUwKSIgdG8gc2VlCj4gS29udHJvbidz
IEpJREEgY2hpcC4KClRoZSBvbmx5IEVFUFJPTXMgd2hpY2ggYXJlIGF1dG8tZGV0ZWN0ZWQgYXJl
IFNQRCBhbmQgRURJRCBFRVBST01zLiBUaGUKbGVnYWN5IGVlcHJvbSBkcml2ZXIgaXMgdXNlZCBm
b3IgdGhlc2UuIFRoZSAyNEMzMiBpcyBhIGxhcmdlciBFRVBST00sCnlvdSBtdXN0IHVzZSB0aGUg
YXQyNCBkcml2ZXIgZm9yIGl0IGFuZCBpdCBkb2Vzbid0IHN1cHBvcnQKYXV0by1kZXRlY3Rpb24g
KHRoaXMgaXMgc2ltcGx5IG5vdCBwb3NzaWJsZS4pCgpJbiB0aGUgbG9uZyBydW4sIHRoZSBsZWdh
Y3kgZWVwcm9tIGRyaXZlciBjb3VsZCBiZSBraWxsZWQsIGJ1dCBub3QKYmVmb3JlIHNvbWVvbmUg
dmVyaWZpZXMgdGhhdCB0aGUgYXQyNCBkcml2ZXIgY2FuIHRha2Ugb3ZlciBjb21wbGV0ZWx5LApp
bmNsdWRpbmcgdGhlIGF1dG8tZGV0ZWN0aW9uIGZlYXR1cmUsIHBlcmZvcm1hbmNlIG9wdGltaXph
dGlvbnMgYW5kCmNvcm5lciBjYXNlIGNvdmVyYWdlLgoKPiA+ICguLi4pCj4gPiBTdXJlLCBpdCBk
b2VzIHdvcmssIEkgYW0ganVzdCBub3Qgc3VyZSB3aGF0IHRoZSBiZW5lZml0cyBhcmUgb2YgaGF2
aW5nIHR3bwo+ID4gbWFzdGVycyBpbiB0aGlzIHNjZW5hcmlvLgo+IAo+IE15IHRob3VnaHRzIGV4
YWN0bHkuIEkgd291bGQgaGF2ZSBhdm9pZGVkIGl0LiBSaWdodCBub3cgSSBhbSB0cnlpbmcgdG8g
aW1wcm92ZQo+IGFuZCBleGlzdGluZyBkZXNpZ24uCgpJdCBtaWdodCBiZSBhIGxpdHRsZSBsYXRl
IG5vdywgYnV0IHlvdSBtYXkgd2FudCB0byBsb29rIGludG8gdGhlClBDQTk1NDEsIGZvciB3aGlj
aCBHdWVudGVyIGhhcyB3cml0dGVuIGEgZHJpdmVyLgoKLS0gCkplYW4gRGVsdmFyZQoKX19fX19f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbG0tc2Vuc29ycyBtYWls
aW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3JzLm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29y
cy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5zb3Jz

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

Salut Jean, merci de participer!

On 2013-05-13, at 4:11 AM, Jean Delvare wrote:

> 
> Guenter is right. You never have to disable auto-detection in the slave
> drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> auto-detection", not "I _will_ do auto-detection." It's always up to the
> I2C adapter driver, whether auto-detection will happen or not. And it
> is disabled by default. So if you don't want it, just do not enable it
> in the bus driver. You can even set it per adapter, when the driver
> controls more than one adapter, and per bus segment, when multiplexing
> is taking place.

I am just wondering where the clean hook is for doing this. From what I can
gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
flexibility? Something along the lines of patching the pdata of such a master
driver to provide a 'class' variable in pdata? If so, one would have to take
into account the existing users of the master which expect the previous class
setting which may not be '0', thus requiring patching the existing upstreamed
users...  Suggestions?

> [...]
> It might be a little late now, but you may want to look into the
> PCA9541, for which Guenter has written a driver.

Thanks for that. Unfortunately, I am bound to existing hardware on this
platform. I will keep it in mind for future designs though.

Since my first post, I have stumbled on more problems with multimaster. Namely,
the master drivers are not consistent with the way they deal with arbitration
loss. I've all but re-written i2c-xiic, and dealt with arblost by catching
BusNotBusy interrupt and retrying the transfer. But the same cannot be said
about i2c-kempld (Kontron's in-PLD i2c master). Even if I manage to get the
kempld driver to handle this correctly, xiic still has some issues. The IP core
seems to enter a state when scanning unpopulated addresses (i2cdetect) for which
only a soft reset is required to recover, but this means it looses track of the
peer start-stop sequences and it's detection of the bus being busy or not.

I have decided to abandon the multimaster option for now as the hurdles are too
great. The only time I need another i2c master is when I need to hot reset the
fpga which houses the xiic core. (the reset pin is driven from gpo on an i2c i/o
expander). I will therefore handle this special case in userspace by unloading
i2c-xiic, then loading i2c-kempld. The slaves are arch declared on hard-coded
bus 0, so this should just work.

Thanks for all your help!
/jfd

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean-François Dagenais]

Salut Jean, merci de participer!

On 2013-05-13, at 4:11 AM, Jean Delvare wrote:

> 
> Guenter is right. You never have to disable auto-detection in the slave
> drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> auto-detection", not "I _will_ do auto-detection." It's always up to the
> I2C adapter driver, whether auto-detection will happen or not. And it
> is disabled by default. So if you don't want it, just do not enable it
> in the bus driver. You can even set it per adapter, when the driver
> controls more than one adapter, and per bus segment, when multiplexing
> is taking place.

I am just wondering where the clean hook is for doing this. From what I can
gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
flexibility? Something along the lines of patching the pdata of such a master
driver to provide a 'class' variable in pdata? If so, one would have to take
into account the existing users of the master which expect the previous class
setting which may not be '0', thus requiring patching the existing upstreamed
users...  Suggestions?

> [...]
> It might be a little late now, but you may want to look into the
> PCA9541, for which Guenter has written a driver.

Thanks for that. Unfortunately, I am bound to existing hardware on this
platform. I will keep it in mind for future designs though.

Since my first post, I have stumbled on more problems with multimaster. Namely,
the master drivers are not consistent with the way they deal with arbitration
loss. I've all but re-written i2c-xiic, and dealt with arblost by catching
BusNotBusy interrupt and retrying the transfer. But the same cannot be said
about i2c-kempld (Kontron's in-PLD i2c master). Even if I manage to get the
kempld driver to handle this correctly, xiic still has some issues. The IP core
seems to enter a state when scanning unpopulated addresses (i2cdetect) for which
only a soft reset is required to recover, but this means it looses track of the
peer start-stop sequences and it's detection of the bus being busy or not.

I have decided to abandon the multimaster option for now as the hurdles are too
great. The only time I need another i2c master is when I need to hot reset the
fpga which houses the xiic core. (the reset pin is driven from gpo on an i2c i/o
expander). I will therefore handle this special case in userspace by unloading
i2c-xiic, then loading i2c-kempld. The slaves are arch declared on hard-coded
bus 0, so this should just work.

Thanks for all your help!
/jfd




_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean Delvare]

On Mon, 13 May 2013 09:54:47 -0400, Jean-François Dagenais wrote:
> Salut Jean, merci de participer!
> 
> On 2013-05-13, at 4:11 AM, Jean Delvare wrote:
> 
> > 
> > Guenter is right. You never have to disable auto-detection in the slave
> > drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> > auto-detection", not "I _will_ do auto-detection." It's always up to the
> > I2C adapter driver, whether auto-detection will happen or not. And it
> > is disabled by default. So if you don't want it, just do not enable it
> > in the bus driver. You can even set it per adapter, when the driver
> > controls more than one adapter, and per bus segment, when multiplexing
> > is taking place.
> 
> I am just wondering where the clean hook is for doing this. From what I can
> gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
> I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
> flexibility? Something along the lines of patching the pdata of such a master
> driver to provide a 'class' variable in pdata?

Yes, exactly.

> If so, one would have to take
> into account the existing users of the master which expect the previous class
> setting which may not be '0', thus requiring patching the existing upstreamed
> users...  Suggestions?

Yes, you have to do something like that. The static class declarations
come from the PC world drivers where they (almost) never change.
Embedded systems definitely want a per-bus decision and should avoid
static declarations as much as possible. Especially when in most cases
they know exactly what slaves they are so they don't need
auto-detection. There's a reason why auto-detection is an optional
feature...

-- 
Jean Delvare

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Jean Delvare]

T24gTW9uLCAxMyBNYXkgMjAxMyAwOTo1NDo0NyAtMDQwMCwgSmVhbi1GcmFuw6dvaXMgRGFnZW5h
aXMgd3JvdGU6Cj4gU2FsdXQgSmVhbiwgbWVyY2kgZGUgcGFydGljaXBlciEKPiAKPiBPbiAyMDEz
LTA1LTEzLCBhdCA0OjExIEFNLCBKZWFuIERlbHZhcmUgd3JvdGU6Cj4gCj4gPiAKPiA+IEd1ZW50
ZXIgaXMgcmlnaHQuIFlvdSBuZXZlciBoYXZlIHRvIGRpc2FibGUgYXV0by1kZXRlY3Rpb24gaW4g
dGhlIHNsYXZlCj4gPiBkcml2ZXJzIChqYzQyIGV0Yy4pIEFsbCB0aGVzZSBzbGF2ZSBkcml2ZXJz
IGRvIGlzIGNsYWltICJJIF9jYW5fIGRvCj4gPiBhdXRvLWRldGVjdGlvbiIsIG5vdCAiSSBfd2ls
bF8gZG8gYXV0by1kZXRlY3Rpb24uIiBJdCdzIGFsd2F5cyB1cCB0byB0aGUKPiA+IEkyQyBhZGFw
dGVyIGRyaXZlciwgd2hldGhlciBhdXRvLWRldGVjdGlvbiB3aWxsIGhhcHBlbiBvciBub3QuIEFu
ZCBpdAo+ID4gaXMgZGlzYWJsZWQgYnkgZGVmYXVsdC4gU28gaWYgeW91IGRvbid0IHdhbnQgaXQs
IGp1c3QgZG8gbm90IGVuYWJsZSBpdAo+ID4gaW4gdGhlIGJ1cyBkcml2ZXIuIFlvdSBjYW4gZXZl
biBzZXQgaXQgcGVyIGFkYXB0ZXIsIHdoZW4gdGhlIGRyaXZlcgo+ID4gY29udHJvbHMgbW9yZSB0
aGFuIG9uZSBhZGFwdGVyLCBhbmQgcGVyIGJ1cyBzZWdtZW50LCB3aGVuIG11bHRpcGxleGluZwo+
ID4gaXMgdGFraW5nIHBsYWNlLgo+IAo+IEkgYW0ganVzdCB3b25kZXJpbmcgd2hlcmUgdGhlIGNs
ZWFuIGhvb2sgaXMgZm9yIGRvaW5nIHRoaXMuIEZyb20gd2hhdCBJIGNhbgo+IGdhdGhlciwgdGhl
IG1hc3RlciBkcml2ZXIocyBJJ3ZlIHNlZW4pIGRlY2xhcmUgIi5jbGFzcyA9IEkyQ19DTEFTU19I
V01PTiB8Cj4gSTJDX0NMQVNTX1NQRCwiIHByZXR0eSBzdGF0aWNhbGx5LiBJcyBpdCBqdXN0IHRo
YXQgdGhleSBhcmUgbWlzc2luZyB0aGlzCj4gZmxleGliaWxpdHk/IFNvbWV0aGluZyBhbG9uZyB0
aGUgbGluZXMgb2YgcGF0Y2hpbmcgdGhlIHBkYXRhIG9mIHN1Y2ggYSBtYXN0ZXIKPiBkcml2ZXIg
dG8gcHJvdmlkZSBhICdjbGFzcycgdmFyaWFibGUgaW4gcGRhdGE/CgpZZXMsIGV4YWN0bHkuCgo+
IElmIHNvLCBvbmUgd291bGQgaGF2ZSB0byB0YWtlCj4gaW50byBhY2NvdW50IHRoZSBleGlzdGlu
ZyB1c2VycyBvZiB0aGUgbWFzdGVyIHdoaWNoIGV4cGVjdCB0aGUgcHJldmlvdXMgY2xhc3MKPiBz
ZXR0aW5nIHdoaWNoIG1heSBub3QgYmUgJzAnLCB0aHVzIHJlcXVpcmluZyBwYXRjaGluZyB0aGUg
ZXhpc3RpbmcgdXBzdHJlYW1lZAo+IHVzZXJzLi4uICBTdWdnZXN0aW9ucz8KClllcywgeW91IGhh
dmUgdG8gZG8gc29tZXRoaW5nIGxpa2UgdGhhdC4gVGhlIHN0YXRpYyBjbGFzcyBkZWNsYXJhdGlv
bnMKY29tZSBmcm9tIHRoZSBQQyB3b3JsZCBkcml2ZXJzIHdoZXJlIHRoZXkgKGFsbW9zdCkgbmV2
ZXIgY2hhbmdlLgpFbWJlZGRlZCBzeXN0ZW1zIGRlZmluaXRlbHkgd2FudCBhIHBlci1idXMgZGVj
aXNpb24gYW5kIHNob3VsZCBhdm9pZApzdGF0aWMgZGVjbGFyYXRpb25zIGFzIG11Y2ggYXMgcG9z
c2libGUuIEVzcGVjaWFsbHkgd2hlbiBpbiBtb3N0IGNhc2VzCnRoZXkga25vdyBleGFjdGx5IHdo
YXQgc2xhdmVzIHRoZXkgYXJlIHNvIHRoZXkgZG9uJ3QgbmVlZAphdXRvLWRldGVjdGlvbi4gVGhl
cmUncyBhIHJlYXNvbiB3aHkgYXV0by1kZXRlY3Rpb24gaXMgYW4gb3B0aW9uYWwKZmVhdHVyZS4u
LgoKLS0gCkplYW4gRGVsdmFyZQoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX18KbG0tc2Vuc29ycyBtYWlsaW5nIGxpc3QKbG0tc2Vuc29yc0BsbS1zZW5zb3Jz
Lm9yZwpodHRwOi8vbGlzdHMubG0tc2Vuc29ycy5vcmcvbWFpbG1hbi9saXN0aW5mby9sbS1zZW5z
b3Jz

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Mon, May 13, 2013 at 04:14:13PM +0200, Jean Delvare wrote:
> On Mon, 13 May 2013 09:54:47 -0400, Jean-François Dagenais wrote:
> > Salut Jean, merci de participer!
> > 
> > On 2013-05-13, at 4:11 AM, Jean Delvare wrote:
> > 
> > > 
> > > Guenter is right. You never have to disable auto-detection in the slave
> > > drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> > > auto-detection", not "I _will_ do auto-detection." It's always up to the
> > > I2C adapter driver, whether auto-detection will happen or not. And it
> > > is disabled by default. So if you don't want it, just do not enable it
> > > in the bus driver. You can even set it per adapter, when the driver
> > > controls more than one adapter, and per bus segment, when multiplexing
> > > is taking place.
> > 
> > I am just wondering where the clean hook is for doing this. From what I can
> > gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
> > I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
> > flexibility? Something along the lines of patching the pdata of such a master
> > driver to provide a 'class' variable in pdata?
> 
> Yes, exactly.
> 
Or use devicetree, which is quite prevalent in embedded systems nowadays.

Guenter

> > If so, one would have to take
> > into account the existing users of the master which expect the previous class
> > setting which may not be '0', thus requiring patching the existing upstreamed
> > users...  Suggestions?
> 
> Yes, you have to do something like that. The static class declarations
> come from the PC world drivers where they (almost) never change.
> Embedded systems definitely want a per-bus decision and should avoid
> static declarations as much as possible. Especially when in most cases
> they know exactly what slaves they are so they don't need
> auto-detection. There's a reason why auto-detection is an optional
> feature...
> 
> -- 
> Jean Delvare
> 

Re: [lm-sensors] i2c multimaster and the device driver detect function

[Guenter Roeck]

On Mon, May 13, 2013 at 04:14:13PM +0200, Jean Delvare wrote:
> On Mon, 13 May 2013 09:54:47 -0400, Jean-François Dagenais wrote:
> > Salut Jean, merci de participer!
> > 
> > On 2013-05-13, at 4:11 AM, Jean Delvare wrote:
> > 
> > > 
> > > Guenter is right. You never have to disable auto-detection in the slave
> > > drivers (jc42 etc.) All these slave drivers do is claim "I _can_ do
> > > auto-detection", not "I _will_ do auto-detection." It's always up to the
> > > I2C adapter driver, whether auto-detection will happen or not. And it
> > > is disabled by default. So if you don't want it, just do not enable it
> > > in the bus driver. You can even set it per adapter, when the driver
> > > controls more than one adapter, and per bus segment, when multiplexing
> > > is taking place.
> > 
> > I am just wondering where the clean hook is for doing this. From what I can
> > gather, the master driver(s I've seen) declare ".class = I2C_CLASS_HWMON |
> > I2C_CLASS_SPD," pretty statically. Is it just that they are missing this
> > flexibility? Something along the lines of patching the pdata of such a master
> > driver to provide a 'class' variable in pdata?
> 
> Yes, exactly.
> 
Or use devicetree, which is quite prevalent in embedded systems nowadays.

Guenter

> > If so, one would have to take
> > into account the existing users of the master which expect the previous class
> > setting which may not be '0', thus requiring patching the existing upstreamed
> > users...  Suggestions?
> 
> Yes, you have to do something like that. The static class declarations
> come from the PC world drivers where they (almost) never change.
> Embedded systems definitely want a per-bus decision and should avoid
> static declarations as much as possible. Especially when in most cases
> they know exactly what slaves they are so they don't need
> auto-detection. There's a reason why auto-detection is an optional
> feature...
> 
> -- 
> Jean Delvare
> 

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors