Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Wolfram Sang]

On Wed, Mar 13, 2013 at 09:36:21AM -0700, Doug Anderson wrote:
> The i2c-arbitrator-cros-ec driver implements the arbitration scheme
> that the Embedded Controller (EC) on the ARM Chromebook expects to use
> for bus multimastering.  This i2c-arbitrator-cros-ec driver could also
> be used in other places where standard I2C bus arbitration can't be
> used and two extra GPIOs are available for arbitration.
> 
> This driver is based on code that Simon Glass added to the i2c-s3c2410
> driver in the Chrome OS kernel 3.4 tree.  The current incarnation as a
> mux driver is as suggested by Grant Likely.  See
>  for some history.
> 
> Signed-off-by: Doug Anderson 
> Signed-off-by: Simon Glass 
> Signed-off-by: Naveen Krishna Chatradhi 
> Reviewed-by: Stephen Warren 
> Tested-by: Naveen Krishna Chatradhi 

I'd like to have the bindings more generic. They should allow for n
possible masters IMO. It doesn't need to be implemented right now, but
it should be possible to add that later.

> ---
> Changes in v4: None
> Changes in v3:
> - Handle of_find_i2c_adapter_by_node() failure more properly by
>   changing init order.
> - Don't warn on -EPROBE_DEFER from calls that could return it.
> - Move to module_platform_driver().  As we pull in parts of the system
>   that rely on devices under this i2c bus we'll need to make sure they
>   can handle the fact that they'll be initted later now.
> 
> Changes in v2:
> - Renamed to i2c-arbitrator-cros-ec.
> - Documented "microsecond" properties as optional; removed
>   "bus-arbitration" prefix since it was just extra wordy.
> - Split GPIOs into two properties to make it cleaner.
> - Capitalized I2C in freeform text.
> - Get 'active low' from device tree.
> 
>  .../bindings/i2c/i2c-arbitrator-cros-ec.txt|  76 +++

I wonder about a more generic name. i2c-arb-gpio-challenge.* maybe?

>  drivers/i2c/muxes/Kconfig  |  11 +
>  drivers/i2c/muxes/Makefile |   2 +
>  drivers/i2c/muxes/i2c-arbitrator-cros-ec.c | 222 
> +
>  4 files changed, 311 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
>  create mode 100644 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt 
> b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
> new file mode 100644
> index 000..1f893e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
> @@ -0,0 +1,76 @@
> +GPIO-based Arbitration used by the ARM Chromebook (exynos5250-snow)
> +===
> +This uses GPIO lines between the AP (Application Processor) and an attached
> +EC (Embedded Controller) which both want to talk on the same I2C bus as 
> master.
> +
> +The AP and EC each have a 'bus claim' line, which is an output that the
> +other can see. These are both active low, with pull-ups enabled.
> +
> +- AP_CLAIM: output from AP, signalling to the EC that the AP wants the bus
> +- EC_CLAIM: output from EC, signalling to the AP that the EC wants the bus

I'd like to drop the specific terms of AP and EC and just talk about
multiple masters.

> +This mechanism is used instead of standard I2C multimaster to avoid some of 
> the
> +subtle driver and silicon bugs that are often present with I2C multimaster.
> +
> +
> +Algorithm:
> +
> +The basic algorithm is to assert your line when you want the bus, then make
> +sure that the other side doesn't want it also. A detailed explanation is best
> +done with an example.
> +
> +Let's say the AP wants to claim the bus. It:
> +1. Asserts AP_CLAIM.
> +2. Waits a little bit for the other side to notice (slew time, say 10
> +   microseconds).
> +3. Checks EC_CLAIM. If this is not asserted then the AP has the bus and we 
> are
> +   done.
> +4. Otherwise, wait for a few milliseconds and see if EC_CLAIM is released.
> +5. If not, back off, release the claim and wait for a few more milliseconds.
> +6. Go back to 1 (until retry time has expired).
> +
> +
> +Required properties:
> +- compatible: i2c-arbitrator-cros-ec
> +- ap-claim-gpio: The GPIO that we (the AP) use to claim the bus.
> +- ec-claim-gpio: The GPIO that the other side (the EC) uses the claim the 
> bus.

An array based approach like in the i2c-mux-gpio driver would be more
generic. Just mention that the driver only supports 2 entries at the
moment.

> +- Standard I2C mux properties. See mux.txt in this directory.
> +- Single I2C child bus node at reg 0. See mux.txt in this directory.
> +
> +Optional properties:
> +- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 
> us.
> +- wait-retry-us: we'll attempt another claim after this many microseconds.
> +Default is 3000 us.
> +- wait-free-us: we'll give up after this many microseconds. Default is 5 
> us.

Grant, I assume it is 

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Wolfram Sang]

Guenter,

> I think there is a difference between a bad driver or underlying hardware. To
> me, "shipped" applies to hardware or firmware which can not be upgraded, not 
> to
> the software running on it.

OK. Valuable distinction.

> "board support hosted in the I2C directory". But that is exactly what I am
> talking about, isn't it ? I have board specific multiplexers and a board
> specific I2C controller, and that is just talking about the I2C code.

Yes and no. I think I can accept that some hardware has GPIOs wired to
handle I2C arbitration. I still have problems in having arbitrator
drivers per board, each one with various ideas how to do it. That would
be a maintenance horror and has nothing to do with I2C, strictly
speaking. What I would love to see is a few generic arbitrator drivers,
e.g. utilizing a timeout-based scheme (as proposed here). Or like the
old SCSI method putting IDs on the wire and the lowest wins. Stuff like
that.

> A better example might be Kontron board support. They implement gpio, I2C mux,
> and a watchdog in a PLD. They too have an access arbitration scheme where
> one has to acquire a hardware mutex before accessing the pld, if I understand
> correctly because some microcontroller might need to access it as well.
> Leaving the actual code aside, would you reject that too if you don't like
> the arbitration scheme, or because you don't want to have board support
> in the i2c directory ?

