Re: [PATCH v9 2/2] media: i2c: Add the ov7740 image sensor driver
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
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
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
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
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
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
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
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
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
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
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
