On 03/10/2014 03:37 PM, Hans Verkuil wrote:
[...]
+
+static int adv7180_s_power(struct v4l2_subdev *sd, int on)
+{
+       struct adv7180_state *state = to_state(sd);
+       struct i2c_client *client = v4l2_get_subdevdata(sd);
+       int ret;
+
+       ret = mutex_lock_interruptible(&state->mutex);
+       if (ret)
+               return ret;
+
+       ret = adv7180_set_power(state, client, on);
+       if (ret)
+               goto out;
+
+       state->powered = on;
+out:

I would change this to:

        if (!ret)
                state->powered = on;

and drop the 'goto'.


ok.

[...]
  static int adv7180_resume(struct device *dev)
@@ -656,10 +687,11 @@ static int adv7180_resume(struct device *dev)
        struct adv7180_state *state = to_state(sd);
        int ret;

-       ret = i2c_smbus_write_byte_data(client, ADV7180_PWR_MAN_REG,
-                                       ADV7180_PWR_MAN_ON);
-       if (ret < 0)
-               return ret;
+       if (state->powered) {
+               ret = adv7180_set_power(state, client, true);
+               if (ret)
+                       return ret;
+       }
        ret = init_device(client, state);
        if (ret < 0)
                return ret;


What is the initial state of the driver when loaded? Shouldn't probe() set the
'powered' variable to true initially?

Yep, st->powered should be set to true by default.

What's your process in general, want me to resend the whole series, or are you going to apply the patches that are ok?

Thanks for the quick review,
- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to