lm3554: fix the usage of the gpio API
authorAndy Shevchenko <andriy.shevchenko@linux.intel.com>
Tue, 27 Mar 2012 14:06:01 +0000 (17:06 +0300)
committerbuildbot <buildbot@intel.com>
Tue, 10 Apr 2012 20:19:25 +0000 (13:19 -0700)
BZ: 30218

For each change of a gpio line the current driver calls a sequence:
gpio_request();
gpio_direction_output();
gpio_free();

The requesting and freeing of the gpio line is affecting the sysfs. Moreover
there is no guarantee that the gpio line will keep the state after the
gpio_free() call.

In our case gpio_direction_output() might sleep. Due to this we end up with
following traceback in the timer function.

[   51.552511] BUG: sleeping function called from invalid context at drivers/gpio/gpio
[   51.552536] in_atomic(): 1, irqs_disabled(): 0, pid: 0, name: kworker/0:0
[   51.552558] Pid: 0, comm: kworker/0:0 Not tainted 3.0.22-mid26-00191-g8147537 #90
[   51.552569] Call Trace:
[   51.552618]  [<c10357c3>] ? vprintk+0x365/0x3ca
[   51.552641]  [<c1028e00>] __might_sleep+0xf9/0x100
[   51.552668]  [<c121d0d1>] gpio_free+0x23/0xd3
[   51.552702]  [<f867d4f2>] set_gpio_output+0x2c/0x33 [lm3554]
[   51.552717]  [<f867d58c>] lm3554_flash_off_delay+0x28/0x3e [lm3554]
[   51.552744]  [<c1040723>] run_timer_softirq+0x1ba/0x2b0
...

The patch changes the logic of the usage of the gpio API.
Probe:
gpio_request();
gpio_direction_output();
Remove:
gpio_direction_output();
gpio_free();
In driver usage:
gpio_set_value();

Change-Id: I616bdec6af68c73485c88a078b44a3f07249078b
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-on: http://android.intel.com:8080/42148
Reviewed-by: Shevchenko, Andriy <andriy.shevchenko@intel.com>
Reviewed-by: Toivonen, Tuukka <tuukka.toivonen@intel.com>
Reviewed-by: Gross, Mark <mark.gross@intel.com>
Reviewed-by: Cohen, David A <david.a.cohen@intel.com>
Reviewed-by: Koskinen, Ilkka <ilkka.koskinen@intel.com>
Tested-by: Lampila, KalleX <kallex.lampila@intel.com>
Reviewed-by: buildbot <buildbot@intel.com>
Tested-by: buildbot <buildbot@intel.com>
drivers/media/video/lm3554.c

index 56c354b..56d1a5a 100644 (file)
@@ -127,16 +127,6 @@ static void get_reg_field(struct v4l2_subdev *sd,
        *value = (tmp & mask) >> field->lsb;
 }
 
-static int set_gpio_output(int gpio, const char *name, int val)
-{
-       int ret = gpio_request(gpio, name);
-       if (ret < 0)
-               return ret;
-       ret = gpio_direction_output(gpio, val);
-       gpio_free(gpio);
-       return ret;
-}
-
 static int lm3554_hw_reset(struct i2c_client *client)
 {
        struct v4l2_subdev *sd = i2c_get_clientdata(client);
@@ -147,10 +137,13 @@ static int lm3554_hw_reset(struct i2c_client *client)
        ret = gpio_request(pdata->gpio_reset, "flash reset");
        if (ret < 0)
                return ret;
+
        gpio_set_value(pdata->gpio_reset, 0);
        msleep(50);
+
        gpio_set_value(pdata->gpio_reset, 1);
        msleep(50);
+
        gpio_free(pdata->gpio_reset);
        return ret;
 }
@@ -266,11 +259,8 @@ static int lm3554_s_flash_strobe(struct v4l2_subdev *sd, u32 val)
         * If timer is killed before run, flash is not strobe off,
         * so must strobe off here
         */