If I understood correctly, getting the mutex would be done in some
platform code and the I2C driver will simply call the necessary function
to obtain and release the mutex. Or maybe the MFD layer can help, dunno.
Both are fine with me and I don't care about the actual PLD arbitration.

Thanks,

   Wolfram
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Wolfram Sang]

Guenter,

 I think there is a difference between a bad driver or underlying hardware. To
 me, shipped applies to hardware or firmware which can not be upgraded, not 
 to
 the software running on it.

OK. Valuable distinction.

 board support hosted in the I2C directory. But that is exactly what I am
 talking about, isn't it ? I have board specific multiplexers and a board
 specific I2C controller, and that is just talking about the I2C code.

Yes and no. I think I can accept that some hardware has GPIOs wired to
handle I2C arbitration. I still have problems in having arbitrator
drivers per board, each one with various ideas how to do it. That would
be a maintenance horror and has nothing to do with I2C, strictly
speaking. What I would love to see is a few generic arbitrator drivers,
e.g. utilizing a timeout-based scheme (as proposed here). Or like the
old SCSI method putting IDs on the wire and the lowest wins. Stuff like
that.

 A better example might be Kontron board support. They implement gpio, I2C mux,
 and a watchdog in a PLD. They too have an access arbitration scheme where
 one has to acquire a hardware mutex before accessing the pld, if I understand
 correctly because some microcontroller might need to access it as well.
 Leaving the actual code aside, would you reject that too if you don't like
 the arbitration scheme, or because you don't want to have board support
 in the i2c directory ?

If I understood correctly, getting the mutex would be done in some
platform code and the I2C driver will simply call the necessary function
to obtain and release the mutex. Or maybe the MFD layer can help, dunno.
Both are fine with me and I don't care about the actual PLD arbitration.

Thanks,

   Wolfram
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Wolfram Sang]

On Wed, Mar 13, 2013 at 09:36:21AM -0700, Doug Anderson wrote:
 The i2c-arbitrator-cros-ec driver implements the arbitration scheme
 that the Embedded Controller (EC) on the ARM Chromebook expects to use
 for bus multimastering.  This i2c-arbitrator-cros-ec driver could also
 be used in other places where standard I2C bus arbitration can't be
 used and two extra GPIOs are available for arbitration.
 
 This driver is based on code that Simon Glass added to the i2c-s3c2410
 driver in the Chrome OS kernel 3.4 tree.  The current incarnation as a
 mux driver is as suggested by Grant Likely.  See
 https://patchwork.kernel.org/patch/1877311/ for some history.
 
 Signed-off-by: Doug Anderson diand...@chromium.org
 Signed-off-by: Simon Glass s...@chromium.org
 Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
 Reviewed-by: Stephen Warren swar...@nvidia.com
 Tested-by: Naveen Krishna Chatradhi ch.nav...@samsung.com

I'd like to have the bindings more generic. They should allow for n
possible masters IMO. It doesn't need to be implemented right now, but
it should be possible to add that later.

 ---
 Changes in v4: None
 Changes in v3:
 - Handle of_find_i2c_adapter_by_node() failure more properly by
   changing init order.
 - Don't warn on -EPROBE_DEFER from calls that could return it.
 - Move to module_platform_driver().  As we pull in parts of the system
   that rely on devices under this i2c bus we'll need to make sure they
   can handle the fact that they'll be initted later now.
 
 Changes in v2:
 - Renamed to i2c-arbitrator-cros-ec.
 - Documented microsecond properties as optional; removed
   bus-arbitration prefix since it was just extra wordy.
 - Split GPIOs into two properties to make it cleaner.
 - Capitalized I2C in freeform text.
 - Get 'active low' from device tree.
 
  .../bindings/i2c/i2c-arbitrator-cros-ec.txt|  76 +++

I wonder about a more generic name. i2c-arb-gpio-challenge.* maybe?

  drivers/i2c/muxes/Kconfig  |  11 +
  drivers/i2c/muxes/Makefile |   2 +
  drivers/i2c/muxes/i2c-arbitrator-cros-ec.c | 222 
 +
  4 files changed, 311 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
  create mode 100644 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c
 
 diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt 
 b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
 new file mode 100644
 index 000..1f893e7
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
 @@ -0,0 +1,76 @@
 +GPIO-based Arbitration used by the ARM Chromebook (exynos5250-snow)
 +===
 +This uses GPIO lines between the AP (Application Processor) and an attached
 +EC (Embedded Controller) which both want to talk on the same I2C bus as 
 master.
 +
 +The AP and EC each have a 'bus claim' line, which is an output that the
 +other can see. These are both active low, with pull-ups enabled.
 +
 +- AP_CLAIM: output from AP, signalling to the EC that the AP wants the bus
 +- EC_CLAIM: output from EC, signalling to the AP that the EC wants the bus

I'd like to drop the specific terms of AP and EC and just talk about
multiple masters.

 +This mechanism is used instead of standard I2C multimaster to avoid some of 
 the
 +subtle driver and silicon bugs that are often present with I2C multimaster.
 +
 +
 +Algorithm:
 +
 +The basic algorithm is to assert your line when you want the bus, then make
 +sure that the other side doesn't want it also. A detailed explanation is best
 +done with an example.
 +
 +Let's say the AP wants to claim the bus. It:
 +1. Asserts AP_CLAIM.
 +2. Waits a little bit for the other side to notice (slew time, say 10
 +   microseconds).
 +3. Checks EC_CLAIM. If this is not asserted then the AP has the bus and we 
 are
 +   done.
 +4. Otherwise, wait for a few milliseconds and see if EC_CLAIM is released.
 +5. If not, back off, release the claim and wait for a few more milliseconds.
 +6. Go back to 1 (until retry time has expired).
 +
 +
 +Required properties:
 +- compatible: i2c-arbitrator-cros-ec
 +- ap-claim-gpio: The GPIO that we (the AP) use to claim the bus.
 +- ec-claim-gpio: The GPIO that the other side (the EC) uses the claim the 
 bus.

An array based approach like in the i2c-mux-gpio driver would be more
generic. Just mention that the driver only supports 2 entries at the
moment.

 +- Standard I2C mux properties. See mux.txt in this directory.
 +- Single I2C child bus node at reg 0. See mux.txt in this directory.
 +
 +Optional properties:
 +- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 
 us.
 +- wait-retry-us: we'll attempt another claim after this many microseconds.
 +Default is 3000 us.
 +- wait-free-us: we'll give up after this many microseconds. Default is 5 
 us.

Grant, I assume it is okay to 

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Guenter Roeck]

On Sat, Apr 06, 2013 at 10:11:32PM +0200, Wolfram Sang wrote:
> Hi,
> 
> > Very interesting discussion, especially the argument that "we already 
> > shipped"
> > would not be a convincing argument.
> > 
> > I had senior kernel maintainers tell me and the company I work for that we 
> > should
> > submit _all_ our platform specific kernel code and drivers for inclusion 
> > into
> > the upstream kernel.
> 
> Yes, great. Really!
> 
Yes, though, thinking about it, it was "submit" and didn't say anything
about potential for acceptance.

> > This exchange suggests that "it is a shipping product" does not count for 
> > such
> > submissions, and that "Benefit for the kernel" would be the deciding factor
> > instead. Which of course is a very vague statement - if code supporting
> > Chromebookis is of no benefit for the kernel, support for my company's 
> > products
> > for sure is much less so.
> 
> First, let me state that I did not intend to say that the arbitrator
> muxer here has NO benefit for the kernel. I was aware there might be
> arguments for that and I wanted some more discussion to make that
> clearer to me. Simon's mail was very helpful in that regard and I will
> comment on that somewhen later.
> 
> Still, I do have problems with "we already shipped it". If the driver is
> bad or the underlying concept is fragile, I want the freedom to reject a
> patch, product shipped or not. I will be supportive to find a proper
> solution, promised. If all fails, there is still staging/ or the
> possibility of out-of-tree patches.
> 
I think there is a difference between a bad driver or underlying hardware. To
me, "shipped" applies to hardware or firmware which can not be upgraded, not to
the software running on it.

> > Which kind of leaves me in a bind. On one side I promote that we should 
> > submit
> > all our kernel code, on the other side there is a very compelling case to be
> > made that it won't be accepted anyway. That doesn't make my life easier -
> 
> Concentrate on argumenting why the driver is useful and it will be fine.
> "we already shipped this" feels a bit like blackmailing to me. And since
> most drivers do have well thought reasons for their existance, I'd
> primarily like to hear about those.
> 
> > essentially since it supports those who say that we should not submit 
> > anything
> > in the first place. And believe me, there are many of those. 
> > 
> > Just to give some examples:
> > - I2C multiplexers. We have a bunch of those. Looking at this exchange,
> >   it doesn't look good to get that code included.
> 
> Multiplexers should be easy going, it is the arbitration which is discussed 
> here.
> I am open to consider some generic arbitration schemes. What I am reluctant to
> do is to allow every board to have its own arbitration scheme. This
> would be board support hosted in the I2C directory. Meh.
> 
"board support hosted in the I2C directory". But that is exactly what I am
talking about, isn't it ? I have board specific multiplexers and a board
specific I2C controller, and that is just talking about the I2C code.

> > It would be nice have to get some well defined guidelines for "acceptable"
> > contributions. "Benefit for the kernel" sure isn't one.
> 
> I don't think it is possible to write down concrete guidelines for this.
> "According to rule 4a) of the guidelines you have to accept my patch"?
> That would be a mess, I think.
> 
Looking at it from a maintainer perspective, I agree.

Where it gets murky is really the hardware part. The (in my opinion)
philosophical arguments around not permitting device-tree based instantiation
of uio devices is one example. Another practical example I had to deal with
in my previous company is VGA memory space. Some hw geniuses decided to re-use
the VGA memory space in an embedded x86 device for an EEPROM. Guess what -
the x86 kernel writes into that space no matter what. A patch to address that
problem was rejected because "you should not re-use VGA memory space".
As if I had a choice.

A better example might be Kontron board support. They implement gpio, I2C mux,
and a watchdog in a PLD. They too have an access arbitration scheme where
one has to acquire a hardware mutex before accessing the pld, if I understand
correctly because some microcontroller might need to access it as well.
Leaving the actual code aside, would you reject that too if you don't like
the arbitration scheme, or because you don't want to have board support
in the i2c directory ?

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Guenter Roeck]

On Sat, Apr 06, 2013 at 10:11:32PM +0200, Wolfram Sang wrote:
 Hi,
 
  Very interesting discussion, especially the argument that we already 
  shipped
  would not be a convincing argument.
  
  I had senior kernel maintainers tell me and the company I work for that we 
  should
  submit _all_ our platform specific kernel code and drivers for inclusion 
  into
  the upstream kernel.
 
 Yes, great. Really!
 
Yes, though, thinking about it, it was submit and didn't say anything
about potential for acceptance.

  This exchange suggests that it is a shipping product does not count for 
  such
  submissions, and that Benefit for the kernel would be the deciding factor
  instead. Which of course is a very vague statement - if code supporting
  Chromebookis is of no benefit for the kernel, support for my company's 
  products
  for sure is much less so.
 
 First, let me state that I did not intend to say that the arbitrator
 muxer here has NO benefit for the kernel. I was aware there might be
 arguments for that and I wanted some more discussion to make that
 clearer to me. Simon's mail was very helpful in that regard and I will
 comment on that somewhen later.
 
 Still, I do have problems with we already shipped it. If the driver is
 bad or the underlying concept is fragile, I want the freedom to reject a
 patch, product shipped or not. I will be supportive to find a proper
 solution, promised. If all fails, there is still staging/ or the
 possibility of out-of-tree patches.
 
