Hi Hans,

On Tue, Apr 21, 2015 at 5:21 PM, Hans de Goede <hdego...@redhat.com> wrote:
>> @@ -128,7 +129,7 @@ static int sn9c2028_long_command(struct gspca_dev
>> *gspca_dev, u8 *command)
>>         status = -1;
>>         for (i = 0; i < 256 && status < 2; i++)
>>                 status = sn9c2028_read1(gspca_dev);
>> -       if (status != 2) {
>> +       if (status < 0) {
>>                 pr_err("long command status read error %d\n", status);
>>                 return (status < 0) ? status : -EIO;
>>         }
>
>
> Do you really need this change ? sn9c2028_read1 returns either a negative
> error code, or the byte read from the sn9c2028 chip. This functions wait for
> the sn9c2028 to return a read value of 2. I admit that the check in the for
> vs the check in the error reporting is not chosen well, both should probably
> be != 2. But checking for status < 0 is not good as this does not catch
> a successful read from the chip not returning 2.

For this cam it returns 1 on some commands. Anyway, this value is not
used anywhere later, so I just extended condition.

Regards,
Vasily
--
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