On 03/10/2014 04:24 PM, Lars-Peter Clausen wrote:
> 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?
When do you think you can have a fixed version of this patch ready? If it is
today/tomorrow, then I'll wait for that, otherwise I'll make a pull request
for the other 6 patches (which look fine).
In any case there is no need to resend the whole series.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