I think there is a difference between a bad driver or underlying hardware. To
me, shipped applies to hardware or firmware which can not be upgraded, not to
the software running on it.

  Which kind of leaves me in a bind. On one side I promote that we should 
  submit
  all our kernel code, on the other side there is a very compelling case to be
  made that it won't be accepted anyway. That doesn't make my life easier -
 
 Concentrate on argumenting why the driver is useful and it will be fine.
 we already shipped this feels a bit like blackmailing to me. And since
 most drivers do have well thought reasons for their existance, I'd
 primarily like to hear about those.
 
  essentially since it supports those who say that we should not submit 
  anything
  in the first place. And believe me, there are many of those. 
  
  Just to give some examples:
  - I2C multiplexers. We have a bunch of those. Looking at this exchange,
it doesn't look good to get that code included.
 
 Multiplexers should be easy going, it is the arbitration which is discussed 
 here.
 I am open to consider some generic arbitration schemes. What I am reluctant to
 do is to allow every board to have its own arbitration scheme. This
 would be board support hosted in the I2C directory. Meh.
 
board support hosted in the I2C directory. But that is exactly what I am
talking about, isn't it ? I have board specific multiplexers and a board
specific I2C controller, and that is just talking about the I2C code.

  It would be nice have to get some well defined guidelines for acceptable
  contributions. Benefit for the kernel sure isn't one.
 
 I don't think it is possible to write down concrete guidelines for this.
 According to rule 4a) of the guidelines you have to accept my patch?
 That would be a mess, I think.
 
Looking at it from a maintainer perspective, I agree.

Where it gets murky is really the hardware part. The (in my opinion)
philosophical arguments around not permitting device-tree based instantiation
of uio devices is one example. Another practical example I had to deal with
in my previous company is VGA memory space. Some hw geniuses decided to re-use
the VGA memory space in an embedded x86 device for an EEPROM. Guess what -
the x86 kernel writes into that space no matter what. A patch to address that
problem was rejected because you should not re-use VGA memory space.
As if I had a choice.

A better example might be Kontron board support. They implement gpio, I2C mux,
and a watchdog in a PLD. They too have an access arbitration scheme where
one has to acquire a hardware mutex before accessing the pld, if I understand
correctly because some microcontroller might need to access it as well.
Leaving the actual code aside, would you reject that too if you don't like
the arbitration scheme, or because you don't want to have board support
in the i2c directory ?

Thanks,
Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Wolfram Sang]

Hi,

> Very interesting discussion, especially the argument that "we already shipped"
> would not be a convincing argument.
> 
> I had senior kernel maintainers tell me and the company I work for that we 
> should
> submit _all_ our platform specific kernel code and drivers for inclusion into
> the upstream kernel.

Yes, great. Really!

> This exchange suggests that "it is a shipping product" does not count for such
> submissions, and that "Benefit for the kernel" would be the deciding factor
> instead. Which of course is a very vague statement - if code supporting
> Chromebookis is of no benefit for the kernel, support for my company's 
> products
> for sure is much less so.

First, let me state that I did not intend to say that the arbitrator
muxer here has NO benefit for the kernel. I was aware there might be
arguments for that and I wanted some more discussion to make that
clearer to me. Simon's mail was very helpful in that regard and I will
comment on that somewhen later.

Still, I do have problems with "we already shipped it". If the driver is
bad or the underlying concept is fragile, I want the freedom to reject a
patch, product shipped or not. I will be supportive to find a proper
solution, promised. If all fails, there is still staging/ or the
possibility of out-of-tree patches.

> Which kind of leaves me in a bind. On one side I promote that we should submit
> all our kernel code, on the other side there is a very compelling case to be
> made that it won't be accepted anyway. That doesn't make my life easier -

Concentrate on argumenting why the driver is useful and it will be fine.
"we already shipped this" feels a bit like blackmailing to me. And since
most drivers do have well thought reasons for their existance, I'd
primarily like to hear about those.

> essentially since it supports those who say that we should not submit anything
> in the first place. And believe me, there are many of those. 
> 
> Just to give some examples:
> - I2C multiplexers. We have a bunch of those. Looking at this exchange,
>   it doesn't look good to get that code included.

Multiplexers should be easy going, it is the arbitration which is discussed 
here.
I am open to consider some generic arbitration schemes. What I am reluctant to
do is to allow every board to have its own arbitration scheme. This
would be board support hosted in the I2C directory. Meh.

> It would be nice have to get some well defined guidelines for "acceptable"
> contributions. "Benefit for the kernel" sure isn't one.

I don't think it is possible to write down concrete guidelines for this.
"According to rule 4a) of the guidelines you have to accept my patch"?
That would be a mess, I think.

Regards,

   Wolfram

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Guenter Roeck]

