Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Tue, Dec 19, 2017 at 12:31:46PM -0200, Fabio Estevam wrote:
> On Tue, Dec 19, 2017 at 11:43 AM, Sakari Ailus  wrote:
> 
> > Both seem to exist. See e.g. c3a3d1d6b8b363a02234e5564692db3647f183e6 .
> 
> This patch fixes .h files to use /* SPDX style comment, which is the
> recommendation.
> 
> .c files should use // SPDX style.

Agreed. I reverted the changes.

Thanks.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Tue, Dec 19, 2017 at 12:31:46PM -0200, Fabio Estevam wrote:
> On Tue, Dec 19, 2017 at 11:43 AM, Sakari Ailus  wrote:
> 
> > Both seem to exist. See e.g. c3a3d1d6b8b363a02234e5564692db3647f183e6 .
> 
> This patch fixes .h files to use /* SPDX style comment, which is the
> recommendation.
> 
> .c files should use // SPDX style.

Agreed. I reverted the changes.

Thanks.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

On Tue, Dec 19, 2017 at 11:43 AM, Sakari Ailus  wrote:

> Both seem to exist. See e.g. c3a3d1d6b8b363a02234e5564692db3647f183e6 .

This patch fixes .h files to use /* SPDX style comment, which is the
recommendation.

.c files should use // SPDX style.

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

On Tue, Dec 19, 2017 at 11:43 AM, Sakari Ailus  wrote:

> Both seem to exist. See e.g. c3a3d1d6b8b363a02234e5564692db3647f183e6 .

This patch fixes .h files to use /* SPDX style comment, which is the
recommendation.

.c files should use // SPDX style.

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Tue, Dec 19, 2017 at 11:19:06AM -0200, Fabio Estevam wrote:
> Hi Sakari,
> 
> On Tue, Dec 19, 2017 at 11:05 AM, Sakari Ailus  wrote:
> 
> > I guess it depends on who do you ask and when. Looking at what has been
> > recently merged to media tree master, the latter is preferred.
> 
> Just did 'git grep SPDX drivers/media'
> 
> and it consistently shows // SPDX style for C files.

Both seem to exist. See e.g. c3a3d1d6b8b363a02234e5564692db3647f183e6 .

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Tue, Dec 19, 2017 at 11:19:06AM -0200, Fabio Estevam wrote:
> Hi Sakari,
> 
> On Tue, Dec 19, 2017 at 11:05 AM, Sakari Ailus  wrote:
> 
> > I guess it depends on who do you ask and when. Looking at what has been
> > recently merged to media tree master, the latter is preferred.
> 
> Just did 'git grep SPDX drivers/media'
> 
> and it consistently shows // SPDX style for C files.

Both seem to exist. See e.g. c3a3d1d6b8b363a02234e5564692db3647f183e6 .

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

Hi Sakari,

On Tue, Dec 19, 2017 at 11:05 AM, Sakari Ailus  wrote:

> I guess it depends on who do you ask and when. Looking at what has been
> recently merged to media tree master, the latter is preferred.

Just did 'git grep SPDX drivers/media'

and it consistently shows // SPDX style for C files.

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

Hi Sakari,

On Tue, Dec 19, 2017 at 11:05 AM, Sakari Ailus  wrote:

> I guess it depends on who do you ask and when. Looking at what has been
> recently merged to media tree master, the latter is preferred.

Just did 'git grep SPDX drivers/media'

and it consistently shows // SPDX style for C files.

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Tue, Dec 19, 2017 at 10:50:44AM -0200, Fabio Estevam wrote:
> Hi Sakari,
> 
> On Tue, Dec 19, 2017 at 7:22 AM, Sakari Ailus  wrote:
> > On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
> >> The ov7740 (color) image sensor is a high performance VGA CMOS
> >> image snesor, which supports for output formats: RAW RGB and YUV
> >> and image sizes: VGA, and QVGA, CIF and any size smaller.
> >>
> >> Signed-off-by: Songjun Wu 
> >> Signed-off-by: Wenyou Yang 
> >
> > Applied with this diff:
> >
> > diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> > index 0308ba437bbb..041a77039d70 100644
> > --- a/drivers/media/i2c/ov7740.c
> > +++ b/drivers/media/i2c/ov7740.c
> > @@ -1,5 +1,7 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -// Copyright (c) 2017 Microchip Corporation.
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0
> > + * Copyright (c) 2017 Microchip Corporation.
> > + */
> 
> The original version is the recommended format for the SPDX identifier.

