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 <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>
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!
--
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/