On Fri, Apr 05, 2013 at 02:03:52PM -0600, Stephen Warren wrote:
> On 04/05/2013 01:37 PM, Simon Glass wrote:
> > HI Wolfram,
> > 
> > On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang  wrote:
> >> Doug,
> >>
> >>> Separately from a discussion of the technical merits, I'd say that
> >>> this patch is needed because the Embedded Controller (EC) on the ARM
> >>> Chromebook shipped expecting to communicate with this scheme.  While
> >>
> >> Uhrm, with all respect, "we already shipped it" is not a convincing
> >> argument regarding inclusion. Benefit for the kernel is.
> 
> I'm not quite sure why that isn't a convincing argument.
> 
> The hardware has shipped. I don't know whether the EC microcode can be
> updated in the field; it seems risky to do so even if it's possible. So,
> it either gets supported or not; the HW/ucode isn't going to change I
> suspect.
> 
> Hence, it seems that the decision would be:
> 
> a) Disallow the implementation of the arbitration scheme in the kernel,
> and hence don't support this board in the kernel. (or at least some very
> core features of this board)
> 
> b) Allow the implementation of the arbitration scheme in the kernel, and
> hence make possible support this board in the kernel.
> 
> From that perspective, the benefit for the kernel question comes down
> to: do we see a benefit for the kernel to support this board? I can't
> see why that wouldn't be a benefit.
> 
> The only disadvantage would be having to carrying code to support that
> board. That same argument can be made for any board, and I think
> typically doesn't cause any issue. The code for this I2C mux seems
> pretty self-contained, so even if it was absolutely terrible (which I
> don't think it is), it still wouldn't cause any wide-spread issues, I think.

Very interesting discussion, especially the argument that "we already shipped"
would not be a convincing argument.

I had senior kernel maintainers tell me and the company I work for that we 
should
submit _all_ our platform specific kernel code and drivers for inclusion into
the upstream kernel.

This exchange suggests that "it is a shipping product" does not count for such
submissions, and that "Benefit for the kernel" would be the deciding factor
instead. Which of course is a very vague statement - if code supporting
Chromebookis is of no benefit for the kernel, support for my company's products
for sure is much less so.

Which kind of leaves me in a bind. On one side I promote that we should submit
all our kernel code, on the other side there is a very compelling case to be
made that it won't be accepted anyway. That doesn't make my life easier -
essentially since it supports those who say that we should not submit anything
in the first place. And believe me, there are many of those. 

Just to give some examples:
- I2C multiplexers. We have a bunch of those. Looking at this exchange,
  it doesn't look good to get that code included.
- Custom multi-function FPGAs and CPLDs, amongst others implementing I2C
  controllers, I2C muxes, GPIO access, Flash access, and other functions. Same
  as above.
- Devicetree support for UIO devices (mostly forwarding ASICs), including gpio
  bindings, interrupt bindings, and clock bindings. Looking at older exchanges,
  that doesn't look good either. And please dont expect me to implement hacks
  around a clean solution because any devicetree binding for UIO drivers
  "does not describe hardware but its use".

Now, I can understand that there may be technical or architectural issues
preventing one driver or another from being accepted. For example, I can
understand if a driver for an USB-I2C adapder isn't accepted because the adapter
reports itself to the USB subsystem as serial driver. But "Benefit for the
kernel" is vague enough to reject anything for no real reason other than
someone not liking it (or the submitter, or the company the submitter
works for, or the hardware architecture).

It would be nice have to get some well defined guidelines for "acceptable"
contributions. "Benefit for the kernel" sure isn't one.

Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Guenter Roeck]

On Fri, Apr 05, 2013 at 02:03:52PM -0600, Stephen Warren wrote:
 On 04/05/2013 01:37 PM, Simon Glass wrote:
  HI Wolfram,
  
  On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang w...@the-dreams.de wrote:
  Doug,
 
  Separately from a discussion of the technical merits, I'd say that
  this patch is needed because the Embedded Controller (EC) on the ARM
  Chromebook shipped expecting to communicate with this scheme.  While
 
  Uhrm, with all respect, we already shipped it is not a convincing
  argument regarding inclusion. Benefit for the kernel is.
 
 I'm not quite sure why that isn't a convincing argument.
 
 The hardware has shipped. I don't know whether the EC microcode can be
 updated in the field; it seems risky to do so even if it's possible. So,
 it either gets supported or not; the HW/ucode isn't going to change I
 suspect.
 
 Hence, it seems that the decision would be:
 
 a) Disallow the implementation of the arbitration scheme in the kernel,
 and hence don't support this board in the kernel. (or at least some very
 core features of this board)
 
 b) Allow the implementation of the arbitration scheme in the kernel, and
 hence make possible support this board in the kernel.
 
 From that perspective, the benefit for the kernel question comes down
 to: do we see a benefit for the kernel to support this board? I can't
 see why that wouldn't be a benefit.
 
 The only disadvantage would be having to carrying code to support that
 board. That same argument can be made for any board, and I think
 typically doesn't cause any issue. The code for this I2C mux seems
 pretty self-contained, so even if it was absolutely terrible (which I
 don't think it is), it still wouldn't cause any wide-spread issues, I think.

Very interesting discussion, especially the argument that we already shipped
would not be a convincing argument.

I had senior kernel maintainers tell me and the company I work for that we 
should
submit _all_ our platform specific kernel code and drivers for inclusion into
the upstream kernel.

This exchange suggests that it is a shipping product does not count for such
submissions, and that Benefit for the kernel would be the deciding factor
instead. Which of course is a very vague statement - if code supporting
Chromebookis is of no benefit for the kernel, support for my company's products
for sure is much less so.

Which kind of leaves me in a bind. On one side I promote that we should submit
all our kernel code, on the other side there is a very compelling case to be
made that it won't be accepted anyway. That doesn't make my life easier -
essentially since it supports those who say that we should not submit anything
in the first place. And believe me, there are many of those. 

Just to give some examples:
- I2C multiplexers. We have a bunch of those. Looking at this exchange,
  it doesn't look good to get that code included.
- Custom multi-function FPGAs and CPLDs, amongst others implementing I2C
  controllers, I2C muxes, GPIO access, Flash access, and other functions. Same
  as above.
- Devicetree support for UIO devices (mostly forwarding ASICs), including gpio
  bindings, interrupt bindings, and clock bindings. Looking at older exchanges,
  that doesn't look good either. And please dont expect me to implement hacks
  around a clean solution because any devicetree binding for UIO drivers
  does not describe hardware but its use.

Now, I can understand that there may be technical or architectural issues
preventing one driver or another from being accepted. For example, I can
understand if a driver for an USB-I2C adapder isn't accepted because the adapter
reports itself to the USB subsystem as serial driver. But Benefit for the
kernel is vague enough to reject anything for no real reason other than
someone not liking it (or the submitter, or the company the submitter
works for, or the hardware architecture).

It would be nice have to get some well defined guidelines for acceptable
contributions. Benefit for the kernel sure isn't one.

Guenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Wolfram Sang]

Hi,

 Very interesting discussion, especially the argument that we already shipped
 would not be a convincing argument.
 
 I had senior kernel maintainers tell me and the company I work for that we 
 should
 submit _all_ our platform specific kernel code and drivers for inclusion into
 the upstream kernel.

Yes, great. Really!

 This exchange suggests that it is a shipping product does not count for such
 submissions, and that Benefit for the kernel would be the deciding factor
 instead. Which of course is a very vague statement - if code supporting
 Chromebookis is of no benefit for the kernel, support for my company's 
 products
 for sure is much less so.

First, let me state that I did not intend to say that the arbitrator
muxer here has NO benefit for the kernel. I was aware there might be
arguments for that and I wanted some more discussion to make that
clearer to me. Simon's mail was very helpful in that regard and I will
comment on that somewhen later.

Still, I do have problems with we already shipped it. If the driver is
bad or the underlying concept is fragile, I want the freedom to reject a
patch, product shipped or not. I will be supportive to find a proper
solution, promised. If all fails, there is still staging/ or the
possibility of out-of-tree patches.

 Which kind of leaves me in a bind. On one side I promote that we should submit
 all our kernel code, on the other side there is a very compelling case to be
 made that it won't be accepted anyway. That doesn't make my life easier -

Concentrate on argumenting why the driver is useful and it will be fine.
we already shipped this feels a bit like blackmailing to me. And since
most drivers do have well thought reasons for their existance, I'd
primarily like to hear about those.

 essentially since it supports those who say that we should not submit anything
 in the first place. And believe me, there are many of those. 
 
 Just to give some examples:
 - I2C multiplexers. We have a bunch of those. Looking at this exchange,
   it doesn't look good to get that code included.

Multiplexers should be easy going, it is the arbitration which is discussed 
here.
I am open to consider some generic arbitration schemes. What I am reluctant to
do is to allow every board to have its own arbitration scheme. This
would be board support hosted in the I2C directory. Meh.

 It would be nice have to get some well defined guidelines for acceptable
 contributions. Benefit for the kernel sure isn't one.

I don't think it is possible to write down concrete guidelines for this.
According to rule 4a) of the guidelines you have to accept my patch?
That would be a mess, I think.

Regards,

   Wolfram

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Stephen Warren]

On 04/05/2013 01:37 PM, Simon Glass wrote:
> HI Wolfram,
> 
> On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang  wrote:
>> Doug,
>>
>>> Separately from a discussion of the technical merits, I'd say that
>>> this patch is needed because the Embedded Controller (EC) on the ARM
>>> Chromebook shipped expecting to communicate with this scheme.  While
>>
>> Uhrm, with all respect, "we already shipped it" is not a convincing
>> argument regarding inclusion. Benefit for the kernel is.

I'm not quite sure why that isn't a convincing argument.

The hardware has shipped. I don't know whether the EC microcode can be
updated in the field; it seems risky to do so even if it's possible. So,
it either gets supported or not; the HW/ucode isn't going to change I
suspect.

Hence, it seems that the decision would be:

a) Disallow the implementation of the arbitration scheme in the kernel,
and hence don't support this board in the kernel. (or at least some very
core features of this board)

b) Allow the implementation of the arbitration scheme in the kernel, and
hence make possible support this board in the kernel.

>From that perspective, the benefit for the kernel question comes down
to: do we see a benefit for the kernel to support this board? I can't
see why that wouldn't be a benefit.

The only disadvantage would be having to carrying code to support that
board. That same argument can be made for any board, and I think
typically doesn't cause any issue. The code for this I2C mux seems
pretty self-contained, so even if it was absolutely terrible (which I
don't think it is), it still wouldn't cause any wide-spread issues, I think.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Stephen Warren]

On 04/05/2013 01:37 PM, Simon Glass wrote:
 HI Wolfram,
 
 On Wed, Apr 3, 2013 at 12:19 PM, Wolfram Sang w...@the-dreams.de wrote:
 Doug,

 Separately from a discussion of the technical merits, I'd say that
 this patch is needed because the Embedded Controller (EC) on the ARM
 Chromebook shipped expecting to communicate with this scheme.  While

 Uhrm, with all respect, we already shipped it is not a convincing
 argument regarding inclusion. Benefit for the kernel is.

I'm not quite sure why that isn't a convincing argument.

The hardware has shipped. I don't know whether the EC microcode can be
updated in the field; it seems risky to do so even if it's possible. So,
it either gets supported or not; the HW/ucode isn't going to change I
suspect.

Hence, it seems that the decision would be:

a) Disallow the implementation of the arbitration scheme in the kernel,
and hence don't support this board in the kernel. (or at least some very
core features of this board)

b) Allow the implementation of the arbitration scheme in the kernel, and
hence make possible support this board in the kernel.

From that perspective, the benefit for the kernel question comes down
to: do we see a benefit for the kernel to support this board? I can't
see why that wouldn't be a benefit.

The only disadvantage would be having to carrying code to support that
board. That same argument can be made for any board, and I think
typically doesn't cause any issue. The code for this I2C mux seems
pretty self-contained, so even if it was absolutely terrible (which I
don't think it is), it still wouldn't cause any wide-spread issues, I think.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Stephen Warren]

On 03/13/2013 10:59 AM, Doug Anderson wrote:
> Stephen,
> 
> On Wed, Mar 13, 2013 at 9:53 AM, Stephen Warren  wrote:
>>> Changes in v4: None
>>
>> Isn't this 'PATCH V3 REPOST' then?
> 
> In this case part 2 in the patch series changes but not parts 1 and 3.
>  I could have just reposted part 2 with a higher version, but that
> makes it a little harder to piece together all of the parts of the
> series so I decided to repost all 3.  I can do differently in the
> future if you prefer, but my understanding was that it was a matter of
> preference/judgement call.

Oh no you're quite right. I didn't notice it was a 3-part series, since
I only got patch 1/3 filtered into my inbox; I guess I wasn't CC'd on
the rest. Sorry for the noise.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Doug Anderson]

Stephen,

On Wed, Mar 13, 2013 at 9:53 AM, Stephen Warren  wrote:
>> Changes in v4: None
>
> Isn't this 'PATCH V3 REPOST' then?

In this case part 2 in the patch series changes but not parts 1 and 3.
 I could have just reposted part 2 with a higher version, but that
makes it a little harder to piece together all of the parts of the
series so I decided to repost all 3.  I can do differently in the
future if you prefer, but my understanding was that it was a matter of
preference/judgement call.