I guess it depends on who do you ask and when. Looking at what has been
recently merged to media tree master, the latter is preferred.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Tue, Dec 19, 2017 at 10:50:44AM -0200, Fabio Estevam wrote:
> Hi Sakari,
> 
> On Tue, Dec 19, 2017 at 7:22 AM, Sakari Ailus  wrote:
> > On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
> >> The ov7740 (color) image sensor is a high performance VGA CMOS
> >> image snesor, which supports for output formats: RAW RGB and YUV
> >> and image sizes: VGA, and QVGA, CIF and any size smaller.
> >>
> >> Signed-off-by: Songjun Wu 
> >> Signed-off-by: Wenyou Yang 
> >
> > Applied with this diff:
> >
> > diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> > index 0308ba437bbb..041a77039d70 100644
> > --- a/drivers/media/i2c/ov7740.c
> > +++ b/drivers/media/i2c/ov7740.c
> > @@ -1,5 +1,7 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -// Copyright (c) 2017 Microchip Corporation.
> > +/*
> > + * SPDX-License-Identifier: GPL-2.0
> > + * Copyright (c) 2017 Microchip Corporation.
> > + */
> 
> The original version is the recommended format for the SPDX identifier.

I guess it depends on who do you ask and when. Looking at what has been
recently merged to media tree master, the latter is preferred.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

Hi Sakari,

On Tue, Dec 19, 2017 at 7:22 AM, Sakari Ailus  wrote:
> On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
>> The ov7740 (color) image sensor is a high performance VGA CMOS
>> image snesor, which supports for output formats: RAW RGB and YUV
>> and image sizes: VGA, and QVGA, CIF and any size smaller.
>>
>> Signed-off-by: Songjun Wu 
>> Signed-off-by: Wenyou Yang 
>
> Applied with this diff:
>
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> index 0308ba437bbb..041a77039d70 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -1,5 +1,7 @@
> -// SPDX-License-Identifier: GPL-2.0
> -// Copyright (c) 2017 Microchip Corporation.
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + * Copyright (c) 2017 Microchip Corporation.
> + */

The original version is the recommended format for the SPDX identifier.

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Fabio Estevam]

Hi Sakari,

On Tue, Dec 19, 2017 at 7:22 AM, Sakari Ailus  wrote:
> On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
>> The ov7740 (color) image sensor is a high performance VGA CMOS
>> image snesor, which supports for output formats: RAW RGB and YUV
>> and image sizes: VGA, and QVGA, CIF and any size smaller.
>>
>> Signed-off-by: Songjun Wu 
>> Signed-off-by: Wenyou Yang 
>
> Applied with this diff:
>
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
> index 0308ba437bbb..041a77039d70 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -1,5 +1,7 @@
> -// SPDX-License-Identifier: GPL-2.0
> -// Copyright (c) 2017 Microchip Corporation.
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + * Copyright (c) 2017 Microchip Corporation.
> + */

The original version is the recommended format for the SPDX identifier.

RE: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Wenyou.Yang]

Hi Sakari,

> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
> Sent: 2017年12月19日 17:23
> To: Wenyou Yang - A41535 <wenyou.y...@microchip.com>
> Cc: Mauro Carvalho Chehab <mche...@s-opensource.com>; Rob Herring
> <robh...@kernel.org>; Mark Rutland <mark.rutl...@arm.com>; linux-
> ker...@vger.kernel.org; Nicolas Ferre - M43238 <nicolas.fe...@microchip.com>;
> devicet...@vger.kernel.org; Jonathan Corbet <cor...@lwn.net>; Hans Verkuil
> <hverk...@xs4all.nl>; linux-arm-ker...@lists.infradead.org; Linux Media 
> Mailing List
> <linux-me...@vger.kernel.org>; Songjun Wu <songjun...@microchip.com>
> Subject: Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver
> 
> On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
> > The ov7740 (color) image sensor is a high performance VGA CMOS image
> > snesor, which supports for output formats: RAW RGB and YUV and image
> > sizes: VGA, and QVGA, CIF and any size smaller.
> >
> > Signed-off-by: Songjun Wu <songjun...@microchip.com>
> > Signed-off-by: Wenyou Yang <wenyou.y...@microchip.com>
> 
> Applied with this diff:

