[PATCH v2 00/11] media: ov772x/tw9910 cleanup

[PATCH v2 00/11] media: ov772x/tw9910 cleanup

[Jacopo Mondi]

Hi Mauro,
   as I had one more patch to add to the series, I have now re-based it on top
of Joe's changes, which were based on top of yours already part of media-tree
master branch.

Please apply on top of:

commit bc3c49d6bbfb ("media: tw9910: Miscellaneous neatening")
commit 71c07c61b340 ("media: tw9910: Whitespace alignment")
commit ae24b8a1d5f9 ("media: tw9910: solve coding style issues")
commit 2d595d14fe8b ("media: ov772x: fix whitespace issues")

Thanks
  j

v1 -> v2:
- Rebased on top of Joe's cleanup patches: 2 patches squashed
- Add patch 12/12


Jacopo Mondi (11):
  media: tw9910: Re-order variables declaration
  media: tw9910: Re-organize in-code comments
  media: tw9910: Mixed style fixes
  media: tw9910: Sort includes alphabetically
  media: tw9910: Replace msleep(1) with usleep_range
  media: ov772x: Align function parameters
  media: ov772x: Re-organize in-code comments
  media: ov772x: Empty line before end-of-function return
  media: ov772x: Re-order variables declaration
  media: ov772x: Replace msleep(1) with usleep_range
  media: ov772x: Unregister async subdevice

 drivers/media/i2c/ov772x.c | 65 ++++++++++++++++--------------------
 drivers/media/i2c/tw9910.c | 83 ++++++++++++++++++----------------------------
 2 files changed, 61 insertions(+), 87 deletions(-)

--
2.7.4

[PATCH v2 01/11] media: tw9910: Re-order variables declaration

[Jacopo Mondi]

