From: Wolfram Sang <wsa@the-dreams.de>
To: Doug Anderson <dianders@chromium.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>,
Simon Glass <sjg@chromium.org>,
Naveen Krishna Chatradhi <ch.naveen@samsung.com>,
grant.likely@secretlab.ca, yuvaraj.cd@gmail.com,
ben.dooks@codethink.co.uk, u.kleine-koenig@pengutronix.de,
broonie@opensource.wolfsonmicro.com,
girish.shivananjappa@linaro.org, bhushan.r@samsung.com,
sreekumar.c@samsung.com, prashanth.g@samsung.com, olof@lixom.net,
djkurtz@chromium.org, swarren@wwwdotorg.org, linux@roeck-us.net,
Rob Herring <rob.herring@calxeda.com>,
Rob Landley <rob@landley.net>,
"Ben Dooks (embedded platforms)" <ben-linux@fluff.org>,
Stephen Warren <swarren@nvidia.com>,
Jean Delvare <khali@linux-fr.org>,
Peter Korsgaard <peter.korsgaard@barco.com>,
Guenter Roeck <guenter.roeck@ericsson.com>,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org
Subject: Re: [PATCH v5 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver
Date: Tue, 16 Apr 2013 11:36:33 +0200 [thread overview]
Message-ID: <20130416093633.GC16978@the-dreams.de> (raw)
In-Reply-To: <1365543270-10736-1-git-send-email-dianders@chromium.org>
Doug,
On Tue, Apr 09, 2013 at 02:34:28PM -0700, Doug Anderson wrote:
> The i2c-arb-gpio-challenge driver implements an I2C arbitration scheme
> where masters need to claim the bus with a GPIO before they can start
> a transcation. This should generally only be used when standard I2C
> multimaster isn't appropriate for some reason (errata/bugs).
>
> 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@chromium.org>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
Mostly good, except for some documentation updates.
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt b/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
> new file mode 100644
> index 0000000..726e099
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-arb-gpio-challenge.txt
> @@ -0,0 +1,80 @@
> +GPIO-based I2C Arbitration
> +==========================
> +This uses GPIO lines to arbitrate who is the master of an I2C bus in a
> +multimaster situation.
"uses GPIO lines and a challange & response mechanism" or something like
that. There might be other GPIO based arbitrations in the future (though
I hope there won't).
> +
> +In many cases using GPIOs to arbitrate is not needed and a design can use
> +the standard I2C multi-master rules. Using GPIOs is generally useful in
> +the case where there is a device on the bus that has errata and/or bugs
> +that makes standard multimaster mode not feasible.
I like this paragraph!
...
> +- their-claim-gpios: The GPIOs that the other sides use the claim the bus.
> + Note that some implementations may only support a single other master.
Stronger? "Currently, only one other master is supported"?
...
> diff --git a/drivers/i2c/muxes/Kconfig b/drivers/i2c/muxes/Kconfig
> index 0be5b83..ab4dcaf 100644
> --- a/drivers/i2c/muxes/Kconfig
> +++ b/drivers/i2c/muxes/Kconfig
> @@ -5,6 +5,17 @@
> menu "Multiplexer I2C Chip support"
> depends on I2C_MUX
>
> +config I2C_ARB_GPIO_CHALLENGE
> + tristate "GPIO-based I2C arbitration"
> + depends on GENERIC_GPIO && OF
> + help
> + If you say yes to this option, support will be included for an
> + I2C multimaster arbitration scheme using GPIOs where masters have
> + to claim the bus by asserting a GPIO.
"callenge & response"?
...
> diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> new file mode 100644
> index 0000000..bda020a
> --- /dev/null
> +++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> @@ -0,0 +1,252 @@
> +/*
> + * GPIO-based I2C Arbitration
"callenge & response"?
...
> + *
> + * Copyright (C) 2012 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-mux.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of_i2c.h>
> +#include <linux/of_gpio.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +
> +/**
> + * struct i2c_arbitrator_data - Driver data for I2C arbitrator
> + *
> + * @parent: Parent adapter
> + * @child: Child bus
> + * @our_gpio: GPIO we'll use to claim.
> + * @our_gpio_release: 0 if active high; 1 if active low; AKA if the GPIO ==
> + * this then consider it released.
> + * @their_gpio: GPIO that the other side will use to claim.
> + * @their_gpio_release: 0 if active high; 1 if active low; AKA if the GPIO ==
> + * this then consider it released.
> + * @slew_delay_us: microseconds to wait for a GPIO to go high.
> + * @wait_retry_us: we'll attempt another claim after this many microseconds.
> + * @wait_free_us: we'll give up after this many microseconds.
> + */
> +
> +struct i2c_arbitrator_data {
> + struct i2c_adapter *parent;
> + struct i2c_adapter *child;
> +
No empty line.
> + int our_gpio;
> + int our_gpio_release;
> + int their_gpio;
> + int their_gpio_release;
> + unsigned int slew_delay_us;
> + unsigned int wait_retry_us;
> + unsigned int wait_free_us;
> +};
Single space as indentaion after "int".
Other than that, looks fine to me.
Thanks!
next prev parent reply other threads:[~2013-04-16 9:37 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1360887677-20758-1-git-send-email-dianders@chromium.org>
2013-02-15 0:21 ` [PATCH v2 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow Doug Anderson
2013-02-15 0:21 ` [PATCH v2 3/3] ARM: dts: Add sbs-battery " Doug Anderson
2013-02-15 16:39 ` [PATCH v2 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver Stephen Warren
[not found] ` <CAD=FV=W9WwSsid_KqtDRmAkFXnneRXu5zcakDB3t4hLhOpuCtw@mail.gmail.com>
[not found] ` <511E7295.4000902@wwwdotorg.org>
2013-02-15 17:44 ` Mark Brown
2013-02-15 19:46 ` [PATCH v3 " Doug Anderson
2013-02-15 19:46 ` [PATCH v3 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow Doug Anderson
2013-03-05 23:48 ` Naveen Krishna Ch
2013-03-13 7:30 ` Kukjin Kim
2013-03-13 15:14 ` Doug Anderson
2013-02-15 19:46 ` [PATCH v3 3/3] ARM: dts: Add sbs-battery " Doug Anderson
2013-03-05 23:49 ` Naveen Krishna Ch
2013-02-15 21:31 ` [PATCH v3 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver Stephen Warren
2013-03-11 16:05 ` Doug Anderson
2013-03-13 16:36 ` [PATCH v4 " Doug Anderson
2013-03-13 16:36 ` [PATCH v4 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow Doug Anderson
2013-03-13 16:36 ` [PATCH v4 3/3] ARM: dts: Add sbs-battery " Doug Anderson
2013-03-13 16:53 ` [PATCH v4 1/3] i2c: mux: Add i2c-arbitrator-cros-ec 'mux' driver Stephen Warren
2013-03-13 16:59 ` Doug Anderson
2013-03-13 17:29 ` Stephen Warren
[not found] ` <CAD=FV=UW4wGFqsg2B3MmC24jhnba5PSvCh8Wck-V6iuYk+Xifw@mail.gmail.com>
[not found] ` <20130329115821.GC6359@the-dreams.de>
[not found] ` <CAD=FV=XyBZXD1dsOy4kY3CjkFiJXCO+taAJ-zZ8op6V-z9J8jQ@mail.gmail.com>
[not found] ` <20130403191938.GA7875@the-dreams.de>
[not found] ` <CAPnjgZ0bt+ppAMAB1RtoeUKrzUjxxZ3+=GQwHqxByce7fmOLDg@mail.gmail.com>
2013-04-05 20:03 ` Stephen Warren
2013-04-06 18:30 ` Guenter Roeck
2013-04-06 20:11 ` Wolfram Sang
2013-04-07 18:10 ` Guenter Roeck
2013-04-08 9:55 ` Wolfram Sang
2013-04-08 10:26 ` Wolfram Sang
[not found] ` <1365538361-2763-1-git-send-email-dianders@chromium.org>
2013-04-09 20:12 ` [PATCH v5 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow Doug Anderson
2013-04-09 20:12 ` [PATCH v5 3/3] ARM: dts: Add sbs-battery " Doug Anderson
2013-04-09 21:34 ` [PATCH v5 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver Doug Anderson
2013-04-09 21:34 ` [PATCH v5 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow Doug Anderson
2013-04-10 10:59 ` Kukjin Kim
2013-04-10 11:02 ` Wolfram Sang
2013-04-10 14:12 ` Kukjin Kim
2013-04-09 21:34 ` [PATCH v5 3/3] ARM: dts: Add sbs-battery " Doug Anderson
2013-04-16 9:36 ` Wolfram Sang [this message]
2013-04-16 9:44 ` [PATCH v5 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver Peter Korsgaard
2013-04-16 13:38 ` Guenter Roeck
2013-04-16 15:42 ` Stephen Warren
2013-04-16 16:29 ` [PATCH v6 " Doug Anderson
2013-04-16 16:29 ` [PATCH v6 2/3] ARM: dts: Add i2c-arbitrator bus for exynos5250-snow Doug Anderson
2013-04-16 16:35 ` Olof Johansson
2013-04-16 16:29 ` [PATCH v6 3/3] ARM: dts: Add sbs-battery " Doug Anderson
2013-04-16 16:36 ` Olof Johansson
2013-04-16 16:34 ` [PATCH v6 1/3] i2c: mux: Add i2c-arb-gpio-challenge 'mux' driver Olof Johansson
2013-04-17 9:34 ` Wolfram Sang
2013-04-16 16:45 ` Guenter Roeck
2013-04-17 9:32 ` Wolfram Sang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130416093633.GC16978@the-dreams.de \
--to=wsa@the-dreams.de \
--cc=ben-linux@fluff.org \
--cc=ben.dooks@codethink.co.uk \
--cc=bhushan.r@samsung.com \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=ch.naveen@samsung.com \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=dianders@chromium.org \
--cc=djkurtz@chromium.org \
--cc=girish.shivananjappa@linaro.org \
--cc=grant.likely@secretlab.ca \
--cc=guenter.roeck@ericsson.com \
--cc=kgene.kim@samsung.com \
--cc=khali@linux-fr.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=olof@lixom.net \
--cc=peter.korsgaard@barco.com \
--cc=prashanth.g@samsung.com \
--cc=rob.herring@calxeda.com \
--cc=rob@landley.net \
--cc=sjg@chromium.org \
--cc=sreekumar.c@samsung.com \
--cc=swarren@nvidia.com \
--cc=swarren@wwwdotorg.org \
--cc=u.kleine-koenig@pengutronix.de \
--cc=yuvaraj.cd@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).