[5/5] smiapp: Switch to gpiod API for GPIO control

Commit Message

Sakari Ailus Aug. 31, 2016, 7:42 a.m. UTC

Switch from the old gpio API to the new descriptor based gpiod API.

Signed-off-by: Sakari Ailus <[e-mail]>
---
 drivers/media/i2c/smiapp/smiapp-core.c | 28 +++++++++++++++-------------
 drivers/media/i2c/smiapp/smiapp.h      |  1 +
 2 files changed, 16 insertions(+), 13 deletions(-)

Comments

Sebastian Reichel Aug. 31, 2016, 12:09 p.m. UTC | #1

Hi Sakari,

On Wed, Aug 31, 2016 at 10:42:05AM +0300, Sakari Ailus wrote:
> -	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
> +	if (client->dev.of_node) {
> +		sensor->xshutdown =
> +			devm_gpiod_get_optional(&client->dev, "xshutdown",
> +						GPIOD_OUT_LOW);
> +	} else if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
>  		rval = devm_gpio_request_one(
>  			&client->dev, sensor->hwcfg->xshutdown, 0,
>  			"SMIA++ xshutdown");
> @@ -2581,8 +2582,13 @@ static int smiapp_init(struct smiapp_sensor *sensor)
>  				sensor->hwcfg->xshutdown);
>  			return rval;
>  		}
> +
> +		sensor->xshutdown = gpio_to_desc(sensor->hwcfg->xshutdown);
>  	}

You can drop the devm_gpio_request_one() part and xshutdown from
smiapp_platform_data. The gpiod consumer interface can also be
used with data provided from boardfiles as documented in
Documentation/gpio/board.txt, section "Platform Data". It basically
works like assigning regulators to devices from platform data.

You will obviously have to change every platform_device users of
smiapp, but it looks like upstream has none:

~/linux/arch $ git grep smiapp_platform_data || echo "Not found"
Not found

-- Sebastian

Sakari Ailus Aug. 31, 2016, 12:42 p.m. UTC | #2

Hi Sebastian,

Thanks for the review!

On 08/31/16 15:09, Sebastian Reichel wrote:
> Hi Sakari,
> 
> On Wed, Aug 31, 2016 at 10:42:05AM +0300, Sakari Ailus wrote:
>> -	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
>> +	if (client->dev.of_node) {
>> +		sensor->xshutdown =
>> +			devm_gpiod_get_optional(&client->dev, "xshutdown",
>> +						GPIOD_OUT_LOW);
>> +	} else if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
>>  		rval = devm_gpio_request_one(
>>  			&client->dev, sensor->hwcfg->xshutdown, 0,
>>  			"SMIA++ xshutdown");
>> @@ -2581,8 +2582,13 @@ static int smiapp_init(struct smiapp_sensor *sensor)
>>  				sensor->hwcfg->xshutdown);
>>  			return rval;
>>  		}
>> +
>> +		sensor->xshutdown = gpio_to_desc(sensor->hwcfg->xshutdown);
>>  	}
> 
> You can drop the devm_gpio_request_one() part and xshutdown from
> smiapp_platform_data. The gpiod consumer interface can also be
> used with data provided from boardfiles as documented in
> Documentation/gpio/board.txt, section "Platform Data". It basically
> works like assigning regulators to devices from platform data.

Good point. I'll fix that.

Patch

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index aaf5299..d07e060 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -24,8 +24,8 @@ 
 #include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/module.h>
-#include <linux/of_gpio.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/smiapp.h>
@@ -1212,8 +1212,7 @@  static int smiapp_power_on(struct smiapp_sensor *sensor)
 	}
 	usleep_range(1000, 1000);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 1);
+	gpiod_set_value(sensor->xshutdown, 1);
 
 	sleep = SMIAPP_RESET_DELAY(sensor->hwcfg->ext_clk);
 	usleep_range(sleep, sleep);
@@ -1322,8 +1321,7 @@  static int smiapp_power_on(struct smiapp_sensor *sensor)
 	return 0;
 
 out_cci_addr_fail:
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -1348,8 +1346,7 @@  static void smiapp_power_off(struct smiapp_sensor *sensor)
 			     SMIAPP_REG_U8_SOFTWARE_RESET,
 			     SMIAPP_SOFTWARE_RESET);
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown))
-		gpio_set_value(sensor->hwcfg->xshutdown, 0);
+	gpiod_set_value(sensor->xshutdown, 0);
 	if (sensor->hwcfg->set_xclk)
 		sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 	else
@@ -2571,7 +2568,11 @@  static int smiapp_init(struct smiapp_sensor *sensor)
 		}
 	}
 
-	if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
+	if (client->dev.of_node) {
+		sensor->xshutdown =
+			devm_gpiod_get_optional(&client->dev, "xshutdown",
+						GPIOD_OUT_LOW);
+	} else if (gpio_is_valid(sensor->hwcfg->xshutdown)) {
 		rval = devm_gpio_request_one(
 			&client->dev, sensor->hwcfg->xshutdown, 0,
 			"SMIA++ xshutdown");
@@ -2581,8 +2582,13 @@  static int smiapp_init(struct smiapp_sensor *sensor)
 				sensor->hwcfg->xshutdown);
 			return rval;
 		}
+
+		sensor->xshutdown = gpio_to_desc(sensor->hwcfg->xshutdown);
 	}
 
+	if (!sensor->xshutdown)
+		dev_dbg(&client->dev, "no xshutdown GPIO available\n");
+
 	rval = smiapp_power_on(sensor);
 	if (rval)
 		return -ENODEV;
@@ -3019,9 +3025,6 @@  static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
 	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
-	/* xshutdown GPIO is optional */
-	hwcfg->xshutdown = of_get_named_gpio(dev->of_node, "reset-gpios", 0);
-
 	/* NVM size is not mandatory */
 	of_property_read_u32(dev->of_node, "nokia,nvm-size",
 				    &hwcfg->nvm_size);
@@ -3119,8 +3122,7 @@  static int smiapp_remove(struct i2c_client *client)
 	v4l2_async_unregister_subdev(subdev);
 
 	if (sensor->power_count) {
-		if (gpio_is_valid(sensor->hwcfg->xshutdown))
-			gpio_set_value(sensor->hwcfg->xshutdown, 0);
+		gpiod_set_value(sensor->xshutdown, 0);
 		if (sensor->hwcfg->set_xclk)
 			sensor->hwcfg->set_xclk(&sensor->src->sd, 0);
 		else
diff --git a/drivers/media/i2c/smiapp/smiapp.h b/drivers/media/i2c/smiapp/smiapp.h
index 6ff095a..c504bd8 100644
--- a/drivers/media/i2c/smiapp/smiapp.h
+++ b/drivers/media/i2c/smiapp/smiapp.h
@@ -200,6 +200,7 @@  struct smiapp_sensor {
 	struct smiapp_hwconfig *hwcfg;
 	struct regulator *vana;
 	struct clk *ext_clk;
+	struct gpio_desc *xshutdown;
 	u32 limits[SMIAPP_LIMIT_LAST];
 	u8 nbinning_subtypes;
 	struct smiapp_binning_subtype binning_subtypes[SMIAPP_BINNING_SUBTYPES];