Thanks!  :)

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Stephen Warren]

On 03/13/2013 10:36 AM, Doug Anderson wrote:
> The i2c-arbitrator-cros-ec driver implements the arbitration scheme
> that the Embedded Controller (EC) on the ARM Chromebook expects to use
> for bus multimastering.  This i2c-arbitrator-cros-ec driver could also
> be used in other places where standard I2C bus arbitration can't be
> used and two extra GPIOs are available for arbitration.
> 
> This driver is based on code that Simon Glass added to the i2c-s3c2410
> driver in the Chrome OS kernel 3.4 tree.  The current incarnation as a
> mux driver is as suggested by Grant Likely.  See
>  for some history.
...
> Changes in v4: None

Isn't this 'PATCH V3 REPOST' then?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Doug Anderson]

The i2c-arbitrator-cros-ec driver implements the arbitration scheme
that the Embedded Controller (EC) on the ARM Chromebook expects to use
for bus multimastering.  This i2c-arbitrator-cros-ec driver could also
be used in other places where standard I2C bus arbitration can't be
used and two extra GPIOs are available for arbitration.

This driver is based on code that Simon Glass added to the i2c-s3c2410
driver in the Chrome OS kernel 3.4 tree.  The current incarnation as a
mux driver is as suggested by Grant Likely.  See
 for some history.

Signed-off-by: Doug Anderson 
Signed-off-by: Simon Glass 
Signed-off-by: Naveen Krishna Chatradhi 
Reviewed-by: Stephen Warren 
Tested-by: Naveen Krishna Chatradhi 
---
Changes in v4: None
Changes in v3:
- Handle of_find_i2c_adapter_by_node() failure more properly by
  changing init order.
- Don't warn on -EPROBE_DEFER from calls that could return it.
- Move to module_platform_driver().  As we pull in parts of the system
  that rely on devices under this i2c bus we'll need to make sure they
  can handle the fact that they'll be initted later now.

Changes in v2:
- Renamed to i2c-arbitrator-cros-ec.
- Documented "microsecond" properties as optional; removed
  "bus-arbitration" prefix since it was just extra wordy.
- Split GPIOs into two properties to make it cleaner.
- Capitalized I2C in freeform text.
- Get 'active low' from device tree.

 .../bindings/i2c/i2c-arbitrator-cros-ec.txt|  76 +++
 drivers/i2c/muxes/Kconfig  |  11 +
 drivers/i2c/muxes/Makefile |   2 +
 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c | 222 +
 4 files changed, 311 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
 create mode 100644 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt 