Thank you very much.

> 
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c index
> 0308ba437bbb..041a77039d70 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -1,5 +1,7 @@
> -// SPDX-License-Identifier: GPL-2.0
> -// Copyright (c) 2017 Microchip Corporation.
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + * Copyright (c) 2017 Microchip Corporation.
> + */
> 
>  #include 
>  #include 
> 
> --
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi


Best Regards,
Wenyou Yang

RE: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Wenyou.Yang]

Hi Sakari,

> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
> Sent: 2017年12月19日 17:23
> To: Wenyou Yang - A41535 
> Cc: Mauro Carvalho Chehab ; Rob Herring
> ; Mark Rutland ; linux-
> ker...@vger.kernel.org; Nicolas Ferre - M43238 ;
> devicet...@vger.kernel.org; Jonathan Corbet ; Hans Verkuil
> ; linux-arm-ker...@lists.infradead.org; Linux Media 
> Mailing List
> ; Songjun Wu 
> Subject: Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver
> 
> On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
> > The ov7740 (color) image sensor is a high performance VGA CMOS image
> > snesor, which supports for output formats: RAW RGB and YUV and image
> > sizes: VGA, and QVGA, CIF and any size smaller.
> >
> > Signed-off-by: Songjun Wu 
> > Signed-off-by: Wenyou Yang 
> 
> Applied with this diff:

Thank you very much.

> 
> diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c index
> 0308ba437bbb..041a77039d70 100644
> --- a/drivers/media/i2c/ov7740.c
> +++ b/drivers/media/i2c/ov7740.c
> @@ -1,5 +1,7 @@
> -// SPDX-License-Identifier: GPL-2.0
> -// Copyright (c) 2017 Microchip Corporation.
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + * Copyright (c) 2017 Microchip Corporation.
> + */
> 
>  #include 
>  #include 
> 
> --
> Sakari Ailus
> e-mail: sakari.ai...@iki.fi


Best Regards,
Wenyou Yang

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
> The ov7740 (color) image sensor is a high performance VGA CMOS
> image snesor, which supports for output formats: RAW RGB and YUV
> and image sizes: VGA, and QVGA, CIF and any size smaller.
> 
> Signed-off-by: Songjun Wu 
> Signed-off-by: Wenyou Yang 

Applied with this diff:

diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 0308ba437bbb..041a77039d70 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -1,5 +1,7 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (c) 2017 Microchip Corporation.
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ * Copyright (c) 2017 Microchip Corporation.
+ */
 
 #include 
 #include 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

On Mon, Dec 11, 2017 at 09:31:46AM +0800, Wenyou Yang wrote:
> The ov7740 (color) image sensor is a high performance VGA CMOS
> image snesor, which supports for output formats: RAW RGB and YUV
> and image sizes: VGA, and QVGA, CIF and any size smaller.
> 
> Signed-off-by: Songjun Wu 
> Signed-off-by: Wenyou Yang 

Applied with this diff:

diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index 0308ba437bbb..041a77039d70 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -1,5 +1,7 @@
-// SPDX-License-Identifier: GPL-2.0
-// Copyright (c) 2017 Microchip Corporation.
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ * Copyright (c) 2017 Microchip Corporation.
+ */
 
 #include 
 #include 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

Hi Wenyou,

