Date:	Wed, 13 Feb 2013 10:45:59 -0800
From:	Olof Johansson <olof@...om.net>
To:	Doug Anderson <dianders@...omium.org>
Cc:	Wolfram Sang <w.sang@...gutronix.de>,
	Simon Glass <sjg@...omium.org>,
	Naveen Krishna Chatradhi <ch.naveen@...sung.com>,
	Grant Likely <grant.likely@...retlab.ca>,
	Yuvaraj Kumar <yuvaraj.cd@...il.com>,
	Ben Dooks <ben.dooks@...ethink.co.uk>,
	u.kleine-koenig@...gutronix.de,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Girish Shivananjappa <girish.shivananjappa@...aro.org>,
	bhushan.r@...sung.com, sreekumar.c@...sung.com,
	Prashanth G <prashanth.g@...sung.com>,
	Daniel Kurtz <djkurtz@...omium.org>,
	Grant Grundler <grundler@...omium.org>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	"Ben Dooks (embedded platforms)" <ben-linux@...ff.org>,
	Jean Delvare <khali@...ux-fr.org>,
	Peter Korsgaard <peter.korsgaard@...co.com>,
	Stephen Warren <swarren@...dia.com>,
	Guenter Roeck <guenter.roeck@...csson.com>,
	devicetree-discuss@...ts.ozlabs.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org
Subject: Re: [PATCH v1 1/4] i2c: mux: Add i2c-arbitrator 'mux' driver

Hi,

The driver pieces have been reviewed a bit already, but I have a question on
the bindings below.

On Wed, Feb 13, 2013 at 10:02:09AM -0800, Doug Anderson wrote:
> The i2c-arbitrator driver implements the arbitration scheme that the
> Embedded Controller (EC) on the ARM Chromebook expects to use for bus
> multimastering.  This i2c-arbitrator 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 <dianders@...omium.org>
> Signed-off-by: Simon Glass <sjg@...omium.org>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@...sung.com>
> ---
> Changes in v1: None
> 
>  .../devicetree/bindings/i2c/i2c-arbitrator.txt     |  76 +++++++
>  drivers/i2c/muxes/Kconfig                          |  11 +
>  drivers/i2c/muxes/Makefile                         |   1 +
>  drivers/i2c/muxes/i2c-arbitrator.c                 | 242 +++++++++++++++++++++
>  4 files changed, 330 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-arbitrator.txt
>  create mode 100644 drivers/i2c/muxes/i2c-arbitrator.c
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-arbitrator.txt b/Documentation/devicetree/bindings/i2c/i2c-arbitrator.txt
> new file mode 100644
> index 0000000..bb9cca7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-arbitrator.txt
> @@ -0,0 +1,76 @@
> +GPIO-based Arbitration
> +======================
> +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
> +- bus-arbitration-gpios: Two GPIOs to use with the GPIO-based bus
> +    arbitration protocol.  The first should be an output, and is used to
> +    claim the I2C bus for us (AP_CLAIM).  The second should be an input and
> +    signals that the other side wants to claim the bus (EC_CLAIM).
> +- bus-arbitration-slew-delay-us: microseconds to wait for a GPIO to go high.
> +- bus-arbitration-wait-retry-us: we'll attempt another claim after this many
> +    microseconds.
> +- bus-arbitration-wait-free-us: we'll give up after this many microseconds.
> +- Standard I2C mux properties. See mux.txt in this directory.
> +- Single I2C child bus node at reg 0. See mux.txt in this directory.
> +
> +
> +Example:
> +	i2c@...A0000 {
> +		compatible = "acme,some-i2c-device";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +	};
> +
> +	i2c-arbitrator {
> +		compatible = "i2c-arbitrator";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		i2c-parent = <&{/i2c@...A0000}>;

Why use this custom i2c-parent property? The arbitrator should just be
under the i2c controller in the device tree, and thus parent would be
derived from the topology there.


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

Powered by blists - more mailing lists