diff mbox series

media: s5k4ecgx: Switch to GPIO descriptors

Message ID 20220531084546.67737-1-linus.walleij@linaro.org
State Superseded
Headers show
Series media: s5k4ecgx: Switch to GPIO descriptors | expand

Commit Message

Linus Walleij May 31, 2022, 8:45 a.m. UTC
The driver has an option to pass in GPIO numbers from platform
data but this is not used in the kernel so delete this and the
whole platform data mechanism.

Get GPIO descriptors using the standard API and simplify the code,
gpiolib will handle any inversions.

Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/media/i2c/s5k4ecgx.c | 132 ++++++++---------------------------
 include/media/i2c/s5k4ecgx.h |  33 ---------
 2 files changed, 31 insertions(+), 134 deletions(-)
 delete mode 100644 include/media/i2c/s5k4ecgx.h

Comments

kernel test robot May 31, 2022, 11:53 p.m. UTC | #1
Hi Linus,

I love your patch! Perhaps something to improve:

[auto build test WARNING on media-tree/master]
[also build test WARNING on v5.18 next-20220531]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Linus-Walleij/media-s5k4ecgx-Switch-to-GPIO-descriptors/20220531-164948
base:   git://linuxtv.org/media_tree.git master
config: i386-randconfig-a001 (https://download.01.org/0day-ci/archive/20220601/202206010746.udUX5CYK-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/d6a5013d9dd546a9c9d7ed3714e861f7593b1635
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Linus-Walleij/media-s5k4ecgx-Switch-to-GPIO-descriptors/20220531-164948
        git checkout d6a5013d9dd546a9c9d7ed3714e861f7593b1635
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/media/i2c/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/media/i2c/s5k4ecgx.c: In function 's5k4ecgx_probe':
>> drivers/media/i2c/s5k4ecgx.c:897:45: warning: missing terminating " character
     897 |         gpiod_set_consumer_name(priv->stby, "S5K4ECGX_STBY);
         |                                             ^
   drivers/media/i2c/s5k4ecgx.c:897:45: error: missing terminating " character
     897 |         gpiod_set_consumer_name(priv->stby, "S5K4ECGX_STBY);
         |                                             ^~~~~~~~~~~~~~~~
   drivers/media/i2c/s5k4ecgx.c:898:76: error: expected ')' before ';' token
     898 |         priv->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
         |                                                                            ^
         |                                                                            )
   drivers/media/i2c/s5k4ecgx.c:897:32: note: to match this '('
     897 |         gpiod_set_consumer_name(priv->stby, "S5K4ECGX_STBY);
         |                                ^
   drivers/media/i2c/s5k4ecgx.c:898:21: error: passing argument 2 of 'gpiod_set_consumer_name' from incompatible pointer type [-Werror=incompatible-pointer-types]
     898 |         priv->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
         |         ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                     |
         |                     struct gpio_desc *
   In file included from drivers/media/i2c/s5k4ecgx.c:18:
   include/linux/gpio/consumer.h:166:65: note: expected 'const char *' but argument is of type 'struct gpio_desc *'
     166 | int gpiod_set_consumer_name(struct gpio_desc *desc, const char *name);
         |                                                     ~~~~~~~~~~~~^~~~
   drivers/media/i2c/s5k4ecgx.c:927:20: error: expected ';' before '}' token
     927 |         return ret;
         |                    ^
         |                    ;
     928 | }
         | ~                   
   drivers/media/i2c/s5k4ecgx.c:895:17: error: label 'out_err' used but not defined
     895 |                 goto out_err;
         |                 ^~~~
>> drivers/media/i2c/s5k4ecgx.c:860:18: warning: unused variable 'i' [-Wunused-variable]
     860 |         int ret, i;
         |                  ^
   drivers/media/i2c/s5k4ecgx.c:928:1: error: control reaches end of non-void function [-Werror=return-type]
     928 | }
         | ^
   At top level:
   drivers/media/i2c/s5k4ecgx.c:827:12: warning: 's5k4ecgx_init_v4l2_ctrls' defined but not used [-Wunused-function]
     827 | static int s5k4ecgx_init_v4l2_ctrls(struct s5k4ecgx *priv)
         |            ^~~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +897 drivers/media/i2c/s5k4ecgx.c

   853	
   854	static int s5k4ecgx_probe(struct i2c_client *client,
   855				  const struct i2c_device_id *id)
   856	{
   857		struct s5k4ecgx_platform_data *pdata = client->dev.platform_data;
   858		struct v4l2_subdev *sd;
   859		struct s5k4ecgx *priv;
 > 860		int ret, i;
   861	
   862		if (pdata == NULL) {
   863			dev_err(&client->dev, "platform data is missing!\n");
   864			return -EINVAL;
   865		}
   866	
   867		priv = devm_kzalloc(&client->dev, sizeof(struct s5k4ecgx), GFP_KERNEL);
   868		if (!priv)
   869			return -ENOMEM;
   870	
   871		mutex_init(&priv->lock);
   872		priv->streaming = 0;
   873	
   874		sd = &priv->sd;
   875		/* Registering subdev */
   876		v4l2_i2c_subdev_init(sd, client, &s5k4ecgx_ops);
   877		/* Static name; NEVER use in new drivers! */
   878		strscpy(sd->name, S5K4ECGX_DRIVER_NAME, sizeof(sd->name));
   879	
   880		sd->internal_ops = &s5k4ecgx_subdev_internal_ops;
   881		/* Support v4l2 sub-device user space API */
   882		sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
   883	
   884		priv->pad.flags = MEDIA_PAD_FL_SOURCE;
   885		sd->entity.function = MEDIA_ENT_F_CAM_SENSOR;
   886		ret = media_entity_pads_init(&sd->entity, 1, &priv->pad);
   887		if (ret)
   888			return ret;
   889	
   890		/* Request GPIO lines asserted */
   891		priv->stby = devm_gpiod_get(&client->dev, "standby", GPIOD_OUT_HIGH);
   892		if (IS_ERR(priv->stby)) {
   893			v4l2_err(sd, "failed to request gpio S5K4ECGX_STBY");
   894			ret = PTR_ERR(priv->stby);
   895			goto out_err;
   896		}
 > 897		gpiod_set_consumer_name(priv->stby, "S5K4ECGX_STBY);
   898		priv->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
   899		if (IS_ERR(priv->reset)) {
   900			v4l2_err(sd, "failed to request gpio S5K4ECGX_RST");
   901			ret = PTR_ERR(priv->reset);
   902			goto out_err;
   903		}
   904		gpiod_set_consumer_name(priv->reset, "S5K4ECGX_RST");
   905	
   906		for (i = 0; i < S5K4ECGX_NUM_SUPPLIES; i++)
   907			priv->supplies[i].supply = s5k4ecgx_supply_names[i];
   908	
   909		ret = devm_regulator_bulk_get(&client->dev, S5K4ECGX_NUM_SUPPLIES,
   910					 priv->supplies);
   911		if (ret) {
   912			dev_err(&client->dev, "Failed to get regulators\n");
   913			goto out_err;
   914		}
   915		ret = s5k4ecgx_init_v4l2_ctrls(priv);
   916		if (ret)
   917			goto out_err;
   918	
   919		priv->curr_pixfmt = &s5k4ecgx_formats[0];
   920		priv->curr_frmsize = &s5k4ecgx_prev_sizes[0];
   921	
   922		return 0;
   923	
   924	out_err:
   925		media_entity_cleanup(&priv->sd.entity);
   926	
   927		return ret;
   928	}
   929
diff mbox series

Patch

diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c
index af9a305242cd..73162a48799f 100644
--- a/drivers/media/i2c/s5k4ecgx.c
+++ b/drivers/media/i2c/s5k4ecgx.c
@@ -15,7 +15,7 @@ 
 #include <linux/ctype.h>
 #include <linux/delay.h>
 #include <linux/firmware.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
 #include <linux/regulator/consumer.h>
@@ -23,7 +23,6 @@ 
 #include <asm/unaligned.h>
 
 #include <media/media-entity.h>
-#include <media/i2c/s5k4ecgx.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-mediabus.h>
@@ -171,12 +170,6 @@  static const char * const s5k4ecgx_supply_names[] = {
 
 #define S5K4ECGX_NUM_SUPPLIES ARRAY_SIZE(s5k4ecgx_supply_names)
 
-enum s5k4ecgx_gpio_id {
-	STBY,
-	RSET,
-	GPIO_NUM,
-};
-
 struct s5k4ecgx {
 	struct v4l2_subdev sd;
 	struct media_pad pad;
@@ -190,7 +183,8 @@  struct s5k4ecgx {
 	u8 set_params;
 
 	struct regulator_bulk_data supplies[S5K4ECGX_NUM_SUPPLIES];
-	struct s5k4ecgx_gpio gpio[GPIO_NUM];
+	struct gpio_desc *stby;
+	struct gpio_desc *reset;
 };
 
 static inline struct s5k4ecgx *to_s5k4ecgx(struct v4l2_subdev *sd)
@@ -454,15 +448,6 @@  static int s5k4ecgx_init_sensor(struct v4l2_subdev *sd)
 	return ret;
 }
 
-static int s5k4ecgx_gpio_set_value(struct s5k4ecgx *priv, int id, u32 val)
-{
-	if (!gpio_is_valid(priv->gpio[id].gpio))
-		return 0;
-	gpio_set_value(priv->gpio[id].gpio, val);
-
-	return 1;
-}
-
 static int __s5k4ecgx_power_on(struct s5k4ecgx *priv)
 {
 	int ret;
@@ -472,23 +457,22 @@  static int __s5k4ecgx_power_on(struct s5k4ecgx *priv)
 		return ret;
 	usleep_range(30, 50);
 
-	/* The polarity of STBY is controlled by TSP */
-	if (s5k4ecgx_gpio_set_value(priv, STBY, priv->gpio[STBY].level))
-		usleep_range(30, 50);
-
-	if (s5k4ecgx_gpio_set_value(priv, RSET, priv->gpio[RSET].level))
-		usleep_range(30, 50);
+	/* De-assert standby and reset */
+	gpiod_set_value(priv->stby, 0);
+	usleep_range(30, 50);
+	gpiod_set_value(priv->reset, 0);
+	usleep_range(30, 50);
 
 	return 0;
 }
 
 static int __s5k4ecgx_power_off(struct s5k4ecgx *priv)
 {
-	if (s5k4ecgx_gpio_set_value(priv, RSET, !priv->gpio[RSET].level))
-		usleep_range(30, 50);
-
-	if (s5k4ecgx_gpio_set_value(priv, STBY, !priv->gpio[STBY].level))
-		usleep_range(30, 50);
+	/* Assert reset and standby */
+	gpiod_set_value(priv->reset, 1);
+	usleep_range(30, 50);
+	gpiod_set_value(priv->stby, 1);
+	usleep_range(30, 50);
 
 	priv->streaming = 0;
 
@@ -840,68 +824,6 @@  static const struct v4l2_subdev_ops s5k4ecgx_ops = {
 	.video = &s5k4ecgx_video_ops,
 };
 
-/*
- * GPIO setup
- */
-static int s5k4ecgx_config_gpio(int nr, int val, const char *name)
-{
-	unsigned long flags = val ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW;
-	int ret;
-
-	if (!gpio_is_valid(nr))
-		return 0;
-	ret = gpio_request_one(nr, flags, name);
-	if (!ret)
-		gpio_export(nr, 0);
-
-	return ret;
-}
-
-static void s5k4ecgx_free_gpios(struct s5k4ecgx *priv)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(priv->gpio); i++) {
-		if (!gpio_is_valid(priv->gpio[i].gpio))
-			continue;
-		gpio_free(priv->gpio[i].gpio);
-		priv->gpio[i].gpio = -EINVAL;
-	}
-}
-
-static int s5k4ecgx_config_gpios(struct s5k4ecgx *priv,
-				  const struct s5k4ecgx_platform_data *pdata)
-{
-	const struct s5k4ecgx_gpio *gpio = &pdata->gpio_stby;
-	int ret;
-
-	priv->gpio[STBY].gpio = -EINVAL;
-	priv->gpio[RSET].gpio  = -EINVAL;
-
-	ret = s5k4ecgx_config_gpio(gpio->gpio, gpio->level, "S5K4ECGX_STBY");
-
-	if (ret) {
-		s5k4ecgx_free_gpios(priv);
-		return ret;
-	}
-	priv->gpio[STBY] = *gpio;
-	if (gpio_is_valid(gpio->gpio))
-		gpio_set_value(gpio->gpio, 0);
-
-	gpio = &pdata->gpio_reset;
-
-	ret = s5k4ecgx_config_gpio(gpio->gpio, gpio->level, "S5K4ECGX_RST");
-	if (ret) {
-		s5k4ecgx_free_gpios(priv);
-		return ret;
-	}
-	priv->gpio[RSET] = *gpio;
-	if (gpio_is_valid(gpio->gpio))
-		gpio_set_value(gpio->gpio, 0);
-
-	return 0;
-}
-
 static int s5k4ecgx_init_v4l2_ctrls(struct s5k4ecgx *priv)
 {
 	const struct v4l2_ctrl_ops *ops = &s5k4ecgx_ctrl_ops;
@@ -965,11 +887,22 @@  static int s5k4ecgx_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
-	ret = s5k4ecgx_config_gpios(priv, pdata);
-	if (ret) {
-		dev_err(&client->dev, "Failed to set gpios\n");
-		goto out_err1;
+	/* Request GPIO lines asserted */
+	priv->stby = devm_gpiod_get(&client->dev, "standby", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->stby)) {
+		v4l2_err(sd, "failed to request gpio S5K4ECGX_STBY");
+		ret = PTR_ERR(priv->stby);
+		goto out_err;
+	}
+	gpiod_set_consumer_name(priv->stby, "S5K4ECGX_STBY);
+	priv->reset = devm_gpiod_get(&client->dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->reset)) {
+		v4l2_err(sd, "failed to request gpio S5K4ECGX_RST");
+		ret = PTR_ERR(priv->reset);
+		goto out_err;
 	}
+	gpiod_set_consumer_name(priv->reset, "S5K4ECGX_RST");
+
 	for (i = 0; i < S5K4ECGX_NUM_SUPPLIES; i++)
 		priv->supplies[i].supply = s5k4ecgx_supply_names[i];
 
@@ -977,20 +910,18 @@  static int s5k4ecgx_probe(struct i2c_client *client,
 				 priv->supplies);
 	if (ret) {
 		dev_err(&client->dev, "Failed to get regulators\n");
-		goto out_err2;
+		goto out_err;
 	}
 	ret = s5k4ecgx_init_v4l2_ctrls(priv);
 	if (ret)
-		goto out_err2;
+		goto out_err;
 
 	priv->curr_pixfmt = &s5k4ecgx_formats[0];
 	priv->curr_frmsize = &s5k4ecgx_prev_sizes[0];
 
 	return 0;
 
-out_err2:
-	s5k4ecgx_free_gpios(priv);
-out_err1:
+out_err:
 	media_entity_cleanup(&priv->sd.entity);
 
 	return ret;
@@ -1002,7 +933,6 @@  static int s5k4ecgx_remove(struct i2c_client *client)
 	struct s5k4ecgx *priv = to_s5k4ecgx(sd);
 
 	mutex_destroy(&priv->lock);
-	s5k4ecgx_free_gpios(priv);
 	v4l2_device_unregister_subdev(sd);
 	v4l2_ctrl_handler_free(&priv->handler);
 	media_entity_cleanup(&sd->entity);
diff --git a/include/media/i2c/s5k4ecgx.h b/include/media/i2c/s5k4ecgx.h
deleted file mode 100644
index 92202eb35249..000000000000
--- a/include/media/i2c/s5k4ecgx.h
+++ /dev/null
@@ -1,33 +0,0 @@ 
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * S5K4ECGX image sensor header file
- *
- * Copyright (C) 2012, Linaro
- * Copyright (C) 2012, Samsung Electronics Co., Ltd.
- */
-
-#ifndef S5K4ECGX_H
-#define S5K4ECGX_H
-
-/**
- * struct s5k4ecgx_gpio - data structure describing a GPIO
- * @gpio: GPIO number
- * @level: indicates active state of the @gpio
- */
-struct s5k4ecgx_gpio {
-	int gpio;
-	int level;
-};
-
-/**
- * struct s5k4ecgx_platform_data - s5k4ecgx driver platform data
- * @gpio_reset:	 GPIO driving RESET pin
- * @gpio_stby:	 GPIO driving STBY pin
- */
-
-struct s5k4ecgx_platform_data {
-	struct s5k4ecgx_gpio gpio_reset;
-	struct s5k4ecgx_gpio gpio_stby;
-};
-
-#endif /* S5K4ECGX_H */