Re-order variables declaration to respect 'reverse christmas tree'
ordering whenever possible.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/tw9910.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index cc648de..3a5e307 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
 
 static int tw9910_power(struct i2c_client *client, int enable)
 {
-	int ret;
 	u8 acntl1;
 	u8 acntl2;
+	int ret;
 
 	if (enable) {
 		acntl1 = 0;
@@ -428,8 +428,8 @@ static int tw9910_power(struct i2c_client *client, int enable)
 static const struct tw9910_scale_ctrl *tw9910_select_norm(v4l2_std_id norm,
 							  u32 width, u32 height)
 {
-	const struct tw9910_scale_ctrl *scale;
 	const struct tw9910_scale_ctrl *ret = NULL;
+	const struct tw9910_scale_ctrl *scale;
 	__u32 diff = 0xffffffff, tmp;
 	int size, i;
 
@@ -462,8 +462,8 @@ static int tw9910_s_stream(struct v4l2_subdev *sd, int enable)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct tw9910_priv *priv = to_tw9910(client);
-	u8 val;
 	int ret;
+	u8 val;
 
 	if (!enable) {
 		switch (priv->revision) {
@@ -512,10 +512,10 @@ static int tw9910_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	struct tw9910_priv *priv = to_tw9910(client);
-	const unsigned int hact = 720;
 	const unsigned int hdelay = 15;
-	unsigned int vact;
+	const unsigned int hact = 720;
 	unsigned int vdelay;
+	unsigned int vact;
 	int ret;
 
 	if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
@@ -760,8 +760,8 @@ static int tw9910_get_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_pad_config *cfg,
 			  struct v4l2_subdev_format *format)
 {
-	struct v4l2_mbus_framefmt *mf = &format->format;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct v4l2_mbus_framefmt *mf = &format->format;
 	struct tw9910_priv *priv = to_tw9910(client);
 
 	if (format->pad)
@@ -813,8 +813,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_pad_config *cfg,
 			  struct v4l2_subdev_format *format)
 {
-	struct v4l2_mbus_framefmt *mf = &format->format;
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
+	struct v4l2_mbus_framefmt *mf = &format->format;
 	struct tw9910_priv *priv = to_tw9910(client);
 	const struct tw9910_scale_ctrl *scale;
 
@@ -852,8 +852,8 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
 static int tw9910_video_probe(struct i2c_client *client)
 {
 	struct tw9910_priv *priv = to_tw9910(client);
-	s32 id;
 	int ret;
+	s32 id;
 
 	/*
 	 * tw9910 only use 8 or 16 bit bus width
@@ -949,10 +949,9 @@ static int tw9910_probe(struct i2c_client *client,
 			const struct i2c_device_id *did)
 
 {
-	struct tw9910_priv		*priv;
-	struct tw9910_video_info	*info;
-	struct i2c_adapter		*adapter =
-		to_i2c_adapter(client->dev.parent);
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct tw9910_video_info *info;
+	struct tw9910_priv *priv;
 	int ret;
 
 	if (!client->dev.platform_data) {
-- 
2.7.4

Re: [PATCH v2 01/11] media: tw9910: Re-order variables declaration

[Mauro Carvalho Chehab]

Em Fri,  2 Mar 2018 15:46:33 +0100
Jacopo Mondi <jacopo+renesas@jmondi.org> escreveu:

> Re-order variables declaration to respect 'reverse christmas tree'
> ordering whenever possible.

To be frank, I don't like the idea of reverse christmas tree ordering
myself... Perhaps due to the time I used to program on assembler, 
where alignment issues could happen, I find a way more logic to order
based on complexity and size of the argument...

> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  drivers/media/i2c/tw9910.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> index cc648de..3a5e307 100644
> --- a/drivers/media/i2c/tw9910.c
> +++ b/drivers/media/i2c/tw9910.c
> @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
>  
>  static int tw9910_power(struct i2c_client *client, int enable)
>  {
> -	int ret;
>  	u8 acntl1;
>  	u8 acntl2;
> +	int ret;

... So, in this case, the order is already the right one, according
with my own criteria :-)

There was some discussion about the order sometime ago at LKML:

	https://patchwork.kernel.org/patch/9411999/

As I'm not seeing the proposed patch there at checkpatch, nor any
comments about xmas tree at coding style, I think that there were no
agreements about the ordering.

So, while there's no consensus about that, let's keep it as-is.

Regards,
Mauro

Re: [PATCH v2 01/11] media: tw9910: Re-order variables declaration

[jacopo mondi]

Hi Mauro,

On Tue, Mar 06, 2018 at 01:51:52PM -0300, Mauro Carvalho Chehab wrote:
> Em Fri,  2 Mar 2018 15:46:33 +0100
> Jacopo Mondi <jacopo+renesas@jmondi.org> escreveu:
>
> > Re-order variables declaration to respect 'reverse christmas tree'
> > ordering whenever possible.
>
> To be frank, I don't like the idea of reverse christmas tree ordering
> myself... Perhaps due to the time I used to program on assembler,
> where alignment issues could happen, I find a way more logic to order
> based on complexity and size of the argument...
>
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > ---
> >  drivers/media/i2c/tw9910.c | 23 +++++++++++------------
> >  1 file changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> > index cc648de..3a5e307 100644
> > --- a/drivers/media/i2c/tw9910.c
> > +++ b/drivers/media/i2c/tw9910.c
> > @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
> >
> >  static int tw9910_power(struct i2c_client *client, int enable)
> >  {
> > -	int ret;
> >  	u8 acntl1;
> >  	u8 acntl2;
> > +	int ret;
>
> ... So, in this case, the order is already the right one, according
> with my own criteria :-)
>
> There was some discussion about the order sometime ago at LKML:
>
> 	https://patchwork.kernel.org/patch/9411999/
>
> As I'm not seeing the proposed patch there at checkpatch, nor any
> comments about xmas tree at coding style, I think that there were no
> agreements about the ordering.
>
> So, while there's no consensus about that, let's keep it as-is.

Thanks for explaining. I was sure it was part of the coding style
rules! My bad, feel free to ditch this patch (same for ov772x ofc).

Thanks
   j

>
> Regards,
> Mauro

Re: [PATCH v2 01/11] media: tw9910: Re-order variables declaration

[Mauro Carvalho Chehab]

Em Tue, 6 Mar 2018 17:57:15 +0100
jacopo mondi <jacopo@jmondi.org> escreveu:

> Hi Mauro,
> 
> On Tue, Mar 06, 2018 at 01:51:52PM -0300, Mauro Carvalho Chehab wrote:
> > Em Fri,  2 Mar 2018 15:46:33 +0100
> > Jacopo Mondi <jacopo+renesas@jmondi.org> escreveu:
> >  
> > > Re-order variables declaration to respect 'reverse christmas tree'
> > > ordering whenever possible.  
> >
> > To be frank, I don't like the idea of reverse christmas tree ordering
> > myself... Perhaps due to the time I used to program on assembler,
> > where alignment issues could happen, I find a way more logic to order
> > based on complexity and size of the argument...
> >  
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> > > ---
> > >  drivers/media/i2c/tw9910.c | 23 +++++++++++------------
> > >  1 file changed, 11 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
> > > index cc648de..3a5e307 100644
> > > --- a/drivers/media/i2c/tw9910.c
> > > +++ b/drivers/media/i2c/tw9910.c
> > > @@ -406,9 +406,9 @@ static void tw9910_reset(struct i2c_client *client)
> > >
> > >  static int tw9910_power(struct i2c_client *client, int enable)
> > >  {
> > > -	int ret;
> > >  	u8 acntl1;
> > >  	u8 acntl2;
> > > +	int ret;  
> >
> > ... So, in this case, the order is already the right one, according
> > with my own criteria :-)
> >
> > There was some discussion about the order sometime ago at LKML:
> >
> > 	https://patchwork.kernel.org/patch/9411999/
> >
> > As I'm not seeing the proposed patch there at checkpatch, nor any
> > comments about xmas tree at coding style, I think that there were no
> > agreements about the ordering.
> >
> > So, while there's no consensus about that, let's keep it as-is.  
> 
> Thanks for explaining. I was sure it was part of the coding style
> rules! My bad, feel free to ditch this patch (same for ov772x ofc).

Heh, there are so many rules that it is hard to get all of them.

Also, some maintainers might actually be expecting some ordering.

I ditched this patch (and the one for ov772x) and applied the
remaining ones.

Thanks,
Mauro

[PATCH v2 02/11] media: tw9910: Re-organize in-code comments

[Jacopo Mondi]

A lot of comments that would fit a single line were spread on two or
more lines. Also fix capitalization and punctuation where appropriate.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/tw9910.c | 44 +++++++++++++-------------------------------
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 3a5e307..1c3c8f0 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -388,7 +388,7 @@ static int tw9910_set_hsync(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	/* So far only revisions 0 and 1 have been seen */
+	/* So far only revisions 0 and 1 have been seen. */
 	/* bit 2 - 0 */
 	if (priv->revision == 1)
 		ret = tw9910_mask_set(client, HSLOWCTL, 0x77,
@@ -652,21 +652,15 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 *width, u32 *height)
 	int ret = -EINVAL;
 	u8 val;
 
-	/*
-	 * select suitable norm
-	 */
+	/* Select suitable norm. */
 	priv->scale = tw9910_select_norm(priv->norm, *width, *height);
 	if (!priv->scale)
 		goto tw9910_set_fmt_error;
 
-	/*
-	 * reset hardware
-	 */
+	/* Reset hardware. */
 	tw9910_reset(client);
 
-	/*
-	 * set bus width
-	 */
+	/* Set bus width. */
 	val = 0x00;
 	if (priv->info->buswidth == 16)
 		val = LEN;
@@ -675,9 +669,7 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 *width, u32 *height)
 	if (ret < 0)
 		goto tw9910_set_fmt_error;
 
-	/*
-	 * select MPOUT behavior
-	 */
+	/* Select MPOUT behavior. */
 	switch (priv->info->mpout) {
 	case TW9910_MPO_VLOSS:
 		val = RTSEL_VLOSS; break;
@@ -703,16 +695,12 @@ static int tw9910_set_frame(struct v4l2_subdev *sd, u32 *width, u32 *height)
 	if (ret < 0)
 		goto tw9910_set_fmt_error;
 
-	/*
-	 * set scale
-	 */
+	/* Set scale. */
 	ret = tw9910_set_scale(client, priv->scale);
 	if (ret < 0)
 		goto tw9910_set_fmt_error;
 
-	/*
-	 * set hsync
-	 */
+	/* Set hsync. */
 	ret = tw9910_set_hsync(client);
 	if (ret < 0)
 		goto tw9910_set_fmt_error;
@@ -739,7 +727,7 @@ static int tw9910_get_selection(struct v4l2_subdev *sd,
 
 	if (sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
 		return -EINVAL;
-	/* Only CROP, CROP_DEFAULT and CROP_BOUNDS are supported */
+	/* Only CROP, CROP_DEFAULT and CROP_BOUNDS are supported. */
 	if (sel->target > V4L2_SEL_TGT_CROP_BOUNDS)
 		return -EINVAL;
 
@@ -791,9 +779,7 @@ static int tw9910_s_fmt(struct v4l2_subdev *sd,
 	WARN_ON(mf->field != V4L2_FIELD_ANY &&
 		mf->field != V4L2_FIELD_INTERLACED_BT);
 
-	/*
-	 * check color format
-	 */
+	/* Check color format. */
 	if (mf->code != MEDIA_BUS_FMT_UYVY8_2X8)
 		return -EINVAL;
 
@@ -831,9 +817,7 @@ static int tw9910_set_fmt(struct v4l2_subdev *sd,
 	mf->code = MEDIA_BUS_FMT_UYVY8_2X8;
 	mf->colorspace = V4L2_COLORSPACE_SMPTE170M;
 
-	/*
-	 * select suitable norm
-	 */
+	/* Select suitable norm. */
 	scale = tw9910_select_norm(priv->norm, mf->width, mf->height);
 	if (!scale)
 		return -EINVAL;
@@ -855,9 +839,7 @@ static int tw9910_video_probe(struct i2c_client *client)
 	int ret;
 	s32 id;
 
-	/*
-	 * tw9910 only use 8 or 16 bit bus width
-	 */
+	/* TW9910 only use 8 or 16 bit bus width. */
 	if (priv->info->buswidth != 16 && priv->info->buswidth != 8) {
 		dev_err(&client->dev, "bus width error\n");
 		return -ENODEV;
@@ -868,8 +850,8 @@ static int tw9910_video_probe(struct i2c_client *client)
 		return ret;
 
 	/*
-	 * check and show Product ID
-	 * So far only revisions 0 and 1 have been seen
+	 * Check and show Product ID.
+	 * So far only revisions 0 and 1 have been seen.
 	 */
 	id = i2c_smbus_read_byte_data(client, ID);
 	priv->revision = GET_REV(id);
-- 
2.7.4

[PATCH v2 03/11] media: tw9910: Mixed style fixes

[Jacopo Mondi]

Two minor style fixes, align function parameter and remove un-necessary
spaces.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/tw9910.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 1c3c8f0..0232017 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -533,9 +533,9 @@ static int tw9910_s_std(struct v4l2_subdev *sd, v4l2_std_id norm)
 	}
 	if (!ret)
 		ret = i2c_smbus_write_byte_data(client, CROP_HI,
-						((vdelay >> 2) & 0xc0) |
-						((vact >> 4) & 0x30) |
-						((hdelay >> 6) & 0x0c) |
+						((vdelay >> 2) & 0xc0)	|
+						((vact >> 4) & 0x30)	|
+						((hdelay >> 6) & 0x0c)	|
 						((hact >> 8) & 0x03));
 	if (!ret)
 		ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
@@ -953,7 +953,7 @@ static int tw9910_probe(struct i2c_client *client,
 	if (!priv)
 		return -ENOMEM;
 
-	priv->info   = info;
+	priv->info = info;
 
 	v4l2_i2c_subdev_init(&priv->subdev, client, &tw9910_subdev_ops);
 
-- 
2.7.4

[PATCH v2 04/11] media: tw9910: Sort includes alphabetically

[Jacopo Mondi]

Sort include directives alphabetically to ease maintenance.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/tw9910.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 0232017..9c12bda 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -16,13 +16,13 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
 #include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/i2c.h>
 #include <linux/slab.h>
-#include <linux/kernel.h>
-#include <linux/delay.h>
 #include <linux/v4l2-mediabus.h>
 #include <linux/videodev2.h>
 
-- 
2.7.4

[PATCH v2 05/11] media: tw9910: Replace msleep(1) with usleep_range

[Jacopo Mondi]

msleep() can sleep up to 20ms.

As suggested by Documentation/timers/timers_howto.txt replace it with
usleep_range() with up to 5ms delay.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/tw9910.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/tw9910.c b/drivers/media/i2c/tw9910.c
index 9c12bda..6f2f1d7 100644
--- a/drivers/media/i2c/tw9910.c
+++ b/drivers/media/i2c/tw9910.c
@@ -401,7 +401,7 @@ static int tw9910_set_hsync(struct i2c_client *client)
 static void tw9910_reset(struct i2c_client *client)
 {
 	tw9910_mask_set(client, ACNTL1, SRESET, SRESET);
-	msleep(1);
+	usleep_range(1000, 5000);
 }
 
 static int tw9910_power(struct i2c_client *client, int enable)
-- 
2.7.4

[PATCH v2 06/11] media: ov772x: Align function parameters

[Jacopo Mondi]

Align all function parameters to first open brace when declaring
functions.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/ov772x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 16665af..a418455 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1064,7 +1064,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 
 static int ov772x_get_selection(struct v4l2_subdev *sd,
 				struct v4l2_subdev_pad_config *cfg,
-		struct v4l2_subdev_selection *sel)
+				struct v4l2_subdev_selection *sel)
 {
 	struct ov772x_priv *priv = to_ov772x(sd);
 
@@ -1087,7 +1087,7 @@ static int ov772x_get_selection(struct v4l2_subdev *sd,
 
 static int ov772x_get_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_pad_config *cfg,
-		struct v4l2_subdev_format *format)
+			  struct v4l2_subdev_format *format)
 {
 	struct v4l2_mbus_framefmt *mf = &format->format;
 	struct ov772x_priv *priv = to_ov772x(sd);
@@ -1106,7 +1106,7 @@ static int ov772x_get_fmt(struct v4l2_subdev *sd,
 
 static int ov772x_set_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_pad_config *cfg,
-		struct v4l2_subdev_format *format)
+			  struct v4l2_subdev_format *format)
 {
 	struct ov772x_priv *priv = to_ov772x(sd);
 	struct v4l2_mbus_framefmt *mf = &format->format;
@@ -1219,7 +1219,7 @@ static int ov772x_enum_frame_interval(struct v4l2_subdev *sd,
 
 static int ov772x_enum_mbus_code(struct v4l2_subdev *sd,
 				 struct v4l2_subdev_pad_config *cfg,
-		struct v4l2_subdev_mbus_code_enum *code)
+				 struct v4l2_subdev_mbus_code_enum *code)
 {
 	if (code->pad || code->index >= ARRAY_SIZE(ov772x_cfmts))
 		return -EINVAL;
-- 
2.7.4

[PATCH v2 07/11] media: ov772x: Re-organize in-code comments

[Jacopo Mondi]

A lot of comments that would fit a single line were spread on two or
more lines. Also fix capitalization and punctuation where appropriate.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/ov772x.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index a418455..8849da1 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -910,17 +910,13 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 	int ret;
 	u8  val;
 
-	/*
-	 * reset hardware
-	 */
+	/* Reset hardware. */
 	ov772x_reset(client);
 
-	/*
-	 * Edge Ctrl
-	 */
+	/* Edge Ctrl. */
 	if (priv->info->edgectrl.strength & OV772X_MANUAL_EDGE_CTRL) {
 		/*
-		 * Manual Edge Control Mode
+		 * Manual Edge Control Mode.
 		 *
 		 * Edge auto strength bit is set by default.
 		 * Remove it when manual mode.
@@ -944,9 +940,9 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 
 	} else if (priv->info->edgectrl.upper > priv->info->edgectrl.lower) {
 		/*
-		 * Auto Edge Control Mode
+		 * Auto Edge Control Mode.
 		 *
-		 * set upper and lower limit
+		 * Set upper and lower limit.
 		 */
 		ret = ov772x_mask_set(client,
 				      EDGE_UPPER, OV772X_EDGE_UPPER_MASK,
@@ -961,7 +957,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 			goto ov772x_set_fmt_error;
 	}
 
-	/* Format and window size */
+	/* Format and window size. */
 	ret = ov772x_write(client, HSTART, win->rect.left >> 2);
 	if (ret < 0)
 		goto ov772x_set_fmt_error;
@@ -993,9 +989,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 	if (ret < 0)
 		goto ov772x_set_fmt_error;
 
-	/*
-	 * set DSP_CTRL3
-	 */
+	/* Set DSP_CTRL3. */
 	val = cfmt->dsp3;
 	if (val) {
 		ret = ov772x_mask_set(client,
@@ -1011,9 +1005,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 			goto ov772x_set_fmt_error;
 	}
 
-	/*
-	 * set COM3
-	 */
+	/* Set COM3. */
 	val = cfmt->com3;
 	if (priv->info->flags & OV772X_FLAG_VFLIP)
 		val |= VFLIP_IMG;
@@ -1041,9 +1033,7 @@ static int ov772x_set_params(struct ov772x_priv *priv,
 	if (ret < 0)
 		goto ov772x_set_fmt_error;
 
-	/*
-	 * set COM8
-	 */
+	/* Set COM8. */
 	if (priv->band_filter) {
 		ret = ov772x_mask_set(client, COM8, BNDF_ON_OFF, 1);
 		if (!ret)
@@ -1153,9 +1143,7 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
 	if (ret < 0)
 		return ret;
 
-	/*
-	 * check and show product ID and manufacturer ID
-	 */
+	/* Check and show product ID and manufacturer ID. */
 	pid = ov772x_read(client, PID);
 	ver = ov772x_read(client, VER);
 
-- 
2.7.4

[PATCH v2 08/11] media: ov772x: Empty line before end-of-function return

[Jacopo Mondi]

Add an empty line before return at the end of functions.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/ov772x.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 8849da1..4f464ac 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1129,6 +1129,7 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
 
 	priv->win = win;
 	priv->cfmt = cfmt;
+
 	return 0;
 }
 
@@ -1172,6 +1173,7 @@ static int ov772x_video_probe(struct ov772x_priv *priv)
 
 done:
 	ov772x_s_power(&priv->subdev, 0);
+
 	return ret;
 }
 
@@ -1213,6 +1215,7 @@ static int ov772x_enum_mbus_code(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	code->code = ov772x_cfmts[code->index].code;
+
 	return 0;
 }
 
@@ -1327,6 +1330,7 @@ static int ov772x_remove(struct i2c_client *client)
 		gpiod_put(priv->pwdn_gpio);
 	v4l2_device_unregister_subdev(&priv->subdev);
 	v4l2_ctrl_handler_free(&priv->hdl);
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v2 09/11] media: ov772x: Re-order variables declaration

[Jacopo Mondi]

Re-order variables declaration to respect 'reverse christmas tree'
ordering whenever possible.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/ov772x.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 4f464ac..1fd6d4b 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1098,8 +1098,8 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
 			  struct v4l2_subdev_pad_config *cfg,
 			  struct v4l2_subdev_format *format)
 {
-	struct ov772x_priv *priv = to_ov772x(sd);
 	struct v4l2_mbus_framefmt *mf = &format->format;
+	struct ov772x_priv *priv = to_ov772x(sd);
 	const struct ov772x_color_format *cfmt;
 	const struct ov772x_win_size *win;
 	int ret;
@@ -1135,10 +1135,11 @@ static int ov772x_set_fmt(struct v4l2_subdev *sd,
 
 static int ov772x_video_probe(struct ov772x_priv *priv)
 {
-	struct i2c_client  *client = v4l2_get_subdevdata(&priv->subdev);
-	u8                  pid, ver;
-	const char         *devname;
-	int		    ret;
+	struct i2c_client *client = v4l2_get_subdevdata(&priv->subdev);
+	const char *devname;
+	int ret;
+	u8 pid;
+	u8 ver;
 
 	ret = ov772x_s_power(&priv->subdev, 1);
 	if (ret < 0)
@@ -1246,9 +1247,9 @@ static const struct v4l2_subdev_ops ov772x_subdev_ops = {
 static int ov772x_probe(struct i2c_client *client,
 			const struct i2c_device_id *did)
 {
-	struct ov772x_priv	*priv;
-	struct i2c_adapter	*adapter = client->adapter;
-	int			ret;
+	struct i2c_adapter *adapter = client->adapter;
+	struct ov772x_priv *priv;
+	int ret;
 
 	if (!client->dev.platform_data) {
 		dev_err(&client->dev, "Missing ov772x platform data\n");
-- 
2.7.4

[PATCH v2 10/11] media: ov772x: Replace msleep(1) with usleep_range

[Jacopo Mondi]

msleep() can sleep up to 20ms.

As suggested by Documentation/timers/timers_howto.txt replace it with
usleep_range() with up to 5ms delay.

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/ov772x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 1fd6d4b..2d5281a 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -574,7 +574,7 @@ static int ov772x_reset(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	msleep(1);
+	usleep_range(1000, 5000);
 
 	return ov772x_mask_set(client, COM2, SOFT_SLEEP_MODE, SOFT_SLEEP_MODE);
 }
-- 
2.7.4

[PATCH v2 11/11] media: ov772x: Unregister async subdevice

[Jacopo Mondi]

As the media subdevice is registered with 'v4l2_async_register_subdev()'
unregister it at module removal time with
'v4l2_async_unregister_subdev()'

Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
 drivers/media/i2c/ov772x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov772x.c b/drivers/media/i2c/ov772x.c
index 2d5281a..3d1ea58 100644
--- a/drivers/media/i2c/ov772x.c
+++ b/drivers/media/i2c/ov772x.c
@@ -1329,7 +1329,7 @@ static int ov772x_remove(struct i2c_client *client)
 	clk_put(priv->clk);
 	if (priv->pwdn_gpio)
 		gpiod_put(priv->pwdn_gpio);
-	v4l2_device_unregister_subdev(&priv->subdev);
+	v4l2_async_unregister_subdev(&priv->subdev);
 	v4l2_ctrl_handler_free(&priv->hdl);
 
 	return 0;
-- 
2.7.4