2018-04-09 16:36 GMT+09:00 jacopo mondi <jacopo@xxxxxxxxxx>:
> Hi Akinobu,
>
> On Sun, Apr 08, 2018 at 12:48:06AM +0900, Akinobu Mita wrote:
>> This change adds checks for register read errors and returns correct
>> error code.
>>
>
> I feel like error conditions are anyway captured by the switch()
> default case, but I understand there may be merits in returning the
> actual error code.
>
>> Cc: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
>> Cc: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>> Cc: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
>> Signed-off-by: Akinobu Mita <akinobu.mita@xxxxxxxxx>
>> ---
>> drivers/media/i2c/ov772x.c | 11 +++++++++--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
>> index 283ae2c..c56f910 100644
>> --- a/drivers/media/i2c/ov772x.c
>> +++ b/drivers/media/i2c/ov772x.c
>> @@ -1169,8 +1169,15 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
>> return ret;
>>
>> /* Check and show product ID and manufacturer ID. */
>> - pid = ov772x_read(client, PID);
>> - ver = ov772x_read(client, VER);
>> + ret = ov772x_read(client, PID);
>> + if (ret < 0)
>> + return ret;
>> + pid = ret;
>> +
>> + ret = ov772x_read(client, VER);
>> + if (ret < 0)
>> + return ret;
>> + ver = ret;
>
> You can assign the ov772x_read() return value to pid and ver directly
> and save two assignments.
OK. This needs to change the data types of pid and ver from 'u8' to 'int'.
>>
>> switch (VERSION(pid, ver)) {
>> case OV7720:
>
> If we want to check for return values here, which is always a good
> thing, could you do the same for MIDH and MIDL below?
Sounds good.
> Nit: You can also fix the dev_info() parameters alignment to span to
> the whole line length while at there. Ie.
>
> dev_info(&client->dev,
> "%s Product ID %0x:%0x Manufacturer ID %x:%x\n",
> devname, pid, ver, ov772x_read(client, MIDH),
> ov772x_read(client, MIDL));
>
> Thanks
> j
>
>
>> --
>> 2.7.4
>>