-       if (timer_pending != 0) {
-               ret = set_gpio_output(pdata->gpio_strobe, "flash", 0);
-               if (ret < 0)
-                       goto err;
-       }
+       if (timer_pending != 0)
+               gpio_set_value(pdata->gpio_strobe, 0);
 
        /* Restore flash current settings */
        ret = set_reg_field(sd, &flash_current,
@@ -279,9 +269,7 @@ static int lm3554_s_flash_strobe(struct v4l2_subdev *sd, u32 val)
                goto err;
 
        /* Strobe on Flash */
-       ret = set_gpio_output(pdata->gpio_strobe, "flash", val);
-       if (ret < 0)
-               goto err;
+       gpio_set_value(pdata->gpio_strobe, val);
 
        return 0;
 err:
@@ -540,14 +528,6 @@ static int lm3554_detect(struct i2c_client *client)
 
        lm3554_hw_reset(client);
 
-       ret = set_gpio_output(pdata->gpio_strobe, "flash", 0);
-       if (ret < 0)
-               goto fail;
-
-       ret = set_gpio_output(pdata->gpio_torch, "torch", 0);
-       if (ret < 0)
-               goto fail;
-
        /* Set to TX2 mode, then ENVM/TX2 pin is a power
         * amplifier sync input:
         * ENVM/TX pin asserted, flash forced into torch;
@@ -575,19 +555,70 @@ static int lm3554_detect(struct i2c_client *client)
        return 0;
 
 fail:
-       dev_err(&client->dev, "gpio request/direction_output fail");
        return ret;
 }
 
 static void lm3554_flash_off_delay(long unsigned int arg)
 {
-       struct i2c_client *client = (struct i2c_client *)arg;
+       struct v4l2_subdev *sd = i2c_get_clientdata((struct i2c_client *)arg);
+       struct lm3554_priv *p_lm3554_priv = to_lm3554_priv(sd);
+       struct camera_flash_platform_data *pdata = p_lm3554_priv->platform_data;
+
+       gpio_set_value(pdata->gpio_strobe, 0);
+}
+
+static int __devinit lm3554_gpio_init(struct i2c_client *client)
+{
        struct v4l2_subdev *sd = i2c_get_clientdata(client);
        struct lm3554_priv *p_lm3554_priv = to_lm3554_priv(sd);
        struct camera_flash_platform_data *pdata = p_lm3554_priv->platform_data;
+       int ret;
+
+       ret = gpio_request(pdata->gpio_strobe, "flash");
+       if (ret < 0)
+               return ret;
 
-       if (set_gpio_output(pdata->gpio_strobe, "flash", 0) < 0)
-               dev_err(&client->dev, "failed to flash strobe off\n");
+       ret = gpio_direction_output(pdata->gpio_strobe, 0);
+       if (ret < 0)
+               goto err_gpio_flash;
+
+       ret = gpio_request(pdata->gpio_torch, "torch");
+       if (ret < 0)
+               goto err_gpio_flash;
+
+       ret = gpio_direction_output(pdata->gpio_torch, 0);
+       if (ret < 0)
+               goto err_gpio_torch;
+
+       return 0;
+
+err_gpio_torch:
+       gpio_free(pdata->gpio_torch);
+err_gpio_flash:
+       gpio_free(pdata->gpio_strobe);
+       return ret;
+}
+
+static int lm3554_gpio_uninit(struct i2c_client *client)
+{
+       struct v4l2_subdev *sd = i2c_get_clientdata(client);
+       struct lm3554_priv *p_lm3554_priv = to_lm3554_priv(sd);
+       struct camera_flash_platform_data *pdata = p_lm3554_priv->platform_data;
+       int ret;
+
+       ret = gpio_direction_output(pdata->gpio_torch, 0);
+       if (ret < 0)
+               return ret;
+
+       ret = gpio_direction_output(pdata->gpio_strobe, 0);
+       if (ret < 0)
+               return ret;
+
+       gpio_free(pdata->gpio_torch);
+
+       gpio_free(pdata->gpio_strobe);
+
+       return 0;
 }
 
 static int __devinit lm3554_probe(struct i2c_client *client,
@@ -633,6 +664,12 @@ static int __devinit lm3554_probe(struct i2c_client *client,
                goto fail2;
        }
 
+       err = lm3554_gpio_init(client);
+       if (err) {
+               dev_err(&client->dev, "gpio request/direction_output fail");
+               goto fail2;
+       }
+
        return 0;
 fail2:
        media_entity_cleanup(&p_lm3554_priv->sd.entity);
@@ -655,7 +692,7 @@ static int lm3554_remove(struct i2c_client *client)
 
        del_timer_sync(&p_lm3554_priv->flash_off_delay);
 
-       ret = set_gpio_output(pdata->gpio_strobe, "flash", 0);
+       ret = lm3554_gpio_uninit(client);
        if (ret < 0)
                goto fail;