Hi Linus,
On 2019-06-01 18:59, Linus Walleij wrote:
> Instead of complex code picking GPIOs out of the device tree
> and keeping track of polarity for each GPIO line, use descriptors
> and pull polarity handling into the gpiolib.
>
> We look for "our-claim" and "their-claim" since the gpiolib
> code will try e.g. "our-claim-gpios" and "our-claim-gpio" in
> turn to locate these GPIO lines from the device tree.
>
> Cc: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Cc: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> Cc: Doug Anderson <dianders@xxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
Samsung Snow Chromebook works fine with linux-next 20190603 and this patch.
Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> ---
> ChangeLog v1->v2:
> - Print some errors if we fail to obtain GPIOs.
> - Switch to devm_gpiod_get_index() and !IS_ERR() on the result
> to check for an unsupported multimaster set-up.
> - Especially handle if the second master returns -EPROBE_DEFER
> even in the case of the optional GPIO that we explicitly bail
> out on if found, as it may be a sign that the GPIO controllers
> are not yet up.
> ---
> drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 79 ++++++++--------------
> 1 file changed, 27 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> index 812b8cff265f..a664f637bc3d 100644
> --- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> +++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> @@ -15,12 +15,11 @@
> */
>
> #include <linux/delay.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/kernel.h>
> #include <linux/i2c.h>
> #include <linux/i2c-mux.h>
> #include <linux/module.h>
> -#include <linux/of_gpio.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
>
> @@ -28,22 +27,16 @@
> /**
> * struct i2c_arbitrator_data - Driver data for I2C arbitrator
> *
> - * @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.
> + * @our_gpio: GPIO descriptor we'll use to claim.
> + * @their_gpio: GPIO descriptor that the other side will use to claim.
> * @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 {
> - int our_gpio;
> - int our_gpio_release;
> - int their_gpio;
> - int their_gpio_release;
> + struct gpio_desc *our_gpio;
> + struct gpio_desc *their_gpio;
> unsigned int slew_delay_us;
> unsigned int wait_retry_us;
> unsigned int wait_free_us;
> @@ -64,15 +57,15 @@ static int i2c_arbitrator_select(struct i2c_mux_core *muxc, u32 chan)
> stop_time = jiffies + usecs_to_jiffies(arb->wait_free_us) + 1;
> do {
> /* Indicate that we want to claim the bus */
> - gpio_set_value(arb->our_gpio, !arb->our_gpio_release);
> + gpiod_set_value(arb->our_gpio, 1);
> udelay(arb->slew_delay_us);
>
> /* Wait for the other master to release it */
> stop_retry = jiffies + usecs_to_jiffies(arb->wait_retry_us) + 1;
> while (time_before(jiffies, stop_retry)) {
> - int gpio_val = !!gpio_get_value(arb->their_gpio);
> + int gpio_val = gpiod_get_value(arb->their_gpio);
>
> - if (gpio_val == arb->their_gpio_release) {
> + if (!gpio_val) {
> /* We got it, so return */
> return 0;
> }
> @@ -81,13 +74,13 @@ static int i2c_arbitrator_select(struct i2c_mux_core *muxc, u32 chan)
> }
>
> /* It didn't release, so give up, wait, and try again */
> - gpio_set_value(arb->our_gpio, arb->our_gpio_release);
> + gpiod_set_value(arb->our_gpio, 0);
>
> usleep_range(arb->wait_retry_us, arb->wait_retry_us * 2);
> } while (time_before(jiffies, stop_time));
>
> /* Give up, release our claim */
> - gpio_set_value(arb->our_gpio, arb->our_gpio_release);
> + gpiod_set_value(arb->our_gpio, 0);
> udelay(arb->slew_delay_us);
> dev_err(muxc->dev, "Could not claim bus, timeout\n");
> return -EBUSY;
> @@ -103,7 +96,7 @@ static int i2c_arbitrator_deselect(struct i2c_mux_core *muxc, u32 chan)
> const struct i2c_arbitrator_data *arb = i2c_mux_priv(muxc);
>
> /* Release the bus and wait for the other master to notice */
> - gpio_set_value(arb->our_gpio, arb->our_gpio_release);
> + gpiod_set_value(arb->our_gpio, 0);
> udelay(arb->slew_delay_us);
>
> return 0;
> @@ -116,8 +109,7 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
> struct device_node *parent_np;
> struct i2c_mux_core *muxc;
> struct i2c_arbitrator_data *arb;
> - enum of_gpio_flags gpio_flags;
> - unsigned long out_init;
> + struct gpio_desc *dummy;
> int ret;
>
> /* We only support probing from device tree; no platform_data */
> @@ -138,45 +130,28 @@ static int i2c_arbitrator_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, muxc);
>
> - /* Request GPIOs */
> - ret = of_get_named_gpio_flags(np, "our-claim-gpio", 0, &gpio_flags);
> - if (!gpio_is_valid(ret)) {
> - if (ret != -EPROBE_DEFER)
> - dev_err(dev, "Error getting our-claim-gpio\n");
> - return ret;
> - }
> - arb->our_gpio = ret;
> - arb->our_gpio_release = !!(gpio_flags & OF_GPIO_ACTIVE_LOW);
> - out_init = (gpio_flags & OF_GPIO_ACTIVE_LOW) ?
> - GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW;
> - ret = devm_gpio_request_one(dev, arb->our_gpio, out_init,
> - "our-claim-gpio");
> - if (ret) {
> - if (ret != -EPROBE_DEFER)
> - dev_err(dev, "Error requesting our-claim-gpio\n");
> - return ret;
> + /* Request GPIOs, our GPIO as unclaimed to begin with */
> + arb->our_gpio = devm_gpiod_get(dev, "our-claim", GPIOD_OUT_LOW);
> + if (IS_ERR(arb->our_gpio)) {
> + dev_err(dev, "could not get \"our-claim\" GPIO (%ld)\n",
> + PTR_ERR(arb->our_gpio));
> + return PTR_ERR(arb->our_gpio);
> }
>
> - ret = of_get_named_gpio_flags(np, "their-claim-gpios", 0, &gpio_flags);
> - if (!gpio_is_valid(ret)) {
> - if (ret != -EPROBE_DEFER)
> - dev_err(dev, "Error getting their-claim-gpio\n");
> - return ret;
> - }
> - arb->their_gpio = ret;
> - arb->their_gpio_release = !!(gpio_flags & OF_GPIO_ACTIVE_LOW);
> - ret = devm_gpio_request_one(dev, arb->their_gpio, GPIOF_IN,
> - "their-claim-gpio");
> - if (ret) {
> - if (ret != -EPROBE_DEFER)
> - dev_err(dev, "Error requesting their-claim-gpio\n");
> - return ret;
> + arb->their_gpio = devm_gpiod_get(dev, "their-claim", GPIOD_IN);
> + if (IS_ERR(arb->their_gpio)) {
> + dev_err(dev, "could not get \"their-claim\" GPIO (%ld)\n",
> + PTR_ERR(arb->their_gpio));
> + return PTR_ERR(arb->their_gpio);
> }
>
> /* At the moment we only support a single two master (us + 1 other) */
> - if (gpio_is_valid(of_get_named_gpio(np, "their-claim-gpios", 1))) {
> + dummy = devm_gpiod_get_index(dev, "their-claim", 1, GPIOD_IN);
> + if (!IS_ERR(dummy)) {
> dev_err(dev, "Only one other master is supported\n");
> return -EINVAL;
> + } else if (PTR_ERR(dummy) == -EPROBE_DEFER) {
> + return -EPROBE_DEFER;
> }
>
> /* Arbitration parameters */
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