b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
new file mode 100644
index 000..1f893e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
@@ -0,0 +1,76 @@
+GPIO-based Arbitration used by the ARM Chromebook (exynos5250-snow)
+===
+This uses GPIO lines between the AP (Application Processor) and an attached
+EC (Embedded Controller) which both want to talk on the same I2C bus as master.
+
+The AP and EC each have a 'bus claim' line, which is an output that the
+other can see. These are both active low, with pull-ups enabled.
+
+- AP_CLAIM: output from AP, signalling to the EC that the AP wants the bus
+- EC_CLAIM: output from EC, signalling to the AP that the EC wants the bus
+
+This mechanism is used instead of standard I2C multimaster to avoid some of the
+subtle driver and silicon bugs that are often present with I2C multimaster.
+
+
+Algorithm:
+
+The basic algorithm is to assert your line when you want the bus, then make
+sure that the other side doesn't want it also. A detailed explanation is best
+done with an example.
+
+Let's say the AP wants to claim the bus. It:
+1. Asserts AP_CLAIM.
+2. Waits a little bit for the other side to notice (slew time, say 10
+   microseconds).
+3. Checks EC_CLAIM. If this is not asserted then the AP has the bus and we are
+   done.
+4. Otherwise, wait for a few milliseconds and see if EC_CLAIM is released.
+5. If not, back off, release the claim and wait for a few more milliseconds.
+6. Go back to 1 (until retry time has expired).
+
+
+Required properties:
+- compatible: i2c-arbitrator-cros-ec
+- ap-claim-gpio: The GPIO that we (the AP) use to claim the bus.
+- ec-claim-gpio: The GPIO that the other side (the EC) uses the claim the bus.
+- Standard I2C mux properties. See mux.txt in this directory.
+- Single I2C child bus node at reg 0. See mux.txt in this directory.
+
+Optional properties:
+- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 us.
+- wait-retry-us: we'll attempt another claim after this many microseconds.
+Default is 3000 us.
+- wait-free-us: we'll give up after this many microseconds. Default is 5 
us.
+
+
+Example:
+   i2c@12CA {
+   compatible = "acme,some-i2c-device";
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+
+   i2c-arbitrator {
+   compatible = "i2c-arbitrator-cros-ec";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   i2c-parent = <&{/i2c@12CA}>;
+
+   ap-claim-gpio = < 3 1 0x1 0>;
+   ec-claim-gpio = < 4 0 0x10003 0>;
+   slew-delay-us = <10>;
+   wait-retry-us = <3000>;
+   wait-free-us = <5>;
+
+   i2c@0 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   

[PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Doug Anderson]

The i2c-arbitrator-cros-ec driver implements the arbitration scheme
that the Embedded Controller (EC) on the ARM Chromebook expects to use
for bus multimastering.  This i2c-arbitrator-cros-ec driver could also
be used in other places where standard I2C bus arbitration can't be
used and two extra GPIOs are available for arbitration.

This driver is based on code that Simon Glass added to the i2c-s3c2410
driver in the Chrome OS kernel 3.4 tree.  The current incarnation as a
mux driver is as suggested by Grant Likely.  See
https://patchwork.kernel.org/patch/1877311/ for some history.

Signed-off-by: Doug Anderson diand...@chromium.org
Signed-off-by: Simon Glass s...@chromium.org
Signed-off-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
Reviewed-by: Stephen Warren swar...@nvidia.com
Tested-by: Naveen Krishna Chatradhi ch.nav...@samsung.com
---
Changes in v4: None
Changes in v3:
- Handle of_find_i2c_adapter_by_node() failure more properly by
  changing init order.
- Don't warn on -EPROBE_DEFER from calls that could return it.
- Move to module_platform_driver().  As we pull in parts of the system
  that rely on devices under this i2c bus we'll need to make sure they
  can handle the fact that they'll be initted later now.

Changes in v2:
- Renamed to i2c-arbitrator-cros-ec.
- Documented microsecond properties as optional; removed
  bus-arbitration prefix since it was just extra wordy.
- Split GPIOs into two properties to make it cleaner.
- Capitalized I2C in freeform text.
- Get 'active low' from device tree.

 .../bindings/i2c/i2c-arbitrator-cros-ec.txt|  76 +++
 drivers/i2c/muxes/Kconfig  |  11 +
 drivers/i2c/muxes/Makefile |   2 +
 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c | 222 +
 4 files changed, 311 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
 create mode 100644 drivers/i2c/muxes/i2c-arbitrator-cros-ec.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt 
b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
new file mode 100644
index 000..1f893e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator-cros-ec.txt
@@ -0,0 +1,76 @@
+GPIO-based Arbitration used by the ARM Chromebook (exynos5250-snow)
+===
+This uses GPIO lines between the AP (Application Processor) and an attached
+EC (Embedded Controller) which both want to talk on the same I2C bus as master.
+
+The AP and EC each have a 'bus claim' line, which is an output that the
+other can see. These are both active low, with pull-ups enabled.
+
+- AP_CLAIM: output from AP, signalling to the EC that the AP wants the bus
+- EC_CLAIM: output from EC, signalling to the AP that the EC wants the bus
+
+This mechanism is used instead of standard I2C multimaster to avoid some of the
+subtle driver and silicon bugs that are often present with I2C multimaster.
+
+
+Algorithm:
+
+The basic algorithm is to assert your line when you want the bus, then make
+sure that the other side doesn't want it also. A detailed explanation is best
+done with an example.
+
+Let's say the AP wants to claim the bus. It:
+1. Asserts AP_CLAIM.
+2. Waits a little bit for the other side to notice (slew time, say 10
+   microseconds).
+3. Checks EC_CLAIM. If this is not asserted then the AP has the bus and we are
+   done.
+4. Otherwise, wait for a few milliseconds and see if EC_CLAIM is released.
+5. If not, back off, release the claim and wait for a few more milliseconds.
+6. Go back to 1 (until retry time has expired).
+
+
+Required properties:
+- compatible: i2c-arbitrator-cros-ec
+- ap-claim-gpio: The GPIO that we (the AP) use to claim the bus.
+- ec-claim-gpio: The GPIO that the other side (the EC) uses the claim the bus.
+- Standard I2C mux properties. See mux.txt in this directory.
+- Single I2C child bus node at reg 0. See mux.txt in this directory.
+
+Optional properties:
+- slew-delay-us: microseconds to wait for a GPIO to go high. Default is 10 us.
+- wait-retry-us: we'll attempt another claim after this many microseconds.
+Default is 3000 us.
+- wait-free-us: we'll give up after this many microseconds. Default is 5 
us.
+
+
+Example:
+   i2c@12CA {
+   compatible = acme,some-i2c-device;
+   #address-cells = 1;
+   #size-cells = 0;
+   };
+
+   i2c-arbitrator {
+   compatible = i2c-arbitrator-cros-ec;
+   #address-cells = 1;
+   #size-cells = 0;
+
+   i2c-parent = {/i2c@12CA};
+
+   ap-claim-gpio = gpf0 3 1 0x1 0;
+   ec-claim-gpio = gpe0 4 0 0x10003 0;
+   slew-delay-us = 10;
+   wait-retry-us = 3000;
+   wait-free-us = 5;
+
+   i2c@0 {
+   reg = 0;
+   #address-cells = 1;
+  

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Stephen Warren]

On 03/13/2013 10:36 AM, Doug Anderson wrote:
 The i2c-arbitrator-cros-ec driver implements the arbitration scheme
 that the Embedded Controller (EC) on the ARM Chromebook expects to use
 for bus multimastering.  This i2c-arbitrator-cros-ec driver could also
 be used in other places where standard I2C bus arbitration can't be
 used and two extra GPIOs are available for arbitration.
 
 This driver is based on code that Simon Glass added to the i2c-s3c2410
 driver in the Chrome OS kernel 3.4 tree.  The current incarnation as a
 mux driver is as suggested by Grant Likely.  See
 https://patchwork.kernel.org/patch/1877311/ for some history.
...
 Changes in v4: None

Isn't this 'PATCH V3 REPOST' then?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Doug Anderson]

Stephen,

On Wed, Mar 13, 2013 at 9:53 AM, Stephen Warren swar...@wwwdotorg.org wrote:
 Changes in v4: None

 Isn't this 'PATCH V3 REPOST' then?

In this case part 2 in the patch series changes but not parts 1 and 3.
 I could have just reposted part 2 with a higher version, but that
makes it a little harder to piece together all of the parts of the
series so I decided to repost all 3.  I can do differently in the
future if you prefer, but my understanding was that it was a matter of
preference/judgement call.

Thanks!  :)

-Doug
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver

[Stephen Warren]

On 03/13/2013 10:59 AM, Doug Anderson wrote:
 Stephen,
 
 On Wed, Mar 13, 2013 at 9:53 AM, Stephen Warren swar...@wwwdotorg.org wrote:
 Changes in v4: None

 Isn't this 'PATCH V3 REPOST' then?
 
 In this case part 2 in the patch series changes but not parts 1 and 3.
  I could have just reposted part 2 with a higher version, but that
 makes it a little harder to piece together all of the parts of the
 series so I decided to repost all 3.  I can do differently in the
 future if you prefer, but my understanding was that it was a matter of
 preference/judgement call.

Oh no you're quite right. I didn't notice it was a 3-part series, since
I only got patch 1/3 filtered into my inbox; I guess I wasn't CC'd on
the rest. Sorry for the noise.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/