On Tue, Dec 19, 2017 at 02:11:28AM +, wenyou.y...@microchip.com wrote:
> Hi Sakari,
> 
> > -Original Message-
> > From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
> > Sent: 2017年12月14日 4:06
> > To: Wenyou Yang - A41535 <wenyou.y...@microchip.com>; Mauro Carvalho
> > Chehab <mche...@s-opensource.com>; Rob Herring <robh...@kernel.org>;
> > Mark Rutland <mark.rutl...@arm.com>
> > Cc: linux-kernel@vger.kernel.org; Nicolas Ferre - M43238
> > <nicolas.fe...@microchip.com>; devicet...@vger.kernel.org; Jonathan Corbet
> > <cor...@lwn.net>; Hans Verkuil <hverk...@xs4all.nl>; linux-arm-
> > ker...@lists.infradead.org; Linux Media Mailing List  > me...@vger.kernel.org>; Songjun Wu <songjun...@microchip.com>
> > Subject: Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver
> > 
> > Hi Wenyou,
> > 
> > Wenyou Yang wrote:
> > ...
> > > +static int ov7740_start_streaming(struct ov7740 *ov7740) {
> > > + int ret;
> > > +
> > > + if (ov7740->fmt) {
> > > + ret = regmap_multi_reg_write(ov7740->regmap,
> > > +  ov7740->fmt->regs,
> > > +  ov7740->fmt->reg_num);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + if (ov7740->frmsize) {
> > > + ret = regmap_multi_reg_write(ov7740->regmap,
> > > +  ov7740->frmsize->regs,
> > > +  ov7740->frmsize->reg_num);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return __v4l2_ctrl_handler_setup(ov7740->subdev.ctrl_handler);
> > 
> > I believe you're still setting the controls after starting streaming.
> 
> Yes, it sees it does so.
> 
> The OV7740 sensor generates the stream pixel data at the constant frame
> rate, no such start or stop control.

That'd be odd but I guess hardware sometimes is. I'll apply the patches.

Thanks!

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

Hi Wenyou,

On Tue, Dec 19, 2017 at 02:11:28AM +, wenyou.y...@microchip.com wrote:
> Hi Sakari,
> 
> > -Original Message-
> > From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
> > Sent: 2017年12月14日 4:06
> > To: Wenyou Yang - A41535 ; Mauro Carvalho
> > Chehab ; Rob Herring ;
> > Mark Rutland 
> > Cc: linux-kernel@vger.kernel.org; Nicolas Ferre - M43238
> > ; devicet...@vger.kernel.org; Jonathan Corbet
> > ; Hans Verkuil ; linux-arm-
> > ker...@lists.infradead.org; Linux Media Mailing List  > me...@vger.kernel.org>; Songjun Wu 
> > Subject: Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver
> > 
> > Hi Wenyou,
> > 
> > Wenyou Yang wrote:
> > ...
> > > +static int ov7740_start_streaming(struct ov7740 *ov7740) {
> > > + int ret;
> > > +
> > > + if (ov7740->fmt) {
> > > + ret = regmap_multi_reg_write(ov7740->regmap,
> > > +  ov7740->fmt->regs,
> > > +  ov7740->fmt->reg_num);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + if (ov7740->frmsize) {
> > > + ret = regmap_multi_reg_write(ov7740->regmap,
> > > +  ov7740->frmsize->regs,
> > > +  ov7740->frmsize->reg_num);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + return __v4l2_ctrl_handler_setup(ov7740->subdev.ctrl_handler);
> > 
> > I believe you're still setting the controls after starting streaming.
> 
> Yes, it sees it does so.
> 
> The OV7740 sensor generates the stream pixel data at the constant frame
> rate, no such start or stop control.

That'd be odd but I guess hardware sometimes is. I'll apply the patches.

Thanks!

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

RE: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Wenyou.Yang]

Hi Sakari,

> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
> Sent: 2017年12月14日 4:06
> To: Wenyou Yang - A41535 <wenyou.y...@microchip.com>; Mauro Carvalho
> Chehab <mche...@s-opensource.com>; Rob Herring <robh...@kernel.org>;
> Mark Rutland <mark.rutl...@arm.com>
> Cc: linux-kernel@vger.kernel.org; Nicolas Ferre - M43238
> <nicolas.fe...@microchip.com>; devicet...@vger.kernel.org; Jonathan Corbet
> <cor...@lwn.net>; Hans Verkuil <hverk...@xs4all.nl>; linux-arm-
> ker...@lists.infradead.org; Linux Media Mailing List  me...@vger.kernel.org>; Songjun Wu <songjun...@microchip.com>
> Subject: Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver
> 
> Hi Wenyou,
> 
> Wenyou Yang wrote:
> ...
> > +static int ov7740_start_streaming(struct ov7740 *ov7740) {
> > +   int ret;
> > +
> > +   if (ov7740->fmt) {
> > +   ret = regmap_multi_reg_write(ov7740->regmap,
> > +ov7740->fmt->regs,
> > +ov7740->fmt->reg_num);
> > +   if (ret)
> > +   return ret;
> > +   }
> > +
> > +   if (ov7740->frmsize) {
> > +   ret = regmap_multi_reg_write(ov7740->regmap,
> > +ov7740->frmsize->regs,
> > +ov7740->frmsize->reg_num);
> > +   if (ret)
> > +   return ret;
> > +   }
> > +
> > +   return __v4l2_ctrl_handler_setup(ov7740->subdev.ctrl_handler);
> 
> I believe you're still setting the controls after starting streaming.

Yes, it sees it does so.

The OV7740 sensor generates the stream pixel data at the constant frame rate, 
no such start or stop control. 

> 
> --
> Sakari Ailus
> sakari.ai...@iki.fi

Wenyou Yang


imx7d-sdb.dtb
Description: imx7d-sdb.dtb

RE: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Wenyou.Yang]

Hi Sakari,

> -Original Message-
> From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
> Sent: 2017年12月14日 4:06
> To: Wenyou Yang - A41535 ; Mauro Carvalho
> Chehab ; Rob Herring ;
> Mark Rutland 
> Cc: linux-kernel@vger.kernel.org; Nicolas Ferre - M43238
> ; devicet...@vger.kernel.org; Jonathan Corbet
> ; Hans Verkuil ; linux-arm-
> ker...@lists.infradead.org; Linux Media Mailing List  me...@vger.kernel.org>; Songjun Wu 
> Subject: Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver
> 
> Hi Wenyou,
> 
> Wenyou Yang wrote:
> ...
> > +static int ov7740_start_streaming(struct ov7740 *ov7740) {
> > +   int ret;
> > +
> > +   if (ov7740->fmt) {
> > +   ret = regmap_multi_reg_write(ov7740->regmap,
> > +ov7740->fmt->regs,
> > +ov7740->fmt->reg_num);
> > +   if (ret)
> > +   return ret;
> > +   }
> > +
> > +   if (ov7740->frmsize) {
> > +   ret = regmap_multi_reg_write(ov7740->regmap,
> > +ov7740->frmsize->regs,
> > +ov7740->frmsize->reg_num);
> > +   if (ret)
> > +   return ret;
> > +   }
> > +
> > +   return __v4l2_ctrl_handler_setup(ov7740->subdev.ctrl_handler);
> 
> I believe you're still setting the controls after starting streaming.

Yes, it sees it does so.

The OV7740 sensor generates the stream pixel data at the constant frame rate, 
no such start or stop control. 

> 
> --
> Sakari Ailus
> sakari.ai...@iki.fi

Wenyou Yang


imx7d-sdb.dtb
Description: imx7d-sdb.dtb

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

Hi Wenyou,

Wenyou Yang wrote:
...
> +static int ov7740_start_streaming(struct ov7740 *ov7740)
> +{
> + int ret;
> +
> + if (ov7740->fmt) {
> + ret = regmap_multi_reg_write(ov7740->regmap,
> +  ov7740->fmt->regs,
> +  ov7740->fmt->reg_num);
> + if (ret)
> + return ret;
> + }
> +
> + if (ov7740->frmsize) {
> + ret = regmap_multi_reg_write(ov7740->regmap,
> +  ov7740->frmsize->regs,
> +  ov7740->frmsize->reg_num);
> + if (ret)
> + return ret;
> + }
> +
> + return __v4l2_ctrl_handler_setup(ov7740->subdev.ctrl_handler);

I believe you're still setting the controls after starting streaming.

-- 
Sakari Ailus
sakari.ai...@iki.fi

Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Sakari Ailus]

Hi Wenyou,

Wenyou Yang wrote:
...
> +static int ov7740_start_streaming(struct ov7740 *ov7740)
> +{
> + int ret;
> +
> + if (ov7740->fmt) {
> + ret = regmap_multi_reg_write(ov7740->regmap,
> +  ov7740->fmt->regs,
> +  ov7740->fmt->reg_num);
> + if (ret)
> + return ret;
> + }
> +
> + if (ov7740->frmsize) {
> + ret = regmap_multi_reg_write(ov7740->regmap,
> +  ov7740->frmsize->regs,
> +  ov7740->frmsize->reg_num);
> + if (ret)
> + return ret;
> + }
> +
> + return __v4l2_ctrl_handler_setup(ov7740->subdev.ctrl_handler);

I believe you're still setting the controls after starting streaming.

-- 
Sakari Ailus
sakari.ai...@iki.fi

[PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Wenyou Yang]

The ov7740 (color) image sensor is a high performance VGA CMOS
image snesor, which supports for output formats: RAW RGB and YUV
and image sizes: VGA, and QVGA, CIF and any size smaller.

Signed-off-by: Songjun Wu 
Signed-off-by: Wenyou Yang 
---

Changes in v9:
 - Use the new SPDX ids.

Changes in v8:
 - As the registers are written at stream start, remove the written
   code from the set fmt function.

Changes in v7:
 - Fix the wrong handle of default register configuration.
 - Add the missed assignment of ov7740->frmsize.

Changes in v6:
 - Remove unnecessary #include .
 - Remove unnecessary comments and extra newline.
 - Add const for some structures.
 - Add the check of the return value from regmap_write().
 - Simplify the calling of __v4l2_ctrl_handler_setup().
 - Add the default format initialization function.
 - Integrate the set_power() and enable/disable the clock into
   one function.

Changes in v5:
 - Squash the driver and MAINTAINERS entry patches to one.
 - Precede the driver patch with the bindings patch.

Changes in v4:
 - Assign 'val' a initial value to avoid warning: 'val' may be
   used uninitialized.
 - Rename REG_REG15 to avoid warning: "REG_REG15" redefined.

Changes in v3:
 - Put the MAINTAINERS change to a separate patch.

Changes in v2:
 - Split off the bindings into a separate patch.
 - Add a new entry to the MAINTAINERS file.

 MAINTAINERS|8 +
 drivers/media/i2c/Kconfig  |8 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov7740.c | 1216 
 4 files changed, 1233 insertions(+)
 create mode 100644 drivers/media/i2c/ov7740.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a52a66aa991..1de965009b13 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10053,6 +10053,14 @@ S: Maintained
 F: drivers/media/i2c/ov7670.c
 F: Documentation/devicetree/bindings/media/i2c/ov7670.txt
 
+OMNIVISION OV7740 SENSOR DRIVER
+M: Wenyou Yang 
+L: linux-me...@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ov7740.c
+F: Documentation/devicetree/bindings/media/i2c/ov7740.txt
+
 ONENAND FLASH DRIVER
 M: Kyungmin Park 
 L: linux-...@lists.infradead.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff82915..00b1c4c031d4 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -665,6 +665,14 @@ config VIDEO_OV7670
  OV7670 VGA camera.  It currently only works with the M88ALP01
  controller.
 
+config VIDEO_OV7740
+   tristate "OmniVision OV7740 sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV7740 VGA camera sensor.
+
 config VIDEO_OV9650
tristate "OmniVision OV9650/OV9652 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9efce966..9b19ec7fcaf4 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
+obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
 obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
 obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
new file mode 100644
index ..0308ba437bbb
--- /dev/null
+++ b/drivers/media/i2c/ov7740.c
@@ -0,0 +1,1216 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Microchip Corporation.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define REG_OUTSIZE_LSB 0x34
+
+/* OV7740 register tables */
+#define REG_GAIN   0x00/* Gain lower 8 bits (rest in vref) */
+#define REG_BGAIN  0x01/* blue gain */
+#define REG_RGAIN  0x02/* red gain */
+#define REG_GGAIN  0x03/* green gain */
+#define REG_REG04  0x04/* analog setting, dont change*/
+#define REG_BAVG   0x05/* b channel average */
+#define REG_GAVG   0x06/* g channel average */
+#define REG_RAVG   0x07/* r channel average */
+
+#define REG_REG0C  0x0C/* filp enable */
+#define REG0C_IMG_FLIP 0x80
+#define REG0C_IMG_MIRROR   0x40
+
+#define REG_REG0E  0x0E/* blc line */
+#define REG_HAEC   0x0F/* auto exposure cntrl */
+#define REG_AEC0x10/* auto exposure cntrl */
+
+#define REG_CLK0x11/* Clock control */
+#define REG_REG55  0x55/* Clock PLL DIV/PreDiv */
+
+#define REG_REG12  0x12
+
+#define REG_REG13  0x13/* 

[PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver

[Wenyou Yang]

The ov7740 (color) image sensor is a high performance VGA CMOS
image snesor, which supports for output formats: RAW RGB and YUV
and image sizes: VGA, and QVGA, CIF and any size smaller.

Signed-off-by: Songjun Wu 
Signed-off-by: Wenyou Yang 
---

Changes in v9:
 - Use the new SPDX ids.

Changes in v8:
 - As the registers are written at stream start, remove the written
   code from the set fmt function.

Changes in v7:
 - Fix the wrong handle of default register configuration.
 - Add the missed assignment of ov7740->frmsize.

Changes in v6:
 - Remove unnecessary #include .
 - Remove unnecessary comments and extra newline.
 - Add const for some structures.
 - Add the check of the return value from regmap_write().
 - Simplify the calling of __v4l2_ctrl_handler_setup().
 - Add the default format initialization function.
 - Integrate the set_power() and enable/disable the clock into
   one function.

Changes in v5:
 - Squash the driver and MAINTAINERS entry patches to one.
 - Precede the driver patch with the bindings patch.

Changes in v4:
 - Assign 'val' a initial value to avoid warning: 'val' may be
   used uninitialized.
 - Rename REG_REG15 to avoid warning: "REG_REG15" redefined.

Changes in v3:
 - Put the MAINTAINERS change to a separate patch.

Changes in v2:
 - Split off the bindings into a separate patch.
 - Add a new entry to the MAINTAINERS file.

 MAINTAINERS|8 +
 drivers/media/i2c/Kconfig  |8 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov7740.c | 1216 
 4 files changed, 1233 insertions(+)
 create mode 100644 drivers/media/i2c/ov7740.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7a52a66aa991..1de965009b13 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10053,6 +10053,14 @@ S: Maintained
 F: drivers/media/i2c/ov7670.c
 F: Documentation/devicetree/bindings/media/i2c/ov7670.txt
 
+OMNIVISION OV7740 SENSOR DRIVER
+M: Wenyou Yang 
+L: linux-me...@vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+S: Maintained
+F: drivers/media/i2c/ov7740.c
+F: Documentation/devicetree/bindings/media/i2c/ov7740.txt
+
 ONENAND FLASH DRIVER
 M: Kyungmin Park 
 L: linux-...@lists.infradead.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index cb5d7ff82915..00b1c4c031d4 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -665,6 +665,14 @@ config VIDEO_OV7670
  OV7670 VGA camera.  It currently only works with the M88ALP01
  controller.
 
+config VIDEO_OV7740
+   tristate "OmniVision OV7740 sensor support"
+   depends on I2C && VIDEO_V4L2
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV7740 VGA camera sensor.
+
 config VIDEO_OV9650
tristate "OmniVision OV9650/OV9652 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 548a9efce966..9b19ec7fcaf4 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -68,6 +68,7 @@ obj-$(CONFIG_VIDEO_OV5670) += ov5670.o
 obj-$(CONFIG_VIDEO_OV6650) += ov6650.o
 obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
 obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
+obj-$(CONFIG_VIDEO_OV7740) += ov7740.o
 obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
 obj-$(CONFIG_VIDEO_OV13858) += ov13858.o
 obj-$(CONFIG_VIDEO_MT9M032) += mt9m032.o
diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
new file mode 100644
index ..0308ba437bbb
--- /dev/null
+++ b/drivers/media/i2c/ov7740.c
@@ -0,0 +1,1216 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2017 Microchip Corporation.
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define REG_OUTSIZE_LSB 0x34
+
+/* OV7740 register tables */
+#define REG_GAIN   0x00/* Gain lower 8 bits (rest in vref) */
+#define REG_BGAIN  0x01/* blue gain */
+#define REG_RGAIN  0x02/* red gain */
+#define REG_GGAIN  0x03/* green gain */
+#define REG_REG04  0x04/* analog setting, dont change*/
+#define REG_BAVG   0x05/* b channel average */
+#define REG_GAVG   0x06/* g channel average */
+#define REG_RAVG   0x07/* r channel average */
+
+#define REG_REG0C  0x0C/* filp enable */
+#define REG0C_IMG_FLIP 0x80
+#define REG0C_IMG_MIRROR   0x40
+
+#define REG_REG0E  0x0E/* blc line */
+#define REG_HAEC   0x0F/* auto exposure cntrl */
+#define REG_AEC0x10/* auto exposure cntrl */
+
+#define REG_CLK0x11/* Clock control */
+#define REG_REG55  0x55/* Clock PLL DIV/PreDiv */
+
+#define REG_REG12  0x12
+
+#define REG_REG13  0x13/* auto/manual AGC, AEC, Write Balance*/
+#define REG13_AEC_EN   0x01
+#define REG13_AGC_EN   0x04
+
+#define