Re: [PATCH v3 1/4] i2c: Switch to using gpiod interface for gpio bus recovery

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Wed, 2017-08-30 at 14:17 +0800, Phil Reid wrote:
> Currently the i2c gpio recovery code uses gpio integer interface
> instead of the gpiod. This change switch the core code to use
> the gpiod while still retaining compatibility with the gpio integer
> interface. This will allow individual driver to be updated and tested
> individual to switch to using the gpiod interface.

>  static int i2c_get_gpios_for_recovery(struct i2c_adapter *adap)
> @@ -158,6 +158,7 @@ static int i2c_get_gpios_for_recovery(struct
> i2c_adapter *adap)
>  		dev_warn(dev, "Can't get SCL gpio: %d\n", bri-
> >scl_gpio);
>  		return ret;
>  	}
> +	bri->scl_gpiod = gpio_to_desc(bri->scl_gpio);
>  
>  	if (bri->get_sda) {
>  		if (gpio_request_one(bri->sda_gpio, GPIOF_IN, "i2c-
> sda")) {
> @@ -167,6 +168,7 @@ static int i2c_get_gpios_for_recovery(struct
> i2c_adapter *adap)
>  			bri->get_sda = NULL;
>  		}
>  	}

> +	bri->sda_gpiod = gpio_to_desc(bri->sda_gpio);

Shouldn't it be inside conditional?

>  	return ret;
>  }
> @@ -175,10 +177,13 @@ static void i2c_put_gpios_for_recovery(struct
> i2c_adapter *adap)
>  {
>  	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
>  
> -	if (bri->get_sda)
> +	if (bri->get_sda) {
>  		gpio_free(bri->sda_gpio);
> +		bri->sda_gpiod = NULL;
> +	}
>  
>  	gpio_free(bri->scl_gpio);
> +	bri->scl_gpiod = NULL;

Can we go other way around, i.e. put descriptors and assign plain
integers to something like -ENOENT?

>  }


> +	if ((bri->scl_gpiod) &&

Redundant parens.

> +	    (bri->recover_bus == i2c_generic_scl_recovery)) {

Ditto, though here it might be slightly better to read.

> +		bri->get_scl = get_scl_gpio_value;
> +		bri->set_scl = set_scl_gpio_value;
> +		if (bri->sda_gpiod)
> +			bri->get_sda = get_sda_gpio_value;
> +		return;
> +	}

>  	int scl_gpio;
>  	int sda_gpio;
> +	struct gpio_desc *scl_gpiod;
> +	struct gpio_desc *sda_gpiod;

I think we even could get rid of plain integers completely.
In case some call needs it we can derive it still from the descriptor.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy