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+rene...@jmondi.org>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: Hans Verkuil <hans.verk...@cisco.com>
> Cc: Sakari Ailus <sakari.ai...@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>
> Signed-off-by: Akinobu Mita <akinobu.m...@gmail.com>
> ---
> 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.
>
> 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?
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
>
signature.asc
Description: PGP signature
